From 9c2f617ac049903fea1508db2d0864ee14cc425c Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 08:20:26 +0000 Subject: [PATCH] fix: explain a rejected dust threshold instead of snapping back silently (closes #233) The dust threshold field resynced to the stored value on a rejected input and said nothing, so the box changed to a different number with no reason given. It was the only validated input in the settings view that rejected without a message. Rejected input now shows one full sentence naming the constraint, using the flash line already used by the RPC and Blockscout validation in the same file. No layout shift: #flash-msg is always present with its height reserved. Hex and exponent notation are rejected rather than accepted. Number() reads 0x10 as 16 and 1e3 as 1000, which the earlier parseInt did not, and storing either would put a number in the field that the user never typed - the same silent substitution the message exists to end. Accepted input is plain decimal digits only; the field is inputmode="numeric" and the unit is printed beside it. The parse moves to src/popup/dustThreshold.js, pure and unit tested, with the message beside it so there is one wording. Tests cover the accepted set, the rejected notations, that a rejection flashes the message and stores nothing while a valid value stores and stays quiet, and that the flash line reserves its height. --- README.md | 7 +- TODO.md | 5 + src/popup/dustThreshold.js | 37 ++++++ src/popup/views/settings.js | 19 +-- tests/dustThreshold.test.js | 232 ++++++++++++++++++++++++++++++++++++ 5 files changed, 292 insertions(+), 8 deletions(-) create mode 100644 src/popup/dustThreshold.js create mode 100644 tests/dustThreshold.test.js diff --git a/README.md b/README.md index 33949f9..f8acaaa 100644 --- a/README.md +++ b/README.md @@ -811,7 +811,12 @@ of it. - "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 dcc905f..d4a7a84 100644 --- a/TODO.md +++ b/TODO.md @@ -51,6 +51,11 @@ undefined identifiers, which is how Ethereum mainnet ERC-20s — with `TOKENS` in `src/shared/tokenList.js` named as the authoritative set ([#239](https://git.eeqj.de/sneak/AutistMask/issues/239)). +- 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-11: Known-symbol spoof verification became a Settings toggle (`hideSpoofedSymbols`), on by default, governing the transaction-history filter and the fraud-contract learning it feeds diff --git a/src/popup/dustThreshold.js b/src/popup/dustThreshold.js new file mode 100644 index 0000000..d816978 --- /dev/null +++ b/src/popup/dustThreshold.js @@ -0,0 +1,37 @@ +// 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. + +const DUST_THRESHOLD_MESSAGE = + "Please enter the dust threshold as 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..6172782 --- /dev/null +++ b/tests/dustThreshold.test.js @@ -0,0 +1,232 @@ +// 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("showing the message shifts no layout", () => { + const fs = require("fs"); + const path = require("path"); + + const POPUP_HTML = fs.readFileSync( + path.join(__dirname, "..", "src", "popup", "index.html"), + "utf8", + ); + + // README, No Layout Shift: the message goes to the always-present flash + // line, whose height is reserved whether or not it holds text. Nothing + // is inserted next to the field itself. + test("the flash line is always present with its height reserved", () => { + const flashLine = POPUP_HTML.match( + / { + let elements; + let flashes; + let saves; + let state; + + // A stand-in for one DOM node: enough of an element for init() to set + // properties on it and hang listeners off it. + function fakeElement() { + return { + value: "", + checked: false, + textContent: "", + href: "", + style: {}, + dataset: {}, + classList: { add() {}, remove() {} }, + listeners: {}, + addEventListener(event, handler) { + this.listeners[event] = handler; + }, + querySelectorAll: () => [], + }; + } + + function loadSettingsView() { + elements = {}; + flashes = []; + saves = 0; + + jest.resetModules(); + + jest.doMock("../src/popup/views/helpers", () => ({ + $: (id) => (elements[id] ||= fakeElement()), + showView: () => {}, + updateDebugBanner: () => {}, + showFlash: (msg) => flashes.push(msg), + escapeHtml: (s) => s, + flashCopyFeedback: () => {}, + goBack: () => {}, + pushCurrentView: () => {}, + onViewLeave: () => {}, + VIEWS: [], + })); + + state = require("../src/shared/state").state; + state.dustThresholdGwei = 100000; + + const settings = require("../src/popup/views/settings"); + settings.init({}); + return elements["settings-dust-threshold"]; + } + + beforeEach(() => { + globalThis.chrome = { + runtime: { sendMessage: () => {} }, + storage: { + local: { + get: async () => ({}), + set: async () => { + saves++; + }, + }, + }, + }; + }); + + afterEach(() => { + jest.dontMock("../src/popup/views/helpers"); + delete globalThis.chrome; + }); + + async function change(field, typed) { + field.value = typed; + await field.listeners.change(); + } + + test("a valid value is stored and says nothing", async () => { + const field = loadSettingsView(); + + await change(field, "250"); + + expect(state.dustThresholdGwei).toBe(250); + expect(field.value).toBe(250); + expect(flashes).toEqual([]); + expect(saves).toBe(1); + }); + + test("a rejected value shows the message and is not stored", async () => { + const field = loadSettingsView(); + + await change(field, "1.5"); + + expect(state.dustThresholdGwei).toBe(100000); + expect(flashes).toEqual([DUST_THRESHOLD_MESSAGE]); + expect(saves).toBe(0); + }); + + // The snap-back is the behaviour the message explains, so it stays. + test("a rejected value still resyncs the field to what is stored", async () => { + const field = loadSettingsView(); + + await change(field, "100 gwei"); + + expect(field.value).toBe(100000); + }); + + test("every rejected notation gets the same one message", async () => { + for (const typed of ["", "-1", "1.5", "100 gwei", "0x10", "1e3"]) { + const field = loadSettingsView(); + + await change(field, typed); + + expect(flashes).toEqual([DUST_THRESHOLD_MESSAGE]); + expect(state.dustThresholdGwei).toBe(100000); + } + }); + + test("zero is accepted, not treated as an empty field", async () => { + const field = loadSettingsView(); + + await change(field, "0"); + + expect(state.dustThresholdGwei).toBe(0); + expect(flashes).toEqual([]); + }); +});