Skip to content

accounts: restore a Ready account from its private key at the release it was deployed with (#776) - #777

Merged
derek-flossman merged 14 commits into
release/v0.10.8from
fix/ready-key-import-releases
Oct 27, 2026
Merged

derek-flossman merged 14 commits into
release/v0.10.8from
fix/ready-key-import-releases

Conversation

@l-henri

@l-henri l-henri commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Closes #776

Why

A user imported their Ready private key as Ready Account and got an empty Argent 0.4.0 address, reported as a successful import. Their real account was deployed at Argent 0.3.1 ([owner, 0]) and later upgraded to 0.4.0 and given a guardian. One key has a different address under every Argent release, because the class hash and the constructor calldata both feed the address, and the raw-key import only ever derived 0.4.0. It never read the chain either.

What changes

  1. Raw-key import scans every Argent release (apps/extension/src/background/accounts/raw-key-class.ts). For both the in-wallet import and the first-run key vault, the background previews the key's address at each Argent deployment class through a new key-free vault method (previewImportedAccount), reads the chain, and passes the deployed release (else a funded one) to the vault. The class is chosen in the background and never read from the message. The vault still refuses any class Argent was never deployed with.
  2. No account means no import. When no release has a deployed or funded account for the key, the import is refused before anything is written. An unreachable network fails the import too, rather than storing an address nobody uses. The Bramble Account key import is unchanged, since that path can legitimately create a new account.
  3. Phrase scan: every Argent release is now tried on both key ladders. Previously the 0.3.x releases were only tried on the Ready extension's ETH-seeded ladder (Seed import finds zero Ready (Argent X) software accounts — wrong master seed, wrong axis #505), and Bramble's own BIP-44 ladder only got 0.4.0. This adds two candidate shapes per scanned index. The new rows appear only with on-chain evidence.
  4. Shared activity read: "deployed or funded" now lives in account-activity.ts, used by both phrase discovery and raw-key import.
  5. Guardian refusal copy now names the in-wallet escape (shipped in v0.10.5) instead of sending the user back to Ready.

Overlaps #763 (Xverse) in recoveryScanShapes. Whichever lands second resolves a small conflict; with this PR, #763's class is scanned on the own ladder automatically and #763's filter keeps it off Ready's.

Copy for owner review

  • New: "No Ready account uses this key on this network." (refusal, friendly-error-copy.ts)
  • Changed: "This account has a guardian, a second approver this wallet can't provide. Use it in Ready, or remove the guardian there." → "This account has a guardian, a second approver this wallet can't provide. Remove it with Recover account on Home, or use the account in Ready."

Verification

  • bun run check and bun run check:coverage pass locally (run under LC_ALL=en_US.UTF-8; the terminal tests are locale-sensitive). The new modules have 100% coverage.
  • New tests:
    • tests/background/accounts/raw-key-class.test.ts: deployed/funded/none/other-family/RPC-failure cases, plus routing through wallet.importAccount for "stores the deployed release" and "writes nothing, key not echoed".
    • tests/background/onboarding/recovery-discovery-own-ladder.test.ts: fails without change 3.
    • Copy assertions in friendly-error.test.ts.
  • Real Krusty, throwaway script: previewImportedAccount addresses match the fixture-pinned accountAddressCandidates for every Argent release, and the output never contains the key.
  • On-chain: the reported key with 0.3.1 [owner, 0] reproduces the user's address exactly. Not exercised with the user's actual key.

🤖 Generated with Claude Code

l-henri and others added 4 commits October 26, 2026 19:13
A user importing a Ready account got a fresh, empty Argent 0.4.0 address
instead of their own. Their account was deployed at Argent 0.3.1 with
[owner, 0], later upgraded to 0.4.0 and given a guardian, and its owner key
is the phrase's plain BIP-44 key, not the Ready extension's ETH-seeded one
(most likely made in Ready mobile). The scan derived exactly that key but,
on this ladder, tried only the release this wallet creates, so the 0.3.1
address was never computed and the creatable 0.4.0 default was the only row.

Scan every Argent deployment class on both ladders. The extra 0.3.x shapes
on the own ladder are listed only with on-chain evidence, like every other
non-creatable shape; selection, vault derivation and duplicate checks
already carry the ladder explicitly, so no other layer changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A user imported their Ready private key as "Ready Account" and got an
empty Argent 0.4.0 address, reported as a successful import. Their account
was deployed at Argent 0.3.1 with [owner, 0] and later upgraded; one key has
a different address under every release, because the class and the
constructor calldata are part of it, and the raw-key import only ever
derived the release this wallet creates.

Both raw-key paths (in-wallet import and the first-run key vault) now
preview the key's address at every Argent deployment class through a new
key-free vault preview, read the chain for each, and store the deployed
release, else a funded one. With no evidence anywhere they keep the
previous default, a fresh account this wallet can deploy. An unreachable
network fails the import instead of storing an address nobody uses. The
class is chosen in the background, never taken from the message.

The deployed-or-funded read is now shared with phrase discovery
(account-activity.ts) so the two cannot disagree about what "exists" means.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A raw-key "Ready Account" exists only to restore an account another app
made: this wallet creates its own accounts from its phrase. When no Argent
release shows a deployed or funded account for the key, the import used to
store the empty 0.4.0 address anyway and report success, which is exactly
how the reported user ended up believing a stranger's address was theirs.

Refuse before anything is written, in both the in-wallet import and the
first-run key vault, with "No Ready account uses this key on this network."
A missing key-free preview now fails closed rather than falling back to the
default release.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since the in-wallet guardian escape shipped, "remove the guardian there"
sends the reader back to Ready for something Bramble does itself. Name the
control that does it, and keep Ready as the fallback for the cases where
Home cannot offer it (a backup guardian, or a guardian change in progress).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

PR Summary

High Risk
Touches raw private-key import, vault writes, and on-chain verification at the trust boundary; mistakes could import wrong addresses or persist keys under stale session state.

Overview
Fixes Ready Account raw-key import (#776): one Argent private key maps to a different address per contract release, so importing only at the wallet’s default release could create an empty 0.4.0 account while the user’s funds sat at 0.3.1.

Ready-key flow: New wallet.discoverReadyKeyAccounts derives each Argent release locally (previewImportedAccount), uses shared readAccountActivity (deployed vs recovery-token balances), and returns public candidates only. Onboarding and Settings run discovery first, show a picker (deployed rows pre-ticked; funded-only rows optional), then send classHashes on wallet.importAccount / wallet.createKeyVault. The background re-proves picks with verifyReadyKeyClasses before any write; Argent imports must name releases, other families must not. The vault can persist multiple accounts from one key in a single commit.

Recovery phrase scan: Every Argent deployment class is tried on both the Ready ETH-seeded ladder and the wallet’s own BIP-44 ladder (not only 0.4.0 on the own ladder), with onboarding using the same activity helper and stricter balance parsing.

Safety / UX: Session authorization is captured around chain reads on in-wallet import; phrase recovery unlocks inside the coordinator task; network switches announce so the picker clears; new friendly errors for “no Ready account” and unavailable checks; guardian copy points at Recover account.

Reviewed by Cursor Bugbot for commit 12ec2d4. Bugbot is set up for automated code reviews on this repo. Configure here.

derek-flossman and others added 2 commits October 26, 2026 23:33
Resolves the conflict with #763 (Xverse) in recoveryScanShapes. Bramble's
own ladder scans every Argent deployment release, Xverse's build included.
Ready's argentx ladder scans all but Xverse's build. The discovery test
asserts both counts.

Refs #776
Refs #727

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Independent review of #777 (Codex gpt-6-astra high, DeepSeek v4 pro) found
that the raw-key import chose the first deployed release, else the first
funded one, in table order. Dust sent to another release's address could
steer the import, and a key deployed at two releases silently lost one.

Nothing is chosen silently now:
- wallet.discoverReadyKeyAccounts (extension-only, in the key-boundary
  catalog) previews the key at every Argent release through the vault's
  key-free preview and lists each address with on-chain evidence: deployed,
  or funded with the tokens it holds. No evidence refuses ("Couldn't find
  a Ready account for this key on this network."). An unreadable chain
  refuses with its own message ("Couldn't check this key's Ready accounts.
  Try again."), never an empty list. With a vault present, a locked or
  changed session refuses before and after the reads.
- First run (Continue) and Settings → Import private key (submit) show a
  picker. Deployed rows start ticked. Funded-only rows are listed but never
  preselected. Accounts already in the wallet are locked.
- wallet.importAccount and wallet.createKeyVault carry the ticked
  `classHashes`. They are required for Argent and refused for other
  families. The background re-proves each pick (an Argent deployment class
  whose address still shows evidence) before any write. The class is never
  the message's word alone, and a refusal never locks the session.
- The vault stores one account per picked release, all signing with the
  same key, in a single write: first run creates the vault with every pick
  at once, and an in-wallet import refuses the whole batch if any pick is
  already stored.
- The Bramble Account key import is unchanged and reads no chain.

Also from the review:
- readAccountActivity no longer reports a deployed account as unfunded. A
  deployed answer carries no funding field, and an undeployed one reports
  what it holds.
- Tests:
  - real Krusty plus real vault: each release's preview matches the address
    pinned by real Mainnet accounts (0.3.1 among them) and carries no key;
  - multi-release first-run write;
  - a duplicate pick, or a pick the chain no longer shows, writes nothing;
  - exact Xverse membership on both ladders;
  - two E2E tests (picker import, and refusal with no picker).

Refs #776
Refs #727

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

derek-flossman and others added 6 commits October 27, 2026 00:46
- wallet.importAccount captures the session authorization before the
  release verification reads and re-checks it inside the coordinator, so a
  lock, re-unlock or replaced vault during the reads writes nothing and
  leaves the network preference alone, without locking the session.
- First-run wallet.discoverReadyKeyAccounts re-reads the vault status after
  the chain reads and refuses when a vault appeared meanwhile.
- readAccountActivity fails closed on a balance that is not a canonical
  base-units integer instead of throwing from BigInt.
- verifyReadyKeyClasses refuses an empty pick list before any read.
- Document the SNF-operated RPC trust model for pre-ticked deployed rows
  (owner decision 2026-10-27, as #398).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A discovery and its picker now carry a snapshot of the key, salt and
network plus a request generation. A late or out-of-order answer is
dropped, any input change (the network included) invalidates the picker,
only one picker is ever mounted, and the import sends exactly the
snapshot the picker was built from, or refuses and lists again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reword the pre-tick comment's issue reference so the hex-colour scan does
not read "#398" as a colour, drop exports nothing imports, and format the
Ready-key test fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wallet.recoverAccounts unlocked the new vault only after its coordinator
task returned, unlike every other vault-creation path. A Ready-key import
queued behind it could enter the coordinator against the replaced vault
while the old session generation was still current and pass its
authorization check. The unlock now happens inside the task, so the
generation has moved before any queued task runs.

Covered by a recovery that replaces the vault while an import's
verification reads are in flight: no write to either vault, the import's
network is never applied, and the boundary is released already unlocked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A repaint is skipped when the markup is unchanged, so a picker listed for
one network could stay visible on the next and revive on a switch back.
The paint loop now announces a network change on the side-panel root, and
both key-import forms drop the picker and any answer still in flight when
it fires, so an A-to-B-to-A switch cannot mount the first A's answer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failed node match makes the runner print a whole happy-dom node, which
exhausts memory instead of reporting the failure; counting keeps a
regression a plain test failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6813eb9. Configure here.

Comment thread packages/presentation/wallet-ui/src/sidepanel/app/start.ts
l-henri and others added 2 commits October 27, 2026 09:12
A network switch in an unlocked wallet skips paint() until hydrateView
loads the new network's data, and the network-change event only fired
from paint(). For that window the Ready-key picker in Settings → Import
stayed up for the old network, so Import selected could write the
previous network's accounts and switch the preference back (Bugbot on
#777). Announce from render() as soon as the new status arrives; the
announcer reports each network once, so the later paint adds nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@derek-flossman
derek-flossman merged commit a8949a5 into release/v0.10.8 Oct 27, 2026
6 checks passed
@derek-flossman

Copy link
Copy Markdown
Collaborator

Merged by the repo owner via the release-review bypass; no second review. Head ea5df8c was up to date with release/v0.10.8, with fast-checks and pr-guards green.

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