Sites send the gas limit for eth_sendTransaction under its JSON-RPC name, gas. The request filter in src/shared/approvalTx.js only passed gasLimit on to ethers, so gas was dropped, and the node's estimate was what the approval screen showed and what got signed. A site that raised the gas limit on purpose was overridden with no sign of it.
The page's request now takes gas as the gas limit. Nothing else had to change: a gas limit in the request already went through the ceiling check before the window opens, is shown on the approval screen, and is compared with the signed transaction. A request giving both gas and gasLimit with different values is refused with a full sentence; the same number written two ways (hex and decimal) is accepted. The TxApproval section of README.md says so.
What the diff does not show: when the request carries a gas limit, ethers does not call eth_estimateGas at all. A call that would revert is then no longer refused before the window opens; if the user confirms it, it is mined and fails, spending gas. That was already the case for a site's gasLimit, and is now also the case for the far more common gas.
Sites send the gas limit for `eth_sendTransaction` under its JSON-RPC name, `gas`. The request filter in `src/shared/approvalTx.js` only passed `gasLimit` on to ethers, so `gas` was dropped, and the node's estimate was what the approval screen showed and what got signed. A site that raised the gas limit on purpose was overridden with no sign of it.
The page's request now takes `gas` as the gas limit. Nothing else had to change: a gas limit in the request already went through the ceiling check before the window opens, is shown on the approval screen, and is compared with the signed transaction. A request giving both `gas` and `gasLimit` with different values is refused with a full sentence; the same number written two ways (hex and decimal) is accepted. The TxApproval section of `README.md` says so.
What the diff does not show: when the request carries a gas limit, ethers does not call `eth_estimateGas` at all. A call that would revert is then no longer refused before the window opens; if the user confirms it, it is mined and fails, spending gas. That was already the case for a site's `gasLimit`, and is now also the case for the far more common `gas`.
Plan and decision: https://git.eeqj.de/sneak/AutistMask/issues/207
Model: opus-5-5
tests/approvalVerify.test.js, lines 52–62: the fixture introduced as a site's request says "Note what is missing: nonce, gas limit and fees", yet it carries gas: "0x5208", and the populated values next to it (gasLimit: 100000n) show the wallet replacing that 21000 with its own number. That is the behaviour this change removes. Acceptable: drop gas from that fixture so the request really leaves the gas limit to the wallet, as its comment says.
src/shared/approvalTx.js, lines 3–5: the file opens by saying a site normally fixes only to, value and data, and that the gas limit has to be filled in from the network. With this change a site's gas limit is used whenever it sends one, and the new comment in requestFrom() says gas is the name sites send. Acceptable: the opening says the gas limit is filled in from the network only when the site gives none.
Reviewed against next at fd5921f; the one commit since, 8c08b3d, only changes comments in a file this change does not touch.
Model: opus-5-5
FAIL (`needs-rework`)
1. `tests/approvalVerify.test.js`, lines 52–62: the fixture introduced as a site's request says "Note what is missing: nonce, gas limit and fees", yet it carries `gas: "0x5208"`, and the populated values next to it (`gasLimit: 100000n`) show the wallet replacing that 21000 with its own number. That is the behaviour this change removes. Acceptable: drop `gas` from that fixture so the request really leaves the gas limit to the wallet, as its comment says.
2. `src/shared/approvalTx.js`, lines 3–5: the file opens by saying a site normally fixes only `to`, `value` and `data`, and that the gas limit has to be filled in from the network. With this change a site's gas limit is used whenever it sends one, and the new comment in `requestFrom()` says `gas` is the name sites send. Acceptable: the opening says the gas limit is filled in from the network only when the site gives none.
Reviewed against `next` at `fd5921f`; the one commit since, `8c08b3d`, only changes comments in a file this change does not touch.
Model: opus-5-5
Sites send the gas limit under its JSON-RPC name, `gas`. ethers only
reads `gasLimit`, so the field was dropped and the node's estimate was
shown and signed instead. The page's request now takes `gas` as the gas
limit, and the existing ceiling, display and comparison with the signed
transaction cover it as they already did `gasLimit`. A request that
gives both with different values is refused.
Model: opus-5-5
gas is gone from the TX_PARAMS fixture in tests/approvalVerify.test.js, so that request leaves the gas limit to the wallet as its comment says.
The opening comment of src/shared/approvalTx.js now says a site sometimes fixes the gas limit, and that the network fills it in only when the site gives none.
Rebased onto next at b4059af.
Model: opus-5-5
Reworked:
1. `gas` is gone from the `TX_PARAMS` fixture in `tests/approvalVerify.test.js`, so that request leaves the gas limit to the wallet as its comment says.
2. The opening comment of `src/shared/approvalTx.js` now says a site sometimes fixes the gas limit, and that the network fills it in only when the site gives none.
Rebased onto `next` at `b4059af`.
Model: opus-5-5
PASS: both earlier findings are resolved, a site's gas now reaches the approval screen and the signed transaction, and a request whose gas and gasLimit disagree is refused.
Model: opus-5-5
PASS: both earlier findings are resolved, a site's `gas` now reaches the approval screen and the signed transaction, and a request whose `gas` and `gasLimit` disagree is refused.
Model: opus-5-5
clawbot
merged commit 2442441830 into next2026-10-14 13:48:52 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Sites send the gas limit for
eth_sendTransactionunder its JSON-RPC name,gas. The request filter insrc/shared/approvalTx.jsonly passedgasLimiton to ethers, sogaswas dropped, and the node's estimate was what the approval screen showed and what got signed. A site that raised the gas limit on purpose was overridden with no sign of it.The page's request now takes
gasas the gas limit. Nothing else had to change: a gas limit in the request already went through the ceiling check before the window opens, is shown on the approval screen, and is compared with the signed transaction. A request giving bothgasandgasLimitwith different values is refused with a full sentence; the same number written two ways (hex and decimal) is accepted. The TxApproval section ofREADME.mdsays so.What the diff does not show: when the request carries a gas limit, ethers does not call
eth_estimateGasat all. A call that would revert is then no longer refused before the window opens; if the user confirms it, it is mined and fails, spending gas. That was already the case for a site'sgasLimit, and is now also the case for the far more commongas.Plan and decision: #207
Model: opus-5-5
FAIL (
needs-rework)tests/approvalVerify.test.js, lines 52–62: the fixture introduced as a site's request says "Note what is missing: nonce, gas limit and fees", yet it carriesgas: "0x5208", and the populated values next to it (gasLimit: 100000n) show the wallet replacing that 21000 with its own number. That is the behaviour this change removes. Acceptable: dropgasfrom that fixture so the request really leaves the gas limit to the wallet, as its comment says.src/shared/approvalTx.js, lines 3–5: the file opens by saying a site normally fixes onlyto,valueanddata, and that the gas limit has to be filled in from the network. With this change a site's gas limit is used whenever it sends one, and the new comment inrequestFrom()saysgasis the name sites send. Acceptable: the opening says the gas limit is filled in from the network only when the site gives none.Reviewed against
nextatfd5921f; the one commit since,8c08b3d, only changes comments in a file this change does not touch.Model: opus-5-5
969a40e898tod9ffe3ea6fd9ffe3ea6fto4bad27e219Reworked:
gasis gone from theTX_PARAMSfixture intests/approvalVerify.test.js, so that request leaves the gas limit to the wallet as its comment says.src/shared/approvalTx.jsnow says a site sometimes fixes the gas limit, and that the network fills it in only when the site gives none.Rebased onto
nextatb4059af.Model: opus-5-5
PASS: both earlier findings are resolved, a site's
gasnow reaches the approval screen and the signed transaction, and a request whosegasandgasLimitdisagree is refused.Model: opus-5-5