ConfirmTx used the balances Send handed it, so a popup reopened onto a confirmation saved before a network switch in another popup showed the old network's balance and checked the amount against it.
ConfirmTx now reads the From address's balances each time it is shown, from the node of the network it is drawn for: ETH, and for a token send balanceOf() at the scale the amount is shown with. 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.
show() builds that node once and passes it to the balance read, the fee estimate and the recipient check, so switching the network in Settings mid-read changes none of them. 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.
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, so a popup reopened onto a confirmation saved before a network switch in another popup showed the old network's balance and checked the amount against it.
ConfirmTx now reads the From address's balances each time it is shown, from the node of the network it is drawn for: ETH, and for a token send `balanceOf()` at the scale the amount is shown with. 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.
`show()` builds that node once and passes it to the balance read, the fee estimate and the recipient check, so switching the network in Settings mid-read changes none of them. 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.
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
ConfirmTx showed and checked the balance Send handed it, so a popup reopened onto a confirmation saved before a network switch stated the other network's balance and checked the amount against it.
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 of the network it is drawn for, and gives that node to the fee estimate, which starts once the read is done. 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
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
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, so a popup reopened onto a confirmation saved before a network switch in another popup showed the old network's balance and checked the amount against it.
ConfirmTx now reads the From address's balances each time it is shown, from the node of the network it is drawn for: ETH, and for a token send
balanceOf()at the scale the amount is shown with. 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.show()builds that node once and passes it to the balance read, the fee estimate and the recipient check, so switching the network in Settings mid-read changes none of them. 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.
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
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.