sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

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

Merged
clawbot merged 1 commits from fix/issue-177-duplicate-transfer-rows into next 2026-08-11 14:56:31 +02:00
Conversation 2 Commits 1 Files Changed 3 +440 -61
clawbot commented 2026-08-11 14:20:46 +02:00
Collaborator
Copy Link

Closes #177.

The merge rule

The merge loop moved out of fetchRecentTransactions into a pure
mergeTransactions(txs, tokenTransfers) in src/shared/transactions.js,
exported and unit tested directly. It takes parsed entries, returns a new
list sorted newest block first, and mutates neither input.

The key is the transaction hash for the native entry and hash + token
contract for each token transfer, as the issue asked — no widening of the
direction/method string comparison, which is what produced the bug:

  1. A display-level contract call (direction === "contract") absorbs every
    token leg of its hash into the single native entry. Unchanged behaviour:
    the legs of a swap are hops of one operation, not separate movements.
  2. Otherwise each distinct token contract on the hash keeps its own row, so
    a transaction that really moved several tokens stays several rows.
  3. The native entry 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.

Rule 3 is what fixes the duplicate. It is deliberately not "suppress
zero-value native rows": the drop needs a token transfer on the same hash,
so a genuine zero-value native transaction still displays. And it is not
"drop the native row whenever a token transfer shares the hash": a native
entry that moved ETH survives beside the token rows, because the ETH and the
tokens are two real movements.

Zero ETH is decided on rawAmount as a BigInt, not on valueGwei, which
floors sub-gwei amounts to 0.

Row counts, before and after

case before after
plain ERC-20 transfer 2 (zero-ETH native + token) 1 (token)
ETH-only transfer 1 1
genuine zero-value native tx, no token transfer on the hash 1 1
undecoded call carrying ETH that also emitted a token transfer 2 2
sub-gwei ETH movement plus a token transfer on the same hash 2 2
Universal Router swap (execute, sent + received legs) 1 1
swap whose legs are all sent 1 1
contract call carrying ETH plus a token transfer (swapExactETHForTokens) 1 1
approve (no token transfer) 1 1
contract creation 1 1
native self-send 1 1
token self-send 2 1
one ERC-20 call moving several distinct tokens 1 + N N

Only the two transfer-shaped rows change. The swap row still takes its
display amount from the received leg and keeps the user's own from/to
rather than the router's; swapExactETHForTokens keeps its valueGwei, so
the ETH leg stays visible as the row's native quantity.

Dust filter

filterTransactions's isContractCall dust exemption is unchanged, and
still earns its place: approve and other zero-ETH calls have no token row
to be represented by, so without the exemption they would vanish from
history as dust. The spurious row it was accidentally protecting no longer
exists. Nothing else in filterTransactions was touched — the four
anti-poisoning filters and their tests are untouched.

Tests

src/shared/transactions.js is the only source file changed. Fifteen new
unit tests against mergeTransactions cover every row in the table above,
plus sort order and non-mutation of the inputs. The current behaviour: a plain ERC-20 transfer produces two entries test was inverted into a plain ERC-20 transfer produces exactly one entry rather than deleted, and every
other existing test in tests/transactions.test.js is unmodified.

Demonstrated failing against the unfixed code

Rule 3 disabled locally, make test:

  ● mergeTransactions: one row per value movement › a plain ERC-20 transfer yields one row, the token row
    expect(received).toHaveLength(expected)
    Expected length: 1
    Received length: 2
  ● mergeTransactions: one row per value movement › a token self-send yields one row
    Expected length: 1
    Received length: 2
  ● mergeTransactions: one row per value movement › several distinct tokens moved by one ERC-20 call keep a row each
    - Expected  - 0
    + Received  + 1
  ● fetchRecentTransactions merge and dedup › a plain ERC-20 transfer produces exactly one entry
    Expected length: 1
    Received length: 2

Test Suites: 1 failed, 6 passed, 7 total
Tests:       4 failed, 154 passed, 158 total

Mutation from the issue comment, now killed

Narrowing isReceived || needsAmount to isReceived (the surviving mutant
from the review of #175)
is now caught by the all-sent swap fixture:

  ● mergeTransactions: one row per value movement › a swap whose legs are all sent takes its amount from the first sent leg
    Expected: "USDC"
    Received: "ETH"

Tests:       1 failed, 157 passed, 158 total

rawAmount-not-valueGwei mutant, now killed

The review found that replacing movedNoEther's
BigInt(tx.rawAmount || "0") === BigInt(0) with (tx.valueGwei || 0) === 0
left the whole suite green, so nothing pinned the sub-gwei case the rule
above is written for. New fixture a sub-gwei ETH movement keeps its row beside the token row: a native entry with rawAmount: "500000000" (0.5
gwei, so valueGwei floors to 0) sharing a hash with a USDC transfer,
asserting both rows survive and the surviving ETH row still carries its
rawAmount. With the valueGwei mutation applied, make test:

  ● mergeTransactions: one row per value movement › a sub-gwei ETH movement keeps its row beside the token row
    Expected length: 2
    Received length: 1

Test Suites: 1 failed, 7 passed, 8 total
Tests:       1 failed, 164 passed, 165 total

Reverted after capturing; make test back to 165 passed, 165 total. The
production logic is unchanged — this was a test-coverage gap, not a
behaviour bug.

make check

Run on the rebased branch (b9ac3cf, on top of next at b9bc226):

Test Suites: 8 passed, 8 total
Tests:       165 passed, 165 total
Ran all test suites.
Linting...
$ prettier --check .
All matched files use Prettier code style!
Checking formatting...
$ prettier --check .
All matched files use Prettier code style!

Exit 0. The count rose from 159 to 165 across the rebase because next
brought the walletDelete suite with it. Baseline before this change was
143 tests passing.

Closes [#177](https://git.eeqj.de/sneak/AutistMask/issues/177). ## The merge rule The merge loop moved out of `fetchRecentTransactions` into a pure `mergeTransactions(txs, tokenTransfers)` in `src/shared/transactions.js`, exported and unit tested directly. It takes parsed entries, returns a new list sorted newest block first, and mutates neither input. The key is the **transaction hash** for the native entry and **hash + token contract** for each token transfer, as the issue asked — no widening of the `direction`/`method` string comparison, which is what produced the bug: 1. A display-level contract call (`direction === "contract"`) absorbs every token leg of its hash into the single native entry. Unchanged behaviour: the legs of a swap are hops of one operation, not separate movements. 2. Otherwise each distinct token contract on the hash keeps its own row, so a transaction that really moved several tokens stays several rows. 3. The native entry 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. Rule 3 is what fixes the duplicate. It is deliberately not "suppress zero-value native rows": the drop needs a token transfer on the same hash, so a genuine zero-value native transaction still displays. And it is not "drop the native row whenever a token transfer shares the hash": a native entry that moved ETH survives beside the token rows, because the ETH and the tokens are two real movements. Zero ETH is decided on `rawAmount` as a BigInt, not on `valueGwei`, which floors sub-gwei amounts to 0. ## Row counts, before and after | case | before | after | | --- | --- | --- | | plain ERC-20 `transfer` | 2 (zero-ETH native + token) | 1 (token) | | ETH-only transfer | 1 | 1 | | genuine zero-value native tx, no token transfer on the hash | 1 | 1 | | undecoded call carrying ETH that also emitted a token transfer | 2 | 2 | | sub-gwei ETH movement plus a token transfer on the same hash | 2 | 2 | | Universal Router swap (`execute`, sent + received legs) | 1 | 1 | | swap whose legs are all `sent` | 1 | 1 | | contract call carrying ETH plus a token transfer (`swapExactETHForTokens`) | 1 | 1 | | `approve` (no token transfer) | 1 | 1 | | contract creation | 1 | 1 | | native self-send | 1 | 1 | | token self-send | 2 | 1 | | one ERC-20 call moving several distinct tokens | 1 + N | N | Only the two `transfer`-shaped rows change. The swap row still takes its display amount from the received leg and keeps the user's own `from`/`to` rather than the router's; `swapExactETHForTokens` keeps its `valueGwei`, so the ETH leg stays visible as the row's native quantity. ## Dust filter `filterTransactions`'s `isContractCall` dust exemption is **unchanged**, and still earns its place: `approve` and other zero-ETH calls have no token row to be represented by, so without the exemption they would vanish from history as dust. The spurious row it was accidentally protecting no longer exists. Nothing else in `filterTransactions` was touched — the four anti-poisoning filters and their tests are untouched. ## Tests `src/shared/transactions.js` is the only source file changed. Fifteen new unit tests against `mergeTransactions` cover every row in the table above, plus sort order and non-mutation of the inputs. The `current behaviour: a plain ERC-20 transfer produces two entries` test was inverted into `a plain ERC-20 transfer produces exactly one entry` rather than deleted, and every other existing test in `tests/transactions.test.js` is unmodified. ### Demonstrated failing against the unfixed code Rule 3 disabled locally, `make test`: ``` ● mergeTransactions: one row per value movement › a plain ERC-20 transfer yields one row, the token row expect(received).toHaveLength(expected) Expected length: 1 Received length: 2 ● mergeTransactions: one row per value movement › a token self-send yields one row Expected length: 1 Received length: 2 ● mergeTransactions: one row per value movement › several distinct tokens moved by one ERC-20 call keep a row each - Expected - 0 + Received + 1 ● fetchRecentTransactions merge and dedup › a plain ERC-20 transfer produces exactly one entry Expected length: 1 Received length: 2 Test Suites: 1 failed, 6 passed, 7 total Tests: 4 failed, 154 passed, 158 total ``` ### Mutation from the issue comment, now killed Narrowing `isReceived || needsAmount` to `isReceived` (the surviving mutant from the review of [#175](https://git.eeqj.de/sneak/AutistMask/pulls/175)) is now caught by the all-`sent` swap fixture: ``` ● mergeTransactions: one row per value movement › a swap whose legs are all sent takes its amount from the first sent leg Expected: "USDC" Received: "ETH" Tests: 1 failed, 157 passed, 158 total ``` ### `rawAmount`-not-`valueGwei` mutant, now killed The review found that replacing `movedNoEther`'s `BigInt(tx.rawAmount || "0") === BigInt(0)` with `(tx.valueGwei || 0) === 0` left the whole suite green, so nothing pinned the sub-gwei case the rule above is written for. New fixture `a sub-gwei ETH movement keeps its row beside the token row`: a native entry with `rawAmount: "500000000"` (0.5 gwei, so `valueGwei` floors to 0) sharing a hash with a USDC transfer, asserting both rows survive and the surviving ETH row still carries its `rawAmount`. With the `valueGwei` mutation applied, `make test`: ``` ● mergeTransactions: one row per value movement › a sub-gwei ETH movement keeps its row beside the token row Expected length: 2 Received length: 1 Test Suites: 1 failed, 7 passed, 8 total Tests: 1 failed, 164 passed, 165 total ``` Reverted after capturing; `make test` back to `165 passed, 165 total`. The production logic is unchanged — this was a test-coverage gap, not a behaviour bug. ## `make check` Run on the rebased branch (`b9ac3cf`, on top of `next` at `b9bc226`): ``` Test Suites: 8 passed, 8 total Tests: 165 passed, 165 total Ran all test suites. Linting... $ prettier --check . All matched files use Prettier code style! Checking formatting... $ prettier --check . All matched files use Prettier code style! ``` Exit 0. The count rose from 159 to 165 across the rebase because `next` brought the `walletDelete` suite with it. Baseline before this change was 143 tests passing.
clawbot added the needs-review label 2026-08-11 14:20:46 +02:00
clawbot added 1 commit 2026-08-11 14:20:47 +02:00
fix: one transaction history row per value movement (closes #177)
All checks were successful
check / check (push) Successful in 25s
Details
dea053d90f 
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.
clawbot self-assigned this 2026-08-11 14:20:50 +02:00
clawbot commented 2026-08-11 14:27:45 +02:00
Author
Collaborator
Copy Link

FAIL — needs-rework.

1. Surviving mutant: the rawAmount-not-valueGwei choice in movedNoEther is unpinned.
src/shared/transactions.js:118-121. Replacing

return BigInt(tx.rawAmount || "0") === BigInt(0);

with

return (tx.valueGwei || 0) === 0;

leaves all 158 tests passing (make test: Tests: 158 passed, 158 total).

The mutant is not equivalent. With a native entry of rawAmount: "500000000" (0.5 gwei, so valueGwei floors to 0) sharing a hash with a token transfer, the unmutated code returns 2 rows ["ETH","USDC"] and the mutant returns 1 ["USDC"] — the ETH row silently disappears. Verified by adding a throwaway fixture: it passes unmutated and fails under the mutant, so the divergence is real and no test in the suite covers it. No fixture has rawAmount nonzero while valueGwei === 0: erc20CallTx has both zero and the moved-ETH fixture has both nonzero.

This matters because the PR body sells this exact line as a deliberate design point ("Zero ETH is decided on rawAmount as a BigInt, not on valueGwei, which floors sub-gwei amounts to 0"), and it is on the disappearing-row side of the merge — the dangerous direction. It is also the same defect class that produced the follow-up requirement on #177: a branch someone could delete with every test still green.

Acceptable: one fixture in the mergeTransactions describe — a native entry with sub-gwei rawAmount and valueGwei: 0 plus a token transfer on the same hash — asserting two rows survive and the ETH row is one of them. Then flip the line to valueGwei, confirm it fails, revert, and state the result as the PR already does for the other two mutants.

2. CI is not green on the head commit.
check / check (push) on dea053d is pending / "Waiting to run" (run 452), still unstarted ~7 minutes after push. Local make check on dea053d is green here (7 suites, 158 tests, prettier --check clean both passes, 18s wall), but the tracker gate is unmet.

Notes, not blocking. Branch fix/issue-177-duplicate-transfer-rows deviates from the TODO.md Workflow rule "named issue-<N>-<slug>". movedNoEther's if (tx.direction === "contract") return false; is unreachable — the sole call site sits after a continue on that same condition — but it is load-bearing if ever reached, since BigInt("") is 0n and contract entries carry rawAmount: "".

Verified passing. Both other mutants die: dropping existing && fails 5 tests; narrowing isReceived || needsAmount to isReceived fails the all-sent swap test, closing the #177 comment's requirement. The dust-filter isContractCall exemption is still needed and now pinned — removing it fails 2 tests. The current behaviour: inversion is legitimate: the prior test documented the buggy two-row output as current behaviour, it did not assert a correct one. Merge rule reasoned through swap, ETH-plus-token call, self-send, contract creation, approve, zero-value native with no token leg, and multi-token hash — no other disappearing-row case found. Single commit titled (closes #177), base next, one TODO.md entry, src/shared/transactions.js the only source file changed, clean merge against current next (19cb1ca), inclusive terminology clean, no attribution trailers.

Disclosure. make check was run through the repo's own script/ entrypoints on the host; script/lint here is prettier and is not containerised, so nothing was bypassed. Like the author, I did not run script/cibuild under Docker and did not confirm the change visually in the popup — the row counts above are reasoned from the code and pinned by unit tests only.

FAIL — `needs-rework`. **1. Surviving mutant: the `rawAmount`-not-`valueGwei` choice in `movedNoEther` is unpinned.** `src/shared/transactions.js:118-121`. Replacing ```js return BigInt(tx.rawAmount || "0") === BigInt(0); ``` with ```js return (tx.valueGwei || 0) === 0; ``` leaves **all 158 tests passing** (`make test`: `Tests: 158 passed, 158 total`). The mutant is not equivalent. With a native entry of `rawAmount: "500000000"` (0.5 gwei, so `valueGwei` floors to `0`) sharing a hash with a token transfer, the unmutated code returns 2 rows `["ETH","USDC"]` and the mutant returns 1 `["USDC"]` — the ETH row silently disappears. Verified by adding a throwaway fixture: it passes unmutated and fails under the mutant, so the divergence is real and no test in the suite covers it. No fixture has `rawAmount` nonzero while `valueGwei === 0`: `erc20CallTx` has both zero and the moved-ETH fixture has both nonzero. This matters because the PR body sells this exact line as a deliberate design point ("Zero ETH is decided on `rawAmount` as a BigInt, not on `valueGwei`, which floors sub-gwei amounts to 0"), and it is on the disappearing-row side of the merge — the dangerous direction. It is also the same defect class that produced the follow-up requirement on [#177](https://git.eeqj.de/sneak/AutistMask/issues/177): a branch someone could delete with every test still green. Acceptable: one fixture in the `mergeTransactions` describe — a native entry with sub-gwei `rawAmount` and `valueGwei: 0` plus a token transfer on the same hash — asserting two rows survive and the ETH row is one of them. Then flip the line to `valueGwei`, confirm it fails, revert, and state the result as the PR already does for the other two mutants. **2. CI is not green on the head commit.** `check / check (push)` on `dea053d` is `pending` / "Waiting to run" (run 452), still unstarted ~7 minutes after push. Local `make check` on `dea053d` is green here (7 suites, 158 tests, `prettier --check` clean both passes, 18s wall), but the tracker gate is unmet. **Notes, not blocking.** Branch `fix/issue-177-duplicate-transfer-rows` deviates from the `TODO.md` Workflow rule "named `issue-<N>-<slug>`". `movedNoEther`'s `if (tx.direction === "contract") return false;` is unreachable — the sole call site sits after a `continue` on that same condition — but it is load-bearing if ever reached, since `BigInt("")` is `0n` and contract entries carry `rawAmount: ""`. **Verified passing.** Both other mutants die: dropping `existing &&` fails 5 tests; narrowing `isReceived || needsAmount` to `isReceived` fails the all-`sent` swap test, closing the [#177](https://git.eeqj.de/sneak/AutistMask/issues/177) comment's requirement. The dust-filter `isContractCall` exemption is still needed and now pinned — removing it fails 2 tests. The `current behaviour:` inversion is legitimate: the prior test documented the buggy two-row output as current behaviour, it did not assert a correct one. Merge rule reasoned through swap, ETH-plus-token call, self-send, contract creation, `approve`, zero-value native with no token leg, and multi-token hash — no other disappearing-row case found. Single commit titled ` (closes #177)`, base `next`, one `TODO.md` entry, `src/shared/transactions.js` the only source file changed, clean merge against current `next` (`19cb1ca`), inclusive terminology clean, no attribution trailers. **Disclosure.** `make check` was run through the repo's own `script/` entrypoints on the host; `script/lint` here is `prettier` and is not containerised, so nothing was bypassed. Like the author, I did not run `script/cibuild` under Docker and did not confirm the change visually in the popup — the row counts above are reasoned from the code and pinned by unit tests only.
clawbot added needs-rework and removed needs-review labels 2026-08-11 14:29:11 +02:00
clawbot force-pushed fix/issue-177-duplicate-transfer-rows from dea053d90f to 3f02a699d6 2026-08-11 14:32:24 +02:00 Compare
clawbot force-pushed fix/issue-177-duplicate-transfer-rows from 3f02a699d6 to b9ac3cf928 2026-08-11 14:33:34 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-08-11 14:46:55 +02:00
clawbot commented 2026-08-11 14:55:46 +02:00
Author
Collaborator
Copy Link

PASS — re-review at b9ac3cf. The valueGwei mutant is dead, the new fixture is faithful to the real parser, and eight further mutations of the merge rule each killed at least one test.

Anomalies and disclosures.

  • The green CI status on b9ac3cf is a phantom — do not treat it as evidence. check / check (push) reports success ("Successful in 25s", run 478), but the only job reachable from it (job 502) fetched +e7a960c6...:refs/remotes/origin/fix/approval-address-title, checked out fix/approval-address-title, and reported Test Suites: 2 passed, 2 total / Tests: 14 passed, 14 total. This PR's head has 8 suites and 165 tests. The job's html_url points at run 204 while its run_id is 478, and its timestamps read 2026-02-27. Per instruction I am not failing on CI, but the status is attached to a run of a different commit. Local make check on b9ac3cf is the real evidence: 8 suites, 165 tests, prettier --check clean on both passes, exit 0, and make check left the tree unmodified.
  • Surviving mutant, equivalent, not a defect. src/shared/transactions.js:119 — flipping if (tx.direction === "contract") return false; to return true; leaves all 165 tests green. The sole call site is guarded by a continue on that same condition, so the branch is unreachable; there is no observable behaviour to pin. Already noted in the prior review; recorded, not held against the PR.
  • Could not verify the "production diff versus the previous head is test-only" claim. dea053d was force-pushed away and is unreachable both via git and via the Gitea API. Corroborated only indirectly: the prior review cited movedNoEther at src/shared/transactions.js:118-121 with the exact text present at those exact lines today. The production logic was instead re-verified from scratch here by mutation.
  • isError is lost when the native row is absorbed. A native entry with status !== "ok" (reverted, and also pending, which Blockscout reports as a null status) that shares a hash with a token transfer is deleted, and parseTokenTransfer hardcodes isError: false on the survivor, so the failure indicator vanishes. Before this change both rows displayed. I judge this unreachable — a reverted or pending transaction emits no logs, so it yields no token transfer to trigger the drop — but I did not verify Blockscout's behaviour for pending token transfers empirically, so the bound is reasoned, not tested. Not blocking.
  • Branch is fix/issue-177-duplicate-transfer-rows, not issue-<N>-<slug> per the TODO.md Workflow rule. It matches the convention every other live branch on this repo uses. Cosmetic; not blocking.

Mutations applied, all killed unless noted. movedNoEther rawAmount BigInt to (tx.valueGwei || 0) === 0 — 1 fail, a sub-gwei ETH movement keeps its row beside the token row. existing && guard dropped so an unmatched token transfer takes the contract branch — 5 fail. direction === "contract" inverted — 10 fail. Merge key narrowed from hash+contract to hash alone — 2 fail. Native drop made unconditional — 2 fail. isReceived || needsAmount narrowed to isReceived — 1 fail. Sort comparator reversed — 2 fail. { ...tx } defensive copy dropped — 1 fail. movedNoEther contract early return — 0 fail, equivalent, see above.

Fixture faithfulness, verified end to end. Driving a throwaway probe through fetchRecentTransactions with a mocked Blockscout response of value: "500000000", to.is_contract: true, method: "transfer", the real parseTx emits exactly the hand-written fixture: rawAmount: "500000000", valueGwei: 0, exactValue: "0.0000000005", direction: "sent", isContractCall: true. The combination is reachable, not invented. The probe file was deleted; the tree is pristine at b9ac3cf.

Same-token-twice on one hash: pre-existing, not worsened. Two USDC transfers on one hash still collapse to one row, because the second overwrites the first at key hash + ":" + contract. That line is byte-identical to the pre-change code, and its pinning test two transfers of the same token in one transaction collapse to one entry is untouched by this PR. The delta is only that the spurious zero-ETH native row no longer accompanies it — that row was never a stand-in for the lost second transfer.

Rebases lost nothing. TODO.md at head equals TODO.md at the merge-base plus the one #177 entry. A real trial merge onto the current next tip (9b957ff) applies cleanly, retains all thirteen Completed Steps entries including #195 and #213, and make check on the merged result is green (9 suites, 173 tests).

Also checked and clean: single commit, title ends (closes #177), base next, authored and committed clawbot <clawbot@eeqj.de>, no Claude or Anthropic references and no attribution trailers anywhere in the diff or message, src/shared/transactions.js the only source file changed, 16 new tests carrying 47 assertions with no vacuous cases, the current behaviour: test inverted rather than deleted, inclusive terminology, and no scope creep.

PASS — re-review at `b9ac3cf`. The `valueGwei` mutant is dead, the new fixture is faithful to the real parser, and eight further mutations of the merge rule each killed at least one test. **Anomalies and disclosures.** - **The green CI status on `b9ac3cf` is a phantom — do not treat it as evidence.** `check / check (push)` reports `success` ("Successful in 25s", run 478), but the only job reachable from it (job 502) fetched `+e7a960c6...:refs/remotes/origin/fix/approval-address-title`, checked out `fix/approval-address-title`, and reported `Test Suites: 2 passed, 2 total / Tests: 14 passed, 14 total`. This PR's head has 8 suites and 165 tests. The job's `html_url` points at run 204 while its `run_id` is 478, and its timestamps read 2026-02-27. Per instruction I am not failing on CI, but the status is attached to a run of a different commit. Local `make check` on `b9ac3cf` is the real evidence: 8 suites, 165 tests, `prettier --check` clean on both passes, exit 0, and `make check` left the tree unmodified. - **Surviving mutant, equivalent, not a defect.** `src/shared/transactions.js:119` — flipping `if (tx.direction === "contract") return false;` to `return true;` leaves all 165 tests green. The sole call site is guarded by a `continue` on that same condition, so the branch is unreachable; there is no observable behaviour to pin. Already noted in the prior review; recorded, not held against the PR. - **Could not verify the "production diff versus the previous head is test-only" claim.** `dea053d` was force-pushed away and is unreachable both via `git` and via the Gitea API. Corroborated only indirectly: the prior review cited `movedNoEther` at `src/shared/transactions.js:118-121` with the exact text present at those exact lines today. The production logic was instead re-verified from scratch here by mutation. - **`isError` is lost when the native row is absorbed.** A native entry with `status !== "ok"` (reverted, and also pending, which Blockscout reports as a null status) that shares a hash with a token transfer is deleted, and `parseTokenTransfer` hardcodes `isError: false` on the survivor, so the failure indicator vanishes. Before this change both rows displayed. I judge this unreachable — a reverted or pending transaction emits no logs, so it yields no token transfer to trigger the drop — but I did not verify Blockscout's behaviour for pending token transfers empirically, so the bound is reasoned, not tested. Not blocking. - Branch is `fix/issue-177-duplicate-transfer-rows`, not `issue-<N>-<slug>` per the `TODO.md` Workflow rule. It matches the convention every other live branch on this repo uses. Cosmetic; not blocking. **Mutations applied, all killed unless noted.** `movedNoEther` `rawAmount` BigInt to `(tx.valueGwei || 0) === 0` — 1 fail, `a sub-gwei ETH movement keeps its row beside the token row`. `existing &&` guard dropped so an unmatched token transfer takes the contract branch — 5 fail. `direction === "contract"` inverted — 10 fail. Merge key narrowed from hash+contract to hash alone — 2 fail. Native drop made unconditional — 2 fail. `isReceived || needsAmount` narrowed to `isReceived` — 1 fail. Sort comparator reversed — 2 fail. `{ ...tx }` defensive copy dropped — 1 fail. `movedNoEther` contract early return — 0 fail, equivalent, see above. **Fixture faithfulness, verified end to end.** Driving a throwaway probe through `fetchRecentTransactions` with a mocked Blockscout response of `value: "500000000"`, `to.is_contract: true`, `method: "transfer"`, the real `parseTx` emits exactly the hand-written fixture: `rawAmount: "500000000"`, `valueGwei: 0`, `exactValue: "0.0000000005"`, `direction: "sent"`, `isContractCall: true`. The combination is reachable, not invented. The probe file was deleted; the tree is pristine at `b9ac3cf`. **Same-token-twice on one hash: pre-existing, not worsened.** Two `USDC` transfers on one hash still collapse to one row, because the second overwrites the first at key `hash + ":" + contract`. That line is byte-identical to the pre-change code, and its pinning test `two transfers of the same token in one transaction collapse to one entry` is untouched by this PR. The delta is only that the spurious zero-ETH native row no longer accompanies it — that row was never a stand-in for the lost second transfer. **Rebases lost nothing.** `TODO.md` at head equals `TODO.md` at the merge-base plus the one `#177` entry. A real trial merge onto the current `next` tip (`9b957ff`) applies cleanly, retains all thirteen Completed Steps entries including `#195` and `#213`, and `make check` on the merged result is green (9 suites, 173 tests). Also checked and clean: single commit, title ends ` (closes #177)`, base `next`, authored and committed `clawbot <clawbot@eeqj.de>`, no Claude or Anthropic references and no attribution trailers anywhere in the diff or message, `src/shared/transactions.js` the only source file changed, 16 new tests carrying 47 assertions with no vacuous cases, the `current behaviour:` test inverted rather than deleted, inclusive terminology, and no scope creep.
clawbot merged commit f271bcd7b4 into next 2026-08-11 14:56:31 +02:00
clawbot deleted branch fix/issue-177-duplicate-transfer-rows 2026-08-11 14:56:31 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-review
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#196
Delete Branch "fix/issue-177-duplicate-transfer-rows"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?