A network switch clears every address's balances. A refresh that started on the previous network wrote that network's balances back over them when its requests returned, and stamped lastBalanceRefresh, so the new network then waited a full interval.
doRefreshAndRender() (src/popup/index.js) and backgroundRefresh() (src/background/index.js) now write a refresh's results only if the network it started on is still the active one. Otherwise nothing is written and lastBalanceRefresh stays at the 0 the switch set.
What the diff does not show:
The background checks inside updateState(), under the lock from #644. A dropped refresh still writes that record back unchanged.
An open popup is not told of a switch the background makes after the user approves a site's request, so the popup checks the stored record too. saveState() takes an optional function, called inside the lock with the stored record as just read; it may change state, or return false so nothing is written. The popup refreshes a structuredClone() of its wallets and copies the results onto state by address only from inside that function, so a dropped refresh leaves nothing for a later save to write.
saveStateOnce() now snapshots the page's state after reading the stored record, not before, so that function's change is in the write.
Disclosure:
Judgement call: the popup's copy loop repeats the background's instead of sharing a function. A new export from src/shared/balances.js would have to be added to the stand-in for that module in every test that replaces it.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/AutistMask/issues/577.
A network switch clears every address's balances. A refresh that started on the previous network wrote that network's balances back over them when its requests returned, and stamped `lastBalanceRefresh`, so the new network then waited a full interval.
`doRefreshAndRender()` (`src/popup/index.js`) and `backgroundRefresh()` (`src/background/index.js`) now write a refresh's results only if the network it started on is still the active one. Otherwise nothing is written and `lastBalanceRefresh` stays at the 0 the switch set.
What the diff does not show:
- The background checks inside `updateState()`, under the lock from https://git.eeqj.de/sneak/AutistMask/pulls/644. A dropped refresh still writes that record back unchanged.
- An open popup is not told of a switch the background makes after the user approves a site's request, so the popup checks the stored record too. `saveState()` takes an optional function, called inside the lock with the stored record as just read; it may change `state`, or return false so nothing is written. The popup refreshes a `structuredClone()` of its wallets and copies the results onto `state` by address only from inside that function, so a dropped refresh leaves nothing for a later save to write.
- `saveStateOnce()` now snapshots the page's state after reading the stored record, not before, so that function's change is in the write.
Disclosure:
- Judgement call: the popup's copy loop repeats the background's instead of sharing a function. A new export from `src/shared/balances.js` would have to be added to the stand-in for that module in every test that replaces it.
Model: opus-5-5
Closed the gap the PR disclosed: the popup now also drops its refresh when the stored record's network, read inside its save under the lock, is not the one the refresh started on, which catches a switch the background makes. This goes through a new optional argument to saveState() (see the PR body), and the popup copies the results onto its own address records only from inside it. Added a test in tests/balanceRefreshNetworkSwitch.test.js where the background switches the network while a popup refresh is held open; it fails against the previous head 0375f83. The README sentence now states the rule as the stored network.
Model: opus-5-5
Closed the gap the PR disclosed: the popup now also drops its refresh when the stored record's network, read inside its save under the lock, is not the one the refresh started on, which catches a switch the background makes. This goes through a new optional argument to `saveState()` (see the PR body), and the popup copies the results onto its own address records only from inside it. Added a test in `tests/balanceRefreshNetworkSwitch.test.js` where the background switches the network while a popup refresh is held open; it fails against the previous head `0375f83`. The README sentence now states the rule as the stored network.
Model: opus-5-5
doRefreshAndRender() in src/popup/index.js: no test covers the check of the page's own network (state.networkId !== networkId). It is the only check that catches a switch the user makes in Settings while the refresh's save is waiting for the lock or for its read of the stored record. At that point the stored record still holds the old network, so without this check the old network's balances are written under the new one. Every test still passes with that line removed. Acceptable: a test in tests/balanceRefreshNetworkSwitch.test.js that holds the refresh's save at its storage read, switches the network in Settings during it, and checks that no old balance reaches the new network, in state or in storage. It must fail with that line removed.
saveStateOnce() in src/shared/state.js: if (beforeWrite && !beforeWrite(fresh)) return; skips the write on any falsy return, so a callback that changes state and returns nothing writes nothing, and no error is raised. The comment on saveState() says the write is skipped when the callback returns false. Acceptable: skip only on an explicit false (beforeWrite(fresh) === false), as the comment says, so a callback that returns nothing still saves.
The comment above saveState() in src/shared/state.js still says each save "compares the page's state as it stands when its turn begins". After this change, the page's state is copied only once the stored record has been read and beforeWrite has run. Acceptable: that sentence says when the copy is taken now.
Judgement call: the copy loop repeated in the popup and the background, disclosed in the PR body, is not raised.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
1. `doRefreshAndRender()` in `src/popup/index.js`: no test covers the check of the page's own network (`state.networkId !== networkId`). It is the only check that catches a switch the user makes in Settings while the refresh's save is waiting for the lock or for its read of the stored record. At that point the stored record still holds the old network, so without this check the old network's balances are written under the new one. Every test still passes with that line removed. Acceptable: a test in `tests/balanceRefreshNetworkSwitch.test.js` that holds the refresh's save at its storage read, switches the network in Settings during it, and checks that no old balance reaches the new network, in `state` or in storage. It must fail with that line removed.
2. `saveStateOnce()` in `src/shared/state.js`: `if (beforeWrite && !beforeWrite(fresh)) return;` skips the write on any falsy return, so a callback that changes `state` and returns nothing writes nothing, and no error is raised. The comment on `saveState()` says the write is skipped when the callback returns false. Acceptable: skip only on an explicit `false` (`beforeWrite(fresh) === false`), as the comment says, so a callback that returns nothing still saves.
3. The comment above `saveState()` in `src/shared/state.js` still says each save "compares the page's state as it stands when its turn begins". After this change, the page's state is copied only once the stored record has been read and `beforeWrite` has run. Acceptable: that sentence says when the copy is taken now.
Judgement call: the copy loop repeated in the popup and the background, disclosed in the PR body, is not raised.
Model: opus-5-5
The popup's doRefreshAndRender() and the background's backgroundRefresh()
each note the network a refresh starts on and write its balances only if
that network is still the active one when the requests return. Otherwise
nothing is written and lastBalanceRefresh stays as the switch left it. The
background checks inside updateState(). The popup refreshes a copy of its
wallets and copies the results onto `state` from inside its save, under the
lock, only if both its own network and the stored record's still match, so
a switch the background makes is caught too. saveState() takes an optional
function, run there on the stored record before the page's state is
compared, that may change `state` or return false to cancel the write.
Model: opus-5-5
Added a test to tests/balanceRefreshNetworkSwitch.test.js that holds the refresh's save at its read of the stored record while the user switches to Sepolia in Settings, then checks state and storage. It fails with the page's network check removed.
saveStateOnce() now skips the write only when beforeWrite returns false.
The comment above saveState() now says the page's state is copied once the stored record has been read and beforeWrite has run.
Model: opus-5-5
Rework for review comment 152217:
1. Added a test to `tests/balanceRefreshNetworkSwitch.test.js` that holds the refresh's save at its read of the stored record while the user switches to Sepolia in Settings, then checks `state` and storage. It fails with the page's network check removed.
2. `saveStateOnce()` now skips the write only when `beforeWrite` returns `false`.
3. The comment above `saveState()` now says the page's state is copied once the stored record has been read and `beforeWrite` has run.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #577.
A network switch clears every address's balances. A refresh that started on the previous network wrote that network's balances back over them when its requests returned, and stamped
lastBalanceRefresh, so the new network then waited a full interval.doRefreshAndRender()(src/popup/index.js) andbackgroundRefresh()(src/background/index.js) now write a refresh's results only if the network it started on is still the active one. Otherwise nothing is written andlastBalanceRefreshstays at the 0 the switch set.What the diff does not show:
updateState(), under the lock from #644. A dropped refresh still writes that record back unchanged.saveState()takes an optional function, called inside the lock with the stored record as just read; it may changestate, or return false so nothing is written. The popup refreshes astructuredClone()of its wallets and copies the results ontostateby address only from inside that function, so a dropped refresh leaves nothing for a later save to write.saveStateOnce()now snapshots the page's state after reading the stored record, not before, so that function's change is in the write.Disclosure:
src/shared/balances.jswould have to be added to the stand-in for that module in every test that replaces it.Model: opus-5-5
0375f83470toc13828000dclawbot referenced this pull request2026-10-20 21:05:23 +02:00
Closed the gap the PR disclosed: the popup now also drops its refresh when the stored record's network, read inside its save under the lock, is not the one the refresh started on, which catches a switch the background makes. This goes through a new optional argument to
saveState()(see the PR body), and the popup copies the results onto its own address records only from inside it. Added a test intests/balanceRefreshNetworkSwitch.test.jswhere the background switches the network while a popup refresh is held open; it fails against the previous head0375f83. The README sentence now states the rule as the stored network.Model: opus-5-5
Review: FAIL (
needs-rework)doRefreshAndRender()insrc/popup/index.js: no test covers the check of the page's own network (state.networkId !== networkId). It is the only check that catches a switch the user makes in Settings while the refresh's save is waiting for the lock or for its read of the stored record. At that point the stored record still holds the old network, so without this check the old network's balances are written under the new one. Every test still passes with that line removed. Acceptable: a test intests/balanceRefreshNetworkSwitch.test.jsthat holds the refresh's save at its storage read, switches the network in Settings during it, and checks that no old balance reaches the new network, instateor in storage. It must fail with that line removed.saveStateOnce()insrc/shared/state.js:if (beforeWrite && !beforeWrite(fresh)) return;skips the write on any falsy return, so a callback that changesstateand returns nothing writes nothing, and no error is raised. The comment onsaveState()says the write is skipped when the callback returns false. Acceptable: skip only on an explicitfalse(beforeWrite(fresh) === false), as the comment says, so a callback that returns nothing still saves.saveState()insrc/shared/state.jsstill says each save "compares the page's state as it stands when its turn begins". After this change, the page's state is copied only once the stored record has been read andbeforeWritehas run. Acceptable: that sentence says when the copy is taken now.Judgement call: the copy loop repeated in the popup and the background, disclosed in the PR body, is not raised.
Model: opus-5-5
c13828000dtod5921f0639Rework for review comment 152217:
tests/balanceRefreshNetworkSwitch.test.jsthat holds the refresh's save at its read of the stored record while the user switches to Sepolia in Settings, then checksstateand storage. It fails with the page's network check removed.saveStateOnce()now skips the write only whenbeforeWritereturnsfalse.saveState()now says the page's state is copied once the stored record has been read andbeforeWritehas run.Model: opus-5-5
Review: PASS
Model: opus-5-5