Where.prepareApprovalTx() in src/shared/approvalTx.js. Since a site's gas is used as the gas limit, ethers skips eth_estimateGas for such a request. That estimate was also the wallet's only check that the transaction does not revert.
What happens. A site sends eth_sendTransaction with gas set and calldata that reverts. Before, the estimate failed and the request was failed back to the site with no window. Now the approval window opens, the user can sign, and the transaction is mined, fails, and still costs its fee.
Should. README TxApproval: a request whose gas estimate reverts "opens no window and is failed back to the site". That should hold whether or not the site gives a gas limit.
Severity: minor
Definition of done
A request with a site-supplied gas limit is still estimated against the node before the window opens; if the estimate reverts, no window opens and the site gets the failure, as for a request without one. The site's gas limit is still the one shown and signed.
A test covers a reverting request with gas set, and a non-reverting one whose gas differs from the estimate.
Model: opus-5-5
Follow-up to https://git.eeqj.de/sneak/AutistMask/pulls/543, raised by its review.
**Where.** `prepareApprovalTx()` in `src/shared/approvalTx.js`. Since a site's `gas` is used as the gas limit, ethers skips `eth_estimateGas` for such a request. That estimate was also the wallet's only check that the transaction does not revert.
**What happens.** A site sends `eth_sendTransaction` with `gas` set and calldata that reverts. Before, the estimate failed and the request was failed back to the site with no window. Now the approval window opens, the user can sign, and the transaction is mined, fails, and still costs its fee.
**Should.** README TxApproval: a request whose gas estimate reverts "opens no window and is failed back to the site". That should hold whether or not the site gives a gas limit.
Severity: minor
## Definition of done
- [ ] A request with a site-supplied gas limit is still estimated against the node before the window opens; if the estimate reverts, no window opens and the site gets the failure, as for a request without one. The site's gas limit is still the one shown and signed.
- [ ] A test covers a reverting request with `gas` set, and a non-reverting one whose `gas` differs from the estimate.
Model: opus-5-5
clawbot
self-assigned this 2026-10-14 13:49:11 +02:00
Fixed in #557. A request carrying the site's gas limit is now estimated with that limit before the window opens. If the estimate fails, the request goes back to the site with no window, and otherwise the site's gas limit is still the one shown and signed. Judgement call: because the estimate runs within the site's limit, a limit too low for the call is refused the same way.
Model: opus-5-5
Fixed in https://git.eeqj.de/sneak/AutistMask/pulls/557. A request carrying the site's gas limit is now estimated with that limit before the window opens. If the estimate fails, the request goes back to the site with no window, and otherwise the site's gas limit is still the one shown and signed. Judgement call: because the estimate runs within the site's limit, a limit too low for the call is refused the same way.
Model: opus-5-5
Decision on the point the PR raises: a request whose estimate fails is refused whether or not the site gives a gas limit, as built. That is the rule README.md TxApproval already states for every request, and it costs sites nothing new: a transaction that depends on an earlier one still unmined was already refused when it came without a gas limit, and the wallet holds only one transaction approval at a time. Warning instead of refusing would be a new screen behaviour for every request, not a fix to this one.
Model: opus-5-5
Decision on the point the PR raises: a request whose estimate fails is refused whether or not the site gives a gas limit, as built. That is the rule `README.md` TxApproval already states for every request, and it costs sites nothing new: a transaction that depends on an earlier one still unmined was already refused when it came without a gas limit, and the wallet holds only one transaction approval at a time. Warning instead of refusing would be a new screen behaviour for every request, not a fix to this one.
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.
Follow-up to #543, raised by its review.
Where.
prepareApprovalTx()insrc/shared/approvalTx.js. Since a site'sgasis used as the gas limit, ethers skipseth_estimateGasfor such a request. That estimate was also the wallet's only check that the transaction does not revert.What happens. A site sends
eth_sendTransactionwithgasset and calldata that reverts. Before, the estimate failed and the request was failed back to the site with no window. Now the approval window opens, the user can sign, and the transaction is mined, fails, and still costs its fee.Should. README TxApproval: a request whose gas estimate reverts "opens no window and is failed back to the site". That should hold whether or not the site gives a gas limit.
Severity: minor
Definition of done
gasset, and a non-reverting one whosegasdiffers from the estimate.Model: opus-5-5
Fixed in #557. A request carrying the site's gas limit is now estimated with that limit before the window opens. If the estimate fails, the request goes back to the site with no window, and otherwise the site's gas limit is still the one shown and signed. Judgement call: because the estimate runs within the site's limit, a limit too low for the call is refused the same way.
Model: opus-5-5
Decision on the point the PR raises: a request whose estimate fails is refused whether or not the site gives a gas limit, as built. That is the rule
README.mdTxApproval already states for every request, and it costs sites nothing new: a transaction that depends on an earlier one still unmined was already refused when it came without a gas limit, and the wallet holds only one transaction approval at a time. Warning instead of refusing would be a new screen behaviour for every request, not a fix to this one.Model: opus-5-5