feat: add TransferIndex for unique transfer identification by DNK90 · Pull Request #79 · fystack/multichain-indexer

DNK90 · 2026-03-20T03:36:21Z

Closes #78

Summary

Add TransferIndex and BlockHash fields to types.Transaction for unique transfer identification within transactions. Enables proper dedup for batch transfers, reorg-aware NATS delivery, and stable consumer upsert keys.

Two identity levels

Identity	Key	Used for
Transfer Identity (logical)	`NetworkId + TxHash + TransferIndex + Direction`	Downstream consumer upsert key
Event Instance Identity (delivery)	`NetworkId + TxHash + BlockHash + TransferIndex + Direction`	NATS idempotency key (`Hash()`)

TODO Checklist

Phase 1: Add fields + parse positional data

1.1 Add TransferIndex + BlockHash to types.Transaction
1.2 Add TransactionIndex to Txn
1.3 Add hexIndexToDecimal helper

Phase 2: Set TransferIndex in extraction functions

2.1 Set TransferIndex in ExtractTransfers (native + ERC20)
2.2 Set TransferIndex in ExtractSafeTransfers
2.3 Set TransferIndex in ExtractInternalTransfers + walkCallTrace

Phase 3: Update call sites

3.1 Set BlockHash on all transfers in convertBlock (post-extraction loop)
3.2 Add TransferIndex to DedupTransfers key
3.3 Add BlockHash + TransferIndex to Transaction.Hash() (NATS idempotency key)

Phase 4: Tests — Unit (16 tests)

Phase 4: Tests — Worker-level (3 tests)

TestEmitBlock_TwoWayIndexing_UniqueHashes
TestEmitBlock_DuplicateAmountTransfers_UniqueHashes
TestEmitBlock_MixedSources_UniqueHashes

Phase 4: Tests — Integration (2 tests, need TRACE_RPC_URL)

TestEndToEnd_BatchSendNative_TransferIndex
TestEndToEnd_SafeTx_TransferIndex

Implementation Plan

See implementation plan comment on the issue.

- Add TransferIndex + BlockHash fields to types.Transaction - Add TransactionIndex to evm.Txn (parsed from RPC response) - Add hexIndexToDecimal helper for hex→decimal index conversion - Set TransferIndex in ExtractTransfers (native: txIndex, ERC20: txIndex:logIndex) - Set TransferIndex in ExtractSafeTransfers (txIndex:safe:0) - Set TransferIndex in ExtractInternalTransfers (txIndex:trace:N with DFS counter) - Set BlockHash on all transfers in convertBlock (post-extraction loop) - Add TransferIndex to DedupTransfers key - Update Transaction.Hash() with BlockHash + TransferIndex for NATS idempotency - EVM: NetworkId|TxHash|BlockHash|TransferIndex|Direction - Non-EVM fallback: NetworkId|TxHash|BlockHash|From|To|Timestamp|Direction Closes #78

transfer_index_test.go (10 tests): - hexIndexToDecimal: normal, edge cases, malformed input - ExtractTransfers: native (txIndex), ERC20 log (txIndex:logIndex), ERC20 input (txIndex) - ExtractSafeTransfers: txIndex:safe:0 format - ExtractInternalTransfers: sequential trace counters, DFS ordering - DedupTransfers: same amount different index (not deduped), same index (deduped) hash_test.go (6 tests): - Different TransferIndex → different Hash - Two-way indexing in/out → different Hash - Same position → same Hash (idempotent) - Non-EVM fallback: different addresses → different Hash - Non-EVM fallback: same everything → same Hash - Reorg (different BlockHash) → different Hash

transfer_index_integration_test.go (2 tests, need TRACE_RPC_URL): - BatchSendNative: 12 internal transfers each get unique sequential TransferIndex - SafeTx: trace transfers use trace format, Safe heuristic uses safe:0 format emit_test.go (3 tests): - TwoWayIndexing: in/out events for same transfer produce different Hash() - DuplicateAmountTransfers: same amount to same address at different positions produce different Hash() — NATS delivers both - MixedSources: trace/safe/erc20/native transfers in same tx all have unique Hash()

DNK90 · 2026-03-20T03:59:42Z

Test Results — All Pass

Unit Tests (19 tests)

=== RUN   TestHexIndexToDecimal (8 sub-tests)                          --- PASS
=== RUN   TestExtractTransfers_TransferIndex_NativeTransfer            --- PASS
=== RUN   TestExtractTransfers_TransferIndex_ERC20Log                  --- PASS
=== RUN   TestExtractTransfers_TransferIndex_ERC20Input                --- PASS
=== RUN   TestExtractSafeTransfers_TransferIndex                       --- PASS
=== RUN   TestExtractInternalTransfers_TransferIndex                   --- PASS
=== RUN   TestExtractInternalTransfers_TransferIndex_Ordering          --- PASS
=== RUN   TestDedupTransfers_SameAmountDifferentIndex                  --- PASS
=== RUN   TestDedupTransfers_SameIndex                                 --- PASS
=== RUN   TestTransactionHash_IncludesTransferIndex                    --- PASS
=== RUN   TestTransactionHash_TwoWayIndexing_DirectionDiffers          --- PASS
=== RUN   TestTransactionHash_SamePositionSameHash                     --- PASS
=== RUN   TestTransactionHash_NoTransferIndex_FallbackBehavior         --- PASS
=== RUN   TestTransactionHash_NoTransferIndex_SameAddresses_SameHash   --- PASS
=== RUN   TestTransactionHash_ReorgProducesDifferentHash               --- PASS
=== RUN   TestEmitBlock_TwoWayIndexing_UniqueHashes                    --- PASS
=== RUN   TestEmitBlock_DuplicateAmountTransfers_UniqueHashes          --- PASS
=== RUN   TestEmitBlock_MixedSources_UniqueHashes                      --- PASS

Integration Tests (2 tests, need RPC_URL)

=== RUN   TestEndToEnd_BatchSendNative_TransferIndex                   --- PASS
=== RUN   TestEndToEnd_SafeTx_TransferIndex                            --- PASS

Run with:

RPC_URL=<your-debug-rpc-url> go test ./internal/rpc/evm/ -v -run "TestEndToEnd_.*TransferIndex" -count=1

Optionally set RPC_ORIGIN and RPC_REFERER if the RPC requires custom headers.

Test Coverage for Changed Code

File	Function	Coverage
utils.go	`hexIndexToDecimal`	100%
tx.go	`ExtractTransfers`	100%
tx.go	`parseERC20Logs`	87.5%
tx.go	`parseERC20Input`	61.5% (pre-existing — transferFrom path untested before this PR)
types.go	`parseERC20Transfers`	81.8%
safe.go	`ExtractSafeTransfers`	100%
trace.go	`ExtractInternalTransfers`	100%
trace.go	`walkCallTrace`	93.3%
evm.go	`convertBlock`	88.5% (includes BlockHash propagation loop)
types.go	`Hash()`	100% (both EVM + non-EVM branches)
tx.go	`DedupTransfers`	tested cross-package*

*DedupTransfers is in pkg/common/utils/ which has no test file. Verified by TestDedupTransfers_SameAmountDifferentIndex + TestDedupTransfers_SameIndex in the evm package — Go coverage doesn't track cross-package calls.

anhthii · 2026-03-20T10:51:34Z

Summary

Adds TransferIndex and BlockHash fields to types.Transaction for unique transfer identification within transactions. The implementation cleanly separates extraction (which produces TransferIndex) from block-level concerns (BlockHash is set post-extraction in convertBlock). The Hash() function is updated to produce position-aware NATS idempotency keys with a sensible fallback for non-EVM chains.

Review Checklist

Code correctness — no logic errors or bugs
Error handling — fail-fast, no silent swallowing
Security — no secrets, injection, or SSRF risks
Database — n/a (no migrations)
Concurrency — no race conditions or goroutine leaks
API contract — request/response types are correct and consistent
Code reusability — no unnecessary duplication
Tests — comprehensive coverage (16 unit + 3 worker-level + 2 integration)

Findings

Priority	File	Issue
📘	`pkg/common/types/types.go`	`Hash()` non-EVM fallback is not identical to old behavior — `BlockHash` field insertion changes all existing hashes

Verdict

correct — clean implementation with thorough tests. One informational note below about the Hash() backward compatibility, but it's low-impact and likely acceptable.

anhthii

See inline comments.

anhthii · 2026-03-20T10:51:44Z

pkg/common/types/types.go

 	builder.WriteString(t.TxHash)
 	builder.WriteByte('|')
-	builder.WriteString(t.FromAddress)
+	builder.WriteString(t.BlockHash)


📘 note — Non-EVM hash backward compatibility

The comment says "preserve current hash behavior" for the non-EVM fallback, but the old Hash() produced NetworkId|TxHash|From|To|Timestamp|Direction while the new one produces NetworkId|TxHash|BlockHash|From|To|Timestamp|Direction. For chains that don't set BlockHash (e.g. TRON), this inserts an empty string between two | delimiters, changing every existing hash.

This means all NATS idempotency keys for non-EVM chains will change on deployment. In practice this is low-risk since NATS dedup windows are short, but it could cause a brief window of duplicate deliveries for in-flight messages. Worth noting in deployment docs or release notes so operators are aware.

No code change needed — just calling it out as a known behavioral change.

anhthii · 2026-03-20T10:51:49Z

Summary

This PR adds TransferIndex and BlockHash fields to transactions for unique transfer identification within EVM transactions. It modifies Transaction.Hash() to use positional identity (TransferIndex) instead of from/to/timestamp, enabling correct NATS dedup for batch transactions that produce multiple transfers with identical from/to/amount. Non-EVM chains fall back to the previous hash behavior.

Review Checklist

Code correctness — hexIndexToDecimal silent fallback to "0" creates collision risk
Error handling — hexIndexToDecimal swallows parse errors without logging
Security — no secrets, injection, or SSRF risks
Database — no migrations needed
Concurrency — no race conditions or goroutine leaks
API contract — JSON fields are additive (backward compatible)
Code reusability — no unnecessary duplication
Tests — good coverage with unit and integration tests

Findings

Priority	File	Issue
⚠️	`internal/rpc/evm/utils.go:18-23`	`hexIndexToDecimal` silently falls back to `"0"` on malformed input, creating hash collision risk
⚠️	`pkg/common/types/types.go:106`	`Hash()` uses `BlockHash` which is empty string for TRON, TON, Bitcoin, Aptos — changes hash output for all non-EVM chains
📘	`internal/rpc/evm/utils.go:18-23`	No logging when `hexIndexToDecimal` encounters malformed hex — silent failure
📘	`internal/worker/base.go:215,231`	`EmitTransaction` errors are discarded with `_ =` (pre-existing, not introduced in this diff)

Verdict

needs attention — the hexIndexToDecimal fallback and missing BlockHash on non-EVM chains need to be addressed before merge.

anhthii

See inline comments.

internal/rpc/evm/utils.go

pkg/common/types/types.go

internal/rpc/evm/types.go

internal/rpc/evm/safe.go

…nt hash collisions

DNK90 added 4 commits March 20, 2026 10:35

feat: add TransferIndex for unique transfer identification

051b9ec

DNK90 changed the title ~~[WIP] feat: add TransferIndex for unique transfer identification~~ feat: add TransferIndex for unique transfer identification Mar 20, 2026

DNK90 marked this pull request as ready for review March 20, 2026 04:01

chore: rename TRACE_RPC_URL/ORIGIN/REFERER to RPC_URL/ORIGIN/REFERER

07ba490

anhthii reviewed Mar 20, 2026

View reviewed changes

internal/rpc/evm/utils.go Show resolved Hide resolved

pkg/common/types/types.go Show resolved Hide resolved

internal/rpc/evm/types.go Show resolved Hide resolved

internal/rpc/evm/safe.go Show resolved Hide resolved

fix: return raw string on malformed hex in hexIndexToDecimal to preve…

9131d39

…nt hash collisions

anhthii merged commit f586d7c into main Mar 20, 2026
0 of 2 checks passed

anhthii deleted the feat/transfer-index branch March 20, 2026 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add TransferIndex for unique transfer identification#79

feat: add TransferIndex for unique transfer identification#79
anhthii merged 6 commits intomainfrom
feat/transfer-index

DNK90 commented Mar 20, 2026 •

edited

Loading

Uh oh!

DNK90 commented Mar 20, 2026 •

edited

Loading

Uh oh!

anhthii commented Mar 20, 2026

Uh oh!

anhthii left a comment

Uh oh!

anhthii Mar 20, 2026

Uh oh!

anhthii commented Mar 20, 2026

Uh oh!

anhthii left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DNK90 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Two identity levels

TODO Checklist

Phase 1: Add fields + parse positional data

Phase 2: Set TransferIndex in extraction functions

Phase 3: Update call sites

Phase 4: Tests — Unit (16 tests)

Phase 4: Tests — Worker-level (3 tests)

Phase 4: Tests — Integration (2 tests, need TRACE_RPC_URL)

Implementation Plan

Uh oh!

DNK90 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results — All Pass

Unit Tests (19 tests)

Integration Tests (2 tests, need RPC_URL)

Test Coverage for Changed Code

Uh oh!

anhthii commented Mar 20, 2026

Summary

Review Checklist

Findings

Verdict

Uh oh!

anhthii left a comment

Choose a reason for hiding this comment

Uh oh!

anhthii Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

anhthii commented Mar 20, 2026

Summary

Review Checklist

Findings

Verdict

Uh oh!

anhthii left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DNK90 commented Mar 20, 2026 •

edited

Loading

DNK90 commented Mar 20, 2026 •

edited

Loading