refactor(rpc): trace block reads

[EgeCaner]

What

Reworks the RPC trace path (v9 + v10) so tracing a finalised block reads only what it
actually needs, and so a latest/l1_accepted tag cannot resolve to two different
blocks within one call.

Receipts off the hot path

traceFinalisedBlock used to read the whole block — header, transactions and
receipts — then hand all of it to the VM path, which only ever uses the transactions.
Receipts are needed solely by the feeder-gateway branch, which exists for
Starknet ≤ 0.13.2 blocks and never runs on a modern chain. That branch is now its own
function (fetchTracesFromFeederGateway) and is the only caller that reads receipts,
via a new paired accessor:

• core.GetTransactionsAndReceiptsByBlockNumber / Blockchain.TransactionsAndReceiptsByBlockNumber
  — both live under the same key, so the legacy path still pays one block read, not two.
• The VM path calls TransactionsByBlockNumber and decodes no receipts at all.

So the common case drops a full receipt-slice CBOR decode per traced block (plus the
header decode, since the header is now passed down rather than re-read).

Block ids pinned once

TraceBlockTransactions resolves the block id to a header up front and passes that
header down; every subsequent read goes by header.Number. Previously the tag was
re-resolved per read, so a block stored between two reads could make one call mix data
from two different blocks. Same treatment for the pre-confirmed branch.

Cache moved up, keyed on felt.Felt

The trace cache is now checked and populated in one place (traceFinalisedBlock, right
after the header read) instead of inside each branch. The rpccore.TraceCacheKey
wrapper struct is gone; the cache is keyed directly on felt.Felt.

Feeder-gateway blocks are cached with an empty (non-nil) InitialReads, since the
gateway never supplies them — an empty set is the final answer for those blocks, so a
later call that does pass the flag is still served from the cache.

Error classification

Failed reads on the trace path reported "not found" unconditionally, hiding real DB and
decode failures behind ErrTxnHashNotFound / ErrBlockNotFound. Header, pre-confirmed
chain, transaction, and parent-state reads now return not-found only for
db.ErrKeyNotFound (plus pending.ErrPreConfirmedNotFound for the pre-confirmed
chain) and surface anything else as ErrInternal, matching what stateByBlockID
already did.

Notes

• v9 and v10 are kept in lockstep; v8 only picks up the felt.Felt cache key.
• x-execution-steps is still absent on cache hits (pre-existing; noted separately).
• Call / SimulateTransactions still resolve the block id twice (state, then header)
  and can straddle a store — same class of race, left for a follow-up.

[github-actions]

Reviewing PR #3952

• Gather context (diff, CLAUDE.md, related files)
• Review core/blockchain accessor changes
• Review rpc/v9 + v10 trace path (cache, block id pinning, error classification)
• Review tests
• Post findings

View job run · branch refactor/rpc/trace-block-reads

[codecov]

Codecov Report

❌ Patch coverage is 64.24242% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.53%. Comparing base (c5a1917) to head (8d92dbf).

Files with missing lines	Patch %	Lines
rpc/v10/trace.go	65.21%	21 Missing and 3 partials ⚠️
rpc/v9/trace.go	63.33%	18 Missing and 4 partials ⚠️
core/accessors.go	57.14%	5 Missing and 1 partial ⚠️
core/block_transaction_serializer.go	50.00%	2 Missing and 2 partials ⚠️
blockchain/blockchain.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3952      +/-   ##
==========================================
- Coverage   74.75%   74.53%   -0.22%     
==========================================
  Files         461      461              
  Lines       40928    40959      +31     
==========================================
- Hits        30594    30528      -66     
- Misses       8230     8324      +94     
- Partials     2104     2107       +3     

Flag	Coverage Δ
jsonv2	77.23% <65.43%> (-0.14%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude finished @EgeCaner's task in 10m 4s —— View job

---

Review of PR #3952

• Gather context (diff, related files)
• Review core/blockchain accessor changes
• Review rpc/v9 + v10 trace path (cache, block id pinning, error classification)
• Review tests
• Post findings

The refactor itself is sound — pinning the block id to a header up front genuinely closes the latest/l1_accepted straddle, dropping receipts from the VM path is a real win, and hoisting the cache into one place (keyed on felt.Felt) reads much better than the per-branch version. Four findings, three of them worth acting on.

Important

1. v9 TraceTransaction still masks internal errors — rpc/v9/trace.go:91-95 (inline). findAndTraceFinalisedTransaction now returns ErrInternal for real DB failures, but this caller squashes it back to ErrTxnHashNotFound and drops the cause. v10 was fixed in the same commit; v9 was not, so the error-classification work is unreachable for starknet_traceTransaction on v9. rpc/v9/trace_test.go:335-345 asserts the masked result, so the test pins the old behaviour too.

2. blockTraces[txIndex] is indexed with an index from a different read — rpc/v10/trace.go:284, rpc/v9/trace.go:314 (inline). The old path fetched the block by hash and derived the index from that same block's transaction list, so index and slice were consistent by construction. Going through BlockNumberAndIndexByTxHash → BlockHeaderByNumber(number) means a reorg between the two reads can yield either an index-out-of-range panic or the trace of an unrelated transaction returned for the requested hash. A bounds + TransactionHash equality guard fixes it in three lines (and removes the unchecked *TraceRoot deref).

3. GetTransactionsAndReceiptsByBlockNumber adds a full blob copy to a hot path — core/accessors.go:516-538 (inline). BlockTransactionsBucket.Get runs extractAll, which does slices.Clone(b.Data) because it escapes the database.Get callback; the two partial buckets it replaces decode in-callback and never clone. So GetBlockByNumber now trades one key lookup for a memcpy of the entire tx+receipt payload. A combined partial serializer (extractAllTransactionsAndReceipts) gets the single read and keeps it zero-copy.

Nit

4. Cached *InitialReads is aliased into every response — rpc/v10/trace.go:382-390 (inline). Harmless today, latent later.

Checked and fine

• new(adaptVMInitialReads(...)) at rpc/v10/trace.go:469 is valid under go 1.26.0.
• felt.TransactionHash implements UnmarshalJSON (core/felt/hash.go:133), so the handler signature changes are safe at the JSON-RPC boundary.
• cacheKey := *header.Hash can't nil-deref: both callers are guarded (id.IsPreConfirmed() early return, and BlockHeaderByNumber always yields a hash).
• Error wrapping in the new accessor keeps errors.Is(err, db.ErrKeyNotFound) working through fmt.Errorf("...: %w").
• The TransactionsAndReceiptsByBlockNumber read label is a dynamic Prometheus label, no registration needed.
• Receipts really are off the VM path — TestTraceBlockTransactions only mocks TransactionsByBlockNumber, so gomock would fail on an unexpected receipt read.

go build ./rpc/... ./core/... ./blockchain/... and go vet ./rpc/v9/... ./rpc/v10/... ./core/... both pass locally. I did not run the trace tests — they need the Rust VM built (vm/rust/target/release is absent in this checkout), so I relied on CI for that.
· branch refactor/rpc/trace-block-reads

[rodrodros]

Thank you for detail attention and updating the types 🙏

[github-actions]

Claude finished @EgeCaner's task in 5m 59s —— View job

---

Re-review of PR #3952

• Verify finding 1 (v9 TraceTransaction error masking)
• Verify finding 2 (blockTraces[txIndex] index/slice mismatch)
• Verify finding 3 (blob clone in the combined accessor)
• Review the new commits for fresh issues
• Build + vet
• Post findings

All three findings from the previous round are addressed, and the accessor fix ended up better than the status quo on main. Two nits left, both about the new code's own coverage.

Previous findings — resolved

1. v9 error masking — fixed in eb7fbb0b8. rpc/v9/trace.go:91-97 now returns the original err instead of squashing it to ErrTxnHashNotFound, and the "other error" subtest (rpc/v9/trace_test.go:334-345) asserts ErrInternal.CloneWithData(...). v9 and v10 TraceTransaction are now byte-for-byte the same shape. The err != rpccore.ErrTxnHashNotFound identity comparison is still sound: every not-found path returns that package var directly, and CloneWithData always yields a fresh pointer.

2. blockTraces[txIndex] mismatch — guarded in eb7fbb0b8 at rpc/v10/trace.go:285-289 and rpc/v9/trace.go:317-321. The bounds check plus TransactionHash equality means a reorg between the index lookup and the block read can no longer return an unrelated transaction's trace or panic. Correctly applied to both versions.

3. Blob clone — fixed in d4b7b8b55, and this is now a net win rather than a wash. extractAllTransactionsAndReceipts (core/block_transaction_serializer.go:115-133) runs inside the database.Get callback (db/typed/partial/partial.go:32-43), so no slices.Clone of the payload, and GetBlockByNumber (core/accessors.go:580) drops from two key lookups + two Indexes decodes to one of each. Error wrapping preserves errors.Is(err, db.ErrKeyNotFound) through the %w. The new bucket reuses BlockTransactionsBucket.Bucket, so no new key prefix and nothing to migrate.

4. Aliased *InitialReads — your call, and the reasoning holds: with the hit condition being hit && (!returnInitialReads || cachedResponse.InitialReads != nil), caching nil would send every flagged request back to the gateway, and the hit path hands out the cached value anyway, so copying only on the miss path would make the aliasing inconsistent rather than remove it. "Entries are immutable, replaced whole" is the right invariant.

Nits (new)

1. No test for BlockTransactionsAllTransactionsAndReceiptsPartialSerializer — core/block_transaction_serializer.go:214 (inline). It's the only partial serializer in the file without a round-trip test, and it's now the sole decode path for every block read. Nine lines with the existing assertPartialSerializer helper. Worth it specifically because this one decodes both sections in a single pass — an offset mistake between the two halves is invisible to the single-half tests.

2. Guard trusts the slice it's guarding — rpc/v10/trace.go:285-289 (inline). TransactionHash.Equal nil-derefs (felt.Felt.Equal forwards to (*fp.Element).Equal, which dereferences), and *TraceRoot on line 291 is still unchecked. Neither can fire today — both producers populate both fields — so this is one line of defence, not a bug. The guard branch also has no test in either version, which is part of the 62–66% patch coverage on the two trace files.

Also checked, no issue

• fetchTracesFromFeederGateway reads transactions+receipts by header.Number while fetching the gateway trace by header.Hash — consistent, since the header pins both.
• traceFinalisedBlock can surface ErrBlockNotFound out of starknet_traceTransaction if the index entry outlives the block body. Pre-existing (StateAtBlockHash could already do this), not introduced here.
• v9's traceFinalisedBlock caches on both branches and keys on *header.Hash; felt.Felt is a [4]uint64, so it's a valid comparable map key.
• v9 findAndTraceInPreConfirmed indexes entry.Block.Transactions[txIndex] with an index from that same entry — no cross-read mismatch there.

go build ./... succeeds for every package except cmd/juno, which fails only at link time on the absent Rust static libs (-ljuno_starknet_rs, -ljuno_starknet_compiler_rs) — an environment gap, not a code problem. go vet ./core/... ./rpc/v9/... ./rpc/v10/... ./blockchain/... is clean. I could not run the test suites in this environment (go test isn't in my allowed tools here), so the trace and serializer tests are CI's word.
· branch refactor/rpc/trace-block-reads

[github-actions]

Claude finished @EgeCaner's task in 2m 54s —— View job

---

Re-review of PR #3952

• Diff the new commit against the last reviewed head (eb7fbb0b8)
• Verify the new serializer test actually covers the combined decode path
• Check for fresh issues
• Build + vet
• Post findings

One new commit since the last round: 8d92dbf06, ten lines of test. No new findings — no inline comments this round.

Verified

8d92dbf06 closes the last actionable item. The subtest at core/block_transaction_test.go:152-161 runs BlockTransactionsAllTransactionsAndReceiptsPartialSerializer through assertPartialSerializer against the same serialised blob as its siblings, asserting core.TransactionsAndReceipts{Transactions: transactions, Receipts: receipts} over 100 transactions. That's exactly the case the single-half tests couldn't reach: extractAllTransactionsAndReceipts (core/block_transaction_serializer.go:114-133) decodes both sections from one Indexes decode, so an offset mistake between the two halves — receipts read against the transaction index range, or vice versa — now fails the suite. Every partial serializer in the file is covered.

The serializer is on the sole decode path for GetBlockByNumber (core/accessors.go:580 → GetTransactionsAndReceiptsByBlockNumber at line 518), so this test now guards every block read, starknet_getBlockWithTxs, WithReceipts, and sync.

State of the earlier findings

#	Finding	Status
1	v9 TraceTransaction masked ErrInternal	fixed in eb7fbb0b8
2	blockTraces[txIndex] indexed from a different read	guarded in eb7fbb0b8 (rpc/v10/trace.go:285-288, rpc/v9/trace.go:320-323)
3	full blob clone in the combined accessor	fixed in d4b7b8b55, now covered by 8d92dbf06
4	aliased *InitialReads	declined, reasoning accepted
nit	nil clauses inside the guard	declined — "if either were nil the producer is broken" is a defensible line, and it matches the len(Traces) == len(transactions) invariant the VM path already trusts

Still open and non-blocking: the guard branch itself has no test in either version — a subtest where BlockNumberAndIndexByTxHash returns an out-of-range index and the assertion is ErrTxnHashNotFound rather than a panic would pin it. That branch is part of the 62–66% patch coverage Codecov reports on the two trace files. Not worth holding the PR for.

go vet ./core/... ./rpc/v9/... ./rpc/v10/... ./blockchain/... is clean, and vet type-checks test files, so the new subtest compiles. I could not execute the tests here — go test isn't in my allowed tools in this environment — so the serializer and trace suites are CI's word.
· branch refactor/rpc/trace-block-reads