fix: a cold worker answers eth_chainId and net_version from never-loaded state, reporting mainnet to a page whose user is on Sepolia #317
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/AutistMask#317
Reference in New Issue
Block a user
Found while reworking #313. The read-side twin of #316, same root cause, no data loss.
eth_chainId(src/background/index.js:667) andnet_version(:671) answer fromcurrentNetwork(), which reads the module-levelstatesingleton. The MV3 service worker never populates that singleton, so a worker revived by the page's own message holdsDEFAULT_STATEand reports mainnet —0x1/1— to a page whose user is actually on Sepolia.Consequence
Chain confusion in the page's direction: a dApp asks what chain the wallet is on, is told mainnet, and builds its interaction accordingly while the user's wallet is on a testnet. The user's own signing path is not directly at risk —
chainIdis verified against local state before signing, and the transaction path already callsloadState()atsrc/background/index.js:1286— so this misleads the page rather than mis-signing the transaction. It is filed against 1.0.0 because chain confusion is the class this milestone is closing, and the fix is one line.The write-side of the same defect destroys all wallets and is tracked separately as #316, fixed in #313.
Audit context from that rework, worth keeping: only four background call sites reach
saveState(), and the chain switch was the only defective one — the remembered-approval (:588/:595), remembered-denial (:602/:609) andbackgroundRefresh(:1052/:1064) paths each alreadyloadState()before mutating. So the write side is now believed complete; this is the remaining read side.Definition of done
eth_chainIdandnet_versionanswer from loaded state, not from the unpopulated singleton.currentNetwork()or the singleton is audited and named in the PR body.loadState()reports the stored chain, not the default. Must fail against current head and pass after.make checkgreen.Fixed in #319 (base
next).eth_chainIdandnet_versionare now one branch behind a singleawait loadState(), rather than two loads for the same read of the same value. Same placement idiom as the chain-switch handler and the transaction path.Read-side audit: the other singleton reads in the background are
wallet_switchEthereumChain, the transaction verify/broadcast path andbackgroundRefresh, and all three already load first; every other handler answers from storage per call throughgetState(). One stale read is named but not fixed, being a different handler rather than the same one-line shape —handleSendTransactioncallsgetProvider()with no network name (src/background/index.js:934), sosrc/shared/balances.js:26falls back to the unloaded singleton for ethers' static network hint and a cold-worker send on Sepolia is prepared with a mainnetchainId. It is caught before broadcast by the verify step against loaded state, so it fails the send rather than sending on the wrong chain. Full table in the PR body.Verified:
tests/coldWorkerChainId.test.jsdrives the real state module over write-retaining storage and never loads state itself. Reverting onlysrc/background/index.jstonextgives 3 failed / 775 passed — exactly the three cases that read the chain on a cold worker, each answering0x1/1instead of0xaa36a7/11155111. With the fixmake checkis green: 36 suites / 778 tests,test-verify-build18 cases,check-censored143 files, and lint executed uncached in the pinned container.Correction to the comment above: the
await loadState()approach it describes was rejected in review and is not what #319 now does.Loading the singleton here introduced a new defect. These two methods are ungated and the injected provider sends
eth_chainIdon every page load, so any page could land aloadState()inside an in-flightbackgroundRefresh()— which detaches the address objectsrefreshBalances()is mutating in place, persisting the pre-refresh balances while still stampinglastBalanceRefresh.Both now answer from
getState()instead — the per-call detached storage read the other read handlers use — so the read touches no shared state at all. The audit and the verification are unchanged in substance; details in the PR body.