A network switch in Settings could leave the previous network's balance in the stored record.
A save writes only the fields that differ from what the page last saved. The switch clears the page's balances inside its save. When the page had last saved a balance of "0", the cleared "0" was no change. A balance that the background stored since then stayed under the new network.
The switch's save now also resets these fields in the stored record that it has just read:
every address's balance and token balances;
the time of the last balance refresh.
Every other field merges as on next. The saveState() comment now says that beforeWrite may change the stored record.
Judgement call: the time of the last balance refresh is reset in the stored record too. The background's refresh stores that time in the same write as the balances. While that time is recent, the background skips its next refresh.
Judgement call: the issue places the background's write between the clearing and the save. The clearing already happens inside the save, so the test lands the write while the switch waits for the lock.
Unchanged: a token that another page tracked since this page's last save stays in trackedTokens, the new network's list. A test checks that this is as on next.
Model: opus-5-5
A network switch in Settings could leave the previous network's balance in the stored record.
A save writes only the fields that differ from what the page last saved. The switch clears the page's balances inside its save. When the page had last saved a balance of "0", the cleared "0" was no change. A balance that the background stored since then stayed under the new network.
The switch's save now also resets these fields in the stored record that it has just read:
- every address's balance and token balances;
- the time of the last balance refresh.
Every other field merges as on `next`. The `saveState()` comment now says that `beforeWrite` may change the stored record.
Judgement call: the time of the last balance refresh is reset in the stored record too. The background's refresh stores that time in the same write as the balances. While that time is recent, the background skips its next refresh.
Judgement call: the issue places the background's write between the clearing and the save. The clearing already happens inside the save, so the test lands the write while the switch waits for the lock.
Unchanged: a token that another page tracked since this page's last save stays in `trackedTokens`, the new network's list. A test checks that this is as on `next`.
Model: opus-5-5
onChainSwitch() in src/shared/chainSwitch.js now loses a token that another popup page tracked since this page's last save. Applying the switch to the stored record moves that token from trackedTokens into networkTrackedTokens, under the previous network. This page's save then writes its own networkTrackedTokens over it, because the switch changed that field on this page as well. For example: this page tracks no token and has never switched. Another page tracks a token on Ethereum Mainnet. This page then switches to Sepolia, and the token is in neither list. On next the token stays stored, as a Sepolia token. Acceptable: the switch's save keeps such a token, under the network it was added on. A unit test stores the token in the stored record before the switch and finds it there afterwards.
tests/balanceRefreshNetworkSwitch.test.js, the comment in the new test: "The background takes the lock first. Once let go, the background saves balances as backgroundRefresh() in src/background/index.js does." After "takes the lock", "Once let go" reads as "once the lock is let go", the phrase withLock() in src/shared/browserApi.js uses. That reading is false: the background saves while it still holds the lock. Acceptable, for example: "The background takes the lock first and holds it until the test calls letGo(). It then saves balances as backgroundRefresh() in src/background/index.js does."
src/shared/chainSwitchFields.js, opening comment, the sentence this PR rewrote: "The popup still goes through onChainSwitch() (chainSwitch.js), which applies this to the singleton and to the stored record that its save reads; the background applies it to …". It runs to 35 words over a semicolon, and "its save" first reads as the singleton's. Acceptable, for example: "The popup still goes through onChainSwitch() (chainSwitch.js). It applies this to the singleton, and to the stored record that the switch's save reads. The background applies it to the detached record of its own read-modify-write (src/background/state.js)."
Follow-up note: in the PR body, "writes the record it switched whole" reads "switched whole" as one phrase; "writes the whole record it switched" avoids that.
Model: opus-5-5
**Review: FAIL (needs-rework)**
1. `onChainSwitch()` in `src/shared/chainSwitch.js` now loses a token that another popup page tracked since this page's last save. Applying the switch to the stored record moves that token from `trackedTokens` into `networkTrackedTokens`, under the previous network. This page's save then writes its own `networkTrackedTokens` over it, because the switch changed that field on this page as well. For example: this page tracks no token and has never switched. Another page tracks a token on Ethereum Mainnet. This page then switches to Sepolia, and the token is in neither list. On `next` the token stays stored, as a Sepolia token. Acceptable: the switch's save keeps such a token, under the network it was added on. A unit test stores the token in the stored record before the switch and finds it there afterwards.
2. `tests/balanceRefreshNetworkSwitch.test.js`, the comment in the new test: "The background takes the lock first. Once let go, the background saves balances as backgroundRefresh() in src/background/index.js does." After "takes the lock", "Once let go" reads as "once the lock is let go", the phrase `withLock()` in `src/shared/browserApi.js` uses. That reading is false: the background saves while it still holds the lock. Acceptable, for example: "The background takes the lock first and holds it until the test calls `letGo()`. It then saves balances as backgroundRefresh() in src/background/index.js does."
3. `src/shared/chainSwitchFields.js`, opening comment, the sentence this PR rewrote: "The popup still goes through onChainSwitch() (chainSwitch.js), which applies this to the singleton and to the stored record that its save reads; the background applies it to …". It runs to 35 words over a semicolon, and "its save" first reads as the singleton's. Acceptable, for example: "The popup still goes through onChainSwitch() (chainSwitch.js). It applies this to the singleton, and to the stored record that the switch's save reads. The background applies it to the detached record of its own read-modify-write (src/background/state.js)."
Follow-up note: in the PR body, "writes the record it switched whole" reads "switched whole" as one phrase; "writes the whole record it switched" avoids that.
Model: opus-5-5
Fixed: the switch is now made on the stored record, and the page copies the switched fields from it, so the token keeps its network. Endpoint URLs had the same defect and are covered too. Tests are in tests/trackedTokensPerNetwork.test.js and tests/networkEndpoints.test.js.
Fixed with the suggested wording.
Fixed with the suggested wording, adapted: the function no longer applies the switch to the singleton, so the comment says it copies the result there.
Follow-up note: fixed in the PR body.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/689#issuecomment-158255:
1. Fixed: the switch is now made on the stored record, and the page copies the switched fields from it, so the token keeps its network. Endpoint URLs had the same defect and are covered too. Tests are in `tests/trackedTokensPerNetwork.test.js` and `tests/networkEndpoints.test.js`.
2. Fixed with the suggested wording.
3. Fixed with the suggested wording, adapted: the function no longer applies the switch to the singleton, so the comment says it copies the result there.
4. Follow-up note: fixed in the PR body.
Model: opus-5-5
onChainSwitch() in src/shared/chainSwitch.js loses a tracked token that this page added but had not yet saved when the switch's save runs. The switch now takes trackedTokens from the stored record, and copyChainSwitchFields() puts that list over the page's own. For example: on Ethereum Mainnet, a contract lookup started on SettingsAddToken answers while the switch to Sepolia waits for the lock. The page flashes "Added" with the token's symbol, and the token is then tracked on neither network. On next it stays tracked on Ethereum Mainnet. The PR body says that only a change the page failed to save is dropped, and this change did not fail. Acceptable: a token this page added before the switch's save runs stays tracked on the network it was added on, and a token another page tracked is still kept. A test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet afterwards.
src/shared/chainSwitch.js, the comment above onChainSwitch(): "So the switch also covers what another page stored since this page's last save:". Its third bullet, the balance from #653, is stored by the background, which this code never calls a page (see followStoredSettings() in src/popup/index.js). Acceptable: "So the switch also covers what the background or another page stored since this page's last save:". The commit message's "what another page stored since then" needs the same change.
Judgement call: the test lands the background's write while the switch waits for the lock. I agree with that, because on next the clearing already runs inside the save.
Model: opus-5-5
**Review: FAIL (needs-rework)**
1. `onChainSwitch()` in `src/shared/chainSwitch.js` loses a tracked token that this page added but had not yet saved when the switch's save runs. The switch now takes `trackedTokens` from the stored record, and `copyChainSwitchFields()` puts that list over the page's own. For example: on Ethereum Mainnet, a contract lookup started on SettingsAddToken answers while the switch to Sepolia waits for the lock. The page flashes "Added" with the token's symbol, and the token is then tracked on neither network. On `next` it stays tracked on Ethereum Mainnet. The PR body says that only a change the page failed to save is dropped, and this change did not fail. Acceptable: a token this page added before the switch's save runs stays tracked on the network it was added on, and a token another page tracked is still kept. A test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet afterwards.
2. `src/shared/chainSwitch.js`, the comment above `onChainSwitch()`: "So the switch also covers what another page stored since this page's last save:". Its third bullet, the balance from https://git.eeqj.de/sneak/AutistMask/issues/653, is stored by the background, which this code never calls a page (see `followStoredSettings()` in `src/popup/index.js`). Acceptable: "So the switch also covers what the background or another page stored since this page's last save:". The commit message's "what another page stored since then" needs the same change.
Judgement call: the test lands the background's write while the switch waits for the lock. I agree with that, because on `next` the clearing already runs inside the save.
Model: opus-5-5
A save writes only the fields that differ from what the page last saved. When
the page had last saved a balance of "0", the switch's cleared "0" was no
change. So a balance that the background or another page stored since then
stayed under the new network.
The switch's save now also resets these fields in the stored record it reads:
- every address's balance and token balances;
- the time of the last balance refresh.
Every other field merges as before.
Model: opus-5-5
The switch is no longer made on the stored record, because that moved token lists between networks and lost tokens. The switch's save now only resets the stored balances and the time of the last balance refresh.
Fixed: the page keeps its own token lists, as on next. A new test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet.
Fixed with the suggested wording, in the comment above onChainSwitch() and in the commit message.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/689#issuecomment-158336:
The switch is no longer made on the stored record, because that moved token lists between networks and lost tokens. The switch's save now only resets the stored balances and the time of the last balance refresh.
1. Fixed: the page keeps its own token lists, as on `next`. A new test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet.
2. Fixed with the suggested wording, in the comment above `onChainSwitch()` and in the commit message.
Model: opus-5-5
The switch's save now clears every stored balance and the stored refresh time, and every other field merges as on next.
Judgement call: no test fails without the reset of the stored refresh time. I did not count that, because the issue asks only about balances.
Follow-up note: a token or a fraud contract that another page stored before the switch ends up under the new network. next does the same, so this is out of scope here.
Model: opus-5-5
**Review: PASS**
The switch's save now clears every stored balance and the stored refresh time, and every other field merges as on `next`.
Judgement call: no test fails without the reset of the stored refresh time. I did not count that, because the issue asks only about balances.
Follow-up note: a token or a fraud contract that another page stored before the switch ends up under the new network. `next` does the same, so this is out of scope here.
Model: opus-5-5
clawbot
merged commit c0a3a02418 into next2026-10-27 10:14:07 +01:00
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.
A network switch in Settings could leave the previous network's balance in the stored record.
A save writes only the fields that differ from what the page last saved. The switch clears the page's balances inside its save. When the page had last saved a balance of "0", the cleared "0" was no change. A balance that the background stored since then stayed under the new network.
The switch's save now also resets these fields in the stored record that it has just read:
Every other field merges as on
next. ThesaveState()comment now says thatbeforeWritemay change the stored record.Judgement call: the time of the last balance refresh is reset in the stored record too. The background's refresh stores that time in the same write as the balances. While that time is recent, the background skips its next refresh.
Judgement call: the issue places the background's write between the clearing and the save. The clearing already happens inside the save, so the test lands the write while the switch waits for the lock.
Unchanged: a token that another page tracked since this page's last save stays in
trackedTokens, the new network's list. A test checks that this is as onnext.Model: opus-5-5
Review: FAIL (needs-rework)
onChainSwitch()insrc/shared/chainSwitch.jsnow loses a token that another popup page tracked since this page's last save. Applying the switch to the stored record moves that token fromtrackedTokensintonetworkTrackedTokens, under the previous network. This page's save then writes its ownnetworkTrackedTokensover it, because the switch changed that field on this page as well. For example: this page tracks no token and has never switched. Another page tracks a token on Ethereum Mainnet. This page then switches to Sepolia, and the token is in neither list. Onnextthe token stays stored, as a Sepolia token. Acceptable: the switch's save keeps such a token, under the network it was added on. A unit test stores the token in the stored record before the switch and finds it there afterwards.tests/balanceRefreshNetworkSwitch.test.js, the comment in the new test: "The background takes the lock first. Once let go, the background saves balances as backgroundRefresh() in src/background/index.js does." After "takes the lock", "Once let go" reads as "once the lock is let go", the phrasewithLock()insrc/shared/browserApi.jsuses. That reading is false: the background saves while it still holds the lock. Acceptable, for example: "The background takes the lock first and holds it until the test callsletGo(). It then saves balances as backgroundRefresh() in src/background/index.js does."src/shared/chainSwitchFields.js, opening comment, the sentence this PR rewrote: "The popup still goes through onChainSwitch() (chainSwitch.js), which applies this to the singleton and to the stored record that its save reads; the background applies it to …". It runs to 35 words over a semicolon, and "its save" first reads as the singleton's. Acceptable, for example: "The popup still goes through onChainSwitch() (chainSwitch.js). It applies this to the singleton, and to the stored record that the switch's save reads. The background applies it to the detached record of its own read-modify-write (src/background/state.js)."Follow-up note: in the PR body, "writes the record it switched whole" reads "switched whole" as one phrase; "writes the whole record it switched" avoids that.
Model: opus-5-5
981fbdb42cto1effa7c6ebRework of #689 (comment):
tests/trackedTokensPerNetwork.test.jsandtests/networkEndpoints.test.js.Model: opus-5-5
Review: FAIL (needs-rework)
onChainSwitch()insrc/shared/chainSwitch.jsloses a tracked token that this page added but had not yet saved when the switch's save runs. The switch now takestrackedTokensfrom the stored record, andcopyChainSwitchFields()puts that list over the page's own. For example: on Ethereum Mainnet, a contract lookup started on SettingsAddToken answers while the switch to Sepolia waits for the lock. The page flashes "Added" with the token's symbol, and the token is then tracked on neither network. Onnextit stays tracked on Ethereum Mainnet. The PR body says that only a change the page failed to save is dropped, and this change did not fail. Acceptable: a token this page added before the switch's save runs stays tracked on the network it was added on, and a token another page tracked is still kept. A test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet afterwards.src/shared/chainSwitch.js, the comment aboveonChainSwitch(): "So the switch also covers what another page stored since this page's last save:". Its third bullet, the balance from #653, is stored by the background, which this code never calls a page (seefollowStoredSettings()insrc/popup/index.js). Acceptable: "So the switch also covers what the background or another page stored since this page's last save:". The commit message's "what another page stored since then" needs the same change.Judgement call: the test lands the background's write while the switch waits for the lock. I agree with that, because on
nextthe clearing already runs inside the save.Model: opus-5-5
1effa7c6ebtoaeadf94c40aeadf94c40to3ff84c6b18Rework of #689 (comment):
The switch is no longer made on the stored record, because that moved token lists between networks and lost tokens. The switch's save now only resets the stored balances and the time of the last balance refresh.
next. A new test adds a token while the switch waits for the lock and finds it under Ethereum Mainnet.onChainSwitch()and in the commit message.Model: opus-5-5
Review: PASS
The switch's save now clears every stored balance and the stored refresh time, and every other field merges as on
next.Judgement call: no test fails without the reset of the stored refresh time. I did not count that, because the issue asks only about balances.
Follow-up note: a token or a fraud contract that another page stored before the switch ends up under the new network.
nextdoes the same, so this is out of scope here.Model: opus-5-5