Skip to content

feat(wallet-cli): add the mm wallet send command#9636

Draft
sirtimid wants to merge 13 commits into
mainfrom
sirtimid/wallet-cli-send-transaction
Draft

feat(wallet-cli): add the mm wallet send command#9636
sirtimid wants to merge 13 commits into
mainfrom
sirtimid/wallet-cli-send-transaction

Conversation

@sirtimid

Copy link
Copy Markdown
Member

Explanation

Adds the user-facing mm wallet send command that sends a transaction end-to-end through the daemon-hosted TransactionController, closing #9513. This is the CLI surface on top of the daemon's transaction capability (#9512 / #9612): it collects transaction parameters, dispatches them to the daemon, waits for the broadcast, and reports the resulting transaction hash.

Dedicated sendTransaction RPC handler

TransactionController:addTransaction returns a Result shaped like { transactionMeta, result }, where result is a Promise<hash> that resolves once the transaction is signed and broadcast. That promise is not JSON-serializable, so it cannot travel back over the daemon's generic call dispatch. The daemon therefore exposes a dedicated sendTransaction handler (src/daemon/send-transaction.ts) that, server-side:

  1. resolves the network client — from networkClientId, or from chainId via NetworkController:findNetworkClientIdByChainId;
  2. resolves the sender — the provided --from, or the selected account;
  3. calls addTransaction(txParams, { networkClientId, origin: 'metamask', isInternal: true }) (internal, so it skips origin/permitted-account validation and is auto-approved by the headless daemon);
  4. awaits the broadcast and re-reads the live record so the returned status reflects the post-broadcast state (submitted), not the unapproved creation snapshot;
  5. returns a serializable { transactionHash, transactionId, status }.

Params are validated with superstruct at the daemon boundary (exactly one of networkClientId / chainId; 0x address and hex quantities).

The mm wallet send command

A thin client over that handler (src/commands/wallet/send.ts):

  • --to (required), --value (ether — converted to wei via @metamask/utils' shared toWei), --data, --from, --network-client-id / --chain-id, gas overrides (--gas / --max-fee-per-gas / --max-priority-fee-per-gas / --gas-price), --timeout.
  • Because the daemon auto-approves, the CLI is the confirmation boundary: it previews the resolved plan and prompts for confirmation before broadcasting. --yes skips the prompt; --dry-run resolves + validates without broadcasting.

Testing

  • Unit tests for the handler (network/sender resolution, internal submit, dry-run, broadcast + live status, param validation) and the command (arg parsing, preview/confirm/abort, --yes, --dry-run, error surfaces); 100% coverage maintained.
  • Real-chain e2e (tests/wallet-send.e2e.test.ts): boots a local anvil node, adds it as a custom network, and drives the built mm CLI to sign, broadcast, and mine a real transaction (asserts receipt status: 0x1 and a recipient balance increase). It is skip-if-absent; CI installs anvil for it only when packages/wallet-cli/ changed. See packages/wallet-cli/tests/README.md.
  • build, package test, test:e2e, yarn lint, changelog:validate pass.

References

Note

Stacked on sirtimid/wallet-cli-daemon-transaction-capable (#9612); this PR is based on that branch so the diff is scoped. Once #9612 merges, this will be rebased onto main and retargeted.

Checklist

  • Tests cover the handler and command (unit) plus a real-chain broadcast (e2e); 100% coverage maintained.
  • build, test, test:e2e, yarn lint:fix, yarn lint, changelog:validate pass.
  • Confirmation prompt (--yes to skip) and --dry-run gate a real, irreversible send.

🤖 Generated with Claude Code

sirtimid added a commit that referenced this pull request Jul 23, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Jul 23, 2026
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) <noreply@anthropic.com>
sirtimid and others added 13 commits July 24, 2026 14:02
Complete the two consumer-side pieces a wired `TransactionController` needs
to process a transaction end-to-end in the daemon:

- Headless auto-approval: subscribe to `ApprovalController:stateChanged` and
  accept every pending request via `ApprovalController:acceptRequest`, so an
  awaited `addRequest` (raised by a transaction/signature flow) resolves
  instead of hanging on the headless daemon's no-op `showApprovalRequest`.
  The subscription is torn down in the `createWallet` `dispose` path.
- Gas-fee slot: the `gasFeeController` slot (`clientId: 'cli'`) is already
  wired; document it in `buildInstanceOptions` and rely on the wallet
  package's production-default EIP-1559 endpoint rather than re-specifying it.

The auto-approval trust model is documented in `subscribeToAutoApproval` and
the README: the daemon accepts every approval without a per-request prompt,
so the trust boundary is its `0600` same-user Unix socket.

Closes #9512

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Import `Logger` type alias in `auto-approval.ts` instead of redeclaring the
  inline shape, making compatibility with `wallet-factory.ts` explicit
- Fix misleading catch-block comment that implied a rethrow; the error is
  intentionally suppressed
- Clarify `subscribeToApprovalStateChanged` docstring: root cause is the
  missing `ControllerStateChangedEvent` in `ApprovalControllerEvents`, not
  dynamic string widening as in the persistence layer; add TODO pointing at
  the upstream fix
- Add dropped-Patch[]-parameter note to `ApprovalStateChangeHandler`
- Extend `runUnsubscribe` failure log to note the subscription may remain live
- Replace fragile 5x Promise.resolve() flush with setImmediate pattern
- Add missing startup-failure test: subscribeToAutoApproval throws means
  persistence listener is still cleaned up, wallet destroyed, store closed
- Extend auto-approval unsubscribe test to also assert wallet.destroy ran
- Add 5 s per-test timeout to the auto-approval integration test
- Split dense CHANGELOG entry into lead sentence + nested security note

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The feat commit dropped the TODO when it replaced the old comment, but the
future work (exposing approval requests over the daemon transport for proper
user confirmation) is still planned.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep only the ordering rationale; the rest is already in the function name
and its JSDoc.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop inline TODO that duplicated the JSDoc TODO in subscribeToApprovalStateChanged
- Remove four in-test comments that restate their it() descriptions
- Trim integration test comment block to only the non-obvious shouldShowRequest note

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the long inline trust model with a one-paragraph summary and a
reference to #9513 (the mm-send issue) for the planned scoped policy.
Keep only the inFlight guard rationale and error-swallowing note, which
are non-obvious implementation details an editor of this code needs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…then async rejection test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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<hash>` 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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Base automatically changed from sirtimid/wallet-cli-daemon-transaction-capable to main July 24, 2026 11:04
@sirtimid
sirtimid force-pushed the sirtimid/wallet-cli-send-transaction branch from 14da5e6 to 927bcba Compare July 24, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant