Repository navigation
recovery: find Xverse accounts at Xverse's Argent build (#734) - #763
Conversation
Xverse derives Starknet keys on this wallet's own ladder
(m/44'/9004'/{i}'/0/0, grindKey) and deploys with salt = public key and
the Argent 0.4.0 constructor, but since 2025-06-30 at its own Argent
build 0x663fc01a…bd024, which the class table did not know. Recent
Xverse accounts therefore landed at addresses the scan never derived.
Add the class to ACCOUNT_CLASSES (argent, signer-owner-guardian-option,
argent-signer-array) and scan it once per index on the own ladder. It is
kept off Ready's argentx ladder, where Xverse never deploys.
Refs #734
Refs #727
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR SummaryMedium Risk Overview The wallet model registers that class like other Argent deployments (0.4.0 constructor + signer-array auth) and clarifies that Tests cover xverse-core-compatible address derivation, background discovery (ladder choice, deployed/funded vs empty), and the sidepanel recovery gate copy/selection rules; the general Argent ladder test expects one fewer Ready scan shape and one more own-ladder shape. Reviewed by Cursor Bugbot for commit 5821796. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
AI-assisted review (Claude, with an independent Codex gpt-6-astra pass), posted from @derek-flossman's account. Medium — signing compatibility only checked at ABI level. The class is classified Follow-up held until #733 merges: deploying funded but undeployed Xverse accounts (patch drafted; needs a live deploy + transfer test). |
Assert per index that Xverse's Argent build is previewed exactly once on the wallet's own ladder and never on Ready's argentx ladder. Cover the common real case: a funded, undeployed Xverse account is found and the picker says Bramble cannot deploy that release, while an unused one is never kept or selectable. Note on AccountClassRole that "deployment" means an account may have been deployed with the class, not that this wallet deploys it. Refs #734 Refs #727 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adrienlacombe
left a comment
There was a problem hiding this comment.
Nice, well-sourced change. I checked the key facts independently:
- Class hash: xverse-core
developstarknet/constants.tshasargentXContractClassHashV0_4_0 = 0x663fc01a…bd024, andstarknet_getClassonmainnet.nodes.starknet.org/rpc/v0_10returns a Cairo 1 class whose ABI includesargent::account::interface::IAccount. It matchesXVERSE_ARGENT_CLASS_HASH. - Derivation and address: xverse-core's
getAccountDerivationPathism/44'/9004'/{i}'/0/0withgrindKey.createAX040W0GContractDescriptionusesowner = Signer::Starknet(pubkey)andguardian = None, andgetExpectedAccountContractAddressuses salt = pubkey and deployer 0. That matches the new test and the own-ladder-only scan shape. - No mis-attribution or secret risk. Each candidate address comes from the user's own derived key, so a hit can't belong to someone else. Unused Xverse addresses are never listed (not "creatable"). The PR adds no key storage or logging.
- Scope of the class-table change: Ledger derivation picks one class per family via
ledgerDeployClassHash(the pinned Argent class), so the new entry doesn't add probes to Ledger scans. Deploying still only accepts the pinned 0.4.0 class. - RPC cost: one more preview per index on the own ladder only. Over a full 40-index search that's about 40 more candidates, each with its own deployment and balance reads. That's acceptable for what it finds.
- CI on 5821796: fast-checks, test-and-publish, pr-guards and Bugbot all pass. The branch is 3 docs-only commits behind the base.
Things to keep in mind before shipping:
- Funded-but-undeployed Xverse accounts can be imported, but Bramble can't deploy them yet. The row says so, which is honest. But a user who imports one sees funds they can't move from Bramble until the deploy follow-up lands. Worth calling out in the v0.10.8 notes, or holding that row view-only until deploy is supported.
- Overlap with #733: both edit
onboarding-ops.ts. YourrecoveryScanShapeshunk ends a few lines above #733'sisCreatableCandidatehunk. Both also add tests tosidepanel-recovery-scan-gate.test.tsandrecovery-discovery.test.tsat different spots. They don't overlap, and I agree with your merge-tree result. Whichever merges second should still re-run fast-checks, since both change the expected scan shapes. YourdeployableHerefollow-up after #733 is the right order. - Still to check live (from your list): a real activated Xverse account,
get_version(), and one Mainnet transfer to confirmargent-signer-arraysigning on this build.
Copy note for the owner: rows will read "Account N · Argent 0.4.0 (Xverse)" inside the Argent/Ready group.
|
Review follow-up (AI-assisted, by Claude), posted from @derek-flossman's account. Thanks @adrienlacombe. Follow-up on your "still to check live" point, and the owner decision. Xverse class vs the pinned Argent 0.4.0 class. I fetched both with
So Owner decision (#1 in your review): keep funded but undeployed Xverse rows importable with the "can't deploy" copy, and follow up in v0.10.8 with the deploy patch after #733 merges. It's small given the identical constructor. If that slips, the release notes will call it out. Run on this PR merged onto current
Merge-tree: no conflicts with #731, #733, #766 or #768. #755's conflicts come from #741/#762, which are already on the base, not from this PR. |
|
Merged by the repo owner via the release-review bypass; no second review. Head 94ade16 was up to date with release/v0.10.8, with fast-checks and pr-guards green. |
Merged with today's release/v0.10.8 (#763 added Xverse tests to the same files), recovery-discovery.test.ts reached 502 lines and sidepanel-recovery-scan-gate.test.ts 510, which check:files refuses. No test changes: - The index-zero listing test moves to recovery-discovery-listing.test.ts. - The picker fixtures move to tests/support/recovery-picker-harness.ts, the same file name and export style #755 uses for them. Refs #718 Refs #727 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
Refs #734
Refs #727
Summary
Seed import now finds Starknet accounts made in Xverse after 2025-06-30.
Xverse derives keys on Bramble's own ladder (
m/44'/9004'/{i}'/0/0, thengrindKey) and deploys with salt = public key, deployer 0 and the Argent 0.4.0 constructor(Signer::Starknet(pubkey), guardian: None). Since 2025-06-30 it deploys at its own Argent build,0x663fc01a…bd024, which Bramble's class table didn't know, so those accounts sat at addresses the scan never derived.account-families.ts): addsXVERSE_ARGENT_CLASS_HASHas an Argent entry:signer-owner-guardian-optionconstructor,argent-signer-arrayauthorization, deployment + implementation.onboarding-ops.ts): one extra preview per index, on the wallet's own ladder only. It is filtered out of the Ready (argentx-bip44) ladder scan, where Xverse never deploys. Cost: one derivation and one status read per index.Verification
starknet_getClasson Mainnet (mainnet.nodes.starknet.org/rpc/v0_10) returns a Cairo 1 class with Argent's ABI (get_owner,get_owner_type,get_guardian,get_version,__validate_deploy__) andconstructor(owner: Signer, guardian: Option<Signer>).develop,starknet/constants.ts(argentXContractClassHashV0_4_0) andstarknet/utils.ts(path,grindKey, calldata, salt, deployer 0).account-candidates-xverse.test.tsrebuilds the address exactly as xverse-core does (starknet.jsCallData.compilewithCairoCustomEnum/CairoOption) for three keys and requires Bramble's candidate to match.recovery-discovery-xverse.test.tschecks the Xverse class is previewed once per index on the own ladder only, and that a deployed one is listed as a restorable Argent candidate onstarknet-bip44.bun run checkpasses.bun run check:coveragepasses (functions 95.54%, lines 96.12%). E2E not run (the machine was loaded by other jobs).git merge-tree).Not in this PR (decision for owner)
transactions/deployment.ts). Enabling it means accepting any Argent deployment class with thesigner-owner-guardian-optionshape there, plusdeployableHereinrecovery-account-ui.ts. That file is also changed by recovery + ledger: list Braavos accounts like Ready's, and find them in first-run Ledger setup (#718) #733, so this is left as a follow-up after recovery + ledger: list Braavos accounts like Ready's, and find them in first-run Ledger setup (#718) #733 merges.get_version(); one Mainnet transfer to confirm signing.Copy for owner review
Files on the key-derivation / discovery path
packages/foundation/wallet-model/src/accounts/account-families.tsapps/extension/src/background/onboarding/onboarding-ops.ts🤖 Generated with Claude Code