fix: a send the confirmation screen refuses moves nothing on it (closes #583)
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
This commit is contained in:
@@ -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 network 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,12 @@ 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 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
|
||||
line, no screen change
|
||||
- "Sign & Send" signs as the From address above, not as whichever address is
|
||||
|
||||
@@ -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
|
||||
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">
|
||||
<label class="block mb-1 text-xs">Password</label>
|
||||
<input
|
||||
|
||||
@@ -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";
|
||||
@@ -564,8 +567,8 @@ async function estimateGas(txInfo) {
|
||||
// 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.
|
||||
// the product gasLimit × maxFeePerGas is over the bound, and the caller then
|
||||
// shows the refusal reserved for it rather than sending.
|
||||
async function populateVerifyAndSend(connectedSigner, tx, fees = null) {
|
||||
let request;
|
||||
if (tx.token === "ETH") {
|
||||
@@ -593,15 +596,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 +634,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 +652,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;
|
||||
}
|
||||
|
||||
@@ -724,10 +718,13 @@ function init(_ctx) {
|
||||
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
|
||||
// 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 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;
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
// 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
|
||||
// DOM path around it — the refusal shown in the box reserved for it — is
|
||||
// covered by the Chrome e2e suite.
|
||||
|
||||
globalThis.chrome = {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
+95
-1
@@ -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,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)
|
||||
//
|
||||
// README.md (WaitTx) documents two ways the wait ends on the error screen: a
|
||||
|
||||
@@ -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("");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user