1 Commits
Author SHA1 Message Date
sneak ee2e3f6dfd fix: a send the confirmation screen refuses moves nothing on it (closes #583)
check / check (push) Canceled after 0s
e2e / e2e-chrome (push) Canceled after 0s
e2e / e2e-firefox (push) Canceled after 0s
Sign & Send wrote its two refusals, a network changed since review and a fee over the bound, into the errors box above the password field. That box reserves one empty line, so a refusal grew it and pushed the password field and Sign & Send down.

Each refusal now has its own box with its sentence fixed in index.html, holding its space from the first paint and only made visible, as #558 did for the balance messages. The fee refusal no longer quotes the fee; its full reason is logged. A Chrome test raises the stub node's fee only at signing and checks the password field and Sign & Send stay where they were.

Model: opus-5-5
2026-10-19 10:54:29 +00:00
8 changed files with 161 additions and 47 deletions
+15 -1
View File
@@ -430,6 +430,11 @@ reserve while sitting on the same side of the estimate, so swapping the two in
what [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) was, and it was what [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) was, and it was
previously correct by reading only. previously correct by reading only.
It also covers a send that Sign & Send refuses for its network fee: the stub
node quotes an ordinary fee while the confirmation screen is drawn and one over
the bound when Sign & Send asks again, and the refusal must broadcast nothing
and leave the password field and Sign & Send where they were.
It also covers both ways the wait for a sent transaction's receipt ends on the It also covers both ways the wait for a sent transaction's receipt ends on the
error screen: lookups that still find no receipt 60 seconds after the broadcast, error screen: lookups that still find no receipt 60 seconds after the broadcast,
and six lookups in a row that fail. Each must show its own message, and Done and six lookups in a row that fail. Each must show its own message, and Done
@@ -1793,6 +1798,9 @@ view would leave a wallet one click from deletion.
estimated, and it says the wallet does not know how many decimal places estimated, and it says the wallet does not know how many decimal places
the token uses and that the transaction cannot be sent; for any other the token uses and that the transaction cannot be sent; for any other
failure it asks the user to go back and try again failure it asks the user to go back and try again
- Two more reserved error boxes, made visible when "Sign & Send" refuses a
send because the active network changed or because of its network fee (see
Transitions), so the refusal moves nothing
- Password: an inline field on this screen, not a modal, with its own error - Password: an inline field on this screen, not a modal, with its own error
line line
- "Sign & Send" button (disabled if errors, while the balance is being read, - "Sign & Send" button (disabled if errors, while the balance is being read,
@@ -1810,7 +1818,7 @@ view would leave a wallet one click from deletion.
- "Sign & Send" after the active network was switched in Settings since this - "Sign & Send" after the active network was switched in Settings since this
transaction was reviewed → nothing is signed; "The active network changed transaction was reviewed → nothing is signed; "The active network changed
after this transaction was reviewed, so it was not sent. Please go back after this transaction was reviewed, so it was not sent. Please go back
and review it again." in the errors box, no screen change. After a switch, and review it again." in its own box, no screen change. After a switch,
"Back" from Settings draws this screen again from the saved transaction, "Back" from Settings draws this screen again from the saved transaction,
as a popup reopened onto it does; when Send is beneath it on the stack, as as a popup reopened onto it does; when Send is beneath it on the stack, as
when Send showed it in the same popup, "Back" lands below Send instead when Send showed it in the same popup, "Back" lands below Send instead
@@ -1824,6 +1832,12 @@ view would leave a wallet one click from deletion.
fee estimate and the contract and never-sent recipient checks ask that fee estimate and the contract and never-sent recipient checks ask that
network's RPC endpoint, and the Etherscan label check asks that network's network's RPC endpoint, and the Etherscan label check asks that network's
explorer explorer
- "Sign & Send" (correct password) when the gas limit and fee the RPC
endpoint fills in at signing go over the bounds `assertWithinCeilings()`
in `src/shared/approvalVerify.js` sets → nothing is signed; "This
transaction would allow a network fee of more than this wallet will sign
for, so it was not sent. Please go back and try again." in its own box, no
screen change
- "Sign & Send" (wrong password) → "Wrong password." on the password error - "Sign & Send" (wrong password) → "Wrong password." on the password error
line, no screen change line, no screen change
- "Sign & Send" signs as the From address above, not as whichever address is - "Sign & Send" signs as the From address above, not as whichever address is
+19
View File
@@ -697,6 +697,25 @@
id="confirm-fee-unknown-error" id="confirm-fee-unknown-error"
class="mb-2 border border-border border-dashed p-2 text-xs invisible" class="mb-2 border border-border border-dashed p-2 text-xs invisible"
></div> ></div>
<!-- Shown when Sign & Send refuses a send for its network or
its fee. Each holds its space from the first paint, so the
refusal moves nothing. -->
<div
id="confirm-network-changed-error"
class="mb-2 border border-border border-dashed p-2 text-xs invisible"
>
The active network changed after this transaction was
reviewed, so it was not sent. Please go back and review it
again.
</div>
<div
id="confirm-fee-bound-error"
class="mb-2 border border-border border-dashed p-2 text-xs invisible"
>
This transaction would allow a network fee of more than this
wallet will sign for, so it was not sent. Please go back and
try again.
</div>
<div class="mb-2"> <div class="mb-2">
<label class="block mb-1 text-xs">Password</label> <label class="block mb-1 text-xs">Password</label>
<input <input
+16 -19
View File
@@ -291,9 +291,12 @@ function show(txInfo) {
renderValidation(txInfo); renderValidation(txInfo);
// Reset password field and error // Reset password field and error, and any refusal of an earlier Sign &
// Send.
$("confirm-tx-password").value = ""; $("confirm-tx-password").value = "";
hideError("confirm-tx-password-error"); hideError("confirm-tx-password-error");
setVisible("confirm-network-changed-error", false);
setVisible("confirm-fee-bound-error", false);
// Gas estimate — show placeholder then fetch async // Gas estimate — show placeholder then fetch async
$("confirm-fee").style.visibility = "visible"; $("confirm-fee").style.visibility = "visible";
@@ -564,8 +567,8 @@ async function estimateGas(txInfo) {
// hostile node can set — the dApp path's ceilings never reached this one. // hostile node can set — the dApp path's ceilings never reached this one.
// Populating before the check is what makes assertWithinCeilings() see the // Populating before the check is what makes assertWithinCeilings() see the
// same numbers that would be signed; it throws an ApprovalMismatchError when // same numbers that would be signed; it throws an ApprovalMismatchError when
// the product gasLimit × maxFeePerGas is over the bound, which the caller // the product gasLimit × maxFeePerGas is over the bound, and the caller then
// shows in the reserved error area rather than sending. // shows the refusal reserved for it rather than sending.
async function populateVerifyAndSend(connectedSigner, tx, fees = null) { async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
let request; let request;
if (tx.token === "ETH") { if (tx.token === "ETH") {
@@ -593,15 +596,6 @@ async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
return connectedSigner.sendTransaction(populated); return connectedSigner.sendTransaction(populated);
} }
// Show a full-sentence send failure in the reserved errors box, the same
// element and markup renderValidation() uses for the invalid-amount message,
// so it never moves anything on the screen.
function showSendError(message) {
const el = $("confirm-errors");
el.innerHTML = `<div class="text-xs">${escapeHtml(message)}</div>`;
el.style.visibility = "visible";
}
async function checkRecipientHistory(txInfo) { async function checkRecipientHistory(txInfo) {
try { try {
const provider = reviewedNetworkProvider(txInfo); const provider = reviewedNetworkProvider(txInfo);
@@ -640,6 +634,10 @@ function init(_ctx) {
onViewLeave("confirm-tx", clearPassword); onViewLeave("confirm-tx", clearPassword);
$("btn-confirm-send").addEventListener("click", async () => { $("btn-confirm-send").addEventListener("click", async () => {
// A press shows only its own refusal, if any.
setVisible("confirm-network-changed-error", false);
setVisible("confirm-fee-bound-error", false);
const wallet = state.wallets[state.selectedWallet]; const wallet = state.wallets[state.selectedWallet];
// Every Send button refuses a defective wallet before this screen, // Every Send button refuses a defective wallet before this screen,
// but the popup also reopens onto it from a saved view. // but the popup also reopens onto it from a saved view.
@@ -654,11 +652,7 @@ function init(_ctx) {
// Settings after a switch draws this screen again from the saved // Settings after a switch draws this screen again from the saved
// transaction, as a popup reopened onto it does. // transaction, as a popup reopened onto it does.
if (currentNetwork().chainId !== pendingTx.chainId) { if (currentNetwork().chainId !== pendingTx.chainId) {
showSendError( setVisible("confirm-network-changed-error", true);
"The active network changed after this transaction was" +
" reviewed, so it was not sent. Please go back and review" +
" it again.",
);
return; return;
} }
@@ -724,10 +718,13 @@ function init(_ctx) {
decryptedSecret = null; decryptedSecret = null;
// A fee over the bound is refused before anything is broadcast, so // A fee over the bound is refused before anything is broadcast, so
// there is no transaction that may have reached the network to warn // there is no transaction that may have reached the network to warn
// about: the message stays on the confirmation screen where the // about: the refusal stays on the confirmation screen where the
// user can go back, rather than routing to the sent/failed screen. // user can go back, rather than routing to the sent/failed screen.
// Its sentence is fixed in index.html, so that its box can hold its
// space from the first paint; the reason in full is logged.
if (e && e.approvalMismatch) { if (e && e.approvalMismatch) {
showSendError(e.message); log.errorf("send refused:", e.message);
setVisible("confirm-fee-bound-error", true);
return; return;
} }
const hash = tx ? tx.hash : null; const hash = tx ? tx.hash : null;
+1 -1
View File
@@ -12,7 +12,7 @@
// The check is driven here with a fake connected signer rather than a real // The check is driven here with a fake connected signer rather than a real
// one: populateTransaction() returns the fees the node would have produced, // one: populateTransaction() returns the fees the node would have produced,
// and sendTransaction() records whether the send actually happened. The real // and sendTransaction() records whether the send actually happened. The real
// DOM path around it — reading the fee error into the reserved errors box — is // DOM path around it — the refusal shown in the box reserved for it — is
// covered by the Chrome e2e suite. // covered by the Chrome e2e suite.
globalThis.chrome = { globalThis.chrome = {
+5 -18
View File
@@ -440,12 +440,8 @@ describe.each([
expect(mockNode.broadcast).toEqual([]); expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled(); expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled(); expect(txStatus.showError).not.toHaveBeenCalled();
expect(el("confirm-errors").style.visibility).toBe("visible"); expect(shown("confirm-network-changed-error")).toBe(true);
expect(el("confirm-errors").innerHTML).toContain( expect(shown("confirm-fee-bound-error")).toBe(false);
"The active network changed after this transaction was" +
" reviewed, so it was not sent. Please go back and review" +
" it again.",
);
}); });
}, },
); );
@@ -497,10 +493,7 @@ describe.each([
expect(mockNode.broadcast).toEqual([]); expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled(); expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled(); expect(txStatus.showError).not.toHaveBeenCalled();
expect(el("confirm-errors").innerHTML).toContain( expect(shown("confirm-network-changed-error")).toBe(true);
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
}); });
test("is sent on that network, as described, once it is chosen in Settings again and Back pressed", async () => { test("is sent on that network, as described, once it is chosen in Settings again and Back pressed", async () => {
@@ -579,10 +572,7 @@ describe.each([
expect(signTransaction).not.toHaveBeenCalled(); expect(signTransaction).not.toHaveBeenCalled();
expect(mockNode.broadcast).toEqual([]); expect(mockNode.broadcast).toEqual([]);
expect(errors()).toContain( expect(shown("confirm-network-changed-error")).toBe(true);
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
}); });
const tokenSend = { const tokenSend = {
@@ -629,10 +619,7 @@ describe.each([
expect(signTransaction).not.toHaveBeenCalled(); expect(signTransaction).not.toHaveBeenCalled();
expect(mockNode.broadcast).toEqual([]); expect(mockNode.broadcast).toEqual([]);
expect(errors()).toContain( expect(shown("confirm-network-changed-error")).toBe(true);
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
}); });
test("with a fee quoted by a node with no type-2 pricing, shows the fee in that network's native currency and price", async () => { test("with a fee quoted by a node with no type-2 pricing, shows the fee in that network's native currency and price", async () => {
+7 -4
View File
@@ -226,8 +226,9 @@ const RPC_RESULTS = {
// The "latest" block, which ethers' getFeeData() reads baseFeePerGas from // The "latest" block, which ethers' getFeeData() reads baseFeePerGas from
// to derive maxFeePerGas. Without it every fee is a legacy gasPrice, the // to derive maxFeePerGas. Without it every fee is a legacy gasPrice, the
// reserve and the estimate collapse to the same number, and the gate tests // reserve and the estimate collapse to the same number, and the gate tests
// stop being able to distinguish them. // stop being able to distinguish them. opts.baseFeeWei replaces its base fee,
function latestBlock() { // read at request time like every other fixture switch.
function latestBlock(opts) {
return { return {
hash: "0x" + "11".repeat(32), hash: "0x" + "11".repeat(32),
parentHash: "0x" + "22".repeat(32), parentHash: "0x" + "22".repeat(32),
@@ -239,7 +240,7 @@ function latestBlock() {
gasUsed: "0xf4240", gasUsed: "0xf4240",
miner: STUB_COUNTERPARTY, miner: STUB_COUNTERPARTY,
extraData: "0x", extraData: "0x",
baseFeePerGas: hex(BASE_FEE_WEI), baseFeePerGas: opts.baseFeeWei || hex(BASE_FEE_WEI),
transactions: [], transactions: [],
}; };
} }
@@ -481,7 +482,7 @@ function rpcReply(req, opts, report) {
}); });
} }
if (req.method === "eth_getBlockByNumber") { if (req.method === "eth_getBlockByNumber") {
return Object.assign(envelope, { result: latestBlock() }); return Object.assign(envelope, { result: latestBlock(opts) });
} }
// The end of the dApp transaction round trip: the raw signed transaction // The end of the dApp transaction round trip: the raw signed transaction
// the background hands to the node. It is recorded verbatim so a test can // the background hands to the node. It is recorded verbatim so a test can
@@ -625,6 +626,8 @@ function traceEnabled(raw) {
* @param {string} [opts.ethBalanceWei] hex wei answered to eth_getBalance; * @param {string} [opts.ethBalanceWei] hex wei answered to eth_getBalance;
* defaults to zero, which is what every test that predates the funded * defaults to zero, which is what every test that predates the funded
* fixture expects. * fixture expects.
* @param {string} [opts.baseFeeWei] hex wei answered as the latest block's
* baseFeePerGas; defaults to BASE_FEE_WEI. Read at request time.
* @param {boolean} [opts.failGasEstimate] answer eth_estimateGas with a * @param {boolean} [opts.failGasEstimate] answer eth_estimateGas with a
* node-side refusal. * node-side refusal.
* @param {boolean} [opts.holdBalance] hold every batch containing an * @param {boolean} [opts.holdBalance] hold every batch containing an
+95 -1
View File
@@ -2232,7 +2232,8 @@ function feeEth(wei) {
// What the confirmation screen is showing right now, read out of the DOM in // What the confirmation screen is showing right now, read out of the DOM in
// one pass: whether sending is allowed, which reason it is giving, what the // one pass: whether sending is allowed, which reason it is giving, what the
// fee block says, how tall the whole view is, and where Sign & Send sits in it. // fee block says, how tall the whole view is, and where the password field and
// Sign & Send sit in it.
async function confirmState(page) { async function confirmState(page) {
return page.evaluate(() => { return page.evaluate(() => {
const el = (id) => document.getElementById(id); const el = (id) => document.getElementById(id);
@@ -2245,6 +2246,9 @@ async function confirmState(page) {
}; };
return { return {
height: el("view-confirm-tx").getBoundingClientRect().height, height: el("view-confirm-tx").getBoundingClientRect().height,
passwordTop:
el("confirm-tx-password").getBoundingClientRect().top -
el("view-confirm-tx").getBoundingClientRect().top,
sendTop: sendTop:
el("btn-confirm-send").getBoundingClientRect().top - el("btn-confirm-send").getBoundingClientRect().top -
el("view-confirm-tx").getBoundingClientRect().top, el("view-confirm-tx").getBoundingClientRect().top,
@@ -2262,6 +2266,12 @@ async function confirmState(page) {
amountFeeError: shown("confirm-amount-fee-error"), amountFeeError: shown("confirm-amount-fee-error"),
gasError: shown("confirm-gas-error"), gasError: shown("confirm-gas-error"),
feeUnknownError: shown("confirm-fee-unknown-error"), feeUnknownError: shown("confirm-fee-unknown-error"),
// Its sentence is wrapped across lines in index.html.
feeBoundError: shown("confirm-fee-bound-error")
? el("confirm-fee-bound-error")
.textContent.replace(/\s+/g, " ")
.trim()
: "",
sendDisabled: el("btn-confirm-send").disabled, sendDisabled: el("btn-confirm-send").disabled,
}; };
}); });
@@ -3057,6 +3067,90 @@ test("a token that lies about decimals() at signing time broadcasts nothing (#30
} }
}); });
// ------------------------- a send Sign & Send refuses moves nothing (#583)
//
// README, No Layout Shift. Sign & Send refuses a send on the confirmation
// screen when the active network changed since review and when its fee is over
// the bound, and each refusal's sentence sits in a box that held its space from
// the first paint. This drives the fee refusal, the longer of the two.
// The base fee the stub node quotes once Sign & Send is pressed. ethers asks
// twice it plus the 1 gwei priority fee per gas, 50,001 gwei, so the 21,000-gas
// send would allow a fee of about 1.05 ETH, over the 1 ETH this wallet signs
// for (MAX_TOTAL_FEE in src/shared/approvalVerify.js).
const OVER_BOUND_BASE_FEE_WEI = 25000n * 10n ** 9n;
test("a send refused for its fee leaves the password field and Sign & Send where they were (#583)", async (env) => {
env.errors.expect("confirmTx logging the refused send", /send refused/);
try {
await goToConfirm(env.page, {
token: "ETH",
balance: FUNDED_ETH_TEXT + " ETH",
amount: COMFORTABLE_AMOUNT,
});
await waitForEstimate(env.page);
const before = await confirmState(env.page);
assert(
!before.sendDisabled && before.feeBoundError === "",
"the screen is not ready to send: " + JSON.stringify(before),
);
// Only now, with the screen drawn and its fee estimated, does the
// node quote a fee over the bound.
env.routeOpts.baseFeeWei = toHexWei(OVER_BOUND_BASE_FEE_WEI);
const sent = env.routeOpts.broadcastTransactions.length;
await fillPasswordAndSend(env.page);
await env.page.waitForFunction(
() =>
getComputedStyle(
document.getElementById("confirm-fee-bound-error"),
).visibility === "visible",
null,
{ timeout: 60000 },
);
const after = await confirmState(env.page);
// Printed pass or fail, as the dust threshold test does.
console.log(
"# refused send: password field top " +
before.passwordTop +
" -> " +
after.passwordTop +
"px, Sign & Send top " +
before.sendTop +
" -> " +
after.sendTop +
"px",
);
assert(
env.routeOpts.broadcastTransactions.length === sent,
"a send over the fee bound reached the RPC",
);
assert(
after.feeBoundError ===
"This transaction would allow a network fee of more than" +
" this wallet will sign for, so it was not sent. Please" +
" go back and try again.",
"wrong fee refusal: " + JSON.stringify(after.feeBoundError),
);
assert(
after.passwordTop === before.passwordTop,
"the password field moved " +
(after.passwordTop - before.passwordTop) +
"px when the send was refused",
);
assert(
after.sendTop === before.sendTop,
"Sign & Send moved " +
(after.sendTop - before.sendTop) +
"px when the send was refused",
);
} finally {
env.routeOpts.baseFeeWei = undefined;
await backToAddress(env.page);
}
});
// ------------------------------ the wait for a receipt ending in error (#315) // ------------------------------ the wait for a receipt ending in error (#315)
// //
// README.md (WaitTx) documents two ways the wait ends on the error screen: a // README.md (WaitTx) documents two ways the wait ends on the error screen: a
+3 -3
View File
@@ -313,9 +313,9 @@ describe("Back after a switch from the Settings gear", () => {
boot.node("confirm-tx-password").value = "any password"; boot.node("confirm-tx-password").value = "any password";
await boot.click("btn-confirm-send"); await boot.click("btn-confirm-send");
expect(boot.visibleViews()).toEqual(["confirm-tx"]); expect(boot.visibleViews()).toEqual(["confirm-tx"]);
expect(boot.node("confirm-errors").innerHTML).toContain( expect(
"The active network changed after this transaction was reviewed", boot.node("confirm-network-changed-error").style.visibility,
); ).toBe("visible");
// Refused before the password is even tried. // Refused before the password is even tried.
expect(boot.text("confirm-tx-password-error")).toBe(""); expect(boot.text("confirm-tx-password-error")).toBe("");
}); });