feat(wallet-cli): add the mm wallet send command#9636
Open
sirtimid wants to merge 8 commits into
Open
Conversation
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>
Base automatically changed from
sirtimid/wallet-cli-daemon-transaction-capable
to
main
July 24, 2026 11:04
sirtimid
force-pushed
the
sirtimid/wallet-cli-send-transaction
branch
from
July 24, 2026 12:11
14da5e6 to
927bcba
Compare
sirtimid
added a commit
that referenced
this pull request
Jul 24, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid
added a commit
that referenced
this pull request
Jul 24, 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
added a commit
that referenced
this pull request
Jul 24, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid
added a commit
that referenced
this pull request
Jul 24, 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
force-pushed
the
sirtimid/wallet-cli-send-transaction
branch
from
July 24, 2026 13:20
927bcba to
fcffb1e
Compare
sirtimid
marked this pull request as ready for review
July 24, 2026 13:20
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>
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) <noreply@anthropic.com>
sirtimid
force-pushed
the
sirtimid/wallet-cli-send-transaction
branch
from
July 24, 2026 19:12
b92b41a to
b41b710
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b41b710. Configure here.
… 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) <noreply@anthropic.com>
… lint
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) <noreply@anthropic.com>
sirtimid
enabled auto-merge
July 24, 2026 20:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Explanation
Adds the user-facing
mm wallet sendcommand that sends a transaction end-to-end through the daemon-hostedTransactionController, 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.Send a transaction.
--valueis in ether; select the network with--network-client-idor--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:Because the daemon auto-approves the confirmation prompt — or your explicit
--yes— is the only boundary before funds move; use--dry-runfirst if unsure. Gas is estimated automatically unless overridden with--gas/--max-fee-per-gas/--max-priority-fee-per-gas/--gas-price(each a0x-prefixed hex quantity).Dedicated
sendTransactionRPC handlerTransactionController:addTransactionreturns aResultshaped like{ transactionMeta, result }, whereresultis aPromise<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 genericcalldispatch. The daemon therefore exposes a dedicatedsendTransactionhandler (src/daemon/send-transaction.ts) that, server-side:networkClientId, or fromchainIdviaNetworkController:findNetworkClientIdByChainId;--from, or the selected account;addTransaction(txParams, { networkClientId, origin: 'metamask', isInternal: true })(internal, so it skips origin/permitted-account validation and is auto-approved by the headless daemon);submitted), not theunapprovedcreation snapshot;{ transactionHash, transactionId, status }.Params are validated with superstruct at the daemon boundary (exactly one of
networkClientId/chainId;0xaddress and hex quantities).The
mm wallet sendcommandA thin client over that handler (
src/commands/wallet/send.ts):Testing
--yes,--dry-run, error surfaces); 100% coverage maintained.tests/wallet-send.e2e.test.ts): boots a localanvilnode, adds it as a custom network, and drives the builtmmCLI to sign, broadcast, and mine a real transaction (asserts receiptstatus: 0x1and a recipient balance increase). It is skip-if-absent; CI installsanvilfor it only whenpackages/wallet-cli/changed. Seepackages/wallet-cli/tests/README.md.build, packagetest,test:e2e,yarn lint,changelog:validatepass.References
wallet-cli: add themmsend-transaction command #9513wallet-cli: make the daemon transaction-capable — consumeGasFeeController+ headless auto-approval #9512 / feat!: wireNetworkEnablementControllertoConfigRegistryController#9611 (daemon transaction-capable) and feat(wallet-cli): wire the TransactionController slot in the daemon wallet #9509 (theTransactionControllerslot); the mutating-RPC safety prerequisitewallet-cli: makesendCommandretry idempotency-safe before mutating RPC ships #9511 landed in fix(wallet-cli): retrysendCommandonly onECONNREFUSED#9608.packages/wallet-cli/src/daemon/send-transaction.ts,src/commands/wallet/send.ts,tests/wallet-send.e2e.test.ts.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 ontomainand retargeted.Checklist
build,test,test:e2e,yarn lint:fix,yarn lint,changelog:validatepass.--yesto skip) and--dry-rungate a real, irreversible send.🤖 Generated with Claude Code
Note
High Risk
Introduces an irreversible fund-moving send path; the headless daemon still auto-approves, so the CLI confirmation (or
--yes) is the main per-tx gate—timeouts and duplicate-send guidance are safety-critical.Overview
Adds
mm wallet send, a CLI path to sign and broadcast through the daemon’sTransactionController. Ether--valueis converted to wei; the network is chosen with--network-client-idor--chain-id; interactive runs dry-run preview →confirmSend→ broadcast, with--yes,--dry-run, optional gas overrides, and a longer default timeout on broadcast (with guidance if the client times out while a send may still be in flight).Because
addTransaction’sresultpromise is not JSON-serializable, the daemon gains a dedicatedsendTransactionRPC (runSendTransaction) that resolves network/sender, supportsdryRun, submits as internal/auto-approved, awaits the hash server-side, and returns{ transactionHash, transactionId, status }.CI/docs/tooling: wallet-cli e2e installs anvil via
@metamask/foundryup, setsMM_E2E_REQUIRE_ANVILin CI, addswallet-send.e2e.test.ts(local anvil, real broadcast), and ignores.metamask// knipanvilbinary.Reviewed by Cursor Bugbot for commit 208b060. Bugbot is set up for automated code reviews on this repo. Configure here.