fix: a recipient warning on the confirmation screen is about the recipient on screen (closes #575)
checkRecipientHistory() showed the warning boxes whenever its lookups answered, so after Back and a review to another recipient, a late answer about the first recipient could show the never-sent-a-transaction, contract or Etherscan phishing box on the second recipient's screen. It now drops a result for a transaction no longer on screen, as estimateGas() and readBalances() already do. tests/confirmTxLateResult.test.js holds back the first screen's answer until the second screen is shown, for each of the three boxes, the balance read and the fee estimate. Model: opus-5-5
This commit was merged in pull request #605.
This commit is contained in:
@@ -609,6 +609,9 @@ async function checkRecipientHistory(txInfo) {
|
||||
fromAddress: txInfo.from,
|
||||
chainId: txInfo.chainId,
|
||||
});
|
||||
// The user may have left this transaction while the checks were in
|
||||
// flight; its warnings must not reach another recipient's screen.
|
||||
if (pendingTx !== txInfo) return;
|
||||
for (const w of asyncWarnings) {
|
||||
if (w.type === "contract") {
|
||||
$("confirm-contract-warning").style.visibility = "visible";
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
// A late answer for a confirmation screen the user has left changes nothing on
|
||||
// the screen shown since (https://git.eeqj.de/sneak/AutistMask/issues/575).
|
||||
//
|
||||
// The confirmation screen asks the node and the explorer about its transaction
|
||||
// without waiting for the answers: the recipient checks, which can show three
|
||||
// warning boxes, the balance read and the fee estimate. Back, a new recipient
|
||||
// and Review again show the screen for another transaction while those
|
||||
// questions may still be open. The recipient checks showed their boxes
|
||||
// whenever they answered, so a box about the first recipient could appear on
|
||||
// the second recipient's screen.
|
||||
//
|
||||
// Driven through the real confirmation screen, with only the node, the
|
||||
// explorer and the DOM stubbed. Each test holds back the first screen's answer
|
||||
// to one question, and gives it once the screen it tests is shown.
|
||||
|
||||
"use strict";
|
||||
|
||||
// The From address of every transaction here. The jest.mock() factory below
|
||||
// may only name outside variables whose names start with "mock".
|
||||
const mockFrom = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
|
||||
|
||||
jest.mock("ethers", () => {
|
||||
const actual = jest.requireActual("ethers");
|
||||
// Every answer is a mock, so a test can hold one back. Every address has
|
||||
// sent one transaction and has no code, and only the From address holds
|
||||
// anything: 5 ETH.
|
||||
class StubProvider {}
|
||||
Object.assign(StubProvider.prototype, {
|
||||
getBalance: jest.fn(async (address) =>
|
||||
address === mockFrom ? actual.parseEther("5") : 0n,
|
||||
),
|
||||
getCode: jest.fn(async () => "0x"),
|
||||
getTransactionCount: jest.fn(async () => 1),
|
||||
// 10 gwei expected, 20 gwei reserved per gas.
|
||||
getFeeData: jest.fn(async () => ({
|
||||
maxFeePerGas: 20000000000n,
|
||||
maxPriorityFeePerGas: 1000000000n,
|
||||
gasPrice: 10000000000n,
|
||||
})),
|
||||
estimateGas: jest.fn(async () => 21000n),
|
||||
});
|
||||
return { ...actual, JsonRpcProvider: StubProvider };
|
||||
});
|
||||
|
||||
// An explorer address page whose title is `title`.
|
||||
function explorerPage(title) {
|
||||
return { ok: true, text: async () => "<title>" + title + "</title>" };
|
||||
}
|
||||
|
||||
// The explorer labels no address.
|
||||
global.fetch = jest.fn(async () => explorerPage("Etherscan"));
|
||||
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
|
||||
|
||||
// A stub DOM: every id resolves to a recording element.
|
||||
const elements = new Map();
|
||||
|
||||
function makeEl(id) {
|
||||
return {
|
||||
id,
|
||||
textContent: "",
|
||||
innerHTML: "",
|
||||
value: "",
|
||||
disabled: false,
|
||||
style: {},
|
||||
dataset: {},
|
||||
classList: {
|
||||
add() {},
|
||||
remove() {},
|
||||
toggle() {},
|
||||
contains: () => false,
|
||||
},
|
||||
addEventListener() {},
|
||||
querySelectorAll: () => [],
|
||||
remove() {},
|
||||
};
|
||||
}
|
||||
|
||||
global.document = {
|
||||
getElementById(id) {
|
||||
if (!elements.has(id)) elements.set(id, makeEl(id));
|
||||
return elements.get(id);
|
||||
},
|
||||
};
|
||||
|
||||
const { JsonRpcProvider, parseEther } = require("ethers");
|
||||
const { loadState } = require("../src/shared/state");
|
||||
const confirmTx = require("../src/popup/views/confirmTx");
|
||||
|
||||
const node = JsonRpcProvider.prototype;
|
||||
const FIRST = "0xC0FfEE0000000000000000000000000000c0fFEe";
|
||||
const SECOND = "0xE2E0000000000000000000000000000000000E2e";
|
||||
|
||||
function el(id) {
|
||||
return global.document.getElementById(id);
|
||||
}
|
||||
|
||||
function shown(id) {
|
||||
return el(id).style.visibility === "visible";
|
||||
}
|
||||
|
||||
// Show the confirmation screen for 1 ETH to `to` on mainnet, as Review on the
|
||||
// Send screen does.
|
||||
function review(to) {
|
||||
confirmTx.show({
|
||||
from: mockFrom,
|
||||
to,
|
||||
ensName: null,
|
||||
amount: "1",
|
||||
token: "ETH",
|
||||
tokenSymbol: null,
|
||||
tokenDecimals: null,
|
||||
max: false,
|
||||
});
|
||||
}
|
||||
|
||||
// show() does not wait for its questions; this lets every answer not held
|
||||
// back arrive.
|
||||
async function settle() {
|
||||
for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
|
||||
}
|
||||
|
||||
// Hold back the next answer `question` gives. Returns the function that gives
|
||||
// it.
|
||||
function holdNextAnswer(question) {
|
||||
let give;
|
||||
question.mockImplementationOnce(() => new Promise((r) => (give = r)));
|
||||
return (answer) => give(answer);
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
await loadState();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
elements.clear();
|
||||
});
|
||||
|
||||
// The three boxes the recipient checks show, each with the question whose
|
||||
// answer shows it for FIRST, and that answer. FIRST holds no ETH, so the
|
||||
// answer that it has sent no transaction is what shows the box saying so.
|
||||
describe.each([
|
||||
{
|
||||
box: "confirm-contract-warning",
|
||||
question: node.getCode,
|
||||
answer: "0x6080604052",
|
||||
},
|
||||
{
|
||||
box: "confirm-recipient-warning",
|
||||
question: node.getTransactionCount,
|
||||
answer: 0,
|
||||
},
|
||||
{
|
||||
box: "confirm-etherscan-warning",
|
||||
question: global.fetch,
|
||||
answer: explorerPage(
|
||||
"Fake_Phishing1 | Address: " + FIRST + " | Etherscan",
|
||||
),
|
||||
},
|
||||
])("the $box box", ({ box, question, answer }) => {
|
||||
test("is shown when the answer about the recipient arrives while its screen is shown", async () => {
|
||||
const give = holdNextAnswer(question);
|
||||
review(FIRST);
|
||||
await settle();
|
||||
expect(shown(box)).toBe(false);
|
||||
|
||||
give(answer);
|
||||
await settle();
|
||||
expect(shown(box)).toBe(true);
|
||||
});
|
||||
|
||||
test("is not shown on the next recipient's screen when the answer about the first arrives after that screen is shown", async () => {
|
||||
const give = holdNextAnswer(question);
|
||||
review(FIRST);
|
||||
await settle();
|
||||
review(SECOND);
|
||||
await settle();
|
||||
expect(shown(box)).toBe(false);
|
||||
|
||||
give(answer);
|
||||
await settle();
|
||||
expect(shown(box)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
test("a balance read answered after the next screen is shown leaves that screen's balance and balance check as they are", async () => {
|
||||
const give = holdNextAnswer(node.getBalance);
|
||||
review(FIRST);
|
||||
await settle();
|
||||
review(SECOND);
|
||||
await settle();
|
||||
expect(el("confirm-balance").textContent).toBe("5.0000 ETH");
|
||||
|
||||
give(parseEther("0.5"));
|
||||
await settle();
|
||||
expect(el("confirm-balance").textContent).toBe("5.0000 ETH");
|
||||
expect(shown("confirm-balance-error")).toBe(false);
|
||||
expect(el("btn-confirm-send").disabled).toBe(false);
|
||||
});
|
||||
|
||||
test("a fee estimate answered after the next screen is shown leaves that screen's fee as it is", async () => {
|
||||
const give = holdNextAnswer(node.getFeeData);
|
||||
review(FIRST);
|
||||
await settle();
|
||||
review(SECOND);
|
||||
await settle();
|
||||
expect(el("confirm-fee-amount").textContent).toBe("~0.0002 ETH");
|
||||
|
||||
// 100 times the fee the second screen was quoted.
|
||||
give({
|
||||
maxFeePerGas: 2000000000000n,
|
||||
maxPriorityFeePerGas: 100000000000n,
|
||||
gasPrice: 1000000000000n,
|
||||
});
|
||||
await settle();
|
||||
expect(el("confirm-fee-amount").textContent).toBe("~0.0002 ETH");
|
||||
expect(el("confirm-fee-reserve").textContent).toBe(
|
||||
"up to 0.0004 ETH reserved",
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user