From 71299093a4cd2ec3f973ecbf5687561b219ee2dd Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 17:18:20 +0300 Subject: [PATCH 1/8] feat(wallet-cli): add the `mm wallet send` command Add the user-facing `mm wallet send` command and a dedicated daemon `sendTransaction` RPC handler for sending a transaction through the daemon-hosted `TransactionController`. `addTransaction` returns `{ transactionMeta, result }` where `result` is a `Promise` that cannot travel back over the generic `call` dispatch, so the daemon exposes a dedicated `sendTransaction` handler that resolves the network client (from `--network-client-id` or `--chain-id`) and sender (defaulting to the selected account), submits the transaction as internal (auto-approved by the daemon), awaits the broadcast server-side, and returns a serializable `{ transactionHash, transactionId, status }`. The command converts the ether `--value` to wei via `@metamask/utils`' shared `toWei`, previews the resolved plan, and broadcasts after confirmation. `--yes` skips the prompt; `--dry-run` resolves and validates without broadcasting. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 3 + packages/wallet-cli/README.md | 12 +- .../src/commands/wallet/send.test.ts | 326 ++++++++++++++++ .../wallet-cli/src/commands/wallet/send.ts | 262 +++++++++++++ .../src/daemon/daemon-entry.test.ts | 39 ++ .../wallet-cli/src/daemon/daemon-entry.ts | 12 + .../wallet-cli/src/daemon/prompts.test.ts | 26 ++ packages/wallet-cli/src/daemon/prompts.ts | 18 + .../src/daemon/send-transaction.test.ts | 359 ++++++++++++++++++ .../wallet-cli/src/daemon/send-transaction.ts | 188 +++++++++ 10 files changed, 1244 insertions(+), 1 deletion(-) create mode 100644 packages/wallet-cli/src/commands/wallet/send.test.ts create mode 100644 packages/wallet-cli/src/commands/wallet/send.ts create mode 100644 packages/wallet-cli/src/daemon/send-transaction.test.ts create mode 100644 packages/wallet-cli/src/daemon/send-transaction.ts diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index cf1fee9105e..b9099ef8ac1 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add the `mm wallet send` command and a dedicated daemon `sendTransaction` RPC handler for sending a transaction through the daemon-hosted `TransactionController` ([#9513](https://github.com/MetaMask/core/issues/9513)) + - The command converts the ether `--value` to wei, resolves the network client (`--network-client-id` or `--chain-id`) and sender (defaulting to the selected account), previews the resolved plan, and broadcasts after confirmation, printing the resulting transaction hash. `--yes` skips the prompt; `--dry-run` resolves and validates without broadcasting. + - The `sendTransaction` handler awaits the broadcast server-side and returns a serializable `{ transactionHash, transactionId, status }`, because `addTransaction`'s `result` promise cannot travel back over the generic `call` dispatch. Transactions are submitted as internal (auto-approved by the daemon). - Auto-accept pending approval requests in the daemon so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the subscription is torn down on `dispose` ([#9612](https://github.com/MetaMask/core/pull/9612)) - **Security note:** the daemon approves every request without a per-request prompt; the trust boundary is its `0600`, same-user Unix socket. - Wire the `transactionController` slot in the daemon wallet's instance options, so the daemon runs the `TransactionController` with an explicit CLI-appropriate configuration (swaps processing disabled, no client hooks) rather than relying on the controller's implicit defaults ([#9509](https://github.com/MetaMask/core/pull/9509)) diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index fd83efa65f9..66783fbaf22 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -32,6 +32,16 @@ mm wallet unlock --password # or: MM_WALLET_PASSWORD= mm wallet unloc mm wallet unlock # prompts interactively (input masked) ``` +Send a transaction. `--value` is in ether; select the network with `--network-client-id` or `--chain-id`; the sender defaults to the selected account. The command previews the resolved plan and asks for confirmation before broadcasting, then prints the transaction hash: + +```sh +mm wallet send --to 0xRecipient --value 0.01 --chain-id 0x1 +mm wallet send --to 0xRecipient --value 0.01 --network-client-id mainnet --yes # skip the prompt +mm wallet send --to 0xContract --data 0xabcdef --value 0 --chain-id 0x1 --dry-run # resolve + validate only +``` + +Because the daemon auto-approves (see the security note below), the confirmation prompt — or your explicit `--yes` — is the only boundary before funds move; use `--dry-run` first if unsure. Gas is estimated via `GasFeeController` unless overridden with `--gas` / `--max-fee-per-gas` / `--max-priority-fee-per-gas` / `--gas-price` (each a `0x`-prefixed hex quantity). + Discover what the running wallet can do — `list` prints every messenger action currently dispatchable via `call`. This surface grows as more controllers are wired, so treat it as evolving rather than a stability contract: ```sh @@ -57,7 +67,7 @@ mm daemon purge # stop, then delete all daemon state files (--force to State (socket, PID file, log, and the SQLite database) lives in the per-user oclif data directory; override it with `MM_DATA_DIR`. -> **Security model — the daemon auto-approves everything.** Because it is headless, the daemon accepts every approval request (transactions and signatures included) with no per-request prompt; otherwise an awaited request would hang forever with no UI to resolve it. The trust boundary is therefore the daemon's `0600`, same-user Unix socket — anything that can reach the socket can move funds. A scoped/opt-in approval policy is planned for when a user-facing send command lands. +> **Security model — the daemon auto-approves everything.** Because it is headless, the daemon accepts every approval request (transactions and signatures included) with no per-request prompt; otherwise an awaited request would hang forever with no UI to resolve it. The trust boundary is therefore the daemon's `0600`, same-user Unix socket — anything that can reach the socket can move funds. For `mm wallet send` the confirmation prompt (or `--yes`) is the only per-transaction gate; a scoped/opt-in approval policy at the daemon level is still planned. ## Troubleshooting diff --git a/packages/wallet-cli/src/commands/wallet/send.test.ts b/packages/wallet-cli/src/commands/wallet/send.test.ts new file mode 100644 index 00000000000..a8ca917f2b5 --- /dev/null +++ b/packages/wallet-cli/src/commands/wallet/send.test.ts @@ -0,0 +1,326 @@ +import { sendCommand } from '../../daemon/daemon-client.js'; +import { confirmSend } from '../../daemon/prompts.js'; +import { runCommand } from '../../test/run-command.js'; +import WalletSend, { parseEtherToWeiHex } from './send.js'; + +jest.mock('../../daemon/daemon-client'); +jest.mock('../../daemon/prompts'); + +const mockSendCommand = jest.mocked(sendCommand); +const mockConfirmSend = jest.mocked(confirmSend); + +const TO = '0x1111111111111111111111111111111111111111'; +const FROM = '0x2222222222222222222222222222222222222222'; + +const DRY_RUN_RESULT = { + dryRun: true, + from: FROM, + to: TO, + value: '0x2386f26fc10000', + networkClientId: 'mainnet', +}; + +const BROADCAST_RESULT = { + transactionHash: '0xhash', + transactionId: 'tx-1', + status: 'submitted', +}; + +/** + * Queue JSON-RPC success responses for successive `sendCommand` calls. + * + * @param results - The `result` payloads to return, in call order. + */ +function queueResults(...results: unknown[]): void { + for (const result of results) { + mockSendCommand.mockResolvedValueOnce({ + jsonrpc: '2.0', + id: '1', + result: result as never, + }); + } +} + +describe('parseEtherToWeiHex', () => { + it.each([ + ['0', '0x0'], + ['1', '0xde0b6b3a7640000'], + ['0.01', '0x2386f26fc10000'], + ['0.1', '0x16345785d8a0000'], + ['123.456', '0x6b14bd1e6eea00000'], + ])('converts %s ether to %s wei', (input, expected) => { + expect(parseEtherToWeiHex(input)).toBe(expected); + }); + + it('trims surrounding whitespace', () => { + expect(parseEtherToWeiHex(' 1 ')).toBe('0xde0b6b3a7640000'); + }); + + it.each(['', 'abc', '1.2.3', '-1', '0x1', '1e18'])( + 'rejects the non-decimal value %p', + (input) => { + expect(() => parseEtherToWeiHex(input)).toThrow( + 'expected a non-negative', + ); + }, + ); + + it('rejects more than 18 fractional digits', () => { + expect(() => parseEtherToWeiHex('0.1234567890123456789')).toThrow( + 'at most 18 decimal places', + ); + }); +}); + +describe('wallet send', () => { + beforeEach(() => { + mockConfirmSend.mockResolvedValue(true); + }); + + it('requires exactly one of --network-client-id / --chain-id', async () => { + const { error } = await runCommand(WalletSend, ['--to', TO]); + expect(error?.message).toContain('exactly one'); + expect(mockSendCommand).not.toHaveBeenCalled(); + }); + + it('rejects supplying both --network-client-id and --chain-id', async () => { + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--chain-id', + '0x1', + ]); + expect(error?.message).toContain('exactly one'); + expect(mockSendCommand).not.toHaveBeenCalled(); + }); + + it('rejects an invalid --value before dispatching', async () => { + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + '--value', + 'not-a-number', + ]); + expect(error?.message).toContain('expected a non-negative'); + expect(mockSendCommand).not.toHaveBeenCalled(); + }); + + it('previews then broadcasts after confirmation', async () => { + queueResults(DRY_RUN_RESULT, BROADCAST_RESULT); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + '--value', + '0.01', + ]); + + expect(mockSendCommand).toHaveBeenCalledTimes(2); + // First call is the dry-run preview... + expect(mockSendCommand).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: 'sendTransaction', + params: expect.objectContaining({ + to: TO, + chainId: '0x1', + value: '0x2386f26fc10000', + dryRun: true, + }), + }), + ); + // ...the second is the real broadcast, without dryRun. + const secondParams = mockSendCommand.mock.calls[1]?.[0]?.params as Record< + string, + unknown + >; + expect(secondParams.dryRun).toBeUndefined(); + expect(mockConfirmSend).toHaveBeenCalledTimes(1); + expect(stdout).toContain('Transaction broadcast.'); + expect(stdout).toContain('0xhash'); + }); + + it('aborts without broadcasting when the user declines', async () => { + queueResults(DRY_RUN_RESULT); + mockConfirmSend.mockResolvedValue(false); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + ]); + + expect(mockSendCommand).toHaveBeenCalledTimes(1); + expect(stdout).toContain('Aborted.'); + }); + + it('aborts cleanly when the confirmation prompt is cancelled', async () => { + queueResults(DRY_RUN_RESULT); + mockConfirmSend.mockRejectedValue( + Object.assign(new Error('User force closed the prompt'), { + name: 'ExitPromptError', + }), + ); + + const { stdout, error } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + ]); + + expect(error).toBeUndefined(); + expect(stdout).toContain('Aborted.'); + expect(mockSendCommand).toHaveBeenCalledTimes(1); + }); + + it('surfaces a genuine confirmation-prompt failure', async () => { + queueResults(DRY_RUN_RESULT); + mockConfirmSend.mockRejectedValue(new Error('prompt crashed')); + + await expect( + runCommand(WalletSend, ['--to', TO, '--chain-id', '0x1']), + ).rejects.toThrow('prompt crashed'); + }); + + it('skips the preview and prompt with --yes', async () => { + queueResults(BROADCAST_RESULT); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--yes', + ]); + + expect(mockSendCommand).toHaveBeenCalledTimes(1); + expect(mockSendCommand).toHaveBeenCalledWith( + expect.objectContaining({ + params: expect.not.objectContaining({ dryRun: true }), + }), + ); + expect(mockConfirmSend).not.toHaveBeenCalled(); + expect(stdout).toContain('0xhash'); + }); + + it('previews and stops with --dry-run', async () => { + queueResults(DRY_RUN_RESULT); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + '--dry-run', + ]); + + expect(mockSendCommand).toHaveBeenCalledTimes(1); + expect(mockSendCommand).toHaveBeenCalledWith( + expect.objectContaining({ + params: expect.objectContaining({ dryRun: true }), + }), + ); + expect(mockConfirmSend).not.toHaveBeenCalled(); + expect(stdout).toContain('About to send:'); + expect(stdout).toContain(TO); + expect(stdout).not.toContain('Transaction broadcast.'); + }); + + it('forwards optional tx fields and the timeout', async () => { + queueResults(BROADCAST_RESULT); + + await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--from', + FROM, + '--data', + '0xabcdef', + '--gas', + '0x5208', + '--max-fee-per-gas', + '0x2', + '--max-priority-fee-per-gas', + '0x1', + '--gas-price', + '0x3', + '--timeout', + '5000', + '--yes', + ]); + + expect(mockSendCommand).toHaveBeenCalledWith( + expect.objectContaining({ + timeoutMs: 5000, + params: expect.objectContaining({ + from: FROM, + data: '0xabcdef', + gas: '0x5208', + maxFeePerGas: '0x2', + maxPriorityFeePerGas: '0x1', + gasPrice: '0x3', + }), + }), + ); + }); + + it('prints (unknown) for fields missing from the result payload', async () => { + queueResults({ transactionHash: '0xhash' }); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--yes', + ]); + + expect(stdout).toContain('Hash: 0xhash'); + expect(stdout).toContain('Id: (unknown)'); + expect(stdout).toContain('Status: (unknown)'); + }); + + it('surfaces a JSON-RPC failure from the daemon', async () => { + mockSendCommand.mockResolvedValue({ + jsonrpc: '2.0', + id: '1', + error: { code: -32000, message: 'Network client not found - foo' }, + }); + + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'foo', + '--yes', + ]); + + expect(error?.message).toContain('Network client not found'); + }); + + it('reports a friendly hint when the daemon is unreachable', async () => { + mockSendCommand.mockRejectedValue( + Object.assign(new Error('no such file'), { code: 'ENOENT' }), + ); + + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--yes', + ]); + + expect(error?.message).toContain('Daemon is not running'); + }); +}); diff --git a/packages/wallet-cli/src/commands/wallet/send.ts b/packages/wallet-cli/src/commands/wallet/send.ts new file mode 100644 index 00000000000..df3e78bfeb1 --- /dev/null +++ b/packages/wallet-cli/src/commands/wallet/send.ts @@ -0,0 +1,262 @@ +import { bigIntToHex, isJsonRpcFailure, toWei } from '@metamask/utils'; +import type { Hex, Json, JsonRpcParams } from '@metamask/utils'; +import { Command, Flags } from '@oclif/core'; + +import { sendCommand } from '../../daemon/daemon-client.js'; +import { getDaemonPaths } from '../../daemon/paths.js'; +import { confirmSend } from '../../daemon/prompts.js'; +import { + emptyToUndefined, + formatJsonRpcError, + makeDaemonConnectionError, +} from '../../daemon/utils.js'; + +const ETHER_DECIMALS = 18; + +/** + * Convert a decimal ether amount to a `0x`-prefixed wei quantity. + * + * The fixed-point conversion is delegated to `@metamask/utils`' `toWei` — the + * shared, audited implementation (a bigint-based reimplementation of + * `@metamask/ethjs-unit`) — rather than re-deriving the wei math here. The + * daemon's `sendTransaction` boundary expects canonical hex wei; this is where + * the human-friendly `--value` (ether) is turned into it. + * + * `toWei` itself accepts negatives and scientific notation and throws opaque + * messages, so the input is guarded up front for a clear error and a strictly + * non-negative value. + * + * @param amount - A non-negative decimal ether amount (e.g. `"0.1"`, `"1"`). + * @returns The equivalent wei as a `0x`-prefixed hex string. + * @throws If `amount` is not a non-negative decimal or has more than 18 + * fractional digits. + */ +export function parseEtherToWeiHex(amount: string): Hex { + const trimmed = amount.trim(); + if (!/^\d+(?:\.\d+)?$/u.test(trimmed)) { + throw new Error( + `Invalid value "${amount}": expected a non-negative decimal amount of ether (e.g. 0.1).`, + ); + } + + if ((trimmed.split('.')[1] ?? '').length > ETHER_DECIMALS) { + throw new Error( + `Invalid value "${amount}": ether has at most ${ETHER_DECIMALS} decimal places.`, + ); + } + + return bigIntToHex(toWei(trimmed, 'ether')); +} + +export default class WalletSend extends Command { + static override description = + 'Send a transaction through the daemon-hosted TransactionController. ' + + 'Resolves gas via GasFeeController estimates unless overridden, signs, ' + + 'broadcasts, and prints the resulting transaction hash. The daemon ' + + 'auto-approves, so the confirmation boundary is this command.'; + + static override examples = [ + '<%= config.bin %> wallet send --to 0xRecipient --value 0.01 --chain-id 0x1', + '<%= config.bin %> wallet send --to 0xRecipient --value 0.01 --network-client-id mainnet --yes', + '<%= config.bin %> wallet send --to 0xContract --data 0xabcdef --value 0 --chain-id 0x1 --dry-run', + ]; + + static override flags = { + to: Flags.string({ + description: 'Recipient address (0x-prefixed)', + required: true, + }), + value: Flags.string({ + description: 'Amount to send, in ether (e.g. 0.01). Defaults to 0.', + default: '0', + }), + from: Flags.string({ + description: + 'Sender address (0x-prefixed). Defaults to the selected account.', + }), + data: Flags.string({ + description: 'Calldata as a 0x-prefixed hex string (for contract calls)', + }), + 'network-client-id': Flags.string({ + description: + 'Network client to send on. Provide this or --chain-id, not both.', + }), + 'chain-id': Flags.string({ + description: + 'Chain ID (0x-prefixed hex) to resolve to a network client. Provide this or --network-client-id, not both.', + }), + gas: Flags.string({ + description: 'Gas limit override, as a 0x-prefixed hex quantity', + }), + 'max-fee-per-gas': Flags.string({ + description: 'maxFeePerGas override, as a 0x-prefixed hex wei quantity', + }), + 'max-priority-fee-per-gas': Flags.string({ + description: + 'maxPriorityFeePerGas override, as a 0x-prefixed hex wei quantity', + }), + 'gas-price': Flags.string({ + description: + 'Legacy gasPrice override, as a 0x-prefixed hex wei quantity', + }), + 'dry-run': Flags.boolean({ + description: + 'Resolve the network client and sender and validate params, but do not broadcast.', + }), + yes: Flags.boolean({ + char: 'y', + description: 'Skip the confirmation prompt and broadcast immediately.', + }), + timeout: Flags.integer({ + char: 't', + description: 'Response timeout in milliseconds', + }), + }; + + public async run(): Promise { + const { flags } = await this.parse(WalletSend); + + const networkClientId = emptyToUndefined(flags['network-client-id']); + const chainId = emptyToUndefined(flags['chain-id']); + if ((networkClientId === undefined) === (chainId === undefined)) { + this.error('Provide exactly one of --network-client-id or --chain-id.'); + } + + let value: string; + try { + value = parseEtherToWeiHex(flags.value); + } catch (error) { + // `parseEtherToWeiHex` only ever throws `Error`. + this.error((error as Error).message); + } + + const params: Record = { + to: flags.to, + value, + ...(networkClientId === undefined ? {} : { networkClientId }), + ...(chainId === undefined ? {} : { chainId }), + ...(flags.from ? { from: flags.from } : {}), + ...(flags.data ? { data: flags.data } : {}), + ...(flags.gas ? { gas: flags.gas } : {}), + ...(flags['max-fee-per-gas'] + ? { maxFeePerGas: flags['max-fee-per-gas'] } + : {}), + ...(flags['max-priority-fee-per-gas'] + ? { maxPriorityFeePerGas: flags['max-priority-fee-per-gas'] } + : {}), + ...(flags['gas-price'] ? { gasPrice: flags['gas-price'] } : {}), + }; + + const { socketPath } = getDaemonPaths(this.config.dataDir); + const timeoutMs = flags.timeout; + + // A `dryRun` resolves the network client and sender and validates params + // server-side without broadcasting. `--dry-run` stops after previewing; + // an interactive run previews first so the confirmation shows the resolved + // sender/network. `--yes` skips both and broadcasts directly. + if (flags['dry-run']) { + const preview = await this.#dispatchSend( + socketPath, + { ...params, dryRun: true }, + timeoutMs, + ); + this.log(formatPlan(preview, flags.value)); + return; + } + + if (!flags.yes) { + const preview = await this.#dispatchSend( + socketPath, + { ...params, dryRun: true }, + timeoutMs, + ); + + let confirmed: boolean; + try { + confirmed = await confirmSend(formatPlan(preview, flags.value)); + } catch (error) { + // Ctrl+C at the prompt (@inquirer/core's ExitPromptError) is a clean + // abort, not a failure; anything else should surface. + if (error instanceof Error && error.name === 'ExitPromptError') { + this.log('Aborted.'); + return; + } + throw error; + } + if (!confirmed) { + this.log('Aborted.'); + return; + } + } + + const result = await this.#dispatchSend(socketPath, params, timeoutMs); + this.log('Transaction broadcast.'); + this.log(`Hash: ${stringField(result, 'transactionHash')}`); + this.log(`Id: ${stringField(result, 'transactionId')}`); + this.log(`Status: ${stringField(result, 'status')}`); + } + + /** + * Send a `sendTransaction` RPC to the daemon and return its result payload, + * translating connection errors and JSON-RPC failures into command errors. + * + * @param socketPath - The daemon Unix socket path. + * @param params - The `sendTransaction` params (with or without `dryRun`). + * @param timeoutMs - Optional response timeout in milliseconds. + * @returns The JSON-RPC `result` as a record. + */ + async #dispatchSend( + socketPath: string, + params: JsonRpcParams, + timeoutMs: number | undefined, + ): Promise> { + let response; + try { + response = await sendCommand({ + socketPath, + method: 'sendTransaction', + params, + ...(timeoutMs === undefined ? {} : { timeoutMs }), + }); + } catch (error) { + this.error(makeDaemonConnectionError(error)); + } + + if (isJsonRpcFailure(response)) { + this.error(formatJsonRpcError(response.error)); + } + + return response.result as Record; + } +} + +/** + * Read a string field off a JSON-RPC result record, tolerating a missing or + * non-string value so output never crashes on an unexpected payload shape. + * + * @param result - The JSON-RPC result record. + * @param key - The field to read. + * @returns The field as a string, or `'(unknown)'` if absent/non-string. + */ +function stringField(result: Record, key: string): string { + const value = result[key]; + return typeof value === 'string' ? value : '(unknown)'; +} + +/** + * Format a dry-run plan for display (and as the confirmation prompt body). + * + * @param plan - The dry-run result returned by the daemon. + * @param etherAmount - The original `--value` (ether), shown alongside the + * resolved wei so the user sees the human amount they typed. + * @returns A multi-line summary of the transaction to be sent. + */ +function formatPlan(plan: Record, etherAmount: string): string { + return [ + 'About to send:', + ` To: ${stringField(plan, 'to')}`, + ` From: ${stringField(plan, 'from')}`, + ` Value: ${etherAmount} ETH (${stringField(plan, 'value')} wei)`, + ` Network: ${stringField(plan, 'networkClientId')}`, + ].join('\n'); +} diff --git a/packages/wallet-cli/src/daemon/daemon-entry.test.ts b/packages/wallet-cli/src/daemon/daemon-entry.test.ts index 7b86456bc2d..dbab9bdf586 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.test.ts @@ -908,4 +908,43 @@ describe('daemon-entry', () => { expect(error).toBeUndefined(); }); }); + + describe('sendTransaction handler', () => { + it('runs a send through the wallet messenger and returns the hash', async () => { + const result = createMockWallet(); + const mockCall = result.wallet.messenger.call as jest.Mock; + mockCall.mockReturnValue({ + result: Promise.resolve('0xhash'), + transactionMeta: { id: 'tx-1', status: 'submitted' }, + }); + mockCreateWallet.mockResolvedValue(result); + mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); + + await importDaemonEntry(); + + const { handlers } = mockStartRpcSocketServer.mock.calls[0][0]; + const sendResult = await handlers.sendTransaction.run({ + to: '0x1111111111111111111111111111111111111111', + from: '0x2222222222222222222222222222222222222222', + networkClientId: 'mainnet', + }); + + expect(mockCall).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.objectContaining({ + from: '0x2222222222222222222222222222222222222222', + to: '0x1111111111111111111111111111111111111111', + }), + expect.objectContaining({ + networkClientId: 'mainnet', + isInternal: true, + }), + ); + expect(sendResult).toStrictEqual({ + transactionHash: '0xhash', + transactionId: 'tx-1', + status: 'submitted', + }); + }); + }); }); diff --git a/packages/wallet-cli/src/daemon/daemon-entry.ts b/packages/wallet-cli/src/daemon/daemon-entry.ts index 6d905d511fc..2f324bccabb 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.ts @@ -9,6 +9,10 @@ import { getDaemonPaths } from './paths.js'; import { startRpcSocketServer } from './rpc-socket-server.js'; import type { RpcSocketServerHandle } from './rpc-socket-server.js'; import { Password, Srp } from './secrets.js'; +import { + runSendTransaction, + SendTransactionParamsStruct, +} from './send-transaction.js'; import { defineHandler } from './types.js'; import type { DaemonStatusInfo, @@ -161,6 +165,14 @@ async function main(): Promise { async (): Promise => constructedWallet.messenger.getRegisteredActionTypes(), ), + // Dedicated send handler: `addTransaction` returns a non-serializable + // `result` promise, so it cannot travel back over the generic `call` + // dispatch. This awaits the broadcast server-side and returns the hash. + sendTransaction: defineHandler( + SendTransactionParamsStruct, + async (params) => + runSendTransaction(constructedWallet.messenger, params), + ), }; // `startRpcSocketServer` restricts the socket to the owner (chmod 0o600) diff --git a/packages/wallet-cli/src/daemon/prompts.test.ts b/packages/wallet-cli/src/daemon/prompts.test.ts index c113060cb9c..2ab323496fe 100644 --- a/packages/wallet-cli/src/daemon/prompts.test.ts +++ b/packages/wallet-cli/src/daemon/prompts.test.ts @@ -44,6 +44,32 @@ describe('confirmPurge', () => { }); }); +describe('confirmSend', () => { + it('invokes @inquirer/confirm with the summary and defaults to false', async () => { + const confirm = (await import('@inquirer/confirm')) + .default as unknown as ConfirmMock; + confirm.mockResolvedValue(true); + const { confirmSend } = await import('./prompts.js'); + + const result = await confirmSend('About to send:\n To: 0xabc'); + + expect(result).toBe(true); + expect(confirm).toHaveBeenCalledWith({ + message: 'About to send:\n To: 0xabc\nBroadcast this transaction?', + default: false, + }); + }); + + it('returns false when the user declines', async () => { + const confirm = (await import('@inquirer/confirm')) + .default as unknown as ConfirmMock; + confirm.mockResolvedValue(false); + const { confirmSend } = await import('./prompts.js'); + + expect(await confirmSend('summary')).toBe(false); + }); +}); + describe('promptPassword', () => { it('invokes @inquirer/password with masked input and returns the user input', async () => { const password = (await import('@inquirer/password')) diff --git a/packages/wallet-cli/src/daemon/prompts.ts b/packages/wallet-cli/src/daemon/prompts.ts index 3db2959a1fc..a0abb0de929 100644 --- a/packages/wallet-cli/src/daemon/prompts.ts +++ b/packages/wallet-cli/src/daemon/prompts.ts @@ -15,6 +15,24 @@ export async function confirmPurge(): Promise { }); } +/** + * Ask the user to confirm broadcasting a transaction. Used by `mm wallet send` + * when neither `--yes` nor `--dry-run` was passed. Same dynamic-import + + * ESM-interop pattern as {@link confirmPurge}. Defaults to `false` so an + * accidental bare Enter never broadcasts. + * + * @param summary - A human-readable description of the transaction to send + * (recipient, amount, network), shown above the prompt. + * @returns True if the user confirmed. + */ +export async function confirmSend(summary: string): Promise { + const { default: confirm } = await import('@inquirer/confirm'); + return confirm({ + message: `${summary}\nBroadcast this transaction?`, + default: false, + }); +} + /** * Prompt the user for the wallet password, with input masked. Used by * `mm wallet unlock` when the user did not pass `--password` or set the diff --git a/packages/wallet-cli/src/daemon/send-transaction.test.ts b/packages/wallet-cli/src/daemon/send-transaction.test.ts new file mode 100644 index 00000000000..3743e5c2262 --- /dev/null +++ b/packages/wallet-cli/src/daemon/send-transaction.test.ts @@ -0,0 +1,359 @@ +import { validate } from '@metamask/superstruct'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '@metamask/wallet'; + +import { + runSendTransaction, + SendTransactionParamsStruct, +} from './send-transaction.js'; +import type { SendTransactionParams } from './send-transaction.js'; + +const TO = '0x1111111111111111111111111111111111111111'; +const FROM = '0x2222222222222222222222222222222222222222'; +const SELECTED = '0x3333333333333333333333333333333333333333'; + +type MessengerCall = (action: string, ...args: unknown[]) => unknown; + +/** + * Build a fake root messenger whose `call` is driven by a per-action lookup, + * cast to the typed messenger `runSendTransaction` expects. + * + * @param handlers - Map of action name to the value (or promise) it returns. + * @returns The fake messenger and its underlying `call` jest mock. + */ +function makeMessenger(handlers: Record): { + messenger: Readonly>; + call: jest.MockedFunction; +} { + const call = jest.fn((action) => { + if (!Object.prototype.hasOwnProperty.call(handlers, action)) { + throw new Error(`Unexpected messenger action: ${action}`); + } + return handlers[action]; + }); + return { + messenger: { call } as unknown as Readonly< + RootMessenger + >, + call, + }; +} + +/** + * A default set of action handlers covering the happy send path. + * + * @returns The action-to-result map. + */ +function defaultHandlers(): Record { + return { + 'NetworkController:findNetworkClientIdByChainId': 'mainnet', + 'AccountsController:getSelectedAccount': { address: SELECTED }, + 'TransactionController:addTransaction': Promise.resolve({ + result: Promise.resolve('0xhash'), + transactionMeta: { id: 'tx-1', status: 'submitted' }, + }), + }; +} + +describe('SendTransactionParamsStruct', () => { + const base = { + to: TO, + networkClientId: 'mainnet', + }; + + it('accepts a minimal params object', () => { + const [error] = validate(base, SendTransactionParamsStruct); + expect(error).toBeUndefined(); + }); + + it('accepts a fully-specified params object', () => { + const [error] = validate( + { + to: TO, + from: FROM, + value: '0x64', + data: '0xabcdef', + gas: '0x5208', + maxFeePerGas: '0x1', + maxPriorityFeePerGas: '0x1', + gasPrice: '0x1', + chainId: '0x1', + dryRun: true, + }, + SendTransactionParamsStruct, + ); + expect(error).toBeUndefined(); + }); + + it('rejects an invalid recipient address', () => { + const [error] = validate( + { ...base, to: '0xnothex' }, + SendTransactionParamsStruct, + ); + expect(error?.message).toContain('20-byte hex address'); + }); + + it('rejects a non-hex value', () => { + const [error] = validate( + { ...base, value: '100' }, + SendTransactionParamsStruct, + ); + expect(error).toBeDefined(); + }); + + it('rejects supplying both networkClientId and chainId', () => { + const [error] = validate( + { to: TO, networkClientId: 'mainnet', chainId: '0x1' }, + SendTransactionParamsStruct, + ); + expect(error?.message).toContain( + 'Exactly one of `networkClientId` or `chainId`', + ); + }); + + it('rejects supplying neither networkClientId nor chainId', () => { + const [error] = validate({ to: TO }, SendTransactionParamsStruct); + expect(error?.message).toContain( + 'Exactly one of `networkClientId` or `chainId`', + ); + }); + + it('rejects unknown keys', () => { + const [error] = validate( + { ...base, surprise: true }, + SendTransactionParamsStruct, + ); + expect(error).toBeDefined(); + }); +}); + +describe('runSendTransaction', () => { + it('resolves the network client from chainId via NetworkController', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + chainId: '0x1', + } as SendTransactionParams); + + expect(call).toHaveBeenCalledWith( + 'NetworkController:findNetworkClientIdByChainId', + '0x1', + ); + }); + + it('uses a provided networkClientId without querying NetworkController', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + networkClientId: 'linea', + } as SendTransactionParams); + + expect(call).not.toHaveBeenCalledWith( + 'NetworkController:findNetworkClientIdByChainId', + expect.anything(), + ); + expect(call).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.objectContaining({ from: SELECTED, to: TO }), + expect.objectContaining({ networkClientId: 'linea' }), + ); + }); + + it('defaults `from` to the selected account', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(call).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.objectContaining({ from: SELECTED }), + expect.anything(), + ); + }); + + it('uses a provided `from` without querying AccountsController', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + from: FROM, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(call).not.toHaveBeenCalledWith( + 'AccountsController:getSelectedAccount', + ); + expect(call).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.objectContaining({ from: FROM }), + expect.anything(), + ); + }); + + it('submits the transaction as internal with the metamask origin', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(call).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.anything(), + { networkClientId: 'mainnet', origin: 'metamask', isInternal: true }, + ); + }); + + it('forwards only the provided optional tx fields', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + value: '0x64', + data: '0xabcdef', + maxFeePerGas: '0x2', + networkClientId: 'mainnet', + } as SendTransactionParams); + + const txParams = call.mock.calls.find( + ([action]) => action === 'TransactionController:addTransaction', + )?.[1]; + expect(txParams).toStrictEqual({ + from: SELECTED, + to: TO, + value: '0x64', + data: '0xabcdef', + maxFeePerGas: '0x2', + }); + }); + + it('forwards every gas override when all are provided', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + value: '0x64', + data: '0xabcdef', + gas: '0x5208', + maxFeePerGas: '0x2', + maxPriorityFeePerGas: '0x1', + gasPrice: '0x3', + networkClientId: 'mainnet', + } as SendTransactionParams); + + const txParams = call.mock.calls.find( + ([action]) => action === 'TransactionController:addTransaction', + )?.[1]; + expect(txParams).toStrictEqual({ + from: SELECTED, + to: TO, + value: '0x64', + data: '0xabcdef', + gas: '0x5208', + maxFeePerGas: '0x2', + maxPriorityFeePerGas: '0x1', + gasPrice: '0x3', + }); + }); + + it('defaults value to 0x0 when omitted', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(call).toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.objectContaining({ value: '0x0' }), + expect.anything(), + ); + }); + + it('awaits the broadcast and returns the hash, id, and status', async () => { + const { messenger } = makeMessenger(defaultHandlers()); + + const result = await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(result).toStrictEqual({ + transactionHash: '0xhash', + transactionId: 'tx-1', + status: 'submitted', + }); + }); + + it('propagates a broadcast failure', async () => { + const { messenger } = makeMessenger({ + ...defaultHandlers(), + 'AccountsController:getSelectedAccount': { address: SELECTED }, + 'TransactionController:addTransaction': Promise.resolve({ + result: Promise.reject(new Error('insufficient funds')), + transactionMeta: { id: 'tx-1', status: 'unapproved' }, + }), + }); + + await expect( + runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams), + ).rejects.toThrow('insufficient funds'); + }); + + describe('dryRun', () => { + it('returns the resolved plan without adding a transaction', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + const result = await runSendTransaction(messenger, { + to: TO, + value: '0x64', + chainId: '0x1', + dryRun: true, + } as SendTransactionParams); + + expect(result).toStrictEqual({ + dryRun: true, + from: SELECTED, + to: TO, + value: '0x64', + networkClientId: 'mainnet', + }); + expect(call).not.toHaveBeenCalledWith( + 'TransactionController:addTransaction', + expect.anything(), + expect.anything(), + ); + }); + + it('still resolves the network client and sender', async () => { + const { messenger, call } = makeMessenger(defaultHandlers()); + + await runSendTransaction(messenger, { + to: TO, + chainId: '0x1', + dryRun: true, + } as SendTransactionParams); + + expect(call).toHaveBeenCalledWith( + 'NetworkController:findNetworkClientIdByChainId', + '0x1', + ); + expect(call).toHaveBeenCalledWith( + 'AccountsController:getSelectedAccount', + ); + }); + }); +}); diff --git a/packages/wallet-cli/src/daemon/send-transaction.ts b/packages/wallet-cli/src/daemon/send-transaction.ts new file mode 100644 index 00000000000..01b699e62af --- /dev/null +++ b/packages/wallet-cli/src/daemon/send-transaction.ts @@ -0,0 +1,188 @@ +import { + boolean, + define, + object, + optional, + refine, + string, +} from '@metamask/superstruct'; +import type { Infer } from '@metamask/superstruct'; +import { isValidHexAddress, StrictHexStruct } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '@metamask/wallet'; + +/** + * Origin recorded on daemon-initiated transactions. Mirrors + * `ORIGIN_METAMASK` from `@metamask/controller-utils` (a single string + * constant, inlined here to avoid pulling that whole package in as a + * dependency for one value). + */ +const INTERNAL_ORIGIN = 'metamask'; + +/** + * Struct for a `0x`-prefixed 20-byte hex address. `isValidHexAddress` accepts + * any-case addresses (checksum optional), which is what a CLI user is most + * likely to paste. + */ +const AddressStruct = define('Address', (value) => + typeof value === 'string' && isValidHexAddress(value as Hex) + ? true + : 'Expected a 0x-prefixed 20-byte hex address', +); + +/** + * Params struct for the `sendTransaction` RPC method. + * + * The transaction fields (`to`, `value`, `data`, gas overrides) are canonical + * `0x`-prefixed hex — any unit conversion (e.g. ether → wei) is the CLI + * command's job, so the daemon boundary stays unambiguous. Exactly one of + * `networkClientId` / `chainId` selects the network client; the refinement + * below rejects supplying both or neither. `dryRun` short-circuits before + * broadcast (see {@link runSendTransaction}). + */ +export const SendTransactionParamsStruct = refine( + object({ + to: AddressStruct, + from: optional(AddressStruct), + value: optional(StrictHexStruct), + data: optional(StrictHexStruct), + gas: optional(StrictHexStruct), + maxFeePerGas: optional(StrictHexStruct), + maxPriorityFeePerGas: optional(StrictHexStruct), + gasPrice: optional(StrictHexStruct), + networkClientId: optional(string()), + chainId: optional(StrictHexStruct), + dryRun: optional(boolean()), + }), + 'SendTransactionParams', + (value) => { + if ( + (value.networkClientId === undefined) === + (value.chainId === undefined) + ) { + return 'Exactly one of `networkClientId` or `chainId` must be provided'; + } + return true; + }, +); + +export type SendTransactionParams = Infer; + +/** + * The resolved plan a `dryRun` returns: the network client and sender the + * daemon would use, without adding or broadcasting anything. + */ +export type SendTransactionDryRunResult = { + dryRun: true; + from: Hex; + to: Hex; + value: Hex; + networkClientId: string; +}; + +/** + * The outcome of a broadcast send: the on-chain hash plus the id/status the + * daemon tracks the transaction under. + */ +export type SendTransactionBroadcastResult = { + transactionHash: string; + transactionId: string; + status: string; +}; + +export type SendTransactionResult = + | SendTransactionDryRunResult + | SendTransactionBroadcastResult; + +/** + * Add and broadcast a transaction through the daemon-hosted + * `TransactionController`, returning a serializable result. + * + * `TransactionController:addTransaction` returns `{ transactionMeta, result }` + * where `result` is a `Promise` that resolves once the transaction is + * signed and broadcast. That promise is not JSON-serializable, so it cannot + * travel back over the daemon's JSON-RPC socket via the generic `call` + * dispatch — hence this dedicated handler, which awaits the broadcast + * server-side and returns only the resolved hash (and id/status). + * + * The network client is resolved from `chainId` (via `NetworkController`) when + * `networkClientId` is not given directly, and `from` defaults to the selected + * account. The transaction is submitted as `isInternal` (the daemon is driven + * only by its owner over the `0600` same-user socket), which skips + * origin/permitted-account validation; its approval request is auto-accepted + * by the daemon's auto-approval subscription. + * + * When `dryRun` is set, the network client and sender are resolved and the + * params validated, but nothing is added or broadcast — the resolved plan is + * returned so the CLI can preview it before the user confirms. + * + * @param messenger - The wallet root messenger. + * @param params - Validated `sendTransaction` params. + * @returns The dry-run plan, or the broadcast hash with its id and status. + */ +export async function runSendTransaction( + messenger: Readonly>, + params: SendTransactionParams, +): Promise { + const { + to, + value = '0x0', + data, + gas, + maxFeePerGas, + maxPriorityFeePerGas, + gasPrice, + networkClientId: networkClientIdParam, + chainId, + dryRun, + } = params; + + // The struct guarantees exactly one of `networkClientId` / `chainId`, so + // `chainId` is defined whenever `networkClientId` is not. + const networkClientId = + networkClientIdParam ?? + messenger.call( + 'NetworkController:findNetworkClientIdByChainId', + chainId as Hex, + ); + + const from = + params.from ?? + (messenger.call('AccountsController:getSelectedAccount').address as Hex); + + if (dryRun) { + return { dryRun: true, from, to, value, networkClientId }; + } + + const txParams = { + from, + to, + value, + ...(data === undefined ? {} : { data }), + ...(gas === undefined ? {} : { gas }), + ...(maxFeePerGas === undefined ? {} : { maxFeePerGas }), + ...(maxPriorityFeePerGas === undefined ? {} : { maxPriorityFeePerGas }), + ...(gasPrice === undefined ? {} : { gasPrice }), + }; + + const { result, transactionMeta } = await messenger.call( + 'TransactionController:addTransaction', + txParams, + { networkClientId, origin: INTERNAL_ORIGIN, isInternal: true }, + ); + + // Awaiting the broadcast server-side is the whole point of this handler: the + // `result` promise cannot be serialized back to the CLI, so the daemon + // resolves it and returns the hash instead. + const transactionHash = await result; + + return { + transactionHash, + transactionId: transactionMeta.id, + status: transactionMeta.status, + }; +} From 71ce40d450229bc2184e19235f56e7063534f61a Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 18:02:01 +0300 Subject: [PATCH 2/8] test(wallet-cli): add real-chain send e2e (anvil) and report live tx status Add a true end-to-end test for `mm wallet send`: it boots a local anvil node (seeded with the same mnemonic the wallet imports, so the account is funded), points the daemon at it via a custom `NetworkController:addNetwork`, sends a transaction through the built CLI, and asserts it was signed, broadcast, and mined (receipt `status: 0x1`, recipient balance increased). The suite is skip-if-absent: when the `anvil` binary is not found it is skipped rather than failed. In CI, anvil is installed (via `@metamask/foundryup`) only when files under `packages/wallet-cli/` changed, so unrelated PRs don't pay the Foundry download and the test skips itself. Also report the post-broadcast transaction status by re-reading the live record via `TransactionController:getTransactions`: the `addTransaction` result's `transactionMeta` is the creation snapshot, whose status is still `unapproved`, so the command previously printed a misleading status after a successful send. - Scope the nock net-connect allowlist to exactly the anvil host:port, restored in `afterEach`, so the shared "no real network" safety net stays in place. - Document both e2e suites in `packages/wallet-cli/tests/README.md`. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/lint-build-test.yml | 36 ++ .gitignore | 4 + packages/wallet-cli/package.json | 3 + .../src/daemon/daemon-entry.test.ts | 14 +- .../src/daemon/send-transaction.test.ts | 29 +- .../wallet-cli/src/daemon/send-transaction.ts | 9 +- packages/wallet-cli/tests/README.md | 74 ++++ .../wallet-cli/tests/wallet-send.e2e.test.ts | 412 ++++++++++++++++++ yarn.lock | 4 +- 9 files changed, 578 insertions(+), 7 deletions(-) create mode 100644 packages/wallet-cli/tests/README.md create mode 100644 packages/wallet-cli/tests/wallet-send.e2e.test.ts diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 97ccd2a5015..0835619d828 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -369,6 +369,9 @@ jobs: runs-on: ubuntu-latest if: contains(fromJson(needs.prepare.outputs.package-names), '@metamask/wallet-cli') needs: prepare + permissions: + contents: read + pull-requests: read strategy: matrix: node-version: [20.x, 22.x, 24.x] @@ -381,6 +384,39 @@ jobs: node-version: ${{ matrix.node-version }} - name: Build wallet-cli and its dependencies run: yarn workspaces foreach --topological-dev --recursive --from '@metamask/wallet-cli' run build + # The real-chain send e2e (`wallet-send.e2e.test.ts`) needs `anvil` + # (Foundry). It is skip-if-absent, so only pay the Foundry download when + # wallet-cli actually changed; on other PRs the test skips itself. + - name: Detect whether wallet-cli changed + id: wallet-cli-changed + uses: actions/github-script@v8 + with: + script: | + try { + if (context.eventName !== 'pull_request') { + core.setOutput('changed', 'true'); + return; + } + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + per_page: 100, + }); + const changed = files.some((file) => + file.filename.startsWith('packages/wallet-cli/'), + ); + core.setOutput('changed', String(changed)); + } catch (error) { + // Never let detection break CI: fall back to running the suite. + core.warning( + `wallet-cli change detection failed; running anyway: ${error}`, + ); + core.setOutput('changed', 'true'); + } + - name: Install anvil for the real-chain e2e + if: steps.wallet-cli-changed.outputs.changed == 'true' + run: yarn workspace @metamask/wallet-cli run test:e2e:install-anvil - run: yarn workspace @metamask/wallet-cli run test:e2e - name: Require clean working directory shell: bash diff --git a/.gitignore b/.gitignore index 650326318de..c0fa9873f29 100644 --- a/.gitignore +++ b/.gitignore @@ -57,3 +57,7 @@ packages/*/*.tsbuildinfo # Docusaurus **/.docusaurus packages/wallet-framework-docs/site/build + +# Foundry (anvil) binaries cached by @metamask/foundryup for the wallet-cli +# real-chain e2e (enableGlobalCache is off, so the cache lands in-repo). +.metamask/ diff --git a/packages/wallet-cli/package.json b/packages/wallet-cli/package.json index c6e8a677cfc..bf5a4471082 100644 --- a/packages/wallet-cli/package.json +++ b/packages/wallet-cli/package.json @@ -41,6 +41,7 @@ "test": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", "test:e2e": "yarn test:prepare && NODE_OPTIONS=--experimental-vm-modules jest --config jest.config.e2e.js", + "test:e2e:install-anvil": "mm-foundryup", "test:prepare": "./scripts/install-binaries.sh", "test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose", "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" @@ -62,12 +63,14 @@ }, "devDependencies": { "@metamask/auto-changelog": "^6.1.0", + "@metamask/foundryup": "^1.0.1", "@ts-bridge/cli": "^0.6.4", "@types/better-sqlite3": "^7.6.13", "@types/jest": "^30.0.0", "deepmerge": "^4.2.2", "jest": "^30.4.2", "jest-environment-node": "^30.4.1", + "nock": "^13.3.1", "ts-jest": "^29.4.11", "tsx": "^4.20.5", "typescript": "~5.3.3" diff --git a/packages/wallet-cli/src/daemon/daemon-entry.test.ts b/packages/wallet-cli/src/daemon/daemon-entry.test.ts index dbab9bdf586..0562fb48360 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.test.ts @@ -913,9 +913,17 @@ describe('daemon-entry', () => { it('runs a send through the wallet messenger and returns the hash', async () => { const result = createMockWallet(); const mockCall = result.wallet.messenger.call as jest.Mock; - mockCall.mockReturnValue({ - result: Promise.resolve('0xhash'), - transactionMeta: { id: 'tx-1', status: 'submitted' }, + mockCall.mockImplementation((action: string) => { + if (action === 'TransactionController:addTransaction') { + return { + result: Promise.resolve('0xhash'), + transactionMeta: { id: 'tx-1', status: 'unapproved' }, + }; + } + if (action === 'TransactionController:getTransactions') { + return [{ id: 'tx-1', status: 'submitted' }]; + } + return undefined; }); mockCreateWallet.mockResolvedValue(result); mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); diff --git a/packages/wallet-cli/src/daemon/send-transaction.test.ts b/packages/wallet-cli/src/daemon/send-transaction.test.ts index 3743e5c2262..9a7521b7fb5 100644 --- a/packages/wallet-cli/src/daemon/send-transaction.test.ts +++ b/packages/wallet-cli/src/daemon/send-transaction.test.ts @@ -53,8 +53,13 @@ function defaultHandlers(): Record { 'AccountsController:getSelectedAccount': { address: SELECTED }, 'TransactionController:addTransaction': Promise.resolve({ result: Promise.resolve('0xhash'), - transactionMeta: { id: 'tx-1', status: 'submitted' }, + // The creation snapshot is `unapproved`; the live status below is what + // the handler should report. + transactionMeta: { id: 'tx-1', status: 'unapproved' }, }), + 'TransactionController:getTransactions': [ + { id: 'tx-1', status: 'submitted' }, + ], }; } @@ -280,7 +285,7 @@ describe('runSendTransaction', () => { ); }); - it('awaits the broadcast and returns the hash, id, and status', async () => { + it('awaits the broadcast and returns the hash, id, and live status', async () => { const { messenger } = makeMessenger(defaultHandlers()); const result = await runSendTransaction(messenger, { @@ -288,6 +293,8 @@ describe('runSendTransaction', () => { networkClientId: 'mainnet', } as SendTransactionParams); + // The status comes from the live `getTransactions` re-read, not the + // `unapproved` creation snapshot. expect(result).toStrictEqual({ transactionHash: '0xhash', transactionId: 'tx-1', @@ -295,6 +302,24 @@ describe('runSendTransaction', () => { }); }); + it('falls back to the snapshot status when the tx is no longer in state', async () => { + const { messenger } = makeMessenger({ + ...defaultHandlers(), + 'TransactionController:addTransaction': Promise.resolve({ + result: Promise.resolve('0xhash'), + transactionMeta: { id: 'tx-1', status: 'submitted' }, + }), + 'TransactionController:getTransactions': [], + }); + + const result = await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(result).toMatchObject({ status: 'submitted' }); + }); + it('propagates a broadcast failure', async () => { const { messenger } = makeMessenger({ ...defaultHandlers(), diff --git a/packages/wallet-cli/src/daemon/send-transaction.ts b/packages/wallet-cli/src/daemon/send-transaction.ts index 01b699e62af..3ad766e3765 100644 --- a/packages/wallet-cli/src/daemon/send-transaction.ts +++ b/packages/wallet-cli/src/daemon/send-transaction.ts @@ -180,9 +180,16 @@ export async function runSendTransaction( // resolves it and returns the hash instead. const transactionHash = await result; + // `transactionMeta` is the snapshot from creation, when the status is still + // `unapproved`; re-read the live record so the reported status reflects the + // post-broadcast state (e.g. `submitted`). + const [current] = messenger.call('TransactionController:getTransactions', { + searchCriteria: { id: transactionMeta.id }, + }); + return { transactionHash, transactionId: transactionMeta.id, - status: transactionMeta.status, + status: current?.status ?? transactionMeta.status, }; } diff --git a/packages/wallet-cli/tests/README.md b/packages/wallet-cli/tests/README.md new file mode 100644 index 00000000000..e30315636d7 --- /dev/null +++ b/packages/wallet-cli/tests/README.md @@ -0,0 +1,74 @@ +# wallet-cli end-to-end tests + +These suites spawn the **built** `mm` CLI (and the native `better-sqlite3` +addon) as real child processes, so they live outside the fast unit `test` run +and its 100%-coverage gate. Run them with: + +```sh +yarn workspace @metamask/wallet-cli run test:e2e +``` + +`test:e2e` first runs `test:prepare` (builds the `better-sqlite3` addon), then +runs jest with `jest.config.e2e.js` (which points jest at this `tests/` +directory). + +## Suites + +### `lifecycle.e2e.test.ts` — offline + +Drives the `mm daemon` lifecycle (`start → call → status → stop → purge`) and +`mm wallet unlock` against a temp data directory. It never touches the network: +daemon startup does not fetch feature flags or reach a chain, so it runs +anywhere with no extra setup. + +### `wallet-send.e2e.test.ts` — real chain (requires `anvil`) + +The true end-to-end test for `mm wallet send`: it boots a local +[anvil](https://book.getfoundry.sh/anvil/) node, points the daemon at it with a +custom network, sends a transaction, and asserts it was signed, broadcast, and +mined (receipt `status: 0x1`, recipient balance increased). anvil is started +with the same mnemonic the wallet imports, so the wallet's account is +pre-funded on the local chain. + +**This suite is skip-if-absent.** When the `anvil` binary cannot be found the +whole suite is skipped (not failed), so it never blocks a machine without +Foundry. + +#### Running it locally + +Install Foundry's `anvil`. Either use the repo's helper (installs it into this +package's `node_modules/.bin`): + +```sh +yarn workspace @metamask/wallet-cli run test:e2e:install-anvil +``` + +…or install Foundry the usual way (`curl -L https://foundry.paradigm.xyz | bash +&& foundryup`) so `anvil` is on your `PATH`. You can also point at a specific +binary with `MM_E2E_ANVIL_PATH=/path/to/anvil`. + +Then: + +```sh +yarn workspace @metamask/wallet-cli run test:e2e +``` + +#### Notes for maintainers + +- **Net-connect allowlist.** The shared test setup + (`tests/setupAfterEnv/nock.ts`) disables all real network connections before + every test as a safety net. This suite re-enables **only** `127.0.0.1:` in its `beforeEach` and restores the block in `afterEach`, so + the safety net stays in place everywhere else. HTTP calls to anvil go through + `node:http` (jest's `node` environment does not expose a global `fetch`, and + `tests/setup.ts` clears it). +- **Gas.** The send passes explicit gas overrides so it never depends on the + external gas-estimation API (which has no data for a local chain id). + +## CI + +The `test-wallet-cli-e2e` job runs `test:e2e` on every push/PR. To avoid paying +the Foundry download on unrelated PRs, it installs `anvil` **only when files +under `packages/wallet-cli/` changed** (detected via the PR file list). On PRs +that don't touch wallet-cli, `anvil` is absent and the real-chain suite skips +itself; the offline lifecycle suite always runs. diff --git a/packages/wallet-cli/tests/wallet-send.e2e.test.ts b/packages/wallet-cli/tests/wallet-send.e2e.test.ts new file mode 100644 index 00000000000..04974cfa6e5 --- /dev/null +++ b/packages/wallet-cli/tests/wallet-send.e2e.test.ts @@ -0,0 +1,412 @@ +import { disableNetConnect, enableNetConnect } from 'nock'; +import type { ChildProcess } from 'node:child_process'; +import { spawn } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { request } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { getDaemonPaths } from '../src/daemon/paths.js'; +import { isProcessAlive, readPidFile } from '../src/daemon/utils.js'; + +// True end-to-end test for `mm wallet send`: it drives the BUILT `mm` CLI +// against a REAL local EVM chain (anvil, from Foundry) and asserts that a +// transaction is signed, broadcast, and mined. Unlike `lifecycle.e2e.test.ts` +// (which is deliberately offline), this exercises the whole send path — the +// dedicated `sendTransaction` RPC, the daemon's TransactionController, gas, +// signing via KeyringController, headless auto-approval, and the broadcast to +// the chain. +// +// Anvil is booted with the SAME mnemonic the wallet imports, so the wallet's +// first account is pre-funded (10000 ETH) on the local chain. A custom network +// pointing at the anvil RPC is added at runtime via +// `NetworkController:addNetwork`, and the send selects it by `--chain-id`. +// +// Requires the `anvil` binary. When it is not installed the whole suite is +// SKIPPED (see `resolveAnvilPath`) rather than failed, so it runs locally and +// in the wallet-cli CI job (which installs Foundry) but never blocks a machine +// without Foundry. Install it with Foundry's `foundryup`, or in this repo: +// yarn workspace @metamask/wallet-cli exec node ../foundryup/dist/cli.mjs +// See `tests/README.md`. + +// The canonical BIP-39 test mnemonic. anvil derives the same accounts from it. +const TEST_SRP = 'test test test test test test test test test test test junk'; +const TEST_PASSWORD = 'testpass'; +const TEST_INFURA_PROJECT_ID = '00000000000000000000000000000000'; + +// A custom chain id that does not collide with any built-in network, so the +// added network is unambiguously the one the send resolves. +const CHAIN_ID_DEC = 31337; +const CHAIN_ID_HEX = `0x${CHAIN_ID_DEC.toString(16)}`; + +const BIN_PATH = join(__dirname, '..', 'bin', 'run.mjs'); + +// Spawning the CLI, importing the SRP (real PBKDF2), booting anvil, and mining +// each step is slow; give the whole flow generous room. Set on the describe +// below (not just the `it`) so the anvil-booting `beforeEach` isn't held to +// jest's 5s default hook timeout. +const STEP_TIMEOUT_MS = 120_000; + +/** + * Locate the `anvil` binary, or return `undefined` so the suite can skip. + * + * Checks, in order: an explicit `MM_E2E_ANVIL_PATH` override, the Foundry + * binary Foundryup installs into this package's `node_modules/.bin`, and + * finally `anvil` on `PATH`. + * + * @returns The path (or bare command) to use for `anvil`, or `undefined`. + */ +function resolveAnvilPath(): string | undefined { + const override = process.env.MM_E2E_ANVIL_PATH; + if (override !== undefined && override.length > 0) { + return existsSync(override) ? override : undefined; + } + const local = join(__dirname, '..', 'node_modules', '.bin', 'anvil'); + if (existsSync(local)) { + return local; + } + // Fall back to PATH; if it is not there either, the spawn check below fails + // and the suite is skipped. + return 'anvil'; +} + +const ANVIL_PATH = resolveAnvilPath(); + +type RunResult = { code: number | null; stdout: string; stderr: string }; + +/** + * Run the built `mm` CLI as a child process and capture its output. + * + * @param args - CLI arguments (e.g. `['wallet', 'send', ...]`). + * @param dataDir - Data directory to point the CLI at (via `MM_DATA_DIR`). + * @returns The exit code and captured stdout/stderr. + */ +async function runMm(args: string[], dataDir: string): Promise { + const env = { ...process.env }; + delete env.NODE_OPTIONS; + + const child = spawn(process.execPath, [BIN_PATH, ...args], { + env: { + ...env, + MM_DATA_DIR: dataDir, + INFURA_PROJECT_ID: TEST_INFURA_PROJECT_ID, + MM_WALLET_PASSWORD: TEST_PASSWORD, + MM_WALLET_SRP: TEST_SRP, + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + return new Promise((resolve, reject) => { + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => (stdout += chunk.toString())); + child.stderr.on('data', (chunk) => (stderr += chunk.toString())); + child.on('error', reject); + child.on('close', (code) => resolve({ code, stdout, stderr })); + }); +} + +/** + * Assert a CLI invocation exited 0, embedding output and the daemon log on + * failure. + * + * @param step - Human-readable label for the CLI step. + * @param result - The captured run result. + * @param dataDir - Data directory the daemon is using (to locate its log). + */ +async function expectSuccessfulRun( + step: string, + result: RunResult, + dataDir: string, +): Promise { + if (result.code === 0) { + return; + } + const { logPath } = getDaemonPaths(dataDir); + const daemonLog = await readFile(logPath, 'utf-8').catch( + (error: unknown) => ``, + ); + throw new Error( + `Expected \`mm ${step}\` to exit 0 but it exited ${String(result.code)}.\n` + + `=== stdout ===\n${result.stdout}\n` + + `=== stderr ===\n${result.stderr}\n` + + `=== ${logPath} ===\n${daemonLog}\n`, + ); +} + +/** + * Make a JSON-RPC call directly to the anvil node over `node:http` (jest's + * `node` test environment does not expose a global `fetch`). + * + * @param port - The anvil port. + * @param method - The JSON-RPC method. + * @param params - The JSON-RPC params. + * @returns The `result` field of the response. + */ +async function anvilRpc( + port: number, + method: string, + params: unknown[] = [], +): Promise { + const body = JSON.stringify({ jsonrpc: '2.0', id: 1, method, params }); + return new Promise((resolve, reject) => { + const req = request( + { + host: '127.0.0.1', + port, + method: 'POST', + path: '/', + headers: { + 'content-type': 'application/json', + 'content-length': Buffer.byteLength(body), + }, + }, + (res) => { + let data = ''; + res.on('data', (chunk) => (data += chunk.toString())); + res.on('end', () => { + try { + const json = JSON.parse(data) as { + result?: unknown; + error?: unknown; + }; + if (json.error) { + reject( + new Error( + `anvil ${method} failed: ${JSON.stringify(json.error)}`, + ), + ); + } else { + resolve(json.result); + } + } catch (error) { + reject(error as Error); + } + }); + }, + ); + req.on('error', reject); + req.write(body); + req.end(); + }); +} + +/** + * Spawn anvil on the given port with the test mnemonic and wait until it + * answers JSON-RPC. + * + * @param port - The port to listen on. + * @returns The anvil child process. + */ +async function startAnvil(port: number): Promise { + const anvil = spawn( + ANVIL_PATH as string, + [ + '--port', + String(port), + '--chain-id', + String(CHAIN_ID_DEC), + '--mnemonic', + TEST_SRP, + '--silent', + ], + { stdio: ['ignore', 'pipe', 'pipe'] }, + ); + let output = ''; + anvil.stdout?.on('data', (chunk) => (output += chunk.toString())); + anvil.stderr?.on('data', (chunk) => (output += chunk.toString())); + let exitInfo: string | undefined; + anvil.on('exit', (code, signal) => { + exitInfo = `anvil exited early (code=${String(code)}, signal=${String(signal)})`; + }); + anvil.on('error', (error) => { + output += `\n[spawn error] ${error.message}`; + }); + + // Poll until anvil answers, or time out. + const deadline = Date.now() + 20_000; + for (;;) { + if (exitInfo !== undefined) { + throw new Error(`${exitInfo} [${ANVIL_PATH as string}]\n${output}`); + } + try { + await anvilRpc(port, 'eth_chainId'); + return anvil; + } catch { + if (Date.now() > deadline) { + anvil.kill('SIGKILL'); + throw new Error( + `anvil did not become ready on :${port} [${ANVIL_PATH as string}]\n${output}`, + ); + } + await new Promise((resolve) => setTimeout(resolve, 200)); + } + } +} + +/** + * Kill any daemon recorded in the data dir and remove the temp directory. + * + * @param dataDir - The temp data directory to tear down. + */ +async function cleanupDaemon(dataDir: string): Promise { + const { pidPath } = getDaemonPaths(dataDir); + const pid = await readPidFile(pidPath).catch(() => undefined); + if (pid !== undefined && isProcessAlive(pid)) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + // Already gone. + } + } + await rm(dataDir, { recursive: true, force: true }); +} + +const anvilSpawnable = + ANVIL_PATH !== undefined && + (ANVIL_PATH === 'anvil' || existsSync(ANVIL_PATH)); + +describe('mm wallet send (real chain e2e)', () => { + jest.setTimeout(STEP_TIMEOUT_MS); + + let dataDir: string; + let anvil: ChildProcess | undefined; + let port: number; + + beforeEach(async () => { + if (!anvilSpawnable) { + return; + } + dataDir = await mkdtemp(join(tmpdir(), 'mm-send-e2e-')); + port = 8600 + (process.pid % 400); + + // The shared test setup (`tests/setupAfterEnv/nock.ts`) disables all net + // connect before every test. Re-allow exactly the anvil host:port — nothing + // else — restored in `afterEach`, so the block stays in place elsewhere. + enableNetConnect(`127.0.0.1:${port}`); + + anvil = await startAnvil(port); + }); + + afterEach(async () => { + if (!anvilSpawnable) { + return; + } + await cleanupDaemon(dataDir); + // Await anvil's exit so its child-process handle is gone before jest checks + // for open handles — otherwise the run can hang after the test completes. + if (anvil) { + const exited = new Promise((resolve) => anvil?.once('exit', resolve)); + anvil.kill('SIGKILL'); + await exited; + } + disableNetConnect(); + }); + + it( + 'signs, broadcasts, and mines a transaction on the local chain', + async () => { + if (!anvilSpawnable) { + console.warn( + 'anvil not found; skipping (install Foundry — see tests/README.md).', + ); + return; + } + + // anvil funds the mnemonic's accounts; account[0] is the wallet's + // account, account[1] is our recipient. + const accounts = (await anvilRpc(port, 'eth_accounts')) as string[]; + const recipient = accounts[1]; + + const recipientBalanceBefore = BigInt( + (await anvilRpc(port, 'eth_getBalance', [ + recipient, + 'latest', + ])) as string, + ); + + // Start the daemon (first run imports the SRP and unlocks the wallet). + const start = await runMm(['daemon', 'start'], dataDir); + await expectSuccessfulRun('daemon start', start, dataDir); + + // Point the daemon at the local chain by adding a custom network. + const networkConfig = { + chainId: CHAIN_ID_HEX, + name: 'Anvil Local', + nativeCurrency: 'ETH', + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + rpcEndpoints: [ + { type: 'custom', url: `http://127.0.0.1:${port}`, name: 'anvil' }, + ], + }; + const addNetwork = await runMm( + [ + 'daemon', + 'call', + 'NetworkController:addNetwork', + JSON.stringify([networkConfig]), + ], + dataDir, + ); + await expectSuccessfulRun( + 'daemon call NetworkController:addNetwork', + addNetwork, + dataDir, + ); + + // Send 1 ETH with explicit gas so the flow never depends on the external + // gas-estimation API (which has no data for a local chain id). + const send = await runMm( + [ + 'wallet', + 'send', + '--to', + recipient, + '--value', + '1', + '--chain-id', + CHAIN_ID_HEX, + '--gas', + '0x5208', + '--max-fee-per-gas', + '0x77359400', + '--max-priority-fee-per-gas', + '0x3b9aca00', + '--yes', + ], + dataDir, + ); + await expectSuccessfulRun('wallet send', send, dataDir); + + // The command prints the broadcast hash. + expect(send.stdout).toContain('Transaction broadcast.'); + const hashMatch = send.stdout.match(/Hash:\s+(0x[0-9a-fA-F]{64})/u); + expect(hashMatch).not.toBeNull(); + const hash = (hashMatch as RegExpMatchArray)[1]; + + // The chain confirms the transaction was mined successfully... + const receipt = (await anvilRpc(port, 'eth_getTransactionReceipt', [ + hash, + ])) as { status: string; to: string } | null; + expect(receipt).not.toBeNull(); + expect((receipt as { status: string }).status).toBe('0x1'); + expect((receipt as { to: string }).to.toLowerCase()).toBe( + recipient.toLowerCase(), + ); + + // ...and the recipient's balance grew by the 1 ETH we sent. + const recipientBalanceAfter = BigInt( + (await anvilRpc(port, 'eth_getBalance', [ + recipient, + 'latest', + ])) as string, + ); + expect(recipientBalanceAfter - recipientBalanceBefore).toBe(10n ** 18n); + + await runMm(['daemon', 'stop'], dataDir); + }, + STEP_TIMEOUT_MS, + ); +}); diff --git a/yarn.lock b/yarn.lock index 414dbc97704..d923832ec0c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7266,7 +7266,7 @@ __metadata: languageName: node linkType: hard -"@metamask/foundryup@workspace:packages/foundryup": +"@metamask/foundryup@npm:^1.0.1, @metamask/foundryup@workspace:packages/foundryup": version: 0.0.0-use.local resolution: "@metamask/foundryup@workspace:packages/foundryup" dependencies: @@ -9359,6 +9359,7 @@ __metadata: "@inquirer/password": "npm:^5.1.1" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" + "@metamask/foundryup": "npm:^1.0.1" "@metamask/remote-feature-flag-controller": "npm:^4.2.2" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/scure-bip39": "npm:^2.1.1" @@ -9375,6 +9376,7 @@ __metadata: immer: "npm:^9.0.6" jest: "npm:^30.4.2" jest-environment-node: "npm:^30.4.1" + nock: "npm:^13.3.1" ts-jest: "npm:^29.4.11" tsx: "npm:^4.20.5" typescript: "npm:~5.3.3" From 8bcf057bc92074a3074c49451f17f26b377c5802 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 18:02:57 +0300 Subject: [PATCH 3/8] docs(wallet-cli): point send changelog entry at PR #9636 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index b9099ef8ac1..12b1a504650 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add the `mm wallet send` command and a dedicated daemon `sendTransaction` RPC handler for sending a transaction through the daemon-hosted `TransactionController` ([#9513](https://github.com/MetaMask/core/issues/9513)) +- Add the `mm wallet send` command and a dedicated daemon `sendTransaction` RPC handler for sending a transaction through the daemon-hosted `TransactionController` ([#9636](https://github.com/MetaMask/core/pull/9636)) - The command converts the ether `--value` to wei, resolves the network client (`--network-client-id` or `--chain-id`) and sender (defaulting to the selected account), previews the resolved plan, and broadcasts after confirmation, printing the resulting transaction hash. `--yes` skips the prompt; `--dry-run` resolves and validates without broadcasting. - The `sendTransaction` handler awaits the broadcast server-side and returns a serializable `{ transactionHash, transactionId, status }`, because `addTransaction`'s `result` promise cannot travel back over the generic `call` dispatch. Transactions are submitted as internal (auto-approved by the daemon). - Auto-accept pending approval requests in the daemon so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the subscription is torn down on `dispose` ([#9612](https://github.com/MetaMask/core/pull/9612)) From be448c037134f6c4345b9ee4a0a0c40996d09efa Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 20:55:58 +0300 Subject: [PATCH 4/8] fix(wallet-cli): harden `mm wallet send` per PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review findings on #9636: - Broadcast now uses a 120s default timeout (was the 30s used for cheap RPCs; `--timeout` still overrides) and, on a read-timeout, warns the transaction may already be broadcasting and not to re-run — closing the double-send door that the ECONNRESET no-retry already guards. - Report `submitted` (never the stale `unapproved` creation snapshot) when the live record is gone after broadcast; the fallback test now uses a realistic snapshot and asserts the record is re-read by id. - Preview shows `data` and gas overrides, and the confirmed broadcast pins the sender/network the preview resolved instead of re-resolving them. - Validate daemon responses against shared structs; a malformed broadcast payload now fails loudly instead of printing `Hash: (unknown)`. - Tighten `networkClientId` to a non-empty string at the daemon boundary. - e2e hard-fails when `MM_E2E_REQUIRE_ANVIL` is set but anvil is absent; CI sets it whenever it installs anvil, so a broken install can no longer pass as a green no-op. - Add the `../foundryup` tsconfig project references and the root README dependency-graph edge required by the new workspace devDependency. - Comment/doc accuracy fixes (toWei, isValidHexAddress, e2e fetch reason, gas attribution) and trim repeated rationale. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/lint-build-test.yml | 5 + README.md | 1 + packages/wallet-cli/README.md | 4 +- .../src/commands/wallet/send.test.ts | 111 +++++++- .../wallet-cli/src/commands/wallet/send.ts | 239 +++++++++++++----- .../wallet-cli/src/daemon/daemon-entry.ts | 5 +- .../src/daemon/send-transaction.test.ts | 40 ++- .../wallet-cli/src/daemon/send-transaction.ts | 61 +++-- packages/wallet-cli/tests/README.md | 13 +- .../wallet-cli/tests/wallet-send.e2e.test.ts | 26 +- packages/wallet-cli/tsconfig.build.json | 19 +- packages/wallet-cli/tsconfig.json | 3 + 12 files changed, 415 insertions(+), 112 deletions(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 0835619d828..094e13f798e 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -417,7 +417,12 @@ jobs: - name: Install anvil for the real-chain e2e if: steps.wallet-cli-changed.outputs.changed == 'true' run: yarn workspace @metamask/wallet-cli run test:e2e:install-anvil + # When wallet-cli changed we installed anvil, so require the real-chain + # suite to actually find and run it — otherwise a broken install would + # let the flagship send test pass as a silent green no-op. - run: yarn workspace @metamask/wallet-cli run test:e2e + env: + MM_E2E_REQUIRE_ANVIL: ${{ steps.wallet-cli-changed.outputs.changed }} - name: Require clean working directory shell: bash run: | diff --git a/README.md b/README.md index 24c9ede1fed..55e827db319 100644 --- a/README.md +++ b/README.md @@ -664,6 +664,7 @@ linkStyle default opacity:0.5 wallet_cli --> remote_feature_flag_controller; wallet_cli --> storage_service; wallet_cli --> wallet; + wallet_cli --> foundryup; ``` diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index 66783fbaf22..7cce2498075 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -40,7 +40,7 @@ mm wallet send --to 0xRecipient --value 0.01 --network-client-id mainnet --yes mm wallet send --to 0xContract --data 0xabcdef --value 0 --chain-id 0x1 --dry-run # resolve + validate only ``` -Because the daemon auto-approves (see the security note below), the confirmation prompt — or your explicit `--yes` — is the only boundary before funds move; use `--dry-run` first if unsure. Gas is estimated via `GasFeeController` unless overridden with `--gas` / `--max-fee-per-gas` / `--max-priority-fee-per-gas` / `--gas-price` (each a `0x`-prefixed hex quantity). +Because the daemon auto-approves (see the security note below), the confirmation prompt — or your explicit `--yes` — is the only boundary before funds move; use `--dry-run` first if unsure. Gas is estimated automatically unless overridden with `--gas` / `--max-fee-per-gas` / `--max-priority-fee-per-gas` / `--gas-price` (each a `0x`-prefixed hex quantity). Discover what the running wallet can do — `list` prints every messenger action currently dispatchable via `call`. This surface grows as more controllers are wired, so treat it as evolving rather than a stability contract: @@ -67,7 +67,7 @@ mm daemon purge # stop, then delete all daemon state files (--force to State (socket, PID file, log, and the SQLite database) lives in the per-user oclif data directory; override it with `MM_DATA_DIR`. -> **Security model — the daemon auto-approves everything.** Because it is headless, the daemon accepts every approval request (transactions and signatures included) with no per-request prompt; otherwise an awaited request would hang forever with no UI to resolve it. The trust boundary is therefore the daemon's `0600`, same-user Unix socket — anything that can reach the socket can move funds. For `mm wallet send` the confirmation prompt (or `--yes`) is the only per-transaction gate; a scoped/opt-in approval policy at the daemon level is still planned. +> **Security model — the daemon auto-approves everything.** Because it is headless, the daemon accepts every approval request (transactions and signatures included) with no per-request prompt; otherwise an awaited request would hang forever with no UI to resolve it. The trust boundary is therefore the daemon's `0600`, same-user Unix socket — anything that can reach the socket can move funds. For `mm wallet send` the confirmation prompt (or `--yes`) is the only per-transaction gate; the daemon itself still applies no per-request policy. ## Troubleshooting diff --git a/packages/wallet-cli/src/commands/wallet/send.test.ts b/packages/wallet-cli/src/commands/wallet/send.test.ts index a8ca917f2b5..69eb66d70fb 100644 --- a/packages/wallet-cli/src/commands/wallet/send.test.ts +++ b/packages/wallet-cli/src/commands/wallet/send.test.ts @@ -48,6 +48,12 @@ describe('parseEtherToWeiHex', () => { ['0.01', '0x2386f26fc10000'], ['0.1', '0x16345785d8a0000'], ['123.456', '0x6b14bd1e6eea00000'], + // Exactly 18 fractional digits is the accepted boundary (19 is rejected). + ['0.123456789012345678', '0x1b69b4ba630f34e'], + // Leading zeros are accepted and parsed by value. + ['007', '0x6124fee993bc0000'], + ['00.5', '0x6f05b59d3b20000'], + ['01', '0xde0b6b3a7640000'], ])('converts %s ether to %s wei', (input, expected) => { expect(parseEtherToWeiHex(input)).toBe(expected); }); @@ -135,12 +141,16 @@ describe('wallet send', () => { }), }), ); - // ...the second is the real broadcast, without dryRun. + // ...the second is the real broadcast, without dryRun. It pins the + // sender/network the preview resolved rather than re-sending --chain-id. const secondParams = mockSendCommand.mock.calls[1]?.[0]?.params as Record< string, unknown >; expect(secondParams.dryRun).toBeUndefined(); + expect(secondParams.networkClientId).toBe('mainnet'); + expect(secondParams.from).toBe(FROM); + expect(secondParams.chainId).toBeUndefined(); expect(mockConfirmSend).toHaveBeenCalledTimes(1); expect(stdout).toContain('Transaction broadcast.'); expect(stdout).toContain('0xhash'); @@ -208,7 +218,37 @@ describe('wallet send', () => { }), ); expect(mockConfirmSend).not.toHaveBeenCalled(); - expect(stdout).toContain('0xhash'); + expect(stdout).toContain('Hash: 0xhash'); + expect(stdout).toContain('Id: tx-1'); + expect(stdout).toContain('Status: submitted'); + }); + + it('shows data and gas overrides in the preview', async () => { + queueResults(DRY_RUN_RESULT); + + const { stdout } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + '--data', + '0xabcdef', + '--gas', + '0x5208', + '--max-fee-per-gas', + '0x2', + '--max-priority-fee-per-gas', + '0x1', + '--gas-price', + '0x3', + '--dry-run', + ]); + + expect(stdout).toContain('Data: 0xabcdef'); + expect(stdout).toContain('gas=0x5208'); + expect(stdout).toContain('maxFeePerGas=0x2'); + expect(stdout).toContain('maxPriorityFeePerGas=0x1'); + expect(stdout).toContain('gasPrice=0x3'); }); it('previews and stops with --dry-run', async () => { @@ -274,10 +314,12 @@ describe('wallet send', () => { ); }); - it('prints (unknown) for fields missing from the result payload', async () => { + it('errors loudly when the broadcast result is missing fields', async () => { + // Missing transactionId/status: the daemon's result contract drifted. On a + // fund-moving send this must fail loudly, not print a half-blank success. queueResults({ transactionHash: '0xhash' }); - const { stdout } = await runCommand(WalletSend, [ + const { error } = await runCommand(WalletSend, [ '--to', TO, '--network-client-id', @@ -285,9 +327,64 @@ describe('wallet send', () => { '--yes', ]); - expect(stdout).toContain('Hash: 0xhash'); - expect(stdout).toContain('Id: (unknown)'); - expect(stdout).toContain('Status: (unknown)'); + expect(error?.message).toContain('unexpected send'); + }); + + it('errors when the dry-run result is malformed', async () => { + // Missing networkClientId: fails dry-run result validation. + queueResults({ dryRun: true, from: FROM, to: TO, value: '0x1' }); + + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + '--dry-run', + ]); + + expect(error?.message).toContain('unexpected dry-run'); + }); + + it('warns of a possible in-flight send when the broadcast times out', async () => { + mockSendCommand.mockRejectedValue(new Error('Socket read timed out')); + + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + 'mainnet', + '--yes', + ]); + + expect(error?.message).toContain('still be broadcasting'); + expect(error?.message).toContain('twice'); + }); + + it('reports a preview timeout as a plain connection error', async () => { + mockSendCommand.mockRejectedValue(new Error('Socket read timed out')); + + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--chain-id', + '0x1', + ]); + + expect(error?.message).toContain('timed out'); + expect(error?.message).not.toContain('twice'); + expect(mockConfirmSend).not.toHaveBeenCalled(); + }); + + it('treats an empty --network-client-id as absent', async () => { + const { error } = await runCommand(WalletSend, [ + '--to', + TO, + '--network-client-id', + '', + ]); + + expect(error?.message).toContain('exactly one'); + expect(mockSendCommand).not.toHaveBeenCalled(); }); it('surfaces a JSON-RPC failure from the daemon', async () => { diff --git a/packages/wallet-cli/src/commands/wallet/send.ts b/packages/wallet-cli/src/commands/wallet/send.ts index df3e78bfeb1..95911ab7af3 100644 --- a/packages/wallet-cli/src/commands/wallet/send.ts +++ b/packages/wallet-cli/src/commands/wallet/send.ts @@ -1,3 +1,5 @@ +import { is } from '@metamask/superstruct'; +import type { Struct } from '@metamask/superstruct'; import { bigIntToHex, isJsonRpcFailure, toWei } from '@metamask/utils'; import type { Hex, Json, JsonRpcParams } from '@metamask/utils'; import { Command, Flags } from '@oclif/core'; @@ -5,6 +7,11 @@ import { Command, Flags } from '@oclif/core'; import { sendCommand } from '../../daemon/daemon-client.js'; import { getDaemonPaths } from '../../daemon/paths.js'; import { confirmSend } from '../../daemon/prompts.js'; +import { + SendTransactionBroadcastResultStruct, + SendTransactionDryRunResultStruct, +} from '../../daemon/send-transaction.js'; +import type { SendTransactionDryRunResult } from '../../daemon/send-transaction.js'; import { emptyToUndefined, formatJsonRpcError, @@ -13,18 +20,39 @@ import { const ETHER_DECIMALS = 18; +/** + * Fallback response timeout (ms) for the broadcast call. Broadcasting waits for + * signing and `eth_sendRawTransaction` server-side, which can far outlast the + * 30s default used for cheap RPCs; too short a timeout would make a slow but + * successful send look like a failure and tempt a duplicate re-send. + * `--timeout` overrides it. + */ +const BROADCAST_TIMEOUT_MS = 120_000; + +/** + * The transaction fields shown in the confirmation preview that the daemon + * does not echo back (they are the raw `--data` / gas overrides the user + * passed, so the CLI supplies them for display). + */ +type PlanExtras = { + data?: string | undefined; + gas?: string | undefined; + maxFeePerGas?: string | undefined; + maxPriorityFeePerGas?: string | undefined; + gasPrice?: string | undefined; +}; + /** * Convert a decimal ether amount to a `0x`-prefixed wei quantity. * - * The fixed-point conversion is delegated to `@metamask/utils`' `toWei` — the - * shared, audited implementation (a bigint-based reimplementation of - * `@metamask/ethjs-unit`) — rather than re-deriving the wei math here. The - * daemon's `sendTransaction` boundary expects canonical hex wei; this is where - * the human-friendly `--value` (ether) is turned into it. + * The fixed-point conversion is delegated to `@metamask/utils`' `toWei` rather + * than re-deriving the wei math here. The daemon's `sendTransaction` boundary + * expects canonical hex wei; this is where the human-friendly `--value` (ether) + * is turned into it. * - * `toWei` itself accepts negatives and scientific notation and throws opaque - * messages, so the input is guarded up front for a clear error and a strictly - * non-negative value. + * `toWei` silently accepts negatives and throws opaque messages on other + * malformed input, so the value is guarded up front for a clear error and a + * strictly non-negative amount. * * @param amount - A non-negative decimal ether amount (e.g. `"0.1"`, `"1"`). * @returns The equivalent wei as a `0x`-prefixed hex string. @@ -51,9 +79,9 @@ export function parseEtherToWeiHex(amount: string): Hex { export default class WalletSend extends Command { static override description = 'Send a transaction through the daemon-hosted TransactionController. ' + - 'Resolves gas via GasFeeController estimates unless overridden, signs, ' + - 'broadcasts, and prints the resulting transaction hash. The daemon ' + - 'auto-approves, so the confirmation boundary is this command.'; + 'Estimates gas automatically unless overridden, signs, broadcasts, and ' + + 'prints the resulting transaction hash. The daemon auto-approves, so the ' + + 'confirmation boundary is this command.'; static override examples = [ '<%= config.bin %> wallet send --to 0xRecipient --value 0.01 --chain-id 0x1', @@ -130,11 +158,12 @@ export default class WalletSend extends Command { this.error((error as Error).message); } - const params: Record = { + // Everything except the network selector. The selector is added per call: + // a confirmed broadcast pins the `networkClientId` the preview resolved + // rather than re-sending `--chain-id` and re-resolving it. + const baseParams: Record = { to: flags.to, value, - ...(networkClientId === undefined ? {} : { networkClientId }), - ...(chainId === undefined ? {} : { chainId }), ...(flags.from ? { from: flags.from } : {}), ...(flags.data ? { data: flags.data } : {}), ...(flags.gas ? { gas: flags.gas } : {}), @@ -146,34 +175,55 @@ export default class WalletSend extends Command { : {}), ...(flags['gas-price'] ? { gasPrice: flags['gas-price'] } : {}), }; + // Exactly one selector is defined (guarded above); guard each spread by its + // own `undefined` check so the object never carries an `undefined` value. + const params: Record = { + ...baseParams, + ...(networkClientId === undefined ? {} : { networkClientId }), + ...(chainId === undefined ? {} : { chainId }), + }; + const planExtras: PlanExtras = { + data: flags.data, + gas: flags.gas, + maxFeePerGas: flags['max-fee-per-gas'], + maxPriorityFeePerGas: flags['max-priority-fee-per-gas'], + gasPrice: flags['gas-price'], + }; const { socketPath } = getDaemonPaths(this.config.dataDir); const timeoutMs = flags.timeout; - // A `dryRun` resolves the network client and sender and validates params - // server-side without broadcasting. `--dry-run` stops after previewing; - // an interactive run previews first so the confirmation shows the resolved - // sender/network. `--yes` skips both and broadcasts directly. + // A `dryRun` resolves the sender and network client server-side without + // broadcasting. `--dry-run` stops after previewing; an interactive run + // previews first so the confirmation shows (and then pins) the resolved + // sender/network; `--yes` skips straight to the broadcast. if (flags['dry-run']) { - const preview = await this.#dispatchSend( + const preview = await this.#dispatchSend({ socketPath, - { ...params, dryRun: true }, + params: { ...params, dryRun: true }, timeoutMs, - ); - this.log(formatPlan(preview, flags.value)); + struct: SendTransactionDryRunResultStruct, + broadcast: false, + }); + this.log(formatPlan(preview, flags.value, planExtras)); return; } + let resolved: SendTransactionDryRunResult | undefined; if (!flags.yes) { - const preview = await this.#dispatchSend( + const preview = await this.#dispatchSend({ socketPath, - { ...params, dryRun: true }, + params: { ...params, dryRun: true }, timeoutMs, - ); + struct: SendTransactionDryRunResultStruct, + broadcast: false, + }); let confirmed: boolean; try { - confirmed = await confirmSend(formatPlan(preview, flags.value)); + confirmed = await confirmSend( + formatPlan(preview, flags.value, planExtras), + ); } catch (error) { // Ctrl+C at the prompt (@inquirer/core's ExitPromptError) is a clean // abort, not a failure; anything else should surface. @@ -187,29 +237,58 @@ export default class WalletSend extends Command { this.log('Aborted.'); return; } + resolved = preview; } - const result = await this.#dispatchSend(socketPath, params, timeoutMs); + // Broadcast the exact sender/network the user reviewed (when they confirmed + // a preview), so what is signed matches what was shown. `--yes` skips the + // preview, so there it re-resolves server-side from `params`. + const broadcastParams: Record = resolved + ? { + ...baseParams, + from: resolved.from, + networkClientId: resolved.networkClientId, + } + : params; + + const result = await this.#dispatchSend({ + socketPath, + params: broadcastParams, + timeoutMs: timeoutMs ?? BROADCAST_TIMEOUT_MS, + struct: SendTransactionBroadcastResultStruct, + broadcast: true, + }); this.log('Transaction broadcast.'); - this.log(`Hash: ${stringField(result, 'transactionHash')}`); - this.log(`Id: ${stringField(result, 'transactionId')}`); - this.log(`Status: ${stringField(result, 'status')}`); + this.log(`Hash: ${result.transactionHash}`); + this.log(`Id: ${result.transactionId}`); + this.log(`Status: ${result.status}`); } /** - * Send a `sendTransaction` RPC to the daemon and return its result payload, - * translating connection errors and JSON-RPC failures into command errors. + * Send a `sendTransaction` RPC to the daemon and return its validated result, + * translating connection errors, broadcast timeouts, JSON-RPC failures, and + * unexpected payloads into command errors. * - * @param socketPath - The daemon Unix socket path. - * @param params - The `sendTransaction` params (with or without `dryRun`). - * @param timeoutMs - Optional response timeout in milliseconds. - * @returns The JSON-RPC `result` as a record. + * @param options - Dispatch options. + * @param options.socketPath - The daemon Unix socket path. + * @param options.params - The `sendTransaction` params (with or without + * `dryRun`). + * @param options.timeoutMs - Optional response timeout in milliseconds. + * @param options.struct - Struct the `result` payload must satisfy; the + * return type is inferred from it. + * @param options.broadcast - Whether this is the real (fund-moving) broadcast. + * When true, a read timeout is reported with guidance that the send may + * already be in flight, so the user does not blindly re-send. + * @returns The validated `result` payload. */ - async #dispatchSend( - socketPath: string, - params: JsonRpcParams, - timeoutMs: number | undefined, - ): Promise> { + async #dispatchSend(options: { + socketPath: string; + params: JsonRpcParams; + timeoutMs: number | undefined; + struct: Struct; + broadcast: boolean; + }): Promise { + const { socketPath, params, timeoutMs, struct, broadcast } = options; let response; try { response = await sendCommand({ @@ -219,6 +298,9 @@ export default class WalletSend extends Command { ...(timeoutMs === undefined ? {} : { timeoutMs }), }); } catch (error) { + if (broadcast && isReadTimeout(error)) { + this.error(broadcastTimeoutMessage()); + } this.error(makeDaemonConnectionError(error)); } @@ -226,21 +308,43 @@ export default class WalletSend extends Command { this.error(formatJsonRpcError(response.error)); } - return response.result as Record; + const { result } = response; + if (!is(result, struct)) { + this.error( + `The daemon returned an unexpected ${broadcast ? 'send' : 'dry-run'} ` + + 'result. It may be running an incompatible version.', + ); + } + return result; } } /** - * Read a string field off a JSON-RPC result record, tolerating a missing or - * non-string value so output never crashes on an unexpected payload shape. + * Whether an error is the daemon-client socket read timeout. It carries no + * errno code, so its message is the only signal. * - * @param result - The JSON-RPC result record. - * @param key - The field to read. - * @returns The field as a string, or `'(unknown)'` if absent/non-string. + * @param error - The caught value. + * @returns True if it is the read timeout. */ -function stringField(result: Record, key: string): string { - const value = result[key]; - return typeof value === 'string' ? value : '(unknown)'; +function isReadTimeout(error: unknown): boolean { + return error instanceof Error && error.message === 'Socket read timed out'; +} + +/** + * The message shown when the broadcast call times out. A send is not + * idempotent, and the daemon may still be broadcasting after the client gives + * up waiting, so this warns against a blind re-run rather than reading as a + * plain connection failure. + * + * @returns The user-facing guidance. + */ +function broadcastTimeoutMessage(): string { + return ( + 'The daemon did not respond in time, but your transaction may still be ' + + 'broadcasting. Do NOT re-run this command — you could send it twice. ' + + 'Check `mm daemon status`, the daemon log, or your account on-chain to ' + + 'confirm, then use --timeout to wait longer if needed.' + ); } /** @@ -249,14 +353,35 @@ function stringField(result: Record, key: string): string { * @param plan - The dry-run result returned by the daemon. * @param etherAmount - The original `--value` (ether), shown alongside the * resolved wei so the user sees the human amount they typed. + * @param extras - The `--data` / gas overrides the daemon does not echo back, + * shown so the user confirms exactly what will be sent. * @returns A multi-line summary of the transaction to be sent. */ -function formatPlan(plan: Record, etherAmount: string): string { - return [ +function formatPlan( + plan: SendTransactionDryRunResult, + etherAmount: string, + extras: PlanExtras, +): string { + const lines = [ 'About to send:', - ` To: ${stringField(plan, 'to')}`, - ` From: ${stringField(plan, 'from')}`, - ` Value: ${etherAmount} ETH (${stringField(plan, 'value')} wei)`, - ` Network: ${stringField(plan, 'networkClientId')}`, - ].join('\n'); + ` To: ${plan.to}`, + ` From: ${plan.from}`, + ` Value: ${etherAmount} ETH (${plan.value} wei)`, + ` Network: ${plan.networkClientId}`, + ]; + if (extras.data) { + lines.push(` Data: ${extras.data}`); + } + const gasParts = [ + extras.gas ? `gas=${extras.gas}` : undefined, + extras.maxFeePerGas ? `maxFeePerGas=${extras.maxFeePerGas}` : undefined, + extras.maxPriorityFeePerGas + ? `maxPriorityFeePerGas=${extras.maxPriorityFeePerGas}` + : undefined, + extras.gasPrice ? `gasPrice=${extras.gasPrice}` : undefined, + ].filter((part): part is string => part !== undefined); + if (gasParts.length > 0) { + lines.push(` Gas: ${gasParts.join(', ')}`); + } + return lines.join('\n'); } diff --git a/packages/wallet-cli/src/daemon/daemon-entry.ts b/packages/wallet-cli/src/daemon/daemon-entry.ts index 2f324bccabb..5d153b8f82d 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.ts @@ -165,9 +165,8 @@ async function main(): Promise { async (): Promise => constructedWallet.messenger.getRegisteredActionTypes(), ), - // Dedicated send handler: `addTransaction` returns a non-serializable - // `result` promise, so it cannot travel back over the generic `call` - // dispatch. This awaits the broadcast server-side and returns the hash. + // Dedicated send handler; see `runSendTransaction` for why the generic + // `call` dispatch cannot carry a broadcast result. sendTransaction: defineHandler( SendTransactionParamsStruct, async (params) => diff --git a/packages/wallet-cli/src/daemon/send-transaction.test.ts b/packages/wallet-cli/src/daemon/send-transaction.test.ts index 9a7521b7fb5..ed3dd12f5b3 100644 --- a/packages/wallet-cli/src/daemon/send-transaction.test.ts +++ b/packages/wallet-cli/src/daemon/send-transaction.test.ts @@ -126,6 +126,14 @@ describe('SendTransactionParamsStruct', () => { ); }); + it('rejects an empty networkClientId', () => { + const [error] = validate( + { to: TO, networkClientId: '' }, + SendTransactionParamsStruct, + ); + expect(error).toBeDefined(); + }); + it('rejects unknown keys', () => { const [error] = validate( { ...base, surprise: true }, @@ -286,15 +294,18 @@ describe('runSendTransaction', () => { }); it('awaits the broadcast and returns the hash, id, and live status', async () => { - const { messenger } = makeMessenger(defaultHandlers()); + const { messenger, call } = makeMessenger(defaultHandlers()); const result = await runSendTransaction(messenger, { to: TO, networkClientId: 'mainnet', } as SendTransactionParams); - // The status comes from the live `getTransactions` re-read, not the - // `unapproved` creation snapshot. + // Re-reads the just-created transaction by its own id... + expect(call).toHaveBeenCalledWith('TransactionController:getTransactions', { + searchCriteria: { id: 'tx-1' }, + }); + // ...and reports that live status, not the `unapproved` creation snapshot. expect(result).toStrictEqual({ transactionHash: '0xhash', transactionId: 'tx-1', @@ -302,13 +313,12 @@ describe('runSendTransaction', () => { }); }); - it('falls back to the snapshot status when the tx is no longer in state', async () => { + it('reports submitted when the tx is no longer in state', async () => { + // The creation snapshot (from `defaultHandlers`) is `unapproved`, and the + // live record is gone. Reaching the return means the broadcast resolved, so + // the status must be `submitted`, never the stale `unapproved` snapshot. const { messenger } = makeMessenger({ ...defaultHandlers(), - 'TransactionController:addTransaction': Promise.resolve({ - result: Promise.resolve('0xhash'), - transactionMeta: { id: 'tx-1', status: 'submitted' }, - }), 'TransactionController:getTransactions': [], }); @@ -320,6 +330,20 @@ describe('runSendTransaction', () => { expect(result).toMatchObject({ status: 'submitted' }); }); + it('reports submitted when the live record carries no status', async () => { + const { messenger } = makeMessenger({ + ...defaultHandlers(), + 'TransactionController:getTransactions': [{ id: 'tx-1' }], + }); + + const result = await runSendTransaction(messenger, { + to: TO, + networkClientId: 'mainnet', + } as SendTransactionParams); + + expect(result).toMatchObject({ status: 'submitted' }); + }); + it('propagates a broadcast failure', async () => { const { messenger } = makeMessenger({ ...defaultHandlers(), diff --git a/packages/wallet-cli/src/daemon/send-transaction.ts b/packages/wallet-cli/src/daemon/send-transaction.ts index 3ad766e3765..cfec7e85bef 100644 --- a/packages/wallet-cli/src/daemon/send-transaction.ts +++ b/packages/wallet-cli/src/daemon/send-transaction.ts @@ -1,6 +1,8 @@ import { boolean, define, + literal, + nonempty, object, optional, refine, @@ -25,8 +27,8 @@ const INTERNAL_ORIGIN = 'metamask'; /** * Struct for a `0x`-prefixed 20-byte hex address. `isValidHexAddress` accepts - * any-case addresses (checksum optional), which is what a CLI user is most - * likely to paste. + * an all-lowercase address or a valid EIP-55 checksummed one, so a plain + * lowercase paste works while a mistyped mixed-case address is rejected. */ const AddressStruct = define('Address', (value) => typeof value === 'string' && isValidHexAddress(value as Hex) @@ -54,7 +56,7 @@ export const SendTransactionParamsStruct = refine( maxFeePerGas: optional(StrictHexStruct), maxPriorityFeePerGas: optional(StrictHexStruct), gasPrice: optional(StrictHexStruct), - networkClientId: optional(string()), + networkClientId: optional(nonempty(string())), chainId: optional(StrictHexStruct), dryRun: optional(boolean()), }), @@ -74,25 +76,36 @@ export type SendTransactionParams = Infer; /** * The resolved plan a `dryRun` returns: the network client and sender the - * daemon would use, without adding or broadcasting anything. + * daemon would use, without adding or broadcasting anything. Exported as a + * struct so the CLI can validate the payload it reads back over JSON-RPC + * (which erases types on the wire) rather than trusting its shape. */ -export type SendTransactionDryRunResult = { - dryRun: true; - from: Hex; - to: Hex; - value: Hex; - networkClientId: string; -}; +export const SendTransactionDryRunResultStruct = object({ + dryRun: literal(true), + from: StrictHexStruct, + to: StrictHexStruct, + value: StrictHexStruct, + networkClientId: string(), +}); /** * The outcome of a broadcast send: the on-chain hash plus the id/status the - * daemon tracks the transaction under. + * daemon tracks the transaction under. Exported as a struct for the same + * client-side validation reason as {@link SendTransactionDryRunResultStruct}. */ -export type SendTransactionBroadcastResult = { - transactionHash: string; - transactionId: string; - status: string; -}; +export const SendTransactionBroadcastResultStruct = object({ + transactionHash: string(), + transactionId: string(), + status: string(), +}); + +export type SendTransactionDryRunResult = Infer< + typeof SendTransactionDryRunResultStruct +>; + +export type SendTransactionBroadcastResult = Infer< + typeof SendTransactionBroadcastResultStruct +>; export type SendTransactionResult = | SendTransactionDryRunResult @@ -175,14 +188,14 @@ export async function runSendTransaction( { networkClientId, origin: INTERNAL_ORIGIN, isInternal: true }, ); - // Awaiting the broadcast server-side is the whole point of this handler: the - // `result` promise cannot be serialized back to the CLI, so the daemon - // resolves it and returns the hash instead. + // `result` resolves only once the transaction is signed and broadcast (see + // the function JSDoc); awaiting it here is what this handler exists for. const transactionHash = await result; - // `transactionMeta` is the snapshot from creation, when the status is still - // `unapproved`; re-read the live record so the reported status reflects the - // post-broadcast state (e.g. `submitted`). + // Re-read the live record for the current status: `transactionMeta` is the + // creation snapshot (still `unapproved`). If the record is already gone, + // fall back to `submitted` — reaching this point means `result` resolved, so + // the transaction was broadcast, never merely `unapproved`. const [current] = messenger.call('TransactionController:getTransactions', { searchCriteria: { id: transactionMeta.id }, }); @@ -190,6 +203,6 @@ export async function runSendTransaction( return { transactionHash, transactionId: transactionMeta.id, - status: current?.status ?? transactionMeta.status, + status: current?.status ?? 'submitted', }; } diff --git a/packages/wallet-cli/tests/README.md b/packages/wallet-cli/tests/README.md index e30315636d7..a3bc009b92e 100644 --- a/packages/wallet-cli/tests/README.md +++ b/packages/wallet-cli/tests/README.md @@ -32,7 +32,9 @@ pre-funded on the local chain. **This suite is skip-if-absent.** When the `anvil` binary cannot be found the whole suite is skipped (not failed), so it never blocks a machine without -Foundry. +Foundry. Set `MM_E2E_REQUIRE_ANVIL=true` to turn that skip into a hard failure +— use it wherever anvil is expected (CI does, whenever it installed it) so a +broken install surfaces loudly instead of passing as a green no-op. #### Running it locally @@ -60,8 +62,8 @@ yarn workspace @metamask/wallet-cli run test:e2e every test as a safety net. This suite re-enables **only** `127.0.0.1:` in its `beforeEach` and restores the block in `afterEach`, so the safety net stays in place everywhere else. HTTP calls to anvil go through - `node:http` (jest's `node` environment does not expose a global `fetch`, and - `tests/setup.ts` clears it). + `node:http` because the shared `tests/setup.ts` deletes `globalThis.fetch` + (so nock/undici can intercept), leaving no global `fetch` to use. - **Gas.** The send passes explicit gas overrides so it never depends on the external gas-estimation API (which has no data for a local chain id). @@ -72,3 +74,8 @@ the Foundry download on unrelated PRs, it installs `anvil` **only when files under `packages/wallet-cli/` changed** (detected via the PR file list). On PRs that don't touch wallet-cli, `anvil` is absent and the real-chain suite skips itself; the offline lifecycle suite always runs. + +When it does install `anvil`, the job also sets `MM_E2E_REQUIRE_ANVIL=true`, so +the real-chain suite fails loudly if the binary is nonetheless missing rather +than skipping green — that would otherwise hide the loss of the only test that +exercises a real broadcast. diff --git a/packages/wallet-cli/tests/wallet-send.e2e.test.ts b/packages/wallet-cli/tests/wallet-send.e2e.test.ts index 04974cfa6e5..d22b5ac2856 100644 --- a/packages/wallet-cli/tests/wallet-send.e2e.test.ts +++ b/packages/wallet-cli/tests/wallet-send.e2e.test.ts @@ -27,8 +27,10 @@ import { isProcessAlive, readPidFile } from '../src/daemon/utils.js'; // SKIPPED (see `resolveAnvilPath`) rather than failed, so it runs locally and // in the wallet-cli CI job (which installs Foundry) but never blocks a machine // without Foundry. Install it with Foundry's `foundryup`, or in this repo: -// yarn workspace @metamask/wallet-cli exec node ../foundryup/dist/cli.mjs -// See `tests/README.md`. +// yarn workspace @metamask/wallet-cli run test:e2e:install-anvil +// Set `MM_E2E_REQUIRE_ANVIL=true` to turn the skip into a hard failure — CI +// does this whenever it installed anvil, so a broken install surfaces loudly +// instead of passing as a green no-op. See `tests/README.md`. // The canonical BIP-39 test mnemonic. anvil derives the same accounts from it. const TEST_SRP = 'test test test test test test test test test test test junk'; @@ -136,8 +138,9 @@ async function expectSuccessfulRun( } /** - * Make a JSON-RPC call directly to the anvil node over `node:http` (jest's - * `node` test environment does not expose a global `fetch`). + * Make a JSON-RPC call directly to the anvil node over `node:http` (the shared + * `tests/setup.ts` deletes `globalThis.fetch` so nock/undici can intercept, so + * `fetch` is not available here). * * @param port - The anvil port. * @param method - The JSON-RPC method. @@ -267,6 +270,11 @@ const anvilSpawnable = ANVIL_PATH !== undefined && (ANVIL_PATH === 'anvil' || existsSync(ANVIL_PATH)); +// CI sets this whenever it installed anvil (i.e. wallet-cli changed), so an +// install that silently produced no usable binary fails the suite loudly +// rather than letting the whole real-chain test skip itself green. +const anvilRequired = process.env.MM_E2E_REQUIRE_ANVIL === 'true'; + describe('mm wallet send (real chain e2e)', () => { jest.setTimeout(STEP_TIMEOUT_MS); @@ -274,6 +282,16 @@ describe('mm wallet send (real chain e2e)', () => { let anvil: ChildProcess | undefined; let port: number; + beforeAll(() => { + if (anvilRequired && !anvilSpawnable) { + throw new Error( + 'MM_E2E_REQUIRE_ANVIL is set but the anvil binary was not found. ' + + 'The real-chain send e2e cannot run; check the Foundry install ' + + '(see tests/README.md).', + ); + } + }); + beforeEach(async () => { if (!anvilSpawnable) { return; diff --git a/packages/wallet-cli/tsconfig.build.json b/packages/wallet-cli/tsconfig.build.json index 934a11754a4..d2b9782342e 100644 --- a/packages/wallet-cli/tsconfig.build.json +++ b/packages/wallet-cli/tsconfig.build.json @@ -6,10 +6,21 @@ "rootDir": "./src" }, "references": [ - { "path": "../base-controller/tsconfig.build.json" }, - { "path": "../remote-feature-flag-controller/tsconfig.build.json" }, - { "path": "../storage-service/tsconfig.build.json" }, - { "path": "../wallet/tsconfig.build.json" } + { + "path": "../base-controller/tsconfig.build.json" + }, + { + "path": "../remote-feature-flag-controller/tsconfig.build.json" + }, + { + "path": "../storage-service/tsconfig.build.json" + }, + { + "path": "../wallet/tsconfig.build.json" + }, + { + "path": "../foundryup/tsconfig.build.json" + } ], "include": ["../../types", "./src"] } diff --git a/packages/wallet-cli/tsconfig.json b/packages/wallet-cli/tsconfig.json index d0fd77d3457..858478a80e5 100644 --- a/packages/wallet-cli/tsconfig.json +++ b/packages/wallet-cli/tsconfig.json @@ -15,6 +15,9 @@ }, { "path": "../wallet" + }, + { + "path": "../foundryup" } ], "include": ["../../types", "./bin", "./src", "./tests"] From d6dcff539a186459a1b50e3358e350d1210e221b Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 24 Jul 2026 16:44:05 +0300 Subject: [PATCH 5/8] ci(wallet-cli): drop reusable-workflow permission escalation in e2e job The `pull-requests: read` permission on `test-wallet-cli-e2e` exceeded what `main.yml` grants the `lint-build-test` reusable-workflow call, which failed `Main` at startup (0s, "workflow file issue") so none of the lint/build/test jobs ran. Install `anvil` unconditionally and always set `MM_E2E_REQUIRE_ANVIL=true` instead of detecting wallet-cli file changes via the PR files API (the reason the extra permission was requested). This job already only runs when `@metamask/wallet-cli` is in the affected package set, so unrelated PRs skip it entirely; the only behavior change is that the real-chain e2e now also runs when wallet-cli is affected as a dependant. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/lint-build-test.yml | 45 +++++---------------------- packages/wallet-cli/tests/README.md | 15 +++++---- 2 files changed, 14 insertions(+), 46 deletions(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 094e13f798e..ef355a8cd0b 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -369,9 +369,6 @@ jobs: runs-on: ubuntu-latest if: contains(fromJson(needs.prepare.outputs.package-names), '@metamask/wallet-cli') needs: prepare - permissions: - contents: read - pull-requests: read strategy: matrix: node-version: [20.x, 22.x, 24.x] @@ -384,45 +381,17 @@ jobs: node-version: ${{ matrix.node-version }} - name: Build wallet-cli and its dependencies run: yarn workspaces foreach --topological-dev --recursive --from '@metamask/wallet-cli' run build - # The real-chain send e2e (`wallet-send.e2e.test.ts`) needs `anvil` - # (Foundry). It is skip-if-absent, so only pay the Foundry download when - # wallet-cli actually changed; on other PRs the test skips itself. - - name: Detect whether wallet-cli changed - id: wallet-cli-changed - uses: actions/github-script@v8 - with: - script: | - try { - if (context.eventName !== 'pull_request') { - core.setOutput('changed', 'true'); - return; - } - const files = await github.paginate(github.rest.pulls.listFiles, { - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - per_page: 100, - }); - const changed = files.some((file) => - file.filename.startsWith('packages/wallet-cli/'), - ); - core.setOutput('changed', String(changed)); - } catch (error) { - // Never let detection break CI: fall back to running the suite. - core.warning( - `wallet-cli change detection failed; running anyway: ${error}`, - ); - core.setOutput('changed', 'true'); - } + # This job only runs when `@metamask/wallet-cli` is in the affected set + # (see the `if` above), so always install `anvil` (Foundry) for the + # real-chain send e2e (`wallet-send.e2e.test.ts`). - name: Install anvil for the real-chain e2e - if: steps.wallet-cli-changed.outputs.changed == 'true' run: yarn workspace @metamask/wallet-cli run test:e2e:install-anvil - # When wallet-cli changed we installed anvil, so require the real-chain - # suite to actually find and run it — otherwise a broken install would - # let the flagship send test pass as a silent green no-op. + # anvil is installed above, so require the real-chain suite to actually + # find and run it — otherwise a broken install would let the flagship + # send test pass as a silent green no-op. - run: yarn workspace @metamask/wallet-cli run test:e2e env: - MM_E2E_REQUIRE_ANVIL: ${{ steps.wallet-cli-changed.outputs.changed }} + MM_E2E_REQUIRE_ANVIL: 'true' - name: Require clean working directory shell: bash run: | diff --git a/packages/wallet-cli/tests/README.md b/packages/wallet-cli/tests/README.md index a3bc009b92e..0ad2e385def 100644 --- a/packages/wallet-cli/tests/README.md +++ b/packages/wallet-cli/tests/README.md @@ -69,13 +69,12 @@ anvil port>` in its `beforeEach` and restores the block in `afterEach`, so ## CI -The `test-wallet-cli-e2e` job runs `test:e2e` on every push/PR. To avoid paying -the Foundry download on unrelated PRs, it installs `anvil` **only when files -under `packages/wallet-cli/` changed** (detected via the PR file list). On PRs -that don't touch wallet-cli, `anvil` is absent and the real-chain suite skips -itself; the offline lifecycle suite always runs. - -When it does install `anvil`, the job also sets `MM_E2E_REQUIRE_ANVIL=true`, so -the real-chain suite fails loudly if the binary is nonetheless missing rather +The `test-wallet-cli-e2e` job runs only when `@metamask/wallet-cli` is in the +affected package set (its `if` gate), so unrelated PRs skip the job entirely and +never pay the Foundry download. When the job does run, it always installs +`anvil` and runs both e2e suites. + +Because `anvil` is always installed, the job also sets `MM_E2E_REQUIRE_ANVIL=true`, +so the real-chain suite fails loudly if the binary is nonetheless missing rather than skipping green — that would otherwise hide the loss of the only test that exercises a real broadcast. From b41b710b56f9e60a849c17fabf4566cc93c9a249 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 24 Jul 2026 16:47:52 +0300 Subject: [PATCH 6/8] chore: remove comments --- .github/workflows/lint-build-test.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index ef355a8cd0b..4010c7aadf0 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -381,14 +381,8 @@ jobs: node-version: ${{ matrix.node-version }} - name: Build wallet-cli and its dependencies run: yarn workspaces foreach --topological-dev --recursive --from '@metamask/wallet-cli' run build - # This job only runs when `@metamask/wallet-cli` is in the affected set - # (see the `if` above), so always install `anvil` (Foundry) for the - # real-chain send e2e (`wallet-send.e2e.test.ts`). - name: Install anvil for the real-chain e2e run: yarn workspace @metamask/wallet-cli run test:e2e:install-anvil - # anvil is installed above, so require the real-chain suite to actually - # find and run it — otherwise a broken install would let the flagship - # send test pass as a silent green no-op. - run: yarn workspace @metamask/wallet-cli run test:e2e env: MM_E2E_REQUIRE_ANVIL: 'true' From 11c0a5e32e4921677bf415f78f969d75a3393532 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 24 Jul 2026 22:17:17 +0300 Subject: [PATCH 7/8] test(wallet-cli): skip real-chain send e2e when anvil is missing from PATH `resolveAnvilPath` returned the bare command `'anvil'` as its last-resort fallback without confirming it was actually on PATH, and `anvilSpawnable` special-cased that string as always spawnable. On a machine with no `node_modules/.bin/anvil` and no `anvil` on PATH, the suite would try to boot anvil in `beforeEach` and fail (spawn ENOENT + readiness timeout) instead of skipping as documented. Probe the PATH fallback with `spawnSync('anvil', ['--version'])` and return `undefined` when it is not runnable, so `anvilSpawnable` reduces to "did we resolve a usable path". The concrete-path and override branches already existed via `existsSync`, so the real-chain run is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../wallet-cli/tests/wallet-send.e2e.test.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/wallet-cli/tests/wallet-send.e2e.test.ts b/packages/wallet-cli/tests/wallet-send.e2e.test.ts index d22b5ac2856..9bac845cfd1 100644 --- a/packages/wallet-cli/tests/wallet-send.e2e.test.ts +++ b/packages/wallet-cli/tests/wallet-send.e2e.test.ts @@ -1,6 +1,6 @@ import { disableNetConnect, enableNetConnect } from 'nock'; import type { ChildProcess } from 'node:child_process'; -import { spawn } from 'node:child_process'; +import { spawn, spawnSync } from 'node:child_process'; import { existsSync } from 'node:fs'; import { mkdtemp, readFile, rm } from 'node:fs/promises'; import { request } from 'node:http'; @@ -68,9 +68,15 @@ function resolveAnvilPath(): string | undefined { if (existsSync(local)) { return local; } - // Fall back to PATH; if it is not there either, the spawn check below fails - // and the suite is skipped. - return 'anvil'; + // Fall back to `anvil` on `PATH`, but only if it is actually runnable there. + // A bare command name always "exists" as a string, so probe it with + // `--version`: a missing binary makes `spawnSync` set `error` (ENOENT), and we + // return `undefined` so the suite skips instead of trying to spawn nothing. + const onPath = spawnSync('anvil', ['--version'], { + stdio: 'ignore', + timeout: 10_000, + }); + return onPath.error === undefined ? 'anvil' : undefined; } const ANVIL_PATH = resolveAnvilPath(); @@ -266,9 +272,10 @@ async function cleanupDaemon(dataDir: string): Promise { await rm(dataDir, { recursive: true, force: true }); } -const anvilSpawnable = - ANVIL_PATH !== undefined && - (ANVIL_PATH === 'anvil' || existsSync(ANVIL_PATH)); +// `resolveAnvilPath` only returns a value once it has confirmed the binary is +// usable (via `existsSync` for a concrete path, or a `--version` probe for the +// bare `PATH` fallback), so a defined path means anvil can be spawned. +const anvilSpawnable = ANVIL_PATH !== undefined; // CI sets this whenever it installed anvil (i.e. wallet-cli changed), so an // install that silently produced no usable binary fails the suite loudly From 208b0604473b96971e51e5a65d5c4a451d2702c3 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 24 Jul 2026 22:36:27 +0300 Subject: [PATCH 8/8] ci(wallet-cli): ignore the external anvil binary in knip's dependency lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real-chain send e2e now probes `anvil` via a literal `spawnSync('anvil', ...)`, which knip's binary detection flags as an unlisted binary. `anvil` (from Foundry) is an external system binary installed via `foundryup`, not an npm package — the same reason `packages/foundryup` already ignores it — so add `anvil` to `ignoreBinaries` for the wallet-cli workspace. Co-Authored-By: Claude Opus 4.8 (1M context) --- knip.config.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/knip.config.ts b/knip.config.ts index 5dca1724946..f9b59119ac1 100644 --- a/knip.config.ts +++ b/knip.config.ts @@ -217,6 +217,10 @@ const config: KnipConfig = { ignoreDependencies: ['immer'], }, 'packages/wallet-cli': { + // `anvil` (from Foundry) is an external system binary the real-chain send + // e2e probes and spawns; it's installed via `foundryup`, not an npm + // package, so knip can't tie the invocation to a dependency. + ignoreBinaries: ['anvil'], // `tsx` is the dev-mode loader: it's referenced only as a `node --import` // argument string (in `daemon-spawn`'s source-entry path and `bin/dev`), // never as a traceable import, so knip can't see it.