Compare commits

..

2 Commits

Author SHA1 Message Date
1197d2171b fix: give every address a row of its own, so none wraps or is shortened (closes #380) (#381)
All checks were successful
check / check (push) Successful in 56s
e2e / e2e-chrome (push) Successful in 1m51s
e2e / e2e-firefox (push) Successful in 40s
2026-08-30 05:25:00 +02:00
a098bb0c32 fix: floor malformed allowedSites, fraudContracts and selectedToken entries (closes #362)
All checks were successful
check / check (push) Successful in 42s
e2e / e2e-chrome (push) Successful in 1m45s
e2e / e2e-firefox (push) Successful in 31s
A stored allowedSites whose value was not a list rendered a working popup and then made every subsequent save fail silently, so the user operated a wallet that persisted nothing -- worse than a blank popup, which is at least visibly broken. fraudContracts and selectedToken had the same shape: a container floored by truthiness or not at all, while its entries were dereferenced. Entries are now floored as well as containers, following the idiom #311 established, and a failed save raises a persistent banner instead of vanishing into a swallowed rejection.

The per-field justifications that used to live in a hand-written header are replaced by a contract test that drives each field's hostile and falsy values through a real popup boot, so a claim about a field answers to the code rather than to prose. Its guarantee is stated narrowly and deliberately: no structural dereference on the code paths a wholly-corrupted profile takes, which is not every path a stored record takes. The paths it does not drive are named where the claim is made, and are tracked in #379.
2026-08-23 23:06:17 +02:00
12 changed files with 458 additions and 130 deletions

View File

@@ -800,7 +800,12 @@ discoverable.
addresses visually, as a security feature. addresses visually, as a security feature.
- **Tailwind CSS**: Utility-first CSS via Tailwind. No custom CSS classes for - **Tailwind CSS**: Utility-first CSS via Tailwind. No custom CSS classes for
styling. Tailwind is configured with a minimal monochrome palette. This keeps styling. Tailwind is configured with a minimal monochrome palette. This keeps
the styling co-located with the markup and eliminates CSS file management. the styling co-located with the markup and eliminates CSS file management. The
handful of classes in `styles/main.css` are not styling: `.copy-flash-*`
carries the copy feedback animation, and `.am-address` carries the rule that
an address never wraps. Both are invariants that hold in every place they
appear, and spelling either out as repeated utilities is how one of those
places drifts away from the rest.
- **Vanilla JS**: No framework (React, Vue, Svelte, etc.). The popup UI is small - **Vanilla JS**: No framework (React, Vue, Svelte, etc.). The popup UI is small
enough that vanilla JS with simple view switching is sufficient. A framework enough that vanilla JS with simple view switching is sufficient. A framework
would add bundle size, build complexity, and attack surface for no benefit at would add bundle size, build complexity, and attack surface for no benefit at
@@ -849,6 +854,12 @@ that the portions still displayed will be more than adequate for the user to
verify addresses even in the case of address spoofing attacks. Clicking an verify addresses even in the case of address spoofing attacks. Clicking an
address will always copy the full, untruncated value. address will always copy the full, untruncated value.
As of the address-row layout change, no view invokes that exception: every
address in the popup is rendered on a row of its own, wide enough for all 42
characters, and no screen truncates one to fit. The cap is still enforced in
`truncateMiddle()` and the 32-character floor in `renderAddressHtml()`, so the
guarantee holds for any future caller; there simply are none today.
**Specific Exception — Transaction Detail view:** The transaction detail screen **Specific Exception — Transaction Detail view:** The transaction detail screen
is the authoritative record of a specific transaction and shows the exact, is the authoritative record of a specific transaction and shows the exact,
untruncated amount with all meaningful decimal places (e.g. "0.00498824598498216 untruncated amount with all meaningful decimal places (e.g. "0.00498824598498216
@@ -1052,17 +1063,18 @@ driving the real code with hostile values — and, for every field whose only
defence is that nothing dereferences it, by booting the real popup entry point defence is that nothing dereferences it, by booting the real popup entry point
over that value onto every view the popup can reopen onto. That last part is over that value onto every view the popup can reopen onto. That last part is
what makes the claim falsifiable, because this defect class lives on the restore what makes the claim falsifiable, because this defect class lives on the restore
path rather than on the home screen: a field one of those boots corrupts and a path rather than on the home screen. Read the claim narrowly, as that file
restorable view dereferences on its render fails `make check`, at either states it: what those boots prove is no structural dereference on the code paths
polarity — a value nothing writes is a wrong-typed one and therefore truthy, so a WHOLLY-CORRUPTED PROFILE takes, which is not every path a stored record takes.
each such field is also driven falsy, or proven unable to be falsy after the Not driven: any pairing of values the four slots do not produce, a view only
floor. So does a field that gains a floor while its row still claims it has forward navigation opens, anything behind a click, and everything a healthy
none, and so does a field added to `PERSISTED_FIELDS` with no row at all. Two profile reaches. Within that boundary the verdict is unconditional — if one of
things the boots do not drive: a MIX of polarities, since one boot puts every those boots leaves the popup unhealthy or off the view it stored, `make check`
corrupted field on the same slot, so a branch reached only when one is truthy fails, including when it takes two corrupted fields at once, because the verdict
and another falsy is not entered; and whatever no stored record reaches by is the combined boot and the per-field re-boot that names a culprit can only
itself — a view only forward navigation opens, and anything behind a click. The decorate the message. So does a field that gains a floor while its row still
per-field justification that used to live in the header of claims it has none, and so does a field added to `PERSISTED_FIELDS` with no row
at all. The per-field justification that used to live in the header of
`src/shared/stateSchema.js` shipped a false claim in three consecutive changes, `src/shared/stateSchema.js` shipped a false claim in three consecutive changes,
each caught only by a reviewer re-deriving thirty fields by hand. each caught only by a reviewer re-deriving thirty fields by hand.
@@ -1181,13 +1193,17 @@ view would leave a wallet one click from deletion.
- Send / Receive quick-action buttons, both acting on the active address - Send / Receive quick-action buttons, both acting on the active address
- ETH/USD price display - ETH/USD price display
- Wallet list: each wallet shows its name (tap to rename inline) and a "+" - Wallet list: each wallet shows its name (tap to rename inline) and a "+"
button for HD and xprv wallets, then one block per address with "Address button for HD and xprv wallets, then one block per address. The block
N" (bold when active), the ENS name if resolved, the full address, an opens with a row carrying the colour dot, "Address N" (bold when active),
`[info]` button, an `[x]` button (only on HD and xprv wallets holding more an `[info]` button and an `[x]` button (only on HD and xprv wallets
than one address), the address USD total, and a balance line for ETH and holding more than one address); the ENS name, if resolved, is below it;
for each token shown for that address then the full address on a row of its own, followed by the address USD
total and a balance line for ETH and for each token shown for that address
- "Recent Transactions": up to 25 transactions merged across every address - "Recent Transactions": up to 25 transactions merged across every address
of every wallet, deduplicated by hash and filtered of every wallet, deduplicated by hash and filtered. Each row is three
lines: age and direction, then the counterparty's colour dot (with our own
name for it, where it is one of our addresses) and the amount, then the
counterparty's full address on a row of its own
- "Add additional wallet..." link at bottom - "Add additional wallet..." link at bottom
- **Transitions**: - **Transitions**:
- Tap address row → sets the active address and broadcasts - Tap address row → sets the active address and broadcasts

28
TODO.md
View File

@@ -45,6 +45,22 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
wallet list was the reported case: the address shared one row with the
`[info]` and `[x]` controls and folded onto a second line, which turns one
42-character string the user is meant to compare into two shorter ones — the
shape an address-poisoning attack wants. The fix is layout, not CSS: every
address in the popup now sits alone on a full-width row, with the colour dot,
the wallet title, the ENS name and the explorer link moved onto a strip above
it, and the transaction rows carry the counterparty's whole address instead of
a `truncateMiddle()`d one squeezed in beside the amount. `truncateMiddle()`
keeps its 10-character cap and its 32-character floor moved into
`renderAddressHtml()`, so the guarantee outlives having no callers. The e2e
suite measures every rendered address in a real Chromium — whole, one line
box, inside its row and inside the popup — across Home, the address, token,
receive, send and transaction detail screens, the confirmation screen and the
dApp transaction prompt.
- 2026-08-23: Both manifests declare toolbar icons, and real PNGs at - 2026-08-23: Both manifests declare toolbar icons, and real PNGs at
16/32/48/128 ship inside both archives 16/32/48/128 ship inside both archives
([#371](https://git.eeqj.de/sneak/AutistMask/issues/371)). Neither manifest ([#371](https://git.eeqj.de/sneak/AutistMask/issues/371)). Neither manifest
@@ -85,9 +101,15 @@ but the review is broader than any of them.
the path this whole class of defect lives on. Each such field is driven at the path this whole class of defect lives on. Each such field is driven at
both polarities — a value nothing writes is wrong-typed and so truthy, so a both polarities — a value nothing writes is wrong-typed and so truthy, so a
falsy slot is driven too, or the field is proven unable to be falsy after the falsy slot is driven too, or the field is proven unable to be falsy after the
floor. A field with no row, a field that gains a floor while its row still floor. The claim is narrow and stated as such: no structural dereference on
claims it has none, and a field one of those boots corrupts and a restorable the code paths a wholly-corrupted profile takes, which is not every path a
view dereferences on its render now all fail `make check`. stored record takes — a pairing of values the four slots do not produce, a
view only forward navigation opens, anything behind a click, and everything a
healthy profile reaches are all undriven. Within that boundary the verdict is
unconditional, including a dereference that takes two corrupted fields at
once, since the assertion is on the combined boot and the per-field re-boot
can only decorate the message. A field with no row and a field that gains a
floor while its row still claims it has none also fail `make check`.
- 2026-08-23: A swap amount and the token it is counted in now always come from - 2026-08-23: A swap amount and the token it is counted in now always come from
the same hop, on both sides of the approval screen the same hop, on both sides of the approval screen
([#359](https://git.eeqj.de/sneak/AutistMask/issues/359) and ([#359](https://git.eeqj.de/sneak/AutistMask/issues/359) and

View File

@@ -213,10 +213,7 @@
</div> </div>
<!-- active address display --> <!-- active address display -->
<div <div id="active-address-display" class="text-xs mb-3"></div>
id="active-address-display"
class="text-xs break-all mb-3"
></div>
<!-- quick actions for active address --> <!-- quick actions for active address -->
<div class="flex gap-2 mb-2"> <div class="flex gap-2 mb-2">
@@ -292,7 +289,7 @@
class="font-bold mb-1 hidden flex items-center" class="font-bold mb-1 hidden flex items-center"
></div> ></div>
<div <div
class="text-xs mb-1 cursor-pointer break-all" class="text-xs mb-1 cursor-pointer"
title="Click to copy" title="Click to copy"
id="address-line" id="address-line"
> >
@@ -380,14 +377,14 @@
></div> ></div>
<h2 class="font-bold mb-1">Export Private Key</h2> <h2 class="font-bold mb-1">Export Private Key</h2>
<p class="text-xs mb-1" id="export-privkey-title"></p> <p class="text-xs mb-1" id="export-privkey-title"></p>
<p class="text-xs mb-3"> <div class="text-xs mb-3">
<span id="export-privkey-dot"></span> <span id="export-privkey-dot"></span>
<span <span
id="export-privkey-address" id="export-privkey-address"
class="cursor-pointer" class="cursor-pointer"
title="Click to copy" title="Click to copy"
></span> ></span>
</p> </div>
<p class="text-xs mb-3 text-muted"> <p class="text-xs mb-3 text-muted">
Warning: anyone with this private key can access and Warning: anyone with this private key can access and
transfer all funds from this address. Never share it. transfer all funds from this address. Never share it.
@@ -440,7 +437,7 @@
</div> </div>
<div <div
class="text-xs mb-1 cursor-pointer break-all" class="text-xs mb-1 cursor-pointer"
title="Click to copy" title="Click to copy"
id="address-token-line" id="address-token-line"
> >
@@ -573,19 +570,16 @@
<!-- ERC-20 token contract (hidden for ETH) --> <!-- ERC-20 token contract (hidden for ETH) -->
<div id="confirm-token-section" class="mb-3 hidden"> <div id="confirm-token-section" class="mb-3 hidden">
<div class="text-xs text-muted mb-1">Token contract</div> <div class="text-xs text-muted mb-1">Token contract</div>
<div <div id="confirm-token-contract" class="text-xs"></div>
id="confirm-token-contract"
class="text-xs break-all"
></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">From</div> <div class="text-xs text-muted mb-1">From</div>
<div id="confirm-from" class="text-xs break-all"></div> <div id="confirm-from" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
<div id="confirm-to" class="text-xs break-all"></div> <div id="confirm-to" class="text-xs"></div>
<div <div
id="confirm-to-ens" id="confirm-to-ens"
class="text-xs text-muted hidden" class="text-xs text-muted hidden"
@@ -728,7 +722,7 @@
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
<div id="wait-tx-to" class="text-xs break-all"></div> <div id="wait-tx-to" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">Transaction hash</div> <div class="text-xs text-muted mb-1">Transaction hash</div>
@@ -747,7 +741,7 @@
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
<div id="success-tx-to" class="text-xs break-all"></div> <div id="success-tx-to" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">Block</div> <div class="text-xs text-muted mb-1">Block</div>
@@ -774,7 +768,7 @@
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
<div id="error-tx-to" class="text-xs break-all"></div> <div id="error-tx-to" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div <div
@@ -811,9 +805,9 @@
<canvas id="receive-qr"></canvas> <canvas id="receive-qr"></canvas>
</div> </div>
<div <div
class="border border-border p-2 break-all mb-3 text-xs cursor-pointer" class="border border-border p-2 mb-3 text-xs cursor-pointer"
> >
<span id="receive-address-block" class="select-all"></span> <div id="receive-address-block" class="select-all"></div>
<span id="receive-etherscan-link"></span> <span id="receive-etherscan-link"></span>
</div> </div>
<button <button
@@ -1239,7 +1233,7 @@
</p> </p>
<div <div
id="delete-address-value" id="delete-address-value"
class="text-xs mb-2 break-all min-h-[1rem]" class="text-xs mb-2 min-h-[1rem]"
></div> ></div>
<div <div
class="text-xs mb-2 border border-border border-dashed p-2" class="text-xs mb-2 border border-border border-dashed p-2"
@@ -1429,14 +1423,11 @@
</div> </div>
<div class="mb-2"> <div class="mb-2">
<div class="text-xs text-muted mb-1">From</div> <div class="text-xs text-muted mb-1">From</div>
<div <div id="tx-detail-from" class="text-xs"></div>
id="tx-detail-from"
class="text-xs break-all"
></div>
</div> </div>
<div class="mb-2"> <div class="mb-2">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
<div id="tx-detail-to" class="text-xs break-all"></div> <div id="tx-detail-to" class="text-xs"></div>
</div> </div>
</div> </div>
@@ -1473,7 +1464,7 @@
</div> </div>
<div <div
id="tx-detail-token-contract" id="tx-detail-token-contract"
class="text-xs break-all" class="text-xs"
></div> ></div>
</div> </div>
</div> </div>
@@ -1567,11 +1558,11 @@
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">From</div> <div class="text-xs text-muted mb-1">From</div>
<div id="approve-tx-from" class="text-xs break-all"></div> <div id="approve-tx-from" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">Contract</div> <div class="text-xs text-muted mb-1">Contract</div>
<div id="approve-tx-to" class="text-xs break-all"></div> <div id="approve-tx-to" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">Value</div> <div class="text-xs text-muted mb-1">Value</div>
@@ -1673,7 +1664,7 @@
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">From</div> <div class="text-xs text-muted mb-1">From</div>
<div id="approve-sign-from" class="text-xs break-all"></div> <div id="approve-sign-from" class="text-xs"></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">

View File

@@ -44,3 +44,23 @@ body {
background-color 225ms ease-out, background-color 225ms ease-out,
color 225ms ease-out; color 225ms ease-out;
} }
/* An address is one atomic string, so it gets a row of its own and never
* breaks across lines. A wrapped address reads as two shorter strings, and
* two shorter strings are exactly what an address-poisoning attack needs
* the user to compare instead of the whole thing. Every view that shows an
* address puts it in one of these, alone: the colour dot, the wallet title,
* the ENS name and the explorer link all live on their own line above, so
* nothing competes with the 42 characters for width.
*
* overflow-x is the escape hatch, not the mechanism. The row is wide enough
* for a full address at every nesting depth the popup uses; if that ever
* stops being true — a font with wider glyphs, a browser zoom — the row
* scrolls and the user can still reach the last character, rather than the
* tail being clipped away by #app's overflow-x-hidden with nothing to say
* it happened. tests/e2e asserts the scroll is never actually needed. */
.am-address {
display: block;
white-space: nowrap;
overflow-x: auto;
}

View File

@@ -7,7 +7,6 @@ const {
addressTitle, addressTitle,
escapeHtml, escapeHtml,
displaySymbol, displaySymbol,
truncateMiddle,
renderAddressHtml, renderAddressHtml,
attachCopyHandlers, attachCopyHandlers,
goBack, goBack,
@@ -229,10 +228,12 @@ function renderTransactions(txs) {
const amountStr = tx.value const amountStr = tx.value
? escapeHtml(tx.value + " " + sym) ? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym); : escapeHtml(sym);
const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10)); // The counterparty used to be squeezed in beside the amount and
const displayAddr = // truncated to whatever was left over. It gets its own row now and
title || ensName || truncateMiddle(counterparty, maxAddr); // is shown whole; the title or ENS name, where there is one, names
const addrStr = escapeHtml(displayAddr); // it on the line above rather than replacing it.
const nameStr = escapeHtml(title || ensName || "");
const addrStr = escapeHtml(counterparty);
const dot = addressDotHtml(counterparty); const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : ""; const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : ""; const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -240,7 +241,8 @@ function renderTransactions(txs) {
const iso = escapeHtml(isoDate(tx.timestamp)); const iso = escapeHtml(isoDate(tx.timestamp));
html += `<div class="tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`; html += `<div class="tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`;
html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`; html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`;
html += `<div class="flex justify-between"><span class="flex items-center">${dot}${addrStr}</span><span>${amountStr}</span></div>`; html += `<div class="flex justify-between"><span class="flex items-center">${dot}${nameStr}</span><span>${amountStr}</span></div>`;
html += `<div class="am-address">${addrStr}</div>`;
html += `</div>`; html += `</div>`;
i++; i++;
} }

View File

@@ -10,7 +10,6 @@ const {
addressTitle, addressTitle,
escapeHtml, escapeHtml,
displaySymbol, displaySymbol,
truncateMiddle,
balanceLine, balanceLine,
unknownableAmount, unknownableAmount,
renderAddressHtml, renderAddressHtml,
@@ -305,10 +304,12 @@ function renderTransactions(txs) {
const amountStr = tx.value const amountStr = tx.value
? escapeHtml(tx.value + " " + sym) ? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym); : escapeHtml(sym);
const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10)); // The counterparty used to be squeezed in beside the amount and
const displayAddr = // truncated to whatever was left over. It gets its own row now and
title || ensName || truncateMiddle(counterparty, maxAddr); // is shown whole; the title or ENS name, where there is one, names
const addrStr = escapeHtml(displayAddr); // it on the line above rather than replacing it.
const nameStr = escapeHtml(title || ensName || "");
const addrStr = escapeHtml(counterparty);
const dot = addressDotHtml(counterparty); const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : ""; const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : ""; const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -316,7 +317,8 @@ function renderTransactions(txs) {
const iso = escapeHtml(isoDate(tx.timestamp)); const iso = escapeHtml(isoDate(tx.timestamp));
html += `<div class="tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`; html += `<div class="tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`;
html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`; html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`;
html += `<div class="flex justify-between"><span class="flex items-center">${dot}${addrStr}</span><span>${amountStr}</span></div>`; html += `<div class="flex justify-between"><span class="flex items-center">${dot}${nameStr}</span><span>${amountStr}</span></div>`;
html += `<div class="am-address">${addrStr}</div>`;
html += `</div>`; html += `</div>`;
i++; i++;
} }

View File

@@ -331,6 +331,12 @@ function addressHoldsFunds(addr) {
return false; return false;
} }
// The fewest characters of an address any caller may ask to display. The
// 10-character cap inside truncateMiddle() is the other half of the same
// guarantee; this is the half that used to be spelled out at each call
// site, and is now enforced once in renderAddressHtml().
const ADDRESS_MIN_DISPLAY_LEN = 32;
// Truncate the middle of a string, replacing removed characters with "…". // Truncate the middle of a string, replacing removed characters with "…".
// Safety: refuses to truncate more than 10 characters, which is the maximum // Safety: refuses to truncate more than 10 characters, which is the maximum
// that still prevents address spoofing attacks (see Display Consistency in // that still prevents address spoofing attacks (see Display Consistency in
@@ -518,17 +524,29 @@ function attachCopyHandlers(container) {
// Unified address rendering. // Unified address rendering.
// //
// Produces consistent HTML for any Ethereum address: // Two stacked rows, in this order:
// • Color dot // 1. Identity strip — colour dot, optional title (e.g. "Wallet 1 —
// • Optional title (e.g. "Wallet 1 — Address 2") shown bold above address // Address 2") and the explorer link icon. Optional ENS name below it.
// • Optional ENS name shown bold above address // 2. The address itself, alone on a full-width row that never wraps
// • Full address (or truncated via maxLen) with dashed-underline click-to-copy // (see .am-address in styles/main.css).
// • Etherscan external link icon //
// The split is the point. Everything used to sit on one line: dot, address
// and link together, with `break-all` to let the address fold when the line
// ran out. In the wallet list, where the row also carried [info] and [x],
// it ran out every time — the bug in #380 — and a folded address is a
// spoofing hazard, not a cosmetic one. Nothing shares the address's row
// now, so all 42 characters fit at every nesting depth the popup uses and
// nothing has to be dropped or folded to make room.
// //
// Options object: // Options object:
// title — wallet title string (from addressTitle) // title — wallet title string (from addressTitle)
// ensName — ENS name string // ensName — ENS name string
// maxLen — if set, truncate address display (min 32 chars enforced) // maxLen — if set, truncate address display. Floored at 32 characters
// here rather than by the caller: no view passes it any more
// (every address row is wide enough for all 42 characters),
// so a floor that lived in the callers would have gone away
// with them, and the "at least 32 characters" guarantee has
// to survive having no current callers to be a guarantee.
// noLink — if true, omit etherscan link // noLink — if true, omit etherscan link
// //
// After inserting the returned HTML into the DOM, call // After inserting the returned HTML into the DOM, call
@@ -536,22 +554,22 @@ function attachCopyHandlers(container) {
function renderAddressHtml(address, opts) { function renderAddressHtml(address, opts) {
const { title, ensName, maxLen, noLink } = opts || {}; const { title, ensName, maxLen, noLink } = opts || {};
const dot = addressDotHtml(address); const dot = addressDotHtml(address);
const displayAddr = maxLen ? truncateMiddle(address, maxLen) : address; const displayAddr = maxLen
? truncateMiddle(address, Math.max(ADDRESS_MIN_DISPLAY_LEN, maxLen))
: address;
const link = etherscanAddressUrl(address); const link = etherscanAddressUrl(address);
const extLink = noLink ? "" : etherscanLinkHtml(link); const extLink = noLink ? "" : etherscanLinkHtml(link);
let html = ""; let html = "";
html += `<div class="flex items-center">${dot}`;
if (title) { if (title) {
html += `<div class="flex items-center font-bold">${dot}${escapeHtml(title)}</div>`; html += `<span class="font-bold">${escapeHtml(title)}</span>`;
} }
html += `${extLink}</div>`;
if (ensName) { if (ensName) {
html += `<div class="flex items-center font-bold">${title ? "" : dot}${escapeHtml(ensName)}</div>`; html += `<div class="font-bold">${escapeHtml(ensName)}</div>`;
}
if (title || ensName) {
html += `<div class="flex items-center">${copyableHtml(displayAddr, "break-all")}${extLink}</div>`;
} else {
html += `<div class="flex items-center">${dot}${copyableHtml(displayAddr, "break-all")}${extLink}</div>`;
} }
html += `<div class="am-address">${copyableHtml(displayAddr)}</div>`;
return html; return html;
} }

View File

@@ -9,7 +9,6 @@ const {
addressTitle, addressTitle,
escapeHtml, escapeHtml,
displaySymbol, displaySymbol,
truncateMiddle,
renderAddressHtml, renderAddressHtml,
attachCopyHandlers, attachCopyHandlers,
pushCurrentView, pushCurrentView,
@@ -117,10 +116,13 @@ function renderHomeTxList(ctx) {
const amountStr = tx.value const amountStr = tx.value
? escapeHtml(tx.value + " " + sym) ? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym); : escapeHtml(sym);
// The counterparty used to be squeezed in beside the amount and
// truncated to whatever was left over. It gets its own row now and
// is shown whole; the title, when it is one of our own addresses,
// names it on the line above rather than replacing it.
const title = addressTitle(counterparty, state.wallets); const title = addressTitle(counterparty, state.wallets);
const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10)); const titleStr = title ? escapeHtml(title) : "";
const displayAddr = title || truncateMiddle(counterparty, maxAddr); const addrStr = escapeHtml(counterparty);
const addrStr = escapeHtml(displayAddr);
const dot = addressDotHtml(counterparty); const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : ""; const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : ""; const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -128,7 +130,8 @@ function renderHomeTxList(ctx) {
const iso = escapeHtml(isoDate(tx.timestamp)); const iso = escapeHtml(isoDate(tx.timestamp));
html += `<div class="home-tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`; html += `<div class="home-tx-row py-2 border-b border-border-light text-xs cursor-pointer hover:bg-hover" data-tx="${i}" style="${opacity}">`;
html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`; html += `<div class="flex justify-between"><span class="text-muted" title="${iso}">${ago}</span><span>${dirLabel}${err}</span></div>`;
html += `<div class="flex justify-between"><span class="flex items-center">${dot}${addrStr}</span><span>${amountStr}</span></div>`; html += `<div class="flex justify-between"><span class="flex items-center">${dot}${titleStr}</span><span>${amountStr}</span></div>`;
html += `<div class="am-address">${addrStr}</div>`;
html += `</div>`; html += `</div>`;
i++; i++;
} }
@@ -252,17 +255,22 @@ function walletListHtml() {
: ""; : "";
const dot = addressDotHtml(addr.address); const dot = addressDotHtml(addr.address);
const titleBold = isActive ? "font-bold" : ""; const titleBold = isActive ? "font-bold" : "";
html += `<div class="text-xs ${titleBold}">Address ${ai + 1}</div>`; // [info] and [x] ride on the "Address N" line, which was empty
// to its right, so the address below gets the row to itself.
// They used to sit beside the address and take about a third of
// the width off it, which is what made a 42-character address
// fold onto a second line here and nowhere else (#380).
html += `<div class="flex text-xs items-center justify-between">`;
html += `<span class="flex items-center ${titleBold}">${dot}Address ${ai + 1}</span>`;
html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
html += `</div>`;
if (addr.ensName) { if (addr.ensName) {
// An ENS reverse record is whatever the name owner set it // An ENS reverse record is whatever the name owner set it
// to; renderAddressHtml() escapes its own copy of this and // to; renderAddressHtml() escapes its own copy of this and
// this list was the one that did not. // this list was the one that did not.
html += `<div class="text-xs font-bold flex items-center">${dot}${escapeHtml(addr.ensName)}</div>`; html += `<div class="text-xs font-bold">${escapeHtml(addr.ensName)}</div>`;
} }
html += `<div class="flex text-xs items-center justify-between">`; html += `<div class="am-address text-xs">${escapeHtml(addr.address)}</div>`;
html += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${escapeHtml(addr.address)}</span>`;
html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
html += `</div>`;
const addrTotal = formatAddressTotal(getAddressValue(addr)); const addrTotal = formatAddressTotal(getAddressValue(addr));
html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrTotal || "&nbsp;"}</div>`; html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrTotal || "&nbsp;"}</div>`;
html += balanceLinesForAddress( html += balanceLinesForAddress(

View File

@@ -137,10 +137,16 @@ function render() {
if (tx.contractAddress) { if (tx.contractAddress) {
const dot = addressDotHtml(tx.contractAddress); const dot = addressDotHtml(tx.contractAddress);
const link = explorerUrl("token", tx.contractAddress); const link = explorerUrl("token", tx.contractAddress);
// Hand-rolled rather than renderAddressHtml() because the
// link goes to the explorer's /token/ page, not /address/.
// Same two-row shape though: dot and link on the strip, the
// contract address alone on the row below it.
tokenContractEl.innerHTML = tokenContractEl.innerHTML =
`<div class="flex items-center">${dot}` + `<div class="flex items-center">${dot}` +
copyableHtml(tx.contractAddress, "break-all") +
etherscanLinkHtml(link) + etherscanLinkHtml(link) +
`</div>` +
`<div class="am-address">` +
copyableHtml(tx.contractAddress) +
`</div>`; `</div>`;
tokenContractSection.classList.remove("hidden"); tokenContractSection.classList.remove("hidden");
} else { } else {

View File

@@ -38,15 +38,16 @@
// real popup entry point onto EVERY view the popup can reopen onto. // real popup entry point onto EVERY view the popup can reopen onto.
// //
// That last part is the whole point, because this defect class lives on the // That last part is the whole point, because this defect class lives on the
// RESTORE path and not on Home: a field one of those boots corrupts and a // RESTORE path and not on Home. Take the claim NARROWLY, exactly as that file
// restorable view dereferences on its render turns that suite red, at either // states it: what those boots prove is no structural dereference on the code
// polarity — a value nothing writes is wrong-typed and so truthy, so each such // paths a WHOLLY-CORRUPTED PROFILE takes — which is not every path a stored
// field is also driven falsy, or proven unable to be falsy after the floor. So // record takes. Not driven: any pairing of values the four slots do not
// does a field that gains a floor while its row still claims it has none. Two // produce, a view only forward navigation opens, anything behind a click, and
// things the boots do not drive: a MIX of polarities, since one boot puts every // everything a healthy profile reaches. Within that boundary the verdict is
// corrupted field on the same slot; and whatever no stored record reaches by // unconditional, including a dereference that takes two corrupted fields at
// itself — a view only forward navigation opens, and anything behind a click. A // once. That suite also goes red on a field that gains a floor while its row
// field added to PERSISTED_FIELDS with no row fails the suite too. // still claims it has none, and on a field added to PERSISTED_FIELDS with no
// row at all.
// //
// That test exists because this comment did not work. It carried a // That test exists because this comment did not work. It carried a
// hand-written justification per field, and it shipped a false one in three // hand-written justification per field, and it shipped a false one in three

View File

@@ -1525,6 +1525,7 @@ async function goToConfirm(page, { token, balance, amount }) {
await page.fill("#send-amount", amount); await page.fill("#send-amount", amount);
await page.click("#btn-send-review"); await page.click("#btn-send-review");
await visible(page, "#view-confirm-tx"); await visible(page, "#view-confirm-tx");
await assertAddressesFit(page, "the confirmation screen");
} }
// A balance as the main view renders it: balanceLinesForAddress() writes // A balance as the main view renders it: balanceLinesForAddress() writes
@@ -3262,6 +3263,8 @@ test("eth_sendTransaction signs the approved transaction and broadcasts it (#183
JSON.stringify(screen.data), JSON.stringify(screen.data),
); );
await assertAddressesFit(popup, "the dApp transaction prompt");
const broadcastBefore = env.routeOpts.broadcastTransactions.length; const broadcastBefore = env.routeOpts.broadcastTransactions.length;
await popup.fill("#approve-tx-password", PASSWORD); await popup.fill("#approve-tx-password", PASSWORD);
await popup.click("#btn-approve-tx"); await popup.click("#btn-approve-tx");
@@ -3425,6 +3428,206 @@ test("the password never crossed either boundary in this section (#183)", async
await env.dapp.close(); await env.dapp.close();
}); });
// ------------------------------------------- address layout (#380)
//
// "addresses should never wrap in the common views. this doesn't mean to
// just change the css, but update the layout itself so the untruncated
// addresses are shown in full and don't mess up the layout."
//
// Every one of these questions is about glyph advances and the width of
// the box an address landed in, and nothing in the markup answers any of
// them: a row can hold `white-space: nowrap` and still be too narrow, and
// the popup's own `overflow-x-hidden` would then hide the evidence by
// clipping the tail. So they are measured in a real Chromium, on the real
// rendered views, one assertion per property #380 names:
//
// - the whole address is there (42 characters, no ellipsis)
// - it occupies exactly one line box
// - it fits its row, so the overflow-x escape hatch never engages
// - its row ends inside the popup's content box
// - and the document itself does not scroll sideways
//
// The narrowest containers the popup has are covered here — the
// transaction detail wells (`bg-well p-3 mx-1`) and the token contract
// well — so the wider ones cannot fail while these pass.
// Everything on screen that carries an address, measured in one pass.
// Views other than the current one are display:none and measure zero, so
// filtering on width leaves exactly what a user can see right now.
function addressRowReport(page) {
return page.evaluate(() => {
const app = document.getElementById("app");
const appRight = app.getBoundingClientRect().right;
const rows = [];
for (const el of document.querySelectorAll(".am-address")) {
const box = el.getBoundingClientRect();
if (box.width === 0) continue;
// Line boxes are counted off the inline content, because the
// element's own rect is one box whether the text inside it
// wrapped or not. A Range yields a rect per contained node as
// well as per line, so it is the distinct tops that count:
// a copyable span and the text inside it share one.
const range = document.createRange();
range.selectNodeContents(el);
const tops = new Set(
Array.from(range.getClientRects()).map((r) =>
Math.round(r.top),
),
);
rows.push({
text: el.innerText.trim(),
lineBoxes: tops.size,
overflow: el.scrollWidth - el.clientWidth,
overhang: Math.round(box.right - appRight),
});
}
return {
rows,
pageOverflow:
document.documentElement.scrollWidth -
document.documentElement.clientWidth,
};
});
}
async function assertAddressesFit(page, where) {
const report = await addressRowReport(page);
assert(
report.rows.length > 0,
where + ": no address rows were rendered, so nothing was measured",
);
for (const row of report.rows) {
assert(
/^0x[0-9a-fA-F]{40}$/.test(row.text),
where +
": the address is not shown whole: " +
JSON.stringify(row.text),
);
assert(
row.lineBoxes === 1,
where +
": " +
row.text +
" wrapped onto " +
row.lineBoxes +
" lines",
);
assert(
row.overflow <= 1,
where +
": " +
row.text +
" is " +
row.overflow +
"px wider than the row holding it",
);
assert(
row.overhang <= 1,
where +
": " +
row.text +
" reaches " +
row.overhang +
"px past the popup's content box",
);
}
assert(
report.pageOverflow <= 0,
where + ": the popup scrolls sideways by " + report.pageOverflow + "px",
);
return report.rows.length;
}
// Back to Home from wherever the suite above finished, without assuming
// which screen that was. Every screen the popup can rest on has a Back
// button, and Home has none, so unwinding until Home shows is the one
// route that does not depend on the order of the tests before this point.
async function unwindToHome(page) {
for (let i = 0; i < 12; i++) {
if (await page.isVisible("#view-main")) return;
const back = page
.locator(".view:not(.hidden) button", { hasText: "Back" })
.first();
if ((await back.count()) === 0) break;
await back.click();
await page.waitForTimeout(150);
}
await visible(page, "#view-main");
}
// The reproduction from the issue: a wallet holding more than one address.
// Every address in the list is a full 42 characters competing with the
// [info] and [x] controls for one row's width, which is the state the
// wallet view was reported wrapping in.
test("a wallet with two addresses lists both in full, unwrapped (#380)", async (env) => {
await unwindToHome(env.page);
const before = await env.page
.locator("#wallet-list .btn-addr-info")
.count();
await env.page.locator("#wallet-list .btn-add-address").first().click();
await env.page.waitForFunction(
(n) =>
document.querySelectorAll("#wallet-list .btn-addr-info").length > n,
before,
{ timeout: 60000 },
);
const shown = await assertAddressesFit(env.page, "the wallet list");
assert(
shown >= before + 1,
"the wallet list measured " +
shown +
" addresses, fewer than the " +
(before + 1) +
" it now holds",
);
// The [x] control only exists on a wallet holding more than one
// address, so its presence is also the proof the second one landed.
const removable = await env.page
.locator("#wallet-list .btn-remove-address")
.count();
assert(removable > 0, "the second address did not reach the wallet list");
});
test("every common view shows its addresses in full on one line (#380)", async (env) => {
await unwindToHome(env.page);
await assertAddressesFit(env.page, "Home");
await env.page.locator("#wallet-list .btn-addr-info").first().click();
await visible(env.page, "#view-address");
await visible(env.page, "#tx-list .tx-row");
await assertAddressesFit(env.page, "the address screen");
await env.page.click("#btn-receive");
await visible(env.page, "#view-receive");
await assertAddressesFit(env.page, "the receive screen");
await env.page.click("#btn-receive-back");
await visible(env.page, "#view-address");
await env.page.click("#btn-send");
await visible(env.page, "#view-send");
await assertAddressesFit(env.page, "the send screen");
await env.page.click("#btn-send-back");
await visible(env.page, "#view-address");
// The transaction detail screen carries the narrowest address rows in
// the popup: its fields sit inside a well that takes another 24px of
// padding and 8px of margin off the content width, and the token
// contract row there is narrower still.
await env.page.locator("#address-balances .balance-row").first().click();
await visible(env.page, "#view-address-token");
await assertAddressesFit(env.page, "the token screen");
await env.page.click("#btn-address-token-back");
await visible(env.page, "#view-address");
await env.page.locator("#tx-list .tx-row").first().click();
await visible(env.page, "#view-transaction");
await visible(env.page, "#tx-detail-token-contract-section");
await assertAddressesFit(env.page, "the transaction detail screen");
});
// ---------------------------------------------------------------- runner // ---------------------------------------------------------------- runner
async function main() { async function main() {

View File

@@ -44,12 +44,32 @@
// reason, and a field that cannot be falsy after the floor says so in its row // reason, and a field that cannot be falsy after the floor says so in its row
// and is proven so. // and is proven so.
// //
// What that buys: a field any restorable view dereferences on that view's // READ THE CLAIM NARROWLY. What this file proves is: NO STRUCTURAL
// render turns this file red, at either polarity. What it does not buy is a // DEREFERENCE ON THE CODE PATHS A WHOLLY-CORRUPTED PROFILE TAKES. That is not
// MIX of polarities — one boot puts every swept field on the same slot, so a // every path a stored record takes, and the difference is the whole of what
// branch reached only when one swept field is truthy and another falsy is not // this file does not cover:
// entered. Booting every field separately at every value would be several //
// hundred boots and most of the suite's budget; this is forty-four. // - Only the values in the table, in the SLOT arrangement below: four value
// combinations per view, not the product of twelve fields. A dereference
// reached only under a pairing no slot produces is not driven at all.
// - Only what a stored record reaches by ITSELF. A view only forward
// navigation opens, and anything behind a click, is not driven.
// - Nothing about the paths a HEALTHY profile takes, which is most of the
// popup. This file is a floor under one defect class, not a proof about
// the renderers.
//
// Within that boundary it is unconditional: if one of these boots leaves the
// popup unhealthy or off the view it stored, this file goes red — including
// when it takes two corrupted fields at once, because the verdict is the
// combined boot itself and the per-field re-boot below can only decorate the
// message. That last part is the one thing an earlier version got wrong: it
// asserted on the per-field list, so an observed dead popup that no single
// field reproduced was reported green.
//
// Booting every field separately at every value would be several hundred boots
// and most of the suite's budget; this is forty-four. Widening it further is
// out of scope — proving no field is dereferenced on any reachable render path
// is exhaustive verification of the popup, not a floor under a stored record.
// //
// The three claims this replaced, all false, all caught here by construction: // The three claims this replaced, all false, all caught here by construction:
// rpcUrl reaching `new JsonRpcProvider()` (a synchronous throw, not a caught // rpcUrl reaching `new JsonRpcProvider()` (a synchronous throw, not a caught
@@ -738,17 +758,23 @@ describe("a hostile routing value restoring onto", () => {
// routing turns this red and has to declare `routes` and take the individual // routing turns this red and has to declare `routes` and take the individual
// sweep above. // sweep above.
// //
// Combining does hide one thing, and the last slot is what stops it. A hostile // Combining hides one thing, and the last slot is what stops it. A hostile
// value is wrong-typed and therefore TRUTHY, so on a boot where every swept // value is wrong-typed and therefore TRUTHY, so on a boot where every swept
// field is hostile, no `if (!state.x)` branch is entered — and a dereference // field is hostile, no `if (!state.x)` branch is entered — and a dereference
// inside such a branch would go unseen however loudly it throws. The last slot // inside such a branch would go unseen however loudly it throws. The last slot
// is the falsy one: every swept field that CAN be falsy is falsy on it, which // is the falsy one: every swept field that CAN be falsy is falsy on it, which
// is also the state an ordinary install boots in for three of them, while the // is also the state an ordinary install boots in for three of them, while the
// fields that cannot be falsy stay hostile. Beyond that, a throw fails the boot // fields that cannot be falsy stay hostile-truthy. That makes it a MIX, and a
// whichever field threw, and a renderer that never ran is what `restored` // deliberate one — the interaction between a falsy flag and a still-hostile
// forbids. When it does go red, the same view is re-booted one field at a time // theme is a shape a stored record really produces.
// so the failure names the fields rather than leaving a reader to bisect twelve //
// of them. // The verdict is the combined boot, always. When it goes red the same view is
// re-booted one field at a time, so the failure NAMES a culprit instead of
// leaving a reader to bisect twelve fields — but that loop only decorates the
// message. It cannot clear the failure. A dereference that needs two corrupted
// fields at once is reproduced by neither field alone, and a version of this
// file that asserted on the named list reported exactly that case green while
// watching the popup die.
const UNROUTED = CONTRACT.filter((row) => swept(row) && !routes(row)); const UNROUTED = CONTRACT.filter((row) => swept(row) && !routes(row));
const HOSTILE_SLOTS = Math.max( const HOSTILE_SLOTS = Math.max(
...UNROUTED.map((row) => sweptValues(row).length), ...UNROUTED.map((row) => sweptValues(row).length),
@@ -774,29 +800,42 @@ describe("every field the router does not read, corrupted at once, onto", () =>
restoringOnto(view, fields), restoringOnto(view, fields),
view, view,
); );
if (together.errors.length === 0 && together.restored) {
expect(together).toEqual(RESTORED); // The per-field re-boot only DECORATES the message. The
return; // verdict is `together`, unconditionally: a dereference that
// needs two corrupted fields at once is reproduced by NEITHER
// field alone, so an assertion on the named list would report
// an observed dead popup as green.
const named = [];
if (together.errors.length > 0 || !together.restored) {
for (const row of UNROUTED) {
const one = await restoredHealth(
restoringOnto(view, {
[row.field]: fields[row.field],
}),
view,
);
if (one.errors.length === 0 && one.restored) continue;
named.push(
`${row.field}=${JSON.stringify(
fields[row.field],
)}: ` +
(one.errors.join("; ") || `fell off ${view}`),
);
}
if (named.length === 0) {
named.push(
"no single field reproduces it; it takes two or " +
`more of ${JSON.stringify(fields)}`,
);
}
} }
const named = []; expect({
for (const row of UNROUTED) {
const one = await restoredHealth(
restoringOnto(view, {
[row.field]: fields[row.field],
}),
view,
);
if (one.errors.length === 0 && one.restored) continue;
named.push(
`${row.field}=${JSON.stringify(fields[row.field])}: ` +
(one.errors.join("; ") || `fell off ${view}`),
);
}
expect({ view: view, fields: named }).toEqual({
view: view, view: view,
fields: [], together: together,
}); fields: named,
}).toEqual({ view: view, together: RESTORED, fields: [] });
}); });
} }
} }