Compare commits

...
Author SHA1 Message Date
sneak 65d4dd8e0f docs: name every approval-screen amount string, fix stale zero claim (closes #369)
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
check / check (push) Successful in 1m18s
The README's amount-display section claimed a genuine zero always renders
`0.0000`. That is corrected and scoped: it holds for the ERC-20 amount and for
the swap's `Amount` line on a literal-zero `amountIn` in a V2/V3 exact-in swap
or a zero `WRAP_ETH`. Only two zeros are stated in words before the floor — the
swap's `Min. received` (any zero minimum) and its V4 exact-in `Amount` (an
`amountIn` of zero, V4's open delta).

A new list names every string an amount slot can show: a formatted quantity,
`Unlimited`, `All available (V4 open delta)`, `None (no minimum guaranteed)`,
base units with decimals unknown, and `Unknown (not named in the calldata)`. It
also records that a zero `minBalance` on a `BALANCE_CHECK_ERC20` step now reads
the no-minimum wording where it once read `0.0000`. Each claim checked against
the tree. Docs only.

Model: opus-4-8
2026-09-21 23:30:10 +00:00
clawbot 598de3ff1a fix: re-enable Confirm Delete after a delete, so a second one needs no reopen (closes #335)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
The password route disabled its Confirm Delete button before the decrypt
and never re-enabled it on success, so a second delete in the same popup
session found a dead button until the popup was closed and reopened. The
lost-password route re-enabled its own button in its leave hook, so the
two screens on the one screen behaved differently.

Both routes now reset the button through the shared finishDelete(), the
one path they both take, and the lost-password leave hook no longer
handles it separately. Tests drive a password-route delete and a second
delete in the same session; they fail against the prior head, where the
button stays disabled after the first delete.

Model: opus-4-8
2026-09-22 01:28:02 +02:00
clawbot ae61792aee chore: keep the internal view id out of the release banner (closes #375)
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
check / check (push) Successful in 1m10s
The debug/testnet banner appended the active view's internal id, so the
user saw text like "[TESTNET] (approve-tx)" — developer vocabulary, and on
the approval screen it sat directly above the carefully worded line stating
what is being authorized. The view id is now gated on the compile-time
DEBUG constant instead of isDebug(), so it survives only in a debug build.
A testnet or the runtime debug toggle still raises the banner, but without
the view id, which is what a release build shows.

Model: opus-4-8
2026-09-22 00:45:06 +02:00
clawbot a1f082d686 docs: a release procedure from a green main to tagged, packaged artifacts (closes #387)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
Add docs/RELEASE.md, linked from README.md's Release Artifacts section, giving
the release procedure as a numbered list: confirm main is green in CI, confirm
the one version in package.json and the two manifests matches the intended tag,
make package from a clean checkout, verify SHA256SUMS, create the annotated tag
vX.Y.Z, then distribute per browser. Each step names who performs it, marks the
owner-only ones, and states the check that it worked. Every repo command cited
(make setup, make check, make package) exists on next; tagging and verification
use standard git and coreutils, and the CRX pack line is README's own.

The per-browser distribution step is written as pending the owner's choice on
issue 386, with the Firefox and Chrome options named but none presented as
settled. Docs only: no code or test changes.

Model: opus-4-8
2026-09-21 22:00:18 +02:00
clawbot 33fa25adca harden: bound the total network fee by gasLimit × fee, on both send paths (closes #399)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
The two per-field ceilings in approvalVerify.js were checked independently,
but the fee a validator is paid is gasLimit × fee per gas: a gas limit and a
fee each under their own ceiling still multiply to thousands of ETH, which a
gas-consuming contract really collects. assertWithinCeilings now also bounds
that product against MAX_TOTAL_FEE (1 ETH), so both callers — populating the
dApp transaction and verifying the signed artifact — refuse it with a full
sentence naming the fee and the limit.

The wallet's own send in confirmTx.js pinned no fee fields, so ethers filled
them from the node with no bound; it now populates the transaction and runs the
same check before signing, showing the same error in the confirmation screen's
reserved errors box so nothing on screen moves.

Model: opus-4-8
2026-09-21 21:45:34 +02:00
clawbot 2fe6447625 fix: name a tracked or explorer-known token instead of "Unknown token" (closes #323)
check / check (push) Failing after 0s
e2e / e2e-firefox (push) Failing after 0s
e2e / e2e-chrome (push) Failing after 1m27s
The approval and transaction-status screens read a token's scale from the
bundled list, the tokens the user tracks, then the block explorer, but read
its symbol from the bundled list alone. A token the user added by hand was
scaled correctly yet labelled "Unknown token", and a non-bundled ERC-20 was
carried onto the wait screen as ETH.

resolveTokenSymbol() now draws the symbol through the same sources and
precedence as the scale, and the ERC-20 and Uniswap swap lines both use it. A
tracked or explorer-reported name stays subject to the spoof rule, so it
cannot claim a bundled or native ticker.

Folds in #354.

Model: opus-4-8
Co-authored-by: clawbot <clawbot@noreply.example.org>
2026-09-21 21:28:06 +02:00
21 changed files with 947 additions and 81 deletions
+47 -4
View File
@@ -64,7 +64,9 @@ release/SHA256SUMS
``` ```
Nothing is published by this. Tagging, CRX packing and any upload are Nothing is published by this. Tagging, CRX packing and any upload are
outward-facing acts and are the owner's alone. outward-facing acts and are the owner's alone. The full procedure that turns a
green `main` into a tagged, packaged release — the order of steps, who performs
each, and how to check it worked — is in [docs/RELEASE.md](docs/RELEASE.md).
The archives are deterministic — entries sorted, timestamps fixed, compression The archives are deterministic — entries sorted, timestamps fixed, compression
level fixed — so two builds of one commit produce byte-identical files and the level fixed — so two builds of one commit produce byte-identical files and the
@@ -880,9 +882,18 @@ On those screens, when the truncated string would contain no digit from 1 to 9
and the value does, the amount is extended to its first significant digit and the value does, the amount is extended to its first significant digit
instead: `0.000000000000000001 DAI`, not `0.0000 DAI`. The test is on the whole instead: `0.000000000000000001 DAI`, not `0.0000 DAI`. The test is on the whole
truncated string, integer part included, so `1.00005` still shows as `1.0000` — truncated string, integer part included, so `1.00005` still shows as `1.0000` —
the exception only fires where the entire displayed figure would read as zero. A the exception only fires where the entire displayed figure would read as zero.
genuine zero still renders `0.0000`, and truncation stays truncation: `0.99999` Truncation stays truncation: `0.99999` shows as `0.9999`, never rounded up. A
shows as `0.9999`, never rounded up. genuine zero reaching this rule renders `0.0000`. The ERC-20
`approve`/`transfer` amount does exactly that, and so does the swap's `Amount`
line for a literal-zero `amountIn` on a V2 or V3 exact-in swap, or a zero
`WRAP_ETH` (`0.0000 ETH`). Only two zeros are stated in words before the floor:
the swap's `Min. received` line reads `None (no minimum guaranteed)` for any
zero minimum, and its `Amount` line reads `All available (V4 open delta)` for a
V4 exact-in `amountIn` of zero, which V4 treats as the whole open credit rather
than a quantity. So the guarantee that a zero is never shown as `0.0000` covers
the `Min. received` line and the V4 exact-in `Amount`; a V2/V3 or `WRAP_ETH`
`Amount` still renders it (see the list of amount-slot strings below).
The rule and its exception live in `src/shared/amountDisplay.js` as The rule and its exception live in `src/shared/amountDisplay.js` as
`truncateAmount()` and `truncateAmountNeverZero()`. Everything the approval and `truncateAmount()` and `truncateAmountNeverZero()`. Everything the approval and
@@ -934,6 +945,38 @@ and compare against. Reading the stored field directly instead answers `null`
for a bundled or tracked token the explorer merely omitted, which is not a for a bundled or tracked token the explorer merely omitted, which is not a
refusal the wallet has any reason to make. refusal the wallet has any reason to make.
**Every string an amount slot can show:** taken together, the exceptions above
mean an amount line on the dApp approval screen (and the wait/success/error
screens that carry a figure forward) shows one of a fixed set of strings, not
always a number:
- A formatted quantity, e.g. `17.1900 USDT`: the token's scale is known and the
figure is at or above the floor, or below it and extended to its first
significant digit. This is `truncateAmountNeverZero()`
(`src/shared/amountDisplay.js`).
- `Unlimited`: an unbounded allowance or permit, which needs no scale to
describe — a `uint256`-max ERC-20 `approve` (`src/popup/views/approval.js`) or
a Permit2 amount at the `uint160` max on a swap's `Amount`
(`src/shared/uniswap.js`).
- `All available (V4 open delta)`: a V4 exact-in swap whose `amountIn` is zero.
V4 reads that zero as "use the whole open delta", not as a literal zero, so
the calldata states no quantity at all. Swap `Amount` line only
(`src/shared/uniswap.js`).
- `None (no minimum guaranteed)`: a zero minimum — the swap guarantees nothing
back. It is a literal zero slippage floor on a V2/V3/V4 swap, and it also
reaches a `BALANCE_CHECK_ERC20` step: a zero `minBalance`, which once rendered
`0.0000` beside the token symbol, now reads this. Swap `Min. received` line
(`src/shared/uniswap.js`).
- `<amount> base units (decimals unknown)`: the token's scale could not be
resolved, so the base-unit integer is shown with that caveat rather than
formatted (see Unknown token scale above). Reaches both the ERC-20 amount line
and the swap's `Amount` and `Min. received` (`unknownDecimalsAmount()` in
`src/shared/approvalAmount.js`).
- `Unknown (not named in the calldata)`: not an amount but the currency itself —
the `Token In` or `Token Out` line when nothing in the calldata established
which token, shown beside the amount and, like the strings above, a sentence
rather than a value (`src/shared/uniswap.js`).
#### Partial USD totals #### Partial USD totals
Prices are fetched for the top 25 tokens only, so an address can hold assets the Prices are fetched for the top 25 tokens only, so an address can hold assets the
+77
View File
@@ -45,6 +45,28 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-09-21: The network fee a transaction can commit is bounded by the product
of the gas limit and the fee per gas, not by each field alone, and the
wallet's own send is bounded the same way
([#399](https://git.eeqj.de/sneak/AutistMask/issues/399)). The two per-field
ceilings in `src/shared/approvalVerify.js` were checked independently, so a
gas limit and a fee that were each under their own ceiling still multiplied to
thousands of ETH — a fee a gas-consuming contract really collects — while the
comment claimed the ceiling caught exactly that. `assertWithinCeilings` now
also refuses a transaction whose gas limit times its fee per gas
(`maxFeePerGas` for a type-2 transaction, `gasPrice` for a legacy or type-1
one) exceeds `MAX_TOTAL_FEE`, a new constant of 1 ETH beside the existing
ceilings, so both callers — where the dApp transaction is populated and where
the signed artifact is verified — reject it with a full sentence naming the
fee and the limit. The wallet's own send in `src/popup/views/confirmTx.js`
pinned no fee fields, so ethers filled them from whatever the configured node
answered with nothing bounding them; it now populates the transaction and runs
the same check before signing, showing the same error in the confirmation
screen's reserved errors box so nothing on screen moves. Deliberately out of
scope: comparing a supplied fee against the node's own suggested fee, which
the absolute bound already makes unnecessary for the balance-draining case. 1
ETH is a plain constant, one line to change; the owner may prefer another
figure.
- 2026-09-21: The test recovery phrase no longer survives in a release bundle, - 2026-09-21: The test recovery phrase no longer survives in a release bundle,
and the committed-key guard matches by content and the committed-key guard matches by content
([#351](https://git.eeqj.de/sneak/AutistMask/issues/351)). `DEBUG_MNEMONIC` in ([#351](https://git.eeqj.de/sneak/AutistMask/issues/351)). `DEBUG_MNEMONIC` in
@@ -68,6 +90,16 @@ but the review is broader than any of them.
approve, reject and disconnect paths against a transaction approval approve, reject and disconnect paths against a transaction approval
broadcasting behind them: each is declined and the dApp still receives its broadcasting behind them: each is declined and the dApp still receives its
broadcast result. broadcast result.
- 2026-09-21: `docs/RELEASE.md`, linked from `README.md`, states the release
procedure as a numbered list a newcomer can follow: confirm `main` is green in
CI, confirm the one version in the three files matches the intended tag,
`make package` from a clean checkout, verify `SHA256SUMS`, tag `vX.Y.Z`, then
distribute per browser. Each step names who performs it (owner-only steps
marked) and the check that it worked. The distribution step is written as
pending the owner's choice on
[#386](https://git.eeqj.de/sneak/AutistMask/issues/386), with the Firefox and
Chrome options named but none settled. Docs only
([#387](https://git.eeqj.de/sneak/AutistMask/issues/387)).
- 2026-09-21: Adding a second wallet no longer accepts a different password with - 2026-09-21: Adding a second wallet no longer accepts a different password with
nothing saying it is a separate one nothing saying it is a separate one
@@ -81,6 +113,51 @@ but the review is broader than any of them.
[#312](https://git.eeqj.de/sneak/AutistMask/issues/312) by promising no [#312](https://git.eeqj.de/sneak/AutistMask/issues/312) by promising no
recovery or reset. recovery or reset.
- 2026-09-21: The dApp approval and transaction-status screens resolve a token's
symbol from the bundled list, then the tokens the user tracks, then the block
explorer's report — the same sources and precedence the amount line already
used for the token's scale
([#323](https://git.eeqj.de/sneak/AutistMask/issues/323), folding in
[#354](https://git.eeqj.de/sneak/AutistMask/issues/354)). A token the user
added by hand, or holds a balance of, is now named rather than labelled
`Unknown token`, and a non-bundled ERC-20 is no longer carried onto the wait
screen as `ETH`. A tracked or explorer-reported name stays subject to the
spoof rule, so resolving a symbol is not a new way to wear a known ticker.
- 2026-09-21: The debug/testnet banner no longer shows the internal view id to
the user in a release build
([#375](https://git.eeqj.de/sneak/AutistMask/issues/375)). The banner appended
the active view's id (e.g. `[TESTNET] (approve-tx)`), which is developer
vocabulary sitting directly above the approval screen's carefully worded
authorization text. The suffix is now gated on the compile-time `DEBUG`
constant rather than `isDebug()`, so it survives only in a debug build; a
testnet or the runtime debug toggle still raises the banner but without the
view id.
- 2026-09-21: The Confirm Delete button on the delete-wallet screen no longer
stays dead after a successful delete
([#335](https://git.eeqj.de/sneak/AutistMask/issues/335)). The password route
disabled the button before the decrypt and never re-enabled it, so a second
delete in the same popup session needed a reopen; the lost-password route
re-enabled its own button in its leave hook, so the two screens behaved
differently. Both now reset through the shared `finishDelete()`, the one path
both routes take, so they behave the same and the button is live for the next
delete.
- 2026-09-21: `README.md` now documents the approval screen's amount-slot
vocabulary and no longer contradicts itself
([#369](https://git.eeqj.de/sneak/AutistMask/issues/369)). The stale claim
that a genuine zero still renders `0.0000` is corrected: it holds for the
ERC-20 amount, and also for the swap's `Amount` line on a literal-zero
`amountIn` in a V2/V3 exact-in swap or a zero `WRAP_ETH`. Only two zeros are
stated in words upstream — the swap's `Min. received` (any zero minimum) and
its V4 exact-in `Amount` (an `amountIn` of zero, V4's open delta). The
amount-display section now names every string a slot can show — a formatted
quantity, `Unlimited`, `All available (V4 open delta)`,
`None (no minimum guaranteed)`, base units with decimals unknown, and
`Unknown (not named in the calldata)` — and records that a zero `minBalance`
on a `BALANCE_CHECK_ERC20` step now reads `None (no minimum guaranteed)` where
it once read `0.0000`. Docs only; each claim checked against the tree.
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the - 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 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 wallet list was the reported case: the address shared one row with the
+87
View File
@@ -0,0 +1,87 @@
# Releasing AutistMask
This is the procedure that turns a green `main` into a tagged, packaged release.
It gathers into one place what is otherwise spread across the `Makefile` and
three `README.md` sections, so the person cutting a release does not have to
reconstruct the order from them.
There is one version, declared in three files (`package.json`,
`manifest/chrome.json`, `manifest/firefox.json`), and `make package` builds and
packages but publishes nothing. `make build` and `make package` can be run by
anyone; tagging, signing, packing a CRX and any upload need credentials only the
owner ([@sneak](https://sneak.berlin)) holds and are marked **owner-only**
below. Releases are tagged from `main` (see the Workflow section of `TODO.md`),
so the "release commit" throughout is the `main` commit the milestone PR merged.
## Procedure
1. **Confirm `main` is green in CI.** The `check` workflow
(`.gitea/workflows/check.yml`) runs `script/cibuild`, i.e. `docker build .`,
and the `Dockerfile` runs `make check` as a build step, so a green `check`
run is a green `make check`. Find the run for the exact release commit on the
tracker's Actions view. _Check:_ that commit's `check` run succeeded; running
`make check` on a clean checkout of the commit reproduces it and exits 0.
2. **Confirm the version matches the intended tag.** `package.json`,
`manifest/chrome.json` and `manifest/firefox.json` must all declare the same
`X.Y.Z`. `make build` fails when they disagree, but nothing checks that they
equal the tag you mean to create — that is this manual step. _Check:_ all
three files read the same `X.Y.Z`, and it is the version you intend to tag
`vX.Y.Z`.
3. **Build and package from a clean checkout of that commit.** From a fresh
clone, or a working tree with no local modifications (`git status` clean),
checked out at the release commit: run `make setup`, then `make package`.
`make package` runs `make build` first, so the archives can only be made from
a `dist/` verified against that build's own receipt as a release (not debug)
build. It writes three files into `release/`:
`autistmask-chrome-<version>.zip`, `autistmask-firefox-<version>.xpi`, and
`SHA256SUMS`. _Check:_ those three files exist and `<version>` in the archive
names is the version confirmed in step 2. The Firefox `.xpi` is **unsigned**
(see step 6 and "Installing on Firefox" in `README.md`).
4. **Verify `SHA256SUMS`.** The archives are deterministic — sorted entries,
fixed timestamps, fixed compression — so a second `make package` from another
clean checkout of the same commit produces byte-identical files. Verify the
recorded digests against the files with `sha256sum -c SHA256SUMS`, run from
`release/`. To confirm reproducibility, run `make package` again on a
separate clean checkout and compare the digests. _Check:_ `sha256sum -c`
reports `OK` for every file, and an independent build's digests match.
5. **Tag the release commit.** _(owner-only)_ Create an annotated tag `vX.Y.Z`
on the release commit and push it: `git tag -a vX.Y.Z` (with a message), then
`git push origin vX.Y.Z`. _Check:_ `git tag` lists `vX.Y.Z`, and
`git rev-parse vX.Y.Z^{commit}` resolves to the release commit.
6. **Distribute per browser.** _(owner-only; pending the owner's choice on
https://git.eeqj.de/sneak/AutistMask/issues/386)_ How 1.0.0 is distributed on
each browser is not yet decided; it is the open question on that issue, and
the concrete steps cannot be written until the owner records a choice there.
These steps need credentials only the owner holds. The options under
consideration are:
- **Firefox** — the packaged `.xpi` is unsigned, and release Firefox and ESR
refuse an unsigned add-on:
- (a) AMO self-distribution signing (unlisted): submit the `.xpi` to AMO
with the owner's credentials; AMO returns a signed `.xpi` installable
on every Firefox, with nothing listed publicly.
- (b) AMO listed: as (a), plus a public AMO listing and review.
- (c) Ship the unsigned `.xpi` and state that Firefox support means
Developer Edition, Nightly, or an Unbranded build with
`xpinstall.signatures.required` set to `false`.
- **Chrome** — the repo packs no CRX and publishes nothing; the extension id
is fixed by the `key` in `manifest/chrome.json`:
- (a) Chrome Web Store (unlisted): upload the `.zip` with the owner's
developer account; the store delivers installs and updates.
- (b) Self-hosted CRX signed with the private key the owner holds
(`chrome --pack-extension=dist/chrome --pack-extension-key=<path to the .pem>`),
installable only via enterprise policy on Windows and macOS, so
realistically Linux-only.
- (c) "Load unpacked" from `dist/chrome/` only, as today.
Once the owner decides, the chosen steps — including which credentials they
need and who holds them — are written into this section and `README.md`'s
installation sections are updated to match, which is part of the definition
of done of https://git.eeqj.de/sneak/AutistMask/issues/386. _Check:_ for a
store or AMO route, the artifact installs from the store or AMO on a clean
browser profile; for the CRX or unpacked route, the documented load succeeds
and Chrome reports the extension id `gipbhkogfopeahplcjhipkgpcimdpkip`.
+32 -21
View File
@@ -20,9 +20,9 @@ const {
} = require("ethers"); } = require("ethers");
const { getPrice, formatUsd } = require("../../shared/prices"); const { getPrice, formatUsd } = require("../../shared/prices");
const { ERC20_ABI } = require("../../shared/constants"); const { ERC20_ABI } = require("../../shared/constants");
const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList");
const { const {
resolveTokenDecimals, resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount, unknownDecimalsAmount,
} = require("../../shared/approvalAmount"); } = require("../../shared/approvalAmount");
// Four decimals, with the nonzero floor these screens hold: every amount this // Four decimals, with the nonzero floor these screens hold: every amount this
@@ -63,9 +63,15 @@ function tokenAmountText(rawAmount, decimals, symbol) {
}; };
} }
// The symbol shown for a token line, resolved from the bundled list, the
// tokens the user tracks, and the explorer's report — the same chain the
// amount line's scale comes from. Null when no source names one, so the token
// lines keep saying `Unknown token` for a token nothing knows.
function tokenLabel(address) { function tokenLabel(address) {
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); return resolveTokenSymbol(address, {
return t ? t.symbol : null; trackedTokens: state.trackedTokens,
wallets: state.wallets,
});
} }
// Try to decode calldata using known ABIs. // Try to decode calldata using known ABIs.
@@ -85,8 +91,7 @@ function decodeCalldata(data, toAddress) {
try { try {
const parsed = erc20Iface.parseTransaction({ data }); const parsed = erc20Iface.parseTransaction({ data });
if (parsed) { if (parsed) {
const token = TOKEN_BY_ADDRESS.get(toAddress.toLowerCase()); const tokenSymbol = resolveTokenSymbol(toAddress, decimalsSources);
const tokenSymbol = token ? token.symbol : null;
// null when no source knows this token's scale. It is not // null when no source knows this token's scale. It is not
// defaulted to 18: an amount formatted with a guessed scale is // defaulted to 18: an amount formatted with a guessed scale is
// the wrong number, and for a token with fewer decimals than the // the wrong number, and for a token with fewer decimals than the
@@ -242,8 +247,11 @@ function showTxApproval(details) {
const approvedTx = details.approvedTx; const approvedTx = details.approvedTx;
const toAddr = approvedTx.to; const toAddr = approvedTx.to;
const token = toAddr ? TOKEN_BY_ADDRESS.get(toAddr.toLowerCase()) : null;
const ethValue = formatEther(approvedTx.value || "0"); const ethValue = formatEther(approvedTx.value || "0");
const sources = {
trackedTokens: state.trackedTokens,
wallets: state.wallets,
};
// Build txInfo for status screens // Build txInfo for status screens
pendingTxDetails = { pendingTxDetails = {
@@ -251,14 +259,17 @@ function showTxApproval(details) {
to: toAddr || "", to: toAddr || "",
amount: formatTxValue(ethValue), amount: formatTxValue(ethValue),
token: "ETH", token: "ETH",
tokenSymbol: token ? token.symbol : null, tokenSymbol: null,
}; };
// If this is an ERC-20 call, try to extract the real recipient and amount // If this is an ERC-20 call, try to extract the real recipient and amount
const decoded = decodeCalldata(approvedTx.data, toAddr || ""); const decoded = decodeCalldata(approvedTx.data, toAddr || "");
if (decoded && decoded.details) { if (decoded && decoded.details) {
let decodedTokenAddr = null; // The asset the status summary is counted in: an ERC-20 call's Token
let decodedTokenSymbol = null; // contract, or a swap's input token. Its symbol is resolved from the
// same sources as the approval screen, so a non-bundled token the
// wallet knows is not carried onto the wait and success screens as ETH.
let assetAddr = null;
for (const d of decoded.details) { for (const d of decoded.details) {
if (d.label === "Recipient" && d.address) { if (d.label === "Recipient" && d.address) {
pendingTxDetails.to = d.address; pendingTxDetails.to = d.address;
@@ -266,20 +277,20 @@ function showTxApproval(details) {
if (d.label === "Amount") { if (d.label === "Amount") {
pendingTxDetails.amount = d.rawValue || d.value; pendingTxDetails.amount = d.rawValue || d.value;
} }
if (d.label === "Token In" && d.isToken && d.address) { if (
const t = TOKEN_BY_ADDRESS.get(d.address.toLowerCase()); (d.label === "Token" || d.label === "Token In") &&
if (t) { d.isToken &&
decodedTokenAddr = d.address; d.address
decodedTokenSymbol = t.symbol; ) {
} assetAddr = d.address;
} }
} }
if (token) { if (assetAddr) {
pendingTxDetails.token = toAddr; pendingTxDetails.token = assetAddr;
pendingTxDetails.tokenSymbol = token.symbol; pendingTxDetails.tokenSymbol = resolveTokenSymbol(
} else if (decodedTokenAddr) { assetAddr,
pendingTxDetails.token = decodedTokenAddr; sources,
pendingTxDetails.tokenSymbol = decodedTokenSymbol; );
} }
} }
+51 -24
View File
@@ -30,6 +30,7 @@ const {
displayedDecimals, displayedDecimals,
transferAmountUnits, transferAmountUnits,
} = require("../../shared/transferAmount"); } = require("../../shared/transferAmount");
const { assertWithinCeilings } = require("../../shared/approvalVerify");
const { const {
CODES, CODES,
FEE_PENDING, FEE_PENDING,
@@ -394,6 +395,46 @@ async function estimateGas(txInfo) {
} }
} }
// Populate the transaction this send describes, enforce the fee bound against
// the fees that were actually filled in, then sign and broadcast it. The send
// pins no fee fields, so ethers fills maxFeePerGas and the gas limit from what
// the configured RPC node answers, with nothing otherwise bounding what a
// hostile node can set — the dApp path's ceilings never reached this one.
// Populating before the check is what makes assertWithinCeilings() see the
// same numbers that would be signed; it throws an ApprovalMismatchError when
// the product gasLimit × maxFeePerGas is over the bound, which the caller
// shows in the reserved error area rather than sending.
async function populateVerifyAndSend(connectedSigner, tx) {
let request;
if (tx.token === "ETH") {
request = { to: tx.to, value: parseEther(tx.amount) };
} else {
const contract = new Contract(tx.token, ERC20_ABI, connectedSigner);
// The contract's decimals() is read to be COMPARED with the scale the
// screen rendered this amount at, not to encode with: encoding from it
// signs whatever the contract answers now, which is not what the user
// read. A disagreement throws. See transferAmount.js.
const amount = transferAmountUnits(
tx.amount,
tx.tokenDecimals,
await contract.decimals(),
);
request = await contract.transfer.populateTransaction(tx.to, amount);
}
const populated = await connectedSigner.populateTransaction(request);
assertWithinCeilings(populated);
return connectedSigner.sendTransaction(populated);
}
// Show a full-sentence send failure in the reserved errors box, the same
// element and markup renderValidation() uses for messages carrying the user's
// own numbers, so it never moves anything on the screen.
function showSendError(message) {
const el = $("confirm-errors");
el.innerHTML = `<div class="text-xs">${escapeHtml(message)}</div>`;
el.style.visibility = "visible";
}
async function checkRecipientHistory(txInfo) { async function checkRecipientHistory(txInfo) {
try { try {
const provider = getProvider(state.rpcUrl, state.networkId); const provider = getProvider(state.rpcUrl, state.networkId);
@@ -467,29 +508,7 @@ function init(_ctx) {
const provider = getProvider(state.rpcUrl, state.networkId); const provider = getProvider(state.rpcUrl, state.networkId);
const connectedSigner = signer.connect(provider); const connectedSigner = signer.connect(provider);
if (pendingTx.token === "ETH") { tx = await populateVerifyAndSend(connectedSigner, pendingTx);
tx = await connectedSigner.sendTransaction({
to: pendingTx.to,
value: parseEther(pendingTx.amount),
});
} else {
const contract = new Contract(
pendingTx.token,
ERC20_ABI,
connectedSigner,
);
// The contract's decimals() is read to be COMPARED with the
// scale the screen rendered this amount at, not to encode with:
// encoding from it signs whatever the contract answers now,
// which is not what the user read. A disagreement throws and is
// reported on the error screen. See transferAmount.js.
const amount = transferAmountUnits(
pendingTx.amount,
pendingTx.tokenDecimals,
await contract.decimals(),
);
tx = await contract.transfer(pendingTx.to, amount);
}
// Best-effort: clear decrypted secret after use. // Best-effort: clear decrypted secret after use.
// Note: JS strings are immutable; this nulls the reference but // Note: JS strings are immutable; this nulls the reference but
@@ -498,6 +517,14 @@ function init(_ctx) {
txStatus.showWait(pendingTx, tx.hash); txStatus.showWait(pendingTx, tx.hash);
} catch (e) { } catch (e) {
decryptedSecret = null; decryptedSecret = null;
// A fee over the bound is refused before anything is broadcast, so
// there is no transaction that may have reached the network to warn
// about: the message stays on the confirmation screen where the
// user can go back, rather than routing to the sent/failed screen.
if (e && e.approvalMismatch) {
showSendError(e.message);
return;
}
const hash = tx ? tx.hash : null; const hash = tx ? tx.hash : null;
txStatus.showError(pendingTx, hash, e.shortMessage || e.message); txStatus.showError(pendingTx, hash, e.shortMessage || e.message);
} finally { } finally {
@@ -511,4 +538,4 @@ function init(_ctx) {
}); });
} }
module.exports = { init, show, restore }; module.exports = { init, show, restore, populateVerifyAndSend };
+14 -7
View File
@@ -51,16 +51,12 @@ function clear() {
// The lost-password screen holds no secret — a wallet name is not one — // The lost-password screen holds no secret — a wallet name is not one —
// but it is wiped on leave for the neighbouring reason: a typed // but it is wiped on leave for the neighbouring reason: a typed
// confirmation left standing in a hidden view is one click away from // confirmation left standing in a hidden view is one click away from
// destroying a wallet the user has since navigated off. The button is // destroying a wallet the user has since navigated off.
// re-enabled here too, so a screen left mid-delete is usable on re-entry.
function clearLostPassword() { function clearLostPassword() {
lostPasswordIndex = null; lostPasswordIndex = null;
$("delete-wallet-lost-name-input").value = ""; $("delete-wallet-lost-name-input").value = "";
$("delete-wallet-lost-flash").textContent = ""; $("delete-wallet-lost-flash").textContent = "";
$("delete-wallet-lost-flash").style.visibility = "hidden"; $("delete-wallet-lost-flash").style.visibility = "hidden";
const btn = $("btn-delete-wallet-lost-confirm");
btn.disabled = false;
btn.classList.remove("text-muted");
} }
function show(walletIdx) { function show(walletIdx) {
@@ -98,6 +94,17 @@ function showLostPassword() {
// cleanup and the accountsChanged broadcast cannot drift apart between // cleanup and the accountsChanged broadcast cannot drift apart between
// them. // them.
async function finishDelete(walletIdx) { async function finishDelete(walletIdx) {
// Each route's confirm button was disabled by its own click handler
// before the delete ran. Re-enable both here, on the one path they
// share, so the two routes reset the same way and a second delete in
// the same popup session finds a live button instead of a dead one.
const passwordBtn = $("btn-delete-wallet-confirm");
passwordBtn.disabled = false;
passwordBtn.classList.remove("text-muted");
const lostPasswordBtn = $("btn-delete-wallet-lost-confirm");
lostPasswordBtn.disabled = false;
lostPasswordBtn.classList.remove("text-muted");
const { activeAddressChanged } = removeWalletFromState(state, walletIdx); const { activeAddressChanged } = removeWalletFromState(state, walletIdx);
deleteWalletIndex = null; deleteWalletIndex = null;
@@ -187,8 +194,8 @@ function init(_ctx) {
btn.disabled = true; btn.disabled = true;
btn.classList.add("text-muted"); btn.classList.add("text-muted");
// finishDelete() navigates, and the leave hook re-enables the // finishDelete() re-enables the button; navigating away then runs
// button and wipes the typed name on the way out. // the leave hook that wipes the typed name.
await finishDelete(lostPasswordIndex); await finishDelete(lostPasswordIndex);
}); });
+6 -1
View File
@@ -12,6 +12,7 @@
// escapeHtml lives in src/shared/html.js, where the escape and the // escapeHtml lives in src/shared/html.js, where the escape and the
// reasoning behind it are; it is re-exported below so views keep importing // reasoning behind it are; it is re-exported below so views keep importing
// it from here. // it from here.
const { DEBUG } = require("../../shared/constants");
const { escapeHtml } = require("../../shared/html"); const { escapeHtml } = require("../../shared/html");
const { isDebug } = require("../../shared/log"); const { isDebug } = require("../../shared/log");
const { formatUsd, getPrice } = require("../../shared/prices"); const { formatUsd, getPrice } = require("../../shared/prices");
@@ -119,7 +120,11 @@ function updateDebugBanner(viewName) {
"background:#c00;color:#fff;text-align:center;font-size:10px;padding:1px 0;font-family:monospace;position:sticky;top:0;z-index:9999;"; "background:#c00;color:#fff;text-align:center;font-size:10px;padding:1px 0;font-family:monospace;position:sticky;top:0;z-index:9999;";
document.body.prepend(banner); document.body.prepend(banner);
} }
const suffix = viewName ? " (" + viewName + ")" : ""; // The view id is internal vocabulary; it helps while developing but
// means nothing to a user. Only a debug build appends it, gated on the
// compile-time DEBUG constant so a release build never shows it — not
// isDebug(), which is also true for a testnet or the runtime toggle.
const suffix = DEBUG && viewName ? " (" + viewName + ")" : "";
if (debug && net.isTestnet) { if (debug && net.isTestnet) {
banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix; banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix;
} else if (net.isTestnet) { } else if (net.isTestnet) {
+9 -3
View File
@@ -13,7 +13,7 @@ const {
displaySymbol, displaySymbol,
clearViewStack, clearViewStack,
} = require("./helpers"); } = require("./helpers");
const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); const { resolveTokenSymbol } = require("../../shared/approvalAmount");
const { state } = require("../../shared/state"); const { state } = require("../../shared/state");
const { getProvider } = require("../../shared/balances"); const { getProvider } = require("../../shared/balances");
const { log } = require("../../shared/log"); const { log } = require("../../shared/log");
@@ -232,9 +232,15 @@ function showSuccess(txInfo, txHash, blockNumber) {
ctx.doRefreshAndRender(); ctx.doRefreshAndRender();
} }
// The symbol shown for a decoded token line, resolved from the bundled list,
// the tokens the user tracks, and the explorer's report — the same chain the
// approval screen uses. Null when no source names one, so the line keeps
// saying `Unknown token`.
function tokenLabel(address) { function tokenLabel(address) {
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); return resolveTokenSymbol(address, {
return t ? t.symbol : null; trackedTokens: state.trackedTokens,
wallets: state.wallets,
});
} }
function decodedDetailsHtml(decoded) { function decodedDetailsHtml(decoded) {
+55
View File
@@ -30,6 +30,7 @@
// enumerated rather than coerced. // enumerated rather than coerced.
const { toDecimals } = require("./transferAmount"); const { toDecimals } = require("./transferAmount");
const { TOKEN_BY_ADDRESS } = require("./tokenList"); const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Every decimals the explorer reported for this contract, across all the // Every decimals the explorer reported for this contract, across all the
// addresses whose balances have been fetched. They describe one contract, so // addresses whose balances have been fetched. They describe one contract, so
@@ -74,6 +75,59 @@ function resolveTokenDecimals(tokenAddress, sources) {
return explorerDecimals(lower, sources && sources.wallets); return explorerDecimals(lower, sources && sources.wallets);
} }
// Every symbol the explorer reported for this contract, across the addresses
// whose balances have been fetched. The counterpart to explorerDecimals(): one
// contract, so the reports should agree, and a set that does not agree is a
// name this screen has no way to choose between.
function explorerSymbol(lower, wallets) {
let found = null;
for (const wallet of wallets || []) {
for (const addr of wallet.addresses || []) {
for (const tb of addr.tokenBalances || []) {
if ((tb.address || "").toLowerCase() !== lower) continue;
if (!tb.symbol) continue;
if (found !== null && found !== tb.symbol) return null;
found = tb.symbol;
}
}
}
return found;
}
// The symbol to label a token with, or null when no source the wallet trusts
// names one — in which case the screen keeps saying `Unknown token` rather than
// guessing. The bundled list, then the tokens the user tracks, then what the
// explorer reported: the same sources and the same precedence
// resolveTokenDecimals() uses, so a token's name and its scale are drawn from
// the same place and the two can no longer disagree about which sources they
// trust. `sources` is { trackedTokens, wallets }, shaped as on `state`.
//
// A tracked or explorer-reported symbol is attacker-influenced text, so it is
// held to the spoof rule (symbolSpoof.js): a candidate that wears a bundled or
// native ticker from a contract not entitled to it is refused and the next
// source tried, so resolving a symbol never becomes a new way to claim a known
// ticker. The bundled list is the wallet's own data and is trusted as it is.
function resolveTokenSymbol(tokenAddress, sources) {
const lower = (tokenAddress || "").toLowerCase();
if (!lower) return null;
const bundled = TOKEN_BY_ADDRESS.get(lower);
if (bundled && bundled.symbol) return bundled.symbol;
const tracked = ((sources && sources.trackedTokens) || []).find(
(t) => (t.address || "").toLowerCase() === lower,
);
const candidates = [];
if (tracked && tracked.symbol) candidates.push(tracked.symbol);
const reported = explorerSymbol(lower, sources && sources.wallets);
if (reported) candidates.push(reported);
for (const symbol of candidates) {
if (!isSpoofedSymbol(symbol, tokenAddress)) return symbol;
}
return null;
}
// What the amount line reads when the scale is unknown. The base units are // What the amount line reads when the scale is unknown. The base units are
// exact and the caveat is part of the same string, so the number on the screen // exact and the caveat is part of the same string, so the number on the screen
// cannot be mistaken for a token quantity, and it can never read as zero for a // cannot be mistaken for a token quantity, and it can never read as zero for a
@@ -84,5 +138,6 @@ function unknownDecimalsAmount(rawAmount) {
module.exports = { module.exports = {
resolveTokenDecimals, resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount, unknownDecimalsAmount,
}; };
+35 -2
View File
@@ -51,6 +51,7 @@
const { const {
Transaction, Transaction,
accessListify, accessListify,
formatEther,
getAddress, getAddress,
getBytes, getBytes,
verifyMessage, verifyMessage,
@@ -134,10 +135,19 @@ const FORBIDDEN_FIELDS = [
const MAX_GAS_LIMIT = 100000000n; const MAX_GAS_LIMIT = 100000000n;
// 100,000 gwei per gas: orders of magnitude above the highest fee either // 100,000 gwei per gas: orders of magnitude above the highest fee either
// supported network has produced, and low enough to catch a fee that would // supported network has produced.
// hand the validator the balance.
const MAX_FEE_PER_GAS = 100000000000000n; const MAX_FEE_PER_GAS = 100000000000000n;
// The largest total fee this wallet will sign, in wei. The two ceilings above
// bound the gas limit and the price per gas each on its own, but the fee a
// validator is actually paid is their product, and a gas limit and a price
// that are each under their own ceiling still multiply to thousands of ETH —
// 30,000,000 gas at 100,000 gwei is about 3,000 ETH. Bounding the product is
// what catches a fee that would hand the validator the balance; the per-field
// ceilings alone do not. A full 30,000,000-gas block at 33 gwei reaches this,
// which no ordinary wallet transaction approaches.
const MAX_TOTAL_FEE = 1000000000000000000n; // 1 ETH
// A refusal to act on an artifact: it is not the thing that was approved, so // A refusal to act on an artifact: it is not the thing that was approved, so
// the approval it was offered against is spent and must not be retried. Every // the approval it was offered against is spent and must not be retried. Every
// throw in this module is one of these; the background distinguishes them from // throw in this module is one of these; the background distinguishes them from
@@ -384,6 +394,28 @@ function assertWithinCeilings(tx) {
); );
} }
} }
// The product: gasLimit × the most this transaction could pay per gas —
// maxFeePerGas for a type-2 transaction, gasPrice for a legacy or type-1
// one. This is the fee a gas-consuming contract can really extract, and it
// is the bound the two per-field ceilings above cannot express.
if (present(tx.gasLimit)) {
const gasLimit = normalizeQuantity(tx.gasLimit, "gas limit");
let price = null;
if (present(tx.maxFeePerGas)) {
price = normalizeQuantity(tx.maxFeePerGas, "maximum fee per gas");
} else if (present(tx.gasPrice)) {
price = normalizeQuantity(tx.gasPrice, "gas price");
}
if (price !== null && gasLimit * price > MAX_TOTAL_FEE) {
throw refuse(
"This transaction would allow a network fee of up to " +
formatEther(gasLimit * price) +
" ETH, which is more than the " +
formatEther(MAX_TOTAL_FEE) +
" ETH this wallet will sign for.",
);
}
}
} }
// Refuse a field only a transaction type this wallet does not sign can carry. // Refuse a field only a transaction type this wallet does not sign can carry.
@@ -775,4 +807,5 @@ module.exports = {
TX_STAGE_NONCE, TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
}; };
+2 -3
View File
@@ -2,10 +2,10 @@
// swap details. Designed to be extended with other DEX decoders later. // swap details. Designed to be extended with other DEX decoders later.
const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers"); const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { truncateAmountNeverZero } = require("./amountDisplay"); const { truncateAmountNeverZero } = require("./amountDisplay");
const { const {
resolveTokenDecimals, resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount, unknownDecimalsAmount,
} = require("./approvalAmount"); } = require("./approvalAmount");
@@ -123,9 +123,8 @@ function tokenInfo(address, sources) {
if (address === "0x0000000000000000000000000000000000000000") { if (address === "0x0000000000000000000000000000000000000000") {
return { symbol: "ETH", decimals: 18, address: null }; return { symbol: "ETH", decimals: 18, address: null };
} }
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase());
return { return {
symbol: t ? t.symbol : null, symbol: resolveTokenSymbol(address, sources),
decimals: resolveTokenDecimals(address, sources), decimals: resolveTokenDecimals(address, sources),
address, address,
}; };
+5 -3
View File
@@ -143,16 +143,18 @@ describe("decodeCalldata amount", () => {
state.trackedTokens = [ state.trackedTokens = [
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
]; ];
// The tracked entry supplies both: the scale (5000.0000) and, since
// issue #323, the symbol that the scale is counted in.
expect( expect(
amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN),
).toBe("5000.0000"); ).toBe("5000.0000 NOVEL");
}); });
test("transfer priced off the explorer's decimals shows the true quantity", () => { test("transfer priced off the explorer's decimals shows the true quantity", () => {
state.wallets = walletsHolding(NOVEL_TOKEN, "6"); state.wallets = walletsHolding(NOVEL_TOKEN, "6");
expect( expect(
amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN),
).toBe("5000.0000"); ).toBe("5000.0000 NOVEL");
}); });
test("transfer of an unknown-decimals token shows base units, not a number", () => { test("transfer of an unknown-decimals token shows base units, not a number", () => {
@@ -172,7 +174,7 @@ describe("decodeCalldata amount", () => {
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
]; ];
expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe( expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe(
"5000.0000", "5000.0000 NOVEL",
); );
}); });
+18
View File
@@ -212,6 +212,24 @@ describe("prepareApprovalTx", () => {
).rejects.toThrow(/gas limit no network this wallet supports/); ).rejects.toThrow(/gas limit no network this wallet supports/);
}); });
// The combined bound at population: a gas limit and a fee that are each
// under their own ceiling but multiply to thousands of ETH is refused
// before the approval window opens, so the user is never shown a
// balance-draining fee to click past.
test("refuses a fee whose product with the gas limit is over the bound", async () => {
const gouging = providerWith({
estimateGas: async () => 30000000n,
getFeeData: async () => ({
gasPrice: MAX_FEE_PER_GAS,
maxFeePerGas: MAX_FEE_PER_GAS,
maxPriorityFeePerGas: 1000000000n,
}),
});
await expect(
prepareApprovalTx(gouging, signer.address, TX_PARAMS),
).rejects.toThrow(/network fee of up to/);
});
// No approval and no window: the failure goes back to the page the click // No approval and no window: the failure goes back to the page the click
// came from, in a sentence. // came from, in a sentence.
test("reports a failed estimate as a full sentence", async () => { test("reports a failed estimate as a full sentence", async () => {
+114
View File
@@ -28,6 +28,7 @@ const {
TX_STAGE_NONCE, TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify"); } = require("../src/shared/approvalVerify");
const { prepareApprovalTx } = require("../src/shared/approvalTx"); const { prepareApprovalTx } = require("../src/shared/approvalTx");
const { getSignerForAddress } = require("../src/shared/wallet"); const { getSignerForAddress } = require("../src/shared/wallet");
@@ -475,18 +476,131 @@ describe("verifySignedTx field comparison", () => {
assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }), assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }),
).toThrow(/fee per gas far above any plausible value/); ).toThrow(/fee per gas far above any plausible value/);
} }
// Each field at its own ceiling multiplies to about 10,000 ETH, which
// is exactly the combination the per-field ceilings cannot see and the
// product bound is for: it is refused, not accepted.
expect(() => expect(() =>
assertWithinCeilings({ assertWithinCeilings({
gasLimit: MAX_GAS_LIMIT, gasLimit: MAX_GAS_LIMIT,
maxFeePerGas: MAX_FEE_PER_GAS, maxFeePerGas: MAX_FEE_PER_GAS,
maxPriorityFeePerGas: MAX_FEE_PER_GAS, maxPriorityFeePerGas: MAX_FEE_PER_GAS,
}), }),
).toThrow(/network fee of up to/);
// An ordinary transaction — a modest gas limit and a modest fee, each
// far under its ceiling and their product far under the bound — passes.
expect(() =>
assertWithinCeilings({
gasLimit: 21000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
}),
).not.toThrow(); ).not.toThrow();
// Nothing to bound is not a failure: a type 2 approval carries no gas // Nothing to bound is not a failure: a type 2 approval carries no gas
// price, and a bare object must not be refused for lacking one. // price, and a bare object must not be refused for lacking one.
expect(() => assertWithinCeilings({})).not.toThrow(); expect(() => assertWithinCeilings({})).not.toThrow();
}); });
// The defect this issue closes: gasLimit and maxFeePerGas each under their
// own ceiling, but their product — the fee a gas-consuming contract can
// really extract — thousands of ETH. The per-field ceilings accept it; the
// product bound refuses it, on either side of the screen.
describe("the combined fee bound", () => {
// A gas limit and a fee that are each comfortably under their own
// ceiling but multiply to well over 1 ETH: 30,000,000 gas at 100,000
// gwei is about 3,000 ETH.
const OVER = { gasLimit: 30000000n, maxFeePerGas: 100000000000000n };
test("each field is under its own ceiling", () => {
expect(OVER.gasLimit).toBeLessThan(MAX_GAS_LIMIT);
expect(OVER.maxFeePerGas).toBeLessThanOrEqual(MAX_FEE_PER_GAS);
expect(OVER.gasLimit * OVER.maxFeePerGas).toBeGreaterThan(
MAX_TOTAL_FEE,
);
});
test("assertWithinCeilings refuses the product over the bound", () => {
expect(() =>
assertWithinCeilings({
...OVER,
maxPriorityFeePerGas: 1000000000n,
}),
).toThrow(/network fee of up to 3000\.0 ETH/);
});
test("assertWithinCeilings bounds a legacy gasPrice the same way", () => {
expect(() =>
assertWithinCeilings({
gasLimit: OVER.gasLimit,
gasPrice: OVER.maxFeePerGas,
}),
).toThrow(/network fee of up to/);
});
// The boundary itself, pinned rather than only some value well past
// it. Both fields stay under their own ceilings, so it is the product
// and nothing else that decides these two cases: a gas limit of 10,000
// at the per-gas ceiling is exactly 1 ETH.
test("assertWithinCeilings accepts a product exactly at the bound and refuses one wei over", () => {
expect(MAX_FEE_PER_GAS * 10000n).toBe(MAX_TOTAL_FEE);
expect(() =>
assertWithinCeilings({
gasLimit: 10000n,
maxFeePerGas: MAX_FEE_PER_GAS,
}),
).not.toThrow();
expect(() =>
assertWithinCeilings({
gasLimit: 10001n,
maxFeePerGas: MAX_FEE_PER_GAS,
}),
).toThrow(/network fee of up to/);
});
// The dApp path: an artifact whose fee is within each field's ceiling
// but over the product bound, both displayed and signed, is refused at
// verification just as it is at population.
test("verifySignedTx refuses an over-bound product even when displayed", async () => {
const raw = await signedWith(OVER);
expect(() =>
verifySignedTx(
raw,
approvedFor(TX_PARAMS, OVER),
signer.address,
SELECTED,
),
).toThrow(/network fee of up to/);
});
test("verifySignedTx accepts a product just under the bound", async () => {
// 21,000 gas at 40 gwei is 0.00084 ETH — an ordinary send.
const under = { gasLimit: 21000n, maxFeePerGas: 40000000000n };
expect(under.gasLimit * under.maxFeePerGas).toBeLessThan(
MAX_TOTAL_FEE,
);
const raw = await signedWith(under);
expect(() =>
verifySignedTx(
raw,
approvedFor(TX_PARAMS, under),
signer.address,
SELECTED,
),
).not.toThrow();
});
test("the refusal names the fee and the limit in a full sentence", () => {
try {
assertWithinCeilings(OVER);
throw new Error("expected a rejection");
} catch (e) {
expect(e.approvalMismatch).toBe(true);
expect(e.message).toMatch(/^[A-Z].*\.$/);
expect(e.message).toContain("3000.0 ETH");
expect(e.message).toContain("1.0 ETH");
}
});
});
test("every field mismatch is a refusal, not a warning", async () => { test("every field mismatch is a refusal, not a warning", async () => {
const raw = await signedWith({ nonce: 8 }); const raw = await signedWith({ nonce: 8 });
try { try {
+100
View File
@@ -0,0 +1,100 @@
// The wallet's OWN send path enforces the same combined fee bound the dApp
// path does (https://git.eeqj.de/sneak/AutistMask/issues/399).
//
// The send in src/popup/views/confirmTx.js pins no fee fields, so ethers fills
// maxFeePerGas and the gas limit from whatever the configured RPC node
// answers. Nothing bounded that: a hostile node could report a fee whose
// product with the gas limit is thousands of ETH, and it would be both
// displayed and signed. populateVerifyAndSend() populates the transaction and
// runs assertWithinCeilings() on the populated fees before signing, so an
// over-bound send is refused before anything is broadcast.
//
// The check is driven here with a fake connected signer rather than a real
// one: populateTransaction() returns the fees the node would have produced,
// and sendTransaction() records whether the send actually happened. The real
// DOM path around it — reading the fee error into the reserved errors box — is
// covered by the Chrome e2e suite.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
const { populateVerifyAndSend } = require("../src/popup/views/confirmTx");
const {
MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify");
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
// A signer whose populateTransaction() fills in the fees a node quoted and
// whose sendTransaction() records the call, so a test can assert whether the
// send was reached at all.
function fakeSigner(fees) {
const sent = [];
return {
sent,
populateTransaction: async (request) => ({
...request,
from: RECIPIENT,
nonce: 0,
type: 2,
chainId: 1n,
gasLimit: fees.gasLimit,
maxFeePerGas: fees.maxFeePerGas,
maxPriorityFeePerGas: 1000000000n,
}),
sendTransaction: async (tx) => {
sent.push(tx);
return { hash: "0xabc" };
},
};
}
const ETH_SEND = { token: "ETH", to: RECIPIENT, amount: "1.0" };
describe("populateVerifyAndSend enforces the combined fee bound", () => {
// A gas limit and a fee that are each under their own field ceiling, but
// multiply to about 3,000 ETH — the combination the per-field ceilings
// cannot see.
const OVER = { gasLimit: 30000000n, maxFeePerGas: MAX_FEE_PER_GAS };
test("each field is under its ceiling but the product is over the bound", () => {
expect(OVER.maxFeePerGas).toBeLessThanOrEqual(MAX_FEE_PER_GAS);
expect(OVER.gasLimit * OVER.maxFeePerGas).toBeGreaterThan(
MAX_TOTAL_FEE,
);
});
test("refuses an over-bound send without broadcasting it", async () => {
const signer = fakeSigner(OVER);
let thrown;
try {
await populateVerifyAndSend(signer, ETH_SEND);
} catch (e) {
thrown = e;
}
expect(thrown).toBeDefined();
expect(thrown.approvalMismatch).toBe(true);
expect(thrown.message).toMatch(/^[A-Z].*\.$/);
expect(thrown.message).toContain("3000.0 ETH");
expect(thrown.message).toContain("1.0 ETH");
// The one guarantee that matters: nothing was signed or sent.
expect(signer.sent).toHaveLength(0);
});
test("broadcasts a send whose product is just under the bound", async () => {
// 21,000 gas at 40 gwei is 0.00084 ETH — an ordinary send.
const under = { gasLimit: 21000n, maxFeePerGas: 40000000000n };
expect(under.gasLimit * under.maxFeePerGas).toBeLessThan(MAX_TOTAL_FEE);
const signer = fakeSigner(under);
const tx = await populateVerifyAndSend(signer, ETH_SEND);
expect(tx.hash).toBe("0xabc");
expect(signer.sent).toHaveLength(1);
expect(signer.sent[0].gasLimit).toBe(under.gasLimit);
});
});
+59
View File
@@ -0,0 +1,59 @@
// Tests for the debug/testnet banner (issue #375).
//
// On a testnet the banner is raised even in a release build, but it must not
// append the active view's internal id: the user should see "[TESTNET]", never
// "[TESTNET] (approve-tx)". The suffix is gated on the compile-time DEBUG
// constant, which is false in a plain test load, so this drives exactly the
// text a shipped build renders. Revert the gate to the old unconditional
// suffix and this fails.
//
// The banner is created on demand by updateDebugBanner(); the document stub
// records what it prepends so the assertion can read the resulting text.
function makeBanner() {
return {
id: "",
textContent: "",
style: { cssText: "" },
remove() {},
};
}
function makeDocument() {
let banner = null;
return {
getElementById(id) {
return id === "debug-banner" ? banner : null;
},
createElement: () => makeBanner(),
body: {
prepend(node) {
banner = node;
},
},
};
}
function load() {
jest.resetModules();
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
globalThis.document = makeDocument();
const helpers = require("../src/popup/views/helpers");
const { state } = require("../src/shared/state");
return { helpers, state };
}
describe("the release banner on a testnet", () => {
test("carries no internal view id", () => {
const { helpers, state } = load();
state.networkId = "sepolia";
helpers.updateDebugBanner("approve-tx");
expect(
globalThis.document.getElementById("debug-banner").textContent,
).toBe("[TESTNET]");
});
});
+65 -5
View File
@@ -172,6 +172,15 @@ async function openLostPassword(deleteWallet, walletIdx) {
await click("btn-delete-wallet-lost-password"); await click("btn-delete-wallet-lost-password");
} }
// Delete a wallet through the password route: open its confirm screen,
// enter the password, and confirm. The vault is mocked, so the password
// text itself is irrelevant — decryptWithPassword decides pass or fail.
async function deleteWithPassword(deleteWallet, walletIdx) {
deleteWallet.show(walletIdx);
node("delete-wallet-password").value = "any password";
await click("btn-delete-wallet-confirm");
}
// ------------------------------------------------------------ tests // ------------------------------------------------------------ tests
// The stub is what every persistence assertion below rests on, so its one // The stub is what every persistence assertion below rests on, so its one
@@ -456,15 +465,21 @@ describe("what the screen leaves behind", () => {
); );
}); });
// Left mid-delete, the screen has to come back usable. // Both routes now re-enable through finishDelete(), not their leave
test("the confirm button is re-enabled on the way out", async () => { // hooks, so the button comes back live once a delete completes.
const { helpers, deleteWallet } = load(); test("the confirm button is re-enabled after a delete", async () => {
const { deleteWallet } = load();
await openLostPassword(deleteWallet, 1); await openLostPassword(deleteWallet, 1);
node("btn-delete-wallet-lost-confirm").disabled = true; node("delete-wallet-lost-name-input").value = "Wallet 2";
helpers.showView("settings"); await click("btn-delete-wallet-lost-confirm");
expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false); expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false);
expect(
node("btn-delete-wallet-lost-confirm").classList.contains(
"text-muted",
),
).toBe(false);
}); });
// A wallet name is not a secret, so the screen is excluded for the // A wallet name is not a secret, so the screen is excluded for the
@@ -475,3 +490,48 @@ describe("what the screen leaves behind", () => {
expect(RESTORABLE_VIEWS.has("delete-wallet-confirm")).toBe(false); expect(RESTORABLE_VIEWS.has("delete-wallet-confirm")).toBe(false);
}); });
}); });
// The password route is the pre-existing bug this file's fix addresses:
// its Confirm Delete button was disabled before the decrypt and never
// re-enabled on success, so a second delete in the same popup session
// found a dead button. Now both routes re-enable through finishDelete().
//
// Against head these tests fail: with the re-enable absent, the button
// stays disabled after the first delete, so the disabled assertions read
// true where they expect false.
describe("the password route's confirm button", () => {
test("is re-enabled after a successful delete", async () => {
const { deleteWallet, vault } = load();
vault.decryptWithPassword.mockResolvedValue();
await deleteWithPassword(deleteWallet, 1);
expect(node("btn-delete-wallet-confirm").disabled).toBe(false);
expect(
node("btn-delete-wallet-confirm").classList.contains("text-muted"),
).toBe(false);
});
// The reported symptom: delete one wallet, then open Delete Wallet for
// a second one without reopening the popup. The button must be live on
// that second visit, and the second delete must actually persist.
test("a second delete works in the same popup session", async () => {
const { deleteWallet, vault, storage } = load();
vault.decryptWithPassword.mockResolvedValue();
await deleteWithPassword(deleteWallet, 1);
// Wallet 2 is gone; the list is now [Wallet 1, Wallet 3]. Opening
// the confirm screen for the wallet now at index 1 (Wallet 3) must
// find its button live, not the dead one the first delete left.
deleteWallet.show(1);
expect(node("btn-delete-wallet-confirm").disabled).toBe(false);
node("delete-wallet-password").value = "any password";
await click("btn-delete-wallet-confirm");
expect((await persistedWallets(storage)).map((w) => w.name)).toEqual([
"Wallet 1",
]);
});
});
+3 -2
View File
@@ -253,8 +253,9 @@ describe("the ERC-20 approval line reaches its refusal", () => {
test("a scale the explorer did report still formats", async () => { test("a scale the explorer did report still formats", async () => {
await fetchOnto([row({ decimals: "6" })]); await fetchOnto([row({ decimals: "6" })]);
// The same explorer entry now also names the token (issue #323).
expect(erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL)).toBe( expect(erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL)).toBe(
"1000.0000", "1000.0000 NOVEL",
); );
}); });
}); });
@@ -273,7 +274,7 @@ describe("the swap approval line reaches its refusal", () => {
await fetchOnto([row({ decimals: "6" })]); await fetchOnto([row({ decimals: "6" })]);
expect( expect(
swapAmountLine(swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH)), swapAmountLine(swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH)),
).toBe("1000.0000"); ).toBe("1000.0000 NOVEL");
}); });
}); });
+156
View File
@@ -0,0 +1,156 @@
// The symbol the dApp approval and status screens label a token with.
//
// Issue #323: the approval screen labelled anything outside the bundled list
// `Unknown token`, even a token the user tracks or holds a balance of, while
// the amount line already read that token's *scale* from those same sources
// (issue #306). The name and the scale disagreed about which sources they
// trust. resolveTokenSymbol() closes that gap: it draws the symbol from the
// bundled list, then the tracked tokens, then the explorer's report — the
// precedence resolveTokenDecimals() uses — and returns null, not a guess,
// when nothing names it, so the screens keep saying `Unknown token`.
//
// A tracked or explorer-reported symbol is attacker-influenced text, so it
// stays subject to the spoof rule (src/shared/symbolSpoof.js): resolving a
// symbol must not become a new way for a stray contract to wear a bundled or
// native ticker.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { Interface } = require("ethers");
const { ERC20_ABI } = require("../src/shared/constants");
const { state } = require("../src/shared/state");
const { resolveTokenSymbol } = require("../src/shared/approvalAmount");
const { decodeCalldata } = require("../src/popup/views/approval");
const iface = new Interface(ERC20_ABI);
// Outside the bundled list, as the great majority of ERC-20s are.
const NOVEL_TOKEN = "0xE2E0000000000000000000000000000000000E2e";
// In the bundled list: USDC at 6 decimals, DAI at 18.
const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
const FIVE_THOUSAND_AT_SIX = 5000000000n;
function transferData(amount) {
return iface.encodeFunctionData("transfer", [RECIPIENT, amount]);
}
// A wallet whose block-explorer balance for `token` reports `symbol`, shaped
// as balances.js writes it onto state.
function walletsReporting(token, symbol) {
return [
{
name: "Wallet 1",
addresses: [
{
address: "0x" + "a".repeat(40),
balance: "1.0",
tokenBalances: [
{
address: token,
symbol,
decimals: 6,
balance: "5000.0",
},
],
},
],
},
];
}
beforeEach(() => {
state.trackedTokens = [];
state.wallets = [];
});
describe("resolveTokenSymbol", () => {
test("reads the bundled list", () => {
expect(resolveTokenSymbol(USDC, state)).toBe("USDC");
});
test("prefers the bundled list over a tracked entry", () => {
state.trackedTokens = [{ address: USDC, symbol: "NOTUSDC" }];
expect(resolveTokenSymbol(USDC, state)).toBe("USDC");
});
test("reads a token the user tracks", () => {
state.trackedTokens = [{ address: NOVEL_TOKEN, symbol: "NOVEL" }];
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBe("NOVEL");
});
test("reads the symbol the explorer reported", () => {
state.wallets = walletsReporting(NOVEL_TOKEN, "NOVEL");
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBe("NOVEL");
});
test("is null when no source names the token", () => {
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull();
});
test("refuses a name the explorer's own entries disagree about", () => {
const wallets = walletsReporting(NOVEL_TOKEN, "NOVEL");
wallets[0].addresses.push({
address: "0x" + "b".repeat(40),
balance: "0.0",
tokenBalances: [{ address: NOVEL_TOKEN, symbol: "OTHER" }],
});
state.wallets = wallets;
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull();
});
test("rejects a tracked entry claiming a bundled ticker it is not", () => {
// NOVEL_TOKEN is not the real USDC contract, so it may not wear USDC.
state.trackedTokens = [{ address: NOVEL_TOKEN, symbol: "USDC" }];
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull();
});
test("rejects an explorer entry claiming the native ETH ticker", () => {
state.wallets = walletsReporting(NOVEL_TOKEN, "ETH");
expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull();
});
});
describe("decodeCalldata symbol", () => {
test("a tracked token is named, not called Unknown", () => {
state.trackedTokens = [
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
];
const decoded = decodeCalldata(
transferData(FIVE_THOUSAND_AT_SIX),
NOVEL_TOKEN,
);
expect(decoded.description).toBe("Transfer NOVEL");
const amount = decoded.details.find((d) => d.label === "Amount");
expect(amount.value).toBe("5000.0000 NOVEL");
});
test("a token nothing knows keeps a symbol-less label", () => {
const decoded = decodeCalldata(
transferData(FIVE_THOUSAND_AT_SIX),
NOVEL_TOKEN,
);
expect(decoded.description).toBe("Transfer ERC-20 token");
const token = decoded.details.find((d) => d.label === "Token");
// The Token line carries the address and is flagged for the screen's
// symbol lookup, which resolves to nothing here — so `Unknown token`.
expect(token.isToken).toBe(true);
expect(token.address).toBe(NOVEL_TOKEN);
expect(resolveTokenSymbol(token.address, state)).toBeNull();
});
test("a tracked token spoofing a bundled ticker is not named by it", () => {
state.trackedTokens = [
{ address: NOVEL_TOKEN, symbol: "USDC", decimals: 6 },
];
const decoded = decodeCalldata(
transferData(FIVE_THOUSAND_AT_SIX),
NOVEL_TOKEN,
);
expect(decoded.description).toBe("Transfer ERC-20 token");
const amount = decoded.details.find((d) => d.label === "Amount");
expect(amount.value).not.toMatch(/USDC/);
});
});
+8 -3
View File
@@ -78,15 +78,20 @@ describe("a swap to a token absent from the bundled list", () => {
); );
}); });
test("names the address when the scale is known but the symbol is not", () => { test("names the tracked symbol alongside the address (issue #323)", () => {
// The tracked entry supplies both halves now: the scale, and the
// symbol the output line is named by. Before #323 the symbol was read
// from the bundled list alone, so this line fell back to the address.
const sources = { const sources = {
trackedTokens: [ trackedTokens: [
{ address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 }, { address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 },
], ],
}; };
expect(detail(data(), "Token Out", sources).value).toBe(NOVEL_OUT); expect(detail(data(), "Token Out", sources).value).toBe(
"NOVEL (" + NOVEL_OUT + ")",
);
expect(detail(data(), "Min. received", sources).value).toBe( expect(detail(data(), "Min. received", sources).value).toBe(
"1000.0000", "1000.0000 NOVEL",
); );
}); });
}); });
+4 -3
View File
@@ -98,12 +98,13 @@ describe("a swap of a token outside the bundled list", () => {
state.trackedTokens = [ state.trackedTokens = [
{ address: NOVEL, symbol: "NOVEL", decimals: 6 }, { address: NOVEL, symbol: "NOVEL", decimals: 6 },
]; ];
expect(swapDetail(data(), "Amount").value).toBe("1000.0000"); // The tracked entry names the token as well as scaling it (issue #323).
expect(swapDetail(data(), "Amount").value).toBe("1000.0000 NOVEL");
}); });
test("shows the true quantity from the explorer's decimals", () => { test("shows the true quantity from the explorer's decimals", () => {
state.wallets = walletsHolding(NOVEL, "6"); state.wallets = walletsHolding(NOVEL, "6");
expect(swapDetail(data(), "Amount").value).toBe("1000.0000"); expect(swapDetail(data(), "Amount").value).toBe("1000.0000 NOVEL");
}); });
test("refuses to format when nothing knows the scale", () => { test("refuses to format when nothing knows the scale", () => {
@@ -140,7 +141,7 @@ describe("the Min. received line takes the same rule", () => {
{ address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 }, { address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 },
]; ];
const data = swapData(WETH, HALF_WETH, NOVEL_OUT, THOUSAND_AT_SIX); const data = swapData(WETH, HALF_WETH, NOVEL_OUT, THOUSAND_AT_SIX);
expect(swapDetail(data, "Min. received").value).toBe("1000.0000"); expect(swapDetail(data, "Min. received").value).toBe("1000.0000 NOVEL");
}); });
}); });