Compare commits

...

3 Commits

Author SHA1 Message Date
578a3bc862 fix: never render a nonzero approval amount as zero (closes #322)
All checks were successful
check / check (push) Successful in 31s
e2e / e2e-chrome (push) Successful in 1m11s
e2e / e2e-firefox (push) Successful in 24s
Amounts are truncated to four decimal places per README.md's Display
Consistency rule. With the token's true scale resolved, an amount below 0.0001
still printed as 0.0000 — 1 base unit of an 18-decimal token, 500 base units of
an 8-decimal one. On the dApp approval screen, and on the wait/success/error
screens that carry the same string forward as txInfo.amount, a real transfer,
allowance or swap was therefore stated as nothing. A swap's "Min. received" is
the sharper case: a slippage floor shown as 0.0000 states that the swap may
return nothing.

That truncation existed in three separate copies — src/popup/views/approval.js
(the ERC-20 amount, the ETH value, the max fee), src/shared/uniswap.js (the
swap's Amount and Min. received lines, on that same screen) and
src/shared/transactions.js (history and balance lists). They now share
src/shared/amountDisplay.js, which holds the rule and its one exception side by
side: truncateAmount() truncates, truncateAmountNeverZero() truncates with the
nonzero floor.

approval.js and uniswap.js take the floored function, so every amount those
screens display or hand to the confirmation screens obeys the invariant. When
the truncated string would carry no digit from 1 to 9 and the value does, the
amount is extended to its first significant digit: 0.000000000000000001 DAI,
not 0.0000 DAI. It stays in token units, the same unit as the symbol beside it;
the base-unit rendering already on this screen means "the scale is unknown", and
reusing it for a known scale would blur the two. A genuine zero still renders
0.0000, an amount at or above the floor is untouched, and truncation stays
truncation — 0.99999 shows as 0.9999, never rounded up.

transactions.js takes the unfloored function, keeping its current behaviour
exactly: balance lists and history are out of scope by the issue's definition of
done, and the transaction detail view already shows exact precision there. The
code path is shared; the policy is not.

tests/approvalDisplayFloor.test.js drives decodeCalldata() and uniswap.decode()
and asserts both the displayed line and the rawValue the confirmation screens
carry. Against this tree with src/shared/uniswap.js reverted to its previous
formatAmount(), its two new swap cases fail: a 50-base-unit USDT input gives
"0.0000 USDT" where "0.00005 USDT" is expected, and a 1-wei amountOutMin gives
"0.0000 WETH" where "0.000000000000000001 WETH" is expected. A case pinning the
list rule as unfloored is included so the shared module cannot drift into one
policy.

make check green: 42 suites, 848 tests; verify-build 39 cases; check-censored
153 files; eslint and prettier clean in the pinned container.
2026-08-23 13:40:10 +00:00
12b0c4d1c6 build: remove dist/ when a release build fails (closes #333)
Some checks failed
check / check (push) Successful in 30s
e2e / e2e-chrome (push) Has been cancelled
e2e / e2e-firefox (push) Has been cancelled
A failed release build no longer leaves a complete, loadable debug bundle in dist/ whose every wallet uses the publicly committed test recovery phrase. Each step of the release build runs through script/discard-dist-on-failure, which removes dist/ on failure, says on stderr that it did and why, and returns the step's own status. build-debug is deliberately unwrapped. script/verify-build is untouched.
2026-08-23 15:39:04 +02:00
c36d8b6ddf docs: state the enforced dist/ verification scope precisely (closes #331)
All checks were successful
check / check (push) Successful in 29s
e2e / e2e-chrome (push) Successful in 1m11s
e2e / e2e-firefox (push) Successful in 22s
README, the script synopsis, its header paragraph and the check_dist_tree comment now all say the same thing: regular files and symlinks under dist/ are covered; fifos, sockets, device nodes and empty directories are not, and why. No behaviour change — the walk is untouched.
2026-08-23 15:33:58 +02:00
11 changed files with 671 additions and 76 deletions

View File

@@ -60,19 +60,31 @@ hooks:
# scrubbed from the build itself: with AUTISTMASK_DEBUG=1 exported, this target
# compiles a debug bundle and then fails on it, loudly, rather than quietly
# handing back something other than the release build that was asked for.
#
# Every step of this target is wrapped in script/discard-dist-on-failure, so a
# release build that fails removes dist/ instead of leaving a complete, loadable
# debug bundle there for whoever runs the build, sees it fail, and loads
# dist/chrome/ anyway. A step that succeeds removes nothing, and build-debug is
# deliberately not wrapped.
build:
@echo "Building extension..."
@set -eu; \
receipt="$$(mktemp "$${TMPDIR:-/tmp}/autistmask-build-receipt.XXXXXX")"; \
trap 'rm -f "$$receipt"' EXIT INT TERM; \
AUTISTMASK_BUILD_RECEIPT="$$receipt" yarn run build 2>&1; \
env -u AUTISTMASK_DEBUG script/verify-build --expect release \
script/discard-dist-on-failure \
env AUTISTMASK_BUILD_RECEIPT="$$receipt" yarn run build 2>&1; \
script/discard-dist-on-failure \
env -u AUTISTMASK_DEBUG script/verify-build --expect release \
--receipt "$$receipt"
@script/check-censored --require-dist
@script/discard-dist-on-failure script/check-censored --require-dist
# Development-only build: enables the red DEBUG / INSECURE banner and makes
# the hardcoded test recovery phrase the output of wallet creation. Never
# distribute the artifacts this produces.
#
# No discard-dist-on-failure here, on purpose: a debug build that fails is not
# producing an artifact anyone could mistake for a release one, and its dist/ is
# the evidence of what went wrong.
build-debug:
@echo "Building extension (DEBUG)..."
@set -eu; \

View File

@@ -63,8 +63,12 @@ that runs `make build`, that target compiles a debug bundle and then **fails**,
because it tells `script/verify-build` in so many words that it was supposed to
produce a release build. It used to be that the verifier read the same variable
out of its own environment, agreed with itself, and reported a debug artifact as
verified. The build prints which mode it used. See the
[DEBUG Mode Policy](#debug-mode-policy) for what the flag changes. **Never
verified. The build prints which mode it used. A release build that fails also
**removes `dist/`**, and says so: the bundle it had already written is loadable,
and a loud failure is no protection against someone loading `dist/chrome/`
anyway. `make build-debug` keeps its `dist/` on failure — that output is not
mistakable for a release build, and it is the evidence of what went wrong. See
the [DEBUG Mode Policy](#debug-mode-policy) for what the flag changes. **Never
distribute a debug build** — every wallet it creates gets the same publicly
known test recovery phrase.
@@ -81,17 +85,21 @@ lives.
one of the bundles containing `src/shared/constants.js` — into a build receipt,
and `script/verify-build` checks `dist/` against that receipt: every recorded
file present with exactly the recorded bytes, every audited bundle carrying the
requested `DEBUG` marker, and nothing under `dist/` that the build did not
write. The `Makefile` creates the receipt path with `mktemp` per invocation,
outside the repo, and deletes it afterwards.
requested `DEBUG` marker, and no regular file or symlink under `dist/` that the
build did not write. The `Makefile` creates the receipt path with `mktemp` per
invocation, outside the repo, and deletes it afterwards.
That is what ties the check to a build rather than to a directory. What it
establishes is narrow and worth stating exactly: `dist/` is byte for byte the
output of the `build.js` run that just finished, with nothing added, removed or
altered in between. It establishes nothing about whether the source tree or
`build.js` were honest, and it offers nothing to someone handed a `dist/` from
elsewhere — without the receipt from its own build there is no input to the
check. Verifiable provenance for a third party is signing, which this is not.
output of the `build.js` run that just finished, with no regular file or symlink
added, removed or altered in between. Regular files and symlinks are the whole
of what the tree walk covers; fifos, sockets, device nodes and empty directories
under `dist/` are not checked, because a build emits none of them, none can
carry a shippable payload, and `grep` on a fifo would hang rather than fail. It
establishes nothing about whether the source tree or `build.js` were honest, and
it offers nothing to someone handed a `dist/` from elsewhere — without the
receipt from its own build there is no input to the check. Verifiable provenance
for a third party is signing, which this is not.
There is deliberately no target that re-verifies an existing `dist/` on its own.
The list of files to check has to come from the build that produced them; read
@@ -143,26 +151,35 @@ provide:
serves. Run deliberately, never as part of a build: the output is committed
and there is no runtime fetch, so the shipped list is as fresh as the last
vendoring run that was released
- `script/verify-build --expect release|debug --receipt PATH` — assert that
`dist/` is exactly what the build that just ran emitted, and that the compiled
`DEBUG` state of the bundles in it is the one that was asked for. Both
arguments are required and neither has a default: the expected mode is stated
by the caller rather than read from `AUTISTMASK_DEBUG`, and the file list
comes from the build's receipt rather than from `dist/` (see
- `script/verify-build --expect release|debug --receipt PATH` — assert that the
regular files and symlinks under `dist/` are exactly what the build that just
ran emitted (other file types are out of scope), and that the compiled `DEBUG`
state of the bundles in it is the one that was asked for. Both arguments are
required and neither has a default: the expected mode is stated by the caller
rather than read from `AUTISTMASK_DEBUG`, and the file list comes from the
build's receipt rather than from `dist/` (see
[Build Receipts](#build-receipts)). Run automatically at the end of
`make build` and `make build-debug`; fails loudly rather than passing whenever
it cannot determine something. Not part of `make check`, which does not depend
on build artifacts existing.
- `script/discard-dist-on-failure COMMAND [ARG...]` — run one step of the
**release** build and, if it fails, remove `dist/` before returning that
step's exit status, saying on stderr that it did and why. Every step of
`make build` runs through it; `make build-debug` runs none of them through it.
A step that succeeds removes nothing, and a removal that cannot be completed
is reported as loudly as one that was
- `script/test-verify-build` — exercise every failure mode of
`script/verify-build` against a fixture tree in a temp dir, asserting the exit
status and the message of each, and read the `make build` and
status and the message of each, assert the state of `dist/` on disk after a
failing and a succeeding release build step, and read the `make build` and
`make build-debug` recipes back out of `make -n` to check that they pass the
mode as an argument on a scrubbed environment. Part of `make check`; it reads
no build artifacts and writes nothing under `dist/`. The cases that depend on
file permissions cannot mean anything for a process that is not subject to
them, so the harness proves its runner against a mode-000 file before counting
them, dropping to an unprivileged user when run as root; if it cannot, it
skips those cases and says so in a banner rather than passing them.
mode as an argument on a scrubbed environment and wrap only the release path.
Part of `make check`; it reads no build artifacts and writes nothing under
`dist/`. The cases that depend on file permissions cannot mean anything for a
process that is not subject to them, so the harness proves its runner against
a mode-000 file before counting them, dropping to an unprivileged user when
run as root; if it cannot, it skips those cases and says so in a banner rather
than passing them.
- `script/docker` — build the Docker image tagged via `script/projectname`
- `script/cibuild` — CI entrypoint: plain `docker build .`
- `script/precommit` — run by the git pre-commit hook; runs `script/check`
@@ -176,9 +193,11 @@ The Makefile shims to those. It also carries a few targets that have no
silently rewritten. Use `make setup` for a fresh clone.
- `make hooks` — shims to `script/install-precommit`
- `make build` — build the extension into `dist/chrome/` and `dist/firefox/`,
then verify the result against the build's receipt as a release build
then verify the result against the build's receipt as a release build. A
failure at any step removes `dist/`
- `make build-debug` — the same build with `AUTISTMASK_DEBUG=1`, verified as a
debug build (see [Debug Builds](#debug-builds))
debug build, and keeping its `dist/` on failure (see
[Debug Builds](#debug-builds))
- `make clean` — remove `dist/`
- `make dev` — build in watch mode
@@ -689,6 +708,32 @@ Both are click-copyable. Truncating to 4 decimals in summary views is acceptable
for scannability, but the detail view must never discard precision — it is the
one place the user can always use to verify exact details.
**Specific Exception — nonzero floor on the approval screens:** A nonzero amount
must never render as zero. Truncating to 4 decimals does exactly that to an
amount below 0.0001 — 1 base unit of an 18-decimal token, 500 base units of an
8-decimal one — and on the dApp approval screen and the wait/success/error
screens that carry its amount forward, a real transfer or allowance then reads
as "nothing is being moved". A swap's `Min. received` is the sharper case: a
slippage floor shown as `0.0000` states that the swap may return nothing.
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 rule and its exception live in `src/shared/amountDisplay.js` as
`truncateAmount()` and `truncateAmountNeverZero()`. Everything the approval and
confirmation screens display goes through the floored one — the ERC-20 amount,
the ETH value and max fee (`src/popup/views/approval.js`), and the swap's
`Amount` and `Min. received` lines (`src/shared/uniswap.js`). The history and
balance lists (`src/shared/transactions.js`) use the unfloored one: the
transaction detail view is the authoritative record and already shows exact
precision. The 4-decimal rule is unchanged everywhere else, including for
amounts at or above the floor on the approval screens.
#### Partial USD totals
Prices are fetched for the top 25 tokens only, so an address can hold assets the

42
TODO.md
View File

@@ -26,7 +26,8 @@ milestone is in flight on `next`; its `next` -> `main` PR is
[#190](https://git.eeqj.de/sneak/AutistMask/pulls/190). `make check` verified
green on `next` at `e9fa8be` on 2026-08-10, and `make build` produces
`dist/chrome/` and `dist/firefox/`, verified against the build's own receipt to
be exactly what that build emitted with `DEBUG` compiled off.
hold exactly the regular files and symlinks that build emitted, with `DEBUG`
compiled off.
The backlog lives on the
[Gitea tracker](https://git.eeqj.de/sneak/AutistMask/issues), which is
@@ -44,6 +45,45 @@ but the review is broader than any of them.
# Completed Steps
- 2026-08-23: A failed release build no longer leaves a loadable debug bundle in
`dist/` ([#333](https://git.eeqj.de/sneak/AutistMask/issues/333)). With
`AUTISTMASK_DEBUG=1` exported, `make build` compiled a debug bundle and failed
on it in `script/verify-build` — but the bundle stayed on disk, loadable, with
every wallet it creates using the publicly committed test recovery phrase.
Every step of `make build` now runs through `script/discard-dist-on-failure`,
which removes `dist/` when a step fails and says on stderr that it did and
why; a removal it cannot complete is reported just as loudly.
`make build-debug` is deliberately not wrapped: its output is not mistakable
for a release build and is the evidence of the failure.
`script/test-verify-build` asserts the state of `dist/` on disk after a
failing and a succeeding step, not just the exit status, and reads `make -n`
to check the wrapper is on the release path and only there.
- 2026-08-23: `README.md` and `script/verify-build`'s own comments now state the
emitted-tree guarantee at the width the code actually enforces
([#331](https://git.eeqj.de/sneak/AutistMask/issues/331)). The tree walk is
`-type f -o -type l`, so the guarantee covers regular files and symlinks under
`dist/`; fifos, sockets, device nodes and empty directories are not checked,
because a build emits none of them, none can carry a shippable payload, and
`grep` on a fifo would hang rather than fail. The exclusion is deliberate and
unchanged — the README said "nothing under `dist/` that the build did not
write", which was broader than that. Documentation only; no executable line
changed.
- 2026-08-23: An amount below the 4-decimal display floor no longer reads as
zero on the approval screens
([#322](https://git.eeqj.de/sneak/AutistMask/issues/322)). With the token's
true scale resolved, the 4-decimal truncation still printed a small amount as
`0.0000` — 1 base unit of an 18-decimal token, 500 of an 8-decimal one — so a
real transfer, allowance or swap was stated as nothing on the one screen whose
job is to say what is being authorized, and a swap's `Min. received` claimed
the user might receive nothing. Three copies of that truncation existed; they
now share `src/shared/amountDisplay.js`. Everything the approval and
confirmation screens render (`src/popup/views/approval.js`,
`src/shared/uniswap.js`) extends to the first significant digit when the
truncated figure would otherwise read as zero, keeping the amount in token
units rather than switching to base units mid-line. The history and balance
lists (`src/shared/transactions.js`) keep the unfloored rule, which is out of
scope by the issue's definition of done. `README.md`'s Display Consistency
section records the exception.
- 2026-08-20: A second extension page can no longer silently delete a wallet
([#304](https://git.eeqj.de/sneak/AutistMask/issues/304)). `saveState()` wrote
the entire state blob, and every extension page — the toolbar popup, a dApp

78
script/discard-dist-on-failure Executable file
View File

@@ -0,0 +1,78 @@
#!/bin/sh
# script/discard-dist-on-failure: run one step of the RELEASE build, and if that
# step fails, remove dist/ before returning its exit status. Our own extension
# to scripts-to-rule-them-all, wrapped around every step of make build.
#
# Why: with AUTISTMASK_DEBUG=1 exported in the calling shell, make build
# compiles a debug bundle and then fails on it in script/verify-build — but the
# bundle is already written. It is loadable, and every wallet it creates gets
# the publicly committed test recovery phrase from src/shared/constants.js. A
# failed release build that leaves that behind is a smaller version of the trap
# the verifier exists to close, and "the failure was loud" only works on an
# operator who does not load dist/chrome/ anyway. Removing the artifact does not
# depend on that.
#
# Two things this deliberately does not do. It does not wrap make build-debug: a
# debug build that failed is not a mistakable artifact, and its output is the
# evidence of what went wrong. And it never removes anything on a step that
# SUCCEEDS, including the final check-censored --require-dist pass.
#
# The removal is never silent: it says dist/ is gone and why, on stderr, above
# the build's own failure.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
DIST="$ROOT/dist"
usage() {
echo "usage: discard-dist-on-failure COMMAND [ARG...]" >&2
}
# Remove dist/, and say so. A removal that could not be completed is reported as
# loudly as one that was: the artifact is still on disk, and reporting nothing
# would leave the operator believing it is not.
discard_dist() {
if [ ! -e "$DIST" ] && [ ! -h "$DIST" ]; then
echo "discard-dist-on-failure: the release build failed. There was no" \
"dist/ to remove." >&2
return 0
fi
rm -rf "$DIST" || true
if [ -e "$DIST" ] || [ -h "$DIST" ]; then
echo "discard-dist-on-failure: the release build failed and dist/" \
"COULD NOT BE REMOVED, so it is still on disk. Do not load it:" \
"a release build that failed may hold a complete debug bundle," \
"whose wallets all use the publicly committed test recovery" \
"phrase. Remove it by hand (make clean)." >&2
return 0
fi
echo "discard-dist-on-failure: the release build failed, so dist/ WAS" \
"REMOVED and no longer exists. A release build that fails has often" \
"already emitted a complete, loadable debug bundle — every wallet it" \
"creates gets the publicly committed test recovery phrase — so the" \
"failed build is not left behind to be loaded. Fix the failure and" \
"re-run make build, or run make build-debug if a debug build is what" \
"was wanted; that target keeps its output." >&2
}
main() {
[ "$#" -ge 1 ] || {
usage
echo "discard-dist-on-failure: no command given, so no build step ran" \
"and nothing was removed." >&2
exit 1
}
_status=0
"$@" || _status=$?
[ "$_status" -ne 0 ] || return 0
discard_dist
exit "$_status"
}
main "$@"

View File

@@ -1,7 +1,8 @@
#!/bin/sh
# script/test-verify-build: exercise every failure mode of
# script/verify-build. Our own extension to scripts-to-rule-them-all, run
# from script/check so make check covers it.
# script/verify-build, and what make build does with dist/ after one of them
# (script/discard-dist-on-failure). Our own extension to
# scripts-to-rule-them-all, run from script/check so make check covers it.
#
# Why this exists: verify-build is the build-integrity guard, and four separate
# reviews of it each found a fresh vacuous pass — the grep exit-2 conflation,
@@ -31,6 +32,7 @@ set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
VERIFY_BUILD="$ROOT/script/verify-build"
DISCARD_DIST="$ROOT/script/discard-dist-on-failure"
MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off"
@@ -150,6 +152,7 @@ build_fixture() {
mkdir -p "$FIXTURE/script"
ln -s "$VERIFY_BUILD" "$FIXTURE/script/verify-build"
ln -s "$DISCARD_DIST" "$FIXTURE/script/discard-dist-on-failure"
mkdir -p "$FIXTURE/dist/chrome/src/popup" \
"$FIXTURE/dist/chrome/src/content" \
@@ -513,12 +516,125 @@ c_debug_build() {
write_receipt
}
c_no_dist() { rm -rf dist; }
# --- dist discard -----------------------------------------------------------
#
# make build wraps every step of the release path in
# script/discard-dist-on-failure, so a release build that fails removes dist/:
# with AUTISTMASK_DEBUG=1 exported it has already emitted a complete, loadable
# debug bundle whose every wallet uses the publicly committed test recovery
# phrase, and a loud failure alone does not stop someone loading dist/chrome/
# anyway. make build-debug is deliberately not wrapped.
#
# Both directions are asserted against the state of dist/ ON DISK after the run,
# not against the exit status: a case reading only the status would keep passing
# if the removal quietly stopped happening, which is the flip this exists to
# catch. The wrapper runs against the fixture — its ROOT is the fixture, via the
# symlink in the fixture's script/ — with trivial commands standing in for the
# build steps, because what is under test is what happens after a step says no,
# not the step.
# discard_case <name> <setup> <status> <gone|kept> <want> <unwanted> [cmd...]
discard_case() {
_dc_name="$1"
_dc_setup="$2"
_dc_want_status="$3"
_dc_want_dist="$4"
_dc_want="$5"
_dc_unwanted="$6"
shift 6
build_fixture
if ! (cd "$FIXTURE" && "$_dc_setup") >/dev/null 2>&1; then
FAILED=$((FAILED + 1))
echo " FAIL: $_dc_name"
echo " the case's own setup failed, so nothing was tested."
return 0
fi
_dc_status=0
_dc_out="$(cd "$FIXTURE" &&
"$FIXTURE/script/discard-dist-on-failure" "$@" 2>&1)" || _dc_status=$?
_ok=yes
_why=""
if [ "$_dc_status" -ne "$_dc_want_status" ]; then
_ok=no
_why="exit status $_dc_status, wanted $_dc_want_status"
fi
# The assertion this case exists for: what is on disk now.
if [ -e "$FIXTURE/dist" ] || [ -h "$FIXTURE/dist" ]; then
_dc_dist=kept
else
_dc_dist=gone
fi
if [ "$_dc_dist" != "$_dc_want_dist" ]; then
_ok=no
_why="${_why:+$_why; }dist/ is $_dc_dist after the run, wanted"
_why="$_why $_dc_want_dist"
elif [ "$_dc_want_dist" = kept ] &&
[ ! -f "$FIXTURE/dist/chrome/src/popup/index.js" ]; then
# Kept has to mean intact: a dist/ emptied out is not one left alone.
_ok=no
_why="${_why:+$_why; }dist/ survived but its emitted bundle did not"
fi
_dc_check_message "$_dc_want" want
_dc_check_message "$_dc_unwanted" unwanted
if [ "$_ok" = yes ]; then
PASSED=$((PASSED + 1))
echo " ok: $_dc_name"
return 0
fi
FAILED=$((FAILED + 1))
echo " FAIL: $_dc_name"
echo " $_why"
echo " --- discard-dist-on-failure output ---"
printf '%s\n' "$_dc_out" | sed 's/^/ /'
echo " --- end output ---"
}
# Require ($2 = want) or forbid ($2 = unwanted) a substring in the wrapper's
# output, updating _ok and _why. An empty substring asserts nothing. Same grep
# discipline as everywhere else here: 0 and 1 are answers, anything else means
# the message was never checked.
_dc_check_message() {
[ -n "$1" ] || return 0
_dcm_g=0
printf '%s\n' "$_dc_out" | grep -q -F -e "$1" || _dcm_g=$?
case "$_dcm_g" in
0)
[ "$2" = unwanted ] || return 0
_ok=no
_why="${_why:+$_why; }message contained: $1"
;;
1)
[ "$2" = want ] || return 0
_ok=no
_why="${_why:+$_why; }message did not contain: $1"
;;
*)
_ok=no
_why="${_why:+$_why; }grep exited $_dcm_g matching the message, so the
message was never checked"
;;
esac
}
# --- Makefile wiring --------------------------------------------------------
# The verifier cases above prove what verify-build does when it is told what to
# expect. This proves the Makefile tells it — with the mode as an argument, on
# a scrubbed environment, and identically whether or not AUTISTMASK_DEBUG is
# exported in the shell that ran make. Read off `make -n`, so no build runs.
# expect, and the discard cases prove what the wrapper does with dist/. This
# proves the Makefile wires both up — the mode as an argument, on a scrubbed
# environment, identically whether or not AUTISTMASK_DEBUG is exported in the
# shell that ran make, and the wrapper on the release path only. Read off
# `make -n`, so no build runs.
check_makefile_wiring() {
if ! command -v make >/dev/null 2>&1; then
SKIPPED=$((SKIPPED + 1))
@@ -537,6 +653,34 @@ check_makefile_wiring() {
build-debug "verify-build --expect debug"
_wiring_case "make build-debug scrubs AUTISTMASK_DEBUG for the verifier" \
build-debug "env -u AUTISTMASK_DEBUG"
# The release path runs its steps through the wrapper, including the final
# check-censored pass; the debug path runs none of them through it, which is
# what keeps a failed debug build's dist/ on disk.
_wiring_case "make build wraps its steps in discard-dist-on-failure" \
build "script/discard-dist-on-failure"
_wiring_case "make build wraps check-censored --require-dist too" \
build "script/discard-dist-on-failure script/check-censored"
_wiring_case_absent "make build-debug never discards its dist/" \
build-debug "discard-dist-on-failure"
}
# Run `make -n TARGET` with AUTISTMASK_DEBUG=1 exported, into _wc_out. Returns
# non-zero, having already reported the failure, when make itself failed: a
# recipe that could not be printed was never checked.
_wiring_make_n() {
AUTISTMASK_DEBUG=1
export AUTISTMASK_DEBUG
_wc_status=0
_wc_out="$(cd "$ROOT" && make -n "$_wc_target" 2>&1)" || _wc_status=$?
unset AUTISTMASK_DEBUG
[ "$_wc_status" -ne 0 ] || return 0
FAILED=$((FAILED + 1))
echo " FAIL: $_wc_name"
echo " make -n $_wc_target exited $_wc_status"
return 1
}
_wiring_case() {
@@ -544,18 +688,7 @@ _wiring_case() {
_wc_target="$2"
_wc_want="$3"
AUTISTMASK_DEBUG=1
export AUTISTMASK_DEBUG
_wc_status=0
_wc_out="$(cd "$ROOT" && make -n "$_wc_target" 2>&1)" || _wc_status=$?
unset AUTISTMASK_DEBUG
if [ "$_wc_status" -ne 0 ]; then
FAILED=$((FAILED + 1))
echo " FAIL: $_wc_name"
echo " make -n $_wc_target exited $_wc_status"
return 0
fi
_wiring_make_n || return 0
_wc_g=0
printf '%s\n' "$_wc_out" | grep -q -F -e "$_wc_want" || _wc_g=$?
@@ -577,6 +710,34 @@ _wiring_case() {
esac
}
# The inverse: the recipe must NOT run something.
_wiring_case_absent() {
_wc_name="$1"
_wc_target="$2"
_wc_want="$3"
_wiring_make_n || return 0
_wc_g=0
printf '%s\n' "$_wc_out" | grep -q -F -e "$_wc_want" || _wc_g=$?
case "$_wc_g" in
1)
PASSED=$((PASSED + 1))
echo " ok: $_wc_name"
;;
0)
FAILED=$((FAILED + 1))
echo " FAIL: $_wc_name"
echo " make -n $_wc_target runs: $_wc_want"
;;
*)
FAILED=$((FAILED + 1))
echo " FAIL: $_wc_name"
echo " grep exited $_wc_g, so the recipe was never checked"
;;
esac
}
run_cases() {
check_case "control: untouched dist passes" \
no release 0 "2 bundle(s) $MARKER_OFF" c_control
@@ -712,6 +873,18 @@ run_cases() {
no release 1 "carries a debug marker but the build did not" \
c_marker_on_plain_file
discard_case "a failed release build step removes dist/" \
c_control 3 gone "dist/ WAS REMOVED" "" sh -c 'exit 3'
discard_case "a successful release build step leaves dist/ alone" \
c_control 0 kept "" "REMOVED" true
discard_case "a failed release build step with no dist/ says there was none" \
c_no_dist 3 gone "There was no dist/ to remove" "" sh -c 'exit 3'
discard_case "the wrapper given no command removes nothing" \
c_control 1 kept "no command given" ""
check_makefile_wiring
}
@@ -740,6 +913,10 @@ main() {
echo "test-verify-build: $VERIFY_BUILD is missing or not executable" >&2
exit 1
}
[ -x "$DISCARD_DIST" ] || {
echo "test-verify-build: $DISCARD_DIST is missing or not executable" >&2
exit 1
}
echo "Testing script/verify-build failure modes..."
pick_sha256_tool

View File

@@ -1,8 +1,10 @@
#!/bin/sh
# script/verify-build: assert that dist/ holds exactly what the build that just
# ran emitted, and that the compiled DEBUG state of that output is the one the
# caller asked for. Our own extension to scripts-to-rule-them-all, run at the
# end of make build / make build-debug.
# script/verify-build: assert that the regular files and symlinks under dist/
# are exactly what the build that just ran emitted (other file types are out of
# scope; see "What that does and does not establish" below), and that the
# compiled DEBUG state of that output is the one the caller asked for. Our own
# extension to scripts-to-rule-them-all, run at the end of make build /
# make build-debug.
#
# Why the DEBUG half exists: DEBUG makes the publicly committed test recovery
# phrase the output of wallet creation, so a release artifact built with it live
@@ -29,12 +31,16 @@
# path fresh per invocation, outside the repo, and deletes it afterwards.
#
# What that does and does not establish. It establishes that dist/ is byte for
# byte the output of the build.js run that just finished, with nothing added,
# nothing missing and nothing altered in between, and that the audited bundles
# in it compiled to the requested mode. It does NOT establish that the source
# tree or build.js were honest, and it says nothing at all to someone handed a
# dist/ from elsewhere: without the receipt from its own build they have no
# input to this check. That is signing, and it is not this control.
# byte the output of the build.js run that just finished, with no regular file
# or symlink added, missing or altered in between, and that the audited bundles
# in it compiled to the requested mode. Regular files and symlinks are the whole
# of what the tree walk covers; fifos, sockets, device nodes and empty
# directories under dist/ are not checked, because a build emits none of them,
# none can carry a shippable payload, and grep on a fifo would hang rather than
# fail. It does NOT establish that the source tree or build.js were honest, and
# it says nothing at all to someone handed a dist/ from elsewhere: without the
# receipt from its own build they have no input to this check. That is signing,
# and it is not this control.
#
# It fails rather than passes whenever it cannot determine something. Minified
# output is not a stable contract, so "matched neither marker" is not evidence
@@ -392,8 +398,10 @@ check_receipt_entries() {
# its own command line, so a linked dist/ collapses this walk to one entry
# and cross-checks nothing.
#
# Types other than regular files and symlinks are left out on purpose: a build
# emits none of them, and grep on a fifo would hang rather than fail.
# Types other than regular files and symlinks — fifos, sockets, device nodes and
# empty directories — are left out on purpose, and the guarantee is bounded to
# what is walked: a build emits none of them, none can carry a shippable
# payload, and grep on a fifo would hang rather than fail.
check_dist_tree() {
LISTING="$(mktemp "${TMPDIR:-/tmp}/verify-build-dist.XXXXXX")" ||
fail "could not create a temporary file for the dist/ listing, so the

View File

@@ -25,6 +25,12 @@ const {
resolveTokenDecimals,
unknownDecimalsAmount,
} = require("../../shared/approvalAmount");
// Four decimals, with the nonzero floor these screens hold: every amount this
// view renders — the ERC-20 line, the ETH value, the max fee — and every one
// it carries forward to the wait/success/error screens goes through it.
const {
truncateAmountNeverZero: formatTxValue,
} = require("../../shared/amountDisplay");
const { decryptWithPassword } = require("../../shared/vault");
const { getSignerForAddress } = require("../../shared/wallet");
const { walletDefect } = require("../../shared/walletDefects");
@@ -40,13 +46,6 @@ function approvalAddressHtml(address) {
return renderAddressHtml(address, { title });
}
function formatTxValue(val) {
const parts = val.split(".");
if (parts.length === 1) return val + ".0000";
const dec = (parts[1] + "0000").slice(0, 4);
return parts[0] + "." + dec;
}
// The amount line for a decoded ERC-20 call. With a known scale it is the
// token quantity; with `decimals` null it is the base-unit integer with the
// unknown scale stated, because formatting it with an assumed scale is what

View File

@@ -0,0 +1,46 @@
// The 4-decimal amount rule from README.md's Display Consistency section, and
// the one exception to it, in one place. Three call sites had grown their own
// copy of the truncation — the history and balance lists
// (`src/shared/transactions.js`), the approval screen's ERC-20 amount line
// (`src/popup/views/approval.js`) and its Uniswap swap detail lines
// (`src/shared/uniswap.js`) — and a fix applied to one of them left the other
// two showing a different number for the same value.
//
// The two functions below are the two policies, not two implementations of
// one: summary lists truncate, and the screens that state what is being
// authorized truncate with a floor. Keeping them adjacent is the point, so a
// change to the rule cannot reach one screen and miss another.
// Truncate to exactly four decimal places. Truncation, never rounding: an
// amount must never be displayed as larger than it is, so 0.99999 stays
// 0.9999.
function truncateAmount(val) {
const parts = val.split(".");
if (parts.length === 1) return val + ".0000";
return parts[0] + "." + (parts[1] + "0000").slice(0, 4);
}
// The same rule, plus the invariant the approval and confirmation screens
// hold: a nonzero amount never renders as zero. Truncating to four decimals
// does exactly that to an amount below 0.0001 — one base unit of an 18-decimal
// token, 500 of an 8-decimal one — and a real transfer or allowance then reads
// as "nothing is being moved" on the screen whose whole job is to say what is
// being authorized.
//
// When the truncated string carries no significant digit and the value does,
// the amount is extended to its first significant digit instead. It stays in
// token units, the same unit as the symbol printed beside it. A genuine zero
// still renders 0.0000, and anything at or above the floor is untouched.
function truncateAmountNeverZero(val) {
const truncated = truncateAmount(val);
// Tests the whole truncated string, integer part included: 1.00005 has a
// significant digit already and stays 1.0000.
if (/[1-9]/.test(truncated)) return truncated;
const parts = val.split(".");
if (parts.length === 1) return truncated;
const sig = parts[1].search(/[1-9]/);
if (sig === -1) return truncated;
return parts[0] + "." + parts[1].slice(0, sig + 1);
}
module.exports = { truncateAmount, truncateAmountNeverZero };

View File

@@ -11,6 +11,10 @@ const { log, debugFetch } = require("./log");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { parseHoldersCount, isLowHolderCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// The plain 4-decimal rule. The history and balance lists deliberately keep
// truncation without the approval screens' nonzero floor: the transaction
// detail view is the authoritative record and already shows exact precision.
const { truncateAmount: formatTxValue } = require("./amountDisplay");
// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
// over the address, not part of its identity. Every address comparison in
@@ -20,13 +24,6 @@ function normalizeAddress(addr) {
return (addr || "").toLowerCase();
}
function formatTxValue(val) {
const parts = val.split(".");
if (parts.length === 1) return val + ".0000";
const dec = (parts[1] + "0000").slice(0, 4);
return parts[0] + "." + dec;
}
function parseTx(tx, addrLower) {
const from = tx.from?.hash || "";
const to = tx.to?.hash || "";

View File

@@ -3,6 +3,7 @@
const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { truncateAmountNeverZero } = require("./amountDisplay");
const coder = AbiCoder.defaultAbiCoder();
@@ -34,11 +35,13 @@ const COMMAND_NAMES = {
0x21: "Execute Sub-Plan",
};
// The swap's Amount and Min. received lines land on the same approval screen,
// and Amount is carried to the wait/success/error screens as the ERC-20 line
// is, so they take the same nonzero floor: a swap of an amount below 0.0001 is
// not "0.0000", and a slippage floor of one base unit does not read as "you may
// receive nothing".
function formatAmount(raw, decimals) {
const parts = formatUnits(raw, decimals).split(".");
if (parts.length === 1) return parts[0] + ".0000";
const dec = (parts[1] + "0000").slice(0, 4);
return parts[0] + "." + dec;
return truncateAmountNeverZero(formatUnits(raw, decimals));
}
function tokenInfo(address) {

View File

@@ -0,0 +1,190 @@
// The floor of the approval screen's amount line.
//
// Amounts are truncated to four decimal places for scannability (README.md,
// Display Consistency). With the token's true scale resolved, that truncation
// can still take a real amount below the floor and print it as `0.0000`: one
// base unit of an 18-decimal token, or a few hundred of an 8-decimal one. On
// the one screen whose job is to state what is being authorized, a nonzero
// transfer or allowance then reads as nothing.
//
// The invariant asserted here is narrow: a nonzero amount never renders as
// zero. The four-decimal rule itself is unchanged, and the string the
// confirmation screens carry as `txInfo.amount` is the same one, so it is
// asserted on `rawValue` alongside the displayed line.
//
// Both amount paths of that screen are covered: the ERC-20 line decoded by
// `src/popup/views/approval.js`, and the swap's `Amount` and `Min. received`
// lines decoded by `src/shared/uniswap.js`.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { AbiCoder, Interface } = require("ethers");
const { ERC20_ABI } = require("../src/shared/constants");
const { state } = require("../src/shared/state");
const { decodeCalldata } = require("../src/popup/views/approval");
const uniswap = require("../src/shared/uniswap");
const {
truncateAmount,
truncateAmountNeverZero,
} = require("../src/shared/amountDisplay");
const iface = new Interface(ERC20_ABI);
// Bundled tokens, so the scale and the symbol both come from the list.
const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; // 6 decimals
const WBT = "0x925206b8a707096Ed26ae47C84747fE0bb734F59"; // 8 decimals
const DAI = "0x6B175474E89094C44Da98b954EedeAC495271d0F"; // 18 decimals
// Outside the list, so the scale comes from what the user tracks and the line
// carries no symbol.
const NOVEL = "0xE2E0000000000000000000000000000000000E2e";
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
const SPENDER = "0x1111111111111111111111111111111111111111";
// The Uniswap swap lines land on this same approval screen.
const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af";
const USDT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; // 6 decimals
const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; // 18 decimals
const coder = AbiCoder.defaultAbiCoder();
const routerIface = new Interface([
"function execute(bytes commands, bytes[] inputs, uint256 deadline)",
]);
// A V2_SWAP_EXACT_IN (command 0x08) execute() call: `amountIn` of USDT for at
// least `amountOutMin` of WETH.
function swapData(amountIn, amountOutMin) {
const input = coder.encode(
["address", "uint256", "uint256", "address[]", "bool"],
[RECIPIENT, amountIn, amountOutMin, [USDT, WETH], true],
);
return routerIface.encodeFunctionData("execute", [
"0x08",
[input],
9999999999n,
]);
}
function swapDetail(amountIn, amountOutMin, label) {
const decoded = uniswap.decode(swapData(amountIn, amountOutMin), ROUTER);
return decoded.details.find((d) => d.label === label);
}
function transferData(amount) {
return iface.encodeFunctionData("transfer", [RECIPIENT, amount]);
}
function approveData(amount) {
return iface.encodeFunctionData("approve", [SPENDER, amount]);
}
// The Amount detail as the approval screen renders it: `value` is the line on
// the screen, `rawValue` is what is carried to the wait/success/error screens.
function amount(data, token) {
const decoded = decodeCalldata(data, token);
return decoded.details.find((d) => d.label === "Amount");
}
beforeEach(() => {
state.trackedTokens = [];
state.wallets = [];
});
describe("a nonzero amount never renders as zero", () => {
test("500 base units of a 6-decimal token", () => {
const detail = amount(transferData(500n), USDC);
expect(detail.value).toBe("0.0005 USDC");
expect(detail.rawValue).toBe("0.0005");
});
test("1 base unit of an 18-decimal token", () => {
const detail = amount(transferData(1n), DAI);
expect(detail.value).toBe("0.000000000000000001 DAI");
expect(detail.rawValue).toBe("0.000000000000000001");
});
test("500 base units of an 8-decimal token", () => {
expect(amount(transferData(500n), WBT).rawValue).toBe("0.000005");
});
test("an allowance below the floor is not rendered as zero either", () => {
expect(amount(approveData(1n), DAI).value).toBe(
"0.000000000000000001 DAI",
);
});
// The floor holds at any scale, not only the three above: for every
// decimals a token can declare, one base unit has to show a digit.
test("one base unit shows a significant digit at every scale", () => {
for (let decimals = 0; decimals <= 30; decimals++) {
state.trackedTokens = [{ address: NOVEL, decimals }];
expect(amount(transferData(1n), NOVEL).rawValue).toMatch(/[1-9]/);
}
});
});
// The swap decoder formats its own amounts, so the same floor has to hold on
// the swap lines of the same screen. `Min. received` is the sharper of the
// two: the slippage floor rendered as `0.0000` states that the swap may return
// nothing.
describe("a swap's amounts never render as zero either", () => {
test("a swap input below the floor keeps a significant digit", () => {
// 50 base units of a 6-decimal token is 0.00005.
const detail = swapDetail(50n, 10n ** 15n, "Amount");
expect(detail.value).toBe("0.00005 USDT");
expect(detail.rawValue).toBe("0.00005");
});
test("a min-received below the floor keeps a significant digit", () => {
// 1 wei of an 18-decimal token.
expect(swapDetail(10n ** 6n, 1n, "Min. received").value).toBe(
"0.000000000000000001 WETH",
);
});
test("swap amounts at or above the floor are still truncated", () => {
expect(swapDetail(1000000n, 10n ** 15n, "Amount").rawValue).toBe(
"1.0000",
);
expect(
swapDetail(1000000n, 999999999999999999n, "Min. received").value,
).toBe("0.9999 WETH");
});
});
describe("the four-decimal rule is otherwise unchanged", () => {
test("a whole amount keeps exactly four decimals", () => {
expect(amount(transferData(5000000000n), USDC).rawValue).toBe(
"5000.0000",
);
});
test("precision beyond four decimals is still truncated", () => {
expect(amount(transferData(1234567890123456789n), DAI).rawValue).toBe(
"1.2345",
);
});
test("an amount at the floor is not extended", () => {
expect(amount(transferData(100000000000000n), DAI).rawValue).toBe(
"0.0001",
);
});
test("a genuine zero still renders as zero", () => {
expect(amount(transferData(0n), DAI).rawValue).toBe("0.0000");
});
// The three truncators now share one module. The floor is a policy of the
// approval and confirmation screens only: the history and balance lists
// keep plain truncation, because the transaction detail view is the
// authoritative record and already shows exact precision.
test("the list rule stays unfloored", () => {
expect(truncateAmount("0.000000000000000001")).toBe("0.0000");
expect(truncateAmountNeverZero("0.000000000000000001")).toBe(
"0.000000000000000001",
);
});
});