Compare commits

..

1 Commits

Author SHA1 Message Date
d32ffe7c3a fix: settle a site approval on the port that carries its teardown (closes #275)
All checks were successful
check / check (push) Successful in 30s
Approve and window.close() left the popup on the next line, and the decision
and the disconnect the close caused travelled independent channels with nothing
ordering them. The disconnect handler settled a pending site approval as a
rejection, so whichever landed first decided the outcome. Driven in a tab the
teardown won every time: the user allowed the connection and the dApp was told
they had refused.

The decision now goes out on the approval port the popup already opens, which
is the same port the close disconnects. One channel is ordered -- a message
posted on a port is delivered before that port's own disconnect -- so the
approval is settled before the teardown is even seen, and the disconnect then
finds nothing pending to reject. Nothing waits, nothing is timed, and the popup
closes exactly as immediately as before.

windows.onRemoved no longer decides a site approval whose port is connected
either. In the fallback-window shape that event races the decision on a channel
of its own, which is the same defect one level over; the port disconnect says
the same thing in a defined order, so it is left to say it. A window that
closes before its popup ever connected has nothing else to speak for it and is
still rejected there, so no dApp is left waiting on a window that is gone.

Rejecting reports a rejection, and so does closing without deciding, in both
shapes. AUTISTMASK_APPROVAL_RESPONSE is gone; the port name carries the
approval id, so the popup no longer names one, and the sender check the message
carried moved to the port.

tests/backgroundApproval.test.js drives decide-then-disconnect with nothing
awaited in between, in the toolbar-popup shape that production uses and in the
fallback-window shape, and asserts every close-without-deciding path still
rejects. tests/e2e/run.js drops the deferred-window.close() accommodation it
carried for this bug, so the two site-prompt tests now drive the shipped
decide-then-close in a real Chromium.
2026-08-14 04:22:16 +00:00
6 changed files with 381 additions and 414 deletions

28
TODO.md
View File

@@ -45,22 +45,18 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-14: The parts of the - 2026-08-14: Approving a site connection is no longer a race against the popup
[#150](https://git.eeqj.de/sneak/AutistMask/issues/150) and closing. The decision now rides the approval port the popup already holds,
[#151](https://git.eeqj.de/sneak/AutistMask/issues/151) definition of done the which is the same channel the close disconnects, so it is delivered ahead of
e2e suite did not cover are asserted. It had only shown that the two screens that disconnect however fast the teardown is; `windows.onRemoved` no longer
open without throwing. Now: the Add Token round trip leaves the navigation decides a site approval whose port is connected, since that event is ordered
stack exactly as it found it, read out of extension storage rather than against nothing either. Rejecting and closing without deciding both still
inferred from which screen is up, so an orphaned entry — the second-order report a rejection, and the popup delays its own close by nothing. The e2e
damage of #150 — is caught where it happens rather than one Back press later; harness's deferred-`window.close()` accommodation is gone with it, so the two
a common-token quick-pick puts its contract address in the field; the native site-prompt tests now drive the shipped decide-then-close in a real Chromium;
ETH detail path renders with its own type, value and raw quantity and with the against the unfixed code the approval came back to the page as
token contract row still hidden, against a new `seedNativeTransfer` fixture, `{"settled":"rejected","code":4001}`
since the normal-transactions endpoint answered `[]` unconditionally and there ([#275](https://git.eeqj.de/sneak/AutistMask/issues/275)).
was no non-ERC-20 row to open; and tapping the token contract address puts it
on the real clipboard, read back after a sentinel write. Each of the four was
demonstrated failing against a deliberately broken build
([#188](https://git.eeqj.de/sneak/AutistMask/issues/188)).
- 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js` - 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js`
rebuilt every failure as `new Error(error.message)`, so the code the rebuilt every failure as `new Error(error.message)`, so the code the
background produced and the content script relayed intact was dropped in the background produced and the content script relayed intact was dropped in the

View File

@@ -279,13 +279,50 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
}); });
} }
// Detect when an approval popup (browser-action) closes without a response. // Anything only the extension's own pages may say. A content script speaks
// TX and sign approvals now use windows.create() and are handled by the // with the page's URL, so this is what separates the popup from the site the
// windowsApi.onRemoved listener below, but we still handle site-connection // popup is being asked about.
// approval disconnects here. function isExtensionSender(sender) {
const extUrl = runtime.getURL("");
return !!(sender && sender.url && sender.url.startsWith(extUrl));
}
// The approval popup's port: it carries the user's decision on a
// site-connection approval, and its disconnect is how that approval learns the
// popup closed without one.
//
// The decision travels this port rather than a one-off runtime.sendMessage()
// for exactly one reason: the port is also what the popup's window.close()
// disconnects. A message posted on a port is delivered before that port's
// disconnect, so approve-then-close settles as an approval no matter how fast
// the teardown is. Sent as a one-off message the two crossed on independent
// channels with nothing ordering them, and the teardown won every time when
// the prompt was driven in a tab: the user approved and the dApp was told they
// had refused.
//
// TX and sign approvals do not decide here. They stay pending across a
// disconnect — the user can reopen the toolbar popup — and are rejected by the
// windowsApi.onRemoved listener below.
runtime.onConnect.addListener((port) => { runtime.onConnect.addListener((port) => {
if (port.name.startsWith("approval:")) { if (port.name.startsWith("approval:")) {
const id = port.name.split(":")[1]; const id = port.name.split(":")[1];
if (pendingApprovals[id]) {
// This approval has a popup that can speak for it, so its
// disconnect is a trustworthy "closed"; see onRemoved below.
pendingApprovals[id].portConnected = true;
}
port.onMessage.addListener((msg) => {
if (!msg || msg.type !== "AUTISTMASK_APPROVAL_DECISION") return;
if (!isExtensionSender(port.sender)) return;
const approval = pendingApprovals[id];
if (!approval || approval.type === "tx" || approval.type === "sign")
return;
settleApproval(id, {
approved: !!msg.approved,
remember: !!msg.remember,
});
resetPopupUrl();
});
port.onDisconnect.addListener(() => { port.onDisconnect.addListener(() => {
const approval = pendingApprovals[id]; const approval = pendingApprovals[id];
if (approval) { if (approval) {
@@ -832,20 +869,32 @@ startBackgroundJobs();
// window is an ordinary event with an attempt already in flight behind it. // window is an ordinary event with an attempt already in flight behind it.
// settleApproval() refuses those, which leaves the attempt to report its real // settleApproval() refuses those, which leaves the attempt to report its real
// outcome to the page. // outcome to the page.
//
// A site-connection approval whose popup connected its port is not decided
// here. That popup approves and closes in the same breath, and this event
// races the decision on a channel of its own — the same race the port exists
// to end. Its port disconnect says the same thing this event does, in an order
// that is defined, so the disconnect is left to say it. The window closing
// before any port connected is the one case with nothing else to speak for it,
// and is rejected here so the dApp is not left waiting on a window that is
// gone.
if (windowsApi && windowsApi.onRemoved) { if (windowsApi && windowsApi.onRemoved) {
windowsApi.onRemoved.addListener((windowId) => { windowsApi.onRemoved.addListener((windowId) => {
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.windowId !== windowId) continue; if (approval.windowId !== windowId) continue;
const rejection = const isSite = approval.type !== "tx" && approval.type !== "sign";
approval.type === "tx" || approval.type === "sign" if (isSite && approval.portConnected) continue;
? { settleApproval(
id,
isSite
? { approved: false, remember: false }
: {
error: { error: {
code: 4001, code: 4001,
message: "User rejected the request.", message: "User rejected the request.",
}, },
} },
: { approved: false, remember: false }; );
settleApproval(id, rejection);
} }
}); });
} }
@@ -872,18 +921,16 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
} }
// Validate that popup-only messages originate from the extension itself. // Validate that popup-only messages originate from the extension itself.
// The site-connection decision is not here: it is a port message, and it
// is checked the same way where the port is served.
const POPUP_ONLY_TYPES = [ const POPUP_ONLY_TYPES = [
"AUTISTMASK_GET_APPROVAL", "AUTISTMASK_GET_APPROVAL",
"AUTISTMASK_APPROVAL_RESPONSE",
"AUTISTMASK_TX_RESPONSE", "AUTISTMASK_TX_RESPONSE",
"AUTISTMASK_SIGN_RESPONSE", "AUTISTMASK_SIGN_RESPONSE",
]; ];
if (POPUP_ONLY_TYPES.includes(msg.type)) { if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
const extUrl = runtime.getURL(""); sendResponse({ error: "Unauthorized sender" });
if (!sender.url || !sender.url.startsWith(extUrl)) { return false;
sendResponse({ error: "Unauthorized sender" });
return false;
}
} }
if (msg.type === "AUTISTMASK_GET_APPROVAL") { if (msg.type === "AUTISTMASK_GET_APPROVAL") {
@@ -915,15 +962,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
return false; return false;
} }
if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") {
settleApproval(msg.id, {
approved: msg.approved,
remember: msg.remember,
});
resetPopupUrl();
return false;
}
if (msg.type === "AUTISTMASK_TX_RESPONSE") { if (msg.type === "AUTISTMASK_TX_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;

View File

@@ -441,7 +441,7 @@ function showSignApproval(details) {
function show(id) { function show(id) {
approvalId = id; approvalId = id;
runtime.connect({ name: "approval:" + id }); approvalPort = runtime.connect({ name: "approval:" + id });
runtime.sendMessage({ type: "AUTISTMASK_GET_APPROVAL", id }, (details) => { runtime.sendMessage({ type: "AUTISTMASK_GET_APPROVAL", id }, (details) => {
if (!details) { if (!details) {
window.close(); window.close();
@@ -470,6 +470,14 @@ function show(id) {
} }
let approvalId = null; let approvalId = null;
// The port this approval was opened on. Closing this window disconnects it,
// and the background treats that disconnect as "closed without deciding" for a
// site connection — so the decision goes out on this same port and not as a
// one-off message. One channel is ordered: a message posted on it is delivered
// before its own disconnect, however immediately the close follows. Two
// channels were not, and the close won, reporting a user who approved as
// having refused.
let approvalPort = null;
let pendingTxDetails = null; let pendingTxDetails = null;
// The exact objects shown to the user, kept so the popup signs what it // The exact objects shown to the user, kept so the popup signs what it
// displayed rather than re-fetching or re-populating anything at approval // displayed rather than re-fetching or re-populating anything at approval
@@ -537,6 +545,20 @@ function clearSignPassword() {
hideError("approve-sign-error"); hideError("approve-sign-error");
} }
// Answer a site-connection approval and close. The decision goes out on the
// approval port — see approvalPort above for why — and carries no approval id,
// because the port name already names the approval the background will settle.
function decideSite(approved) {
if (approvalPort) {
approvalPort.postMessage({
type: "AUTISTMASK_APPROVAL_DECISION",
approved,
remember: $("approve-remember").checked,
});
}
window.close();
}
function init(ctx) { function init(ctx) {
onViewLeave("approve-tx", clearTxPassword); onViewLeave("approve-tx", clearTxPassword);
onViewLeave("approve-sign", clearSignPassword); onViewLeave("approve-sign", clearSignPassword);
@@ -547,25 +569,11 @@ function init(ctx) {
}); });
$("btn-approve").addEventListener("click", () => { $("btn-approve").addEventListener("click", () => {
const remember = $("approve-remember").checked; decideSite(true);
runtime.sendMessage({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: true,
remember,
});
window.close();
}); });
$("btn-reject").addEventListener("click", () => { $("btn-reject").addEventListener("click", () => {
const remember = $("approve-remember").checked; decideSite(false);
runtime.sendMessage({
type: "AUTISTMASK_APPROVAL_RESPONSE",
id: approvalId,
approved: false,
remember,
});
window.close();
}); });
$("btn-approve-tx").addEventListener("click", async () => { $("btn-approve-tx").addEventListener("click", async () => {

View File

@@ -32,6 +32,21 @@ const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example"; const HOSTNAME = "dapp.example";
const EXT_URL = "chrome-extension://autistmask/"; const EXT_URL = "chrome-extension://autistmask/";
// An origin the persisted state has never allowed, so asking to connect from
// it raises a prompt rather than being answered from allowedSites.
const FRESH_ORIGIN = "https://fresh.example";
// The approval id in the most recent popup URL of a list, or null when none
// of them carries one. Takes both shapes: the absolute URL windows.create()
// is given and the extension-relative one action.setPopup() is given.
function approvalIdIn(urls) {
for (let i = urls.length - 1; i >= 0; i--) {
if (!urls[i] || !urls[i].includes("?approval=")) continue;
return new URL(urls[i], EXT_URL).searchParams.get("approval");
}
return null;
}
// What the dApp asks for: no nonce, no gas, no fees. This is the shape that // What the dApp asks for: no nonce, no gas, no fees. This is the shape that
// makes a duplicate broadcast possible at all. // makes a duplicate broadcast possible at all.
const TX_PARAMS = { const TX_PARAMS = {
@@ -146,8 +161,13 @@ function loadBackground(options) {
let messageListener = null; let messageListener = null;
let windowRemovedListener = null; let windowRemovedListener = null;
let connectListener = null;
const created = []; const created = [];
const removed = []; const removed = [];
// Every URL the background put on the browser action. A site approval
// raised through action.openPopup() opens no window at all, so this is
// the only place its id appears.
const actionPopups = [];
global.chrome = { global.chrome = {
storage: { storage: {
@@ -163,7 +183,14 @@ function loadBackground(options) {
messageListener = fn; messageListener = fn;
}, },
}, },
onConnect: { addListener: () => {} }, // Captured, not swallowed: the approval port is what carries a
// site connection's decision and the popup teardown that races
// it, so a no-op stub here hides the whole subject of #275.
onConnect: {
addListener: (fn) => {
connectListener = fn;
},
},
lastError: null, lastError: null,
}, },
windows: { windows: {
@@ -189,7 +216,17 @@ function loadBackground(options) {
query: (q, cb) => cb([]), query: (q, cb) => cb([]),
sendMessage: () => {}, sendMessage: () => {},
}, },
action: { setPopup: () => {} }, action: {
setPopup: (o) => {
actionPopups.push(o.popup);
},
// The production route for a site connection. Present only when
// a test asks for it, because with it the prompt is the toolbar
// popup: no window is created, so windows.onRemoved can never
// fire for it and the port disconnect is the only close signal
// that exists.
...(opts.actionPopup ? { openPopup: () => Promise.resolve() } : {}),
},
}; };
require("../src/background/index"); require("../src/background/index");
@@ -248,6 +285,70 @@ function loadBackground(options) {
}; };
} }
// A dApp asking to connect. The origin defaults to one the persisted
// state has never allowed, so the request really does raise a prompt
// instead of being answered from allowedSites.
function requestSite(origin) {
let rpcResult = null;
messageListener(
{
type: "AUTISTMASK_RPC",
method: "eth_requestAccounts",
params: [],
},
{ origin: origin || FRESH_ORIGIN },
(r) => {
rpcResult = r;
},
);
return {
// Wherever the prompt went: the toolbar popup URL when
// action.openPopup() carried it, the created window otherwise.
id: () =>
approvalIdIn(actionPopups) ||
approvalIdIn(created.map((c) => c.url)),
result: () => rpcResult,
};
}
// The popup's approval port, as the browser delivers it. Messages posted
// on a port and that port's disconnect travel one channel in FIFO order,
// which is exactly the property the fix rests on, so this stub delivers
// them in the order the caller emits them and never reorders them.
function connectApproval(id, senderUrl) {
const onMessage = [];
const onDisconnect = [];
const port = {
name: "approval:" + id,
sender: {
url:
senderUrl === undefined
? EXT_URL + "src/popup/index.html?approval=" + id
: senderUrl,
},
onMessage: { addListener: (fn) => onMessage.push(fn) },
onDisconnect: { addListener: (fn) => onDisconnect.push(fn) },
};
connectListener(port);
return {
decide: (approved, remember) => {
for (const fn of onMessage) {
fn(
{
type: "AUTISTMASK_APPROVAL_DECISION",
approved,
remember: !!remember,
},
port,
);
}
},
disconnect: () => {
for (const fn of onDisconnect) fn(port);
},
};
}
// The user closes the approval popup. `created` is index-aligned with the // The user closes the approval popup. `created` is index-aligned with the
// ids the window stub hands back, so window 1 is the first popup opened. // ids the window stub hands back, so window 1 is the first popup opened.
function closeWindow(windowId) { function closeWindow(windowId) {
@@ -258,6 +359,8 @@ function loadBackground(options) {
send, send,
requestTx, requestTx,
requestSign, requestSign,
requestSite,
connectApproval,
closeWindow, closeWindow,
broadcastTransaction, broadcastTransaction,
loadState, loadState,
@@ -1058,3 +1161,136 @@ describe("popup-only messages", () => {
}); });
}); });
}); });
// A site connection decided in a popup that closes on the next line.
//
// The decision and the teardown are two events the popup emits back to back,
// and the background must not be able to reach different outcomes depending on
// which of them it processes first. It cannot, because they are now one
// channel: the decision is posted on the approval port that the close then
// disconnects, so it is delivered first. Every test here therefore emits the
// close IMMEDIATELY after the decision, with nothing awaited in between —
// which is what the popup does, and what used to report a user who approved as
// having refused (#275).
describe("a site connection decided as the popup closes", () => {
// The production route: chrome.action.openPopup() put the prompt in the
// toolbar popup, which is not a window, so nothing but the port
// disconnect can tell the background this prompt is gone.
test("approving in the toolbar popup connects the site", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const id = pending.id();
expect(id).toBeTruthy();
expect(bg.created).toHaveLength(0);
const port = bg.connectApproval(id);
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
});
test("closing the toolbar popup without deciding is a rejection", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
port.disconnect();
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
test("rejecting is a rejection, and the close that follows adds nothing", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
port.decide(false, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
// The port carries a decision now, so it carries the sender check the
// one-off message used to carry. A content script that guessed an
// approval id must not be able to connect the site it is running on.
test("a decision from a page sender is ignored, and the close rejects", async () => {
const bg = loadBackground({ actionPopup: true });
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id(), FRESH_ORIGIN + "/x.html");
port.decide(true, true);
await settle();
expect(pending.result()).toBeNull();
port.disconnect();
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
// The fallback shape, where openPopup() is unavailable and the prompt is
// a window the extension opened. Closing it fires windows.onRemoved as
// well, on a channel of its own that is ordered against nothing — so the
// window event must not be allowed to decide a site approval either.
test("approving in the fallback window survives the window event too", async () => {
const bg = loadBackground();
const pending = bg.requestSite();
await settle();
expect(bg.created).toHaveLength(1);
const port = bg.connectApproval(pending.id());
port.decide(true, false);
bg.closeWindow(1);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
});
// Same shape, and the same window event arriving before the popup has
// said anything at all — which is a user closing the window rather than
// deciding, and still has to reach the dApp as a rejection.
test("closing the fallback window without deciding is a rejection", async () => {
const bg = loadBackground();
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
bg.closeWindow(1);
port.disconnect();
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
// The net under the paragraph above: a prompt whose page never got as far
// as connecting the port has no disconnect to reject it, so the window
// event has to. Otherwise the dApp waits forever on a window that is gone.
test("a window that closes before its popup ever connected still rejects", async () => {
const bg = loadBackground();
const pending = bg.requestSite();
await settle();
bg.closeWindow(1);
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
});
});

View File

@@ -46,24 +46,9 @@ const STUB_TX_HASH =
const STUB_BLOCK_NUMBER = 21000000; const STUB_BLOCK_NUMBER = 21000000;
// The native ETH transfer, seeded by opts.seedNativeTransfer. Its own hash
// and an older block, so it is a second row rather than a leg of the token
// transfer: mergeTransactions() consolidates a native entry and a token
// transfer that share a hash into one row, which would leave nothing native
// to open. 0.25 ETH clears the 100000 gwei dust threshold the default
// filters apply, so the row is not silently dropped.
const STUB_NATIVE_TX_HASH =
"0xe7e0000000000000000000000000000000000000000000000000000000000e7e";
const STUB_NATIVE_BLOCK_NUMBER = STUB_BLOCK_NUMBER - 1;
const STUB_NATIVE_VALUE_WEI = "250000000000000000";
// Fixed instant so timeAgo() output is stable across runs. // Fixed instant so timeAgo() output is stable across runs.
const STUB_TX_TIMESTAMP = "2026-01-02T03:04:05.000000Z"; const STUB_TX_TIMESTAMP = "2026-01-02T03:04:05.000000Z";
const STUB_NATIVE_TX_TIMESTAMP = "2026-01-02T02:03:04.000000Z";
// A 32-byte zero word. Returned for every eth_call, which is what makes // A 32-byte zero word. Returned for every eth_call, which is what makes
// ethers' ENS reverse lookup resolve to "no resolver set" and return null // ethers' ENS reverse lookup resolve to "no resolver set" and return null
// instead of throwing. A throw would be logged by src/shared/ens.js via // instead of throwing. A throw would be logged by src/shared/ens.js via
@@ -273,25 +258,6 @@ function tokenTransferItems(address) {
]; ];
} }
// One received native ETH transfer, in the shape src/shared/transactions.js
// parses. to.is_contract is false and there is no method, so parseTx() keeps
// it a plain transfer rather than a contract call — which is what makes the
// detail screen classify it "Native ETH Transfer" and leave the token
// contract row hidden.
function nativeTransactionItems(address) {
return [
{
hash: STUB_NATIVE_TX_HASH,
block_number: STUB_NATIVE_BLOCK_NUMBER,
timestamp: STUB_NATIVE_TX_TIMESTAMP,
from: { hash: STUB_COUNTERPARTY },
to: { hash: address, is_contract: false },
value: STUB_NATIVE_VALUE_WEI,
status: "ok",
},
];
}
// A holding of 1.5 E2E, in the shape src/shared/balances.js parses. Serving // A holding of 1.5 E2E, in the shape src/shared/balances.js parses. Serving
// this is what puts an ERC-20 in the send screen's token dropdown, which is // this is what puts an ERC-20 in the send screen's token dropdown, which is
// the only way the confirmation screen's ERC-20 path can be reached. // the only way the confirmation screen's ERC-20 path can be reached.
@@ -304,17 +270,12 @@ function tokenBalanceItems() {
]; ];
} }
// Full details for either seeded transaction — the detail screen fetches // Full details for STUB_TX_HASH. raw_input is "0x" so the calldata
// them for whichever row was opened, and an unstubbed hash would be // decoder short-circuits; the on-chain detail fields still populate.
// reported as escaping traffic. raw_input is "0x" so the calldata decoder function transactionDetails() {
// short-circuits; the on-chain detail fields still populate.
function transactionDetails(hash) {
return { return {
hash: hash, hash: STUB_TX_HASH,
block_number: block_number: STUB_BLOCK_NUMBER,
hash === STUB_NATIVE_TX_HASH
? STUB_NATIVE_BLOCK_NUMBER
: STUB_BLOCK_NUMBER,
nonce: 7, nonce: 7,
gas_used: "51000", gas_used: "51000",
gas_price: "1000000000", gas_price: "1000000000",
@@ -518,10 +479,6 @@ function traceEnabled(raw) {
* @param {boolean} [opts.seedTokenTransfer] serve the stubbed ERC-20 * @param {boolean} [opts.seedTokenTransfer] serve the stubbed ERC-20
* transfer. Read at request time, so a test can flip it on the same * transfer. Read at request time, so a test can flip it on the same
* options object without re-registering the route. * options object without re-registering the route.
* @param {boolean} [opts.seedNativeTransfer] serve the stubbed native ETH
* transfer, read at request time like seedTokenTransfer. Without it the
* normal-transactions endpoint answers with an empty list, so there is no
* non-ERC-20 row to open.
* @param {boolean} [opts.seedTokenBalance] serve the stubbed ERC-20 * @param {boolean} [opts.seedTokenBalance] serve the stubbed ERC-20
* holding, which is what makes the token reachable from the send screen. * holding, which is what makes the token reachable from the send screen.
* @param {string} [opts.ethBalanceWei] hex wei answered to eth_getBalance; * @param {string} [opts.ethBalanceWei] hex wei answered to eth_getBalance;
@@ -593,13 +550,7 @@ async function installNetworkStubs(ctx, opts) {
// Blockscout v2 // Blockscout v2
if (p.includes("/api/v2/")) { if (p.includes("/api/v2/")) {
if (/\/addresses\/0x[0-9a-fA-F]{40}\/transactions$/.test(p)) { if (/\/addresses\/0x[0-9a-fA-F]{40}\/transactions$/.test(p)) {
const addr = blockscoutAddress(p); return jsonResponse(route, { items: [] });
return jsonResponse(route, {
items:
opts.seedNativeTransfer && addr
? nativeTransactionItems(addr)
: [],
});
} }
if (/\/addresses\/0x[0-9a-fA-F]{40}\/token-transfers$/.test(p)) { if (/\/addresses\/0x[0-9a-fA-F]{40}\/token-transfers$/.test(p)) {
const addr = blockscoutAddress(p); const addr = blockscoutAddress(p);
@@ -616,10 +567,8 @@ async function installNetworkStubs(ctx, opts) {
opts.seedTokenBalance ? tokenBalanceItems() : [], opts.seedTokenBalance ? tokenBalanceItems() : [],
); );
} }
for (const hash of [STUB_TX_HASH, STUB_NATIVE_TX_HASH]) { if (p.endsWith("/transactions/" + STUB_TX_HASH)) {
if (p.endsWith("/transactions/" + hash)) { return jsonResponse(route, transactionDetails());
return jsonResponse(route, transactionDetails(hash));
}
} }
} }
@@ -691,8 +640,6 @@ module.exports = {
FEE_ESTIMATE_WEI, FEE_ESTIMATE_WEI,
FEE_RESERVE_WEI, FEE_RESERVE_WEI,
STUB_COUNTERPARTY, STUB_COUNTERPARTY,
STUB_NATIVE_TX_HASH,
STUB_NATIVE_VALUE_WEI,
STUB_TOKEN, STUB_TOKEN,
STUB_TX_HASH, STUB_TX_HASH,
}; };

View File

@@ -36,8 +36,6 @@ const {
FEE_ESTIMATE_WEI, FEE_ESTIMATE_WEI,
FEE_RESERVE_WEI, FEE_RESERVE_WEI,
STUB_COUNTERPARTY, STUB_COUNTERPARTY,
STUB_NATIVE_TX_HASH,
STUB_NATIVE_VALUE_WEI,
STUB_TOKEN, STUB_TOKEN,
STUB_TX_HASH, STUB_TX_HASH,
} = require("./network"); } = require("./network");
@@ -171,264 +169,6 @@ test("transaction detail renders an ERC-20 transfer (#151)", async (env) => {
assert(dots > 0, "token contract row rendered without its colour dot"); assert(dots > 0, "token contract row rendered without its colour dot");
}); });
// --------------------- the rest of the #150 and #151 definition of done
//
// The two tests above assert that the screens #150 and #151 broke now open
// without throwing, which is narrower than what those issues asked for.
// The four items below are the remainder (#188): the navigation stack out
// of Add Token, the quick-pick actually populating the field, the native
// ETH detail path the ERC-20 fix could have regressed, and tap-to-copy.
// Leave the transaction detail screen for the address screen it was opened
// from. The two tests above finish on it, and so does the last test here.
async function leaveTransactionDetail(page) {
if (await page.isVisible("#view-transaction")) {
await page.click("#btn-tx-back");
}
await openAddressDetail(page);
}
// Back out to Home from wherever the previous test finished.
async function goHome(page) {
await leaveTransactionDetail(page);
await page.click("#btn-address-back");
await visible(page, "#view-main");
}
// The navigation stack as it was actually persisted, read out of extension
// storage rather than inferred from which screen is showing. A stale entry
// left behind by a forward navigation that threw is invisible on screen
// until the user presses Back one time too many — which is exactly the
// second-order damage #150 did — so the stack itself is what gets asserted.
function persistedViewStack(page) {
return page.evaluate(
() =>
new Promise((resolve) => {
chrome.storage.local.get("autistmask", (r) => {
resolve((r.autistmask && r.autistmask.viewStack) || []);
});
}),
);
}
// saveState() is fired from showView() without being awaited, so the write
// lands shortly after the screen does. Polling for the expected stack keeps
// that race out of the assertion; a stack that never becomes the expected
// one fails with what it actually was.
const VIEW_STACK_SETTLE_MS = 5000;
async function waitForViewStack(page, expected, where) {
const want = JSON.stringify(expected);
const deadline = Date.now() + VIEW_STACK_SETTLE_MS;
let seen;
for (;;) {
seen = await persistedViewStack(page);
if (JSON.stringify(seen) === want) return;
if (Date.now() >= deadline) break;
await sleep(50);
}
throw new Error(
"navigation stack " +
where +
" is " +
JSON.stringify(seen) +
", expected " +
want,
);
}
// The invariant is stated as a delta against whatever the earlier tests
// left on the stack, not as an absolute: a round trip into Add Token and
// back out must leave the stack exactly as it found it. That is what "no
// duplicated or orphaned stack entry" means, and it holds whatever the
// starting depth is.
test("Back from Add Token unwinds the stack exactly once (#150)", async (env) => {
await goHome(env.page);
const base = await persistedViewStack(env.page);
await env.page.locator("#wallet-list .btn-addr-info").first().click();
await visible(env.page, "#view-address");
await waitForViewStack(env.page, base.concat("main"), "on address detail");
await env.page.click("#btn-add-token");
await visible(env.page, "#view-add-token");
await waitForViewStack(
env.page,
base.concat("main", "address"),
"on the add token screen",
);
await env.page.click("#btn-add-token-back");
await visible(env.page, "#view-address");
assert(
!(await env.page.isVisible("#view-add-token")),
"the add token screen is still showing after Back",
);
await waitForViewStack(
env.page,
base.concat("main"),
"after Back from add token",
);
await env.page.click("#btn-address-back");
await visible(env.page, "#view-main");
await waitForViewStack(env.page, base, "after a second Back");
});
test("a common-token quick-pick fills in the contract address (#150)", async (env) => {
await openAddressDetail(env.page);
await env.page.click("#btn-add-token");
await visible(env.page, "#view-add-token");
const before = await env.page.inputValue("#add-token-address");
assert(
before === "",
"the add token screen opened with the address field already filled: " +
JSON.stringify(before),
);
const pick = env.page.locator("#common-token-list .common-token").first();
const wanted = await pick.getAttribute("data-address");
assert(
/^0x[0-9a-fA-F]{40}$/.test(wanted || ""),
"the first quick-pick button carries no contract address: " +
JSON.stringify(wanted),
);
await pick.click();
const after = await env.page.inputValue("#add-token-address");
assert(
after === wanted,
"clicking the " +
(await pick.innerText()).trim() +
" quick-pick left the address field as " +
JSON.stringify(after) +
", expected " +
JSON.stringify(wanted),
);
await env.page.click("#btn-add-token-back");
await visible(env.page, "#view-address");
});
// The native amount as the transaction list writes it (four decimals) and
// as the detail screen writes it (full precision). Both are rendered here
// from the fixture rather than read off the screen, so the assertions
// compare against the wei the stub served.
const NATIVE_ROW_TEXT =
parseFloat(formatEther(STUB_NATIVE_VALUE_WEI)).toFixed(4) + " ETH";
const NATIVE_DETAIL_TEXT = formatEther(STUB_NATIVE_VALUE_WEI) + " ETH";
test("the native ETH transaction detail still renders (#151)", async (env) => {
// The ERC-20 fix could only have regressed this path by making the
// token-contract branch run for a transfer that has no contract, so
// the assertions below are as much about that row staying hidden as
// about the screen coming up.
env.routeOpts.seedNativeTransfer = true;
await env.page.reload();
await openAddressDetail(env.page);
const row = env.page
.locator("#tx-list .tx-row")
.filter({ hasText: NATIVE_ROW_TEXT });
await row.waitFor({ state: "visible", timeout: 30000 });
await row.click();
await visible(env.page, "#view-transaction");
const hash = await env.page.locator("#tx-detail-hash").innerText();
assert(
hash.includes(STUB_NATIVE_TX_HASH),
"the native transaction detail shows the wrong hash: " + hash,
);
const type = (await env.page.locator("#tx-detail-type").innerText()).trim();
assert(
type === "Native ETH Transfer",
"the native transaction was classified " + JSON.stringify(type),
);
const value = await env.page.locator("#tx-detail-value").innerText();
assert(
value.includes(NATIVE_DETAIL_TEXT),
"the native transaction detail shows " +
JSON.stringify(value) +
", expected it to contain " +
NATIVE_DETAIL_TEXT,
);
const native = await env.page.locator("#tx-detail-native").innerText();
assert(
native.includes(STUB_NATIVE_VALUE_WEI + " wei"),
"the raw quantity row shows " +
JSON.stringify(native) +
", expected the value in wei",
);
assert(
!(await env.page.isVisible("#tx-detail-token-contract-section")),
"the token contract row is showing on a transfer that has no token " +
"contract",
);
// Back to one seeded transaction for everything after this: the tests
// below were written against a list holding the token transfer alone.
env.routeOpts.seedNativeTransfer = false;
});
test("tap-to-copy on the transaction detail screen copies the address (#151)", async (env) => {
// Read the clipboard back rather than watching the handler run: what
// #151 asks for is the address reaching the clipboard, and a spy on
// navigator.clipboard would assert the call and not the effect.
//
// Granted context-wide rather than for the popup's origin: an
// origin-scoped grant is refused for chrome-extension: URLs, which
// both Playwright and Chrome treat as opaque here.
await env.ctx.grantPermissions(["clipboard-read", "clipboard-write"]);
await leaveTransactionDetail(env.page);
const row = env.page
.locator("#tx-list .tx-row")
.filter({ hasText: STUB_TOKEN.symbol });
await row.waitFor({ state: "visible", timeout: 30000 });
await row.click();
await visible(env.page, "#view-transaction");
await visible(env.page, "#tx-detail-token-contract-section");
// Seed a sentinel first, so a clipboard that nothing writes to cannot
// pass on whatever was left in it.
const SENTINEL = "e2e-clipboard-untouched";
await env.page.evaluate((s) => navigator.clipboard.writeText(s), SENTINEL);
const seeded = await env.page.evaluate(() =>
navigator.clipboard.readText(),
);
assert(
seeded === SENTINEL,
"the harness could not seed the clipboard, so the assertion below " +
"would prove nothing; it read back " +
JSON.stringify(seeded),
);
await env.page.locator("#tx-detail-token-contract [data-copy]").click();
const copied = await env.page.evaluate(() =>
navigator.clipboard.readText(),
);
assert(
copied.toLowerCase() === STUB_TOKEN.address,
"tapping the token contract address put " +
JSON.stringify(copied) +
" on the clipboard, expected " +
STUB_TOKEN.address,
);
const flash = await env.page.locator("#flash-msg").innerText();
assert(
flash.trim() === "Copied!",
"the copy gave no confirmation, flash line reads " +
JSON.stringify(flash),
);
});
// -------------------------------------------- recovery phrase (#161) // -------------------------------------------- recovery phrase (#161)
// The gear toggles, so pressing it while Settings is already up leaves it. // The gear toggles, so pressing it while Settings is already up leaves it.
@@ -1856,32 +1596,13 @@ async function reserveApprovalTab(env) {
// one down with it. // one down with it.
env.approvalTab = await env.ctx.newPage(); env.approvalTab = await env.ctx.newPage();
// The one accommodation this section makes to the shipped code, and the // This tab runs the shipped popup with nothing patched. The site
// reason for it. // approval buttons decide and then close on the next line, and the two
// // site-approval tests below are therefore the real-browser
// Both approval buttons call runtime.sendMessage() and then window.close() // approve-then-immediate-close and reject-then-immediate-close cases: the
// on the next line. Closing this page disconnects the approval port, and // decision rides the approval port, which also carries the disconnect the
// the disconnect handler in src/background/index.js settles a pending // close causes, so it is delivered ahead of it and the outcome does not
// site approval as a rejection. In a tab those two race and the teardown // depend on the teardown timing (#275).
// wins: the approve message is never acted on, and the page is told the
// user rejected. Measured — with the close left in place the approval
// resolves as a rejection every time; with it deferred it resolves as an
// approval every time.
//
// It is deferred, not removed: the harness closes the page itself once
// the outcome has been observed, which is what window.close() would have
// done, only after the message it was racing has been processed.
//
// This affects the site-connection prompt only. The sign and transaction
// prompts run in windows the extension opens itself, with window.close()
// untouched, and their disconnect handler deliberately keeps a tx or sign
// approval pending rather than rejecting it — so there is no race there
// to accommodate. Whether the same ordering holds in a real toolbar popup
// is not observable from a headless harness and is reported rather than
// assumed either way.
await env.approvalTab.addInitScript(() => {
window.close = function () {};
});
await env.approvalTab.goto("about:blank"); await env.approvalTab.goto("about:blank");
await sleep(APPROVAL_TAB_SETTLE_MS); await sleep(APPROVAL_TAB_SETTLE_MS);
return env.approvalTab; return env.approvalTab;
@@ -1930,6 +1651,28 @@ async function closeApprovalPages(ctx) {
} }
} }
// Click a button whose own handler closes the window it lives in — every
// Reject, and Allow on the site prompt.
//
// page.click() dispatches the click and then waits for the renderer to
// acknowledge it, and a page torn down by the handler never gets to. The
// dispatch is what the test needs and the log shows it happening ("performing
// click action") immediately before the failure; the page going away is the
// button working, not the click failing. Observed on #btn-reject-sign and
// #btn-reject-tx, whose windows have always closed themselves.
//
// This swallows nothing that matters: a click that did not land leaves the
// dApp promise unsettled and the assertion after the call still fails. A
// button that is missing or unclickable raises a different error, which is
// rethrown.
async function clickAndClose(page, selector) {
try {
await page.click(selector);
} catch (e) {
if (!String((e && e.message) || e).includes("has been closed")) throw e;
}
}
// Record every message the approval window sends to the background worker. // Record every message the approval window sends to the background worker.
// //
// This is the direct observation the password check needs. It is installed // This is the direct observation the password check needs. It is installed
@@ -2150,7 +1893,7 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as
// origin in deniedSites and every later test in this section is // origin in deniedSites and every later test in this section is
// auto-rejected with no prompt at all, which would look like a pass. // auto-rejected with no prompt at all, which would look like a pass.
await popup.uncheck("#approve-remember"); await popup.uncheck("#approve-remember");
await popup.click("#btn-reject"); await clickAndClose(popup, "#btn-reject");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2181,7 +1924,7 @@ test("eth_requestAccounts approved returns the selected address (#183)", async (
// does not, and the sign and transaction tests below all require the // does not, and the sign and transaction tests below all require the
// origin to still be authorized. // origin to still be authorized.
await popup.check("#approve-remember"); await popup.check("#approve-remember");
await popup.click("#btn-approve"); await clickAndClose(popup, "#btn-approve");
outcome = await settleRequest(env.dapp, "accounts"); outcome = await settleRequest(env.dapp, "accounts");
} finally { } finally {
@@ -2289,7 +2032,7 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await popup.click("#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2392,7 +2135,7 @@ test("eth_signTypedData_v4 rejected returns a rejection to the page (#183)", asy
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await popup.click("#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2550,7 +2293,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
]); ]);
const popup = await waitForApprovalWindow(env.ctx); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-tx"); await visible(popup, "#view-approve-tx");
await popup.click("#btn-reject-tx"); await clickAndClose(popup, "#btn-reject-tx");
await assertUserRejection( await assertUserRejection(
env.dapp, env.dapp,
@@ -2631,7 +2374,6 @@ async function main() {
// starting state of a run is readable without hunting through tests. // starting state of a run is readable without hunting through tests.
const routeOpts = { const routeOpts = {
seedTokenTransfer: false, seedTokenTransfer: false,
seedNativeTransfer: false,
seedTokenBalance: false, seedTokenBalance: false,
ethBalanceWei: null, ethBalanceWei: null,
failGasEstimate: false, failGasEstimate: false,