fix: a send the confirmation screen refuses moves nothing on it (closes #583)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run

Sign & Send wrote its refusals, a network changed since review and a gas limit or fee over its 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. One sentence is true of all three bounds and quotes no number; the full reason is logged. A Chrome test raises the stub node's fee only at signing and checks nothing moved; a unit test refuses a gas limit over its bound into the same box.

Model: opus-5-5
This commit was merged in pull request #608.
This commit is contained in:
2026-10-19 14:45:01 +02:00
parent 99269675b7
commit a73fb17295
8 changed files with 342 additions and 63 deletions
+16 -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
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
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
@@ -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
the token uses and that the transaction cannot be sent; for any other
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 gas limit or fee
(see Transitions), so the refusal moves nothing
- Password: an inline field on this screen, not a modal, with its own error
line
- "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
transaction was reviewed → nothing is signed; "The active network changed
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,
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
@@ -1824,6 +1832,13 @@ view would leave a wallet one click from deletion.
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
explorer
- "Sign & Send" (correct password) when the gas limit, a fee per gas or the
total fee the RPC endpoint fills in at signing is over its bound in
`assertWithinCeilings()` (`src/shared/approvalVerify.js`) → nothing is
signed; "The gas limit or fee the RPC endpoint quoted for this transaction
is more than this wallet will sign for, so it was not sent. Please go back
and try again." in its own box, the same sentence for all three, no screen
change
- "Sign & Send" (wrong password) → "Wrong password." on the password error
line, no screen change
- "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"
class="mb-2 border border-border border-dashed p-2 text-xs invisible"
></div>
<!-- Shown when Sign & Send refuses a send for its network, or
for its gas limit or 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"
>
The gas limit or fee the RPC endpoint quoted for this
transaction is more than this wallet will sign for, so it
was not sent. Please go back and try again.
</div>
<div class="mb-2">
<label class="block mb-1 text-xs">Password</label>
<input
+29 -30
View File
@@ -291,9 +291,12 @@ function show(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 = "";
hideError("confirm-tx-password-error");
setVisible("confirm-network-changed-error", false);
setVisible("confirm-fee-bound-error", false);
// Gas estimate — show placeholder then fetch async
$("confirm-fee").style.visibility = "visible";
@@ -556,16 +559,17 @@ async function estimateGas(txInfo) {
}
}
// Populate the transaction this send describes, enforce the fee bound against
// the fees that were actually filled in, then sign and broadcast it. Apart
// from a max ETH send, which passes its estimate's fee fields as `fees`, the
// send pins no fee fields, so ethers fills maxFeePerGas and the gas limit from
// what the configured RPC node answers, with nothing otherwise bounding what a
// hostile node can set — the dApp path's ceilings never reached this one.
// Populating before the check is what makes assertWithinCeilings() see the
// same numbers that would be signed; it throws an ApprovalMismatchError when
// the product gasLimit × maxFeePerGas is over the bound, which the caller
// shows in the reserved error area rather than sending.
// Populate the transaction this send describes, enforce the bounds against the
// gas limit and fees that were actually filled in, then sign and broadcast it.
// Apart from a max ETH send, which passes its estimate's fee fields as `fees`,
// the send pins no fee fields, so ethers fills maxFeePerGas and the gas limit
// from what the configured RPC node answers, with nothing otherwise bounding
// what a hostile node can set — the dApp path's ceilings never reached this
// one. Populating before the check is what makes assertWithinCeilings() see
// the same numbers that would be signed; it throws an ApprovalMismatchError
// when the gas limit, a fee per gas, or the product gasLimit × maxFeePerGas is
// over its bound, and the caller then shows the one refusal reserved for all
// three rather than sending.
async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
let request;
if (tx.token === "ETH") {
@@ -593,15 +597,6 @@ async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
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) {
try {
const provider = reviewedNetworkProvider(txInfo);
@@ -640,6 +635,10 @@ function init(_ctx) {
onViewLeave("confirm-tx", clearPassword);
$("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];
// Every Send button refuses a defective wallet before this screen,
// but the popup also reopens onto it from a saved view.
@@ -654,11 +653,7 @@ function init(_ctx) {
// Settings after a switch draws this screen again from the saved
// transaction, as a popup reopened onto it does.
if (currentNetwork().chainId !== pendingTx.chainId) {
showSendError(
"The active network changed after this transaction was" +
" reviewed, so it was not sent. Please go back and review" +
" it again.",
);
setVisible("confirm-network-changed-error", true);
return;
}
@@ -722,12 +717,16 @@ function init(_ctx) {
txStatus.showWait(pendingTx, tx.hash);
} catch (e) {
decryptedSecret = null;
// A fee over the bound is refused before anything is broadcast, so
// there is no transaction that may have reached the network to warn
// about: the message stays on the confirmation screen where the
// user can go back, rather than routing to the sent/failed screen.
// A gas limit or fee over its bound is refused before anything is
// broadcast, so there is no transaction that may have reached the
// network to warn about: the refusal stays on the confirmation
// screen where the 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, naming the bound, is logged.
if (e && e.approvalMismatch) {
showSendError(e.message);
log.errorf("send refused:", e.message);
setVisible("confirm-fee-bound-error", true);
return;
}
const hash = tx ? tx.hash : null;
+167 -6
View File
@@ -11,20 +11,115 @@
//
// The check is driven here with a fake connected signer rather than a real
// one: populateTransaction() returns the fees the node would have produced,
// and sendTransaction() records whether the send actually happened. The real
// DOM path around it — reading the fee error into the reserved errors box — is
// covered by the Chrome e2e suite.
// and sendTransaction() records whether the send actually happened.
//
// The last test drives the real confirmation screen instead, with only the
// node and the DOM stubbed: a gas limit over its own bound at an ordinary fee
// is refused in the same box as a fee over the bound
// (https://git.eeqj.de/sneak/AutistMask/issues/583). That the box moves
// nothing on the screen is checked by the Chrome e2e suite.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
// What the stub node quotes for that test: a gas limit of 150,000,000 at a
// maximum fee of 2 gwei per gas, 0.3 ETH in all. And the signed transactions
// it was handed.
const mockNode = {
gasLimit: 150000000n,
maxFeePerGas: 2000000000n,
broadcast: [],
};
jest.mock("ethers", () => {
const actual = jest.requireActual("ethers");
class StubProvider {
constructor(url, network) {
this.network = network;
}
async getNetwork() {
return this.network;
}
async getBalance() {
return actual.parseEther("5");
}
async getCode() {
return "0x";
}
async getTransactionCount() {
return 1;
}
async getFeeData() {
return {
maxFeePerGas: mockNode.maxFeePerGas,
maxPriorityFeePerGas: 1000000000n,
gasPrice: 1000000000n,
};
}
async estimateGas() {
return mockNode.gasLimit;
}
async broadcastTransaction(signed) {
mockNode.broadcast.push(signed);
return { hash: "0x" + "ab".repeat(32) };
}
}
return { ...actual, JsonRpcProvider: StubProvider };
});
// The screens a send that was not refused goes on to.
jest.mock("../src/popup/views/txStatus", () => ({
showWait: jest.fn(),
showError: jest.fn(),
}));
const { makeStorageStub } = require("./support/storageStub");
global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
const { populateVerifyAndSend } = require("../src/popup/views/confirmTx");
// A stub DOM: every id resolves to a recording element.
const elements = new Map();
function makeEl(id) {
const handlers = new Map();
return {
id,
textContent: "",
innerHTML: "",
value: "",
disabled: false,
style: {},
dataset: {},
classList: {
add() {},
remove() {},
toggle() {},
contains: () => false,
},
handlers,
addEventListener(name, fn) {
handlers.set(name, fn);
},
querySelectorAll: () => [],
remove() {},
};
}
global.document = {
getElementById(id) {
if (!elements.has(id)) elements.set(id, makeEl(id));
return elements.get(id);
},
};
const { Wallet } = require("ethers");
const { encryptWithPassword } = require("../src/shared/vault");
const { state, loadState } = require("../src/shared/state");
const txStatus = require("../src/popup/views/txStatus");
const confirmTx = require("../src/popup/views/confirmTx");
const { populateVerifyAndSend } = confirmTx;
const {
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify");
@@ -98,3 +193,69 @@ describe("populateVerifyAndSend enforces the combined fee bound", () => {
expect(signer.sent[0].gasLimit).toBe(under.gasLimit);
});
});
describe("Sign & Send on the confirmation screen", () => {
const PRIVATE_KEY = "0x" + "11".repeat(32);
const PASSWORD = "correct horse battery staple";
let encryptedSecret;
beforeAll(async () => {
await loadState();
encryptedSecret = await encryptWithPassword(PRIVATE_KEY, PASSWORD);
});
function el(id) {
return global.document.getElementById(id);
}
function shown(id) {
return el(id).style.visibility === "visible";
}
// show() reads the balance and estimates the fee without awaiting
// either; this lets both finish.
async function settle() {
for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
}
test("refuses a gas limit over its bound at an ordinary fee in the box for a refused gas limit or fee, and sends nothing", async () => {
expect(mockNode.gasLimit).toBeGreaterThan(MAX_GAS_LIMIT);
expect(mockNode.gasLimit * mockNode.maxFeePerGas).toBeLessThan(
MAX_TOTAL_FEE,
);
const from = new Wallet(PRIVATE_KEY).address;
state.wallets = [
{
type: "key",
name: "Wallet 1",
encryptedSecret,
addresses: [{ address: from }],
},
];
state.selectedWallet = 0;
state.selectedAddress = 0;
confirmTx.init({});
confirmTx.show({
from,
to: RECIPIENT,
ensName: null,
amount: "1",
token: "ETH",
tokenSymbol: null,
tokenDecimals: null,
max: false,
});
await settle();
expect(el("btn-confirm-send").disabled).toBe(false);
expect(shown("confirm-fee-bound-error")).toBe(false);
el("confirm-tx-password").value = PASSWORD;
await el("btn-confirm-send").handlers.get("click")();
expect(shown("confirm-fee-bound-error")).toBe(true);
expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled();
});
});
+5 -18
View File
@@ -440,12 +440,8 @@ describe.each([
expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled();
expect(el("confirm-errors").style.visibility).toBe("visible");
expect(el("confirm-errors").innerHTML).toContain(
"The active network changed after this transaction was" +
" reviewed, so it was not sent. Please go back and review" +
" it again.",
);
expect(shown("confirm-network-changed-error")).toBe(true);
expect(shown("confirm-fee-bound-error")).toBe(false);
});
},
);
@@ -497,10 +493,7 @@ describe.each([
expect(mockNode.broadcast).toEqual([]);
expect(txStatus.showWait).not.toHaveBeenCalled();
expect(txStatus.showError).not.toHaveBeenCalled();
expect(el("confirm-errors").innerHTML).toContain(
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
expect(shown("confirm-network-changed-error")).toBe(true);
});
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(mockNode.broadcast).toEqual([]);
expect(errors()).toContain(
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
expect(shown("confirm-network-changed-error")).toBe(true);
});
const tokenSend = {
@@ -629,10 +619,7 @@ describe.each([
expect(signTransaction).not.toHaveBeenCalled();
expect(mockNode.broadcast).toEqual([]);
expect(errors()).toContain(
"The active network changed after this transaction was" +
" reviewed, so it was not sent.",
);
expect(shown("confirm-network-changed-error")).toBe(true);
});
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
// 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
// stop being able to distinguish them.
function latestBlock() {
// stop being able to distinguish them. opts.baseFeeWei replaces its base fee,
// read at request time like every other fixture switch.
function latestBlock(opts) {
return {
hash: "0x" + "11".repeat(32),
parentHash: "0x" + "22".repeat(32),
@@ -239,7 +240,7 @@ function latestBlock() {
gasUsed: "0xf4240",
miner: STUB_COUNTERPARTY,
extraData: "0x",
baseFeePerGas: hex(BASE_FEE_WEI),
baseFeePerGas: opts.baseFeeWei || hex(BASE_FEE_WEI),
transactions: [],
};
}
@@ -481,7 +482,7 @@ function rpcReply(req, opts, report) {
});
}
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 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;
* defaults to zero, which is what every test that predates the funded
* 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
* node-side refusal.
* @param {boolean} [opts.holdBalance] hold every batch containing an
+96 -1
View File
@@ -2232,7 +2232,8 @@ function feeEth(wei) {
// 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
// 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) {
return page.evaluate(() => {
const el = (id) => document.getElementById(id);
@@ -2245,6 +2246,9 @@ async function confirmState(page) {
};
return {
height: el("view-confirm-tx").getBoundingClientRect().height,
passwordTop:
el("confirm-tx-password").getBoundingClientRect().top -
el("view-confirm-tx").getBoundingClientRect().top,
sendTop:
el("btn-confirm-send").getBoundingClientRect().top -
el("view-confirm-tx").getBoundingClientRect().top,
@@ -2262,6 +2266,12 @@ async function confirmState(page) {
amountFeeError: shown("confirm-amount-fee-error"),
gasError: shown("confirm-gas-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,
};
});
@@ -3057,6 +3067,91 @@ 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 gas limit
// or fee is over its bound, and each refusal's sentence sits in a box that held
// its space from the first paint. This drives the total fee refusal, the longer
// of the two sentences.
// 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 ===
"The gas limit or fee the RPC endpoint quoted for this" +
" transaction is 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)
//
// 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";
await boot.click("btn-confirm-send");
expect(boot.visibleViews()).toEqual(["confirm-tx"]);
expect(boot.node("confirm-errors").innerHTML).toContain(
"The active network changed after this transaction was reviewed",
);
expect(
boot.node("confirm-network-changed-error").style.visibility,
).toBe("visible");
// Refused before the password is even tried.
expect(boot.text("confirm-tx-password-error")).toBe("");
});