Compare commits

...

2 Commits

Author SHA1 Message Date
d38a1ec917 fix: one wording for a rejected password on every screen (closes #172)
All checks were successful
check / check (push) Successful in 42s
The send confirmation and the delete-wallet confirmation rendered
"Wrong password." — a fragment, which README Language & Labeling and
RULES.md:120 both forbid — while the two reveal screens said "That
password is not correct." and the two dApp approval paths said "That
password is incorrect." Three wordings for one condition, on screens a
user can reach minutes apart.

All six decryptWithPassword call sites, across the five views that own
one, now show the wording the approval paths introduced:

    That password is incorrect. Please try again.

Strings only. Nothing about how a wrong password is handled changes: it
still fails closed on every screen, and the approval paths' settlement,
claim/release interlock and retry behaviour are untouched.

The new test scans the source for the call sites rather than driving
each view, because the invariant is about the set: a seventh call site
has to join it, and a per-view test cannot notice a screen nobody wrote
one for. It asserts per call site, not per file — each decrypt is read
back to its own catch handler and the prose that handler shows must be
the canonical sentence and nothing else. approval.js decrypts twice and
is where the divergence came from, so a per-file check that only asks
whether the sentence appears somewhere in the file passes while one of
those two says something novel. Exact equality catches a new wording,
not only a known-superseded one.
2026-08-12 09:48:36 +00:00
5af89a1b63 test: drive ConfirmTx in the e2e suite, gate assertion included (closes #238)
All checks were successful
check / check (push) Successful in 38s
ConfirmTx -- the screen that decides what gets signed -- had no automated
coverage of its own behaviour. The arithmetic underneath was well tested; the
wiring was not, so a mutant making the spend gate read the displayed fee
estimate instead of the reserve would have reintroduced the #154 overspend with
the suite still green.

Nine end-to-end tests now drive it for both the native and ERC-20 paths,
covering the pending, funded, over-balance and estimate-failed states, and
asserting that the gate reads the reserve rather than the estimate. Swapping the
two makes the suite fail. The view height is asserted constant across every
state transition rather than merely printed.

Reaching the screen needs a funded balance and a gas estimate, so the route
interception gains fixtures for both. Testing the estimate-failed state means
provoking the console error the code is supposed to emit, which the harness
otherwise fails a run on; an expectation mechanism consumes exactly one matching
record, is scoped to the declaring test, and fails that test if nothing matched,
so it cannot mask an unrelated error.
2026-08-12 11:35:20 +02:00
11 changed files with 1087 additions and 28 deletions

View File

@@ -146,6 +146,23 @@ fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
offline; unrecognised outbound requests are reported as failures rather than
silently allowed.
It also covers the confirmation screen, for both a native ETH send and an ERC-20
send: Send disabled while the fee estimate is in flight, enabled once it lands,
the fee block quoting the expected cost and the reserve separately, the distinct
message for an estimate that failed, and the view height staying constant across
every one of those transitions. The load-bearing one is that the spend gate uses
the **reserve** and not the displayed **estimate** — the two are stubbed far
apart on purpose, and the funded and refused sends sit on opposite sides of the
reserve while sitting on the same side of the estimate, so swapping the two in
`src/popup/views/confirmTx.js` fails the suite instead of passing it. That is
what [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) was, and it was
previously correct by reading only.
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
consumes exactly one matching record, and a declaration nothing matched fails
its test just as an undeclared error does.
That reporting has one bound worth knowing. Observation ends when the browser
context is torn down, and nothing can watch traffic after that, so the run keeps
collecting for a fixed grace period after the last test returns

16
TODO.md
View File

@@ -44,6 +44,22 @@ undefined identifiers, which is how
# Completed Steps
- 2026-08-12: One wording for a rejected password on every screen that asks for
one — the send confirmation and the delete-wallet confirmation no longer say
"Wrong password." (a fragment, which `RULES.md` Language & Labeling forbids)
and the two reveal screens no longer say "not correct", so all five
`decryptWithPassword` call sites now show the sentence the dApp approval paths
introduced. Strings only, no behaviour change, and each error container
measured at a 360px viewport in the pinned Playwright container
([#172](https://git.eeqj.de/sneak/AutistMask/issues/172)).
- 2026-08-12: The transaction confirmation screen has browser coverage. The
end-to-end suite reaches ConfirmTx for both the native ETH and the ERC-20 path
off a funded-balance fixture, and asserts the pending, funded, over-balance
and estimate-failed states, the fee block quoting the estimate and the reserve
separately, and a constant view height across every one of those transitions.
The load-bearing assertion is that the spend gate reads the reserve and not
the displayed estimate: swapping the two fails the suite
([#238](https://git.eeqj.de/sneak/AutistMask/issues/238)).
- 2026-08-12: The dust threshold field now explains a rejection instead of
snapping back in silence, with the parse in a pure, unit-tested module that
accepts plain decimal digits only — hex and exponent notation are refused

View File

@@ -422,7 +422,10 @@ function init(ctx) {
password,
);
} catch (e) {
showError("confirm-tx-password-error", "Wrong password.");
showError(
"confirm-tx-password-error",
"That password is incorrect. Please try again.",
);
return;
}

View File

@@ -74,7 +74,8 @@ function init(_ctx) {
try {
await decryptWithPassword(wallet.encryptedSecret, pw);
} catch (_e) {
$("delete-wallet-flash").textContent = "Wrong password.";
$("delete-wallet-flash").textContent =
"That password is incorrect. Please try again.";
$("delete-wallet-flash").style.visibility = "visible";
btn.disabled = false;
btn.classList.remove("text-muted");

View File

@@ -144,7 +144,7 @@ async function reveal() {
$("export-privkey-flash").style.visibility = "hidden";
} catch {
if (!isCurrentReveal(generation)) return;
fail("That password is not correct. Please try again.");
fail("That password is incorrect. Please try again.");
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");

View File

@@ -126,7 +126,7 @@ async function reveal() {
if (!isCurrentReveal(generation)) return;
// Deliberately not the caught error: the message is fixed so that
// nothing derived from the ciphertext or the attempt can surface.
fail("That password is not correct. Please try again.");
fail("That password is incorrect. Please try again.");
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");

View File

@@ -53,15 +53,47 @@ function isAllowed(text) {
// after that — the route handler and the console listeners are gone with
// the context — so there is no post-teardown phase to collect, and this
// class deliberately offers no mechanism pretending to cover one.
//
// One narrow exception exists, and it is not a mute: expect(). A test that
// drives a failure path on purpose — a refused gas estimate, say — provokes
// the console.error the code is supposed to emit, and that error is the
// behaviour under test rather than an escape. Declaring it consumes exactly
// one matching record and no more, and an expectation nothing matched fails
// its test just as an unexpected error does. So it cannot be used to
// silence anything: it can only assert that a specific error happened.
class ErrorCollector {
constructor() {
this.entries = [];
this.taken = 0;
this.expectations = [];
}
// Declare a console.error this test is about to cause deliberately.
// `label` names it in the failure message if it never arrives.
expect(label, pattern) {
this.expectations.push({ label, pattern, matched: false });
}
// Declared expectations that nothing matched, clearing the list so each
// test starts with none outstanding.
unmatchedExpectations() {
const out = this.expectations
.filter((e) => !e.matched)
.map((e) => e.label);
this.expectations = [];
return out;
}
record(kind, text) {
const line = kind + ": " + String(text).split("\n")[0];
if (isAllowed(line)) return;
const expected = this.expectations.find(
(e) => !e.matched && e.pattern.test(line),
);
if (expected) {
expected.matched = true;
return;
}
this.entries.push(line);
}
@@ -290,13 +322,18 @@ async function createWallet(page) {
return phrase;
}
// Reach the address detail screen from wherever the popup restored to.
// Clicking .address-row does not open it; the [info] button does.
// Reach the address detail screen of the FIRST address of the first wallet,
// from wherever the popup restored to. Clicking .address-row does not open
// it; the [info] button does.
//
// .first() rather than a bare selector because the suite adds a second
// wallet partway through, and every later test would otherwise die in
// Playwright's strict mode rather than on an assertion.
async function openAddressDetail(page) {
const onAddress = await page.isVisible("#view-address");
if (!onAddress) {
await visible(page, "#view-main");
await page.click("#wallet-list .btn-addr-info");
await page.locator("#wallet-list .btn-addr-info").first().click();
}
await visible(page, "#view-address");
}

View File

@@ -53,18 +53,96 @@ const STUB_TX_TIMESTAMP = "2026-01-02T03:04:05.000000Z";
// log.errorf(), i.e. console.error, which fails the run on its own.
const ZERO_WORD = "0x" + "0".repeat(64);
function hex(value) {
return "0x" + BigInt(value).toString(16);
}
// A bigint as a 32-byte ABI word.
function word(value) {
return "0x" + BigInt(value).toString(16).padStart(64, "0");
}
// ------------------------------------------------------------ fee fixture
//
// The confirmation screen carries two different numbers for the same
// transaction and may gate on only one of them:
//
// reserve = gasLimit * maxFeePerGas — what a node requires to be
// available for a type-2 transaction, and what the spend gate
// must use.
// estimate = gasLimit * gasPrice — what the transfer is expected to
// actually cost. Display only.
//
// Issue #154 was the gate reading the smaller of the two. ethers derives
// maxFeePerGas as baseFeePerGas * 2 + maxPriorityFeePerGas, so the numbers
// below put the reserve at very nearly twice the estimate. That gap is the
// entire point of these values: it leaves room for a send that an
// estimate-based gate accepts and a reserve-based gate refuses, which is
// what lets the ConfirmTx tests tell the two apart at all. Collapse the gap
// — by dropping baseFeePerGas from the block below, say — and those tests
// go on passing while asserting nothing.
const GAS_LIMIT = 21000n;
const BASE_FEE_WEI = 100000000000n; // 100 gwei
const PRIORITY_FEE_WEI = 1000000000n; // 1 gwei
const GAS_PRICE_WEI = BASE_FEE_WEI + PRIORITY_FEE_WEI; // 101 gwei
const MAX_FEE_WEI = BASE_FEE_WEI * 2n + PRIORITY_FEE_WEI; // 201 gwei
const FEE_ESTIMATE_WEI = GAS_LIMIT * GAS_PRICE_WEI; // 0.002121 ETH
const FEE_RESERVE_WEI = GAS_LIMIT * MAX_FEE_WEI; // 0.004221 ETH
const RPC_RESULTS = {
eth_chainId: "0x1",
net_version: "1",
eth_blockNumber: "0x1406f40",
eth_getBalance: "0x0",
eth_call: ZERO_WORD,
eth_gasPrice: "0x3b9aca00",
eth_estimateGas: "0x5208",
eth_getCode: "0x",
eth_gasPrice: hex(GAS_PRICE_WEI),
eth_estimateGas: hex(GAS_LIMIT),
eth_getTransactionCount: "0x0",
eth_maxPriorityFeePerGas: "0x3b9aca00",
eth_maxPriorityFeePerGas: hex(PRIORITY_FEE_WEI),
};
// The "latest" block, which ethers' getFeeData() reads baseFeePerGas from
// to derive maxFeePerGas. Without it every fee is a legacy gasPrice, the
// reserve and the estimate collapse to the same number, and the gate tests
// stop being able to distinguish them.
function latestBlock() {
return {
hash: "0x" + "11".repeat(32),
parentHash: "0x" + "22".repeat(32),
number: hex(STUB_BLOCK_NUMBER),
timestamp: hex(1767326645),
nonce: "0x0000000000000000",
difficulty: "0x0",
gasLimit: "0x1c9c380",
gasUsed: "0xf4240",
miner: STUB_COUNTERPARTY,
extraData: "0x",
baseFeePerGas: hex(BASE_FEE_WEI),
transactions: [],
};
}
// keccak("decimals()")[0:4].
const SELECTOR_DECIMALS = "0x313ce567";
// Every eth_call still answers with a zero word except decimals() on the
// stub token. ethers reads that before it can encode an ERC-20 transfer,
// and a zero there makes parseUnits() reject any fractional amount — so the
// ERC-20 confirmation path would fail its gas estimate for a reason that
// has nothing to do with what is being tested.
function ethCallResult(req) {
const call = Array.isArray(req.params) ? req.params[0] : null;
if (!call || typeof call !== "object") return ZERO_WORD;
const data = String(call.data || call.input || "").toLowerCase();
const to = String(call.to || "").toLowerCase();
if (data.startsWith(SELECTOR_DECIMALS) && to === STUB_TOKEN.address) {
return word(STUB_TOKEN.decimals);
}
return ZERO_WORD;
}
function tokenObject() {
return {
address_hash: STUB_TOKEN.address,
@@ -92,6 +170,18 @@ function tokenTransferItems(address) {
];
}
// 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
// the only way the confirmation screen's ERC-20 path can be reached.
function tokenBalanceItems() {
return [
{
value: "1500000",
token: tokenObject(),
},
];
}
// Full details for STUB_TX_HASH. raw_input is "0x" so the calldata
// decoder short-circuits; the on-chain detail fields still populate.
function transactionDetails() {
@@ -121,7 +211,82 @@ function blockscoutAddress(pathname) {
return m ? m[1] : null;
}
function handleRpc(route, postData, report) {
function sleep(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}
// How long a deliberately held reply is allowed to stay held, and how often
// the release flag is re-read while it is.
const HOLD_POLL_MS = 25;
const HOLD_MAX_MS = 30000;
// Hold a gas estimate open for as long as the test asks.
//
// opts.holdGasEstimate is read here rather than captured, so a test flips it
// on the same options object the route was registered with — the same
// pattern as seedTokenTransfer. This is the only way to observe the
// confirmation screen while its estimate is genuinely in flight; sampling
// the screen and hoping to win a race against the network would assert
// nothing on a slow machine.
//
// It never gives up quietly. A hold that outlives the bound is reported like
// any other harness fault, because a "pending" state that stopped being
// pending on its own is a green assertion about the wrong screen.
async function awaitRelease(opts, report) {
const started = Date.now();
while (opts.holdGasEstimate) {
if (Date.now() - started > HOLD_MAX_MS) {
report(
"held gas estimate was never released after " +
HOLD_MAX_MS +
"ms",
);
return;
}
await sleep(HOLD_POLL_MS);
}
}
// One JSON-RPC reply. Methods whose answer depends on a fixture a test has
// set, or on the call itself, are resolved here; every other method is a
// constant in RPC_RESULTS.
function rpcReply(req, opts, report) {
const envelope = { jsonrpc: "2.0", id: req.id };
if (req.method === "eth_getBalance") {
return Object.assign(envelope, {
result: opts.ethBalanceWei || RPC_RESULTS.eth_getBalance,
});
}
if (req.method === "eth_call") {
return Object.assign(envelope, { result: ethCallResult(req) });
}
if (req.method === "eth_getBlockByNumber") {
return Object.assign(envelope, { result: latestBlock() });
}
if (req.method === "eth_estimateGas" && opts.failGasEstimate) {
// A refusal the node itself would produce, not a transport error:
// this is the shape the confirmation screen has to turn into
// "Unable to estimate" rather than into a fee of zero.
return Object.assign(envelope, {
error: {
code: -32000,
message: "e2e fixture: gas required exceeds allowance",
},
});
}
const result = RPC_RESULTS[req.method];
if (result === undefined) {
report("unstubbed RPC method: " + req.method);
return Object.assign(envelope, {
error: { code: -32601, message: "unstubbed in e2e harness" },
});
}
return Object.assign(envelope, { result });
}
async function handleRpc(route, postData, opts, report) {
let payload;
try {
payload = JSON.parse(postData || "null");
@@ -148,18 +313,11 @@ function handleRpc(route, postData, report) {
report("unstubbed request: POST " + route.request().url());
return route.abort();
}
const replies = batch.map((req) => {
const result = RPC_RESULTS[req.method];
if (result === undefined) {
report("unstubbed RPC method: " + req.method);
return {
jsonrpc: "2.0",
id: req.id,
error: { code: -32601, message: "unstubbed in e2e harness" },
};
}
return { jsonrpc: "2.0", id: req.id, result };
});
if (batch.some((req) => req.method === "eth_estimateGas")) {
await awaitRelease(opts, report);
}
const replies = batch.map((req) => rpcReply(req, opts, report));
return jsonResponse(route, Array.isArray(payload) ? replies : replies[0]);
}
@@ -199,6 +357,15 @@ function traceEnabled(raw) {
* @param {boolean} [opts.seedTokenTransfer] serve the stubbed ERC-20
* transfer. Read at request time, so a test can flip it on the same
* options object without re-registering the route.
* @param {boolean} [opts.seedTokenBalance] serve the stubbed ERC-20
* holding, which is what makes the token reachable from the send screen.
* @param {string} [opts.ethBalanceWei] hex wei answered to eth_getBalance;
* defaults to zero, which is what every test that predates the funded
* fixture expects.
* @param {boolean} [opts.failGasEstimate] answer eth_estimateGas with a
* node-side refusal.
* @param {boolean} [opts.holdGasEstimate] hold every batch containing an
* eth_estimateGas until this is cleared again.
* @returns {Promise<{waitForServiceWorkerTraffic: (ms: number) =>
* Promise<string|null>}>}
*/
@@ -241,7 +408,7 @@ async function installNetworkStubs(ctx, opts) {
// JSON-RPC endpoint (any host): a POST with a JSON-RPC body.
if (req.method() === "POST") {
return handleRpc(route, req.postData(), report);
return handleRpc(route, req.postData(), opts, report);
}
// Blockscout v2
@@ -259,7 +426,10 @@ async function installNetworkStubs(ctx, opts) {
});
}
if (/\/addresses\/0x[0-9a-fA-F]{40}\/token-balances$/.test(p)) {
return jsonResponse(route, []);
return jsonResponse(
route,
opts.seedTokenBalance ? tokenBalanceItems() : [],
);
}
if (p.endsWith("/transactions/" + STUB_TX_HASH)) {
return jsonResponse(route, transactionDetails());
@@ -329,6 +499,9 @@ async function installNetworkStubs(ctx, opts) {
module.exports = {
installNetworkStubs,
FEE_ESTIMATE_WEI,
FEE_RESERVE_WEI,
STUB_COUNTERPARTY,
STUB_TOKEN,
STUB_TX_HASH,
};

View File

@@ -9,6 +9,7 @@
"use strict";
const { formatEther } = require("ethers");
const {
PASSWORD,
createWallet,
@@ -18,7 +19,13 @@ const {
pageCompilesWasm,
visible,
} = require("./harness");
const { STUB_TOKEN, STUB_TX_HASH } = require("./network");
const {
FEE_ESTIMATE_WEI,
FEE_RESERVE_WEI,
STUB_COUNTERPARTY,
STUB_TOKEN,
STUB_TX_HASH,
} = require("./network");
const { DUST_THRESHOLD_MESSAGE } = require("../../src/popup/dustThreshold");
const TEST_TIMEOUT_MS = 120000;
@@ -623,6 +630,573 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => {
}
});
// --------------------------------------------- confirmation screen (#238)
//
// The screen that decides what gets signed. The arithmetic underneath it
// lives in src/shared/txValidation.js and is unit tested there; what these
// tests cover is the wiring — which number reaches the gate, when the gate
// re-runs, what the fee block renders, and whether Send is enabled.
//
// The load-bearing one is "gates on the fee RESERVE": the confirmation
// screen quotes the ESTIMATE and gates on the RESERVE, and issue #154 was
// the gate reading the quoted number. Every other assertion here would
// survive that mutation, so the funded and gap sends are deliberately sized
// on opposite sides of the reserve while sitting on the same side of the
// estimate.
// The balance the funded fixture serves, and the amounts sent against it.
const FUNDED_ETH_WEI = 10n ** 18n;
const FUNDED_ETH_TEXT = "1.0";
const COMFORTABLE_AMOUNT = "0.1";
const OVER_BALANCE_AMOUNT = "2.0";
// A send the balance covers to the wei once the ESTIMATE is added, and does
// not cover once the RESERVE is. Sending this is allowed by a gate reading
// the estimate and refused by a gate reading the reserve, which is the whole
// discrimination these tests exist to make.
const GAP_AMOUNT = formatEther(FUNDED_ETH_WEI - FEE_ESTIMATE_WEI);
// The ERC-20 side. The ETH balance is set to exactly the estimate for the
// fee test: it covers the expected cost to the wei and falls short of the
// reserve, so the same swap flips this assertion too — through a different
// balance and a different message than the ETH path uses.
const TOKEN_BALANCE_TEXT = "1.5";
const TOKEN_AMOUNT = "0.25";
const OVER_TOKEN_AMOUNT = "9.0";
const FEE_ONLY_ETH_WEI = FEE_ESTIMATE_WEI;
function toHexWei(wei) {
return "0x" + wei.toString(16);
}
// A fee in wei as the confirmation screen writes it. Deliberately a second
// implementation of formatFeeEth() from src/popup/views/confirmTx.js rather
// than an import of it: that module pulls in the whole popup and cannot be
// required outside a browser, and asserting against an independent rendering
// is stronger than asserting a function equals itself.
function feeEth(wei) {
const parts = formatEther(wei).split(".");
const dec =
parts.length > 1 ? parts[1].slice(0, 6).replace(/0+$/, "") || "0" : "0";
return parts[0] + "." + dec + " ETH";
}
// What the confirmation screen is showing right now, read out of the DOM in
// one pass: whether sending is allowed, which reason it is giving, what the
// fee block says, and how tall the whole view is.
async function confirmState(page) {
return page.evaluate(() => {
const el = (id) => document.getElementById(id);
// Both mechanisms matter. The two fee messages are dropped with
// display:none for the transaction type they cannot apply to, and
// shown or hidden with visibility for the one they can.
const shown = (id) => {
const cs = getComputedStyle(el(id));
return cs.display !== "none" && cs.visibility === "visible";
};
return {
height: el("view-confirm-tx").getBoundingClientRect().height,
type: el("confirm-type").textContent.trim(),
balance: el("confirm-balance").textContent.trim(),
fee: el("confirm-fee-amount").textContent.trim(),
reserve: el("confirm-fee-reserve").textContent.trim(),
reserveShown: shown("confirm-fee-reserve"),
errors: shown("confirm-errors")
? el("confirm-errors").textContent.trim()
: "",
amountFeeError: shown("confirm-amount-fee-error"),
gasError: shown("confirm-gas-error"),
feeUnknownError: shown("confirm-fee-unknown-error"),
sendDisabled: el("btn-confirm-send").disabled,
};
});
}
async function waitForEstimate(page) {
await page.waitForFunction(
() =>
document.getElementById("confirm-fee-amount").textContent.trim() !==
"Estimating...",
null,
{ timeout: 60000 },
);
}
async function backToAddress(page) {
if (await page.isVisible("#view-confirm-tx")) {
await page.click("#btn-confirm-back");
await visible(page, "#view-send");
}
if (await page.isVisible("#view-send")) {
await page.click("#btn-send-back");
}
await openAddressDetail(page);
}
// Drive the popup to the confirmation screen for one send.
//
// It waits for the send screen to be showing `balance` before filling
// anything in. That figure is the exact number the spend gate compares
// against, so waiting for it — rather than for a refresh to have probably
// landed — is what keeps every assertion below deterministic after a
// fixture change.
async function goToConfirm(page, { token, balance, amount }) {
await backToAddress(page);
await page.click("#btn-send");
await visible(page, "#view-send");
await page.selectOption("#send-token", token);
await page.waitForFunction(
(want) =>
document.getElementById("send-balance").textContent.trim() === want,
"Current balance: " + balance,
{ timeout: 60000 },
);
await page.fill("#send-to", STUB_COUNTERPARTY);
await page.fill("#send-amount", amount);
await page.click("#btn-send-review");
await visible(page, "#view-confirm-tx");
}
// A balance as the main view renders it: balanceLinesForAddress() writes
// every quantity with four decimal places.
function quantity(wei) {
return parseFloat(formatEther(wei)).toFixed(4);
}
// Wait on the main view until a changed balance fixture has been picked up.
//
// Deliberately not a reload: the popup re-refreshes on a 10-second timer by
// itself, and reloading aborts whatever fetch the home screen has open at
// that instant, which the extension reports through log.errorf and the
// harness — correctly — fails the run on.
//
// It also deliberately settles on MAIN rather than on the address screen.
// The address screen builds the send screen's token dropdown once, from the
// balances it holds at that moment, and nothing rebuilds it when a later
// refresh arrives, so entering it early leaves a dropdown with no token in
// it and the ERC-20 path unreachable.
async function settleOnMain(env, { ethWei, expectToken }) {
await backToAddress(env.page);
await env.page.click("#btn-address-back");
await visible(env.page, "#view-main");
await env.page.waitForFunction(
(want) =>
document.getElementById("wallet-list").textContent.includes(want),
quantity(ethWei),
{ timeout: 60000 },
);
if (expectToken) {
await visible(
env.page,
'#wallet-list [data-token="' + STUB_TOKEN.address + '"]',
60000,
);
}
}
test("ConfirmTx blocks sending while the fee estimate is pending (#238)", async (env) => {
env.routeOpts.ethBalanceWei = toHexWei(FUNDED_ETH_WEI);
env.routeOpts.seedTokenBalance = true;
await settleOnMain(env, {
ethWei: FUNDED_ETH_WEI,
expectToken: true,
});
env.routeOpts.holdGasEstimate = true;
await goToConfirm(env.page, {
token: "ETH",
balance: FUNDED_ETH_TEXT + " ETH",
amount: COMFORTABLE_AMOUNT,
});
const st = await confirmState(env.page);
env.ethPendingHeight = st.height;
console.log("# confirm-tx ETH view height: " + st.height + "px");
assert(
st.type === "Native ETH transfer",
"unexpected transaction type: " + JSON.stringify(st.type),
);
assert(
st.fee === "Estimating...",
"the fee line is not showing the pending placeholder: " +
JSON.stringify(st.fee),
);
assert(!st.reserveShown, "the reserve line is shown before any estimate");
assert(
st.sendDisabled,
"Send is enabled while the fee estimate is still in flight",
);
assert(
!st.feeUnknownError,
"the estimate-failed message is shown for an estimate that is merely pending",
);
assert(
!st.amountFeeError && !st.gasError && st.errors === "",
"a balance message is shown before the fee is known",
);
});
test("ConfirmTx enables Send once the estimate lands, quoting both numbers (#238)", async (env) => {
env.routeOpts.holdGasEstimate = false;
await waitForEstimate(env.page);
const st = await confirmState(env.page);
assert(
st.balance === FUNDED_ETH_TEXT + " ETH",
"the confirmation screen shows the wrong balance: " +
JSON.stringify(st.balance),
);
assert(
st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
"the fee line does not quote the estimate: " + JSON.stringify(st.fee),
);
assert(st.reserveShown, "the reserve line is not shown once the fee lands");
assert(
st.reserve === "up to " + feeEth(FEE_RESERVE_WEI) + " reserved",
"the reserve line does not quote the reserve: " +
JSON.stringify(st.reserve),
);
assert(
!st.sendDisabled,
"Send is disabled for a comfortably funded transfer",
);
assert(
st.errors === "" &&
!st.amountFeeError &&
!st.gasError &&
!st.feeUnknownError,
"a balance message is shown for a comfortably funded transfer",
);
assert(
st.height === env.ethPendingHeight,
"the view changed height when the estimate landed: " +
env.ethPendingHeight +
"px -> " +
st.height +
"px",
);
});
// The one that closes the hole. Everything else here survives a gate that
// reads the displayed estimate instead of the reserve; this does not.
test("ConfirmTx gates on the fee RESERVE, not the displayed estimate (#238)", async (env) => {
await goToConfirm(env.page, {
token: "ETH",
balance: FUNDED_ETH_TEXT + " ETH",
amount: GAP_AMOUNT,
});
await waitForEstimate(env.page);
const st = await confirmState(env.page);
// Printed on every run, pass or fail: the two fee numbers and the gate's
// decision side by side is the measurement this test is really making.
console.log(
"# gate probe: balance=" +
FUNDED_ETH_TEXT +
" ETH amount=" +
GAP_AMOUNT +
" estimate=" +
feeEth(FEE_ESTIMATE_WEI) +
" reserve=" +
feeEth(FEE_RESERVE_WEI) +
" sendDisabled=" +
st.sendDisabled +
" amountFeeError=" +
st.amountFeeError,
);
assert(
st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
"the screen is not quoting the estimate, so this send is not in the gap: " +
JSON.stringify(st.fee),
);
assert(
st.sendDisabled,
"Send is ENABLED for a transfer the fee RESERVE does not cover — the " +
"spend gate is reading the displayed estimate, which is issue #154",
);
assert(
st.amountFeeError,
"the amount-plus-fee message is not shown for a send the reserve does not cover",
);
assert(
st.height === env.ethPendingHeight,
"the over-budget state is a different height than the pending state: " +
env.ethPendingHeight +
"px -> " +
st.height +
"px",
);
});
test("ConfirmTx refuses a send that exceeds the balance outright (#238)", async (env) => {
await goToConfirm(env.page, {
token: "ETH",
balance: FUNDED_ETH_TEXT + " ETH",
amount: OVER_BALANCE_AMOUNT,
});
await waitForEstimate(env.page);
const st = await confirmState(env.page);
const want =
"Insufficient balance. You have " +
FUNDED_ETH_TEXT +
" ETH but are trying to send " +
OVER_BALANCE_AMOUNT +
" ETH.";
assert(
st.errors === want,
"wrong over-balance message: " + JSON.stringify(st.errors),
);
assert(st.sendDisabled, "Send is enabled for a send that exceeds balance");
assert(
!st.amountFeeError,
"the amount-plus-fee message is shown for an amount that alone exceeds the balance",
);
});
test("ConfirmTx refuses to send when the fee estimate fails, with its own message (#238)", async (env) => {
// The refusal is logged by confirmTx via log.errorf, i.e. console.error,
// which fails a test on its own. Declaring it here consumes exactly that
// one record — and fails this test if it never arrives.
env.errors.expect(
"confirmTx logging the failed gas estimate",
/gas estimation failed/,
);
env.routeOpts.failGasEstimate = true;
env.routeOpts.holdGasEstimate = true;
await goToConfirm(env.page, {
token: "ETH",
balance: FUNDED_ETH_TEXT + " ETH",
amount: COMFORTABLE_AMOUNT,
});
const pending = await confirmState(env.page);
assert(
pending.fee === "Estimating..." && pending.sendDisabled,
"the screen is not in the pending state before the estimate fails",
);
env.routeOpts.holdGasEstimate = false;
await waitForEstimate(env.page);
const st = await confirmState(env.page);
env.routeOpts.failGasEstimate = false;
assert(
st.fee === "Unable to estimate",
"the fee line does not report the failure: " + JSON.stringify(st.fee),
);
assert(
!st.reserveShown,
"the reserve line is shown after a failed estimate",
);
assert(
st.feeUnknownError,
"the estimate-failed message is not shown after a failed estimate",
);
assert(
!st.amountFeeError && !st.gasError && st.errors === "",
"a balance message is shown for an estimate that simply failed",
);
assert(
st.sendDisabled,
"Send is enabled with no usable fee estimate — an unknown fee is being treated as zero",
);
assert(
st.height === pending.height,
"the view changed height when the estimate failed: " +
pending.height +
"px -> " +
st.height +
"px",
);
});
test("ConfirmTx drives the ERC-20 path from pending to funded (#238)", async (env) => {
env.routeOpts.holdGasEstimate = true;
await goToConfirm(env.page, {
token: STUB_TOKEN.address,
balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
amount: TOKEN_AMOUNT,
});
const pending = await confirmState(env.page);
env.erc20PendingHeight = pending.height;
console.log("# confirm-tx ERC-20 view height: " + pending.height + "px");
assert(
pending.type === "ERC-20 token transfer (" + STUB_TOKEN.symbol + ")",
"unexpected transaction type: " + JSON.stringify(pending.type),
);
assert(
pending.balance === TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
"the ERC-20 screen shows the wrong balance: " +
JSON.stringify(pending.balance),
);
assert(
pending.fee === "Estimating..." && pending.sendDisabled,
"the ERC-20 screen does not block sending while its estimate is pending",
);
env.routeOpts.holdGasEstimate = false;
await waitForEstimate(env.page);
const st = await confirmState(env.page);
assert(
st.fee === "~" + feeEth(FEE_ESTIMATE_WEI),
"the ERC-20 fee line does not quote the estimate: " +
JSON.stringify(st.fee),
);
assert(
st.reserveShown &&
st.reserve === "up to " + feeEth(FEE_RESERVE_WEI) + " reserved",
"the ERC-20 fee block does not quote the reserve: " +
JSON.stringify(st.reserve),
);
assert(!st.sendDisabled, "Send is disabled for a funded ERC-20 transfer");
assert(
st.height === pending.height,
"the ERC-20 view changed height when the estimate landed: " +
pending.height +
"px -> " +
st.height +
"px",
);
});
test("ConfirmTx refuses an ERC-20 send that exceeds the token balance (#238)", async (env) => {
await goToConfirm(env.page, {
token: STUB_TOKEN.address,
balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
amount: OVER_TOKEN_AMOUNT,
});
await waitForEstimate(env.page);
const st = await confirmState(env.page);
const want =
"Insufficient " +
STUB_TOKEN.symbol +
" balance. You have " +
TOKEN_BALANCE_TEXT +
" " +
STUB_TOKEN.symbol +
" but are trying to send " +
OVER_TOKEN_AMOUNT +
" " +
STUB_TOKEN.symbol +
".";
assert(
st.errors === want,
"wrong over-token-balance message: " + JSON.stringify(st.errors),
);
assert(
st.sendDisabled,
"Send is enabled for an ERC-20 transfer that exceeds the token balance",
);
assert(
!st.gasError,
"the ERC-20 gas message is shown for an ETH balance that covers the fee",
);
});
// The same swap, through the other balance and the other message: here the
// token balance is ample and it is the ETH balance that must cover the fee.
// It is set to exactly the estimate, so an estimate-reading gate lets this
// through and the reserve-reading gate refuses it.
test("ConfirmTx gates the ERC-20 fee on the RESERVE, with the ERC-20 message (#238)", async (env) => {
env.routeOpts.ethBalanceWei = toHexWei(FEE_ONLY_ETH_WEI);
await settleOnMain(env, {
ethWei: FEE_ONLY_ETH_WEI,
expectToken: true,
});
await goToConfirm(env.page, {
token: STUB_TOKEN.address,
balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
amount: TOKEN_AMOUNT,
});
await waitForEstimate(env.page);
const st = await confirmState(env.page);
console.log(
"# erc-20 gate probe: ethBalance=" +
formatEther(FEE_ONLY_ETH_WEI) +
" estimate=" +
feeEth(FEE_ESTIMATE_WEI) +
" reserve=" +
feeEth(FEE_RESERVE_WEI) +
" sendDisabled=" +
st.sendDisabled +
" gasError=" +
st.gasError,
);
assert(
st.sendDisabled,
"Send is ENABLED for an ERC-20 transfer whose fee RESERVE exceeds the " +
"ETH balance — the spend gate is reading the displayed estimate (#154)",
);
assert(
st.gasError,
"the ERC-20 network-fee message is not shown when the ETH balance cannot cover the reserve",
);
assert(
!st.amountFeeError,
"the native-ETH over-budget message is shown on an ERC-20 transfer",
);
assert(
st.errors === "",
"a token-balance message is shown for a transfer the token balance covers: " +
JSON.stringify(st.errors),
);
assert(
st.height === env.erc20PendingHeight,
"the ERC-20 fee-error state is a different height than its pending state: " +
env.erc20PendingHeight +
"px -> " +
st.height +
"px",
);
});
test("ConfirmTx reports a failed ERC-20 estimate as unknown, not as a fee problem (#238)", async (env) => {
env.errors.expect(
"confirmTx logging the failed ERC-20 gas estimate",
/gas estimation failed/,
);
env.routeOpts.failGasEstimate = true;
await goToConfirm(env.page, {
token: STUB_TOKEN.address,
balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
amount: TOKEN_AMOUNT,
});
await waitForEstimate(env.page);
const st = await confirmState(env.page);
env.routeOpts.failGasEstimate = false;
assert(
st.fee === "Unable to estimate",
"the ERC-20 fee line does not report the failure: " +
JSON.stringify(st.fee),
);
assert(
st.feeUnknownError,
"the estimate-failed message is not shown on the ERC-20 path",
);
assert(
!st.gasError,
"the ERC-20 network-fee message is shown for a fee that is unknown rather than unaffordable",
);
assert(
st.sendDisabled,
"Send is enabled on the ERC-20 path with no usable fee estimate",
);
assert(
st.height === env.erc20PendingHeight,
"the ERC-20 estimate-failed state is a different height than its pending state: " +
env.erc20PendingHeight +
"px -> " +
st.height +
"px",
);
});
// ---------------------------------------------------------------- runner
async function main() {
@@ -638,7 +1212,15 @@ async function main() {
return;
}
const routeOpts = { seedTokenTransfer: false };
// Every fixture switch the suite can flip, declared in one place so the
// starting state of a run is readable without hunting through tests.
const routeOpts = {
seedTokenTransfer: false,
seedTokenBalance: false,
ethBalanceWei: null,
failGasEstimate: false,
holdGasEstimate: false,
};
let session;
try {
@@ -660,9 +1242,16 @@ async function main() {
popupUrl: session.popupUrl,
routeOpts,
page: null,
// The error collector, so a test that drives a failure path on
// purpose can declare the console.error it is about to provoke.
errors: session.errors,
// The recovery phrase of the wallet created in test 2, so later
// tests can assert on the real secret rather than its shape.
phrase: null,
// Confirmation-screen heights, measured in the pending state and
// compared against every later state of the same screen.
ethPendingHeight: null,
erc20PendingHeight: null,
};
// Attribution of collected errors is total. session.errors has no
@@ -709,6 +1298,16 @@ async function main() {
failure = "uncaught browser errors during this test";
}
// A test that declared an error it meant to provoke and did not
// provoke it asserted nothing. Failing here is what keeps expect()
// from being usable as a mute.
const unmatched = session.errors.unmatchedExpectations();
if (!failure && unmatched.length > 0) {
failure =
"expected browser error(s) that never arrived: " +
unmatched.join("; ");
}
if (failure) {
failed += 1;
console.log("not ok " + n + " - " + t.name);

View File

@@ -247,7 +247,7 @@ describe("a reveal that is not interrupted", () => {
expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-flash").textContent).toBe(
"That password is not correct. Please try again.",
"That password is incorrect. Please try again.",
);
});
});

View File

@@ -0,0 +1,213 @@
// One wording for one condition (issue #172).
//
// Every screen that asks for the password decrypts the vault itself, and
// each one used to write its own sentence for the same failure: the send
// confirmation and the delete-wallet confirmation said "Wrong password."
// (a fragment, which RULES.md Language & Labeling forbids), the reveal
// screens said "That password is not correct.", and the two dApp approval
// paths said "That password is incorrect." A user hitting two of those
// minutes apart had no way to tell whether the wallet meant the same
// thing.
//
// This scans the source rather than driving six views, because the
// invariant is about the set of call sites and not about any one of them:
// a seventh screen that decrypts the vault has to join the set, and a
// DOM test per view cannot notice one that was never written.
//
// The assertions are per CALL SITE, not per file. approval.js decrypts in
// two places and is where the divergence came from; a per-file check that
// only asks whether the canonical sentence appears somewhere in the file
// passes while one of those two says something else entirely. So each
// call site is read back to its own catch handler and the prose that
// handler shows the user must be the canonical sentence and nothing else
// — which fails on a novel wording, not only on a known-superseded one.
const fs = require("fs");
const path = require("path");
const SRC = path.join(__dirname, "..", "src");
const CANONICAL = "That password is incorrect. Please try again.";
// Wordings this repo has actually shipped for the same condition. This is
// a secondary, whole-file sweep for stragglers outside a decrypt handler;
// divergence at a call site is caught by the exact-match assertion, which
// needs no list of phrasings to guess at.
const SUPERSEDED = [
"Wrong password.",
"That password is not correct. Please try again.",
];
function jsFilesUnder(dir) {
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) return jsFilesUnder(full);
return entry.name.endsWith(".js") ? [full] : [];
});
}
// Blank out the interior of every comment and string literal, keeping the
// offsets and line breaks, so braces can be counted without a quote or a
// commented-out block throwing the count off. The literals are returned
// alongside with the offset of their opening quote, which is how a
// message is later attributed to the handler it sits in.
function scan(source) {
const masked = source.split("");
const strings = [];
const blank = (from, to) => {
for (let k = from; k < to; k++) if (masked[k] !== "\n") masked[k] = " ";
};
let i = 0;
while (i < source.length) {
const two = source.slice(i, i + 2);
if (two === "//") {
const nl = source.indexOf("\n", i);
const stop = nl === -1 ? source.length : nl;
blank(i, stop);
i = stop;
} else if (two === "/*") {
const close = source.indexOf("*/", i + 2);
const stop = close === -1 ? source.length : close + 2;
blank(i, stop);
i = stop;
} else if (
source[i] === '"' ||
source[i] === "'" ||
source[i] === "`"
) {
const quote = source[i];
let j = i + 1;
let value = "";
while (j < source.length && source[j] !== quote) {
if (source[j] === "\\") {
value += source[j + 1];
j += 2;
continue;
}
value += source[j];
j += 1;
}
blank(i + 1, j);
strings.push({ offset: i, value });
i = j + 1;
} else {
i += 1;
}
}
return { masked: masked.join(""), strings };
}
// Offset of the `{` that opens the block containing `at`, or -1.
function enclosingBlockStart(masked, at) {
let depth = 0;
for (let i = at; i >= 0; i--) {
if (masked[i] === "}") depth += 1;
else if (masked[i] === "{") {
if (depth === 0) return i;
depth -= 1;
}
}
return -1;
}
// Offset just past the `}` matching the `{` at `open`.
function blockEnd(masked, open) {
let depth = 0;
for (let i = open; i < masked.length; i++) {
if (masked[i] === "{") depth += 1;
else if (masked[i] === "}") {
depth -= 1;
if (depth === 0) return i + 1;
}
}
throw new Error("unterminated block");
}
// The catch handler guarding a given decryptWithPassword call: walk out to
// the try block the call sits in, then take the catch that follows it.
function handlerSpan(masked, callOffset, label) {
const tryOpen = enclosingBlockStart(masked, callOffset);
if (tryOpen === -1 || !/\btry\s*$/.test(masked.slice(0, tryOpen)))
throw new Error(`${label}: the decrypt is not inside a try block`);
const rest = masked.slice(blockEnd(masked, tryOpen));
const catchMatch = /^\s*catch\s*(\([^)]*\)\s*)?\{/.exec(rest);
if (!catchMatch)
throw new Error(`${label}: the decrypt's try block has no catch`);
const catchOpen = blockEnd(masked, tryOpen) + catchMatch[0].length - 1;
return [catchOpen, blockEnd(masked, catchOpen)];
}
// The prose the handler puts in front of the user. Element ids, class
// names and visibility keywords are single words; a sentence has a space
// in it, and that is the whole distinction needed here.
function handlerMessages(file, callOffset, label) {
const { masked, strings } = scan(fs.readFileSync(file, "utf8"));
const [from, to] = handlerSpan(masked, callOffset, label);
return strings
.filter((s) => s.offset >= from && s.offset < to)
.map((s) => s.value)
.filter((v) => v.includes(" "));
}
// The call sites are found, not listed: the file layout moves (the private
// key export was in addressDetail.js when #172 was filed and is its own
// view now), and a hardcoded list would quietly stop covering a screen it
// no longer names.
function callSites() {
const sites = [];
for (const file of jsFilesUnder(SRC)) {
if (file === path.join(SRC, "shared", "vault.js")) continue;
const { masked } = scan(fs.readFileSync(file, "utf8"));
const rel = path.relative(SRC, file).split(path.sep).join("/");
let n = 0;
let at = masked.indexOf("decryptWithPassword(");
while (at !== -1) {
n += 1;
sites.push({ file, rel, offset: at, label: `${rel} #${n}` });
at = masked.indexOf("decryptWithPassword(", at + 1);
}
}
return sites.sort((a, b) => a.label.localeCompare(b.label));
}
describe("password failure messages", () => {
const sites = callSites();
const files = [...new Set(sites.map((s) => s.file))].sort();
test("the call sites are found where they are expected", () => {
const counts = {};
for (const site of sites)
counts[site.rel] = (counts[site.rel] ?? 0) + 1;
expect(counts).toEqual({
"popup/views/approval.js": 2,
"popup/views/confirmTx.js": 1,
"popup/views/deleteWallet.js": 1,
"popup/views/exportPrivkey.js": 1,
"popup/views/showPhrase.js": 1,
});
});
test("the canonical message is a full sentence", () => {
expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/);
});
// Exact equality, per call site: a message that is merely different
// rather than known-obsolete fails here too, which a scan for historic
// wordings cannot do.
test.each(sites.map((s) => [s.label, s]))(
"%s answers a rejected password with the canonical sentence",
(label, site) => {
expect(handlerMessages(site.file, site.offset, label)).toEqual([
CANONICAL,
]);
},
);
test.each(files.map((f) => [path.relative(SRC, f), f]))(
"%s carries no superseded wording",
(_rel, file) => {
const source = fs.readFileSync(file, "utf8");
for (const old of SUPERSEDED) expect(source).not.toContain(old);
},
);
});