fix: count the network fee in the confirm-screen balance check (closes #154) #197

clawbot · 2026-08-11T14:21:06+02:00

clawbot commented

2026-08-11 14:21:06 +02:00

Closes #154.

The arithmetic now enforced

Extracted into src/shared/txValidation.js as pure functions
(feeReserveWei, feeEstimateWei, validateTransfer) over 18-decimal fixed
point (BigInt, via ethers parseUnits) — no floats, no DOM, no network, so
they are unit tested directly rather than through the confirmation screen.

Native ETH: amount + feeReserve <= balance. When the amount alone
already exceeds the balance the existing "Insufficient balance." message is
shown; when only the fee tips it over, a distinct message is.
ERC-20: the token amount is checked against the token balance as
before, and the ETH balance must separately cover feeReserve. That
shortfall is its own error, so a user with plenty of tokens and no ETH is
told exactly what is missing. The fee is never charged against the token
balance — sending the full token balance stays valid.

Scaling both sides of a comparison to 18 decimals is exact and independent of
a token's own decimals, because the amount and the token balance both arrive
as human decimal strings. An amount carrying more precision than that, one
that is not a number at all, or a negative one, is treated as unusable and
blocks sending, rather than silently comparing as NaN or passing every
comparison trivially.

Note that txInfo.balance is the display balance, truncated to 6 decimals by
formatBalance(), so it understates the true wei balance by at most 1e-6 ETH.
The error is in the conservative direction: the check can only refuse a send
that would have just barely fit, never allow one that does not.

The fee reserved is the fee the node charges

The gate reserves gasLimit * (maxFeePerGas ?? gasPrice), not
gasLimit * gasPrice.

The send pins no fee fields, so ethers populates a type-2 transaction with
maxFeePerGas = baseFeePerGas * 2 + maxPriorityFeePerGas, and a node
validates that against value + gasLimit * maxFeePerGas. Since eth_gasPrice
is roughly baseFee + tip, gating on gasPrice under-reserves by about
gasLimit * baseFee — on mainnet roughly the whole fee again — and lets
through a send the node then rejects with
insufficient funds for gas * price + value. That is the exact failure this
issue exists to eliminate, so gating on it would have left the issue unfixed.

Route taken: gate on the reserve, do not pin fee fields. Pinning was the
other option and was rejected deliberately. The estimate is taken when the
screen opens and the user may sit on the password field for minutes; a pinned
maxFeePerGas from that moment can be below the base fee by the time Sign &
Send is pressed, turning a fee that is merely estimated stale today into a
transaction that is rejected or stuck. Pinning also changes the artifact that
gets signed, which the approval/verification path would then have to expect.
Gating on maxFeePerGas while leaving the broadcast untouched keeps the
signed transaction exactly what it was.

What this does and does not guarantee. The reserve is not an absolute
upper bound. It is computed from the feeData fetched when the screen opens,
and the broadcast re-fetches feeData and derives a fresh
maxFeePerGas = baseFee * 2 + tip. If the base fee roughly doubles while the
user is on the password field — about six full blocks, roughly 72 seconds —
the node will require more than was reserved and the send can still fail the
funds check. What the change buys is the difference between a deterministic
failure on every max-value send and a rare one that needs the base fee to
move sharply inside the signing window. An earlier revision of this description
claimed a transaction clearing the gate could not fail the funds check; that
was wrong and is corrected here.

feeReserveWei() falls back to gasPrice only for a network with no type-2
pricing at all, and returns null when the provider offers neither — which
estimateGas() turns into the existing "Unable to estimate" path rather than
into a free transaction.

The fee line shows both numbers

The gasPrice-based fee display predates this PR and was not raised as a
defect, but leaving it alone would have made the screen contradict itself: it
would show 0.000441 ETH while refusing a send whose arithmetic works out
against that number. Replacing it with the reserve alone removes the
contradiction but quotes mainnet users roughly double what they will typically
pay, on every send.

Both are shown instead, under the label "Network fee":

Network fee
~0.000441 ETH ($1.28)
up to 0.000861 ETH reserved

The first line is feeEstimateWei() — gasLimit * gasPrice, what the
transfer is expected to cost, carrying the USD figure. The second is
feeReserveWei() — gasLimit * maxFeePerGas, what is held back and what the
gate uses. On a network with no type-2 pricing the two are the same number, so
the second line is omitted and only the single exact figure is shown. Nothing
gates on feeEstimateWei(); it is display only.

No layout shift. The reserve line is a static element in index.html that
holds its own line of space from the first paint under visibility: hidden,
exactly like the reserved warning boxes; the estimate landing only flips
visibility and swaps text. Neither line can wrap: #app is p-2 pr-5
inside a 396px body, so a text-xs line has about 368px, and the longest string
either line can produce (up to 1000.123456 ETH reserved, 30 characters, or
~0.000441 ETH ($1,234.56), 25) is well inside that. The confirm screen is one
line taller than before, from the first paint — a fixed height, not a shift.

README.md and docs/README.md are updated to match.

Fails closed on an unusable input

validateTransfer() previously returned {canSend: true, codes: []} — no fee
counted at all, on a full-balance send — for three fee inputs, and cleared a
negative amount outright. Each errs in the direction that lets money out. All
now block:

{feeStatus: FEE_KNOWN, feeWei: null}
{feeStatus: FEE_KNOWN, feeWei: 420000000000000} (a number, not a bigint)
{feeStatus: "bogus"} (any unrecognised status)
a negative feeWei
a negative amount, on both the ETH and the ERC-20 path

Nothing outside the three constants is a recognised status, nothing but a
non-negative bigint is a usable fee, and nothing but a non-negative decimal is
a usable amount; anything else is unavailable or invalid, never a fee of zero
and never an amount that passes every comparison. The guard is in the module
rather than in its one caller, because the module is what carries the
documented contract.

The negative-amount case is the same defect class as the fee inputs, on the
other argument to the same guard. toFixedPoint("-1") yields
-1000000000000000000n, a perfectly valid bigint, so AMOUNT_INVALID did not
fire and both > comparisons were trivially false. Downstream,
parseEther("-1") reaches sendTransaction({value: -1e18n}) and dies at
encode time — a confirmation screen affirmatively clearing a transaction that
then fails at broadcast, which is what
#154 exists to eliminate.
Latent today, because src/popup/views/send.js:198 rejects
parseFloat(amount) <= 0 before ConfirmTx is reached.

Pending and failed estimates

Both were explicitly left open in the issue. The choices made:

Pending (the estimate is in flight): Send is disabled and no error
is shown. The fee line already reads "Estimating...", which is the whole
signal the user needs; showing an error for a fee we have simply not
received yet would be false. A known-bad amount (over the balance on its
own) is still reported immediately, without waiting. Validation re-runs when
the estimate resolves, at which point Send enables if the transaction is
fundable.
Failed ("Unable to estimate"): Send stays disabled, with the message
"The network fee could not be estimated, so this transaction cannot be
checked against your balance. Please go back and try again." Unknown is
never treated as zero. The rationale: this wallet's premise is that the
confirmation screen verifies everything before signing, and without a fee we
cannot verify the transaction is fundable. It also costs the user almost
nothing — the broadcast path calls the same eth_estimateGas on the same
endpoint, so a transaction whose estimate failed here would in nearly every
case fail at broadcast anyway. Recovery is Back, retry, or a different RPC
endpoint in Settings.

A late estimate belonging to a transaction the user has already left is
discarded (pendingTx !== txInfo) instead of being written to the screen and
into the balance check.

Test evidence

tests/txValidation.test.js, 36 cases across the ETH path, the ERC-20 path,
the fee reserve, the fee estimate and the fail-closed contract.

Negative amount, failing first. The two new cases were written against the
previous head and run before the guard was added, with make test:

  ● validateTransfer, native ETH › rejects a negative amount
  ● validateTransfer, ERC-20 › rejects a negative token amount

    - Expected  - 4
    + Received  + 2

      Object {
    -   "canSend": false,
    -   "codes": Array [
    -     "amount-invalid",
    -   ],
    +   "canSend": true,
    +   "codes": Array [],
      }

Tests:       2 failed, 178 passed, 180 total

Both are green after the guard.

The type-2 reserve is pinned by two cases. feeReserveWei(21000n, {gasPrice: 21 gwei, maxFeePerGas: 41 gwei}) must be 861000000000000n and must not be
441000000000000n; and a 0.999559 ETH send against a 1.0 ETH balance, which
is fundable to the wei under the gasPrice reserve, must be blocked with
INSUFFICIENT_ETH_WITH_FEE under the reserve the node will require. Four more
pin feeEstimateWei() as the other number: below the reserve on a type-2
network, equal to it where there is no type-2 pricing, and null on the same
unusable inputs.

Mutation-tested against the rebased head, one mutant at a time, each
reverted after and the tree confirmed clean at ef82c62:

mutant	result
fee dropped from the ETH comparison	3 failed — killed
ERC-20 ETH-for-gas check removed	2 failed — killed
`>` relaxed to `>=` at the ETH+fee boundary	3 failed — killed
`FEE_UNAVAILABLE` treated as zero	7 failed — killed
fee charged against the token balance	1 failed — killed
fee reserve back on the `gasPrice` basis	3 failed — killed
negative-amount guard removed	2 failed — killed

The first six are the mutants confirmed on the previous head; all six still
die. The seventh is new and covers the negative-amount guard.

`make check`

Re-run after the rebase onto next at
12acf4d, resolving the
TODO.md conflict by keeping every side's entries:

Test Suites: 11 passed, 11 total
Tests:       1 skipped, 286 passed, 287 total
Time:        4.667 s
Linting...
$ prettier --check .
All matched files use Prettier code style!
Checking formatting...
$ prettier --check .
All matched files use Prettier code style!

The one skipped test is pre-existing and not in this branch's files.

make test-e2e also run in the pinned container (README asks for it on any
change under src/popup/): 4/4 passed. It does not reach the confirmation
screen, so it proves the popup and its bundles still load clean, not the new
behaviour.

1..4
ok 1 - popup loads and reaches the welcome view
ok 2 - wallet creation through the UI reaches the main view
ok 3 - add token screen opens from address detail (#150)
ok 4 - transaction detail renders an ERC-20 transfer (#151)
# 4/4 tests passed

Out of scope

"Send max" is not implemented. It did not fall out of this change: it
needs the exact wei balance rather than the 6-decimal display string, and it
has to re-derive the amount every time the estimate moves. Worth its own
issue if wanted.
The confirmation screen has no end-to-end coverage. Verifying the
disabled-button and no-shift behaviour in a real browser would mean
extending tests/e2e/ with a funded-balance fixture and a send flow —
larger than this issue, and not in its Definition of Done. The no-shift
claim above is a reading-and-measurement argument, not a browser
measurement.

Closes [#154](https://git.eeqj.de/sneak/AutistMask/issues/154). ## The arithmetic now enforced Extracted into `src/shared/txValidation.js` as pure functions (`feeReserveWei`, `feeEstimateWei`, `validateTransfer`) over 18-decimal fixed point (BigInt, via ethers `parseUnits`) — no floats, no DOM, no network, so they are unit tested directly rather than through the confirmation screen. - **Native ETH**: `amount + feeReserve <= balance`. When the amount alone already exceeds the balance the existing "Insufficient balance." message is shown; when only the fee tips it over, a distinct message is. - **ERC-20**: the token amount is checked against the token balance as before, and the ETH balance must separately cover `feeReserve`. That shortfall is its own error, so a user with plenty of tokens and no ETH is told exactly what is missing. The fee is never charged against the token balance — sending the full token balance stays valid. Scaling both sides of a comparison to 18 decimals is exact and independent of a token's own decimals, because the amount and the token balance both arrive as human decimal strings. An amount carrying more precision than that, one that is not a number at all, or a negative one, is treated as unusable and blocks sending, rather than silently comparing as `NaN` or passing every comparison trivially. Note that `txInfo.balance` is the display balance, truncated to 6 decimals by `formatBalance()`, so it understates the true wei balance by at most 1e-6 ETH. The error is in the conservative direction: the check can only refuse a send that would have just barely fit, never allow one that does not. ## The fee reserved is the fee the node charges The gate reserves `gasLimit * (maxFeePerGas ?? gasPrice)`, not `gasLimit * gasPrice`. The send pins no fee fields, so ethers populates a **type-2** transaction with `maxFeePerGas = baseFeePerGas * 2 + maxPriorityFeePerGas`, and a node validates that against `value + gasLimit * maxFeePerGas`. Since `eth_gasPrice` is roughly `baseFee + tip`, gating on `gasPrice` under-reserves by about `gasLimit * baseFee` — on mainnet roughly the whole fee again — and lets through a send the node then rejects with `insufficient funds for gas * price + value`. That is the exact failure this issue exists to eliminate, so gating on it would have left the issue unfixed. **Route taken: gate on the reserve, do not pin fee fields.** Pinning was the other option and was rejected deliberately. The estimate is taken when the screen opens and the user may sit on the password field for minutes; a pinned `maxFeePerGas` from that moment can be below the base fee by the time Sign & Send is pressed, turning a fee that is merely *estimated* stale today into a transaction that is rejected or stuck. Pinning also changes the artifact that gets signed, which the approval/verification path would then have to expect. Gating on `maxFeePerGas` while leaving the broadcast untouched keeps the signed transaction exactly what it was. **What this does and does not guarantee.** The reserve is *not* an absolute upper bound. It is computed from the `feeData` fetched when the screen opens, and the broadcast re-fetches `feeData` and derives a fresh `maxFeePerGas = baseFee * 2 + tip`. If the base fee roughly doubles while the user is on the password field — about six full blocks, roughly 72 seconds — the node will require more than was reserved and the send can still fail the funds check. What the change buys is the difference between a **deterministic** failure on every max-value send and a **rare** one that needs the base fee to move sharply inside the signing window. An earlier revision of this description claimed a transaction clearing the gate could not fail the funds check; that was wrong and is corrected here. `feeReserveWei()` falls back to `gasPrice` only for a network with no type-2 pricing at all, and returns `null` when the provider offers neither — which `estimateGas()` turns into the existing "Unable to estimate" path rather than into a free transaction. ## The fee line shows both numbers The `gasPrice`-based fee display predates this PR and was not raised as a defect, but leaving it alone would have made the screen contradict itself: it would show 0.000441 ETH while refusing a send whose arithmetic works out against that number. Replacing it with the reserve alone removes the contradiction but quotes mainnet users roughly double what they will typically pay, on every send. Both are shown instead, under the label **"Network fee"**: ``` Network fee ~0.000441 ETH ($1.28) up to 0.000861 ETH reserved ``` The first line is `feeEstimateWei()` — `gasLimit * gasPrice`, what the transfer is expected to cost, carrying the USD figure. The second is `feeReserveWei()` — `gasLimit * maxFeePerGas`, what is held back and what the gate uses. On a network with no type-2 pricing the two are the same number, so the second line is omitted and only the single exact figure is shown. Nothing gates on `feeEstimateWei()`; it is display only. **No layout shift.** The reserve line is a static element in `index.html` that holds its own line of space from the first paint under `visibility: hidden`, exactly like the reserved warning boxes; the estimate landing only flips `visibility` and swaps text. Neither line can wrap: `#app` is `p-2 pr-5` inside a 396px body, so a text-xs line has about 368px, and the longest string either line can produce (`up to 1000.123456 ETH reserved`, 30 characters, or `~0.000441 ETH ($1,234.56)`, 25) is well inside that. The confirm screen is one line taller than before, from the first paint — a fixed height, not a shift. `README.md` and `docs/README.md` are updated to match. ## Fails closed on an unusable input `validateTransfer()` previously returned `{canSend: true, codes: []}` — no fee counted at all, on a full-balance send — for three fee inputs, and cleared a negative amount outright. Each errs in the direction that lets money out. All now block: - `{feeStatus: FEE_KNOWN, feeWei: null}` - `{feeStatus: FEE_KNOWN, feeWei: 420000000000000}` (a number, not a bigint) - `{feeStatus: "bogus"}` (any unrecognised status) - a negative `feeWei` - **a negative `amount`**, on both the ETH and the ERC-20 path Nothing outside the three constants is a recognised status, nothing but a non-negative bigint is a usable fee, and nothing but a non-negative decimal is a usable amount; anything else is unavailable or invalid, never a fee of zero and never an amount that passes every comparison. The guard is in the module rather than in its one caller, because the module is what carries the documented contract. The negative-amount case is the same defect class as the fee inputs, on the other argument to the same guard. `toFixedPoint("-1")` yields `-1000000000000000000n`, a perfectly valid bigint, so `AMOUNT_INVALID` did not fire and both `>` comparisons were trivially false. Downstream, `parseEther("-1")` reaches `sendTransaction({value: -1e18n})` and dies at encode time — a confirmation screen affirmatively clearing a transaction that then fails at broadcast, which is what [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) exists to eliminate. Latent today, because `src/popup/views/send.js:198` rejects `parseFloat(amount) <= 0` before ConfirmTx is reached. ## Pending and failed estimates Both were explicitly left open in the issue. The choices made: - **Pending** (the estimate is in flight): Send is **disabled** and *no* error is shown. The fee line already reads "Estimating...", which is the whole signal the user needs; showing an error for a fee we have simply not received yet would be false. A known-bad amount (over the balance on its own) is still reported immediately, without waiting. Validation re-runs when the estimate resolves, at which point Send enables if the transaction is fundable. - **Failed** ("Unable to estimate"): Send stays **disabled**, with the message "The network fee could not be estimated, so this transaction cannot be checked against your balance. Please go back and try again." Unknown is never treated as zero. The rationale: this wallet's premise is that the confirmation screen verifies everything before signing, and without a fee we cannot verify the transaction is fundable. It also costs the user almost nothing — the broadcast path calls the same `eth_estimateGas` on the same endpoint, so a transaction whose estimate failed here would in nearly every case fail at broadcast anyway. Recovery is Back, retry, or a different RPC endpoint in Settings. A late estimate belonging to a transaction the user has already left is discarded (`pendingTx !== txInfo`) instead of being written to the screen and into the balance check. ## Test evidence `tests/txValidation.test.js`, 36 cases across the ETH path, the ERC-20 path, the fee reserve, the fee estimate and the fail-closed contract. **Negative amount, failing first.** The two new cases were written against the previous head and run before the guard was added, with `make test`: ``` ● validateTransfer, native ETH › rejects a negative amount ● validateTransfer, ERC-20 › rejects a negative token amount - Expected - 4 + Received + 2 Object { - "canSend": false, - "codes": Array [ - "amount-invalid", - ], + "canSend": true, + "codes": Array [], } Tests: 2 failed, 178 passed, 180 total ``` Both are green after the guard. The type-2 reserve is pinned by two cases. `feeReserveWei(21000n, {gasPrice: 21 gwei, maxFeePerGas: 41 gwei})` must be `861000000000000n` and must not be `441000000000000n`; and a 0.999559 ETH send against a 1.0 ETH balance, which is fundable to the wei under the `gasPrice` reserve, must be blocked with `INSUFFICIENT_ETH_WITH_FEE` under the reserve the node will require. Four more pin `feeEstimateWei()` as the *other* number: below the reserve on a type-2 network, equal to it where there is no type-2 pricing, and `null` on the same unusable inputs. **Mutation-tested** against the rebased head, one mutant at a time, each reverted after and the tree confirmed clean at `ef82c62`: | mutant | result | | --- | --- | | fee dropped from the ETH comparison | 3 failed — killed | | ERC-20 ETH-for-gas check removed | 2 failed — killed | | `>` relaxed to `>=` at the ETH+fee boundary | 3 failed — killed | | `FEE_UNAVAILABLE` treated as zero | 7 failed — killed | | fee charged against the token balance | 1 failed — killed | | fee reserve back on the `gasPrice` basis | 3 failed — killed | | negative-amount guard removed | 2 failed — killed | The first six are the mutants confirmed on the previous head; all six still die. The seventh is new and covers the negative-amount guard. ## `make check` Re-run after the rebase onto `next` at [`12acf4d`](https://git.eeqj.de/sneak/AutistMask/commit/12acf4d), resolving the `TODO.md` conflict by keeping every side's entries: ``` Test Suites: 11 passed, 11 total Tests: 1 skipped, 286 passed, 287 total Time: 4.667 s Linting... $ prettier --check . All matched files use Prettier code style! Checking formatting... $ prettier --check . All matched files use Prettier code style! ``` The one skipped test is pre-existing and not in this branch's files. `make test-e2e` also run in the pinned container (README asks for it on any change under `src/popup/`): 4/4 passed. It does not reach the confirmation screen, so it proves the popup and its bundles still load clean, not the new behaviour. ``` 1..4 ok 1 - popup loads and reaches the welcome view ok 2 - wallet creation through the UI reaches the main view ok 3 - add token screen opens from address detail (#150) ok 4 - transaction detail renders an ERC-20 transfer (#151) # 4/4 tests passed ``` ## Out of scope - **"Send max"** is not implemented. It did not fall out of this change: it needs the exact wei balance rather than the 6-decimal display string, and it has to re-derive the amount every time the estimate moves. Worth its own issue if wanted. - The confirmation screen has no end-to-end coverage. Verifying the disabled-button and no-shift behaviour in a real browser would mean extending `tests/e2e/` with a funded-balance fixture and a send flow — larger than this issue, and not in its Definition of Done. The no-shift claim above is a reading-and-measurement argument, not a browser measurement.

clawbot added the needs-review label 2026-08-11 14:21:06 +02:00

clawbot added 1 commit 2026-08-11 14:21:06 +02:00

fix: count the network fee in the confirm-screen balance check (closes #154 )

check / check (push) Successful in 29s

Details

c05e69393f

The Send button was enabled whenever the amount alone fit the balance, so
a max-value ETH send passed the confirmation screen and failed at
broadcast, after the user had committed to it.

The arithmetic moves into src/shared/txValidation.js as a pure function
over 18-decimal fixed point: native ETH now requires amount + fee <=
balance, and an ERC-20 transfer requires the ETH balance to cover the fee
on top of the token check, reported as its own error. Validation re-runs
when the async estimate resolves; Send stays disabled while the estimate
is pending and when it fails, so an unknown fee is never treated as zero.
The fee messages are static elements that already reserve their space, so
nothing moves when the estimate lands.

clawbot self-assigned this 2026-08-11 14:21:19 +02:00

clawbot commented

2026-08-11 14:30:28 +02:00

FAIL — needs-rework.

1. The gate is calibrated to gasPrice; the broadcast pays maxFeePerGas. The #154 failure mode is still reachable.

src/popup/views/confirmTx.js:291,307 — the estimate is gasLimit * feeData.gasPrice, and that number is now what validateTransfer() gates Send on. But the broadcast (confirmTx.js:395, connectedSigner.sendTransaction({to, value}), and the ERC-20 contract.transfer() below it) sets no fee fields, so ethers populates a type-2 transaction with maxFeePerGas = feeData.maxFeePerGas, which ethers v6 computes as baseFeePerGas * 2n + maxPriorityFeePerGas (abstract-provider.js getFeeData). A node validates a type-2 transaction against value + gasLimit * maxFeePerGas, not value + gasLimit * gasPrice.

Since gasPrice (from eth_gasPrice) is about baseFee + tip and maxFeePerGas is 2*baseFee + tip, the check under-reserves by gasLimit * baseFee — on mainnet roughly the whole displayed fee over again.

Worked case: balance 1.0 ETH, baseFee 20 gwei, tip 1 gwei. Displayed and validated fee = 21000 * 21 gwei = 0.000441 ETH. The node requires 21000 * 41 gwei = 0.000861 ETH reserved. A user who is shown "Your balance does not cover this amount plus the network fee. Please go back and send a smaller amount", does exactly that, and sends 0.99958, gets an enabled Send button, enters their password, and fails at broadcast with insufficient funds for gas * price + value. That is precisely the outcome #154 exists to eliminate, now arrived at through a confirmation screen that affirmatively cleared the transaction. The ERC-20 ETH-for-gas check has the same under-reservation.

The gasPrice-based fee display predates this PR and is fine as an estimate of what will actually be paid; what is new and wrong is promoting that number to a spending gate. Acceptable: gate on the reserve the node will require — gasLimit * (feeData.maxFeePerGas ?? feeData.gasPrice) — or pin explicit fee fields on the send/transfer call so the broadcast pays exactly the price that was validated. Either way a test should pin the type-2 reserve.

2. validateTransfer() fails open on a malformed known fee, and on an unrecognized feeStatus.

src/shared/txValidation.js:96-97 and 112-113 — feeFp is non-null only when feeStatus === FEE_KNOWN && typeof feeWei === "bigint", and the blocking FEE_PENDING / FEE_UNAVAILABLE codes are pushed only on exact string match. Three inputs therefore count no fee at all and return canSend: true (verified by direct probe against the committed module, full-balance send, 1.0 of 1.0 ETH):

{ feeStatus: FEE_KNOWN, feeWei: null } -> {canSend: true, codes: []}
{ feeStatus: FEE_KNOWN, feeWei: 420000000000000 } (JS number, not bigint) -> {canSend: true, codes: []}
{ feeStatus: "bogus" } -> {canSend: true, codes: []}

Each is the module doing the one thing its own comment forbids — "An unknown fee is never assumed to be zero" — and every one of them fails in the direction that lets money out. Not reachable from today's single caller (gasLimit * gasPrice is always a bigint), so this is latent, but this is an exported pure module with a documented contract sitting on a spend gate, and the guard that makes it safe is in the caller rather than here. Acceptable: an unusable feeWei under FEE_KNOWN, and any feeStatus outside the three constants, must be treated as FEE_UNAVAILABLE and block, with a test for each.

3. Conflicts with current next.

next has moved to 19cb1ca since this branch was cut from d93eda3. git merge-tree origin/next HEAD reports CONFLICT (content): Merge conflict in TODO.md (competing entries at the head of Completed Steps). Gitea's mergeable: true is stale. Rebase.

Judgement calls and disclosure, not defects:

The issue asked for "a distinct error naming the gas shortfall". The ERC-20 message names gas as the missing thing but carries no number. Read as "names gas as what is short" it is satisfied, and the fixed-height reserved box justifies a static sentence. Accepting it; flagging that I read the requirement the permissive way.
Confirm-screen disabled-state and no-shift behaviour is not verified in a browser (e2e does not reach ConfirmTx). Checked by reading instead: the only variable-height element, confirm-errors, is driven solely by AMOUNT_INVALID / INSUFFICIENT_TOKEN / INSUFFICIENT_ETH, none of which depend on feeStatus or feeWei, so its content is identical on both renderValidation() calls and the estimate landing cannot move anything. The three new elements hold constant static text under visibility: hidden. No shift.
The escapeHtml() on the way into innerHTML closes a real pre-existing injection: txInfo.tokenSymbol is attacker-supplied (arbitrary ERC-20 symbol) and previously reached innerHTML raw in the insufficient-balance message. Escaping is correct and complete for the element-content sink used. The untouched confirm-warnings block interpolates only static w.message strings.
Mutation-tested: fee dropped from the ETH comparison, ERC-20 ETH-for-gas check removed, > relaxed to >= at the ETH+fee boundary, FEE_UNAVAILABLE treated as zero, and the fee charged against the token balance — all five killed. The 20 new tests have teeth.
Probed and correct: USDC-style 6-decimal token (exact at full balance, blocked one unit over, blocked below token precision), one-wei-under/exact/over on the ETH boundary, zero amount, missing balances treated as zero.
make check run here: 8 suites / 163 tests passed in 3.2s (real run, no cached markers), prettier clean. make test-e2e run here in the pinned container: 4/4. CI on c05e693 is green.
Single commit, title ends (closes #154), base next, one TODO.md line, no attribution trailers, no forbidden references, no scope creep.

FAIL — `needs-rework`. **1. The gate is calibrated to `gasPrice`; the broadcast pays `maxFeePerGas`. The #154 failure mode is still reachable.** `src/popup/views/confirmTx.js:291,307` — the estimate is `gasLimit * feeData.gasPrice`, and that number is now what `validateTransfer()` gates Send on. But the broadcast (`confirmTx.js:395`, `connectedSigner.sendTransaction({to, value})`, and the ERC-20 `contract.transfer()` below it) sets no fee fields, so ethers populates a **type-2** transaction with `maxFeePerGas = feeData.maxFeePerGas`, which ethers v6 computes as `baseFeePerGas * 2n + maxPriorityFeePerGas` (`abstract-provider.js` `getFeeData`). A node validates a type-2 transaction against `value + gasLimit * maxFeePerGas`, not `value + gasLimit * gasPrice`. Since `gasPrice` (from `eth_gasPrice`) is about `baseFee + tip` and `maxFeePerGas` is `2*baseFee + tip`, the check under-reserves by `gasLimit * baseFee` — on mainnet roughly the whole displayed fee over again. Worked case: balance 1.0 ETH, baseFee 20 gwei, tip 1 gwei. Displayed and validated fee = 21000 * 21 gwei = 0.000441 ETH. The node requires 21000 * 41 gwei = 0.000861 ETH reserved. A user who is shown "Your balance does not cover this amount plus the network fee. Please go back and send a smaller amount", does exactly that, and sends 0.99958, gets an enabled Send button, enters their password, and fails at broadcast with `insufficient funds for gas * price + value`. That is precisely the outcome [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) exists to eliminate, now arrived at through a confirmation screen that affirmatively cleared the transaction. The ERC-20 ETH-for-gas check has the same under-reservation. The gasPrice-based fee *display* predates this PR and is fine as an estimate of what will actually be paid; what is new and wrong is promoting that number to a spending gate. Acceptable: gate on the reserve the node will require — `gasLimit * (feeData.maxFeePerGas ?? feeData.gasPrice)` — or pin explicit fee fields on the send/transfer call so the broadcast pays exactly the price that was validated. Either way a test should pin the type-2 reserve. **2. `validateTransfer()` fails open on a malformed known fee, and on an unrecognized `feeStatus`.** `src/shared/txValidation.js:96-97` and `112-113` — `feeFp` is non-null only when `feeStatus === FEE_KNOWN && typeof feeWei === "bigint"`, and the blocking `FEE_PENDING` / `FEE_UNAVAILABLE` codes are pushed only on exact string match. Three inputs therefore count no fee at all and return `canSend: true` (verified by direct probe against the committed module, full-balance send, 1.0 of 1.0 ETH): - `{ feeStatus: FEE_KNOWN, feeWei: null }` -> `{canSend: true, codes: []}` - `{ feeStatus: FEE_KNOWN, feeWei: 420000000000000 }` (JS number, not bigint) -> `{canSend: true, codes: []}` - `{ feeStatus: "bogus" }` -> `{canSend: true, codes: []}` Each is the module doing the one thing its own comment forbids — "An unknown fee is never assumed to be zero" — and every one of them fails in the direction that lets money out. Not reachable from today's single caller (`gasLimit * gasPrice` is always a bigint), so this is latent, but this is an exported pure module with a documented contract sitting on a spend gate, and the guard that makes it safe is in the caller rather than here. Acceptable: an unusable `feeWei` under `FEE_KNOWN`, and any `feeStatus` outside the three constants, must be treated as `FEE_UNAVAILABLE` and block, with a test for each. **3. Conflicts with current `next`.** `next` has moved to `19cb1ca` since this branch was cut from `d93eda3`. `git merge-tree origin/next HEAD` reports `CONFLICT (content): Merge conflict in TODO.md` (competing entries at the head of Completed Steps). Gitea's `mergeable: true` is stale. Rebase. --- Judgement calls and disclosure, not defects: - The issue asked for "a distinct error naming the gas shortfall". The ERC-20 message names gas as the missing thing but carries no number. Read as "names gas as what is short" it is satisfied, and the fixed-height reserved box justifies a static sentence. Accepting it; flagging that I read the requirement the permissive way. - Confirm-screen disabled-state and no-shift behaviour is not verified in a browser (e2e does not reach ConfirmTx). Checked by reading instead: the only variable-height element, `confirm-errors`, is driven solely by `AMOUNT_INVALID` / `INSUFFICIENT_TOKEN` / `INSUFFICIENT_ETH`, none of which depend on `feeStatus` or `feeWei`, so its content is identical on both `renderValidation()` calls and the estimate landing cannot move anything. The three new elements hold constant static text under `visibility: hidden`. No shift. - The `escapeHtml()` on the way into `innerHTML` closes a real pre-existing injection: `txInfo.tokenSymbol` is attacker-supplied (arbitrary ERC-20 symbol) and previously reached `innerHTML` raw in the insufficient-balance message. Escaping is correct and complete for the element-content sink used. The untouched `confirm-warnings` block interpolates only static `w.message` strings. - Mutation-tested: fee dropped from the ETH comparison, ERC-20 ETH-for-gas check removed, `>` relaxed to `>=` at the ETH+fee boundary, `FEE_UNAVAILABLE` treated as zero, and the fee charged against the token balance — all five killed. The 20 new tests have teeth. - Probed and correct: USDC-style 6-decimal token (exact at full balance, blocked one unit over, blocked below token precision), one-wei-under/exact/over on the ETH boundary, zero amount, missing balances treated as zero. - `make check` run here: 8 suites / 163 tests passed in 3.2s (real run, no cached markers), prettier clean. `make test-e2e` run here in the pinned container: 4/4. CI on `c05e693` is green. - Single commit, title ends ` (closes #154)`, base `next`, one TODO.md line, no attribution trailers, no forbidden references, no scope creep.

clawbot added needs-rework and removed needs-review labels 2026-08-11 14:31:46 +02:00

clawbot force-pushed fix/issue-154-gas-in-balance-check from c05e69393f to dc7451a4e2

2026-08-11 14:39:54 +02:00

Compare

clawbot force-pushed fix/issue-154-gas-in-balance-check from dc7451a4e2 to ce81596100

2026-08-11 14:42:35 +02:00

Compare

clawbot added needs-review and removed needs-rework labels 2026-08-11 15:02:24 +02:00

clawbot commented

2026-08-11 15:10:18 +02:00

FAIL — needs-rebase.

1. Not mergeable. TODO.md conflicts with current next.

The branch is based on cf5f582; next has since advanced four commits to 86cdea5, three of which touch TODO.md. git merge-tree --write-tree origin/next HEAD exits 1 with CONFLICT (content): Merge conflict in TODO.md (competing entries at the head of Completed Steps). Gitea agrees — mergeable: false. Rebase onto 86cdea5, keeping every side's entries.

2. A fourth fail-open input: a negative amount returns canSend: true.

src/shared/txValidation.js:99-131. toFixedPoint("-1") yields -1000000000000000000n, which is a perfectly valid bigint, so AMOUNT_INVALID does not fire; amountFp > ethFp is false, and amountFp + feeFp > ethFp is false. Probed directly against the committed module:

validateTransfer({amount: "-1",   ethBalance: "1.0", feeStatus: FEE_KNOWN, feeWei: 861000000000000n})
  => {"canSend":true,"codes":[]}
validateTransfer({isErc20: true, amount: "-0.5", tokenBalance: "1.0", ethBalance: "1.0",
                  feeStatus: FEE_KNOWN, feeWei: 861000000000000n})
  => {"canSend":true,"codes":[]}

This is the same defect class, in the same guard, that this rework exists to close, and it sits on the one argument that was left unhardened. The module's stated principle is "nothing but a non-negative bigint is a usable fee" — the amount gets no non-negativity check at all, and CODES.AMOUNT_INVALID is documented as "the amount is not a number we can do exact arithmetic on", which a negative amount slips past on a technicality. Downstream, parseEther("-1") reaches sendTransaction({value: -1e18n}) and dies at encode time — i.e. a confirmation screen that affirmatively clears a transaction which then fails at broadcast, the exact outcome #154 exists to eliminate.

Latent today: src/popup/views/send.js:198 rejects parseFloat(amount) <= 0 before ConfirmTx is reached. That is precisely the standing of the three fee inputs this PR was reworked to fix — reachable only through the module's own contract, not through today's single caller — so it is reported on the same footing. Acceptable: amountFp < 0n blocks with AMOUNT_INVALID (and the doc comment says so), with a test for the ETH and ERC-20 paths.

3. Minor: two Screen Map claims went stale in README.md as a result of this change.

README.md:576 still reads "Sign & Send" button (disabled if errors). Send is now also disabled while the fee estimate is in flight, deliberately showing no error — so the parenthetical no longer describes the code. README.md:575 still summarises the error area as Errors (insufficient balance) while the four reserved warning boxes above it are enumerated individually; the three new reserved fee-error boxes are not listed. Acceptable: "disabled if errors, and while the network fee estimate is pending or unavailable", and the three fee-error boxes enumerated alongside the warning boxes.

Judgement calls and disclosure, not defects:

The reserve is correct and sufficient. Verified against the pinned ethers ^6.16.0 in node_modules: abstract-signer.js:112 upgrades to type 2 only when feeData.maxFeePerGas and feeData.maxPriorityFeePerGas are both non-null, and then assigns pop.maxFeePerGas = feeData.maxFeePerGas — the same field feeReserveWei() reserves, so gate and node agree exactly. Probed all fallbacks: maxFeePerGas present without maxPriorityFeePerGas (ethers drops to legacy gasPrice, reserve over-reserves — conservative); gasPrice only (legacy, exact); neither, feeData null/undefined (all null, and confirmTx.js:313-315 throws that into the "Unable to estimate" path rather than a free transaction); negative, non-bigint and string prices (null); gasLimit non-bigint or negative (null). No path reserves less than the node requires.
Overstated claim in the PR body, worth correcting rather than fixing in code: "the reserve is an upper bound on what any node will require, and a transaction that clears it cannot fail the funds check" is not true. The reserve is computed from feeData fetched when the screen opens, but the broadcast re-fetches feeData and derives a fresh maxFeePerGas = baseFee * 2 + tip; if the base fee roughly doubles while the user is on the password field, the node requires more than was reserved. The fix is still right and reduces a deterministic failure on every max-value send to a rare one, but it is not the absolute bound the description claims.
maxFeePerGas: 0n does not fall through to gasPrice (?? only catches null/undefined), and gasLimit: 0n yields a reserve of 0n under FEE_KNOWN. Both correct for a genuinely zero-fee network and both unreachable from ethers' getFeeData (baseFeePerGas of 0n is falsy there, so maxFeePerGas stays null). Noting them, not filing them.
The display change is a product decision taken by the implementer, and it is outside the issue's scope and Definition of Done — #154 asks for the balance check, not the fee label. The self-contradicting-screen argument for changing it is legitimate, but a third option was available and not considered: show the estimate and the reserve ("~0.000441 ETH, up to 0.000861 ETH reserved"), which removes the contradiction without quoting mainnet users roughly double what they will actually pay on every send. As shipped, the on-screen label carries no inline explanation — the "typically costs less" caveat exists only in README.md, and the README Language & Labeling rule asks for "helpful inline descriptions where needed". Not failing on it; flagging it for the owner's call.
All three previously fail-open fee inputs now block, verified by direct probe: FEE_KNOWN + feeWei: null, FEE_KNOWN + a JS number, and an unrecognised feeStatus all return {canSend: false, codes: ["fee-unavailable"]}, as do a negative feeWei, feeStatus of null/0/a boxed String, and feeWei of true/{}/NaN/a boxed bigint. feeStatus: undefined correctly falls to FEE_PENDING; no-argument validateTransfer() blocks on amount-invalid.
All six mutants killed, applied one at a time and reverted after, tree confirmed clean at ce81596: fee dropped from the ETH comparison (3 failed); ERC-20 ETH-for-gas check removed (2); > to >= at the ETH+fee boundary (3); FEE_UNAVAILABLE treated as zero (7); fee charged against the token balance (1); reserve back on the gasPrice basis (2 — reserves gasLimit * maxFeePerGas, not gasLimit * gasPrice and gates out a send the type-2 reserve cannot fund). 29 tests, 47 assertions, none vacuous.
Preserved properties confirmed: exact 6-decimal USDC-style arithmetic (full token balance sends, one micro-unit over blocks, fee never charged to the token balance), one-wei boundary on the type-2 reserve in both directions, both pendingTx !== txInfo staleness guards (confirmTx.js:318 and :336), escapeHtml() still on the innerHTML sink at confirmTx.js:255, and no layout shift — the mutually exclusive fee messages are display: none'd in show() before first paint and confirm-fee-unknown-error always reserves its space, so re-validation only flips visibility.
The disclosed txInfo.balance boundary is genuinely conservative in all cases. formatBalance() and formatTokenBalance() (src/shared/balances.js:29-43) both truncate with .slice(0, 6) — no rounding — so the display balance is always less than or equal to the true balance and the gate can only refuse a send that would just barely fit. For tokens with 6 or fewer decimals the display string is exact, so there is no error at all in that direction either. Accepting the boundary.
make check run here: 9 suites / 178 tests passed in 1.9s, zero cached markers, prettier clean, exit 0. make test-e2e run here in the pinned Playwright container: 4/4, exit 0. Tracker CI was not relied on.
Single commit, title ends (closes #154), base next, authored clawbot <clawbot@eeqj.de>, no Claude/Anthropic references, no attribution trailers, no non-inclusive terminology. No stale "Estimated network fee" left anywhere in README.md, docs/README.md or src/.
Disclosure: the fail-open probes and the six mutants were executed with a direct node require of the committed module and with temporary edits reverted via git checkout; the pass/fail verdicts above come from make check, make test and make test-e2e only.

FAIL — `needs-rebase`. **1. Not mergeable. `TODO.md` conflicts with current `next`.** The branch is based on `cf5f582`; `next` has since advanced four commits to `86cdea5`, three of which touch `TODO.md`. `git merge-tree --write-tree origin/next HEAD` exits 1 with `CONFLICT (content): Merge conflict in TODO.md` (competing entries at the head of Completed Steps). Gitea agrees — `mergeable: false`. Rebase onto `86cdea5`, keeping every side's entries. **2. A fourth fail-open input: a negative amount returns `canSend: true`.** `src/shared/txValidation.js:99-131`. `toFixedPoint("-1")` yields `-1000000000000000000n`, which is a perfectly valid bigint, so `AMOUNT_INVALID` does not fire; `amountFp > ethFp` is false, and `amountFp + feeFp > ethFp` is false. Probed directly against the committed module: ``` validateTransfer({amount: "-1", ethBalance: "1.0", feeStatus: FEE_KNOWN, feeWei: 861000000000000n}) => {"canSend":true,"codes":[]} validateTransfer({isErc20: true, amount: "-0.5", tokenBalance: "1.0", ethBalance: "1.0", feeStatus: FEE_KNOWN, feeWei: 861000000000000n}) => {"canSend":true,"codes":[]} ``` This is the same defect class, in the same guard, that this rework exists to close, and it sits on the one argument that was left unhardened. The module's stated principle is "nothing but a non-negative bigint is a usable fee" — the amount gets no non-negativity check at all, and `CODES.AMOUNT_INVALID` is documented as "the amount is not a number we can do exact arithmetic on", which a negative amount slips past on a technicality. Downstream, `parseEther("-1")` reaches `sendTransaction({value: -1e18n})` and dies at encode time — i.e. a confirmation screen that affirmatively clears a transaction which then fails at broadcast, the exact outcome [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) exists to eliminate. Latent today: `src/popup/views/send.js:198` rejects `parseFloat(amount) <= 0` before ConfirmTx is reached. That is precisely the standing of the three fee inputs this PR was reworked to fix — reachable only through the module's own contract, not through today's single caller — so it is reported on the same footing. Acceptable: `amountFp < 0n` blocks with `AMOUNT_INVALID` (and the doc comment says so), with a test for the ETH and ERC-20 paths. **3. Minor: two Screen Map claims went stale in `README.md` as a result of this change.** `README.md:576` still reads `"Sign & Send" button (disabled if errors)`. Send is now also disabled while the fee estimate is in flight, deliberately showing *no* error — so the parenthetical no longer describes the code. `README.md:575` still summarises the error area as `Errors (insufficient balance)` while the four reserved warning boxes above it are enumerated individually; the three new reserved fee-error boxes are not listed. Acceptable: "disabled if errors, and while the network fee estimate is pending or unavailable", and the three fee-error boxes enumerated alongside the warning boxes. --- Judgement calls and disclosure, not defects: - **The reserve is correct and sufficient.** Verified against the pinned `ethers ^6.16.0` in `node_modules`: `abstract-signer.js:112` upgrades to type 2 only when `feeData.maxFeePerGas` *and* `feeData.maxPriorityFeePerGas` are both non-null, and then assigns `pop.maxFeePerGas = feeData.maxFeePerGas` — the same field `feeReserveWei()` reserves, so gate and node agree exactly. Probed all fallbacks: `maxFeePerGas` present without `maxPriorityFeePerGas` (ethers drops to legacy `gasPrice`, reserve over-reserves — conservative); `gasPrice` only (legacy, exact); neither, `feeData` null/undefined (all `null`, and `confirmTx.js:313-315` throws that into the "Unable to estimate" path rather than a free transaction); negative, non-bigint and string prices (`null`); `gasLimit` non-bigint or negative (`null`). No path reserves less than the node requires. - **Overstated claim in the PR body**, worth correcting rather than fixing in code: "the reserve is an upper bound on what any node will require, and a transaction that clears it cannot fail the funds check" is not true. The reserve is computed from `feeData` fetched when the screen opens, but the broadcast re-fetches `feeData` and derives a fresh `maxFeePerGas = baseFee * 2 + tip`; if the base fee roughly doubles while the user is on the password field, the node requires more than was reserved. The fix is still right and reduces a deterministic failure on every max-value send to a rare one, but it is not the absolute bound the description claims. - `maxFeePerGas: 0n` does not fall through to `gasPrice` (`??` only catches null/undefined), and `gasLimit: 0n` yields a reserve of `0n` under `FEE_KNOWN`. Both correct for a genuinely zero-fee network and both unreachable from ethers' `getFeeData` (`baseFeePerGas` of `0n` is falsy there, so `maxFeePerGas` stays null). Noting them, not filing them. - **The display change is a product decision taken by the implementer, and it is outside the issue's scope and Definition of Done** — [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) asks for the balance check, not the fee label. The self-contradicting-screen argument for changing it is legitimate, but a third option was available and not considered: show the estimate *and* the reserve ("~0.000441 ETH, up to 0.000861 ETH reserved"), which removes the contradiction without quoting mainnet users roughly double what they will actually pay on every send. As shipped, the on-screen label carries no inline explanation — the "typically costs less" caveat exists only in `README.md`, and the README Language & Labeling rule asks for "helpful inline descriptions where needed". Not failing on it; flagging it for the owner's call. - **All three previously fail-open fee inputs now block**, verified by direct probe: `FEE_KNOWN` + `feeWei: null`, `FEE_KNOWN` + a JS number, and an unrecognised `feeStatus` all return `{canSend: false, codes: ["fee-unavailable"]}`, as do a negative `feeWei`, `feeStatus` of `null`/`0`/a boxed `String`, and `feeWei` of `true`/`{}`/`NaN`/a boxed bigint. `feeStatus: undefined` correctly falls to `FEE_PENDING`; no-argument `validateTransfer()` blocks on `amount-invalid`. - **All six mutants killed**, applied one at a time and reverted after, tree confirmed clean at `ce81596`: fee dropped from the ETH comparison (3 failed); ERC-20 ETH-for-gas check removed (2); `>` to `>=` at the ETH+fee boundary (3); `FEE_UNAVAILABLE` treated as zero (7); fee charged against the token balance (1); reserve back on the `gasPrice` basis (2 — `reserves gasLimit * maxFeePerGas, not gasLimit * gasPrice` and `gates out a send the type-2 reserve cannot fund`). 29 tests, 47 assertions, none vacuous. - **Preserved properties confirmed**: exact 6-decimal USDC-style arithmetic (full token balance sends, one micro-unit over blocks, fee never charged to the token balance), one-wei boundary on the type-2 reserve in both directions, both `pendingTx !== txInfo` staleness guards (`confirmTx.js:318` and `:336`), `escapeHtml()` still on the `innerHTML` sink at `confirmTx.js:255`, and no layout shift — the mutually exclusive fee messages are `display: none`'d in `show()` before first paint and `confirm-fee-unknown-error` always reserves its space, so re-validation only flips `visibility`. - **The disclosed `txInfo.balance` boundary is genuinely conservative in all cases.** `formatBalance()` and `formatTokenBalance()` (`src/shared/balances.js:29-43`) both truncate with `.slice(0, 6)` — no rounding — so the display balance is always less than or equal to the true balance and the gate can only refuse a send that would just barely fit. For tokens with 6 or fewer decimals the display string is exact, so there is no error at all in that direction either. Accepting the boundary. - `make check` run here: 9 suites / 178 tests passed in 1.9s, zero `cached` markers, prettier clean, exit 0. `make test-e2e` run here in the pinned Playwright container: 4/4, exit 0. Tracker CI was not relied on. - Single commit, title ends ` (closes #154)`, base `next`, authored `clawbot <clawbot@eeqj.de>`, no Claude/Anthropic references, no attribution trailers, no non-inclusive terminology. No stale "Estimated network fee" left anywhere in `README.md`, `docs/README.md` or `src/`. - Disclosure: the fail-open probes and the six mutants were executed with a direct `node` require of the committed module and with temporary edits reverted via `git checkout`; the pass/fail verdicts above come from `make check`, `make test` and `make test-e2e` only.

clawbot added needs-rework and removed needs-review labels 2026-08-11 15:11:06 +02:00

clawbot force-pushed fix/issue-154-gas-in-balance-check from ce81596100 to ef82c62912

2026-08-11 15:21:55 +02:00

Compare

clawbot added needs-review and removed needs-rework labels 2026-08-11 15:23:40 +02:00

clawbot commented

2026-08-11 15:35:08 +02:00

FAIL — needs-rebase. The substance is clean; only the merge blocks.

1. Not mergeable. TODO.md conflicts with current next.

Branch base is 12acf4d; next has advanced three commits to b155c0f — #210, #223, #161 — each adding at the head of Completed Steps. git merge-tree --write-tree origin/next HEAD exits 1 with CONFLICT (content): Merge conflict in TODO.md (three stages emitted for TODO.md; README.md and src/popup/index.html auto-merge). Gitea's mergeable: true is stale. Rebase onto b155c0f, keeping every side's entries.

2. Minor: the one-line fee branch is chosen on numeric equality, but its comment claims it means "no type-2 pricing".

src/popup/views/confirmTx.js:339 takes the two-line form iff estimateWei < gasCostWei. The else branch at :347-354 therefore also covers gasPrice === maxFeePerGas and gasPrice > maxFeePerGas on a genuinely type-2 network, where it renders the reserve alone with no ~, reading as an exact charge when it is a cap. The comment at :348 ("a network with no type-2 pricing charges exactly what is reserved") is narrower than the condition it explains. The figure shown is the larger, gating one and the arithmetic is unaffected, so this is wording, not behaviour. Acceptable: a comment that matches the condition, or a condition that tests feeData.maxFeePerGas == null.

Verified, not defects:

Nothing gates on feeEstimateWei(). Every consumer traced: estimateWei reaches only textContent and usd() at confirmTx.js:339-346. The gate is feeWei = gasCostWei (:357), the reserve. The USD figure is still derived from the gasPrice basis, the same source as before this PR.
No layout shift — measured, not argued. Drove the built popup in the pinned Playwright container (file:///work/dist/chrome/src/popup/index.html, real compiled CSS). View height is constant at 874px across first paint → estimate landed with the reserve line and the error box visible → estimate failed, on both the ETH and ERC-20 element sets; the fee block is a constant 52px. Body is 396px, #confirm-fee-amount content width 368px, text-xs = 12px/16px mono; wrap begins at 52 characters, and a ~1000000000.123456 ETH ($1,234,567,890.12) line (42 chars) still renders on one line. confirm-errors does grow with its content, but its three driving codes are fee-independent, so it is identical on both renderValidation() calls.
No fifth fail-open input. 43 malformed amounts probed against the committed module on both paths — "", ".", "1.2.3", "1e-9", "1e18", "0x10", "Infinity", NaN, boxed String, bigint, unicode minus, fullwidth digits, "1,000", 19 decimals, {toString}/{valueOf} — all amount-invalid. The only canSend: true results are genuine valid sends (" 0.5 " trimmed, 0.5 as a number, ".5", "-0"/-0 normalising to 0n). Balance and fee inputs re-probed: all fail closed.
All seven mutants killed (applied one at a time in a scratch copy, reverted after, tree confirmed clean): fee dropped from the ETH comparison (3 failed), ERC-20 ETH-for-gas check removed (2), > to >= at the ETH+fee boundary (3), FEE_UNAVAILABLE treated as zero (7), fee charged against the token balance (1), reserve back on the gasPrice basis (3), negative-amount guard removed (2).
make check here: 11 suites / 286 passed, 1 pre-existing skip, 6.9s, zero cached markers, prettier clean, exit 0. make test-e2e here in the pinned container: 4/4, exit 0. Tracker CI not relied on.
Single commit, title ends (closes #154), base next, authored clawbot <clawbot@eeqj.de>, no Claude/Anthropic references, no attribution trailers, no non-inclusive terminology, no scope creep. TODO.md on this branch retains every entry it inherited. The round-2 README.md:576 staleness is fixed.

Disclosure: confirmTx.js has no unit tests, so the one-line/two-line display branch is covered only by my reading and the browser measurement above, not by the suite. This repo's script/lint runs prettier on the host rather than in a container — I used the repo's own entrypoint as-is.

FAIL — `needs-rebase`. The substance is clean; only the merge blocks. **1. Not mergeable. `TODO.md` conflicts with current `next`.** Branch base is [`12acf4d`](https://git.eeqj.de/sneak/AutistMask/commit/12acf4d); `next` has advanced three commits to [`b155c0f`](https://git.eeqj.de/sneak/AutistMask/commit/b155c0f) — [#210](https://git.eeqj.de/sneak/AutistMask/issues/210), [#223](https://git.eeqj.de/sneak/AutistMask/issues/223), [#161](https://git.eeqj.de/sneak/AutistMask/issues/161) — each adding at the head of Completed Steps. `git merge-tree --write-tree origin/next HEAD` exits 1 with `CONFLICT (content): Merge conflict in TODO.md` (three stages emitted for `TODO.md`; `README.md` and `src/popup/index.html` auto-merge). Gitea's `mergeable: true` is stale. Rebase onto `b155c0f`, keeping every side's entries. **2. Minor: the one-line fee branch is chosen on numeric equality, but its comment claims it means "no type-2 pricing".** `src/popup/views/confirmTx.js:339` takes the two-line form iff `estimateWei < gasCostWei`. The else branch at `:347-354` therefore also covers `gasPrice === maxFeePerGas` and `gasPrice > maxFeePerGas` on a genuinely type-2 network, where it renders the reserve alone with no `~`, reading as an exact charge when it is a cap. The comment at `:348` ("a network with no type-2 pricing charges exactly what is reserved") is narrower than the condition it explains. The figure shown is the larger, gating one and the arithmetic is unaffected, so this is wording, not behaviour. Acceptable: a comment that matches the condition, or a condition that tests `feeData.maxFeePerGas == null`. --- Verified, not defects: - **Nothing gates on `feeEstimateWei()`.** Every consumer traced: `estimateWei` reaches only `textContent` and `usd()` at `confirmTx.js:339-346`. The gate is `feeWei = gasCostWei` (`:357`), the reserve. The USD figure is still derived from the `gasPrice` basis, the same source as before this PR. - **No layout shift — measured, not argued.** Drove the built popup in the pinned Playwright container (`file:///work/dist/chrome/src/popup/index.html`, real compiled CSS). View height is constant at 874px across first paint → estimate landed with the reserve line and the error box visible → estimate failed, on both the ETH and ERC-20 element sets; the fee block is a constant 52px. Body is 396px, `#confirm-fee-amount` content width 368px, `text-xs` = 12px/16px mono; wrap begins at 52 characters, and a `~1000000000.123456 ETH ($1,234,567,890.12)` line (42 chars) still renders on one line. `confirm-errors` does grow with its content, but its three driving codes are fee-independent, so it is identical on both `renderValidation()` calls. - **No fifth fail-open input.** 43 malformed amounts probed against the committed module on both paths — `""`, `"."`, `"1.2.3"`, `"1e-9"`, `"1e18"`, `"0x10"`, `"Infinity"`, `NaN`, boxed `String`, bigint, unicode minus, fullwidth digits, `"1,000"`, 19 decimals, `{toString}`/`{valueOf}` — all `amount-invalid`. The only `canSend: true` results are genuine valid sends (`" 0.5 "` trimmed, `0.5` as a number, `".5"`, `"-0"`/`-0` normalising to `0n`). Balance and fee inputs re-probed: all fail closed. - **All seven mutants killed** (applied one at a time in a scratch copy, reverted after, tree confirmed clean): fee dropped from the ETH comparison (3 failed), ERC-20 ETH-for-gas check removed (2), `>` to `>=` at the ETH+fee boundary (3), `FEE_UNAVAILABLE` treated as zero (7), fee charged against the token balance (1), reserve back on the `gasPrice` basis (3), negative-amount guard removed (2). - `make check` here: 11 suites / 286 passed, 1 pre-existing skip, 6.9s, zero cached markers, prettier clean, exit 0. `make test-e2e` here in the pinned container: 4/4, exit 0. Tracker CI not relied on. - Single commit, title ends ` (closes #154)`, base `next`, authored `clawbot <clawbot@eeqj.de>`, no Claude/Anthropic references, no attribution trailers, no non-inclusive terminology, no scope creep. `TODO.md` on this branch retains every entry it inherited. The round-2 `README.md:576` staleness is fixed. Disclosure: `confirmTx.js` has no unit tests, so the one-line/two-line display branch is covered only by my reading and the browser measurement above, not by the suite. This repo's `script/lint` runs prettier on the host rather than in a container — I used the repo's own entrypoint as-is.

clawbot added needs-rebase and removed needs-review labels 2026-08-11 15:35:51 +02:00

clawbot referenced this pull request

2026-08-11 15:36:18 +02:00

test: ConfirmTx has no automated coverage at all — the screen that decides what gets signed #238

clawbot force-pushed fix/issue-154-gas-in-balance-check from ef82c62912 to 36dd4198f1

2026-08-11 15:39:08 +02:00

Compare

clawbot force-pushed fix/issue-154-gas-in-balance-check from 36dd4198f1 to 5cddbb4b7a

2026-08-11 15:41:16 +02:00

Compare

clawbot merged commit 158278d251 into next

2026-08-11 15:41:38 +02:00