fix: one transaction history row per value movement (closes #177)

A plain ERC-20 transfer produced two rows: the token-transfer row and the
zero-ETH native row for the same hash. parseTx leaves method "transfer" out
of the display-level contract-call case, so the merge loop's
direction === "contract" test never absorbed the native side.

The merge is now the pure mergeTransactions(txs, tokenTransfers) in
src/shared/transactions.js, unit tested directly. It keys the native entry
by hash and each token transfer by hash plus token contract, and drops the
native entry when it moved no ETH and a token transfer shares its hash. A
native entry that moved ETH survives beside the token rows, a zero-value
native transaction with no token transfer on its hash still displays, and a
display-level contract call keeps consolidating its legs into one row.

The dust filter's isContractCall exemption is unchanged: it still carries
approve and other zero-ETH calls that have no token row to be represented
by.

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 () => {