diff --git a/README.md b/README.md index f94a9de..0081049 100644 --- a/README.md +++ b/README.md @@ -857,7 +857,12 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. - "Hide fake tokens impersonating a known symbol" checkbox - "Hide tokens with fewer than 1,000 holders" checkbox - "Hide transactions from detected fraud contracts" checkbox - - "Hide dust transactions below N gwei" checkbox + threshold input + - "Hide dust transactions below N gwei" checkbox + threshold input. The + threshold is plain decimal digits, a whole number of gwei, zero or + greater (zero hides nothing). Anything else — a fraction, a negative, + a value carrying its unit, hex (`0x10`) or exponent (`1e3`) notation — + is refused with a flash message and the field snaps back to the stored + threshold, so a number the user did not type is never stored. - Allowed Sites: list with remove buttons - Denied Sites: list with remove buttons - About: project link, license, author, version, release date, and the diff --git a/TODO.md b/TODO.md index 0e177a1..8094a90 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,11 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-12: The dust threshold field now explains a rejection instead of + snapping back in silence, with the parse in a pure, unit-tested module that + accepts plain decimal digits only — hex and exponent notation are refused + rather than read as 16 and 1000 + ([#233](https://git.eeqj.de/sneak/AutistMask/issues/233)). - 2026-08-12: Approval verification became an allowlist — transaction type restricted to 0/1/2 so an EIP-7702 delegation can no longer ride along on an approved transfer, every consequential field compared, the artifact diff --git a/src/popup/dustThreshold.js b/src/popup/dustThreshold.js new file mode 100644 index 0000000..7b64c60 --- /dev/null +++ b/src/popup/dustThreshold.js @@ -0,0 +1,42 @@ +// Parsing for the dust threshold field in Settings. +// +// Pure: no DOM, no state, so the accepted set can be unit tested directly +// instead of through the settings view. +// +// Accepted input is plain decimal digits only, meaning a whole number of +// gwei, zero or greater. Zero is a real setting: it hides nothing. +// +// Deliberately rejected, not coerced: +// "" nothing to save +// "-1" a negative threshold has no meaning +// "1.5" fractional gwei is not a threshold the filter can use +// "100 gwei" the unit is already printed beside the field +// "0x10" hex, which Number() would silently read as 16 +// "1e3" exponent notation, which Number() would silently read as 1000 +// +// The last two are the reason this is a digit test and not a Number() test. +// Number() accepts both, and accepting them would put a number in the field +// that the user did not type — the same silent substitution the visible +// rejection message exists to end. + +// Must render on ONE line of #flash-msg, whose reserved height +// (min-h-[1.25rem]) is exactly one line at text-xs. A string long enough to +// wrap to two lines pushes the settings view down, which the No Layout Shift +// policy forbids. Do not lengthen this without re-running the layout test in +// tests/e2e/run.js, which measures the flash line and goes red on a shift. +const DUST_THRESHOLD_MESSAGE = + "Please enter a whole number of gwei, zero or greater."; + +// Returns the threshold in gwei, or null if the input is not one. +function parseDustThresholdGwei(raw) { + if (typeof raw !== "string") return null; + const trimmed = raw.trim(); + if (!/^[0-9]+$/.test(trimmed)) return null; + const val = Number(trimmed); + // A run of digits long enough to exceed Number's exact integer range + // would round on the way in, so it is not a threshold we can store. + if (!Number.isSafeInteger(val)) return null; + return val; +} + +module.exports = { DUST_THRESHOLD_MESSAGE, parseDustThresholdGwei }; diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index f3ca461..2d1baa3 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -9,6 +9,10 @@ const { pushCurrentView, } = require("./helpers"); const { applyTheme } = require("../theme"); +const { + DUST_THRESHOLD_MESSAGE, + parseDustThresholdGwei, +} = require("../dustThreshold"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { NETWORKS, SUPPORTED_CHAIN_IDS } = require("../../shared/networks"); const { onChainSwitch } = require("../../shared/chainSwitch"); @@ -329,13 +333,14 @@ function init(ctx) { $("settings-dust-threshold").value = state.dustThresholdGwei; $("settings-dust-threshold").addEventListener("change", async () => { - const raw = $("settings-dust-threshold").value.trim(); - const val = Number(raw); - // 0 is accepted and means "hide nothing". Empty, negative, - // fractional and non-numeric input is rejected outright rather than - // coerced, and the field is put back to the stored threshold so it - // never shows a value the wallet is not using. - if (raw !== "" && Number.isInteger(val) && val >= 0) { + const val = parseDustThresholdGwei($("settings-dust-threshold").value); + // Rejected input is never coerced. The field is put back to the + // stored threshold so it never shows a value the wallet is not + // using, and the message says what the field wants so the snap-back + // is explained rather than silent. + if (val === null) { + showFlash(DUST_THRESHOLD_MESSAGE); + } else { state.dustThresholdGwei = val; await saveState(); } diff --git a/tests/dustThreshold.test.js b/tests/dustThreshold.test.js new file mode 100644 index 0000000..8a858d7 --- /dev/null +++ b/tests/dustThreshold.test.js @@ -0,0 +1,243 @@ +// Tests for the dust threshold field in Settings (issue #233). +// +// Two halves: what the parse accepts, and what the settings view does with a +// rejection. The view half runs against the real change handler with the DOM +// helpers stubbed out, because the bug was not in the parse — it was that a +// rejection said nothing. + +const { + DUST_THRESHOLD_MESSAGE, + parseDustThresholdGwei, +} = require("../src/popup/dustThreshold"); + +describe("parsing the dust threshold", () => { + test("accepts a whole number of gwei", () => { + expect(parseDustThresholdGwei("100000")).toBe(100000); + expect(parseDustThresholdGwei("1")).toBe(1); + }); + + // Zero is a real setting, not an empty field: it hides nothing. + test("accepts zero", () => { + expect(parseDustThresholdGwei("0")).toBe(0); + }); + + test("accepts surrounding whitespace", () => { + expect(parseDustThresholdGwei(" 250 ")).toBe(250); + }); + + test("rejects an empty field", () => { + expect(parseDustThresholdGwei("")).toBe(null); + expect(parseDustThresholdGwei(" ")).toBe(null); + }); + + test("rejects a negative threshold", () => { + expect(parseDustThresholdGwei("-1")).toBe(null); + }); + + // parseInt used to read this as 1, which is not what was typed. + test("rejects a fractional value", () => { + expect(parseDustThresholdGwei("1.5")).toBe(null); + expect(parseDustThresholdGwei("1.0")).toBe(null); + }); + + // parseInt used to read this as 100. The unit is printed beside the + // field already. + test("rejects a value carrying its unit", () => { + expect(parseDustThresholdGwei("100 gwei")).toBe(null); + }); + + // Number() reads this as 16. Storing 16 for a field that was told to + // want a whole number of gwei would be the same silent substitution the + // message exists to end. + test("rejects hex notation", () => { + expect(parseDustThresholdGwei("0x10")).toBe(null); + }); + + // Number() reads this as 1000. + test("rejects exponent notation", () => { + expect(parseDustThresholdGwei("1e3")).toBe(null); + }); + + test("rejects other non-numeric input", () => { + expect(parseDustThresholdGwei("lots")).toBe(null); + expect(parseDustThresholdGwei("+5")).toBe(null); + expect(parseDustThresholdGwei("Infinity")).toBe(null); + expect(parseDustThresholdGwei(undefined)).toBe(null); + expect(parseDustThresholdGwei(5)).toBe(null); + }); + + // Beyond 2^53 the digits would round on the way in, so the stored + // threshold would not be the one typed. + test("rejects a value too large to hold exactly", () => { + expect(parseDustThresholdGwei("9007199254740993")).toBe(null); + }); +}); + +describe("the rejection message", () => { + // README, Language & Labeling: error messages are full sentences. + test("is a full sentence naming the constraint", () => { + expect(DUST_THRESHOLD_MESSAGE).toMatch(/^[A-Z].*\.$/); + expect(DUST_THRESHOLD_MESSAGE).toContain("whole number of gwei"); + expect(DUST_THRESHOLD_MESSAGE).toContain("zero or greater"); + }); +}); + +describe("the flash line the message is shown in", () => { + const fs = require("fs"); + const path = require("path"); + + const POPUP_HTML = fs.readFileSync( + path.join(__dirname, "..", "src", "popup", "index.html"), + "utf8", + ); + + // This asserts only that the reservation exists in the markup. It does + // NOT and CANNOT assert that the message fits inside it: jest runs on + // the node environment here, with no layout engine, so every rendered + // height is zero. An earlier version of this block claimed to pin the + // No Layout Shift policy with this regex, and it passed at any message + // length, including one that wrapped to two lines and pushed the + // settings view down 12px. + // + // The assertion that actually measures — empty line vs. the message, + // real Chromium, documented 360x600 popup — is + // "a rejected dust threshold shifts no layout (#233)" in + // tests/e2e/run.js, run by make test-e2e. It is not in make check + // because REPO_POLICIES.md caps make test at 20 seconds and a browser + // suite does not fit; run it before changing the wording. + test("reserves its height in the markup", () => { + const flashLine = POPUP_HTML.match( + /