fix: gas fee is excluded from the insufficient-balance check, so max-value ETH sends fail at broadcast #154

New Issue

clawbot · 2026-08-09T03:43:09+02:00

clawbot commented

2026-08-09 03:43:09 +02:00

Problem

ConfirmTx estimates and displays the network fee correctly
(src/popup/views/confirmTx.js:227-265), but the validation that enables or
disables the Send button compares amount against balance only and ignores
gas entirely (src/popup/views/confirmTx.js:157-183).

Consequence: a user sending their full ETH balance (or anything within one
gas-cost of it) sees no error, the Send button stays enabled, they enter their
password, and the transaction fails at broadcast — after committing to it. For
a wallet whose whole design premise is "verify everything on the confirmation
screen before signing", surfacing this only as an RPC error is the wrong
behaviour.

Implementation requirements

For native ETH transfers, the check must be
amount + estimatedFee <= balance. For ERC-20 transfers, the token
amount is checked against the token balance as today, but the ETH balance
must separately cover estimatedFee, and an insufficient-ETH-for-gas
condition must be its own distinct, clearly worded error.
The fee arrives asynchronously (confirmTx.js:223). Handle the ordering
properly: re-run validation when the estimate resolves. Until it resolves,
do not show a spurious error and do not let a known-bad send through — pick
the conservative behaviour and describe it in the PR.
Handle the estimate-failed path (confirmTx.js:261-264, "Unable to
estimate"). Decide and document whether Send stays enabled when the fee is
unknown; do not silently treat unknown as zero.
Error messages must be full sentences, per the README Language & Labeling
rules — e.g. "Your balance does not cover this amount plus the network fee."
No layout shift: the error area must already reserve its space, per the
README No Layout Shift policy. Re-validating when the async estimate lands
must not move anything on screen.
Consider offering a correct "send max" affordance only if it falls out
naturally; otherwise leave it out of scope and note it.

Definition of done

Attempting to send an ETH amount that leaves less than the estimated fee
shows an error and disables the Send button.
Attempting an ERC-20 transfer with sufficient tokens but insufficient ETH
for gas shows a distinct error naming the gas shortfall, and disables
Send.
A send comfortably within balance is unaffected and still enables Send.
Validation re-runs when the async fee estimate resolves, with no layout
shift.
The estimate-failure path behaves as documented in the PR.
Unit tests cover the balance-versus-amount-plus-fee arithmetic for both
the ETH and ERC-20 cases.
TODO.md updated in the same commit.
make check passes.

## Problem ConfirmTx estimates and displays the network fee correctly (`src/popup/views/confirmTx.js:227-265`), but the validation that enables or disables the Send button compares **amount against balance only** and ignores gas entirely (`src/popup/views/confirmTx.js:157-183`). Consequence: a user sending their full ETH balance (or anything within one gas-cost of it) sees no error, the Send button stays enabled, they enter their password, and the transaction fails at broadcast — after committing to it. For a wallet whose whole design premise is "verify everything on the confirmation screen before signing", surfacing this only as an RPC error is the wrong behaviour. ## Implementation requirements - For **native ETH transfers**, the check must be `amount + estimatedFee <= balance`. For **ERC-20 transfers**, the token amount is checked against the token balance as today, but the ETH balance must separately cover `estimatedFee`, and an insufficient-ETH-for-gas condition must be its own distinct, clearly worded error. - The fee arrives asynchronously (`confirmTx.js:223`). Handle the ordering properly: re-run validation when the estimate resolves. Until it resolves, do not show a spurious error and do not let a known-bad send through — pick the conservative behaviour and describe it in the PR. - Handle the estimate-failed path (`confirmTx.js:261-264`, "Unable to estimate"). Decide and document whether Send stays enabled when the fee is unknown; do not silently treat unknown as zero. - Error messages must be full sentences, per the README Language & Labeling rules — e.g. "Your balance does not cover this amount plus the network fee." - **No layout shift**: the error area must already reserve its space, per the README No Layout Shift policy. Re-validating when the async estimate lands must not move anything on screen. - Consider offering a correct "send max" affordance only if it falls out naturally; otherwise leave it out of scope and note it. ## Definition of done - [ ] Attempting to send an ETH amount that leaves less than the estimated fee shows an error and disables the Send button. - [ ] Attempting an ERC-20 transfer with sufficient tokens but insufficient ETH for gas shows a distinct error naming the gas shortfall, and disables Send. - [ ] A send comfortably within balance is unaffected and still enables Send. - [ ] Validation re-runs when the async fee estimate resolves, with no layout shift. - [ ] The estimate-failure path behaves as documented in the PR. - [ ] Unit tests cover the balance-versus-amount-plus-fee arithmetic for both the ETH and ERC-20 cases. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:43:09 +02:00

clawbot referenced this issue from a commit

2026-08-11 14:20:32 +02:00

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

clawbot referenced a pull request that will close this issue

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

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

clawbot referenced this issue

2026-08-11 14:22:57 +02:00

feat: "send max" affordance — emptying an ETH account now requires manual arithmetic #198

clawbot referenced this issue

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