refactor(node-ui): centralize the PCA owner-access model + resolve signer once (#1375)#1468
Merged
Conversation
… shared leaf (#1375) Precursor for the owner-access model consolidation. Case-insensitive EVM address equality was hand-copied 7× across the PCA owner-access + wallet layers. Extract one pure leaf `pca/address.ts` (no imports) and replace every copy with an import, aliased to each file's existing local name so every call site stays byte-identical (only delete-local-def + add-import per file). detailOwnerMode re-exports eqAddress so ConvictionDetailView's import is untouched. inv-16: in walletOwnerActionSubmitter.ts only the local eqAddress def was swapped for the import — loadContext / assertStillConnected and the per-prompt guards are byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…tests (#1375 item 2) New pure classifier `pca/ownerAccess.ts:resolvePcaOwnerAccess()` — the SINGLE encoding of the owner/signing classification that four sites hand-copied (detailOwnerMode, ownerActions.resolveOwnerActionSubmitterKind, ConvictionOverview.ownerModeFor, ApproveWalletsModal.signerKindForAccount). Preserves both invariants exactly: - inv-17 (owner precedence): daemon check FIRST + the redundant `&& !eqAddress(owner, primaryWallet)` guard on the wallet branch. - Address-only classification: `mode` is derived from addresses only; the wrong-network fold is NOT in the classifier — `writesEnabled` gates writes separately (daemon always; wallet only on the right network), matching the async submitter (address-only kind + separate network block). This keeps every consumer's effective (classify + gate) behavior when it migrates. Model: { mode, wrongNetwork(raw), writesEnabled, submitterKind, ownerUnknownCause?, signableOwners }; two distinct unknown causes (no-snapshot vs wallets-unreadable); `usePcaOwnerAccess` sync hook. Vocabulary (SignableOwner/OwnerActionSubmitterKind) re-exported type-only — single definitions stay in walletBinding/ownerActions. No consumer migrated yet; the submitter's per-prompt inv-16 guards untouched. Tests: direct unit coverage (the model was only ever tested indirectly; detailOwnerMode had no direct test) + an 8-scenario × 6-assertion CALL-SITE PARITY matrix asserting the core reproduces every former site's effective decision for the daemon==connected, wrong-network, and external cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…access model (#1375 item 2) Migrate the render-layer consumers off their per-surface owner-mode copies onto the centralized model, byte-identical render + gating: - ConvictionDetailView: inline ownerMode/writesEnabled block → usePcaOwnerAccess (hoisted above the early returns per Rules of Hooks); drop the hand-injected 'unknown'; DetailBody threads one `access`; ownerGateReason re-expressed on access.mode/access.wrongNetwork (copy unchanged — gateReason deferred). - Funding/PublishingWallets/LifecycleSection: the 3-prop owner bundle (ownerMode+ownerWritesEnabled+ownerTitle) collapses to `access` + the retained `ownerTitle` string (2 props; ownerTitle interpolates accountId/pool state the model doesn't carry yet). - ConvictionOverview: enhancedAccounts.map → resolvePcaOwnerAccess(...); deleted the private ownerModeFor; group buckets still key on access.mode; walletWrongNetwork ≡ mode==='wallet' && wrongNetwork. - PcaAccountCard: re-declared union deleted (= PcaOwnerMode from the model); writes gate via the shared ownerModeWritesEnabled(mode, wrongNetwork) formula on its fallback-resolved mode (usePcaOverview supplies only a partial signal — the fallback is removed in P5). Copy tables kept (gateReason deferred). - ApproveWalletsModal: targetWalletManaged = access.mode==='wallet' && access.writesEnabled; signableOwners = access.signableOwners. signerKindForAccount + the submitter LEFT untouched (P4 / inv-16). Adds `ownerModeWritesEnabled` as the ONE writesEnabled formula (resolvePcaOwnerAccess delegates to it) so the card's fallback path shares it instead of re-deriving. Verified: P3 oracle 162 green (pca-detail-view/account-card/overview/approve-modal/ owner-access/owner-actions, unmodified) + broader PCA domain 574/574 + tsc zero-delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…odal (#1375 item 3) Stop re-discovering the owner/signer on every write. The per-write fetchPca+fetchWalletsBalances in resolveOwnerActionSubmitterForAccount only drove submitter SELECTION (daemon/wallet/read-only) — the actual wallet-prompt safety (loadContext + assertStillConnected + live eth_accounts/eth_chainId + allowance re-check) lives inside walletOwnerActionSubmitter and runs per-prompt regardless. - resolveOwnerActionSubmitterKind now delegates to resolvePcaOwnerAccess(...). submitterKind — the last inv-17 copy collapses onto the model. - New pure ownerActionSubmitterForKind(kind, walletDeps) — selection only. - useOwnerActionSubmitter({access}) builds the submitter ONCE from access.submitterKind; register/deregister/topUp submit directly (no re-fetch). Without access (create-time ownerKey, cross-account deregister), the resolving path is kept verbatim, INCLUDING its pre-flight walletUnavailableReason. - web3/walletOwnerActionSubmitter.ts is byte-untouched — inv-16 guards intact; selection moves up-front, liveness stays call-time (safe because P3's writesEnabled gate blocks the button when the wallet is unusable). Callers: ConvictionDetailView + ApproveWalletsModal's TARGET `owner` (registerAgent) resolve once — killing the N-per-wallet re-resolve on a batch approve. The cross-account deregisters stay per-account: deregisterRenew (the old renew account) on the resolving `deregisterOwner`, and deregisterSelf on a new resolving `crossAccountDeregister` — because self-coverage's prevAccountId varies per wallet and can be owned differently than the target (binding it to the target's submitter would misroute → NotAccountOwner revert). New pin test covers that mixed-owner case. Verified: full inv-16/inv-17 oracle 193 green (existing tests unmodified; +access-path no-fetch tests +mixed-owner self-coverage pin) + PCA/wallet/web3 579 + tsc zero-delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…s unknown (#1375) Adversarial-gate MEDIUM: the item-3 access-path pins the submitter from the caller's PCA snapshot. When that snapshot transiently misses — or a just-created replacement PCA (renew) isn't chain-readable yet — `access.mode` is 'unknown', so `submitterKind` is 'read-only' and every Approve click fails read-only on a genuinely daemon-owned account with no recovery (the modal's useFetch never refetches). Origin re-resolved per click and self-healed. Fix: `useOwnerActionSubmitter` takes the access-path only when the owner is actually classified; on `access.mode === 'unknown'` it falls back to the RESOLVING (re-fetching) path, so a write re-fetches the owner and self-heals on retry. This protects every consumer, not just ApproveWalletsModal. Pin test added. Verified: full inv-16/inv-17 oracle 194 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…+ hook split + pure model (#1375) Address PR #1468 review (1🔴 + 3🟡): - 🔴 (T1) Resolve-once wallet writes could sign with a stale owner classification: the access-path pinned the wallet submitter from a render-time snapshot, so a wallet-owned top-up with a changed owner/connected-wallet could run approveExactIfNeeded (TRAC allowance + gas) before the owner-only call reverts NotAccountOwner. FIX: on the access-path, when submitterKind==='wallet' (or mode==='unknown'), route writes through the RESOLVING path — re-fetch the owner + re-classify per write, returning read-only FIRST when the connected signer is no longer the owner, so no wallet approve/write is attempted. daemon + read-only keep resolve-once (server-side / no-write; safe). Regression test: stale-owner wallet topUp throws ReadOnlyOwnerActionError with neither the wallet approve nor pcaTopUp called. - 🟡 (T3) Split the submitter hook into explicit entry points that encode the same-account vs cross-account boundary in the API: useResolvedOwnerActionSubmitter ({access}) (same-account, resolve-once + the T1 wallet re-verify) and useResolvingOwnerActionSubmitter({accountId?,ownerKey?}) (per-call). Deleted the combined useOwnerActionSubmitter. Callers migrated; create/settle unchanged. - 🟡 (T2, partial) Split ownerAccess.ts into a PURE core (resolvePcaOwnerAccess + ownerModeWritesEnabled + types, no React/store) and a hook file (pca/usePcaOwnerAccess.ts). PcaAccountCard imports the pure formula with no store wiring pulled in. (Held: moving signableOwners/submitterKind off the model — that re-fragments the single model item 2 asked for.) - 🟡 (T4) Extended the wallet-owned wrong-network ConvictionDetailView test to assert every owner-write control (top-up, + Approve publishing wallet, Renew) disabled, not just top-up; probe stays enabled. inv-16 (walletOwnerActionSubmitter.ts byte-untouched) + inv-17 (daemon-first + redundant guard, single-sourced on the model) preserved; independently re-verified by an adversarial pass. Full oracle 195 green + unmodified assertions; tsc zero-delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
… drop the last duplicate classifier (#1375) Review round 2: - 🔴 (T5) ApproveWalletsModal could pin a valid daemon-owned target as read-only while wallets load: it built `access` without a walletsUnknown signal, so if fetchPca resolved before fetchWalletsBalances (owner known, primaryWallet undefined) a daemon-owned PCA classified as external → the resolved read-only submitter rejected a valid owner approval. FIX: pass `walletsUnknown: !wb`, so the load window is 'unknown' (not external) — the resolved submitter then routes through the re-fetching path (the T1 fallback), re-reads wallets, and routes daemon once wallets[0] is known. Test: daemon owner + walletsUnknown ⇒ 'unknown', registerAgent re-fetches to daemon (pcaAddAgent), never the wallet submitter, never read-only. - 🟡 (T6) Actually leave ONE owner-mode classifier: `detailOwnerMode` had no source caller (ConvictionDetailView uses usePcaOwnerAccess; eqAddress is imported from address.ts) — deleted it; the parity test's mode oracle stays on the inline ConvictionOverview.ownerModeFor reproduction. inv-16 (walletOwnerActionSubmitter.ts byte-untouched) + inv-17 preserved. Full oracle 188 green + unmodified; tsc zero-delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
…+ renew test (#1375) Review round 3: - 🟡 (T2) The model was a cross-layer catch-all: sections received submitterKind / signableOwners they never read. Shrink PcaOwnerAccess to the pure owner state { mode, wrongNetwork, writesEnabled, ownerUnknownCause } and derive the layer concerns at their boundaries: submitterKindForOwnerMode(mode) beside ownerActions, signableOwnersFor(primaryWallet, connectedWallet) beside walletBinding. ownerAccess.ts is now a pure leaf (address.js only). Behavior-preserving — the derivations produce the same values; the T1 wallet re-verify is unchanged (submitterKindForOwnerMode(mode) === 'wallet' || mode === 'unknown' → resolving path), inv-17 precedence stays on `mode`. - 🟡 (T7) Add a renew-chain test for a wallet-owned PCA (owner == connected wallet, wallets[0] != connected) asserting the renew CreatePcaModal gets initialOwnerKey === 'hardware'; the daemon-owned counterpart asserts 'hot'. inv-16 (walletOwnerActionSubmitter.ts byte-untouched) + inv-17 preserved; T1/T5 access-path tests unmodified. Full oracle 209 green; tsc zero-delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
… window (#1468) R4 — when the PCA snapshot loads before the wallet balances, the target owner-access is classified `unknown` (the walletsUnknown self-heal that keeps a daemon-owned PCA from being mis-pinned read-only). But the render-time `targetWalletManaged` was false in that window, so a target write that the resolved submitter re-fetches and routes to the BROWSER-WALLET signer (owner == connected wallet) ran with no device-progress step and deviceTotal 0 → walletBatchSigning off → the modal stayed freely dismissable / Cancel unlocked during an in-flight on-chain wallet signature. Arm the lock pessimistically whenever the target write could open a wallet signature: `access.mode === 'unknown' && owner == connectedWallet`. This makes runApproveBatch set the device step + cancel/dismiss lock BEFORE the in-flight wallet action. Daemon-owned targets (owner == wallets[0], normally != the connected wallet) are unaffected and still self-heal to the daemon. Tests (both proven red without the fix): - loading-window: a connected-owner target still arms the wallet-signing lock - inverse guard: a daemon-owned target (owner != connected wallet) is NOT falsely locked and self-heals to the daemon (pins the sameAddress conjunct) inv-16 (walletOwnerActionSubmitter.ts byte-untouched) and inv-17 (owner precedence) preserved. 4-lens adversarial gate: GO. tsc zero-delta; build:ui ok. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3swmeQGT8WGttaZsp1NQJ
This was referenced Jul 6, 2026
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
2 participants
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.
Summary
Closes the #1375 owner-access cleanup. Items 1, 4, 5 already landed in PR6 (#1348) / PR7 (#1350) — this PR delivers the two remaining core items, behavior-preserving, with the owner/signing invariants preserved byte-for-byte:
pca/ownerAccess.ts:resolvePcaOwnerAccess(), replaces the four hand-copied owner-mode classifiers (detailOwnerMode,ownerActions.resolveOwnerActionSubmitterKind,ConvictionOverview.ownerModeFor,PcaAccountCard's fallback). Returns{ mode, wrongNetwork, writesEnabled, submitterKind, ownerUnknownCause?, signableOwners }+ ausePcaOwnerAccesssync hook. Consumed by overview/cards, detail view + its sections, the approve modal, and the owner-action submitter resolution.&& !eqAddress(owner, primaryWallet)guard;settlestays a daemon carve-out.modeis derived from addresses only; wrong-network gates writes separately viawritesEnabled, so every consumer keeps its effective (classify + gate) behavior. A call-site-parity test pins this against all four legacy sites.fetchPca+fetchWalletsBalancesinresolveOwnerActionSubmitterForAccountonly drove submitter selection; the actual wallet-prompt safety lives insidewalletOwnerActionSubmitterand runs per-prompt.useOwnerActionSubmitter({ access })now selects the submitter once from the already-loaded model and submits directly — killing the N-per-wallet re-resolve on a batch approve.web3/walletOwnerActionSubmitter.tsis byte-untouched — inv-16 (loadContext/assertStillConnected/liveeth_accounts/eth_chainId/allowance re-check) intact.eqAddresscopies into one sharedpca/address.tsleaf.Verification: an internal 5-lens adversarial gate (inv-16 liveness / inv-17 precedence / wrong-network-fold parity per consumer / signer-once behavior / completeness) diffing against
main. It confirmed inv-16/inv-17 preserved and caught two behavior-relevant edges, both fixed:deregisterSelfoperates on a per-wallet-varyingprevAccountId, so it stays on the per-account resolving path (not the target's once-resolved submitter); otherwise a differently-owned prevAccount would misroute →NotAccountOwnerrevert. Pin test added.access.mode === 'unknown'(a transient snapshot miss or a just-created replacement PCA not yet chain-readable), so writes recover on retry instead of sticking read-only.Related
useConnectedWalletPcas) and PCA node-UI: extract ApproveWalletsModal batch orchestration into a reducer/runner #1350 (PR7 — the approve batch).Deliberately scoped / offered follow-ups
useWalletTxProgress) is satisfied for Create + Detail (PR6). The Approve flow keeps its ownapproveBatch.tsmachine — a different shape (sequential N-wallet batch, ≤2 tx/wallet, the Gossip ingestion validation is incomplete (I-002) #9 confirmed≠submitted + "i of N" hardened in PR7). Folding it into the fixed single-tx template would regress exactly what PR7's gate hardened, so it stays separate by design.disabledReasonfield / one-prop sections. The model centralizes the decision (mode/writesEnabled/submitterKind/signableOwners); the per-consumer disabled copy (which interpolatesaccountId/pool state) stays consumer-owned, soFunding/PublishingWallets/LifecycleSectiontakeaccess+ownerTitle(3→2 props) rather than one. Happy to add a typedgateReasondiscriminant so the sections dropownerTitleand the card drops its copy tables — flagging as a small, easily-added follow-up if preferred.usePcaOverviewemitting the full model (deletingPcaAccountCard'sownerIsPrimaryWalletfallback) is deferred — it widens a shared bell/dashboard/settings poller's dependency surface; low value, higher blast radius.Files changed
packages/node-ui/src/ui/pca/address.tseqAddressleaf (replaces 7 copies).packages/node-ui/src/ui/pca/ownerAccess.tsresolvePcaOwnerAccesscore +usePcaOwnerAccess+ownerModeWritesEnabled+ the typed model.packages/node-ui/src/ui/pca/ownerActions.tsownerActionSubmitterForKind;useOwnerActionSubmitter({access})resolve-once (with unknown→resolving self-heal).packages/node-ui/src/ui/pages/conviction/ConvictionDetailView.tsxusePcaOwnerAccess; oneaccessthreaded to the sections; owner submitter resolved once..../FundingSection.tsx·.../PublishingWalletsSection.tsx·.../LifecycleSection.tsxaccess(+ retainedownerTitle)..../ConvictionOverview.tsxenhancedAccounts.map(resolvePcaOwnerAccess); deletedownerModeFor..../PcaAccountCard.tsxwritesEnabledvia the shared formula..../ApproveWalletsModal.tsxtargetWalletManaged/signableOwnersfrom the model; targetownerresolved once; cross-account deregisters resolve per-account..../web3/walletOwnerActionSubmitter.tseqAddressimport swap — inv-16 guards byte-untouched.test/pca-owner-access.test.ts(new) ·test/pca-owner-actions.test.ts·test/pca-approve-modal.test.tsTest plan
pca-owner-actions,pca-wallet-owner-actions(the mid-flow account/chain/provider-swap-abort crux),pca-approve-batch,pca-approve-modal,pca-detail-view,pca-account-card,pca-owner-access— 194 passed.tsc -p src/ui/tsconfig.json— zero-delta (41 pre-existing graph-viz errors, none in touched files).main— inv-16/inv-17 preserved; 2 edges caught + fixed.kosava-node-ui-e2e— note: thewm-swm-vm-ui-cycletriples-migration e2e flake is unrelated / tracked in Intermittent WM→SWM promote migration fails to land the root in shared memory (blocks node-ui e2e on main) #1464).