Compare commits

...
5 Commits
Author SHA1 Message Date
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 888 additions and 78 deletions
+3 -1
View File
@@ -64,7 +64,9 @@ release/SHA256SUMS
```
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
level fixed — so two builds of one commit produce byte-identical files and the
+62
View File
@@ -45,6 +45,28 @@ but the review is broader than any of them.
# 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,
and the committed-key guard matches by content
([#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
broadcasting behind them: each is declined and the dApp still receives its
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
nothing saying it is a separate one
@@ -81,6 +113,36 @@ but the review is broader than any of them.
[#312](https://git.eeqj.de/sneak/AutistMask/issues/312) by promising no
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-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
+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");
const { getPrice, formatUsd } = require("../../shared/prices");
const { ERC20_ABI } = require("../../shared/constants");
const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList");
const {
resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount,
} = require("../../shared/approvalAmount");
// 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) {
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase());
return t ? t.symbol : null;
return resolveTokenSymbol(address, {
trackedTokens: state.trackedTokens,
wallets: state.wallets,
});
}
// Try to decode calldata using known ABIs.
@@ -85,8 +91,7 @@ function decodeCalldata(data, toAddress) {
try {
const parsed = erc20Iface.parseTransaction({ data });
if (parsed) {
const token = TOKEN_BY_ADDRESS.get(toAddress.toLowerCase());
const tokenSymbol = token ? token.symbol : null;
const tokenSymbol = resolveTokenSymbol(toAddress, decimalsSources);
// null when no source knows this token's scale. It is not
// defaulted to 18: an amount formatted with a guessed scale is
// the wrong number, and for a token with fewer decimals than the
@@ -242,8 +247,11 @@ function showTxApproval(details) {
const approvedTx = details.approvedTx;
const toAddr = approvedTx.to;
const token = toAddr ? TOKEN_BY_ADDRESS.get(toAddr.toLowerCase()) : null;
const ethValue = formatEther(approvedTx.value || "0");
const sources = {
trackedTokens: state.trackedTokens,
wallets: state.wallets,
};
// Build txInfo for status screens
pendingTxDetails = {
@@ -251,14 +259,17 @@ function showTxApproval(details) {
to: toAddr || "",
amount: formatTxValue(ethValue),
token: "ETH",
tokenSymbol: token ? token.symbol : null,
tokenSymbol: null,
};
// If this is an ERC-20 call, try to extract the real recipient and amount
const decoded = decodeCalldata(approvedTx.data, toAddr || "");
if (decoded && decoded.details) {
let decodedTokenAddr = null;
let decodedTokenSymbol = null;
// The asset the status summary is counted in: an ERC-20 call's Token
// 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) {
if (d.label === "Recipient" && d.address) {
pendingTxDetails.to = d.address;
@@ -266,20 +277,20 @@ function showTxApproval(details) {
if (d.label === "Amount") {
pendingTxDetails.amount = d.rawValue || d.value;
}
if (d.label === "Token In" && d.isToken && d.address) {
const t = TOKEN_BY_ADDRESS.get(d.address.toLowerCase());
if (t) {
decodedTokenAddr = d.address;
decodedTokenSymbol = t.symbol;
}
if (
(d.label === "Token" || d.label === "Token In") &&
d.isToken &&
d.address
) {
assetAddr = d.address;
}
}
if (token) {
pendingTxDetails.token = toAddr;
pendingTxDetails.tokenSymbol = token.symbol;
} else if (decodedTokenAddr) {
pendingTxDetails.token = decodedTokenAddr;
pendingTxDetails.tokenSymbol = decodedTokenSymbol;
if (assetAddr) {
pendingTxDetails.token = assetAddr;
pendingTxDetails.tokenSymbol = resolveTokenSymbol(
assetAddr,
sources,
);
}
}
+51 -24
View File
@@ -30,6 +30,7 @@ const {
displayedDecimals,
transferAmountUnits,
} = require("../../shared/transferAmount");
const { assertWithinCeilings } = require("../../shared/approvalVerify");
const {
CODES,
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) {
try {
const provider = getProvider(state.rpcUrl, state.networkId);
@@ -467,29 +508,7 @@ function init(_ctx) {
const provider = getProvider(state.rpcUrl, state.networkId);
const connectedSigner = signer.connect(provider);
if (pendingTx.token === "ETH") {
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);
}
tx = await populateVerifyAndSend(connectedSigner, pendingTx);
// Best-effort: clear decrypted secret after use.
// Note: JS strings are immutable; this nulls the reference but
@@ -498,6 +517,14 @@ function init(_ctx) {
txStatus.showWait(pendingTx, tx.hash);
} catch (e) {
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;
txStatus.showError(pendingTx, hash, e.shortMessage || e.message);
} 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 —
// but it is wiped on leave for the neighbouring reason: a typed
// confirmation left standing in a hidden view is one click away from
// destroying a wallet the user has since navigated off. The button is
// re-enabled here too, so a screen left mid-delete is usable on re-entry.
// destroying a wallet the user has since navigated off.
function clearLostPassword() {
lostPasswordIndex = null;
$("delete-wallet-lost-name-input").value = "";
$("delete-wallet-lost-flash").textContent = "";
$("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) {
@@ -98,6 +94,17 @@ function showLostPassword() {
// cleanup and the accountsChanged broadcast cannot drift apart between
// them.
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);
deleteWalletIndex = null;
@@ -187,8 +194,8 @@ function init(_ctx) {
btn.disabled = true;
btn.classList.add("text-muted");
// finishDelete() navigates, and the leave hook re-enables the
// button and wipes the typed name on the way out.
// finishDelete() re-enables the button; navigating away then runs
// the leave hook that wipes the typed name.
await finishDelete(lostPasswordIndex);
});
+6 -1
View File
@@ -12,6 +12,7 @@
// 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
// it from here.
const { DEBUG } = require("../../shared/constants");
const { escapeHtml } = require("../../shared/html");
const { isDebug } = require("../../shared/log");
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;";
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) {
banner.textContent = "DEBUG / INSECURE [TESTNET]" + suffix;
} else if (net.isTestnet) {
+9 -3
View File
@@ -13,7 +13,7 @@ const {
displaySymbol,
clearViewStack,
} = require("./helpers");
const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList");
const { resolveTokenSymbol } = require("../../shared/approvalAmount");
const { state } = require("../../shared/state");
const { getProvider } = require("../../shared/balances");
const { log } = require("../../shared/log");
@@ -232,9 +232,15 @@ function showSuccess(txInfo, txHash, blockNumber) {
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) {
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase());
return t ? t.symbol : null;
return resolveTokenSymbol(address, {
trackedTokens: state.trackedTokens,
wallets: state.wallets,
});
}
function decodedDetailsHtml(decoded) {
+55
View File
@@ -30,6 +30,7 @@
// enumerated rather than coerced.
const { toDecimals } = require("./transferAmount");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Every decimals the explorer reported for this contract, across all the
// 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);
}
// 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
// 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
@@ -84,5 +138,6 @@ function unknownDecimalsAmount(rawAmount) {
module.exports = {
resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount,
};
+35 -2
View File
@@ -51,6 +51,7 @@
const {
Transaction,
accessListify,
formatEther,
getAddress,
getBytes,
verifyMessage,
@@ -134,10 +135,19 @@ const FORBIDDEN_FIELDS = [
const MAX_GAS_LIMIT = 100000000n;
// 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
// hand the validator the balance.
// supported network has produced.
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
// 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
@@ -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.
@@ -775,4 +807,5 @@ module.exports = {
TX_STAGE_NONCE,
MAX_GAS_LIMIT,
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.
const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { truncateAmountNeverZero } = require("./amountDisplay");
const {
resolveTokenDecimals,
resolveTokenSymbol,
unknownDecimalsAmount,
} = require("./approvalAmount");
@@ -123,9 +123,8 @@ function tokenInfo(address, sources) {
if (address === "0x0000000000000000000000000000000000000000") {
return { symbol: "ETH", decimals: 18, address: null };
}
const t = TOKEN_BY_ADDRESS.get(address.toLowerCase());
return {
symbol: t ? t.symbol : null,
symbol: resolveTokenSymbol(address, sources),
decimals: resolveTokenDecimals(address, sources),
address,
};
+5 -3
View File
@@ -143,16 +143,18 @@ describe("decodeCalldata amount", () => {
state.trackedTokens = [
{ 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(
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", () => {
state.wallets = walletsHolding(NOVEL_TOKEN, "6");
expect(
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", () => {
@@ -172,7 +174,7 @@ describe("decodeCalldata amount", () => {
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
];
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/);
});
// 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
// came from, in a sentence.
test("reports a failed estimate as a full sentence", async () => {
+114
View File
@@ -28,6 +28,7 @@ const {
TX_STAGE_NONCE,
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
MAX_TOTAL_FEE,
} = require("../src/shared/approvalVerify");
const { prepareApprovalTx } = require("../src/shared/approvalTx");
const { getSignerForAddress } = require("../src/shared/wallet");
@@ -475,18 +476,131 @@ describe("verifySignedTx field comparison", () => {
assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }),
).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(() =>
assertWithinCeilings({
gasLimit: MAX_GAS_LIMIT,
maxFeePerGas: 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();
// 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.
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 () => {
const raw = await signedWith({ nonce: 8 });
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");
}
// 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
// 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.
test("the confirm button is re-enabled on the way out", async () => {
const { helpers, deleteWallet } = load();
// Both routes now re-enable through finishDelete(), not their leave
// hooks, so the button comes back live once a delete completes.
test("the confirm button is re-enabled after a delete", async () => {
const { deleteWallet } = load();
await openLostPassword(deleteWallet, 1);
node("btn-delete-wallet-lost-confirm").disabled = true;
helpers.showView("settings");
node("delete-wallet-lost-name-input").value = "Wallet 2";
await click("btn-delete-wallet-lost-confirm");
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
@@ -475,3 +490,48 @@ describe("what the screen leaves behind", () => {
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 () => {
await fetchOnto([row({ decimals: "6" })]);
// The same explorer entry now also names the token (issue #323).
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" })]);
expect(
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 = {
trackedTokens: [
{ 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(
"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 = [
{ 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", () => {
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", () => {
@@ -140,7 +141,7 @@ describe("the Min. received line takes the same rule", () => {
{ address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 },
];
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");
});
});