diff --git a/TODO.md b/TODO.md index 608523b..f3de612 100644 --- a/TODO.md +++ b/TODO.md @@ -54,6 +54,10 @@ undefined identifiers, which is how remaining wallets, the selection only moves when it was deleted, and the active-address change is broadcast to connected sites ([#156](https://git.eeqj.de/sneak/AutistMask/issues/156)). +- 2026-08-11: One row per on-chain value movement in transaction history: the + merge moved into the pure `mergeTransactions` and the zero-ETH native side of + a plain ERC-20 transfer absorbed into its token row + ([#177](https://git.eeqj.de/sneak/AutistMask/issues/177)). - 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model on `next`, with Status and Next Step refreshed ([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)). diff --git a/src/shared/transactions.js b/src/shared/transactions.js index 5f528eb..303579f 100644 --- a/src/shared/transactions.js +++ b/src/shared/transactions.js @@ -113,6 +113,85 @@ function parseTokenTransfer(tt, addrLower) { }; } +// True when a parsed native entry moved no ETH. Contract-call entries have +// their amount fields blanked by parseTx, so they are never judged here. +function movedNoEther(tx) { + if (tx.direction === "contract") return false; + return BigInt(tx.rawAmount || "0") === BigInt(0); +} + +// Merge parsed normal transactions with parsed ERC-20 token transfers into +// one row per distinct value movement. Pure: it reads only its arguments +// and returns a new list sorted newest block first. +// +// The merge key is the transaction hash for the native entry and +// hash + token contract for each token transfer, so: +// +// - A display-level contract call (a swap and friends, direction +// "contract") absorbs every token leg of its hash into the single +// native entry, because the legs are hops of one operation rather +// than separate movements the user made. +// - Otherwise each distinct token contract in the transaction keeps its +// own row, so a hash carrying several genuine transfers stays several +// rows. +// - The native entry of such a transaction is dropped when it moved no +// ETH and at least one token transfer shares its hash: that entry is +// the ERC-20 call itself, already represented by the token row. A +// native entry that moved ETH survives alongside the token rows, since +// the ETH and the tokens are two real movements, and a zero-value +// native transaction with no token transfer on its hash survives too. +function mergeTransactions(txs, tokenTransfers) { + const byKey = new Map(); + + // Entries are copied so consolidation never writes through to the + // caller's objects. + for (const tx of txs) { + byKey.set(tx.hash, { ...tx }); + } + + const absorbedHashes = new Set(); + + for (const parsed of tokenTransfers) { + const existing = byKey.get(parsed.hash); + if (existing && existing.direction === "contract") { + // For contract calls (swaps), consolidate into the original + // tx entry. Prefer the "received" transfer (swap output) + // for the display amount. If no received transfer exists, + // fall back to the first "sent" transfer (swap input). + const isReceived = parsed.direction === "received"; + const needsAmount = !existing.exactValue; + if (isReceived || needsAmount) { + existing.value = parsed.value; + existing.exactValue = parsed.exactValue; + existing.rawAmount = parsed.rawAmount; + existing.rawUnit = parsed.rawUnit; + existing.symbol = parsed.symbol; + existing.contractAddress = parsed.contractAddress; + existing.holders = parsed.holders; + } + // Keep the original tx's from/to (the user's address and the + // contract they called), not the token transfer's from/to + // which may be a router or Permit2 contract. + continue; + } + if (existing && movedNoEther(existing)) { + absorbedHashes.add(parsed.hash); + } + // Every other token transfer gets its own entry. + byKey.set(parsed.hash + ":" + (parsed.contractAddress || ""), { + ...parsed, + }); + } + + for (const hash of absorbedHashes) { + byKey.delete(hash); + } + + const merged = [...byKey.values()]; + merged.sort((a, b) => b.blockNumber - a.blockNumber); + return merged; +} + async function fetchRecentTransactions(address, blockscoutUrl, count = 25) { log.debugf("fetchRecentTransactions", address); const addrLower = address.toLowerCase(); @@ -145,53 +224,11 @@ async function fetchRecentTransactions(address, blockscoutUrl, count = 25) { const txJson = txResp.ok ? await txResp.json() : {}; const ttJson = ttResp.ok ? await ttResp.json() : {}; - const txsByHash = new Map(); + const txs = mergeTransactions( + (txJson.items || []).map((tx) => parseTx(tx, addrLower)), + (ttJson.items || []).map((tt) => parseTokenTransfer(tt, addrLower)), + ); - for (const tx of txJson.items || []) { - txsByHash.set(tx.hash, parseTx(tx, addrLower)); - } - - // When a token transfer shares a hash with a normal tx, the normal tx - // is the contract call (0 ETH) and the token transfer has the real - // amount and symbol. For contract calls (swaps), a single transaction - // can produce multiple token transfers (input, intermediates, output). - // We consolidate these into the original tx entry using the token - // transfer where the user *receives* tokens (the swap output), so - // the transaction list shows the final result rather than confusing - // intermediate hops. We preserve the original tx's from/to so the - // user sees their own address, not a router or Permit2 contract. - for (const tt of ttJson.items || []) { - const parsed = parseTokenTransfer(tt, addrLower); - const existing = txsByHash.get(parsed.hash); - if (existing && existing.direction === "contract") { - // For contract calls (swaps), consolidate into the original - // tx entry. Prefer the "received" transfer (swap output) - // for the display amount. If no received transfer exists, - // fall back to the first "sent" transfer (swap input). - const isReceived = parsed.direction === "received"; - const needsAmount = !existing.exactValue; - if (isReceived || needsAmount) { - existing.value = parsed.value; - existing.exactValue = parsed.exactValue; - existing.rawAmount = parsed.rawAmount; - existing.rawUnit = parsed.rawUnit; - existing.symbol = parsed.symbol; - existing.contractAddress = parsed.contractAddress; - existing.holders = parsed.holders; - } - // Keep the original tx's from/to (the user's address and the - // contract they called), not the token transfer's from/to - // which may be a router or Permit2 contract. - continue; - } - // Non-contract token transfers get their own entries. - const ttKey = parsed.hash + ":" + (parsed.contractAddress || ""); - txsByHash.set(ttKey, parsed); - } - - const txs = [...txsByHash.values()]; - - txs.sort((a, b) => b.blockNumber - a.blockNumber); const result = txs.slice(0, count); log.debugf("fetchRecentTransactions done, count:", result.length); return result; @@ -265,4 +302,8 @@ function filterTransactions(txs, filters = {}) { return { transactions: filtered, newFraudContracts: newFraud }; } -module.exports = { fetchRecentTransactions, filterTransactions }; +module.exports = { + fetchRecentTransactions, + filterTransactions, + mergeTransactions, +}; diff --git a/tests/transactions.test.js b/tests/transactions.test.js index 73bf7b7..cc68f2f 100644 --- a/tests/transactions.test.js +++ b/tests/transactions.test.js @@ -36,6 +36,7 @@ global.chrome = { storage: { local: {} } }; const { fetchRecentTransactions, filterTransactions, + mergeTransactions, } = require("../src/shared/transactions"); const { KNOWN_SYMBOLS } = require("../src/shared/tokenList"); const { debugFetch } = require("../src/shared/log"); @@ -685,6 +686,339 @@ describe("legitimate transactions are never filtered", () => { }); }); +// --------------------------------------------------------------------------- +// mergeTransactions is the pure core of the merge: it takes parsed native +// entries and parsed token transfers and decides how many rows one on-chain +// transaction becomes. One transaction is one row per distinct value +// movement, so the native side of a plain ERC-20 transfer must not survive +// next to its token row (the duplicate-row bug), while a hash that really +// did move several things must keep a row for each. +// --------------------------------------------------------------------------- + +// A native entry as parseTx produces it for a decoded contract call: the +// amount fields are blanked and direction is "contract". +function contractCallTx(overrides = {}) { + return nativeTx({ + from: VICTIM, + to: USDC_CONTRACT, + value: "", + exactValue: "", + rawAmount: "", + rawUnit: "", + valueGwei: 0, + direction: "contract", + directionLabel: "Approve", + isContractCall: true, + method: "approve", + ...overrides, + }); +} + +// The native entry parseTx produces for a plain ERC-20 transfer: sent to the +// token contract, no ETH, and method "transfer", which is exactly why it is +// not marked as a display-level contract call. +function erc20CallTx(overrides = {}) { + return nativeTx({ + from: VICTIM, + to: USDC_CONTRACT, + value: "0.0000", + exactValue: "0.0", + rawAmount: "0", + valueGwei: 0, + direction: "sent", + directionLabel: "Sent", + isContractCall: true, + method: "transfer", + ...overrides, + }); +} + +describe("mergeTransactions: one row per value movement", () => { + const HASH = "0x" + "d".repeat(64); + const OTHER_HASH = "0x" + "e".repeat(64); + const ROUTER = "0x3fc91a3afd70395cd496c647d5a6cc9d4b2b7fad"; + + test("a plain ERC-20 transfer yields one row, the token row", () => { + const native = erc20CallTx({ hash: HASH }); + const token = tokenTx({ + hash: HASH, + from: VICTIM, + to: ORDINARY_PEER, + direction: "sent", + directionLabel: "Sent", + }); + + const merged = mergeTransactions([native], [token]); + expect(merged).toHaveLength(1); + expect(merged[0].symbol).toBe("USDC"); + expect(merged[0].exactValue).toBe("1500.5"); + expect(merged[0].contractAddress).toBe(USDC_CONTRACT); + }); + + test("an ETH-only transfer keeps its row unchanged", () => { + const merged = mergeTransactions([legitimateEthSend()], []); + expect(merged).toHaveLength(1); + expect(merged[0]).toEqual(legitimateEthSend()); + }); + + test("a genuine zero-value native transaction is still displayed", () => { + const zero = nativeTx({ + hash: HASH, + from: VICTIM, + to: ORDINARY_PEER, + value: "0.0000", + exactValue: "0.0", + rawAmount: "0", + valueGwei: 0, + direction: "sent", + directionLabel: "Sent", + }); + + const merged = mergeTransactions([zero], []); + expect(merged).toEqual([zero]); + }); + + test("a zero-value native row is only absorbed by a transfer sharing its hash", () => { + const zero = erc20CallTx({ hash: HASH }); + const unrelated = tokenTx({ hash: OTHER_HASH }); + + const merged = mergeTransactions([zero], [unrelated]); + expect(merged).toHaveLength(2); + expect(merged.map((t) => t.hash).sort()).toEqual( + [HASH, OTHER_HASH].sort(), + ); + }); + + test("a native transaction that moved ETH keeps its row beside the token row", () => { + // An undecoded call (no method name) carrying ETH that also emitted + // a token transfer: two real movements, so two rows. + const native = nativeTx({ + hash: HASH, + from: VICTIM, + to: ROUTER, + value: "0.2500", + exactValue: "0.25", + rawAmount: "250000000000000000", + valueGwei: 250000000, + direction: "sent", + directionLabel: "Sent", + isContractCall: true, + }); + const token = tokenTx({ hash: HASH, from: ROUTER, to: VICTIM }); + + const merged = mergeTransactions([native], [token]); + expect(merged).toHaveLength(2); + expect(merged.map((t) => t.symbol).sort()).toEqual(["ETH", "USDC"]); + }); + + test("a sub-gwei ETH movement keeps its row beside the token row", () => { + // 500000000 wei is 0.5 gwei, so parseTx's valueGwei floors to 0 while + // rawAmount stays nonzero. Deciding "moved no ETH" on valueGwei would + // delete this row and lose a real ETH movement, so the decision is made + // on rawAmount as a BigInt. + const native = nativeTx({ + hash: HASH, + from: VICTIM, + to: ROUTER, + value: "0.0000", + exactValue: "0.0000000005", + rawAmount: "500000000", + valueGwei: 0, + direction: "sent", + directionLabel: "Sent", + isContractCall: true, + }); + const token = tokenTx({ hash: HASH, from: ROUTER, to: VICTIM }); + + const merged = mergeTransactions([native], [token]); + expect(merged).toHaveLength(2); + expect(merged.map((t) => t.symbol).sort()).toEqual(["ETH", "USDC"]); + expect(merged.find((t) => t.symbol === "ETH").rawAmount).toBe( + "500000000", + ); + }); + + test("a swap consolidates every token leg into one row, preferring the received leg", () => { + const native = contractCallTx({ + hash: HASH, + to: ROUTER, + directionLabel: "Swap", + method: "execute", + }); + const sentLeg = tokenTx({ + hash: HASH, + from: VICTIM, + to: ROUTER, + direction: "sent", + directionLabel: "Sent", + }); + const receivedLeg = tokenTx({ + hash: HASH, + from: ROUTER, + to: VICTIM, + value: "0.2500", + exactValue: "0.25", + rawAmount: "250000000000000000", + rawUnit: "WETH base units (10^-18)", + symbol: "WETH", + contractAddress: WETH_CONTRACT, + holders: 850000, + }); + + const merged = mergeTransactions([native], [sentLeg, receivedLeg]); + expect(merged).toHaveLength(1); + expect(merged[0].symbol).toBe("WETH"); + expect(merged[0].exactValue).toBe("0.25"); + // The user's own address and the contract called are preserved. + expect(merged[0].from).toBe(VICTIM); + expect(merged[0].to).toBe(ROUTER); + expect(merged[0].directionLabel).toBe("Swap"); + }); + + test("a swap whose legs are all sent takes its amount from the first sent leg", () => { + const native = contractCallTx({ + hash: HASH, + to: ROUTER, + directionLabel: "Swap", + method: "execute", + }); + const firstSent = tokenTx({ + hash: HASH, + from: VICTIM, + to: ROUTER, + direction: "sent", + directionLabel: "Sent", + }); + const secondSent = tokenTx({ + hash: HASH, + from: VICTIM, + to: ROUTER, + value: "0.2500", + exactValue: "0.25", + rawAmount: "250000000000000000", + rawUnit: "WETH base units (10^-18)", + symbol: "WETH", + contractAddress: WETH_CONTRACT, + holders: 850000, + direction: "sent", + directionLabel: "Sent", + }); + + const merged = mergeTransactions([native], [firstSent, secondSent]); + expect(merged).toHaveLength(1); + // With no received leg the display amount comes from the first sent + // leg, and a later sent leg does not overwrite it. + expect(merged[0].symbol).toBe("USDC"); + expect(merged[0].exactValue).toBe("1500.5"); + expect(merged[0].contractAddress).toBe(USDC_CONTRACT); + expect(merged[0].holders).toBe(3500000); + }); + + test("a contract call carrying ETH plus a token transfer stays one row", () => { + const native = contractCallTx({ + hash: HASH, + to: ROUTER, + directionLabel: "Swap", + method: "swapExactETHForTokens", + valueGwei: 250000000, + }); + const received = tokenTx({ hash: HASH, from: ROUTER, to: VICTIM }); + + const merged = mergeTransactions([native], [received]); + expect(merged).toHaveLength(1); + expect(merged[0].symbol).toBe("USDC"); + expect(merged[0].exactValue).toBe("1500.5"); + // The ETH leg is still visible as the row's native quantity. + expect(merged[0].valueGwei).toBe(250000000); + }); + + test("an approve keeps its row and survives the filters", () => { + const approve = contractCallTx({ hash: HASH }); + + const merged = mergeTransactions([approve], []); + expect(merged).toEqual([approve]); + expect(filterTransactions(merged, filters()).transactions).toEqual([ + approve, + ]); + }); + + test("a contract creation keeps its row", () => { + const creation = nativeTx({ + hash: HASH, + from: VICTIM, + to: "", + value: "0.0000", + exactValue: "0.0", + rawAmount: "0", + valueGwei: 0, + direction: "sent", + directionLabel: "Sent", + }); + + expect(mergeTransactions([creation], [])).toEqual([creation]); + }); + + test("a native self-send keeps its single row", () => { + const selfSend = nativeTx({ + hash: HASH, + from: VICTIM, + to: VICTIM, + direction: "sent", + directionLabel: "Sent", + }); + + expect(mergeTransactions([selfSend], [])).toEqual([selfSend]); + }); + + test("a token self-send yields one row", () => { + const native = erc20CallTx({ hash: HASH }); + const token = tokenTx({ + hash: HASH, + from: VICTIM, + to: VICTIM, + direction: "sent", + directionLabel: "Sent", + }); + + const merged = mergeTransactions([native], [token]); + expect(merged).toHaveLength(1); + expect(merged[0].symbol).toBe("USDC"); + expect(merged[0].from).toBe(VICTIM); + expect(merged[0].to).toBe(VICTIM); + }); + + test("several distinct tokens moved by one ERC-20 call keep a row each", () => { + const native = erc20CallTx({ hash: HASH }); + const usdc = tokenTx({ hash: HASH }); + const weth = tokenTx({ + hash: HASH, + symbol: "WETH", + contractAddress: WETH_CONTRACT, + holders: 850000, + }); + + const merged = mergeTransactions([native], [usdc, weth]); + expect(merged.map((t) => t.symbol).sort()).toEqual(["USDC", "WETH"]); + }); + + test("rows are sorted by block number, newest first", () => { + const older = nativeTx({ hash: HASH, blockNumber: 21000000 }); + const newer = nativeTx({ hash: OTHER_HASH, blockNumber: 21000010 }); + + const merged = mergeTransactions([older, newer], []); + expect(merged.map((t) => t.blockNumber)).toEqual([21000010, 21000000]); + }); + + test("the entries handed in are never mutated", () => { + const native = contractCallTx({ hash: HASH, method: "execute" }); + const token = tokenTx({ hash: HASH }); + const before = JSON.stringify([native, token]); + + mergeTransactions([native], [token]); + expect(JSON.stringify([native, token])).toBe(before); + }); +}); + // --------------------------------------------------------------------------- // fetchRecentTransactions owns the per-address merge of normal transactions // with ERC-20 transfers. (The cross-address merge Home performs lives in @@ -886,13 +1220,12 @@ describe("fetchRecentTransactions merge and dedup", () => { expect(txs.map((t) => t.symbol).sort()).toEqual(["USDC", "WETH"]); }); - // Documents current behaviour: for a plain ERC-20 transfer the method is - // "transfer", so parseTx does not mark the entry as a contract call in - // the display sense and the merge loop does not consolidate the token - // transfer into it. The result is two entries for one transaction: a - // zero-value native row and the real token row. The zero-value row also - // escapes dust filtering because isContractCall is true. - test("current behaviour: a plain ERC-20 transfer produces two entries", async () => { + // Regression guard for the duplicate-row bug: for a plain ERC-20 + // transfer the method is "transfer", so parseTx does not mark the entry + // as a contract call in the display sense. The native side of that + // transaction moved no ETH and is represented by the token row, so it + // must not survive the merge as a second, zero-value row. + test("a plain ERC-20 transfer produces exactly one entry", async () => { const hash = "0x" + "5".repeat(64); respondWith( [ @@ -925,14 +1258,15 @@ describe("fetchRecentTransactions merge and dedup", () => { ); const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT); - expect(txs).toHaveLength(2); - expect(txs.map((t) => t.symbol).sort()).toEqual(["ETH", "USDC"]); - const nativeRow = txs.find((t) => t.symbol === "ETH"); - expect(nativeRow.exactValue).toBe("0.0"); - expect(nativeRow.isContractCall).toBe(true); - // And the zero-value row is not removed by the dust filter. + expect(txs).toHaveLength(1); + expect(txs[0].symbol).toBe("USDC"); + expect(txs[0].exactValue).toBe("1.0"); + expect(txs[0].direction).toBe("sent"); + expect(txs[0].contractAddress).toBe(USDC_CONTRACT); + // The surviving row is the token row, and the filters keep it. const kept = filterTransactions(txs, filters()).transactions; - expect(kept).toHaveLength(2); + expect(kept).toHaveLength(1); + expect(kept[0].symbol).toBe("USDC"); }); test("entries are sorted by block number descending and capped at count", async () => {