Compare commits
5
Commits
3b94a17d35
..
next
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
598de3ff1a | ||
|
|
ae61792aee | ||
|
|
a1f082d686 | ||
|
|
33fa25adca | ||
|
|
2fe6447625 |
@@ -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
|
||||
|
||||
@@ -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,26 @@ 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-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
|
||||
|
||||
@@ -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`.
|
||||
@@ -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 };
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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]");
|
||||
});
|
||||
});
|
||||
@@ -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",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user