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
12 changed files with 663 additions and 43 deletions
+47 -4
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
@@ -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
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` —
the exception only fires where the entire displayed figure would read as zero. A
genuine zero still renders `0.0000`, and truncation stays truncation: `0.99999`
shows as `0.9999`, never rounded up.
the exception only fires where the entire displayed figure would read as zero.
Truncation stays truncation: `0.99999` shows as `0.9999`, never rounded up. A
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
`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
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
Prices are fetched for the top 25 tokens only, so an address can hold assets the
+67
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
@@ -91,6 +123,41 @@ but the review is broader than any of them.
`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
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`.
+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) {
+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,
};
+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",
]);
});
});