Skip to content

fix: deploy into a pre-created vault account instead of failing - #166

Merged
graikos merged 8 commits into
mainfrom
fix/deploy-existing-account
Oct 9, 2026
Merged

graikos merged 8 commits into
mainfrom
fix/deploy-existing-account

Conversation

@graikos

@graikos graikos commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes F-2026-19261. Builds on #162's pending-note code, now on main; one test-only commit from that branch (6fd5c91, assertions for the failed-replacement cases) rides along here.

In short

Both clients deploy a vault with fundNewAccount() in the same transaction as the deployment, which declares "this creates one account". The address is public once the transaction reaches the mempool, and anyone can create the bare account first by paying into it; the deployment then fails with Invalid_fee_excess, costing the deployer the fee and the proving run, and for an air-gapped CREATE_CHILD a rebuilt bundle. In the web app a failed SubVault creation also left a pending note that locked the parent vault in that browser for a day, because a failed transaction emits no proposal and the status probe only covers recent blocks.

Now both clients look at the address first. A bare account someone created is deployed into as it is, without the creation fee, so the same address can be retried while its key is at hand. An address that already carries a verification key or app state is refused, since only its own key could have put them there, and so is a node error other than "no such account", since guessing would reproduce the failure. And a SubVault creation note expires after 20 minutes, the window #162 gave records the backend never resolves.

Changes

  • ui/lib/deploy-target.ts: classifyDeployTarget(fetchResult) returns new (a 404 from the node: fund it), existing (bare account, deploy into it), or throws for a used address or any other node error. The worker checks the vault address before compiling for a root deploy, and the child address (derived from the child key, which must match the proposal's) before asking for the proposal signature, so a refused address costs no compile and no wallet prompt.
  • offline-cli/src/build-tx.ts: deployTargetFromSnapshot(bundle.accounts[child]) applies the same rule to the bundle's snapshot, keyed by the child key's own address, which must equal input.childAccount; a snapshot of a bare child means "exists, do not fund". The summary tells the signer which it is ("Child account: exists, no creation fee" or "new, 1 MINA creation fee"). The exporter now fails instead of omitting a snapshot when the node answers with an error, so the CLI never decides from a fail-open map. Root deploys are not a CLI action.
  • ui/lib/storage.ts: create notes whose summary is a createChild use the 20-minute expiry; creation notes for other proposal types keep the day, since the indexer resolves those.
  • ui/lib/transaction-preflight.ts: the pre-broadcast state check, which every web and uploaded transaction passes right before sending, re-reads each account from the node and used to refuse a bare account (zkappState: null) as an incomplete answer, so a deployment the target check had accepted was never sent. The reader now tells the three answers apart: no account, a bare account, or app state. A bare account is checked as all-zero state, as the chain checks it, so the deployment's writes and the later preconditions in the same transaction are still verified, and an isNew precondition on it is reported stale. An answer that omits the field still blocks the broadcast.
  • The same rule, applied where the clients still guessed: the worker's execute path counted a transfer receiver as new on any fetchAccount error, so a node hiccup funded an existing receiver and the transaction failed with Invalid_fee_excess after proving; receiverExists now accepts only "no such account" as new and refuses other errors. The offline exporter stored whatever account object the node answered; it now requires nonce, balance and zkappState in the answer, so the CLI never decides from a snapshot that lacks the field it reads.
  • No change to the creation flow itself: a failed web creation is retried as a new SubVault, the web keeps no key past the redirect, and the finding's separate create-then-deploy transaction is not implemented.

What remains

The first attempt can still be griefed once per address; the attacker gains nothing and pays the creation fee, the deployer loses a fee and proving time. A web retry of the same address is possible only while the wizard page is still open, since the web keeps the key in page state; the offline bundle keeps it. Documented as accepted risk 9 in the security guide. Removing it needs the two-transaction flow. The UI half has no end-to-end test (the lightnet suite is not run here); the classifier is unit-tested and the CLI half has e2e coverage.

Tests

  • UI: 61 unit tests pass, 12 new (receiverExists for a 404, existing accounts and a node error; the exporter failing on an account answer without its fields; classifyDeployTarget for a 404, a missing, bare, zero-state and used account, and a non-404 node error; the 20-minute expiry applying to SubVault creations only; the exporter failing on a node error; the state check reading a bare account as zeros and flagging only an isNew precondition on it; the node reader telling absent from bare and rejecting an answer without the field; and a regression that serves one simulated bare-account node answer to both o1js's fetchAccount with classifyDeployTarget and to the state check on a deploy-plus-setup transaction, expecting existing and current, then unavailable once the field is missing). Typecheck and the Next 16 build pass.
  • Offline CLI: 38 unit tests pass, 3 new (deployTargetFromSnapshot; the summary's creation-fee line). The SKIP_PROOFS child-lifecycle e2e block passes in order (10 of 10) with two new cases: a CREATE_CHILD propose into a child account created by a payment yields a transaction with no account-creation update, and the local ledger accepts a deployment into that bare account; a propose into an address that already holds a zkApp (the parent itself, with its own key) is refused, and so is one whose child key belongs to another address.
  • e2e unit tests: 5 pass; the checkpoint export fixture now answers account queries with nonce, balance and zkappState, as a node does.
  • Not run: lightnet test:e2e, manual run against a live node.

Docs

UI audit guide (atomic deploy flows, pending-note expiry, pre-broadcast check on a bare target, receiver existence on execute), offline audit guide (accounts[input.childAccount] in propose bundles, pre-broadcast check on an uploaded CREATE_CHILD response, exporter field check), transaction coordination design (bare account in the state check), security audit guide (accepted risk 9). Desktop guide reviewed; it packages the same UI and needs no change.

@graikos
graikos added this pull request to stack #167 October 7, 2026 12:41
@graikos
graikos requested a review from mellowcroc October 7, 2026 15:21
@mellowcroc

mellowcroc commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

The new deployment account check allows deploying into an existing account with no vault installed. But the final check before broadcast rejects that account's zkappState: null, so the transaction never gets sent. This also affects offline-built deployments uploaded to the UI for broadcast.

What about distinguishing these two responses?

  • "account": {} → reject: the requested state field is missing, so we can't check the transaction.
  • "account": { "zkappState": null } → continue checking the deployment: the account exists but has no zkApp state yet, which is exactly what the deployment will initialize. This should not automatically pass the transaction; its initialization writes and subsequent state preconditions still need checking.

I added a local test during review (not part of this PR) using the explicit-null response. The deployment account check accepts it, but the final check blocks it. This was a simulated node response, not a live-chain test. A regression test covering both checks together would catch the gap.

Base automatically changed from fix/verified-submissions to main October 8, 2026 07:46
@graikos
graikos force-pushed the fix/deploy-existing-account branch from 5e1b1c9 to c105f1e Compare October 8, 2026 08:33
mellowcroc
mellowcroc previously approved these changes Oct 8, 2026
@graikos
graikos force-pushed the fix/deploy-existing-account branch from f35f4a2 to fd2ebd5 Compare October 9, 2026 07:47
@graikos
graikos merged commit fd76497 into main Oct 9, 2026
8 of 10 checks passed
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.

2 participants