[Ryun1]

Summary

Users moving from Ready (and other wallets) with shielded funds import their account into Bramble and then cannot unshield. The pool writes an address's viewing key once (WriteOnce, NON_ZERO_VALUE on a second SetViewingKey) and has no rotate path, so Bramble can never register its own key for that account. Every proof needs the registered key, so the account can never shield or unshield here. The account is still worth importing: its public funds are reachable.

Import always succeeds. Seed, raw key, key vault, Ledger, Ready key (#777) and address import (#781) never block on the pool. Seed discovery and find-by-address read get_public_key for each deployed candidate only to label the row (below); a failed read lists the row unlabelled. The paths that write an account do not read the pool.

The check happens at the point of use. When a private action is attempted, Bramble reads get_public_key for the active account across every configured RPC endpoint (withRpcFallback). The account is flagged only when that key is non-zero and differs from the viewing public key Bramble would register (previewStrk20ViewingPublicKey / previewImportedStrk20ViewingPublicKey; only the public x-coordinate leaves the vault). Zero and a matching key behave as they do today. A failed pool read or key derivation is not a flag: public sends and swaps continue, and the private action asks for a retry.

The blocker is added to resolvePrivacyContext in strk20-op-context.ts, and every private route that bypasses the chokepoint gets the same check: state read, registration, shield, send, unshield, shadow sweep, and dapp approval staging and execution.

Ledger accounts stay on the existing Ledger blocker, which is checked first. A TODO records that the retained Ledger viewing key should be compared once Ledger provisioning ships.

Fee reads stay on the first endpoint. A non-zero pool key may be remembered for the process; a zero key is not, because the address can still be registered.

User-facing copy (for owner approval)

• Privacy screen: "Private balances are off for this account. Another wallet set up its privacy."
• Secondary line: "Use that wallet to unshield. The funds will then show here."
• Refused private action, and the notice a site's private request is held on: "Another wallet set up privacy for this account. Use that wallet to shield or unshield."
• Retry when the pool or the derivation cannot be read: "Couldn't check this account's privacy setup. Try again."
• Recovery picker row, deployed account another wallet registered (advisory, row stays selectable): "Deployed on Starknet · private balances off, set up in another wallet"

Decisions where the plan was silent

• A missing pool reader is treated as a failed read (retry), not as clear.
• Dapp private requests are held on a close-only notice instead of answered at once, so a site cannot probe the registration. Closing the notice delivers the action sentence.
• The retry sentence above maps derivation and read failures, as requested in review. It is not one of the three approved screen lines.

Verified against the pinned sources

• The pool computes public keys with core::ec::stark_curve::{GEN_X, GEN_Y}, the standard Stark generator, so Krusty's public key for Bramble's viewing key is directly comparable.
• The SDK passes the wallet's viewing key to the pool unchanged (compiler.ts: toBigInt(this.userViewingKey)).
• The STRK20 docs publish a single Mainnet pool, 0x040337b1…ffe812a, which is the address Bramble configures (STRK20_MAINNET_POOL). Ready ships STRK20 through the official stack, so it registers in the same pool. Sources are in the comment thread. A Ready-registered address would settle it on-chain.

Other changes

• readStrk20Registered now wraps a new readStrk20PublicKey, so the key value is available.
• The public-key probe in deriveImportedAccount is extracted as recoverStarkPublicKey.
• Test support: the account stub copied across discovery tests is now tests/support/recovery-accounts.ts; makeRuntimeRpc answers the pool read as "not registered" by default.

History

The first two commits refused the import outright. Review changed the product call: import the account and turn privacy off for it with an explanation. The third commit is that rework, with release/v0.10.8 merged in.

Refs #727

Roadmap & release

• Roadmap issue: Refs Release v0.10.8 #727
• Target release: release/v0.10.8
• Acceptance criterion satisfied: a migrating account can be imported and can move its public funds; shield, unshield, and other private actions stay off when another wallet registered the pool key, with an explanation
• This change is in scope for the target release (for the reviewer to confirm)

Checks

• bun run check passes locally (3,358 tests; check:lint exits 0 with a Biome schema info, CLI 2.5.14 vs schema 2.5.11)
• bun run test:e2e passes (CI)
• Tests are included for the behavior changed
• No lint or type suppressions added
• All changed files stay under the 500-line cap

Dependencies

none

---

The claims about pool and contract behaviour come from the vendored Cairo and SDK sources. A reviewer should check them before relying on them in user-facing copy.

[cursor]

PR Summary

Medium Risk
Broad changes to STRK20 gating and vault preview surfaces that compare pool registration keys, but failures default to retry rather than treating accounts as foreign, and import paths stay non-blocking.

Overview
Adds a point-of-use check that compares the privacy pool’s on-chain viewing public key with the public x-coordinate Bramble would register for the same account. Import and public activity are unchanged; shield, unshield, registration, dapp STRK20 flows, and wallet privacy context now refuse or hold when the keys differ, with a new other-wallet privacy blocker and approved copy in the side panel, send flow, and recovery picker.

Recovery discovery and address import only attach an advisory privacyPool: "other-wallet" label on deployed rows when the pool read succeeds and keys mismatch; failed pool or vault preview reads leave rows unlabeled and do not block import.

Vault and RPC plumbing: preview APIs for phrase, stored, and imported accounts (previewStrk20ViewingPublicKey, etc.), readStrk20PublicKey with multi-endpoint failover (fee reads still use the first RPC only), optional getStrk20PublicKey on the background runtime, and a small in-process cache for non-zero pool keys only.

Dapp private requests that would leak registration state are queued as close-only notices (strk20-unavailable / other-wallet) instead of answering immediately. Ledger accounts still hit the existing ledger blocker first.

^{Reviewed by Cursor Bugbot for commit 9824f05. Bugbot is set up for automated code reviews on this repo. Configure here.}

[derek-flossman]

AI-assisted review (Claude, with an independent Codex gpt-6-astra pass), posted from @derek-flossman's account. Findings were checked against the code; treat them as review input, not approval.

Medium — test gap on the viewing-key comparison (strk20-viewing-key.ts). The new tests stub the viewing public keys with constants, so nothing exercises the real vault derivation against the pinned SDK/pool computation. If the conventions differ (pool-address normalisation, key encoding), the guard would block restoring Bramble's own registered accounts. Suggest one deterministic test with real derivation, including restoring an account Bramble registered.

Low (documented trade-off): the seed-import refusal is enforced only in the picker; the write step doesn't re-check the pool. Fine if that's the intended trade-off — just confirming it's deliberate.

Low (plausible): in rawKeyPrivacyRefusal, errors from preview(...) propagate unmapped. That's intended for an invalid key, but a KMS or RPC outage will surface whatever the router maps rather than "Couldn't check this account's privacy setup. Try again."

[Ryun1]

i need to test it more, so draft for now

[adrienlacombe]

Reviewing while it's still a draft. The reasoning about the write-once pool key is clear and well sourced, thanks.

Leaks: I didn't find any. Only the viewing public x-coordinate leaves the vault. The previews store nothing, the pool reads are public state, and nothing new is logged. A deployed recovery candidate costs one extra get_public_key read.

Lock-out / false-positive concerns

1. The block covers the whole account, not just privacy. A migrating Ready user with public balances can't import the account to move those funds either. Would "import, but disable shield/unshield for this account with an explanation" serve users better than a hard refusal? Either way, I'd get an explicit product call before this leaves draft.
2. Fail-closed reaches further than the threat. The Ledger path (all families) and the raw-key/key-vault paths read the pool even for undeployed addresses, which the PR itself says can't be registered. With only the first RPC endpoint used, a flaky primary now blocks adding a brand-new Ledger account or a first-run key vault. Suggestion: treat a not-deployed address as clear, or skip the read for it, and read the pool through withRpcFallback over all endpoints (inline notes below).
3. Ledger is assumed never registered. The vault already has retained Ledger viewing-key slots (ledger-strk20-typed-data-v1 in account-secrets.ts). When Ledger provisioning ships, re-adding a Bramble-registered Ledger account will be refused unless this comparison is updated. Please add a test or TODO that ties the two together.
4. No background re-check on seed import (acknowledged). The picker is the only enforcement. The risk is low since the message is extension-page-only, but I'd prefer the write boundary to re-check before this un-drafts.

Process / CI

• pr-guards is red (blocking): the linked-issue guard needs an issue reference (Refs #727 or a new issue).
• fast-checks pass on 0863a4d.
• The branch is behind base.

Conflicts:

• With #733 and #763 in onboarding-ops.ts, recovery-account-ui.ts, sidepanel-recovery-scan-gate.test.ts (this PR moves its fixtures into recovery-picker-harness.ts) and recovery-discovery.test.ts.
• With #741 in runtime-rpc.ts, the line after getDeploymentStatus, in createDefaultRuntimeRpc.

It's probably easiest to rebase this after those land.

[derek-flossman]

AI-assisted (Claude), posted from @derek-flossman's account.

Product decision (Derek): don't refuse the whole account. Import it, and if the address is registered with the privacy pool by another wallet, turn shield/unshield off for that account with a short explanation of why. We don't want to lock anyone out of their public funds; the refusal doesn't protect the shielded funds anyway, since Bramble can't reach them either way.

For the rebase (things that changed under this PR):

• accounts: restore a Ready account from its private key at the release it was deployed with (#776) #777 (merged) added raw private-key import for Ready accounts (apps/extension/src/background/routing/ready-key-handlers.ts, accounts/ready-key-accounts.ts) and changed accounts/import-inputs.ts.
• recovery: import an account by its address when the phrase can't derive it (#398) #781 (merged, Import an account by address when its key cannot be derived from the phrase #398) added import by address: wallet.findRecoveryAccountByAddress and wallet.findOwnAccountByAddress (onboarding/address-import.ts).
• Both paths are exactly the Ready-migration population this PR targets, so the pool check needs to cover them too, not only seed import.
• The branch currently conflicts with release/v0.10.8 in account-creation-handlers.ts and runtime-rpc.ts.

Open points from the earlier reviews that still apply with the new approach:

1. Treat undeployed addresses as clear (they can't be registered), and read the pool via withRpcFallback over all endpoints, so one flaky endpoint doesn't block anything.
2. Ledger: the vault already has Ledger viewing-key slots, so "Ledger is never registered" needs a TODO or a test tied to Ledger provisioning.
3. Enforce the shield/unshield restriction in the background, not only in the picker.
4. A test that runs the real viewing-key derivation (today it's stubbed), so Bramble can't flag its own registered accounts.
5. Map preview() errors to the "Couldn't check…" copy.
6. Confirm Ready registers in the same mainnet pool; if not, the check passes silently.
7. Add an issue reference to the body (pr-guards), e.g. Refs #727.

New user-facing copy for the "privacy off" explanation needs owner approval — please list it in the PR body.

@Ryun1 — could you comment on whether this approach works for you, and on point 6?

[derek-flossman]

@Ryun1 — notes for picking this up tomorrow. This follows the product decision above: import the account, and turn privacy off for it, rather than refusing the import.

Proposed rework

1. Import always works.

• This covers seed, raw key, Ready key (accounts: restore a Ready account from its private key at the release it was deployed with (#776) #777), import by address (recovery: import an account by its address when the phrase can't derive it (#398) #781) and Ledger.
• Import never reads the pool, so a flaky node can't block anyone.
• The import-time blocking comes out: the picker privacyPool state, the raw-key, key-vault and Ledger refusals, and their copy.

2. The check moves to the point of use, in one place.

• Every private route already goes through resolvePrivacyContext in strk20-op-context.ts, which returns a privacyBlocker (network, ledger).
• The rework adds a third blocker, for an address another wallet registered:
  • read get_public_key for the active account, using withRpcFallback across all configured endpoints;
  • flag the account only if the key is non-zero and differs from the viewing public key Bramble would register.
• Zero (unregistered) or a matching key behaves as it does today.
• Any private route that bypasses the chokepoint gets the same check: state read, registration, shield, send, unshield, shadow sweep, and dapp approval staging and execution.

3. Nothing hidden or stuck.

• Public balance, sends and swaps are untouched.
• The privacy screen shows a clear state instead of a silent 0 or an Activate button.
• Private actions are refused before any proof, signature or fee.
• If the pool read or the key derivation fails, the account is not flagged: public actions carry on, and private actions get a retry.

4. Ledger. No change. resolvePrivacyContext already turns privacy off for Ledger accounts, which covers the earlier "Ledger is never registered" point.

5. Kept from this PR:

• the vault-side previewStrk20ViewingPublicKey and previewImportedStrk20ViewingPublicKey (only the public x-coordinate leaves the vault);
• readStrk20PublicKey;
• recoverStarkPublicKey;
• the test-support dedup.

Approved copy

• Privacy screen: "Private balances are off for this account. Another wallet set up its privacy."
• Secondary line: "Use that wallet to unshield. The funds will then show here."
• Refused private action: "Another wallet set up privacy for this account. Use that wallet to shield or unshield."

Tests the rework needs

• The real viewing-key derivation, compared against a pool key computed the way the pool computes it. Bramble's own registered account must not be flagged, and a different key must be.
• Background:
  • the blocker on the state read and on each write route;
  • unregistered stays normal;
  • a failed read means no flag and a retry;
  • every import path makes zero pool reads.
• The privacy screen's new state.

Status

A first pass of this is being built locally on top of your two commits, with release/v0.10.8 merged in to clear the conflicts. It isn't pushed. If you're happy with the approach, we can push it to this branch for you to review, or you can take it from here. Your call.

Still open:

• Pool (point 6): does Ready register in the same mainnet pool, 0x040337b1…ffe812a? If you know a Ready-registered address, that settles it.
• Issue reference: the PR body needs one for pr-guards, e.g. Refs #727.

[Ryun1]

Point 6 (pool): the STRK20 docs publish exactly one Mainnet privacy pool, 0x040337b1af3c663e86e333bab5a4b28da8d4652a15a69beee2b677776ffe812a (Deployed Contract Addresses). That is the address in STRK20_MAINNET_POOL. Starknet's posts name Ready as a launch wallet for STRK20 shield/unshield, and AVNU's privacy docs say private swaps from Ready run against the same standard (Privacy live on Starknet, AVNU privacy). Ready's own source isn't public, so this is inferred from the single published pool rather than read from their code. If anyone has a Ready-registered address, a get_public_key read on that pool settles it on-chain.

Also done: the PR body is rewritten for the point-of-use approach, the three inline threads on code the rework deleted are resolved as superseded, and an empty commit reran the gates (the last push was recorded as edited, so fast-checks had skipped).

[Ryun1]

Pushed 391b25ac: an import-time note, on top of the point-of-use block.

• Seed discovery and find-by-address read get_public_key for each deployed candidate and, when the key is non-zero and not Bramble's, label the picker row. The row stays selectable: public funds are reachable here.
• Still never blocks: a zero key, a matching key, an unreadable pool, or a failed derivation list the row unlabelled, and the private-action path re-checks. The paths that write an account (seed restore, raw key, key vault, Ledger, Ready key) still make zero pool reads; import-pool-reads.test.ts now pins that split.
• Same reader and cache as the point-of-use check (strk20RegisteredElsewhere beside strk20RegistrationBlock).

New copy for owner approval (picker row): "Deployed on Starknet · private balances off, set up in another wallet"

39f932c9 removes two vendor/ paths that a git add -A swept into 391b25ac by mistake (a gitlink and a 267 KB compiled class). The tree is clean; the blob stays in branch history unless the two commits are squashed or rewritten before merge.