Compare commits

..

3 Commits

Author SHA1 Message Date
f0441a913b fix: one transaction approval at a time, and honest copy for a nonce collision (closes #271)
All checks were successful
check / check (push) Successful in 27s
Populating the transaction in the background before the approval window
opens is what makes the displayed object the verified object. It also
fixes the nonce before the user has answered anything, so two
eth_sendTransaction calls populated concurrently took the same nonce
from a node that had seen neither of them broadcast, and the second
could never be sent: its approved nonce is spent, and the only way to
give it a fresh one is to populate it again after the user has read the
old one off the screen.

A second transaction approval is now refused while one is unanswered,
with EIP-1193 code -32002. The refusal happens before anything is
populated — no second nonce is allocated, no window opens — and the slot
is released when the requesting page has its answer. Signature approvals
are not gated; a signature consumes no nonce.

A collision that does happen is now reported for what it is. A broadcast
the node refused for the nonce, and an approval carrying a nonce this
worker has already broadcast (caught before the node is asked at all),
both report that the transaction did not reach the network and to send
it again, instead of the standing broadcast wording that warns it may
have sent. "already known" keeps that ambiguous wording deliberately: a
node that says it has the transaction has it.

Nothing about verification is weakened. The approval still carries the
transaction the screen displayed, and the artifact is still compared
against that object field for field.
2026-08-17 06:08:25 +00:00
51e84aefa6 docs: drop the README limit that #274 fixed (closes #285)
All checks were successful
check / check (push) Successful in 26s
The EIP-1193 error code now survives the last hop into the page, so the third standing limit in the End-to-End Tests section no longer describes the code. Verified against src/content/inpage.js (ProviderRpcError carries code and data) and tests/e2e/run.js (the page-side error is asserted, not only the wire).
2026-08-17 08:03:02 +02:00
0be20d7270 fix: render the view "Back" lands on after the popup is reopened (closes #268)
All checks were successful
check / check (push) Successful in 1m27s
2026-08-14 06:14:09 +02:00
9 changed files with 1166 additions and 225 deletions

View File

@@ -185,17 +185,14 @@ to the background — with the message that would carry it required to be presen
so that check cannot pass by observing nothing. That last one is the standing so that check cannot pass by observing nothing. That last one is the standing
floor under [#157](https://git.eeqj.de/sneak/AutistMask/issues/157). floor under [#157](https://git.eeqj.de/sneak/AutistMask/issues/157).
Three limits of that coverage, none of them papered over. The RPC is stubbed Two limits of that coverage, neither of them papered over. The RPC is stubbed
throughout, so this is **not** a real dApp against a real network with real throughout, so this is **not** a real dApp against a real network with real
funds; that remains a human pass before 1.0.0. The site-connection prompt is funds; that remains a human pass before 1.0.0. The site-connection prompt is
raised through `chrome.action.openPopup()`, and headless Chromium's raised through `chrome.action.openPopup()`, and headless Chromium's
browser-action popup is not a page Playwright can see or click, so that one browser-action popup is not a page Playwright can see or click, so that one
prompt is driven at the URL the extension itself puts on the action — the same prompt is driven at the URL the extension itself puts on the action — the same
page and the same approval id, but whether a real toolbar click shows it is not page and the same approval id, but whether a real toolbar click shows it is not
observable here. And the EIP-1193 error code does not survive the last hop: the observable here.
rejection that crosses the boundary carries code 4001 and is asserted to, but
`src/content/inpage.js` rebuilds it as `new Error(message)`, so the calling page
catches an error with no `code` property.
Any test that drives a failure path on purpose declares the `console.error` it Any test that drives a failure path on purpose declares the `console.error` it
is about to provoke, via `errors.expect()`. That is not a mute: the declaration is about to provoke, via `errors.expect()`. That is not a mute: the declaration
@@ -638,6 +635,21 @@ ExportPrivKey and ShowRecoveryPhrase — are deliberately absent from that list,
so the popup can never reopen onto one of them with no password prompt in front so the popup can never reopen onto one of them with no password prompt in front
of it. of it.
A reopened popup renders the wallet list and the one screen it restores onto,
and nothing else, so every screen on the stack behind that one is still the
blank template from `index.html`. "Back" therefore renders its target rather
than only unhiding it, through the same dispatch and data guards as the restore
(`src/popup/viewRouter.js`), and falls back to Home when the state the target
would render is gone.
It renders only a screen this page load has not rendered yet. Forward navigation
renders as it goes, and `viewRouter.js` records every screen that reaches
`showView()`, so "Back" onto a screen already on the page unhides it and nothing
more — rendering it a second time would re-fetch and overwrite what it holds,
such as an edit typed into Settings and not yet saved. Home is the one screen
"Back" always re-renders, so the wallet list reflects anything that changed
while the user was away from it.
Every screen that holds secret material in the page registers a cleanup with Every screen that holds secret material in the page registers a cleanup with
`onViewLeave()` (`src/popup/views/helpers.js`), which `showView()` runs on every `onViewLeave()` (`src/popup/views/helpers.js`), which `showView()` runs on every
exit from that screen rather than only on its "Back" button, so nothing secret exit from that screen rather than only on its "Back" button, so nothing secret

54
TODO.md
View File

@@ -45,22 +45,39 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-14: One transaction approval at a time. Populating in the background - 2026-08-17: `README.md` no longer advertises a defect the wallet does not
have. The End-to-End Tests section listed the EIP-1193 code being dropped in
the last hop into the page as a standing limit of the dApp coverage; that
stopped being true when
[#274](https://git.eeqj.de/sneak/AutistMask/issues/274) landed and did not
touch the README. The paragraph is deleted and the two remaining limits — the
stubbed RPC and the unobservable toolbar popup — were checked against the
current `src/content/inpage.js` and `tests/e2e/` and left as they are
([#285](https://git.eeqj.de/sneak/AutistMask/issues/285)).
- 2026-08-17: One transaction approval at a time. Populating in the background
before the window opens is what makes the displayed object the verified before the window opens is what makes the displayed object the verified
object, and it also fixes the nonce: two `eth_sendTransaction` calls populated object, and it also fixes the nonce: two `eth_sendTransaction` calls populated
concurrently took the same nonce from a node that had seen neither broadcast, concurrently took the same nonce from a node that had seen neither broadcast,
and the second could then never be sent, because the only way to give it a and the second could then never be sent, because the only way to give it a
fresh nonce is to populate it again after the user has read the old one off fresh nonce is to populate it again after the user has read the old one off
the screen. A second request is now refused with EIP-1193 `-32002` while one the screen. A second request is now refused with EIP-1193 `-32002` while one
is unanswered — before anything is populated, so no second nonce is allocated is unanswered — the slot is taken immediately before population, after the
and no second window opens — and the slot is freed when the page has its authorization checks, so no second nonce is allocated, no second window opens,
answer. Signature approvals are not gated, consuming no nonce. A collision and a page the wallet refuses anyway cannot hold the slot against the
that does happen is also reported accurately now: a broadcast the node refused connected site. The slot is freed at `settleApproval()`, the single point an
for the nonce, and an approval carrying a nonce this worker has already approval is retired, so every path that ends an approval ends the hold with
broadcast (caught before the node is asked at all), both say the transaction it; an approval whose window is gone and whose attempt has failed is settled
did not reach the network and to send it again, instead of warning that it may there rather than left waiting on a window that no longer exists, and an
have sent. `already known` deliberately keeps the ambiguous wording, because a approval window that could not be opened at all is answered with `-32603`
node that says it has the transaction has it instead of holding the page's promise open. Signature approvals are not gated,
consuming no nonce. A collision that does happen is also reported accurately
now: a broadcast the node refused for the nonce, and an approval carrying a
nonce this worker has already broadcast for that address on that chain (caught
before the node is asked at all), both say the transaction did not reach the
network and to send it again, instead of warning that it may have sent. The
record is keyed by chain as well as address, because nonce spaces are per
chain and low nonces overlap across them. `already known` deliberately keeps
the ambiguous wording, because a node that says it has the transaction has it
([#271](https://git.eeqj.de/sneak/AutistMask/issues/271)). ([#271](https://git.eeqj.de/sneak/AutistMask/issues/271)).
- 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
@@ -76,6 +93,23 @@ undefined identifiers, which is how
`tests/inpageErrors.test.js`, and the e2e probe that printed the missing code `tests/inpageErrors.test.js`, and the e2e probe that printed the missing code
now requires it on the page's Error as well as on the wire, for all four now requires it on the page's Error as well as on the wire, for all four
rejected flows ([#274](https://git.eeqj.de/sneak/AutistMask/issues/274)). rejected flows ([#274](https://git.eeqj.de/sneak/AutistMask/issues/274)).
- 2026-08-12: "Back" now renders the screen it lands on instead of only unhiding
it. A reopened popup renders the wallet list and the one screen it restores
onto, so every screen further down the stack was still the blank template from
`index.html`, and Back walked straight onto it — an empty address, no
balances, no QR code. The Back path now goes through the same per-view
dispatch and data guards as the restore (`src/popup/viewRouter.js`, shared
with `restoreView()`), falling back to Home when the state the target would
render is gone. It renders only a view this page load has not rendered yet:
`viewRouter.js` records every view that reaches `showView()`, which is where
forward navigation and the restore both end, so Back onto a view already on
the page unhides it and nothing more. That is what keeps a second render from
re-fetching and overwriting what the view holds — an unsaved edit in Settings,
a transaction list already loaded. Home is the exception and is always
re-rendered, as it was before. Covered by unit tests on the real `goBack()`
and by three end-to-end cases against the real popup, each demonstrated
failing on the unfixed build
([#268](https://git.eeqj.de/sneak/AutistMask/issues/268)).
- 2026-08-12: `KNOWN_SYMBOLS` now maps a symbol to the set of contract addresses - 2026-08-12: `KNOWN_SYMBOLS` now maps a symbol to the set of contract addresses
that bear it, not to one of them. A ticker is not unique: seven of the 512 that bear it, not to one of them. A ticker is not unique: seven of the 512
bundled tokens — `FRAX`, `REUSD`, `TON`, `EURE`, `MSUSD`, `MUSD` and `JPYC` bundled tokens — `FRAX`, `REUSD`, `TON`, `EURE`, `MSUSD`, `MUSD` and `JPYC`

View File

@@ -80,35 +80,65 @@ const pendingApprovals = {};
// approval then leaves a hole in. // approval then leaves a hole in.
// //
// Sign approvals are not gated: a signature consumes no nonce. // Sign approvals are not gated: a signature consumes no nonce.
let txApprovalSlotHeld = false; //
// The slot is null when free, and otherwise the handle of the request holding
// it. Once that request has raised its approval the handle carries the
// approval's id, so that retiring the approval frees the slot: every exit from
// pendingApprovals goes through settleApproval(), which makes that one hook
// complete. The holder's own finally is the backstop for the interval before
// the approval exists.
let txApprovalSlot = null;
// EIP-1474 "resource unavailable": the standard code for a request that is // EIP-1474 "resource unavailable": the standard code for a request that is
// refused because another one is already pending. // refused because another one is already pending.
const TX_APPROVAL_PENDING_CODE = -32002; const TX_APPROVAL_PENDING_CODE = -32002;
// True at every moment this can be sent: the slot is taken immediately before
// the transaction is populated, so the other request is either being prepared
// or on screen. It does not claim the other one is displayed yet, because for
// the length of one network round trip it is not.
const TX_APPROVAL_PENDING_MESSAGE = const TX_APPROVAL_PENDING_MESSAGE =
"Another transaction is already waiting to be approved in AutistMask," + "AutistMask handles one transaction at a time, and another one is" +
" so this one was not sent. Please answer that request, then send this" + " already in progress, so this one was not sent. Please finish that" +
" one again."; " transaction, then send this one again.";
// Take the slot, or refuse. Called before the first await of the // Take the slot, or refuse. Nothing awaits between the test and the set, so
// eth_sendTransaction handler, so two requests arriving in the same tick // two requests that reach this in the same tick cannot both pass it — the
// cannot both pass it. // position of the call in the handler is irrelevant to that, which is why it
// sits after the authorization checks. A page the wallet is going to refuse
// anyway must not be able to take the slot away from the connected site.
function reserveTxApprovalSlot() { function reserveTxApprovalSlot() {
if (txApprovalSlotHeld) return false; if (txApprovalSlot) return null;
txApprovalSlotHeld = true; txApprovalSlot = { approvalId: null };
return true; return txApprovalSlot;
} }
function releaseTxApprovalSlot() { // Free the slot, if this handle is still the one holding it.
txApprovalSlotHeld = false; function releaseTxApprovalSlot(handle) {
if (handle && txApprovalSlot !== handle) return;
txApprovalSlot = null;
} }
// Nonces this worker has already handed to the node, per address. This is the // Free the slot held on behalf of a retired approval. Called from
// wallet's own knowledge that a nonce is spent, and it is checked before a // settleApproval() for every approval, and a no-op for the ones the slot was
// broadcast rather than after: a node's pending count can lag a transaction it // not taken for.
// has itself just accepted, and a request populated inside that window would function releaseTxApprovalSlotFor(approvalId) {
// otherwise be signed and sent at a nonce this wallet has already used. if (txApprovalSlot && txApprovalSlot.approvalId === approvalId) {
txApprovalSlot = null;
}
}
// Nonces this worker has already handed to the node, per chain and address.
// This is the wallet's own knowledge that a nonce is spent, and it is checked
// before a broadcast rather than after: a node's pending count can lag a
// transaction it has itself just accepted, and a request populated inside that
// window would otherwise be signed and sent at a nonce this wallet has already
// used.
//
// The chain is part of the key because nonce spaces are per chain and the
// wallet switches networks. Without it a nonce spent on one chain would refuse
// that nonce on every other chain — and low nonces overlap across chains as a
// matter of course, so the refusal would be both routine and false.
// //
// The record dies with the worker, which is correct rather than merely // The record dies with the worker, which is correct rather than merely
// convenient: after a restart the node's count is the only answer available, // convenient: after a restart the node's count is the only answer available,
@@ -116,8 +146,11 @@ function releaseTxApprovalSlot() {
// user does want to be able to send again. // user does want to be able to send again.
const broadcastNonces = {}; const broadcastNonces = {};
function broadcastNoncesFor(address) { function broadcastNoncesFor(chainId, address) {
const key = String(address || "").toLowerCase(); const key =
String(chainId).toLowerCase() +
":" +
String(address || "").toLowerCase();
if (!broadcastNonces[key]) broadcastNonces[key] = new Set(); if (!broadcastNonces[key]) broadcastNonces[key] = new Set();
return broadcastNonces[key]; return broadcastNonces[key];
} }
@@ -224,11 +257,41 @@ function settleApproval(id, result, options) {
const holdsClaim = !!(options && options.holdsClaim); const holdsClaim = !!(options && options.holdsClaim);
if (approval.attemptInFlight && !holdsClaim) return false; if (approval.attemptInFlight && !holdsClaim) return false;
delete pendingApprovals[id]; delete pendingApprovals[id];
// The transaction-approval slot is held for exactly as long as the
// approval it was taken for is alive, and this is the one place an
// approval stops being alive.
releaseTxApprovalSlotFor(id);
approval.resolve(result); approval.resolve(result);
resetPopupUrl(); resetPopupUrl();
return true; return true;
} }
// What a pending approval resolves to when it is given up on rather than
// answered: the window was closed, or could not be opened at all. A tx or sign
// approval answers the requesting page in EIP-1193 shape; a site-connection
// approval answers the connection handler in its own.
function abandonedResult(approval, code, message) {
if (approval.type === "tx" || approval.type === "sign") {
return { error: { code, message } };
}
return { approved: false, remember: false };
}
// A window the user closed without answering is a refusal by the user, which
// is 4001 and the wording every other rejection path already uses.
const APPROVAL_REJECTED_CODE = 4001;
const APPROVAL_REJECTED_MESSAGE = "User rejected the request.";
// The window could not be opened, so the user was never asked. This is the
// wallet failing, not the user refusing, so it does not claim to be a
// rejection: -32603 is the JSON-RPC code for the wallet's own internal
// failure, and the page is told plainly that nothing was shown.
const APPROVAL_WINDOW_FAILED_CODE = -32603;
const APPROVAL_WINDOW_FAILED_MESSAGE =
"AutistMask could not open its approval window, so this request was not" +
" shown to you and nothing was sent.";
// Take exclusive hold of a pending approval for one attempt, or refuse. // Take exclusive hold of a pending approval for one attempt, or refuse.
// //
// An approval that failed retryably has to stay in pendingApprovals, so its // An approval that failed retryably has to stay in pendingApprovals, so its
@@ -248,8 +311,26 @@ function claimApproval(approval) {
// Release an approval whose attempt failed in a way the user can retry. // Release an approval whose attempt failed in a way the user can retry.
// Nothing was broadcast, so the next attempt may claim it. // Nothing was broadcast, so the next attempt may claim it.
//
// Unless the window it would be retried in is already gone. The user closed it
// while the attempt was running and settleApproval() declined then, correctly,
// because the attempt still owned the approval; the attempt has now failed, so
// nothing owns it and nothing can reach it. Left standing it would hold the
// requesting page's promise open forever and, with it, the transaction
// approval slot. It is settled here as the rejection the closed window
// already meant.
function releaseApproval(approval) { function releaseApproval(approval) {
approval.attemptInFlight = false; approval.attemptInFlight = false;
if (approval.windowClosed) {
settleApproval(
approval.id,
abandonedResult(
approval,
APPROVAL_REJECTED_CODE,
APPROVAL_REJECTED_MESSAGE,
),
);
}
} }
// Open approval in a separate popup window. // Open approval in a separate popup window.
@@ -276,10 +357,36 @@ function openApprovalWindow(id) {
); );
} }
windowsApi.create(opts, (win) => { windowsApi.create(opts, (win) => {
const approval = pendingApprovals[id];
if (!approval) {
// Settled while the window was opening — an address switch,
// say. Nothing is waiting on it, and a window showing an
// approval that no longer exists is not left on screen.
if (win) { if (win) {
pendingApprovals[id].windowId = win.id; windowsApi.remove(win.id, () => {
if (runtime.lastError) {
// window already closed
} }
}); });
}
return;
}
if (!win) {
// No window means no way to ever answer this approval, and an
// approval nothing can answer holds the requesting page's
// promise open forever. Settle it now instead.
settleApproval(
id,
abandonedResult(
approval,
APPROVAL_WINDOW_FAILED_CODE,
APPROVAL_WINDOW_FAILED_MESSAGE,
),
);
return;
}
approval.windowId = win.id;
});
}); });
} }
@@ -288,7 +395,7 @@ function openApprovalWindow(id) {
function requestApproval(origin, hostname) { function requestApproval(origin, hostname) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { origin, hostname, resolve }; pendingApprovals[id] = { id, origin, hostname, resolve };
if (actionApi && typeof actionApi.openPopup === "function") { if (actionApi && typeof actionApi.openPopup === "function") {
actionApi.setPopup({ actionApi.setPopup({
@@ -319,10 +426,13 @@ function requestApproval(origin, hostname) {
// it is pinned here rather than read again at signing time — an address switch // it is pinned here rather than read again at signing time — an address switch
// between approval and signing must refuse, not sign from an account this // between approval and signing must refuse, not sign from an account this
// screen never named. // screen never named.
function requestTxApproval(origin, hostname, approvedTx, approvedFrom) { // `slot` is the transaction-approval slot its caller holds. Handing the
// approval's id to it is what makes retiring the approval free the slot.
function requestTxApproval(origin, hostname, approvedTx, approvedFrom, slot) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { pendingApprovals[id] = {
id,
origin, origin,
hostname, hostname,
approvedTx, approvedTx,
@@ -330,6 +440,7 @@ function requestTxApproval(origin, hostname, approvedTx, approvedFrom) {
resolve, resolve,
type: "tx", type: "tx",
}; };
if (slot) slot.approvalId = id;
openApprovalWindow(id); openApprovalWindow(id);
}); });
@@ -343,6 +454,7 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { pendingApprovals[id] = {
id,
origin, origin,
hostname, hostname,
signParams, signParams,
@@ -661,24 +773,7 @@ async function handleRpc(method, params, origin) {
} }
if (method === "eth_sendTransaction") { if (method === "eth_sendTransaction") {
// Synchronous, before any await: two requests delivered in the same
// tick must not both get past this.
if (!reserveTxApprovalSlot()) {
return {
error: {
code: TX_APPROVAL_PENDING_CODE,
message: TX_APPROVAL_PENDING_MESSAGE,
},
};
}
try {
return await handleSendTransaction(params, origin); return await handleSendTransaction(params, origin);
} finally {
// Held until the page has its answer — the approval was broadcast,
// rejected, or retired by a closed window — because until then its
// nonce is allocated and unspent.
releaseTxApprovalSlot();
}
} }
// Proxy safe read-only methods to the RPC node // Proxy safe read-only methods to the RPC node
@@ -695,8 +790,9 @@ async function handleRpc(method, params, origin) {
} }
// The body of eth_sendTransaction, from the connection check through to the // The body of eth_sendTransaction, from the connection check through to the
// user's decision. Its caller holds the single transaction-approval slot for // user's decision. It takes the single transaction-approval slot once it knows
// as long as this runs. // it is going to populate a transaction, and holds it until the requesting
// page has its answer.
async function handleSendTransaction(params, origin) { async function handleSendTransaction(params, origin) {
const s = await getState(); const s = await getState();
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
@@ -722,6 +818,23 @@ async function handleSendTransaction(params, origin) {
}; };
} }
// Everything above refuses without populating anything, so the slot is
// taken here rather than at the top of the handler: a page the wallet was
// never going to serve must not be able to hold the slot and make the
// connected site's own transaction fail as "already in progress". The
// reservation is atomic because nothing awaits between its test and its
// set, not because of where it sits.
const slot = reserveTxApprovalSlot();
if (!slot) {
return {
error: {
code: TX_APPROVAL_PENDING_CODE,
message: TX_APPROVAL_PENDING_MESSAGE,
},
};
}
try {
// Populate here, before any window opens, so that the transaction the // Populate here, before any window opens, so that the transaction the
// user is shown is a complete one and is the same object the signed // user is shown is a complete one and is the same object the signed
// artifact is checked against. A failure raises no approval at all and // artifact is checked against. A failure raises no approval at all and
@@ -756,9 +869,17 @@ async function handleSendTransaction(params, origin) {
hostname, hostname,
approvedTx, approvedTx,
activeAddress, activeAddress,
slot,
); );
if (decision.error) return { error: decision.error }; if (decision.error) return { error: decision.error };
return { result: decision.txHash }; return { result: decision.txHash };
} finally {
// Retiring the approval has normally freed the slot already, through
// settleApproval(); this covers the paths that return before an
// approval exists at all, and frees nothing if another request has
// since taken the slot.
releaseTxApprovalSlot(slot);
}
} }
// Broadcast chainChanged to all tabs when the network is switched. // Broadcast chainChanged to all tabs when the network is switched.
@@ -793,15 +914,11 @@ async function broadcastAccountsChanged() {
// being signed and broadcast right now, and neither rejecting it to the // being signed and broadcast right now, and neither rejecting it to the
// page nor closing the window it is reporting into is survivable. // page nor closing the window it is reporting into is survivable.
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
const rejection = const rejection = abandonedResult(
approval.type === "tx" || approval.type === "sign" approval,
? { APPROVAL_REJECTED_CODE,
error: { APPROVAL_REJECTED_MESSAGE,
code: 4001, );
message: "User rejected the request.",
},
}
: { approved: false, remember: false };
if (!settleApproval(id, rejection)) continue; if (!settleApproval(id, rejection)) continue;
if (approval.windowId) { if (approval.windowId) {
windowsApi.remove(approval.windowId, () => { windowsApi.remove(approval.windowId, () => {
@@ -930,21 +1047,19 @@ startBackgroundJobs();
// verify and broadcast it is waiting on, so a user closing an apparently-hung // verify and broadcast it is waiting on, so a user closing an apparently-hung
// 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 — and the window is recorded as gone, so that an attempt
// which then fails retryably settles instead of waiting in a window that no
// longer exists.
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 rejection = abandonedResult(
approval.type === "tx" || approval.type === "sign" approval,
? { APPROVAL_REJECTED_CODE,
error: { APPROVAL_REJECTED_MESSAGE,
code: 4001, );
message: "User rejected the request.", if (!settleApproval(id, rejection)) approval.windowClosed = true;
},
}
: { approved: false, remember: false };
settleApproval(id, rejection);
} }
}); });
} }
@@ -1074,8 +1189,15 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
} }
(async () => { (async () => {
// The chain this attempt is on, read once. Verification below
// refuses an artifact signed for any other chain, and the nonce
// record is both consulted and written under this one, so a
// network switch part-way through cannot make the check and the
// record disagree about which chain the nonce was spent on.
let chainId;
try { try {
await loadState(); await loadState();
chainId = currentNetwork().chainId;
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
// An address switch between approval and signing refuses. The // An address switch between approval and signing refuses. The
// approval named one account; signing from whichever account // approval named one account; signing from whichever account
@@ -1098,7 +1220,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
msg.rawSignedTx, msg.rawSignedTx,
approval.approvedTx, approval.approvedTx,
approval.approvedFrom, approval.approvedFrom,
currentNetwork().chainId, chainId,
); );
} catch (e) { } catch (e) {
// A signed transaction that is not the approved one is not // A signed transaction that is not the approved one is not
@@ -1123,12 +1245,15 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
return; return;
} }
// A nonce this worker has already broadcast for this address. The // A nonce this worker has already broadcast for this address on
// node is not asked: it has answered once already, and the wallet // this chain. The node is not asked: it has answered once already,
// holding the receipt of that answer is what makes this failure // and the wallet holding the receipt of that answer is what makes
// one the user can be told did not reach the network. // this failure one the user can be told did not reach the network.
// A nonce spent on another chain is not spent here — the chains
// count separately, and refusing across them would block ordinary
// use with a message that is not true.
const nonce = approvedNonce(approval.approvedTx); const nonce = approvedNonce(approval.approvedTx);
const spent = broadcastNoncesFor(approval.approvedFrom); const spent = broadcastNoncesFor(chainId, approval.approvedFrom);
if (nonce !== null && spent.has(nonce)) { if (nonce !== null && spent.has(nonce)) {
const outcome = describeTxFailure(TX_STAGE_NONCE, null); const outcome = describeTxFailure(TX_STAGE_NONCE, null);
settleApproval( settleApproval(

View File

@@ -9,16 +9,17 @@ const {
$, $,
showView, showView,
updateDebugBanner, updateDebugBanner,
setRenderMain, setBackRenderer,
pushCurrentView, pushCurrentView,
goBack, goBack,
clearViewStack, clearViewStack,
} = require("./views/helpers"); } = require("./views/helpers");
const { applyTheme } = require("./theme"); const { applyTheme } = require("./theme");
// Views that can be fully re-rendered from persisted state. All others fall // Renders a view the popup lands on without having navigated to it forward:
// back to the nearest restorable parent; see the module for why the // on restore here, and on Back. Only the views that can be fully re-rendered
// secret-bearing views are absent. // from persisted state (RESTORABLE_VIEWS, src/popup/restorableViews.js) go
const { RESTORABLE_VIEWS } = require("./restorableViews"); // through it; anything else falls back to the nearest restorable parent.
const { renderView, makeBackRenderer } = require("./viewRouter");
const home = require("./views/home"); const home = require("./views/home");
const welcome = require("./views/welcome"); const welcome = require("./views/welcome");
@@ -108,92 +109,23 @@ const ctx = {
}, },
}; };
function needsAddress(view) { // The view modules the router renders through, keyed as it expects them.
return ( const viewModules = {
view === "address" || main: { show: () => fallbackView() },
view === "address-token" || addressDetail,
view === "receive" || addressToken,
view === "transaction" receive,
); settings,
} settingsAddToken,
confirmTx,
function hasValidAddress() { transactionDetail,
return ( txStatus,
state.selectedWallet !== null && };
state.selectedAddress !== null &&
state.wallets[state.selectedWallet] &&
state.wallets[state.selectedWallet].addresses[state.selectedAddress]
);
}
function restoreView() { function restoreView() {
const view = state.currentView; if (!renderView(state.currentView, state, viewModules)) {
if (!view || !RESTORABLE_VIEWS.has(view)) {
return fallbackView();
}
if (needsAddress(view) && !hasValidAddress()) {
return fallbackView();
}
if (view === "address-token" && !state.selectedToken) {
return fallbackView();
}
switch (view) {
case "address":
addressDetail.show();
break;
case "address-token":
addressToken.show();
break;
case "receive":
receive.show();
break;
case "settings":
settings.show();
break;
case "settings-addtoken":
settingsAddToken.show();
break;
case "confirm-tx":
if (state.viewData && state.viewData.pendingTx) {
confirmTx.restore();
} else {
fallbackView(); fallbackView();
} }
break;
case "transaction":
if (state.viewData && state.viewData.tx) {
transactionDetail.render();
} else {
fallbackView();
}
break;
case "wait-tx":
// Resumes the receipt poll from the persisted broadcast time.
if (!txStatus.restoreWait()) {
fallbackView();
}
break;
case "success-tx":
if (state.viewData && state.viewData.hash) {
txStatus.renderSuccess();
} else {
fallbackView();
}
break;
case "error-tx":
if (state.viewData && state.viewData.message) {
txStatus.renderError();
} else {
fallbackView();
}
break;
default:
fallbackView();
break;
}
} }
function fallbackView() { function fallbackView() {
@@ -247,7 +179,7 @@ async function init() {
settings.show(); settings.show();
}); });
setRenderMain(renderWalletList); setBackRenderer(makeBackRenderer(state, viewModules));
welcome.init(ctx); welcome.init(ctx);
addWallet.init(ctx); addWallet.init(ctx);

167
src/popup/viewRouter.js Normal file
View File

@@ -0,0 +1,167 @@
// Rendering a view the popup lands on without having navigated to it
// forward: on restore, and on Back. In both cases the view may never have
// been rendered in this page load — a reopened popup renders only the
// wallet list and the view it restores onto, so every other view is still
// the blank static template from index.html — so unhiding it is not enough.
//
// Forward navigation renders as it goes and must NOT come through here:
// rendering a second time would re-fetch and clobber whatever the view has
// in flight.
//
// The view modules are injected and nothing here touches the DOM, so the
// dispatch and its data guards can be tested directly; src/popup/index.js
// cannot be required outside a browser.
const { RESTORABLE_VIEWS } = require("./restorableViews");
// The views this page load has rendered.
//
// The Back path cannot otherwise tell its two cases apart. A view the popup
// never rendered is still the blank template from index.html and has to be
// rendered; a view already on the page must NOT be rendered again, because
// a second render re-fetches and overwrites whatever the user has typed
// into it and not yet saved.
//
// Registration is showView() in views/helpers.js, which is the last thing
// every render path runs — restoreView()'s, the Back path's, and every
// forward show(). That is the point of putting it there rather than in the
// individual views: a view added later registers itself with no one having
// to remember it, so this cannot decay.
//
// Module scope is page-load scope: the popup loads this module once per
// page load, and a reopened popup gets a fresh, empty set — which is
// exactly the state that makes the Back path render.
const renderedViews = new Set();
function markViewRendered(view) {
if (view) renderedViews.add(view);
}
// Begin a fresh page-load scope. The popup gets one by being loaded; the
// unit tests, which simulate several page loads against one module
// instance, ask for one.
function resetRenderedViews() {
renderedViews.clear();
}
// Home is the exception: Back re-renders it every time, which is what the
// popup did before this router existed (index.js registered
// renderWalletList() as setRenderMain(), and goBack() called it on every
// Back onto "main"). It must stay that way — the wallet list has to reflect
// what changed while the user was away from it, such as a wallet renamed or
// an address removed in Settings — and Home holds no unsaved input to lose.
const ALWAYS_RENDER_ON_BACK = new Set(["main"]);
// Views that render an address the user picked and cannot be rendered
// without one.
const ADDRESS_VIEWS = new Set([
"address",
"address-token",
"receive",
"transaction",
]);
function needsAddress(view) {
return ADDRESS_VIEWS.has(view);
}
function hasValidAddress(state) {
return Boolean(
state.selectedWallet !== null &&
state.selectedAddress !== null &&
state.wallets[state.selectedWallet] &&
state.wallets[state.selectedWallet].addresses[state.selectedAddress],
);
}
// Render `view` from persisted state. Each view module shows itself, so a
// true return means the view is both rendered and on screen.
//
// Returns false when the view is not one the popup renders from state, or
// when the state it would render is gone — a token no longer selected, a
// transaction no longer persisted. The caller falls back rather than
// putting an empty template on screen.
function renderView(view, state, views) {
if (!view || !RESTORABLE_VIEWS.has(view)) return false;
if (needsAddress(view) && !hasValidAddress(state)) return false;
if (view === "address-token" && !state.selectedToken) return false;
const data = state.viewData || {};
switch (view) {
case "main":
views.main.show();
return true;
case "address":
views.addressDetail.show();
return true;
case "address-token":
views.addressToken.show();
return true;
case "receive":
views.receive.show();
return true;
case "settings":
views.settings.show();
return true;
case "settings-addtoken":
views.settingsAddToken.show();
return true;
case "confirm-tx":
if (!data.pendingTx) return false;
views.confirmTx.restore();
return true;
case "transaction":
if (!data.tx) return false;
views.transactionDetail.render();
return true;
case "wait-tx":
// Resumes the receipt poll from the persisted broadcast time,
// and answers false when there is nothing resumable left.
return Boolean(views.txStatus.restoreWait());
case "success-tx":
if (!data.hash) return false;
views.txStatus.renderSuccess();
return true;
case "error-tx":
if (!data.message) return false;
views.txStatus.renderError();
return true;
default:
return false;
}
}
// The Back-path renderer, registered with setBackRenderer() in
// views/helpers.js.
//
// Returns false — leaving goBack() to unhide the view, as it always did —
// in the two cases where the view is known to be on the page already:
//
// - It is not one the popup renders from persisted state. The restored
// stack is filtered against RESTORABLE_VIEWS, so such a view can only
// be on the stack from this page load, where forward navigation
// rendered it on the way in.
// - This page load has rendered it. Re-rendering would re-fetch and
// clobber what it holds; Home is rendered anyway, see above.
//
// What is left is the case the router exists for: a view on the stack that
// this page load has never rendered, whose template is still blank.
function makeBackRenderer(state, views) {
return function renderBack(view) {
if (!RESTORABLE_VIEWS.has(view)) return false;
if (renderedViews.has(view) && !ALWAYS_RENDER_ON_BACK.has(view)) {
return false;
}
if (!renderView(view, state, views)) {
views.main.show();
}
return true;
};
}
module.exports = {
renderView,
makeBackRenderer,
markViewRendered,
resetRenderedViews,
};

View File

@@ -7,6 +7,7 @@ const {
getAddressValueUsd, getAddressValueUsd,
} = require("../../shared/prices"); } = require("../../shared/prices");
const { state, saveState, currentNetwork } = require("../../shared/state"); const { state, saveState, currentNetwork } = require("../../shared/state");
const { markViewRendered } = require("../viewRouter");
// When views are added, removed, or transitions between them change, // When views are added, removed, or transitions between them change,
// update the view-navigation documentation in README.md to match. // update the view-navigation documentation in README.md to match.
@@ -76,6 +77,10 @@ function showView(name) {
} }
clearFlash(); clearFlash();
state.currentView = name; state.currentView = name;
// A view's show() ends here, so this is where the Back path learns the
// view is no longer the blank template from index.html and must not be
// rendered a second time. See viewRouter.js.
markViewRendered(name);
saveState(); saveState();
updateDebugBanner(name); updateDebugBanner(name);
} }
@@ -111,12 +116,19 @@ function updateDebugBanner(viewName) {
} }
} }
// Callback to re-render the main/home view when navigating back to it. // Callback that renders a view being navigated BACK onto. Set once by
// Set once by index.js via setRenderMain(). // index.js via setBackRenderer(), which routes the view through the same
let _renderMain = null; // per-view render and data guards restoreView() uses.
//
// It answers true when it took the navigation — the view is rendered and
// shown, or its backing data was gone and it fell back — and false for a
// view the popup does not render from persisted state. Those can only be
// on the stack from this page load, because the stack is filtered on load,
// so they have already been rendered and only need unhiding.
let _renderBack = null;
function setRenderMain(fn) { function setBackRenderer(fn) {
_renderMain = fn; _renderBack = fn;
} }
// Push the current view onto the navigation stack so goBack() can // Push the current view onto the navigation stack so goBack() can
@@ -136,9 +148,11 @@ function goBack() {
} else { } else {
target = "main"; target = "main";
} }
if (target === "main" && _renderMain) { // A popped view is landed on, not navigated to. If the popup has been
_renderMain(); // closed and reopened since the view was pushed, nothing has ever
} // rendered it in this page load and its template is still blank, so it
// has to be rendered here rather than merely unhidden.
if (_renderBack && _renderBack(target)) return;
showView(target); showView(target);
} }
@@ -470,7 +484,7 @@ module.exports = {
showView, showView,
onViewLeave, onViewLeave,
updateDebugBanner, updateDebugBanner,
setRenderMain, setBackRenderer,
pushCurrentView, pushCurrentView,
goBack, goBack,
clearViewStack, clearViewStack,

View File

@@ -0,0 +1,329 @@
// Back after reopening the popup (#268).
//
// A reopened popup renders the wallet list and the one view it restores
// onto; every other view is still the blank static template from
// index.html. goBack() used to only unhide its target, so Back landed on
// that blank template for any view the popup had not rendered in this page
// load. These tests drive the real goBack() with the real router wired to
// recording view modules, so what is asserted is which view render ran —
// the thing that was missing.
//
// The rendering itself is asserted against the real popup in a real
// browser by tests/e2e/run.js; here the DOM is a stub, because goBack()
// only needs showView() to work.
const els = new Map();
function fakeEl() {
return {
textContent: "",
innerHTML: "",
classList: {
toggle() {},
add() {},
remove() {},
contains: () => false,
},
remove() {},
};
}
globalThis.document = {
getElementById(id) {
if (!els.has(id)) els.set(id, fakeEl());
return els.get(id);
},
};
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const {
showView,
goBack,
setBackRenderer,
pushCurrentView,
} = require("../src/popup/views/helpers");
const {
makeBackRenderer,
markViewRendered,
resetRenderedViews,
} = require("../src/popup/viewRouter");
const { state } = require("../src/shared/state");
const ADDRESS = "0x1111111111111111111111111111111111111111";
const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
let calls;
// Stand-ins for the view modules. Each records itself and then shows its
// view, which is what every real view render ends with — so the assertions
// can tell "rendered and shown" apart from "merely unhidden".
function recorder(name, view) {
return () => {
calls.push(name);
showView(view);
};
}
function makeViews() {
return {
main: { show: recorder("main", "main") },
addressDetail: { show: recorder("addressDetail", "address") },
addressToken: { show: recorder("addressToken", "address-token") },
receive: { show: recorder("receive", "receive") },
settings: { show: recorder("settings", "settings") },
settingsAddToken: {
show: recorder("settingsAddToken", "settings-addtoken"),
},
confirmTx: { restore: recorder("confirmTx", "confirm-tx") },
transactionDetail: {
render: recorder("transactionDetail", "transaction"),
},
txStatus: {
restoreWait: () => {
calls.push("waitTx");
showView("wait-tx");
return true;
},
renderSuccess: recorder("successTx", "success-tx"),
renderError: recorder("errorTx", "error-tx"),
},
};
}
// The popup as it stands just after a reopen: one wallet with one address,
// the view the popup restored onto, and the stack behind it.
//
// A reopen is a fresh page load, so the record of what has been rendered
// starts empty — that emptiness is what makes the Back path render at all.
// Returns the view modules so a test can drive forward navigation through
// the same recorders the router renders through.
function reopenedOn(view, stack, extra) {
calls = [];
resetRenderedViews();
state.wallets = [
{
name: "Wallet 1",
addresses: [{ address: ADDRESS, balance: "0", tokenBalances: [] }],
},
];
state.selectedWallet = 0;
state.selectedAddress = 0;
state.selectedToken = null;
state.viewData = null;
state.currentView = view;
state.viewStack = stack.slice();
Object.assign(state, extra || {});
// Restoring onto a view renders it, so the reopened popup has that one
// view on the page and nothing else.
markViewRendered(view);
const views = makeViews();
setBackRenderer(makeBackRenderer(state, views));
return views;
}
// The reproduction from the issue, step for step.
describe("Back onto a view the reopened popup never rendered", () => {
test("Back from settings renders the address detail underneath", () => {
reopenedOn("settings", ["main", "address"]);
goBack();
expect(calls).toEqual(["addressDetail"]);
expect(state.currentView).toBe("address");
expect(state.viewStack).toEqual(["main"]);
});
test("Back onto the token detail renders it", () => {
reopenedOn("settings", ["main", "address", "address-token"], {
selectedToken: TOKEN,
});
goBack();
expect(calls).toEqual(["addressToken"]);
expect(state.currentView).toBe("address-token");
});
test("Back onto Receive renders it", () => {
reopenedOn("settings", ["main", "address", "receive"]);
goBack();
expect(calls).toEqual(["receive"]);
expect(state.currentView).toBe("receive");
});
test("Back onto the transaction detail renders it", () => {
reopenedOn("settings", ["main", "transaction"], {
viewData: { tx: { hash: "0xdead" } },
});
goBack();
expect(calls).toEqual(["transactionDetail"]);
expect(state.currentView).toBe("transaction");
});
test("Back onto the transaction confirmation restores it", () => {
reopenedOn("settings", ["main", "confirm-tx"], {
viewData: { pendingTx: { to: ADDRESS, amount: "1" } },
});
goBack();
expect(calls).toEqual(["confirmTx"]);
expect(state.currentView).toBe("confirm-tx");
});
test("Back onto the success screen renders it", () => {
reopenedOn("settings", ["main", "success-tx"], {
viewData: { hash: "0xdead" },
});
goBack();
expect(calls).toEqual(["successTx"]);
expect(state.currentView).toBe("success-tx");
});
test("Back onto the failure screen renders it", () => {
reopenedOn("settings", ["main", "error-tx"], {
viewData: { message: "execution reverted" },
});
goBack();
expect(calls).toEqual(["errorTx"]);
expect(state.currentView).toBe("error-tx");
});
test("Back onto Home renders the wallet list", () => {
reopenedOn("settings", ["main"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("Back with an empty stack renders Home", () => {
reopenedOn("settings", []);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
});
// The guards are restoreView()'s, so a popped view whose backing data is
// gone lands on Home rather than on an empty template.
describe("Back onto a view whose backing data is gone", () => {
test("the token detail with no token selected falls back to Home", () => {
reopenedOn("settings", ["main", "address-token"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("the transaction detail with no transaction falls back to Home", () => {
reopenedOn("settings", ["main", "transaction"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("the confirmation with no pending transaction falls back to Home", () => {
reopenedOn("settings", ["main", "confirm-tx"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("an address view with no address selected falls back to Home", () => {
reopenedOn("settings", ["main", "receive"], {
selectedAddress: null,
});
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("the success screen with no transaction hash falls back to Home", () => {
reopenedOn("settings", ["main", "success-tx"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("the failure screen with no message falls back to Home", () => {
reopenedOn("settings", ["main", "error-tx"]);
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
test("a wait that can no longer be resumed falls back to Home", () => {
reopenedOn("settings", ["main", "wait-tx"]);
const views = makeViews();
views.txStatus.restoreWait = () => false;
setBackRenderer(makeBackRenderer(state, views));
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
});
// Forward navigation renders as it goes, and a second render would re-fetch
// and clobber whatever the view holds — an unsaved edit, a request in
// flight. So the Back path renders only a view this page load has never
// rendered, and merely unhides every other one: the views it does not
// render from persisted state, and the views already on the page.
describe("what the Back path leaves alone", () => {
test("forward navigation renders nothing by itself", () => {
reopenedOn("address", ["main"]);
pushCurrentView();
showView("send");
expect(calls).toEqual([]);
expect(state.viewStack).toEqual(["main", "address"]);
});
test("Back onto a live-session view only unhides it", () => {
reopenedOn("confirm-tx", ["main", "address", "send"]);
goBack();
expect(calls).toEqual([]);
expect(state.currentView).toBe("send");
});
test("Back renders its target exactly once", () => {
reopenedOn("settings", ["main", "address"]);
goBack();
expect(calls.filter((c) => c === "addressDetail")).toHaveLength(1);
});
test("Back onto a view this page load already rendered only unhides it", () => {
const views = reopenedOn("main", []);
pushCurrentView();
views.addressDetail.show();
pushCurrentView();
views.settings.show();
calls = [];
goBack();
expect(calls).toEqual([]);
expect(state.currentView).toBe("address");
});
// The unit mirror of the regression the browser suite pins: Settings
// reassigns its fields from persisted state on every render, so a
// re-render on the way back discards an edit the user has not saved.
test("Back onto Settings visited earlier in this page load does not re-render it", () => {
const views = reopenedOn("main", []);
pushCurrentView();
views.settings.show();
pushCurrentView();
views.settingsAddToken.show();
calls = [];
goBack();
expect(calls).toEqual([]);
expect(state.currentView).toBe("settings");
});
// Home is the deliberate exception, unchanged from the popup's
// behaviour before the router existed: it re-renders on every Back so
// the wallet list reflects what changed while the user was away.
test("Back onto Home renders it again even when it is already on the page", () => {
const views = reopenedOn("main", []);
pushCurrentView();
views.addressDetail.show();
calls = [];
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
});
});

View File

@@ -30,6 +30,8 @@ const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ORIGIN = "https://dapp.example"; const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example"; const HOSTNAME = "dapp.example";
// A page the wallet has never been connected to, whose requests are refused.
const UNCONNECTED_ORIGIN = "https://stranger.example";
const EXT_URL = "chrome-extension://autistmask/"; const EXT_URL = "chrome-extension://autistmask/";
// 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
@@ -51,10 +53,16 @@ const MESSAGE = "0x48656c6c6f204175746973744d61736b";
// The transaction the background populates and the approval screen displays. // The transaction the background populates and the approval screen displays.
// The nonce is a parameter because the duplicate case turns on two artifacts // The nonce is a parameter because the duplicate case turns on two artifacts
// differing in a field the dApp fixed nothing for. // differing in a field the dApp fixed nothing for.
function populated(nonce) { // The two chains the tests switch between, as both forms the code uses: the
// hex chain id the wallet's network record carries, and the number the node
// and the signed artifact carry.
const MAINNET = { hex: "0x1", num: 1 };
const SEPOLIA = { hex: "0xaa36a7", num: 11155111 };
function populated(nonce, chainId) {
return { return {
type: 2, type: 2,
chainId: 1, chainId: chainId || MAINNET.num,
nonce, nonce,
gasLimit: 100000n, gasLimit: 100000n,
maxFeePerGas: 2000000000n, maxFeePerGas: 2000000000n,
@@ -65,17 +73,17 @@ function populated(nonce) {
}; };
} }
function signedAtNonce(nonce, withWallet) { function signedAtNonce(nonce, withWallet, chainId) {
return (withWallet || signer).signTransaction(populated(nonce)); return (withWallet || signer).signTransaction(populated(nonce, chainId));
} }
// The node the background populates against. Its answers are the numbers the // The node the background populates against. Its answers are the numbers the
// approval screen shows, so they are also the numbers every artifact below is // approval screen shows, so they are also the numbers every artifact below is
// signed at. // signed at.
function fakeProvider(broadcastTransaction, overrides) { function fakeProvider(broadcastTransaction, overrides, chainId) {
return { return {
broadcastTransaction, broadcastTransaction,
getNetwork: async () => Network.from(1), getNetwork: async () => Network.from(chainId || MAINNET.num),
getTransactionCount: async () => NONCE, getTransactionCount: async () => NONCE,
estimateGas: async () => 100000n, estimateGas: async () => 100000n,
getFeeData: async () => ({ getFeeData: async () => ({
@@ -111,14 +119,20 @@ function loadBackground(options) {
const broadcastTransaction = jest.fn(); const broadcastTransaction = jest.fn();
const loadState = jest.fn(opts.loadState || (async () => {})); const loadState = jest.fn(opts.loadState || (async () => {}));
// The network the wallet is on, which the tests switch under a pending
// approval. The node the transaction is populated against is on the same
// one, as it would be: switching networks switches the RPC endpoint too.
let chain = MAINNET;
jest.doMock("../src/shared/state", () => ({ jest.doMock("../src/shared/state", () => ({
state: { rpcUrl: "https://rpc.invalid", wallets: [] }, state: { rpcUrl: "https://rpc.invalid", wallets: [] },
loadState, loadState,
saveState: jest.fn(async () => {}), saveState: jest.fn(async () => {}),
currentNetwork: () => ({ chainId: "0x1" }), currentNetwork: () => ({ chainId: chain.hex }),
})); }));
jest.doMock("../src/shared/balances", () => ({ jest.doMock("../src/shared/balances", () => ({
getProvider: () => fakeProvider(broadcastTransaction, opts.provider), getProvider: () =>
fakeProvider(broadcastTransaction, opts.provider, chain.num),
refreshBalances: jest.fn(async () => {}), refreshBalances: jest.fn(async () => {}),
})); }));
jest.doMock("../src/shared/phishingDomains", () => ({ jest.doMock("../src/shared/phishingDomains", () => ({
@@ -170,7 +184,9 @@ function loadBackground(options) {
getLastFocused: (cb) => cb(null), getLastFocused: (cb) => cb(null),
create: (options2, cb) => { create: (options2, cb) => {
created.push(options2); created.push(options2);
cb({ id: created.length }); // A browser that answers with no window at all. The approval
// then has no window it can ever be answered in.
cb(opts.noWindow ? undefined : { id: created.length });
}, },
remove: (id, cb) => { remove: (id, cb) => {
removed.push(id); removed.push(id);
@@ -204,7 +220,7 @@ function loadBackground(options) {
// Raise a pending transaction approval the way a dApp does, and dig the // Raise a pending transaction approval the way a dApp does, and dig the
// approval id back out of the popup URL the background opened. // approval id back out of the popup URL the background opened.
function requestTx(txParams) { function requestTx(txParams, origin) {
let rpcResult = null; let rpcResult = null;
// The window this request opens, if it opens one. A request refused // The window this request opens, if it opens one. A request refused
// before an approval is raised opens none, and the window belonging to // before an approval is raised opens none, and the window belonging to
@@ -219,7 +235,7 @@ function loadBackground(options) {
method: "eth_sendTransaction", method: "eth_sendTransaction",
params: [txParams || TX_PARAMS], params: [txParams || TX_PARAMS],
}, },
{ origin: ORIGIN }, { origin: origin || ORIGIN },
sendResponse, sendResponse,
); );
return { return {
@@ -278,6 +294,10 @@ function loadBackground(options) {
setActiveAddress: (address) => { setActiveAddress: (address) => {
persisted.activeAddress = address; persisted.activeAddress = address;
}, },
// The user switching network in the toolbar popup.
setNetwork: (network) => {
chain = network;
},
fromPopup: { url: EXT_URL + "src/popup/index.html" }, fromPopup: { url: EXT_URL + "src/popup/index.html" },
}; };
} }
@@ -477,7 +497,7 @@ describe("one transaction approval at a time", () => {
error: { error: {
code: -32002, code: -32002,
message: expect.stringMatching( message: expect.stringMatching(
/already waiting to be approved/, /one transaction at a time.+already in progress/,
), ),
}, },
}); });
@@ -502,6 +522,103 @@ describe("one transaction approval at a time", () => {
expect(first.result()).toEqual({ result: "0xfeed" }); expect(first.result()).toEqual({ result: "0xfeed" });
}); });
// The slot is only defensible if the wallet was going to raise an approval
// anyway. Taken any earlier, a request the wallet refuses outright still
// holds it, and any page at all — connected or not — can deny the user's
// own transactions for as long as it keeps asking.
test("a request the wallet refuses does not take the slot from the connected site", async () => {
const bg = loadBackground();
// Both delivered before either reaches its first suspension point,
// which is the interleaving the slot exists for.
const stranger = bg.requestTx(TX_PARAMS, UNCONNECTED_ORIGIN);
const connected = bg.requestTx();
await settle();
expect(stranger.result()).toEqual({
error: { code: 4100, message: "Unauthorized" },
});
// The connected site's transaction was raised, not refused as one the
// user already has in progress.
expect(connected.result()).toBeNull();
expect(connected.id()).toBeTruthy();
expect(bg.created).toHaveLength(1);
});
// The user closes an approval window that looks hung while the attempt
// behind it is still running, and that attempt then fails in a way that
// would normally leave the approval standing for a retry. There is no
// window left to retry in, so leaving it standing answers the requesting
// page never — and holds the slot for the life of the worker with it.
test("an approval whose window closed under a failed attempt is answered, and frees the next request", async () => {
const stalled = deferred();
const bg = loadBackground({
loadState: async () => {
await stalled.promise;
throw new Error("The wallet data could not be read.");
},
});
const first = bg.requestTx();
await settle();
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
// The attempt owns the approval, so closing the window does not settle
// it: the attempt may yet broadcast, and it is the one that reports.
bg.closeWindow(1);
await settle();
expect(first.result()).toBeNull();
stalled.resolve();
await settle();
expect(first.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
const second = bg.requestTx();
await settle();
expect(second.result()).toBeNull();
expect(second.id()).toBeTruthy();
expect(bg.created).toHaveLength(2);
});
// An approval with no window is one nothing can ever answer.
test("a request whose approval window cannot be opened is answered rather than left waiting", async () => {
const bg = loadBackground({ noWindow: true });
const first = bg.requestTx();
await settle();
expect(first.result()).toEqual({
error: {
code: -32603,
message: expect.stringMatching(
/could not open its approval window/,
),
},
});
// And it did not take the slot with it.
const second = bg.requestTx();
await settle();
expect(second.result()).toEqual({
error: {
code: -32603,
message: expect.stringMatching(
/could not open its approval window/,
),
},
});
});
test("an answered approval frees the next request", async () => { test("an answered approval frees the next request", async () => {
const bg = loadBackground(); const bg = loadBackground();
const first = bg.requestTx(); const first = bg.requestTx();
@@ -621,6 +738,51 @@ describe("a nonce collision is reported as a transaction that did not go out", (
}, },
}); });
}); });
// Nonce spaces are per chain, and the wallet switches networks. A nonce
// this wallet spent on one chain says nothing about the same nonce on
// another — and low nonces overlap across chains as a matter of course, so
// a record that ignored the chain would refuse ordinary transactions,
// permanently and with a message that is not true of them.
test("a nonce spent on one chain is not refused on another", async () => {
const bg = loadBackground();
const first = bg.requestTx();
await settle();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(first.result()).toEqual({ result: "0xfeed" });
// The user switches network. On this chain the address has sent
// nothing, so the node populates the next transaction at the same
// nonce — correctly.
bg.setNetwork(SEPOLIA);
const second = bg.requestTx();
await settle();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xbeef" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: second.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE, undefined, SEPOLIA.num),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(2);
expect(second.result()).toEqual({ result: "0xbeef" });
});
}); });
// The approval carries the transaction the user was shown and the address it // The approval carries the transaction the user was shown and the address it

View File

@@ -419,6 +419,172 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
assertWiped(st, env.phrase, "after reopening the popup"); assertWiped(st, env.phrase, "after reopening the popup");
}); });
// ------------------------------- Back after reopening the popup (#268)
// A reopened popup renders the wallet list and the view it restores onto,
// and nothing else: every other screen is still the blank static template
// from index.html. Back used to only unhide its target, which is why these
// have to run against the real popup — the template is present and
// well-formed, so only its emptiness distinguishes the defect, and only a
// real reopen produces it.
// Everything the address screen must have on it, read out of the DOM.
function addressScreenState(page) {
return page.evaluate(() => {
const line = document.getElementById("address-line");
const balances = document.getElementById("address-balances");
return {
hidden: document
.getElementById("view-address")
.classList.contains("hidden"),
line: line ? line.innerText.trim() : "",
balances: balances ? balances.innerText.trim() : "",
};
});
}
// Close and reopen the page rather than reload it: that is what the toolbar
// popup does, and it is the only thing that produces the unrendered views.
async function reopenPopup(env, restoredView) {
await env.page.close();
env.page = await openPopup(env.ctx, env.popupUrl);
await visible(env.page, restoredView);
}
// The reproduction from the issue, step for step.
test("Back after reopening the popup renders the address screen (#268)", async (env) => {
await openAddressDetail(env.page);
const before = await addressScreenState(env.page);
assert(
before.line.length > 0,
"the address screen was blank to begin with",
);
await env.page.click("#btn-settings");
await visible(env.page, "#view-settings");
await reopenPopup(env, "#view-settings");
await env.page.click("#btn-settings-back");
await visible(env.page, "#view-address");
const after = await addressScreenState(env.page);
assert(
after.line === before.line,
"the address line reads " +
JSON.stringify(after.line) +
", expected " +
JSON.stringify(before.line),
);
assert(
after.balances.includes("ETH"),
"the balances read " + JSON.stringify(after.balances),
);
});
// The same defect one screen further in. Receive holds the address twice
// over — as text and as the QR code the sender scans — and a blank one is
// worse than a missing screen.
// Everything the Receive screen must have on it. The QR code is read as
// pixels, not as an element: the blank template carries the canvas too, a
// default 300x150 one with nothing drawn on it and every pixel fully
// transparent. A drawn QR paints an opaque background across the whole
// canvas, so a single opaque pixel is the whole question.
function receiveScreenState(page) {
return page.evaluate(() => {
const block = document.getElementById("receive-address-block");
const canvas = document.getElementById("receive-qr");
const px = canvas
.getContext("2d")
.getImageData(0, 0, canvas.width, canvas.height).data;
let opaque = 0;
for (let i = 3; i < px.length; i += 4) {
if (px[i] > 0) opaque += 1;
}
return {
address: block.dataset.full || "",
text: block.innerText.trim(),
qrOpaquePixels: opaque,
};
});
}
test("Back after reopening the popup renders the Receive screen (#268)", async (env) => {
await openAddressDetail(env.page);
await env.page.click("#btn-receive");
await visible(env.page, "#view-receive");
const before = await receiveScreenState(env.page);
assert(
/^0x[0-9a-fA-F]{40}$/.test(before.address),
"Receive showed no address to begin with: " +
JSON.stringify(before.address),
);
await env.page.click("#btn-settings");
await visible(env.page, "#view-settings");
await reopenPopup(env, "#view-settings");
await env.page.click("#btn-settings-back");
await visible(env.page, "#view-receive");
const shown = await receiveScreenState(env.page);
assert(
shown.address === before.address,
"Receive shows " +
JSON.stringify(shown.address) +
", expected " +
JSON.stringify(before.address),
);
assert(
shown.text.includes(before.address),
"the Receive address is not on screen: " + JSON.stringify(shown.text),
);
assert(shown.qrOpaquePixels > 0, "Receive shows an unpainted QR code");
// Leave the suite where it found it.
await env.page.click("#btn-receive-back");
await visible(env.page, "#view-address");
await env.page.click("#btn-address-back");
await visible(env.page, "#view-main");
});
// The other half of the requirement: Back renders a screen this page load
// never rendered, and must NOT re-render one it already has on screen.
// settings.show() reassigns #settings-rpc from persisted state, so
// re-rendering Settings on the way back would silently revert whatever the
// user typed and had not saved yet — and they could then press Save and
// store the value they believed they had replaced. No reopen here: this is
// an ordinary in-session forward-and-back, which is exactly why the render
// must not happen.
test("Back onto Settings keeps unsaved input (#268)", async (env) => {
await visible(env.page, "#view-main");
await env.page.click("#btn-settings");
await visible(env.page, "#view-settings");
const typed = "https://rpc.example.invalid/unsaved";
await env.page.fill("#settings-rpc", typed);
await env.page.click("#btn-settings-add-token");
await visible(env.page, "#view-settings-addtoken");
await env.page.click("#btn-settings-addtoken-back");
await visible(env.page, "#view-settings");
const kept = await env.page.inputValue("#settings-rpc");
assert(
kept === typed,
"the unsaved RPC URL reads " +
JSON.stringify(kept) +
", expected " +
JSON.stringify(typed),
);
// Leave the suite where it found it. The typed value was never saved,
// and Settings reloads the field from state next time it renders.
await env.page.click("#btn-settings-back");
await visible(env.page, "#view-main");
});
// -------------------------------------------- address removal (#162) // -------------------------------------------- address removal (#162)
// Number of address rows across every wallet in the list, counted in the DOM // Number of address rows across every wallet in the list, counted in the DOM