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 is contained in:
@@ -609,6 +609,9 @@ async function checkRecipientHistory(txInfo) {
|
|||||||
fromAddress: txInfo.from,
|
fromAddress: txInfo.from,
|
||||||
chainId: txInfo.chainId,
|
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) {
|
for (const w of asyncWarnings) {
|
||||||
if (w.type === "contract") {
|
if (w.type === "contract") {
|
||||||
$("confirm-contract-warning").style.visibility = "visible";
|
$("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