From dc6d5c33d1e4e5aa4735b26395537616c2b4d578 Mon Sep 17 00:00:00 2001 From: humble-little-bear Date: Fri, 14 Aug 2026 11:11:33 +0800 Subject: [PATCH 1/2] fix: address CodeRabbit review findings on the v0.4.12 merge (#499) - Makefile: add pw-lock and secp256k1_multisig_v2 to .PHONY so make all cannot skip their recipes when same-named files or directories exist - README: list the install command in the Usage block - ckb-debugger-install: tighten legacy-shim detection so only the exact v0.4.x fallback body (shebang + exec offckb debugger) is upgraded; a foreign file that merely mentions offckb debugger is left untouched - proxy-events: isolate per-member failures in the JSON-RPC batch loop so one malformed send_transaction cannot prevent later requests from being recorded; add coverage for a failing member followed by a valid one --- Makefile | 2 +- README.md | 1 + src/tools/ckb-debugger-install.ts | 15 +++++++++++---- src/tools/proxy-events.ts | 6 +++++- tests/ckb-debugger-install.test.ts | 21 +++++++++++++++++++++ tests/proxy-events.test.ts | 27 +++++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 91da672..78f34ef 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: all omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock +.PHONY: all omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock pw-lock secp256k1_multisig_v2 all: omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock pw-lock secp256k1_multisig_v2 diff --git a/README.md b/README.md index 460ac86..5d99c06 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,7 @@ Commands: transfer [options] [toAddress] [amountInCKB] Transfer CKB tokens to address, only devnet and testnet transfer-all [options] [toAddress] Transfer All CKB tokens to address, only devnet and testnet balance [options] [toAddress] Check account balance, only devnet and testnet + install Install a tool binary used by offckb (e.g. the native ckb-debugger) debugger Port of the raw CKB Standalone Debugger status [options] Show ckb-tui status interface logs [options] [target] Show devnet logs: node (default), contract script debug output, miner, or RPC proxy events diff --git a/src/tools/ckb-debugger-install.ts b/src/tools/ckb-debugger-install.ts index 7da4260..bf96e40 100644 --- a/src/tools/ckb-debugger-install.ts +++ b/src/tools/ckb-debugger-install.ts @@ -351,9 +351,13 @@ export class CKBDebuggerInstaller { const binName = isWindows ? 'ckb-debugger.cmd' : 'ckb-debugger'; const targetPath = path.join(path.dirname(offckbPath), binName); - // The marker distinguishes our own shim (and legacy v0.4.x fallback shims - // that `exec offckb debugger`) from a foreign file — e.g. a real binary a - // user installed via cargo — which must never be silently clobbered. + // The marker distinguishes our own shim from a foreign file — e.g. a real + // binary a user installed via cargo — which must never be silently + // clobbered. Legacy v0.4.x fallback shims are also upgraded in place, but + // only when they match the exact body offckb wrote back then (a shebang + // line plus `exec offckb debugger "$@"` on Unix, `@echo off` plus + // `offckb debugger %*` on Windows); a mere mention of `offckb debugger` + // in an arbitrary user script is not enough to claim it. const marker = isWindows ? '@rem offckb-managed ckb-debugger shim' : '# offckb-managed ckb-debugger shim'; const content = isWindows ? `${marker}\r\n@echo off\r\n"${binaryPath}" %*\r\n` @@ -362,7 +366,10 @@ export class CKBDebuggerInstaller { try { if (fs.existsSync(targetPath)) { const existing = fs.readFileSync(targetPath, 'utf8'); - if (!existing.includes(marker) && !existing.includes('offckb debugger')) { + const isLegacyShim = isWindows + ? existing.includes('@echo off') && existing.includes('offckb debugger %*') + : existing.startsWith('#!/bin/sh') && existing.includes('exec offckb debugger "$@"'); + if (!existing.includes(marker) && !isLegacyShim) { logger.warn( `A file already exists at ${targetPath} that was not created by offckb; leaving it untouched. ` + 'Projects that call `ckb-debugger` directly will need it added to PATH manually.', diff --git a/src/tools/proxy-events.ts b/src/tools/proxy-events.ts index 520fe75..ead5794 100644 --- a/src/tools/proxy-events.ts +++ b/src/tools/proxy-events.ts @@ -117,7 +117,11 @@ export function handleProxyRequestBody(reqData: string, ctx: ProxyEventContext): ctx.sink.warn('skipping malformed JSON-RPC batch member'); continue; } - handleOneRequest(jsonRpcContent, ctx); + try { + handleOneRequest(jsonRpcContent, ctx); + } catch (error) { + ctx.sink.warn(`skipping JSON-RPC request event: ${(error as Error).message}`); + } } } catch (err) { ctx.sink.error('Error parsing JSON-RPC req content:', (err as Error).message); diff --git a/tests/ckb-debugger-install.test.ts b/tests/ckb-debugger-install.test.ts index 42c1e82..3692f46 100644 --- a/tests/ckb-debugger-install.test.ts +++ b/tests/ckb-debugger-install.test.ts @@ -253,6 +253,27 @@ describe('CKBDebuggerInstaller', () => { expect(fs.readFileSync(shimPath, 'utf-8')).toBe('#!/bin/sh\nexec /usr/bin/ckb-debugger "$@"\n'); }); + it('does not overwrite a user file that merely mentions offckb debugger', async () => { + const buffer = await buildTarGz('0.208.0'); + mockRelease('0.208.0', buffer); + const shimPath = path.join(root, 'bin', 'ckb-debugger'); + mockSpawnSync.mockImplementation((cmd: string) => { + if (cmd === 'which') { + return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' }; + } + return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' }; + }); + fs.mkdirSync(path.dirname(shimPath), { recursive: true }); + // A user wrapper that documents offckb debugger without being the exact + // v0.4.x fallback body must stay untouched. + const userScript = '#!/bin/sh\n# How to call: offckb debugger --help\n/usr/bin/ckb-debugger "$@"\n'; + fs.writeFileSync(shimPath, userScript); + + await CKBDebuggerInstaller.install(); + + expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript); + }); + it('upgrades a legacy v0.4.x fallback shim', async () => { const buffer = await buildTarGz('0.208.0'); mockRelease('0.208.0', buffer); diff --git a/tests/proxy-events.test.ts b/tests/proxy-events.test.ts index cd46bb9..13f9d37 100644 --- a/tests/proxy-events.test.ts +++ b/tests/proxy-events.test.ts @@ -98,6 +98,33 @@ describe('handleProxyRequestBody', () => { expect(content).toMatch(/send_transaction 0xhash/); }); + it('isolates a failing batch member and still records a valid request after it', () => { + const ctx = makeCtx(transactionsPath); + // Simulate a member whose tx dump blows up mid-processing. + ctx.hashTransaction = jest.fn((tx: unknown) => { + if ((tx as { bad?: boolean }).bad) { + throw new Error('hash boom'); + } + return '0xhash'; + }); + const goodTx = { cell_deps: [], inputs: [], outputs: [] }; + handleProxyRequestBody( + JSON.stringify([ + { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] }, + { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] }, + ]), + ctx, + ); + // The failing member is skipped with a warning instead of aborting the batch. + expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('hash boom')); + expect(ctx.sink.error).not.toHaveBeenCalled(); + // The valid send_transaction after it is still recorded. + expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash')); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true); + const content = fs.readFileSync(ctx.events.filePath, 'utf8'); + expect(content).toMatch(/send_transaction 0xhash/); + }); + it('warns and skips the tx dump when send_transaction has no usable params', () => { const ctx = makeCtx(transactionsPath); handleProxyRequestBody(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'send_transaction' }), ctx); From e920969177e3f2316178f2644ee26f00afbb2ad8 Mon Sep 17 00:00:00 2001 From: humble-little-bear Date: Fri, 14 Aug 2026 12:28:42 +0800 Subject: [PATCH 2/2] fix: address CodeRabbit review on #500 - ckb-debugger-install: classify a legacy v0.4.x fallback shim only when the whole file matches its exact body (CRLF-normalized, one trailing newline ignored); a legacy body with extra user content is left alone - proxy-events: normalize non-Error thrown values (Error.message vs String(error)) before logging so a null/string throw cannot abort the batch loop - tests: near-match legacy shims (Unix/Windows) stay untouched, Windows exact legacy shim upgrades, non-Error and null throws are isolated --- src/tools/ckb-debugger-install.ts | 14 +++---- src/tools/proxy-events.ts | 6 ++- tests/ckb-debugger-install.test.ts | 66 ++++++++++++++++++++++++++++++ tests/proxy-events.test.ts | 49 ++++++++++++++++++++++ 4 files changed, 127 insertions(+), 8 deletions(-) diff --git a/src/tools/ckb-debugger-install.ts b/src/tools/ckb-debugger-install.ts index bf96e40..dab6e21 100644 --- a/src/tools/ckb-debugger-install.ts +++ b/src/tools/ckb-debugger-install.ts @@ -354,10 +354,11 @@ export class CKBDebuggerInstaller { // The marker distinguishes our own shim from a foreign file — e.g. a real // binary a user installed via cargo — which must never be silently // clobbered. Legacy v0.4.x fallback shims are also upgraded in place, but - // only when they match the exact body offckb wrote back then (a shebang - // line plus `exec offckb debugger "$@"` on Unix, `@echo off` plus - // `offckb debugger %*` on Windows); a mere mention of `offckb debugger` - // in an arbitrary user script is not enough to claim it. + // only when the whole file is exactly the body offckb wrote back then (a + // shebang line plus `exec offckb debugger "$@"` on Unix, `@echo off` plus + // `offckb debugger %*` on Windows), line endings normalized and one + // trailing newline ignored; a mere mention of `offckb debugger`, or a + // legacy body with extra user content, is not enough to claim the file. const marker = isWindows ? '@rem offckb-managed ckb-debugger shim' : '# offckb-managed ckb-debugger shim'; const content = isWindows ? `${marker}\r\n@echo off\r\n"${binaryPath}" %*\r\n` @@ -366,9 +367,8 @@ export class CKBDebuggerInstaller { try { if (fs.existsSync(targetPath)) { const existing = fs.readFileSync(targetPath, 'utf8'); - const isLegacyShim = isWindows - ? existing.includes('@echo off') && existing.includes('offckb debugger %*') - : existing.startsWith('#!/bin/sh') && existing.includes('exec offckb debugger "$@"'); + const legacyBody = isWindows ? '@echo off\noffckb debugger %*' : '#!/bin/sh\nexec offckb debugger "$@"'; + const isLegacyShim = existing.replace(/\r\n/g, '\n').replace(/\n$/, '') === legacyBody; if (!existing.includes(marker) && !isLegacyShim) { logger.warn( `A file already exists at ${targetPath} that was not created by offckb; leaving it untouched. ` + diff --git a/src/tools/proxy-events.ts b/src/tools/proxy-events.ts index ead5794..a3790e0 100644 --- a/src/tools/proxy-events.ts +++ b/src/tools/proxy-events.ts @@ -120,7 +120,11 @@ export function handleProxyRequestBody(reqData: string, ctx: ProxyEventContext): try { handleOneRequest(jsonRpcContent, ctx); } catch (error) { - ctx.sink.warn(`skipping JSON-RPC request event: ${(error as Error).message}`); + // Thrown values are not necessarily Error instances (user code can + // throw anything), so normalize before logging; reading .message off + // a non-Error (e.g. null) would itself throw and abort the batch. + const message = error instanceof Error ? error.message : String(error); + ctx.sink.warn(`skipping JSON-RPC request event: ${message}`); } } } catch (err) { diff --git a/tests/ckb-debugger-install.test.ts b/tests/ckb-debugger-install.test.ts index 3692f46..6994103 100644 --- a/tests/ckb-debugger-install.test.ts +++ b/tests/ckb-debugger-install.test.ts @@ -293,6 +293,72 @@ describe('CKBDebuggerInstaller', () => { expect(fs.readFileSync(shimPath, 'utf-8')).toContain(`exec "${binaryPath}" "$@"`); }); + it('does not overwrite a legacy shim that has extra user content', async () => { + const buffer = await buildTarGz('0.208.0'); + mockRelease('0.208.0', buffer); + const shimPath = path.join(root, 'bin', 'ckb-debugger'); + mockSpawnSync.mockImplementation((cmd: string) => { + if (cmd === 'which') { + return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' }; + } + return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' }; + }); + fs.mkdirSync(path.dirname(shimPath), { recursive: true }); + // The complete legacy body plus one user line is not an offckb shim: the + // whole file must match exactly, so it stays untouched. + const userScript = '#!/bin/sh\nexec offckb debugger "$@"\n# custom wrapper\n'; + fs.writeFileSync(shimPath, userScript); + + await CKBDebuggerInstaller.install(); + + expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript); + }); + + it('upgrades an exact legacy v0.4.x fallback shim on Windows', async () => { + mockHost('win32', 'x64'); + const winBinaryPath = path.join(mockDirs.toolsRoot, 'ckb-debugger.exe'); + fs.mkdirSync(path.dirname(winBinaryPath), { recursive: true }); + fs.writeFileSync(winBinaryPath, fakeBinaryPayload('0.208.0')); + const shimPath = path.join(root, 'bin', 'ckb-debugger.cmd'); + mockSpawnSync.mockImplementation((cmd: string) => { + if (cmd === 'where') { + return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' }; + } + return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' }; + }); + fs.mkdirSync(path.dirname(shimPath), { recursive: true }); + // CRLF line endings are normalized before the exact-body comparison. + fs.writeFileSync(shimPath, '@echo off\r\noffckb debugger %*\r\n'); + + const result = await CKBDebuggerInstaller.install(); + + expect(result.alreadyInstalled).toBe(true); + const upgraded = fs.readFileSync(shimPath, 'utf-8'); + expect(upgraded).toContain('offckb-managed'); + expect(upgraded).toContain(`"${winBinaryPath}" %*`); + }); + + it('does not overwrite a legacy shim with extra user content on Windows', async () => { + mockHost('win32', 'x64'); + const winBinaryPath = path.join(mockDirs.toolsRoot, 'ckb-debugger.exe'); + fs.mkdirSync(path.dirname(winBinaryPath), { recursive: true }); + fs.writeFileSync(winBinaryPath, fakeBinaryPayload('0.208.0')); + const shimPath = path.join(root, 'bin', 'ckb-debugger.cmd'); + mockSpawnSync.mockImplementation((cmd: string) => { + if (cmd === 'where') { + return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' }; + } + return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' }; + }); + fs.mkdirSync(path.dirname(shimPath), { recursive: true }); + const userScript = '@echo off\r\noffckb debugger %*\r\nREM custom wrapper\r\n'; + fs.writeFileSync(shimPath, userScript); + + await CKBDebuggerInstaller.install(); + + expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript); + }); + it('throws when the platform has no prebuilt asset', async () => { mockHost('freebsd', 'x64'); await expect(CKBDebuggerInstaller.install()).rejects.toThrow(/no prebuilt binary/); diff --git a/tests/proxy-events.test.ts b/tests/proxy-events.test.ts index 13f9d37..9baa4ce 100644 --- a/tests/proxy-events.test.ts +++ b/tests/proxy-events.test.ts @@ -125,6 +125,55 @@ describe('handleProxyRequestBody', () => { expect(content).toMatch(/send_transaction 0xhash/); }); + it('normalizes a non-Error throw when isolating a failing batch member', () => { + const ctx = makeCtx(transactionsPath); + ctx.hashTransaction = jest.fn((tx: unknown) => { + if ((tx as { bad?: boolean }).bad) { + throw 'hash boom'; + } + return '0xhash'; + }); + const goodTx = { cell_deps: [], inputs: [], outputs: [] }; + handleProxyRequestBody( + JSON.stringify([ + { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] }, + { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] }, + ]), + ctx, + ); + // The thrown string is logged as-is instead of rendering as "undefined". + expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('hash boom')); + expect(ctx.sink.warn).not.toHaveBeenCalledWith(expect.stringContaining('undefined')); + expect(ctx.sink.error).not.toHaveBeenCalled(); + // The valid send_transaction after it is still recorded. + expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash')); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true); + }); + + it('tolerates a null throw when isolating a failing batch member', () => { + const ctx = makeCtx(transactionsPath); + ctx.hashTransaction = jest.fn((tx: unknown) => { + if ((tx as { bad?: boolean }).bad) { + throw null; + } + return '0xhash'; + }); + const goodTx = { cell_deps: [], inputs: [], outputs: [] }; + handleProxyRequestBody( + JSON.stringify([ + { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] }, + { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] }, + ]), + ctx, + ); + // Reading .message off null would itself throw; the warning must not. + expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('null')); + expect(ctx.sink.error).not.toHaveBeenCalled(); + // The valid send_transaction after it is still recorded. + expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash')); + expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true); + }); + it('warns and skips the tx dump when send_transaction has no usable params', () => { const ctx = makeCtx(transactionsPath); handleProxyRequestBody(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'send_transaction' }), ctx);