ConfirmTx used the balances Send handed it. Since #587 a screen the popup reopens onto is drawn for the network its transaction was reviewed on, so that balance was of the right network, but as Send knew it, possibly long before.
ConfirmTx now reads the From address's balances each time it is shown: ETH, and for a token send balanceOf() at the scale the amount is shown with. It asks the node reviewedNetworkProvider() picks, as the fee estimate and recipient checks do, so a screen drawn again after a network switch shows the reviewed network's balance and Sign & Send still refuses because the active network changed. A token whose scale is unknown is not read and stays unknown. Until the read returns the balance line reads "Loading..." and Sign & Send stays disabled; a failed read shows unknown (SYMBOL), never zero.
The fee estimate starts after the read, because a max ETH send's amount is the balance minus the estimate's reserve.
The two messages the read can bring, that the balance does not cover the amount and that it could not be read, each have a box reserved from the first paint, so nothing moves when the read returns. The first no longer quotes the balance; the line above shows it. Send no longer hands its balances to ConfirmTx.
Judgement call: the token balance comes from the node, as the plan on #558 says, so it can differ from the explorer's cached holding Send shows.
Model: opus-5-5
ConfirmTx used the balances Send handed it. Since https://git.eeqj.de/sneak/AutistMask/pulls/587 a screen the popup reopens onto is drawn for the network its transaction was reviewed on, so that balance was of the right network, but as Send knew it, possibly long before.
ConfirmTx now reads the From address's balances each time it is shown: ETH, and for a token send `balanceOf()` at the scale the amount is shown with. It asks the node `reviewedNetworkProvider()` picks, as the fee estimate and recipient checks do, so a screen drawn again after a network switch shows the reviewed network's balance and Sign & Send still refuses because the active network changed. A token whose scale is unknown is not read and stays unknown. Until the read returns the balance line reads "Loading..." and Sign & Send stays disabled; a failed read shows `unknown (SYMBOL)`, never zero.
The fee estimate starts after the read, because a max ETH send's amount is the balance minus the estimate's reserve.
The two messages the read can bring, that the balance does not cover the amount and that it could not be read, each have a box reserved from the first paint, so nothing moves when the read returns. The first no longer quotes the balance; the line above shows it. Send no longer hands its balances to ConfirmTx.
Judgement call: the token balance comes from the node, as the plan on https://git.eeqj.de/sneak/AutistMask/issues/558 says, so it can differ from the explorer's cached holding Send shows.
Model: opus-5-5
The fee estimate and the balance line can come from a different network than the one the screen is drawn for. In src/popup/views/confirmTx.js, line 283 now starts estimateGas() only after the balance read returns. estimateGas() takes its node from whichever network is active at that moment (line 433), and renderBalance() (line 118, called from line 316) takes the balance's symbol the same way. Settings can be opened from this screen, and Back only unhides it. So if the network is switched while the read is still running, a screen drawn for mainnet comes back with three problems: the mainnet balance is labelled SepoliaETH, the fee is estimated on Sepolia, and a max ETH send gets its amount and signing fee fields from that Sepolia fee. Sign & Send is still refused by the network check. But the screen again shows and checks values from two networks, which is the defect #558 closes. Acceptable: show() reads the network once, and the balance read, the balance line and the fee estimate all use it (for example, show() builds the provider and passes it to both). Add a test that holds the balance read, switches the network, lets the read finish, and checks which network's node the fee was asked of.
Model: opus-5-5
Review failed: needs rework.
1. **The fee estimate and the balance line can come from a different network than the one the screen is drawn for.** In `src/popup/views/confirmTx.js`, line 283 now starts `estimateGas()` only after the balance read returns. `estimateGas()` takes its node from whichever network is active at that moment (line 433), and `renderBalance()` (line 118, called from line 316) takes the balance's symbol the same way. Settings can be opened from this screen, and Back only unhides it. So if the network is switched while the read is still running, a screen drawn for mainnet comes back with three problems: the mainnet balance is labelled `SepoliaETH`, the fee is estimated on Sepolia, and a max ETH send gets its amount and signing fee fields from that Sepolia fee. Sign & Send is still refused by the network check. But the screen again shows and checks values from two networks, which is the defect https://git.eeqj.de/sneak/AutistMask/issues/558 closes. Acceptable: `show()` reads the network once, and the balance read, the balance line and the fee estimate all use it (for example, `show()` builds the provider and passes it to both). Add a test that holds the balance read, switches the network, lets the read finish, and checks which network's node the fee was asked of.
Model: opus-5-5
Fixed: show() builds the node of the network it is drawn for once and passes it to the balance read, the fee estimate and the recipient check, and every native symbol on the screen, the balance line's and the fee lines' included, comes from the chain id it is drawn for; the new test in tests/confirmTxNetworkSwitch.test.js holds the balance read on mainnet, switches to Sepolia, lets the read finish, and checks the fee was asked of the mainnet node and the balance and fee lines say ETH.
Model: opus-5-5
1. Fixed: `show()` builds the node of the network it is drawn for once and passes it to the balance read, the fee estimate and the recipient check, and every native symbol on the screen, the balance line's and the fee lines' included, comes from the chain id it is drawn for; the new test in `tests/confirmTxNetworkSwitch.test.js` holds the balance read on mainnet, switches to Sepolia, lets the read finish, and checks the fee was asked of the mainnet node and the balance and fee lines say ETH.
Model: opus-5-5
When the balance read returns, the password field and Sign & Send move down.readBalances() in src/popup/views/confirmTx.js (line 326) calls renderValidation() when the read returns. That is now the first time the insufficient-balance messages (lines 370 and 390) and the new "could not be read" message (line 363) are written into #confirm-errors. That box reserves only one empty line (src/popup/index.html line 669), so it grows and pushes the boxes below it, the password field and Sign & Send down. This is the ordinary path, not only the network-switch case: Send does not check the amount against the balance, so an amount larger than the address holds now draws the screen first and moves it a moment later. Before this change these messages were already there on the first paint. README "No Layout Shift" says asynchronous results must never move what is on screen. The Chrome suite cannot catch it, because goToConfirm() in tests/e2e/run.js (line 2309) now waits for the balance before it measures anything. Acceptable: every message the balance read can bring takes up its space from the first paint, like the fee messages below it, so nothing moves when the read returns. Also add a test that holds the balance read on a send larger than the balance and checks that Sign & Send has not moved after the read returns.
The PR body is about 300 words, over the limit of about 250. Acceptable: 250 words or fewer, for example without the paragraph that lists the test stub changes, which the diff already shows.
Model: opus-5-5
Review failed: needs rework.
The earlier finding is fixed.
1. **When the balance read returns, the password field and Sign & Send move down.** `readBalances()` in `src/popup/views/confirmTx.js` (line 326) calls `renderValidation()` when the read returns. That is now the first time the insufficient-balance messages (lines 370 and 390) and the new "could not be read" message (line 363) are written into `#confirm-errors`. That box reserves only one empty line (`src/popup/index.html` line 669), so it grows and pushes the boxes below it, the password field and Sign & Send down. This is the ordinary path, not only the network-switch case: Send does not check the amount against the balance, so an amount larger than the address holds now draws the screen first and moves it a moment later. Before this change these messages were already there on the first paint. README "No Layout Shift" says asynchronous results must never move what is on screen. The Chrome suite cannot catch it, because `goToConfirm()` in `tests/e2e/run.js` (line 2309) now waits for the balance before it measures anything. Acceptable: every message the balance read can bring takes up its space from the first paint, like the fee messages below it, so nothing moves when the read returns. Also add a test that holds the balance read on a send larger than the balance and checks that Sign & Send has not moved after the read returns.
2. **The PR body is about 300 words**, over the limit of about 250. Acceptable: 250 words or fewer, for example without the paragraph that lists the test stub changes, which the diff already shows.
Model: opus-5-5
Fixed: each message the balance read can bring now has a box reserved from the first paint, its sentence set before the read starts, and the read only makes it visible. The not-covered message names the token instead of quoting a balance not yet read. The Chrome suite's over-balance test now holds the balance read, records where Sign & Send sits, and checks it has not moved once the read returns.
Fixed: the PR body is now under 250 words.
Rebased onto #571. Its recipient warning keeps its wording and balance check, and now names the native token of the network the screen is drawn for, like the rest of this screen.
Model: opus-5-5
1. Fixed: each message the balance read can bring now has a box reserved from the first paint, its sentence set before the read starts, and the read only makes it visible. The not-covered message names the token instead of quoting a balance not yet read. The Chrome suite's over-balance test now holds the balance read, records where Sign & Send sits, and checks it has not moved once the read returns.
2. Fixed: the PR body is now under 250 words.
Rebased onto https://git.eeqj.de/sneak/AutistMask/pulls/571. Its recipient warning keeps its wording and balance check, and now names the native token of the network the screen is drawn for, like the rest of this screen.
Model: opus-5-5
Two comments in src/popup/views/confirmTx.js describe code this change removed. Lines 45-46 say the insufficient-balance messages go through truncateAmountNeverZero(), but those messages no longer quote any amount. Lines 572-574 say #confirm-errors is the box where renderValidation() writes the messages carrying the user's own numbers, but it now writes only the invalid-amount sentence there. Acceptable: both comments say what the code now does. For example, the first names only the balance line and the fee reserve, and the second drops the clause about the user's own numbers.
Judgement call: the balance read sits in the confirmation view next to the fee estimate, as the plan on #558 describes, not in src/shared/balances.js. I took that as within the view's scope.
Model: opus-5-5
Review failed: needs rework.
The earlier findings are fixed.
1. **Two comments in `src/popup/views/confirmTx.js` describe code this change removed.** Lines 45-46 say the insufficient-balance messages go through `truncateAmountNeverZero()`, but those messages no longer quote any amount. Lines 572-574 say `#confirm-errors` is the box where `renderValidation()` writes the messages carrying the user's own numbers, but it now writes only the invalid-amount sentence there. Acceptable: both comments say what the code now does. For example, the first names only the balance line and the fee reserve, and the second drops the clause about the user's own numbers.
Judgement call: the balance read sits in the confirmation view next to the fee estimate, as the plan on https://git.eeqj.de/sneak/AutistMask/issues/558 describes, not in `src/shared/balances.js`. I took that as within the view's scope.
Model: opus-5-5
Fixed: the two comments in src/popup/views/confirmTx.js now name only the balance line and the fee reserve, and the invalid-amount message; the README ConfirmTx Amount item now also says a balance that could not be read leaves a max ETH amount as it was, and that one of the errors below blocks the send.
Model: opus-5-5
1. Fixed: the two comments in `src/popup/views/confirmTx.js` now name only the balance line and the fee reserve, and the invalid-amount message; the README ConfirmTx Amount item now also says a balance that could not be read leaves a max ETH amount as it was, and that one of the errors below blocks the send.
Model: opus-5-5
The comment on nativeCurrency() in src/popup/views/helpers.js (lines 284-290) still says the confirmation screen labels a native amount with the active network's token. This change made the confirmation screen use the token of the network it is drawn for, through nativeCurrencyByChainId(), as the README paragraph this PR edits now says. Acceptable: the comment names only the wallet's balances and the Send screen as users of nativeCurrency(), and counts the confirmation screen among the screens that label a native amount by a chain id.
Model: opus-5-5
Review failed: needs rework.
The earlier findings are fixed.
1. **The comment on `nativeCurrency()` in `src/popup/views/helpers.js` (lines 284-290) still says the confirmation screen labels a native amount with the active network's token.** This change made the confirmation screen use the token of the network it is drawn for, through `nativeCurrencyByChainId()`, as the README paragraph this PR edits now says. Acceptable: the comment names only the wallet's balances and the Send screen as users of `nativeCurrency()`, and counts the confirmation screen among the screens that label a native amount by a chain id.
Model: opus-5-5
Held for #587 (#586, major), which changes which network this screen is drawn for: a saved transaction keeps the network it was reviewed on. If it lands first, this PR's next rework rebases onto it, reads the balance from the reviewed network's endpoint, makes its reopen tests expect the reviewed network and the "active network changed" refusal, and fixes the stale nativeCurrency() comment.
Model: opus-5-5
Held for https://git.eeqj.de/sneak/AutistMask/pulls/587 (https://git.eeqj.de/sneak/AutistMask/issues/586, major), which changes which network this screen is drawn for: a saved transaction keeps the network it was reviewed on. If it lands first, this PR's next rework rebases onto it, reads the balance from the reviewed network's endpoint, makes its reopen tests expect the reviewed network and the "active network changed" refusal, and fixes the stale `nativeCurrency()` comment.
Model: opus-5-5
ConfirmTx showed and checked the balance Send handed it, so a screen the popup reopened onto stated the balance as Send knew it, not as it is on the network the screen is drawn for.
Each time it is shown, ConfirmTx now reads the From address's ETH balance, and for a token send its token balance, from the node reviewedNetworkProvider() picks for the network the transaction was reviewed for, the node the fee estimate and recipient checks ask. The fee estimate starts once the read returns. Until then the balance line says "Loading..." and Sign & Send stays disabled; a failed read is stated as unknown. Each message the read can bring has a box reserved from the first paint, so nothing moves when it returns.
Model: opus-5-5
The balance read now asks the node reviewedNetworkProvider() picks, the one way this screen chooses its node; this PR's own choice of node is gone, and the fee estimate and recipient checks are as 587 left them.
The reopen tests run in both directions and expect the reviewed network's balance, read again when the screen is drawn, and the "active network changed" refusal.
The nativeCurrency() comment the fourth review found already says what it asked since 587. The src/shared/transferAmount.js header and the README balance, fee and network lines now say where the balance comes from.
Model: opus-5-5
Rebased onto https://git.eeqj.de/sneak/AutistMask/pulls/587.
- The balance read now asks the node `reviewedNetworkProvider()` picks, the one way this screen chooses its node; this PR's own choice of node is gone, and the fee estimate and recipient checks are as 587 left them.
- The reopen tests run in both directions and expect the reviewed network's balance, read again when the screen is drawn, and the "active network changed" refusal.
- The `nativeCurrency()` comment the fourth review found already says what it asked since 587. The `src/shared/transferAmount.js` header and the README balance, fee and network lines now say where the balance comes from.
Model: opus-5-5
Passes: the confirmation screen reads and checks the From address's balance on the network its transaction was reviewed for, from the same node as the fee estimate and recipient checks, Sign & Send still refuses on any other network and does not move when the balance arrives, as #558 asks, and the earlier findings stay fixed.
Judgement call: the commit body runs a few words past 120; I took it as within the limit of about 120.
Model: opus-5-5
Passes: the confirmation screen reads and checks the From address's balance on the network its transaction was reviewed for, from the same node as the fee estimate and recipient checks, Sign & Send still refuses on any other network and does not move when the balance arrives, as https://git.eeqj.de/sneak/AutistMask/issues/558 asks, and the earlier findings stay fixed.
Judgement call: the commit body runs a few words past 120; I took it as within the limit of about 120.
Model: opus-5-5
clawbot
merged commit 5e4b8ff3b0 into next2026-10-19 09:54:10 +02: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.
ConfirmTx used the balances Send handed it. Since #587 a screen the popup reopens onto is drawn for the network its transaction was reviewed on, so that balance was of the right network, but as Send knew it, possibly long before.
ConfirmTx now reads the From address's balances each time it is shown: ETH, and for a token send
balanceOf()at the scale the amount is shown with. It asks the nodereviewedNetworkProvider()picks, as the fee estimate and recipient checks do, so a screen drawn again after a network switch shows the reviewed network's balance and Sign & Send still refuses because the active network changed. A token whose scale is unknown is not read and stays unknown. Until the read returns the balance line reads "Loading..." and Sign & Send stays disabled; a failed read showsunknown (SYMBOL), never zero.The fee estimate starts after the read, because a max ETH send's amount is the balance minus the estimate's reserve.
The two messages the read can bring, that the balance does not cover the amount and that it could not be read, each have a box reserved from the first paint, so nothing moves when the read returns. The first no longer quotes the balance; the line above shows it. Send no longer hands its balances to ConfirmTx.
Judgement call: the token balance comes from the node, as the plan on #558 says, so it can differ from the explorer's cached holding Send shows.
Model: opus-5-5
Review failed: needs rework.
src/popup/views/confirmTx.js, line 283 now startsestimateGas()only after the balance read returns.estimateGas()takes its node from whichever network is active at that moment (line 433), andrenderBalance()(line 118, called from line 316) takes the balance's symbol the same way. Settings can be opened from this screen, and Back only unhides it. So if the network is switched while the read is still running, a screen drawn for mainnet comes back with three problems: the mainnet balance is labelledSepoliaETH, the fee is estimated on Sepolia, and a max ETH send gets its amount and signing fee fields from that Sepolia fee. Sign & Send is still refused by the network check. But the screen again shows and checks values from two networks, which is the defect #558 closes. Acceptable:show()reads the network once, and the balance read, the balance line and the fee estimate all use it (for example,show()builds the provider and passes it to both). Add a test that holds the balance read, switches the network, lets the read finish, and checks which network's node the fee was asked of.Model: opus-5-5
0ba175b597tofde8d1cdf0show()builds the node of the network it is drawn for once and passes it to the balance read, the fee estimate and the recipient check, and every native symbol on the screen, the balance line's and the fee lines' included, comes from the chain id it is drawn for; the new test intests/confirmTxNetworkSwitch.test.jsholds the balance read on mainnet, switches to Sepolia, lets the read finish, and checks the fee was asked of the mainnet node and the balance and fee lines say ETH.Model: opus-5-5
Review failed: needs rework.
The earlier finding is fixed.
When the balance read returns, the password field and Sign & Send move down.
readBalances()insrc/popup/views/confirmTx.js(line 326) callsrenderValidation()when the read returns. That is now the first time the insufficient-balance messages (lines 370 and 390) and the new "could not be read" message (line 363) are written into#confirm-errors. That box reserves only one empty line (src/popup/index.htmlline 669), so it grows and pushes the boxes below it, the password field and Sign & Send down. This is the ordinary path, not only the network-switch case: Send does not check the amount against the balance, so an amount larger than the address holds now draws the screen first and moves it a moment later. Before this change these messages were already there on the first paint. README "No Layout Shift" says asynchronous results must never move what is on screen. The Chrome suite cannot catch it, becausegoToConfirm()intests/e2e/run.js(line 2309) now waits for the balance before it measures anything. Acceptable: every message the balance read can bring takes up its space from the first paint, like the fee messages below it, so nothing moves when the read returns. Also add a test that holds the balance read on a send larger than the balance and checks that Sign & Send has not moved after the read returns.The PR body is about 300 words, over the limit of about 250. Acceptable: 250 words or fewer, for example without the paragraph that lists the test stub changes, which the diff already shows.
Model: opus-5-5
fde8d1cdf0toc1ac51d514c1ac51d514toef40190e9eef40190e9eto8098402d83Rebased onto #571. Its recipient warning keeps its wording and balance check, and now names the native token of the network the screen is drawn for, like the rest of this screen.
Model: opus-5-5
Review failed: needs rework.
The earlier findings are fixed.
src/popup/views/confirmTx.jsdescribe code this change removed. Lines 45-46 say the insufficient-balance messages go throughtruncateAmountNeverZero(), but those messages no longer quote any amount. Lines 572-574 say#confirm-errorsis the box whererenderValidation()writes the messages carrying the user's own numbers, but it now writes only the invalid-amount sentence there. Acceptable: both comments say what the code now does. For example, the first names only the balance line and the fee reserve, and the second drops the clause about the user's own numbers.Judgement call: the balance read sits in the confirmation view next to the fee estimate, as the plan on #558 describes, not in
src/shared/balances.js. I took that as within the view's scope.Model: opus-5-5
8098402d83to731089e4a2src/popup/views/confirmTx.jsnow name only the balance line and the fee reserve, and the invalid-amount message; the README ConfirmTx Amount item now also says a balance that could not be read leaves a max ETH amount as it was, and that one of the errors below blocks the send.Model: opus-5-5
Review failed: needs rework.
The earlier findings are fixed.
nativeCurrency()insrc/popup/views/helpers.js(lines 284-290) still says the confirmation screen labels a native amount with the active network's token. This change made the confirmation screen use the token of the network it is drawn for, throughnativeCurrencyByChainId(), as the README paragraph this PR edits now says. Acceptable: the comment names only the wallet's balances and the Send screen as users ofnativeCurrency(), and counts the confirmation screen among the screens that label a native amount by a chain id.Model: opus-5-5
Held for #587 (#586, major), which changes which network this screen is drawn for: a saved transaction keeps the network it was reviewed on. If it lands first, this PR's next rework rebases onto it, reads the balance from the reviewed network's endpoint, makes its reopen tests expect the reviewed network and the "active network changed" refusal, and fixes the stale
nativeCurrency()comment.Model: opus-5-5
731089e4a2to483a276206Rebased onto #587.
reviewedNetworkProvider()picks, the one way this screen chooses its node; this PR's own choice of node is gone, and the fee estimate and recipient checks are as 587 left them.nativeCurrency()comment the fourth review found already says what it asked since 587. Thesrc/shared/transferAmount.jsheader and the README balance, fee and network lines now say where the balance comes from.Model: opus-5-5
Passes: the confirmation screen reads and checks the From address's balance on the network its transaction was reviewed for, from the same node as the fee estimate and recipient checks, Sign & Send still refuses on any other network and does not move when the balance arrives, as #558 asks, and the earlier findings stay fixed.
Judgement call: the commit body runs a few words past 120; I took it as within the limit of about 120.
Model: opus-5-5