fix: honour a dust threshold of 0 and compare addresses case-insensitively (closes #179)
Some checks failed
check / check (push) Has been cancelled
Some checks failed
check / check (push) Has been cancelled
This commit was merged in pull request #228.
This commit is contained in:
@@ -329,18 +329,42 @@ describe("known-symbol spoof verification", () => {
|
||||
expect(result.newFraudContracts).toEqual([]);
|
||||
});
|
||||
|
||||
// Documents current behaviour, not desired behaviour: the spoof check
|
||||
// compares tx.contractAddress against a lowercased known address with
|
||||
// ===, so a caller passing a checksummed address for a genuine token has
|
||||
// it treated as a spoof. In the app this cannot happen because
|
||||
// parseTokenTransfer lowercases, but the exported function is not
|
||||
// defensive about it the way the blocklist check is.
|
||||
test("current behaviour: a checksummed genuine contract is treated as a spoof", () => {
|
||||
const genuineButChecksummed = tokenTx({
|
||||
// Regression guard (#179): EIP-55 mixed case is a checksum over the
|
||||
// address, not part of its identity, so the contract comparison must be
|
||||
// case-insensitive in both directions — a genuine token in any casing is
|
||||
// genuine, and a spoof cannot escape detection by changing its casing.
|
||||
test("a genuine contract in all-lowercase form is not a spoof", () => {
|
||||
const tx = tokenTx({ contractAddress: USDC_CONTRACT });
|
||||
expect(filterTransactions([tx], filters()).transactions).toEqual([tx]);
|
||||
});
|
||||
|
||||
test("a genuine contract in EIP-55 checksummed form is not a spoof", () => {
|
||||
const tx = tokenTx({
|
||||
contractAddress: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48",
|
||||
});
|
||||
const result = filterTransactions([genuineButChecksummed], filters());
|
||||
const result = filterTransactions([tx], filters());
|
||||
expect(result.transactions).toEqual([tx]);
|
||||
expect(result.newFraudContracts).toEqual([]);
|
||||
});
|
||||
|
||||
test("a genuine contract in all-uppercase form is not a spoof", () => {
|
||||
const tx = tokenTx({
|
||||
contractAddress: "0X" + USDC_CONTRACT.slice(2).toUpperCase(),
|
||||
});
|
||||
const result = filterTransactions([tx], filters());
|
||||
expect(result.transactions).toEqual([tx]);
|
||||
expect(result.newFraudContracts).toEqual([]);
|
||||
});
|
||||
|
||||
test("a genuinely different contract claiming USDC is still a spoof in any casing", () => {
|
||||
const tx = tokenTx({
|
||||
contractAddress: "0xD05339F9EA5AB9D9F03B9D57F671D2ABD1F55C82",
|
||||
});
|
||||
const result = filterTransactions([tx], filters());
|
||||
expect(result.transactions).toEqual([]);
|
||||
// The recorded fraud contract is normalised, so the persisted
|
||||
// blocklist matches later transfers whatever casing they arrive in.
|
||||
expect(result.newFraudContracts).toEqual([FAKE_ETH_CONTRACT]);
|
||||
});
|
||||
|
||||
// Documents current behaviour: README.md:810-814 says all four filters
|
||||
@@ -412,6 +436,21 @@ describe("low-holder token filtering (the 1,000-holder rule)", () => {
|
||||
expect(tx.holders).toBeNull();
|
||||
expect(filterTransactions([tx], filters()).transactions).toEqual([tx]);
|
||||
});
|
||||
|
||||
// Regression guard (#179): an unknown holder count on a real token — the
|
||||
// explorer rate-limited the call, or a self-hosted instance omits the
|
||||
// field — must not be read as zero holders. Reading it that way hides a
|
||||
// legitimate transfer from the user's history, the same over-filtering
|
||||
// harm as the zero-threshold bug. This pins the `tx.holders !== null`
|
||||
// guard, which no fixture previously reached.
|
||||
test("a token whose holder count is unknown is not filtered", () => {
|
||||
const tx = tokenTx({
|
||||
symbol: NOVEL_SPAM_SYMBOL,
|
||||
contractAddress: NOVEL_SPAM_CONTRACT,
|
||||
holders: null,
|
||||
});
|
||||
expect(filterTransactions([tx], filters()).transactions).toEqual([tx]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("fraud contract blocklist", () => {
|
||||
@@ -575,16 +614,50 @@ describe("dust threshold filtering", () => {
|
||||
expect(filterTransactions([tx], filters()).transactions).toEqual([tx]);
|
||||
});
|
||||
|
||||
// Documents current behaviour: the threshold is read as
|
||||
// `filters.dustThresholdGwei || 100000`, so a user who sets the threshold
|
||||
// to 0 (the natural way to ask for no dust filtering while leaving the
|
||||
// toggle on) silently gets the 100,000 gwei default instead.
|
||||
test("current behaviour: a threshold of 0 falls back to the 100,000 gwei default", () => {
|
||||
const result = filterTransactions(
|
||||
[dustOf(50)],
|
||||
// Regression guard (#179): 0 is a real threshold meaning "hide nothing",
|
||||
// not an absent one. It used to be swallowed by `|| 100000`, so the one
|
||||
// value a user would pick to see everything was the one that did not
|
||||
// work.
|
||||
test("a threshold of 0 hides nothing, leaving the toggle on", () => {
|
||||
const dust = dustOf(50);
|
||||
const zero = dustOf(0);
|
||||
const opts = filters({ dustThresholdGwei: 0 });
|
||||
expect(filterTransactions([dust], opts).transactions).toEqual([dust]);
|
||||
expect(filterTransactions([zero], opts).transactions).toEqual([zero]);
|
||||
});
|
||||
|
||||
test("a threshold of 0 agrees with clearing the hide-dust checkbox", () => {
|
||||
const tx = nativeDustTransfer();
|
||||
const thresholdZero = filterTransactions(
|
||||
[tx],
|
||||
filters({ dustThresholdGwei: 0 }),
|
||||
);
|
||||
expect(result.transactions).toEqual([]);
|
||||
const toggleOff = filterTransactions(
|
||||
[tx],
|
||||
filters({ hideDustTransactions: false }),
|
||||
);
|
||||
expect(thresholdZero.transactions).toEqual([tx]);
|
||||
expect(toggleOff.transactions).toEqual([tx]);
|
||||
});
|
||||
|
||||
test("0, unset and a set threshold are three distinct behaviours", () => {
|
||||
const tx = dustOf(50);
|
||||
expect(
|
||||
filterTransactions([tx], filters({ dustThresholdGwei: 0 }))
|
||||
.transactions,
|
||||
).toEqual([tx]);
|
||||
expect(
|
||||
filterTransactions([tx], filters({ dustThresholdGwei: undefined }))
|
||||
.transactions,
|
||||
).toEqual([]);
|
||||
expect(
|
||||
filterTransactions([tx], filters({ dustThresholdGwei: 40 }))
|
||||
.transactions,
|
||||
).toEqual([tx]);
|
||||
expect(
|
||||
filterTransactions([tx], filters({ dustThresholdGwei: 60 }))
|
||||
.transactions,
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user