From 6ab7d603e3b6c6c8f066edd1c5630b0e4cf84b67 Mon Sep 17 00:00:00 2001 From: danielntmd Date: Thu, 3 Sep 2026 04:33:34 -0700 Subject: [PATCH 1/6] perf(rpc): add progressive block trace caching --- rpc/v10/adapt_trace.go | 10 +- rpc/v10/handlers.go | 13 +- rpc/v10/trace.go | 267 ++++++------- rpc/v10/trace_cache.go | 232 ++++++++++++ rpc/v10/trace_cache_initial_reads.go | 69 ++++ rpc/v10/trace_cache_initial_reads_test.go | 59 +++ rpc/v10/trace_cache_test.go | 127 +++++++ rpc/v10/trace_progressive.go | 222 +++++++++++ rpc/v10/trace_progressive_test.go | 436 ++++++++++++++++++++++ rpc/v10/trace_test.go | 177 +++++++-- 10 files changed, 1433 insertions(+), 179 deletions(-) create mode 100644 rpc/v10/trace_cache.go create mode 100644 rpc/v10/trace_cache_initial_reads.go create mode 100644 rpc/v10/trace_cache_initial_reads_test.go create mode 100644 rpc/v10/trace_cache_test.go create mode 100644 rpc/v10/trace_progressive.go create mode 100644 rpc/v10/trace_progressive_test.go diff --git a/rpc/v10/adapt_trace.go b/rpc/v10/adapt_trace.go index 801d8eee65..673889dcca 100644 --- a/rpc/v10/adapt_trace.go +++ b/rpc/v10/adapt_trace.go @@ -247,16 +247,8 @@ func AdaptVMStateDiff(vmStateDiff *vm.StateDiff) StateDiff { } } +// adaptVMInitialReads requires non-nil VM output; callers decide how missing reads are handled. func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { - if vmInitialReads == nil { - return InitialReads{ - Storage: []StorageEntry{}, - Nonces: []NonceEntry{}, - ClassHashes: []ClassHashEntry{}, - DeclaredContracts: []DeclaredContractEntry{}, - } - } - storage := make([]StorageEntry, len(vmInitialReads.Storage)) for i, s := range vmInitialReads.Storage { storage[i] = StorageEntry{ diff --git a/rpc/v10/handlers.go b/rpc/v10/handlers.go index 018665e9c1..4949d0d9e6 100644 --- a/rpc/v10/handlers.go +++ b/rpc/v10/handlers.go @@ -11,7 +11,6 @@ import ( "github.com/NethermindEth/juno/blockchain" "github.com/NethermindEth/juno/clients/feeder" "github.com/NethermindEth/juno/core" - "github.com/NethermindEth/juno/core/felt" "github.com/NethermindEth/juno/core/pending" "github.com/NethermindEth/juno/feed" "github.com/NethermindEth/juno/jsonrpc" @@ -20,7 +19,6 @@ import ( "github.com/NethermindEth/juno/starknet/compiler" "github.com/NethermindEth/juno/sync" "github.com/NethermindEth/juno/utils/log" - "github.com/NethermindEth/juno/utils/lru" "github.com/NethermindEth/juno/vm" "github.com/sourcegraph/conc" ) @@ -43,8 +41,8 @@ type Handler struct { idgen func() string subscriptions stdsync.Map // map[string]*subscription - blockTraceCache *lru.Cache[felt.Felt, TraceBlockTransactionsResponse] - // todo(rdr): Can this cache be genericified and can it be applied to the `blockTraceCache` + blockTraceCache *blockTraceCache + // submittedTransactionsCache is a TTL membership set, unlike the coordinated block trace LRU. submittedTransactionsCache *rpccore.TransactionCache filterLimit uint @@ -84,11 +82,8 @@ func New( preConfirmedFeed: feed.New[*pending.PreConfirmed](), l1Heads: feed.New[*core.L1Head](), - blockTraceCache: lru.New[ - felt.Felt, - TraceBlockTransactionsResponse, - ](rpccore.TraceCacheSize), - filterLimit: math.MaxUint, + blockTraceCache: newBlockTraceCache(rpccore.TraceCacheSize), + filterLimit: math.MaxUint, } } diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index b9e81b440c..99e9aafe82 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -8,7 +8,6 @@ import ( "slices" "strconv" - "github.com/NethermindEth/juno/blockchain" "github.com/NethermindEth/juno/blockchain/networks" "github.com/NethermindEth/juno/core" "github.com/NethermindEth/juno/core/felt" @@ -32,6 +31,13 @@ type TransactionTrace struct { ExecutionResources *ExecutionResources `json:"execution_resources"` } +// finalisedBlockTraceSource contains either a complete gateway response or transactions for local +// replay. The caller owns publishing complete responses to the cache. +type finalisedBlockTraceSource struct { + complete *TraceBlockTransactionsResponse + transactions []core.Transaction +} + /**************************************************** Public API Handlers *****************************************************/ @@ -150,29 +156,16 @@ func (h *Handler) TraceBlockTransactions( Core Tracing Logic *****************************************************/ -// traceTransactionsWithState traces a set of transactions using the provided VM and state readers. -// -// Parameters: -// -// - vm: The virtual machine used for execution -// -// - transactions: The transactions to trace -// -// - executionState: The state used for transaction execution -// -// - classLookupState: The state used for class definition lookups. -// This should be at least the state that includes the target block or transaction. -// -// - blockInfo: Block context for execution -// -// - returnInitialReads: Whether to return initial reads in the response +// traceTransactionsWithState traces transactions against executionState. classLookupState must +// include any classes declared by the traced block or transaction. func traceTransactionsWithState( runner vm.VM, transactions []core.Transaction, executionState core.StateReader, classLookupState core.StateReader, blockInfo *vm.BlockInfo, - returnInitialReads bool, + opts vm.TraceOptions, + errorIndexOffset uint64, ) ([]TracedBlockTransaction, *vm.InitialReads, http.Header, *jsonrpc.Error) { httpHeader := defaultExecutionHeader() @@ -190,8 +183,9 @@ func traceTransactionsWithState( paidFeesOnL1, blockInfo, executionState, - vm.TraceOptions{ReturnInitialReads: returnInitialReads}, + opts, ) + vmErr = offsetTransactionExecutionErrorIndex(vmErr, errorIndexOffset) httpHeader.Set(ExecutionStepsHeader, strconv.FormatUint(executionResult.NumSteps, 10)) @@ -203,6 +197,18 @@ func traceTransactionsWithState( } // Adapt traces + if len(executionResult.Traces) != len(transactions) { + return nil, nil, httpHeader, rpccore.ErrUnexpectedError.CloneWithData( + "VM returned an unexpected number of transaction traces", + ) + } + + if len(executionResult.GasConsumed) != len(executionResult.Traces) { + return nil, nil, httpHeader, rpccore.ErrUnexpectedError.CloneWithData( + "VM returned an unexpected number of gas results", + ) + } + traces := make([]TracedBlockTransaction, len(executionResult.Traces)) for index := range executionResult.Traces { // Adapt vm transaction trace to rpc v10 trace and add root level execution resources @@ -226,6 +232,19 @@ func traceTransactionsWithState( return traces, executionResult.InitialReads, httpHeader, nil } +// offsetTransactionExecutionErrorIndex translates a suffix-local VM index to its block index. +func offsetTransactionExecutionErrorIndex(err error, offset uint64) error { + if err == nil || offset == 0 { + return err + } + var transactionErr vm.TransactionExecutionError + if !errors.As(err, &transactionErr) { + return err + } + transactionErr.Index += offset + return transactionErr +} + // fetchDeclaredClassesAndL1Fees collects class declarations and L1Handler placeholder fees. func fetchDeclaredClassesAndL1Fees( transactions []core.Transaction, state core.StateReader, @@ -275,20 +294,57 @@ func (h *Handler) findAndTraceFinalisedTransaction( return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } - blockTracesResp, httpHeader, rpcErr := h.traceFinalisedBlock(ctx, header, false) + if cached, found := h.blockTraceCache.traceAt(*header.Hash, txIndex); found { + trace, valid := transactionTraceIfHashMatches(cached, hash) + if !valid { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return trace, defaultExecutionHeader(), nil + } + + source, rpcErr := h.traceFinalisedBlockSource(ctx, header) if rpcErr != nil { - return TransactionTrace{}, nil, rpcErr + return TransactionTrace{}, defaultExecutionHeader(), rpcErr } - // txIndex comes from the tx-hash index while the traces come from a later read of the block, so - // confirm the trace at that index really is the transaction that was asked for. - blockTraces := blockTracesResp.Traces - if txIndex >= uint64(len(blockTraces)) || - !blockTraces[txIndex].TransactionHash.Equal((*felt.Felt)(hash)) { + if source.complete != nil { + h.blockTraceCache.storeComplete(*header.Hash, *source.complete) + blockTraces := source.complete.Traces + if txIndex >= uint64(len(blockTraces)) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + trace, valid := transactionTraceIfHashMatches(blockTraces[txIndex], hash) + if !valid { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return trace, defaultExecutionHeader(), nil + } + transactions := source.transactions + + // txIndex comes from the tx-hash index while transactions come from a later block read. + if txIndex >= uint64(len(transactions)) || + !transactions[txIndex].Hash().Equal((*felt.Felt)(hash)) { return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } - return *blockTraces[txIndex].TraceRoot, httpHeader, nil + response, httpHeader, rpcErr := h.traceProgressiveBlock( + ctx, header, transactions, txIndex, false, + ) + if rpcErr != nil { + return TransactionTrace{}, httpHeader, rpcErr + } + return *response.Traces[txIndex].TraceRoot, httpHeader, nil +} + +func transactionTraceIfHashMatches( + traced TracedBlockTransaction, + hash *felt.TransactionHash, +) (TransactionTrace, bool) { + if traced.TransactionHash == nil || traced.TraceRoot == nil || + !traced.TransactionHash.Equal((*felt.Felt)(hash)) { + return TransactionTrace{}, false + } + return *traced.TraceRoot, true } // findAndTraceInPreConfirmed traces a transaction located in any block of the @@ -335,7 +391,8 @@ func (h *Handler) findAndTraceInPreConfirmed( state, // execution state state, // class lookup state (same for preconfirmed) &blockInfo, - false, // returnInitialReads + vm.TraceOptions{}, + 0, ) if rpcErr != nil { return TransactionTrace{}, httpHeader, rpcErr @@ -356,130 +413,78 @@ func (h *Handler) traceFinalisedBlock( header *core.Header, returnInitialReads bool, ) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { - // Check if it was already traced. If the caller requested initial reads but - // the cached entry was produced without them, fall through to re-trace so we - // can populate them (cache gets overwritten below). cacheKey := *header.Hash - cachedResponse, hit := h.blockTraceCache.Get(cacheKey) - if hit && (!returnInitialReads || cachedResponse.InitialReads != nil) { - if returnInitialReads { - return cachedResponse, defaultExecutionHeader(), nil - } - return TraceBlockTransactionsResponse{ - Traces: cachedResponse.Traces, - InitialReads: nil, - }, defaultExecutionHeader(), nil + if response, complete := h.blockTraceCache.completeResponse(cacheKey); complete { + return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil } + source, rpcErr := h.traceFinalisedBlockSource(ctx, header) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr + } + if source.complete != nil { + response := *source.complete + h.blockTraceCache.storeComplete(cacheKey, response) + return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil + } + transactions := source.transactions + if len(transactions) > 0 { + return h.traceProgressiveBlock( + ctx, header, transactions, uint64(len(transactions)-1), returnInitialReads, + ) + } + + // The VM produces no traces or execution steps for an empty block. Block preprocessing only + // writes to its temporary cached state, so it also produces no initial reads. + response := TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{}, + InitialReads: emptyInitialReads(), + } + return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil +} + +// shapeTraceResponse applies request flags at the RPC boundary; cached responses remain canonical. +func shapeTraceResponse( + response TraceBlockTransactionsResponse, + returnInitialReads bool, +) TraceBlockTransactionsResponse { + if !returnInitialReads { + response.InitialReads = nil + } + return response +} + +func (h *Handler) traceFinalisedBlockSource( + ctx context.Context, + header *core.Header, +) (finalisedBlockTraceSource, *jsonrpc.Error) { fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(header, h.bcReader.Network()) if err != nil { - return TraceBlockTransactionsResponse{}, - defaultExecutionHeader(), - rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + return finalisedBlockTraceSource{}, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) } if fetchFromFeederGW { traces, rpcErr := h.fetchTracesFromFeederGateway(ctx, header) if rpcErr != nil { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr + return finalisedBlockTraceSource{}, rpcErr } - // The gateway never supplies initial reads, so an empty set is the final answer for these - // blocks. Caching it that way lets a later call with the flag be served from the cache. - cached := TraceBlockTransactionsResponse{ + // The gateway never supplies initial reads, so an empty set is final for these blocks. + response := TraceBlockTransactionsResponse{ Traces: traces, - InitialReads: &InitialReads{}, - } - h.blockTraceCache.Add(cacheKey, cached) - - response := cached - if !returnInitialReads { - response.InitialReads = nil + InitialReads: emptyInitialReads(), } - - return response, defaultExecutionHeader(), nil + return finalisedBlockTraceSource{complete: &response}, nil } transactions, err := h.bcReader.TransactionsByBlockNumber(header.Number) if err != nil { if errors.Is(err, db.ErrKeyNotFound) { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound - } - - return TraceBlockTransactionsResponse{}, - defaultExecutionHeader(), - rpccore.ErrInternal.CloneWithData(err) - } - - response, httpHeader, rpcErr := h.traceBlockWithVM(header, transactions, returnInitialReads) - if rpcErr != nil { - return TraceBlockTransactionsResponse{}, httpHeader, rpcErr - } - h.blockTraceCache.Add(cacheKey, response) - - return response, httpHeader, nil -} - -// traceBlockWithVM traces a block using the local VM. -func (h *Handler) traceBlockWithVM( - header *core.Header, - transactions []core.Transaction, - returnInitialReads bool, -) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { - // Prepare execution state - state, closer, err := h.bcReader.StateAtBlockHash(header.ParentHash) - if err != nil { - if errors.Is(err, db.ErrKeyNotFound) { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound + return finalisedBlockTraceSource{}, rpccore.ErrBlockNotFound } - - return TraceBlockTransactionsResponse{}, - defaultExecutionHeader(), - rpccore.ErrInternal.CloneWithData(err) - } - defer h.callAndLogErr(closer, "Failed to close state in traceBlockTransactions") - - // Get state to read class definitions for declare transactions - var ( - headState core.StateReader - headStateCloser blockchain.StateCloser - ) - - headState, headStateCloser, err = h.bcReader.HeadState() - if err != nil { - return TraceBlockTransactionsResponse{}, - defaultExecutionHeader(), - jsonrpc.Err(jsonrpc.InternalError, err.Error()) + return finalisedBlockTraceSource{}, rpccore.ErrInternal.CloneWithData(err) } - defer h.callAndLogErr(headStateCloser, "Failed to close head state in traceBlockTransactions") - - // Create block info - blockInfo, rpcErr := h.buildBlockInfo(header) - if rpcErr != nil { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr - } - - traces, vmInitialReads, httpHeader, rpcErr := traceTransactionsWithState( - h.vm, - transactions, - state, - headState, - &blockInfo, - returnInitialReads, - ) - if rpcErr != nil { - return TraceBlockTransactionsResponse{}, httpHeader, rpcErr - } - - var adaptedInitialReads *InitialReads - if vmInitialReads != nil && returnInitialReads { - adaptedInitialReads = new(adaptVMInitialReads(vmInitialReads)) - } - - return TraceBlockTransactionsResponse{ - Traces: traces, - InitialReads: adaptedInitialReads, - }, httpHeader, nil + return finalisedBlockTraceSource{transactions: transactions}, nil } // fetchTracesFromFeederGateway fetches block traces from the feeder gateway diff --git a/rpc/v10/trace_cache.go b/rpc/v10/trace_cache.go new file mode 100644 index 0000000000..35857935db --- /dev/null +++ b/rpc/v10/trace_cache.go @@ -0,0 +1,232 @@ +package rpcv10 + +import ( + "sync" + + "github.com/NethermindEth/juno/core/felt" + "github.com/NethermindEth/juno/utils/lru" +) + +// blockTraceCache retains successful trace progress in an LRU and permits one active extension per +// block. An extension becomes visible only after its entire requested suffix succeeds; failed work +// only wakes its waiters. mu protects LRU and flight membership, while per-record locks allow +// unrelated blocks to be read and extended concurrently. +type blockTraceCache struct { + mu sync.Mutex + + records *lru.SimpleCache[felt.Felt, *blockTraceRecord] + flights map[felt.Felt]*traceFlight +} + +// blockTraceRecord is the append-only tracing progress for one block. Returned trace slices are +// capacity-limited, so a later extension cannot change their visible length or elements. +type blockTraceRecord struct { + mu sync.RWMutex + + traces []TracedBlockTransaction + initialReads *InitialReads // non-nil once the record has been published + complete bool +} + +type blockTraceRecordView struct { + traces []TracedBlockTransaction + initialReads *InitialReads + complete bool +} + +type traceFlight struct { + done chan struct{} +} + +type traceCacheLookupKind uint8 + +const ( + traceCacheHit traceCacheLookupKind = iota + 1 + traceCacheWait + traceCacheExtend +) + +type traceCacheLookup struct { + kind traceCacheLookupKind + response TraceBlockTransactionsResponse + done <-chan struct{} + work *traceCacheWork +} + +// traceCacheWork is owned by the caller executing a suffix. Work retains its record across LRU +// eviction; only a successful commit republishes it. After commit removes the flight, the deferred +// abort sees that it is no longer active and becomes a no-op. +type traceCacheWork struct { + cache *blockTraceCache + hash felt.Felt + flight *traceFlight + record *blockTraceRecord +} + +func (r *blockTraceRecord) view() blockTraceRecordView { + r.mu.RLock() + defer r.mu.RUnlock() + return r.viewLocked() +} + +func (r *blockTraceRecord) viewLocked() blockTraceRecordView { + return blockTraceRecordView{ + traces: r.traces[:len(r.traces):len(r.traces)], + initialReads: r.initialReads, + complete: r.complete, + } +} + +func (r *blockTraceRecord) append( + extension TraceBlockTransactionsResponse, + totalTransactions int, +) blockTraceRecordView { + r.mu.Lock() + defer r.mu.Unlock() + r.traces = append(r.traces, extension.Traces...) + r.initialReads = mergeInitialReads(r.initialReads, extension.InitialReads) + r.complete = len(r.traces) == totalTransactions + return r.viewLocked() +} + +func (v blockTraceRecordView) response() TraceBlockTransactionsResponse { + // Responses borrow record-owned data and must be treated as read-only. + response := TraceBlockTransactionsResponse{Traces: v.traces} + if v.complete { + response.InitialReads = v.initialReads + } + return response +} + +func (w *traceCacheWork) prefix() []TracedBlockTransaction { + return w.record.view().traces +} + +func (w *traceCacheWork) commit( + extension TraceBlockTransactionsResponse, + totalTransactions int, +) TraceBlockTransactionsResponse { + view := w.record.append(extension, totalTransactions) + + cache := w.cache + cache.mu.Lock() + defer cache.mu.Unlock() + cache.records.Add(w.hash, w.record) + cache.finishLocked(w.hash, w.flight) + return view.response() +} + +func (w *traceCacheWork) abort() { + cache := w.cache + cache.mu.Lock() + defer cache.mu.Unlock() + if cache.flights[w.hash] != w.flight { + return + } + cache.finishLocked(w.hash, w.flight) +} + +func newBlockTraceCache(limit int) *blockTraceCache { + return &blockTraceCache{ + records: lru.NewSimple[felt.Felt, *blockTraceRecord](limit), + flights: make(map[felt.Felt]*traceFlight), + } +} + +func (c *blockTraceCache) completeResponse( + blockHash felt.Felt, +) (TraceBlockTransactionsResponse, bool) { + record, found := c.record(blockHash) + if !found { + return TraceBlockTransactionsResponse{}, false + } + view := record.view() + if !view.complete { + return TraceBlockTransactionsResponse{}, false + } + return view.response(), true +} + +func (c *blockTraceCache) traceAt( + blockHash felt.Felt, + index uint64, +) (TracedBlockTransaction, bool) { + record, found := c.record(blockHash) + if !found { + return TracedBlockTransaction{}, false + } + view := record.view() + if index >= uint64(len(view.traces)) { + return TracedBlockTransaction{}, false + } + return view.traces[index], true +} + +func (c *blockTraceCache) record(blockHash felt.Felt) (*blockTraceRecord, bool) { + c.mu.Lock() + defer c.mu.Unlock() + return c.records.Get(blockHash) +} + +func (c *blockTraceCache) lookupOrStart( + blockHash felt.Felt, + target uint64, +) traceCacheLookup { + c.mu.Lock() + record, _ := c.records.Get(blockHash) + if flight, found := c.flights[blockHash]; found { + c.mu.Unlock() + if lookup, found := lookupBlockTraceRecord(record, target); found { + return lookup + } + return traceCacheLookup{kind: traceCacheWait, done: flight.done} + } + defer c.mu.Unlock() + + if lookup, found := lookupBlockTraceRecord(record, target); found { + return lookup + } + if record == nil { + record = &blockTraceRecord{} + } + flight := &traceFlight{done: make(chan struct{})} + c.flights[blockHash] = flight + return traceCacheLookup{ + kind: traceCacheExtend, + work: &traceCacheWork{cache: c, hash: blockHash, flight: flight, record: record}, + } +} + +func lookupBlockTraceRecord( + record *blockTraceRecord, + target uint64, +) (traceCacheLookup, bool) { + if record == nil { + return traceCacheLookup{}, false + } + view := record.view() + if target < uint64(len(view.traces)) { + return traceCacheLookup{kind: traceCacheHit, response: view.response()}, true + } + return traceCacheLookup{}, false +} + +// storeComplete takes ownership of the response containers. InitialReads must be non-nil. +func (c *blockTraceCache) storeComplete( + blockHash felt.Felt, + response TraceBlockTransactionsResponse, +) { + record := &blockTraceRecord{ + traces: response.Traces, + initialReads: response.InitialReads, + complete: true, + } + c.mu.Lock() + defer c.mu.Unlock() + c.records.Add(blockHash, record) +} + +func (c *blockTraceCache) finishLocked(blockHash felt.Felt, flight *traceFlight) { + delete(c.flights, blockHash) + close(flight.done) +} diff --git a/rpc/v10/trace_cache_initial_reads.go b/rpc/v10/trace_cache_initial_reads.go new file mode 100644 index 0000000000..8ddaba529b --- /dev/null +++ b/rpc/v10/trace_cache_initial_reads.go @@ -0,0 +1,69 @@ +package rpcv10 + +import "github.com/NethermindEth/juno/core/felt" + +func emptyInitialReads() *InitialReads { + return &InitialReads{ + Storage: []StorageEntry{}, + Nonces: []NonceEntry{}, + ClassHashes: []ClassHashEntry{}, + DeclaredContracts: []DeclaredContractEntry{}, + } +} + +// mergeInitialReads requires non-nil incoming reads and retains the earliest value for each key. +func mergeInitialReads(existing, incoming *InitialReads) *InitialReads { + if existing == nil { + return incoming + } + + type storageKey struct { + address felt.Address + key felt.Felt + } + storageSeen := make(map[storageKey]struct{}, len(existing.Storage)) + for _, read := range existing.Storage { + storageSeen[storageKey{address: read.ContractAddress, key: read.Key}] = struct{}{} + } + for _, read := range incoming.Storage { + key := storageKey{address: read.ContractAddress, key: read.Key} + if _, found := storageSeen[key]; !found { + existing.Storage = append(existing.Storage, read) + storageSeen[key] = struct{}{} + } + } + + nonceSeen := make(map[felt.Address]struct{}, len(existing.Nonces)) + for _, read := range existing.Nonces { + nonceSeen[read.ContractAddress] = struct{}{} + } + for _, read := range incoming.Nonces { + if _, found := nonceSeen[read.ContractAddress]; !found { + existing.Nonces = append(existing.Nonces, read) + nonceSeen[read.ContractAddress] = struct{}{} + } + } + + classHashSeen := make(map[felt.Address]struct{}, len(existing.ClassHashes)) + for _, read := range existing.ClassHashes { + classHashSeen[read.ContractAddress] = struct{}{} + } + for _, read := range incoming.ClassHashes { + if _, found := classHashSeen[read.ContractAddress]; !found { + existing.ClassHashes = append(existing.ClassHashes, read) + classHashSeen[read.ContractAddress] = struct{}{} + } + } + + declaredSeen := make(map[felt.ClassHash]struct{}, len(existing.DeclaredContracts)) + for _, read := range existing.DeclaredContracts { + declaredSeen[read.ClassHash] = struct{}{} + } + for _, read := range incoming.DeclaredContracts { + if _, found := declaredSeen[read.ClassHash]; !found { + existing.DeclaredContracts = append(existing.DeclaredContracts, read) + declaredSeen[read.ClassHash] = struct{}{} + } + } + return existing +} diff --git a/rpc/v10/trace_cache_initial_reads_test.go b/rpc/v10/trace_cache_initial_reads_test.go new file mode 100644 index 0000000000..39d92defff --- /dev/null +++ b/rpc/v10/trace_cache_initial_reads_test.go @@ -0,0 +1,59 @@ +package rpcv10 + +import ( + "testing" + + "github.com/NethermindEth/juno/core/felt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMergeInitialReadsPreservesOriginalValues(t *testing.T) { + address1 := felt.FromUint64[felt.Address](1) + address2 := felt.FromUint64[felt.Address](2) + key1 := felt.FromUint64[felt.Felt](3) + key2 := felt.FromUint64[felt.Felt](4) + class1 := felt.FromUint64[felt.ClassHash](5) + class2 := felt.FromUint64[felt.ClassHash](6) + + existing := &InitialReads{ + Storage: []StorageEntry{{ + ContractAddress: address1, Key: key1, Value: felt.FromUint64[felt.Felt](10), + }}, + Nonces: []NonceEntry{{ContractAddress: address1, Nonce: felt.FromUint64[felt.Felt](11)}}, + ClassHashes: []ClassHashEntry{{ + ContractAddress: address1, ClassHash: class1, + }}, + DeclaredContracts: []DeclaredContractEntry{{ClassHash: class1, IsDeclared: false}}, + } + incoming := &InitialReads{ + Storage: []StorageEntry{ + {ContractAddress: address1, Key: key1, Value: felt.FromUint64[felt.Felt](100)}, + {ContractAddress: address2, Key: key2, Value: felt.FromUint64[felt.Felt](20)}, + }, + Nonces: []NonceEntry{ + {ContractAddress: address1, Nonce: felt.FromUint64[felt.Felt](101)}, + {ContractAddress: address2, Nonce: felt.FromUint64[felt.Felt](21)}, + }, + ClassHashes: []ClassHashEntry{ + {ContractAddress: address1, ClassHash: class2}, + {ContractAddress: address2, ClassHash: class2}, + }, + DeclaredContracts: []DeclaredContractEntry{ + {ClassHash: class1, IsDeclared: true}, + {ClassHash: class2, IsDeclared: false}, + }, + } + + merged := mergeInitialReads(existing, incoming) + require.Same(t, existing, merged) + require.Len(t, merged.Storage, 2) + assert.Equal(t, uint64(10), merged.Storage[0].Value.Uint64()) + assert.Equal(t, address2, merged.Storage[1].ContractAddress) + require.Len(t, merged.Nonces, 2) + assert.Equal(t, uint64(11), merged.Nonces[0].Nonce.Uint64()) + require.Len(t, merged.ClassHashes, 2) + assert.Equal(t, class1, merged.ClassHashes[0].ClassHash) + require.Len(t, merged.DeclaredContracts, 2) + assert.False(t, merged.DeclaredContracts[0].IsDeclared) +} diff --git a/rpc/v10/trace_cache_test.go b/rpc/v10/trace_cache_test.go new file mode 100644 index 0000000000..325db6cc37 --- /dev/null +++ b/rpc/v10/trace_cache_test.go @@ -0,0 +1,127 @@ +package rpcv10 + +import ( + "testing" + "time" + + "github.com/NethermindEth/juno/core/felt" + "github.com/stretchr/testify/require" +) + +func blockTraceCacheState( + cache *blockTraceCache, + blockHash felt.Felt, +) (*blockTraceRecord, bool, bool) { + cache.mu.Lock() + defer cache.mu.Unlock() + record, found := cache.records.Get(blockHash) + _, inflight := cache.flights[blockHash] + return record, found, inflight +} + +func TestBlockTraceRecordAppendPreservesPublishedPrefix(t *testing.T) { + baseHash := felt.FromUint64[felt.Felt](1) + suffixHash := felt.FromUint64[felt.Felt](2) + address := felt.FromUint64[felt.Address](3) + key := felt.FromUint64[felt.Felt](4) + baseValue := felt.FromUint64[felt.Felt](5) + record := &blockTraceRecord{ + traces: []TracedBlockTransaction{{TransactionHash: &baseHash}}, + initialReads: &InitialReads{Storage: []StorageEntry{{ + ContractAddress: address, Key: key, Value: baseValue, + }}}, + } + extension := TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{{TransactionHash: &suffixHash}}, + InitialReads: &InitialReads{Storage: []StorageEntry{{ + ContractAddress: address, + Key: felt.FromUint64[felt.Felt](6), + Value: felt.FromUint64[felt.Felt](7), + }}}, + } + + partial := record.view().response() + require.Nil(t, partial.InitialReads, "initial reads describe block pre-state only when complete") + require.Equal(t, len(partial.Traces), cap(partial.Traces)) + + response := record.append(extension, 2).response() + require.Len(t, response.Traces, 2) + require.NotNil(t, response.InitialReads) + require.Len(t, response.InitialReads.Storage, 2) + require.Same(t, &baseHash, record.traces[0].TransactionHash) + require.Equal(t, uint64(5), record.initialReads.Storage[0].Value.Uint64()) + require.True(t, record.complete) + require.Len(t, partial.Traces, 1, "a previously returned prefix must not grow") +} + +func TestBlockTraceCacheReadsOtherRecordWhileOneIsLocked(t *testing.T) { + cache := newBlockTraceCache(2) + blockedHash := felt.FromUint64[felt.Felt](1) + otherHash := felt.FromUint64[felt.Felt](2) + completeResponse := func() TraceBlockTransactionsResponse { + return TraceBlockTransactionsResponse{ + Traces: make([]TracedBlockTransaction, 1), + InitialReads: emptyInitialReads(), + } + } + cache.storeComplete(blockedHash, completeResponse()) + cache.storeComplete(otherHash, completeResponse()) + + blockedRecord, found, _ := blockTraceCacheState(cache, blockedHash) + require.True(t, found) + blockedRecord.mu.Lock() + defer blockedRecord.mu.Unlock() + + otherDone := make(chan struct{}) + go func() { + defer close(otherDone) + _, _ = cache.traceAt(otherHash, 0) + }() + select { + case <-otherDone: + case <-time.After(time.Second): + t.Fatal("one record lock must not block another block's cache read") + } +} + +func TestBlockTraceCacheLookupTransitions(t *testing.T) { + cache := newBlockTraceCache(1) + blockHash := felt.FromUint64[felt.Felt](1) + firstHash := felt.FromUint64[felt.Felt](2) + secondHash := felt.FromUint64[felt.Felt](3) + + first := cache.lookupOrStart(blockHash, 0) + require.Equal(t, traceCacheExtend, first.kind) + + for _, target := range []uint64{0, 1} { + waiting := cache.lookupOrStart(blockHash, target) + require.Equal(t, traceCacheWait, waiting.kind) + require.True(t, first.work.flight.done == waiting.done) + } + + prefix := first.work.commit(TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{{TransactionHash: &firstHash}}, + InitialReads: &InitialReads{}, + }, 2) + require.Len(t, prefix.Traces, 1) + require.Nil(t, prefix.InitialReads) + select { + case <-first.work.flight.done: + default: + t.Fatal("commit must wake flight waiters") + } + + hit := cache.lookupOrStart(blockHash, 0) + require.Equal(t, traceCacheHit, hit.kind) + require.Len(t, hit.response.Traces, 1) + + second := cache.lookupOrStart(blockHash, 1) + require.Equal(t, traceCacheExtend, second.kind) + require.Len(t, second.work.prefix(), 1) + complete := second.work.commit(TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{{TransactionHash: &secondHash}}, + InitialReads: &InitialReads{}, + }, 2) + require.Len(t, complete.Traces, 2) + require.NotNil(t, complete.InitialReads) +} diff --git a/rpc/v10/trace_progressive.go b/rpc/v10/trace_progressive.go new file mode 100644 index 0000000000..00e1fd2ffb --- /dev/null +++ b/rpc/v10/trace_progressive.go @@ -0,0 +1,222 @@ +package rpcv10 + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/NethermindEth/juno/core" + "github.com/NethermindEth/juno/core/felt" + "github.com/NethermindEth/juno/core/pending" + "github.com/NethermindEth/juno/db" + "github.com/NethermindEth/juno/jsonrpc" + "github.com/NethermindEth/juno/rpc/rpccore" + "github.com/NethermindEth/juno/vm" +) + +func (h *Handler) traceProgressiveBlock( + ctx context.Context, + header *core.Header, + transactions []core.Transaction, + target uint64, + returnInitialReads bool, +) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { + if target >= uint64(len(transactions)) { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrUnexpectedError.CloneWithData(fmt.Sprintf( + "trace target index %d out of range for %d transactions", target, len(transactions), + )) + } + + blockHash := *header.Hash + for { + lookup := h.blockTraceCache.lookupOrStart(blockHash, target) + switch lookup.kind { + case traceCacheHit: + return shapeTraceResponse(lookup.response, returnInitialReads), defaultExecutionHeader(), nil + case traceCacheWait: + select { + case <-ctx.Done(): + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrUnexpectedError.CloneWithData(ctx.Err().Error()) + case <-lookup.done: + continue + } + case traceCacheExtend: + response, responseHeader, rpcErr := h.executeTraceCacheWork( + lookup.work, header, transactions, target, + ) + return shapeTraceResponse(response, returnInitialReads), responseHeader, rpcErr + default: + panic("unknown trace cache lookup result") + } + } +} + +func (h *Handler) executeTraceCacheWork( + work *traceCacheWork, + header *core.Header, + transactions []core.Transaction, + target uint64, +) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { + // Always release waiters, including when VM execution or adaptation panics. + defer work.abort() + + response, responseHeader, rpcErr := h.executeTraceExtension( + header, transactions, work.prefix(), target, + ) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, responseHeader, rpcErr + } + + return work.commit(response, len(transactions)), responseHeader, nil +} + +func (h *Handler) executeTraceExtension( + header *core.Header, + transactions []core.Transaction, + cachedPrefix []TracedBlockTransaction, + target uint64, +) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { + start := uint64(len(cachedPrefix)) + base, baseCloser, err := h.bcReader.StateAtBlockHash(header.ParentHash) + if err != nil { + if errors.Is(err, db.ErrKeyNotFound) { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound + } + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrInternal.CloneWithData(err) + } + defer h.callAndLogErr(baseCloser, "Failed to close base state after trace extension") + + headState, headCloser, err := h.bcReader.HeadState() + if err != nil { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + jsonrpc.Err(jsonrpc.InternalError, err.Error()) + } + defer h.callAndLogErr(headCloser, "Failed to close head state after trace extension") + + executionState := base + if start > 0 { + checkpoint := checkpointFromTraces(cachedPrefix) + newClasses, rpcErr := checkpointClasses(&checkpoint, headState) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr + } + executionState = pending.NewState(&checkpoint, newClasses, base, header.Number) + } + + blockInfo, rpcErr := h.buildBlockInfo(header) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr + } + // Always collect reads for future flagged requests; stitching retains Blockifier's earliest + // pre-state value. + traces, vmInitialReads, responseHeader, rpcErr := traceTransactionsWithState( + h.vm, + transactions[start:target+1], + executionState, + headState, + &blockInfo, + vm.TraceOptions{ReturnInitialReads: true}, + start, + ) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, responseHeader, rpcErr + } + // The production Rust VM serialises state_diff as a required field for every + // successful transaction trace. Enforce that contract before a trace becomes + // an extendable cache record. + for index := range traces { + if traces[index].TraceRoot.StateDiff == nil { + return TraceBlockTransactionsResponse{}, responseHeader, + rpccore.ErrUnexpectedError.CloneWithData(fmt.Sprintf( + "VM omitted state diff for transaction trace %d", start+uint64(index), + )) + } + } + if vmInitialReads == nil { + return TraceBlockTransactionsResponse{}, responseHeader, + rpccore.ErrUnexpectedError.CloneWithData("VM omitted initial reads for trace extension") + } + adaptedReads := adaptVMInitialReads(vmInitialReads) + return TraceBlockTransactionsResponse{ + Traces: traces, InitialReads: &adaptedReads, + }, responseHeader, nil +} + +func checkpointClasses( + diff *core.StateDiff, + classLookup core.StateReader, +) (map[felt.Felt]core.ClassDefinition, *jsonrpc.Error) { + classes := make( + map[felt.Felt]core.ClassDefinition, + len(diff.DeclaredV0Classes)+len(diff.DeclaredV1Classes), + ) + load := func(hash felt.Felt) *jsonrpc.Error { + if _, exists := classes[hash]; exists { + return nil + } + declared, err := classLookup.Class(&hash) + if err != nil { + return jsonrpc.Err(jsonrpc.InternalError, err.Error()) + } + classes[hash] = declared.Class + return nil + } + for _, hash := range diff.DeclaredV0Classes { + if rpcErr := load(*hash); rpcErr != nil { + return nil, rpcErr + } + } + for hash := range diff.DeclaredV1Classes { + if rpcErr := load(hash); rpcErr != nil { + return nil, rpcErr + } + } + return classes, nil +} + +// checkpointFromTraces rebuilds the continuation checkpoint from cached per-transaction state +// diffs. Progressive records only contain traces with non-nil state diffs; executeTraceExtension +// enforces this invariant before publishing an extension. The small recomputation cost avoids +// retaining a duplicate cumulative state diff in the cache. +func checkpointFromTraces(traces []TracedBlockTransaction) core.StateDiff { + result := core.EmptyStateDiff() + for index := range traces { + mergeRPCStateDiff(&result, traces[index].TraceRoot.StateDiff) + } + return result +} + +func mergeRPCStateDiff(result *core.StateDiff, diff *StateDiff) { + for _, storage := range diff.StorageDiffs { + entries, found := result.StorageDiffs[storage.Address] + if !found { + entries = make(map[felt.Felt]*felt.Felt, len(storage.StorageEntries)) + result.StorageDiffs[storage.Address] = entries + } + for _, entry := range storage.StorageEntries { + entries[entry.Key] = entry.Value.Clone() + } + } + for _, nonce := range diff.Nonces { + result.Nonces[nonce.ContractAddress] = nonce.Nonce.Clone() + } + for _, deployed := range diff.DeployedContracts { + result.DeployedContracts[deployed.Address] = deployed.ClassHash.Clone() + } + for _, hash := range diff.DeprecatedDeclaredClasses { + result.DeclaredV0Classes = append(result.DeclaredV0Classes, hash.Clone()) + } + for _, declared := range diff.DeclaredClasses { + result.DeclaredV1Classes[declared.ClassHash] = declared.CompiledClassHash.Clone() + } + for _, replaced := range diff.ReplacedClasses { + result.ReplacedClasses[replaced.ContractAddress] = replaced.ClassHash.Clone() + } + for _, migrated := range diff.MigratedCompiledClasses { + result.MigratedClasses[migrated.ClassHash] = migrated.CompiledClassHash + } +} diff --git a/rpc/v10/trace_progressive_test.go b/rpc/v10/trace_progressive_test.go new file mode 100644 index 0000000000..c8638ac35e --- /dev/null +++ b/rpc/v10/trace_progressive_test.go @@ -0,0 +1,436 @@ +package rpcv10 + +import ( + "context" + "encoding/json" + "fmt" + "sync/atomic" + "testing" + + "github.com/NethermindEth/juno/blockchain/networks" + "github.com/NethermindEth/juno/core" + "github.com/NethermindEth/juno/core/felt" + "github.com/NethermindEth/juno/jsonrpc" + "github.com/NethermindEth/juno/mocks" + "github.com/NethermindEth/juno/rpc/rpccore" + "github.com/NethermindEth/juno/utils/log" + "github.com/NethermindEth/juno/vm" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +type progressiveTestVM struct { + vm.VM + trace func([]core.Transaction, core.StateReader) (vm.ExecutionResults, error) +} + +func (v *progressiveTestVM) Trace( + transactions []core.Transaction, + _ []core.ClassDefinition, + _ []*felt.Felt, + _ *vm.BlockInfo, + state core.StateReader, + _ vm.TraceOptions, +) (vm.ExecutionResults, error) { + return v.trace(transactions, state) +} + +func progressiveTestTransactions(count int) []core.Transaction { + transactions := make([]core.Transaction, count) + for index := range transactions { + transactions[index] = &core.InvokeTransaction{ + TransactionHash: felt.NewFromUint64[felt.Felt](uint64(index + 1)), + } + } + return transactions +} + +func progressiveTestResults(transactions []core.Transaction) vm.ExecutionResults { + traces := make([]vm.TransactionTrace, len(transactions)) + gas := make([]core.GasConsumed, len(transactions)) + for index := range transactions { + traces[index].StateDiff = &vm.StateDiff{} + gas[index].L1Gas = transactions[index].Hash().Uint64() + } + return vm.ExecutionResults{ + Traces: traces, + GasConsumed: gas, + NumSteps: uint64(len(transactions)), + InitialReads: &vm.InitialReads{}, + } +} + +func newProgressiveTestHandler( + t *testing.T, + virtualMachine vm.VM, +) (*Handler, *core.Header, []core.Transaction, *mocks.MockStateReader) { + t.Helper() + ctrl := gomock.NewController(t) + reader := mocks.NewMockReader(ctrl) + state := mocks.NewMockStateReader(ctrl) + header := &core.Header{ + Hash: felt.NewFromUint64[felt.Felt](100), + ParentHash: felt.NewFromUint64[felt.Felt](99), + SequencerAddress: felt.NewFromUint64[felt.Felt](98), + L1GasPriceETH: felt.NewFromUint64[felt.Felt](1), + Number: 1, + ProtocolVersion: "99.12.3", + } + reader.EXPECT().StateAtBlockHash(header.ParentHash). + Return(state, func() error { return nil }, nil).AnyTimes() + reader.EXPECT().HeadState().Return(state, func() error { return nil }, nil).AnyTimes() + handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) + return handler, header, progressiveTestTransactions(3), state +} + +func TestMergeRPCStateDiff(t *testing.T) { + address := felt.FromUint64[felt.Felt](1) + key := felt.FromUint64[felt.Felt](2) + value := felt.FromUint64[felt.Felt](3) + nonce := felt.FromUint64[felt.Felt](4) + classHash := felt.FromUint64[felt.Felt](5) + compiledHash := felt.FromUint64[felt.Felt](6) + replacement := felt.FromUint64[felt.Felt](7) + migratedClass := felt.FromUint64[felt.SierraClassHash](8) + migratedCompiled := felt.FromUint64[felt.CasmClassHash](9) + + converted := core.EmptyStateDiff() + mergeRPCStateDiff(&converted, &StateDiff{ + StorageDiffs: []StorageDiff{{ + Address: address, StorageEntries: []Entry{{Key: key, Value: value}}, + }}, + Nonces: []Nonce{{ContractAddress: address, Nonce: nonce}}, + DeployedContracts: []DeployedContract{{Address: address, ClassHash: classHash}}, + DeprecatedDeclaredClasses: []*felt.Felt{&classHash}, + DeclaredClasses: []DeclaredClass{{ + ClassHash: classHash, CompiledClassHash: compiledHash, + }}, + ReplacedClasses: []ReplacedClass{{ContractAddress: address, ClassHash: replacement}}, + MigratedCompiledClasses: []MigratedCompiledClass{{ + ClassHash: migratedClass, CompiledClassHash: migratedCompiled, + }}, + }) + + require.Equal(t, value, *converted.StorageDiffs[address][key]) + require.Equal(t, nonce, *converted.Nonces[address]) + require.Equal(t, classHash, *converted.DeployedContracts[address]) + require.Equal(t, classHash, *converted.DeclaredV0Classes[0]) + require.Equal(t, compiledHash, *converted.DeclaredV1Classes[classHash]) + require.Equal(t, replacement, *converted.ReplacedClasses[address]) + require.Equal(t, migratedCompiled, converted.MigratedClasses[migratedClass]) + + value.SetUint64(99) + require.Equal(t, uint64(3), converted.StorageDiffs[address][key].Uint64()) +} + +func TestProgressiveTraceCacheWaiterCancellationDoesNotCancelExtension(t *testing.T) { + entered := make(chan struct{}) + release := make(chan struct{}) + var calls atomic.Uint64 + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + _ core.StateReader, + ) (vm.ExecutionResults, error) { + calls.Add(1) + close(entered) + <-release + return progressiveTestResults(transactions), nil + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + + ownerDone := make(chan *jsonrpc.Error, 1) + go func() { + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + ownerDone <- rpcErr + }() + <-entered + cancelled, cancel := context.WithCancel(t.Context()) + cancel() + _, _, rpcErr := handler.traceProgressiveBlock(cancelled, header, transactions, 1, false) + require.NotNil(t, rpcErr) + require.Contains(t, rpcErr.Data, context.Canceled.Error()) + + close(release) + require.Nil(t, <-ownerDone) + response, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + require.Nil(t, rpcErr) + require.Len(t, response.Traces, 2) + require.Equal(t, uint64(1), calls.Load()) +} + +func TestProgressiveTraceCacheFailureRetainsPublishedPrefix(t *testing.T) { + var calls atomic.Uint64 + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + _ core.StateReader, + ) (vm.ExecutionResults, error) { + switch calls.Add(1) { + case 2: + transactionErr := vm.TransactionExecutionError{ + Index: 0, + Cause: json.RawMessage(`"extension failed"`), + } + return vm.ExecutionResults{NumSteps: 9}, fmt.Errorf("VM decorator: %w", transactionErr) + default: + return progressiveTestResults(transactions), nil + } + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + + response, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + require.Nil(t, rpcErr) + require.Len(t, response.Traces, 1) + _, responseHeader, rpcErr := handler.traceProgressiveBlock( + t.Context(), header, transactions, 1, false, + ) + require.NotNil(t, rpcErr) + require.Equal(t, "9", responseHeader.Get(ExecutionStepsHeader)) + require.Contains(t, rpcErr.Data, "transaction #1") + + response, responseHeader, rpcErr = handler.traceProgressiveBlock( + t.Context(), header, transactions, 0, false, + ) + require.Nil(t, rpcErr) + require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) + require.Len(t, response.Traces, 1) + require.Equal(t, uint64(2), calls.Load()) + + response, _, rpcErr = handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + require.Nil(t, rpcErr) + require.Len(t, response.Traces, 2) + require.Equal(t, uint64(3), calls.Load()) +} + +func TestProgressiveTraceCacheDoesNotCacheFailedFirstExtension(t *testing.T) { + virtualMachine := &progressiveTestVM{trace: func( + []core.Transaction, + core.StateReader, + ) (vm.ExecutionResults, error) { + return vm.ExecutionResults{}, vm.TransactionExecutionError{ + Index: 0, + Cause: json.RawMessage(`"extension failed"`), + } + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + require.NotNil(t, rpcErr) + _, cached, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) + require.False(t, cached) + require.False(t, inflight) +} + +func TestProgressiveTraceCacheRejectsMissingStateDiff(t *testing.T) { + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + _ core.StateReader, + ) (vm.ExecutionResults, error) { + results := progressiveTestResults(transactions) + results.Traces[0].StateDiff = nil + return results, nil + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + require.NotNil(t, rpcErr) + require.Contains(t, rpcErr.Data, "VM omitted state diff for transaction trace 0") + + _, cached, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) + require.False(t, cached) + require.False(t, inflight) +} + +func TestProgressiveTraceCacheRejectsMismatchedVMResults(t *testing.T) { + tests := []struct { + name string + traceCount int + gasCount int + want string + }{ + {"too few traces", 1, 2, "unexpected number of transaction traces"}, + {"too few gas results", 2, 1, "unexpected number of gas results"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + virtualMachine := &progressiveTestVM{trace: func( + []core.Transaction, core.StateReader, + ) (vm.ExecutionResults, error) { + return vm.ExecutionResults{ + Traces: make([]vm.TransactionTrace, test.traceCount), + GasConsumed: make([]core.GasConsumed, test.gasCount), + }, nil + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + require.NotNil(t, rpcErr) + require.Contains(t, rpcErr.Data, test.want) + }) + } +} + +func TestProgressiveTraceCachePanicClearsInflight(t *testing.T) { + var calls atomic.Uint64 + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + _ core.StateReader, + ) (vm.ExecutionResults, error) { + if calls.Add(1) == 1 { + panic("trace panic") + } + return progressiveTestResults(transactions), nil + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + + func() { + defer func() { + require.Equal(t, "trace panic", recover()) + }() + _, _, _ = handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + }() + + response, _, rpcErr := handler.traceProgressiveBlock( + t.Context(), header, transactions, 0, false, + ) + require.Nil(t, rpcErr) + require.Len(t, response.Traces, 1) + require.Equal(t, uint64(2), calls.Load()) +} + +func TestTraceFinalisedEmptyBlockReturnsWithoutCaching(t *testing.T) { + ctrl := gomock.NewController(t) + reader := mocks.NewMockReader(ctrl) + virtualMachine := mocks.NewMockVM(ctrl) + header := &core.Header{ + Hash: felt.NewFromUint64[felt.Felt](100), + ProtocolVersion: "99.12.3", + } + + reader.EXPECT().Network().Return(&networks.Mainnet).Times(2) + reader.EXPECT().TransactionsByBlockNumber(header.Number).Return(nil, nil).Times(2) + handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) + + response, responseHeader, rpcErr := handler.traceFinalisedBlock( + t.Context(), header, true, + ) + require.Nil(t, rpcErr) + require.Empty(t, response.Traces) + require.NotNil(t, response.InitialReads) + require.Empty(t, response.InitialReads.Storage) + require.Empty(t, response.InitialReads.Nonces) + require.Empty(t, response.InitialReads.ClassHashes) + require.Empty(t, response.InitialReads.DeclaredContracts) + require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) + + response, responseHeader, rpcErr = handler.traceFinalisedBlock(t.Context(), header, false) + require.Nil(t, rpcErr) + require.Empty(t, response.Traces) + require.Nil(t, response.InitialReads) + require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) +} + +func TestProgressiveTraceCacheAllowsRecordEvictionDuringFlight(t *testing.T) { + entered := make(chan struct{}) + release := make(chan struct{}) + var calls atomic.Uint64 + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + _ core.StateReader, + ) (vm.ExecutionResults, error) { + if calls.Add(1) == 2 { + close(entered) + <-release + } + return progressiveTestResults(transactions), nil + }} + handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) + prefix, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + require.Nil(t, rpcErr) + require.Len(t, prefix.Traces, 1) + + ownerDone := make(chan *jsonrpc.Error, 1) + go func() { + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + ownerDone <- rpcErr + }() + <-entered + for index := range rpccore.TraceCacheSize { + handler.blockTraceCache.storeComplete( + felt.FromUint64[felt.Felt](uint64(1_000+index)), + TraceBlockTransactionsResponse{InitialReads: emptyInitialReads()}, + ) + } + _, found, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) + require.False(t, found, "the base record may be evicted while its owner retains it") + require.True(t, inflight, "active work must remain discoverable through its flight") + + waiting := handler.blockTraceCache.lookupOrStart(*header.Hash, 1) + require.Equal(t, traceCacheWait, waiting.kind) + close(release) + + require.Nil(t, <-ownerDone) + select { + case <-waiting.done: + default: + t.Fatal("commit must wake waiters after republishing the evicted record") + } + require.Equal(t, uint64(2), calls.Load()) + record, found, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) + require.True(t, found) + require.False(t, inflight) + require.Len(t, record.traces, 2, "success should republish the extended record") +} + +func TestProgressiveTraceCacheMakesPrefixDeclarationsAvailableToSuffix(t *testing.T) { + classHash := felt.FromUint64[felt.Felt](44) + classDefinition := &core.DeprecatedCairoClass{} + var calls atomic.Uint64 + virtualMachine := &progressiveTestVM{trace: func( + transactions []core.Transaction, + state core.StateReader, + ) (vm.ExecutionResults, error) { + results := progressiveTestResults(transactions) + switch calls.Add(1) { + case 1: + results.Traces[0].StateDiff.DeprecatedDeclaredClasses = []*felt.Felt{&classHash} + case 2: + declared, err := state.Class(&classHash) + require.NoError(t, err) + require.Same(t, classDefinition, declared.Class) + } + return results, nil + }} + handler, header, transactions, headState := newProgressiveTestHandler(t, virtualMachine) + headState.EXPECT().Class(&classHash).Return(&core.DeclaredClassDefinition{ + Class: classDefinition, + }, nil) + + _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) + require.Nil(t, rpcErr) + response, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) + require.Nil(t, rpcErr) + require.Len(t, response.Traces, 2) + require.Equal(t, uint64(2), calls.Load()) +} + +func TestTransactionTraceIfHashMatchesValidatesCachedEntry(t *testing.T) { + hash := felt.FromUint64[felt.TransactionHash](1) + otherHash := felt.FromUint64[felt.Felt](2) + trace := &TransactionTrace{} + tests := map[string]TracedBlockTransaction{ + "nil hash": {TraceRoot: trace}, + "nil trace": {TransactionHash: (*felt.Felt)(&hash)}, + "hash mismatch": {TransactionHash: &otherHash, TraceRoot: trace}, + } + for name, cached := range tests { + t.Run(name, func(t *testing.T) { + _, valid := transactionTraceIfHashMatches(cached, &hash) + require.False(t, valid) + }) + } + + result, valid := transactionTraceIfHashMatches(TracedBlockTransaction{ + TransactionHash: (*felt.Felt)(&hash), TraceRoot: trace, + }, &hash) + require.True(t, valid) + require.Equal(t, *trace, result) +} diff --git a/rpc/v10/trace_test.go b/rpc/v10/trace_test.go index 3ffed0b521..bb1cf2e24d 100644 --- a/rpc/v10/trace_test.go +++ b/rpc/v10/trace_test.go @@ -422,11 +422,12 @@ func TestTraceTransaction(t *testing.T) { []*felt.Felt{}, &vm.BlockInfo{Header: header}, gomock.Any(), - vm.TraceOptions{}).Return(vm.ExecutionResults{ - OverallFees: overallFee, - GasConsumed: gc, - Traces: []vm.TransactionTrace{vmTrace}, - NumSteps: stepsUsed, + vm.TraceOptions{ReturnInitialReads: true}).Return(vm.ExecutionResults{ + OverallFees: overallFee, + GasConsumed: gc, + Traces: []vm.TransactionTrace{vmTrace}, + NumSteps: stepsUsed, + InitialReads: &vm.InitialReads{}, }, nil) trace, httpHeader, rpcErr := handler.TraceTransaction(t.Context(), hash) @@ -632,6 +633,127 @@ func TestTraceTransaction(t *testing.T) { }) } +func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(mockCtrl) + mockVM := mocks.NewMockVM(mockCtrl) + baseState := mocks.NewMockStateReader(mockCtrl) + headState := mocks.NewMockStateReader(mockCtrl) + + header := &core.Header{ + Hash: felt.NewFromUint64[felt.Felt](100), + ParentHash: felt.NewFromUint64[felt.Felt](99), + SequencerAddress: felt.NewFromUint64[felt.Felt](98), + L1GasPriceETH: felt.NewFromUint64[felt.Felt](1), + ProtocolVersion: "99.12.3", + } + transactions := []core.Transaction{ + &core.InvokeTransaction{TransactionHash: felt.NewFromUint64[felt.Felt](10)}, + &core.InvokeTransaction{TransactionHash: felt.NewFromUint64[felt.Felt](11)}, + &core.InvokeTransaction{TransactionHash: felt.NewFromUint64[felt.Felt](12)}, + } + hashes := make([]*felt.TransactionHash, len(transactions)) + for index := range transactions { + hashes[index] = (*felt.TransactionHash)(transactions[index].Hash()) + } + + address := felt.FromUint64[felt.Felt](20) + addressTyped := felt.Address(address) + key1 := felt.FromUint64[felt.Felt](21) + key2 := felt.FromUint64[felt.Felt](22) + zero := felt.Zero + value1 := felt.FromUint64[felt.Felt](31) + value2 := felt.FromUint64[felt.Felt](32) + nonce1 := felt.FromUint64[felt.Felt](1) + + prefixTraces := []vm.TransactionTrace{ + {StateDiff: &vm.StateDiff{StorageDiffs: []vm.StorageDiff{{ + Address: address, StorageEntries: []vm.Entry{{Key: key1, Value: value1}}, + }}}}, + {StateDiff: &vm.StateDiff{Nonces: []vm.Nonce{{ContractAddress: address, Nonce: nonce1}}}}, + } + suffixTrace := vm.TransactionTrace{StateDiff: &vm.StateDiff{StorageDiffs: []vm.StorageDiff{{ + Address: address, StorageEntries: []vm.Entry{{Key: key2, Value: value2}}, + }}}} + prefixReads := &vm.InitialReads{ + Storage: []vm.InitialReadsStorageEntry{{ContractAddress: addressTyped, Key: key1, Value: zero}}, + Nonces: []vm.InitialReadsNonceEntry{{ContractAddress: addressTyped, Nonce: zero}}, + } + suffixReads := &vm.InitialReads{ + Storage: []vm.InitialReadsStorageEntry{ + {ContractAddress: addressTyped, Key: key1, Value: value1}, + {ContractAddress: addressTyped, Key: key2, Value: zero}, + }, + } + + mockReader.EXPECT().Network().Return(&networks.Mainnet).AnyTimes() + for _, target := range []uint64{1, 0, 2} { + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hashes[target]).Return(header.Number, target, nil) + mockReader.EXPECT().BlockHeaderByNumber(header.Number).Return(header, nil) + } + for range 2 { + mockReader.EXPECT().TransactionsByBlockNumber(header.Number).Return(transactions, nil) + } + mockReader.EXPECT().BlockHeaderByHash(header.Hash).Return(header, nil) + mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(baseState, nopCloser, nil).Times(2) + mockReader.EXPECT().HeadState().Return(headState, nopCloser, nil).Times(2) + + gomock.InOrder( + mockVM.EXPECT().Trace( + transactions[:2], []core.ClassDefinition(nil), []*felt.Felt{}, + &vm.BlockInfo{Header: header}, baseState, vm.TraceOptions{ReturnInitialReads: true}, + ).Return(vm.ExecutionResults{ + Traces: prefixTraces, GasConsumed: []core.GasConsumed{{L1Gas: 1}, {L1Gas: 2}}, + NumSteps: 10, InitialReads: prefixReads, + }, nil), + mockVM.EXPECT().Trace( + transactions[2:], []core.ClassDefinition(nil), []*felt.Felt{}, + &vm.BlockInfo{Header: header}, + gomock.Cond(func(state core.StateReader) bool { + storage, err := state.ContractStorage(&address, &key1) + if err != nil || !storage.Equal(&value1) { + return false + } + nonce, err := state.ContractNonce(&address) + return err == nil && nonce.Equal(&nonce1) + }), + vm.TraceOptions{ReturnInitialReads: true}, + ).Return(vm.ExecutionResults{ + Traces: []vm.TransactionTrace{suffixTrace}, GasConsumed: []core.GasConsumed{{L1Gas: 3}}, + NumSteps: 20, InitialReads: suffixReads, + }, nil), + ) + + handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) + + trace, responseHeader, rpcErr := handler.TraceTransaction(t.Context(), hashes[1]) + require.Nil(t, rpcErr) + require.Equal(t, "10", responseHeader.Get(rpcv10.ExecutionStepsHeader)) + require.Equal(t, uint64(2), trace.ExecutionResources.L1Gas) + + trace, responseHeader, rpcErr = handler.TraceTransaction(t.Context(), hashes[0]) + require.Nil(t, rpcErr) + require.Equal(t, "0", responseHeader.Get(rpcv10.ExecutionStepsHeader)) + require.Equal(t, uint64(1), trace.ExecutionResources.L1Gas) + + trace, responseHeader, rpcErr = handler.TraceTransaction(t.Context(), hashes[2]) + require.Nil(t, rpcErr) + require.Equal(t, "20", responseHeader.Get(rpcv10.ExecutionStepsHeader)) + require.Equal(t, uint64(3), trace.ExecutionResources.L1Gas) + + blockID := rpcv10.BlockIDFromHash(header.Hash) + blockResponse, responseHeader, rpcErr := handler.TraceBlockTransactions( + t.Context(), &blockID, []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag}, + ) + require.Nil(t, rpcErr) + require.Equal(t, "0", responseHeader.Get(rpcv10.ExecutionStepsHeader)) + require.Len(t, blockResponse.Traces, 3) + require.NotNil(t, blockResponse.InitialReads) + require.Len(t, blockResponse.InitialReads.Storage, 2) + require.Equal(t, zero, blockResponse.InitialReads.Storage[0].Value) + require.Equal(t, key2, blockResponse.InitialReads.Storage[1].Key) +} + func TestTraceBlockTransactions(t *testing.T) { errTests := map[string]rpcv10.BlockID{ "latest": rpcv10.BlockIDLatest(), @@ -727,13 +849,14 @@ func TestTraceBlockTransactions(t *testing.T) { []*felt.Felt{}, &vm.BlockInfo{Header: header}, gomock.Any(), - vm.TraceOptions{}). + vm.TraceOptions{ReturnInitialReads: true}). Return(vm.ExecutionResults{ OverallFees: nil, DataAvailability: []core.DataAvailability{{}, {}}, - GasConsumed: []core.GasConsumed{{}, {}}, + GasConsumed: []core.GasConsumed{{}}, Traces: []vm.TransactionTrace{vmTrace}, NumSteps: stepsUsed, + InitialReads: &vm.InitialReads{}, }, nil) expectedTrace := rpcv10.AdaptVMTransactionTrace(&vmTrace) @@ -1402,15 +1525,13 @@ func TestTraceBlockTransactionsWithReturnInitialReads(t *testing.T) { mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() mockReader.EXPECT().BlockHeaderHashByNumber(uint64(90)).Return(revealedHeader.Hash, nil) - returnInitialReads := slices.Contains(test.simulationFlags, rpcv10.ReturnInitialReadsFlag) - mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{ReturnInitialReads: returnInitialReads}, + vm.TraceOptions{ReturnInitialReads: true}, ).Return(vm.ExecutionResults{ OverallFees: []*felt.Felt{&felt.Zero}, DataAvailability: []core.DataAvailability{{L1Gas: 0}}, GasConsumed: []core.GasConsumed{{L1Gas: 0, L1DataGas: 0, L2Gas: 0}}, - Traces: []vm.TransactionTrace{{}}, + Traces: []vm.TransactionTrace{{StateDiff: &vm.StateDiff{}}}, NumSteps: 100, InitialReads: test.initialReads, }, nil) @@ -1424,6 +1545,11 @@ func TestTraceBlockTransactionsWithReturnInitialReads(t *testing.T) { test.traceFlags, ) + if test.initialReads == nil { + require.NotNil(t, err) + require.Contains(t, err.Data, "VM omitted initial reads") + return + } require.Nil(t, err) require.Equal(t, test.expectedInitialReads, traces.InitialReads) @@ -1491,17 +1617,15 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { OverallFees: []*felt.Felt{&felt.Zero}, DataAvailability: []core.DataAvailability{{L1Gas: 0}}, GasConsumed: []core.GasConsumed{{L1Gas: 0, L1DataGas: 0, L2Gas: 0}}, - Traces: []vm.TransactionTrace{{}}, + Traces: []vm.TransactionTrace{{StateDiff: &vm.StateDiff{}}}, NumSteps: 100, InitialReads: reads, } } - // After TraceTransaction has cached the block without initial reads, - // a follow-up TraceBlockTransactions with RETURN_INITIAL_READS must - // re-execute the VM and return populated reads — not the empty struct - // that was served from the poisoned cache. - t.Run("TraceTransaction does not poison RETURN_INITIAL_READS", func(t *testing.T) { + // TraceTransaction captures initial reads for continuation even though it does not return them. + // A follow-up block trace with RETURN_INITIAL_READS can therefore reuse the completed entry. + t.Run("TraceTransaction populates RETURN_INITIAL_READS cache", func(t *testing.T) { t.Parallel() mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) @@ -1522,20 +1646,13 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(block.Header, nil) mockReader.EXPECT().BlockHeaderByNumber(block.Number).Return(block.Header, nil) mockReader.EXPECT().TransactionsByBlockNumber(block.Number). - Return(block.Transactions, nil).Times(2) - mockReader.EXPECT().StateAtBlockHash(block.ParentHash).Return(mockState, nopCloser, nil).Times(2) - mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil).Times(2) + Return(block.Transactions, nil) + mockReader.EXPECT().StateAtBlockHash(block.ParentHash).Return(mockState, nopCloser, nil) + mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - // First VM call: no initial reads requested. - gomock.InOrder( - mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{}, - ).Return(execResultWithReads(nil), nil), - // Second VM call: flag set, VM produces populated reads. - mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{ReturnInitialReads: true}, - ).Return(execResultWithReads(populatedVMReads()), nil), - ) + mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, + vm.TraceOptions{ReturnInitialReads: true}, + ).Return(execResultWithReads(populatedVMReads()), nil) handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) From ff917ed5347727de7b67d5bed51c1f9a17c05e5c Mon Sep 17 00:00:00 2001 From: danielntmd Date: Thu, 3 Sep 2026 05:15:13 -0700 Subject: [PATCH 2/6] fix(rpc): preserve feeder initial reads wire format --- rpc/v10/trace.go | 4 ++-- rpc/v10/trace_test.go | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index 99e9aafe82..0fcbeb037e 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -469,10 +469,10 @@ func (h *Handler) traceFinalisedBlockSource( return finalisedBlockTraceSource{}, rpcErr } - // The gateway never supplies initial reads, so an empty set is final for these blocks. + // The gateway never supplies initial reads. Preserve its historical null-slice wire shape. response := TraceBlockTransactionsResponse{ Traces: traces, - InitialReads: emptyInitialReads(), + InitialReads: &InitialReads{}, } return finalisedBlockTraceSource{complete: &response}, nil } diff --git a/rpc/v10/trace_test.go b/rpc/v10/trace_test.go index bb1cf2e24d..ef0fde1811 100644 --- a/rpc/v10/trace_test.go +++ b/rpc/v10/trace_test.go @@ -190,6 +190,12 @@ func AssertTracedBlockTransactions( require.Nil(t, err) require.Equal(t, httpHeader.Get(rpcv10.ExecutionStepsHeader), "0") require.Equal(t, test.wantTrace, traces) + + withReads, _, err := handler.TraceBlockTransactions( + t.Context(), &blockID, []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag}, + ) + require.Nil(t, err) + require.Equal(t, &rpcv10.InitialReads{}, withReads.InitialReads) }) } } From 0c2c85558cff73ca7a8a74dfda4e59135be539e0 Mon Sep 17 00:00:00 2001 From: danielntmd Date: Mon, 7 Sep 2026 21:59:53 -0700 Subject: [PATCH 3/6] refactor(rpc): simplify progressive trace caching Collect initial reads on demand, replaying the block when cached traces lack requested reads. Shape responses at the RPC boundary and share finalised trace routing. Publish immutable cache records with per-block flight coordination. Trim test fixtures and strengthen checkpoint and prefix preservation coverage. --- rpc/v10/simulation.go | 9 + rpc/v10/trace.go | 161 +++++++---------- rpc/v10/trace_cache.go | 208 ++++++++-------------- rpc/v10/trace_cache_initial_reads.go | 69 ------- rpc/v10/trace_cache_initial_reads_test.go | 59 ------ rpc/v10/trace_cache_test.go | 122 +++++-------- rpc/v10/trace_progressive.go | 106 +++++------ rpc/v10/trace_progressive_test.go | 101 ++++++----- rpc/v10/trace_test.go | 186 +++++++------------ 9 files changed, 355 insertions(+), 666 deletions(-) delete mode 100644 rpc/v10/trace_cache_initial_reads.go delete mode 100644 rpc/v10/trace_cache_initial_reads_test.go diff --git a/rpc/v10/simulation.go b/rpc/v10/simulation.go index 828663e827..c49cc1783b 100644 --- a/rpc/v10/simulation.go +++ b/rpc/v10/simulation.go @@ -160,6 +160,15 @@ type InitialReads struct { DeclaredContracts []DeclaredContractEntry `json:"declared_contracts"` } +func emptyInitialReads() *InitialReads { + return &InitialReads{ + Storage: []StorageEntry{}, + Nonces: []NonceEntry{}, + ClassHashes: []ClassHashEntry{}, + DeclaredContracts: []DeclaredContractEntry{}, + } +} + type BroadcastedTransactionInputs = rpccore.LimitSlice[ BroadcastedTransaction, rpccore.SimulationLimit, diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index 0fcbeb037e..edaac6b859 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -31,13 +31,6 @@ type TransactionTrace struct { ExecutionResources *ExecutionResources `json:"execution_resources"` } -// finalisedBlockTraceSource contains either a complete gateway response or transactions for local -// replay. The caller owns publishing complete responses to the cache. -type finalisedBlockTraceSource struct { - complete *TraceBlockTransactionsResponse - transactions []core.Transaction -} - /**************************************************** Public API Handlers *****************************************************/ @@ -149,7 +142,11 @@ func (h *Handler) TraceBlockTransactions( } returnInitialReads := slices.Contains(traceFlags, TraceReturnInitialReadsFlag) - return h.traceFinalisedBlock(ctx, header, returnInitialReads) + response, responseHeader, rpcErr := h.traceFinalisedBlock(ctx, header, nil, returnInitialReads) + if !returnInitialReads { + response.InitialReads = nil + } + return response, responseHeader, rpcErr } /**************************************************** @@ -295,56 +292,31 @@ func (h *Handler) findAndTraceFinalisedTransaction( } if cached, found := h.blockTraceCache.traceAt(*header.Hash, txIndex); found { - trace, valid := transactionTraceIfHashMatches(cached, hash) - if !valid { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound - } - return trace, defaultExecutionHeader(), nil + return transactionTraceResponse(cached, hash, defaultExecutionHeader()) } - source, rpcErr := h.traceFinalisedBlockSource(ctx, header) + response, responseHeader, rpcErr := h.traceFinalisedBlock( + ctx, header, &traceTarget{index: txIndex, hash: *hash}, false, + ) if rpcErr != nil { - return TransactionTrace{}, defaultExecutionHeader(), rpcErr + return TransactionTrace{}, responseHeader, rpcErr } - - if source.complete != nil { - h.blockTraceCache.storeComplete(*header.Hash, *source.complete) - blockTraces := source.complete.Traces - if txIndex >= uint64(len(blockTraces)) { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound - } - trace, valid := transactionTraceIfHashMatches(blockTraces[txIndex], hash) - if !valid { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound - } - return trace, defaultExecutionHeader(), nil - } - transactions := source.transactions - - // txIndex comes from the tx-hash index while transactions come from a later block read. - if txIndex >= uint64(len(transactions)) || - !transactions[txIndex].Hash().Equal((*felt.Felt)(hash)) { + if txIndex >= uint64(len(response.Traces)) { return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } - - response, httpHeader, rpcErr := h.traceProgressiveBlock( - ctx, header, transactions, txIndex, false, - ) - if rpcErr != nil { - return TransactionTrace{}, httpHeader, rpcErr - } - return *response.Traces[txIndex].TraceRoot, httpHeader, nil + return transactionTraceResponse(response.Traces[txIndex], hash, responseHeader) } -func transactionTraceIfHashMatches( +func transactionTraceResponse( traced TracedBlockTransaction, hash *felt.TransactionHash, -) (TransactionTrace, bool) { + responseHeader http.Header, +) (TransactionTrace, http.Header, *jsonrpc.Error) { if traced.TransactionHash == nil || traced.TraceRoot == nil || !traced.TransactionHash.Equal((*felt.Felt)(hash)) { - return TransactionTrace{}, false + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } - return *traced.TraceRoot, true + return *traced.TraceRoot, responseHeader, nil } // findAndTraceInPreConfirmed traces a transaction located in any block of the @@ -406,85 +378,72 @@ func (h *Handler) findAndTraceInPreConfirmed( Block Tracing Helpers *****************************************************/ -// traceFinalisedBlock gets the trace for a block. The block will always be traced locally except -// on specific case such as with Starknet version 0.13.2 or lower or when it is certain range +// traceTarget identifies the last transaction to trace and the hash expected at that index. +type traceTarget struct { + index uint64 + hash felt.TransactionHash +} + +// traceFinalisedBlock returns traces through target, or the whole block when target is nil. +// Transaction callers check their prefix cache before entering this shared path. func (h *Handler) traceFinalisedBlock( ctx context.Context, header *core.Header, + target *traceTarget, returnInitialReads bool, ) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { cacheKey := *header.Hash - if response, complete := h.blockTraceCache.completeResponse(cacheKey); complete { - return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil - } - - source, rpcErr := h.traceFinalisedBlockSource(ctx, header) - if rpcErr != nil { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr - } - if source.complete != nil { - response := *source.complete - h.blockTraceCache.storeComplete(cacheKey, response) - return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil - } - transactions := source.transactions - if len(transactions) > 0 { - return h.traceProgressiveBlock( - ctx, header, transactions, uint64(len(transactions)-1), returnInitialReads, - ) - } - - // The VM produces no traces or execution steps for an empty block. Block preprocessing only - // writes to its temporary cached state, so it also produces no initial reads. - response := TraceBlockTransactionsResponse{ - Traces: []TracedBlockTransaction{}, - InitialReads: emptyInitialReads(), - } - return shapeTraceResponse(response, returnInitialReads), defaultExecutionHeader(), nil -} - -// shapeTraceResponse applies request flags at the RPC boundary; cached responses remain canonical. -func shapeTraceResponse( - response TraceBlockTransactionsResponse, - returnInitialReads bool, -) TraceBlockTransactionsResponse { - if !returnInitialReads { - response.InitialReads = nil + if target == nil { + response, complete := h.blockTraceCache.completeResponse(cacheKey, returnInitialReads) + if complete { + return response, defaultExecutionHeader(), nil + } } - return response -} -func (h *Handler) traceFinalisedBlockSource( - ctx context.Context, - header *core.Header, -) (finalisedBlockTraceSource, *jsonrpc.Error) { fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(header, h.bcReader.Network()) if err != nil { - return finalisedBlockTraceSource{}, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrUnexpectedError.CloneWithData(err.Error()) } - if fetchFromFeederGW { traces, rpcErr := h.fetchTracesFromFeederGateway(ctx, header) if rpcErr != nil { - return finalisedBlockTraceSource{}, rpcErr + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } - // The gateway never supplies initial reads. Preserve its historical null-slice wire shape. - response := TraceBlockTransactionsResponse{ - Traces: traces, - InitialReads: &InitialReads{}, - } - return finalisedBlockTraceSource{complete: &response}, nil + response := TraceBlockTransactionsResponse{Traces: traces, InitialReads: &InitialReads{}} + h.blockTraceCache.storeComplete(cacheKey, response) + return response, defaultExecutionHeader(), nil } transactions, err := h.bcReader.TransactionsByBlockNumber(header.Number) if err != nil { if errors.Is(err, db.ErrKeyNotFound) { - return finalisedBlockTraceSource{}, rpccore.ErrBlockNotFound + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound } - return finalisedBlockTraceSource{}, rpccore.ErrInternal.CloneWithData(err) + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrInternal.CloneWithData(err) } - return finalisedBlockTraceSource{transactions: transactions}, nil + + if target != nil { + // The tx-hash index and transaction list come from separate reads; validate before execution. + if target.index >= uint64(len(transactions)) || + !transactions[target.index].Hash().Equal((*felt.Felt)(&target.hash)) { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrTxnHashNotFound + } + return h.traceProgressiveBlock(ctx, header, transactions, target.index, returnInitialReads) + } + if len(transactions) > 0 { + return h.traceProgressiveBlock( + ctx, header, transactions, uint64(len(transactions)-1), returnInitialReads, + ) + } + + // Empty local blocks produce no traces or initial reads and are not cached. + return TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{}, + InitialReads: emptyInitialReads(), + }, defaultExecutionHeader(), nil } // fetchTracesFromFeederGateway fetches block traces from the feeder gateway diff --git a/rpc/v10/trace_cache.go b/rpc/v10/trace_cache.go index 35857935db..44c8f5b874 100644 --- a/rpc/v10/trace_cache.go +++ b/rpc/v10/trace_cache.go @@ -7,43 +7,28 @@ import ( "github.com/NethermindEth/juno/utils/lru" ) -// blockTraceCache retains successful trace progress in an LRU and permits one active extension per -// block. An extension becomes visible only after its entire requested suffix succeeds; failed work -// only wakes its waiters. mu protects LRU and flight membership, while per-record locks allow -// unrelated blocks to be read and extended concurrently. +// blockTraceCache publishes immutable trace prefixes and permits one active execution per block. +// mu protects cache and flight membership; execution and trace appends happen outside the lock. type blockTraceCache struct { - mu sync.Mutex - + mu sync.Mutex records *lru.SimpleCache[felt.Felt, *blockTraceRecord] - flights map[felt.Felt]*traceFlight + flights map[felt.Felt]chan struct{} } -// blockTraceRecord is the append-only tracing progress for one block. Returned trace slices are -// capacity-limited, so a later extension cannot change their visible length or elements. +// Published records are immutable. The flight owner may append beyond the prefix's length, +// but must publish a new record. Responses borrow read-only data with capacity-limited slices. type blockTraceRecord struct { - mu sync.RWMutex - - traces []TracedBlockTransaction - initialReads *InitialReads // non-nil once the record has been published - complete bool -} - -type blockTraceRecordView struct { traces []TracedBlockTransaction - initialReads *InitialReads + initialReads *InitialReads // only complete records may contain initial reads complete bool } -type traceFlight struct { - done chan struct{} -} - type traceCacheLookupKind uint8 const ( traceCacheHit traceCacheLookupKind = iota + 1 traceCacheWait - traceCacheExtend + traceCacheExecute ) type traceCacheLookup struct { @@ -53,98 +38,30 @@ type traceCacheLookup struct { work *traceCacheWork } -// traceCacheWork is owned by the caller executing a suffix. Work retains its record across LRU -// eviction; only a successful commit republishes it. After commit removes the flight, the deferred -// abort sees that it is no longer active and becomes a no-op. +// Work retains its prefix across eviction and publishes only after successful execution. type traceCacheWork struct { cache *blockTraceCache hash felt.Felt - flight *traceFlight + flight chan struct{} record *blockTraceRecord } -func (r *blockTraceRecord) view() blockTraceRecordView { - r.mu.RLock() - defer r.mu.RUnlock() - return r.viewLocked() -} - -func (r *blockTraceRecord) viewLocked() blockTraceRecordView { - return blockTraceRecordView{ - traces: r.traces[:len(r.traces):len(r.traces)], - initialReads: r.initialReads, - complete: r.complete, - } -} - -func (r *blockTraceRecord) append( - extension TraceBlockTransactionsResponse, - totalTransactions int, -) blockTraceRecordView { - r.mu.Lock() - defer r.mu.Unlock() - r.traces = append(r.traces, extension.Traces...) - r.initialReads = mergeInitialReads(r.initialReads, extension.InitialReads) - r.complete = len(r.traces) == totalTransactions - return r.viewLocked() -} - -func (v blockTraceRecordView) response() TraceBlockTransactionsResponse { - // Responses borrow record-owned data and must be treated as read-only. - response := TraceBlockTransactionsResponse{Traces: v.traces} - if v.complete { - response.InitialReads = v.initialReads - } - return response -} - -func (w *traceCacheWork) prefix() []TracedBlockTransaction { - return w.record.view().traces -} - -func (w *traceCacheWork) commit( - extension TraceBlockTransactionsResponse, - totalTransactions int, -) TraceBlockTransactionsResponse { - view := w.record.append(extension, totalTransactions) - - cache := w.cache - cache.mu.Lock() - defer cache.mu.Unlock() - cache.records.Add(w.hash, w.record) - cache.finishLocked(w.hash, w.flight) - return view.response() -} - -func (w *traceCacheWork) abort() { - cache := w.cache - cache.mu.Lock() - defer cache.mu.Unlock() - if cache.flights[w.hash] != w.flight { - return - } - cache.finishLocked(w.hash, w.flight) -} - func newBlockTraceCache(limit int) *blockTraceCache { return &blockTraceCache{ records: lru.NewSimple[felt.Felt, *blockTraceRecord](limit), - flights: make(map[felt.Felt]*traceFlight), + flights: make(map[felt.Felt]chan struct{}), } } func (c *blockTraceCache) completeResponse( blockHash felt.Felt, + requireInitialReads bool, ) (TraceBlockTransactionsResponse, bool) { record, found := c.record(blockHash) - if !found { + if !found || !record.complete || requireInitialReads && record.initialReads == nil { return TraceBlockTransactionsResponse{}, false } - view := record.view() - if !view.complete { - return TraceBlockTransactionsResponse{}, false - } - return view.response(), true + return record.response(), true } func (c *blockTraceCache) traceAt( @@ -152,65 +69,39 @@ func (c *blockTraceCache) traceAt( index uint64, ) (TracedBlockTransaction, bool) { record, found := c.record(blockHash) - if !found { + if !found || index >= uint64(len(record.traces)) { return TracedBlockTransaction{}, false } - view := record.view() - if index >= uint64(len(view.traces)) { - return TracedBlockTransaction{}, false - } - return view.traces[index], true -} - -func (c *blockTraceCache) record(blockHash felt.Felt) (*blockTraceRecord, bool) { - c.mu.Lock() - defer c.mu.Unlock() - return c.records.Get(blockHash) + return record.traces[index], true } func (c *blockTraceCache) lookupOrStart( blockHash felt.Felt, target uint64, + requireInitialReads bool, ) traceCacheLookup { c.mu.Lock() + defer c.mu.Unlock() record, _ := c.records.Get(blockHash) - if flight, found := c.flights[blockHash]; found { - c.mu.Unlock() - if lookup, found := lookupBlockTraceRecord(record, target); found { - return lookup - } - return traceCacheLookup{kind: traceCacheWait, done: flight.done} + if record != nil && target < uint64(len(record.traces)) && + (!requireInitialReads || record.initialReads != nil) { + return traceCacheLookup{kind: traceCacheHit, response: record.response()} } - defer c.mu.Unlock() - - if lookup, found := lookupBlockTraceRecord(record, target); found { - return lookup + if flight, found := c.flights[blockHash]; found { + return traceCacheLookup{kind: traceCacheWait, done: flight} } - if record == nil { + // Missing initial reads require a full replay. Keep the old record available until commit. + if record == nil || requireInitialReads { record = &blockTraceRecord{} } - flight := &traceFlight{done: make(chan struct{})} + flight := make(chan struct{}) c.flights[blockHash] = flight return traceCacheLookup{ - kind: traceCacheExtend, + kind: traceCacheExecute, work: &traceCacheWork{cache: c, hash: blockHash, flight: flight, record: record}, } } -func lookupBlockTraceRecord( - record *blockTraceRecord, - target uint64, -) (traceCacheLookup, bool) { - if record == nil { - return traceCacheLookup{}, false - } - view := record.view() - if target < uint64(len(view.traces)) { - return traceCacheLookup{kind: traceCacheHit, response: view.response()}, true - } - return traceCacheLookup{}, false -} - // storeComplete takes ownership of the response containers. InitialReads must be non-nil. func (c *blockTraceCache) storeComplete( blockHash felt.Felt, @@ -226,7 +117,48 @@ func (c *blockTraceCache) storeComplete( c.records.Add(blockHash, record) } -func (c *blockTraceCache) finishLocked(blockHash felt.Felt, flight *traceFlight) { +func (c *blockTraceCache) record(blockHash felt.Felt) (*blockTraceRecord, bool) { + c.mu.Lock() + defer c.mu.Unlock() + return c.records.Get(blockHash) +} + +func (c *blockTraceCache) finishLocked(blockHash felt.Felt, flight chan struct{}) { delete(c.flights, blockHash) - close(flight.done) + close(flight) +} + +func (r *blockTraceRecord) response() TraceBlockTransactionsResponse { + return TraceBlockTransactionsResponse{ + Traces: r.traces[:len(r.traces):len(r.traces)], + InitialReads: r.initialReads, + } +} + +func (w *traceCacheWork) commit( + executed TraceBlockTransactionsResponse, + totalTransactions int, +) TraceBlockTransactionsResponse { + record := &blockTraceRecord{ + traces: append(w.record.traces, executed.Traces...), + initialReads: executed.InitialReads, + } + record.complete = len(record.traces) == totalTransactions + + cache := w.cache + cache.mu.Lock() + defer cache.mu.Unlock() + cache.records.Add(w.hash, record) + cache.finishLocked(w.hash, w.flight) + return record.response() +} + +func (w *traceCacheWork) abort() { + cache := w.cache + cache.mu.Lock() + defer cache.mu.Unlock() + if cache.flights[w.hash] != w.flight { + return + } + cache.finishLocked(w.hash, w.flight) } diff --git a/rpc/v10/trace_cache_initial_reads.go b/rpc/v10/trace_cache_initial_reads.go deleted file mode 100644 index 8ddaba529b..0000000000 --- a/rpc/v10/trace_cache_initial_reads.go +++ /dev/null @@ -1,69 +0,0 @@ -package rpcv10 - -import "github.com/NethermindEth/juno/core/felt" - -func emptyInitialReads() *InitialReads { - return &InitialReads{ - Storage: []StorageEntry{}, - Nonces: []NonceEntry{}, - ClassHashes: []ClassHashEntry{}, - DeclaredContracts: []DeclaredContractEntry{}, - } -} - -// mergeInitialReads requires non-nil incoming reads and retains the earliest value for each key. -func mergeInitialReads(existing, incoming *InitialReads) *InitialReads { - if existing == nil { - return incoming - } - - type storageKey struct { - address felt.Address - key felt.Felt - } - storageSeen := make(map[storageKey]struct{}, len(existing.Storage)) - for _, read := range existing.Storage { - storageSeen[storageKey{address: read.ContractAddress, key: read.Key}] = struct{}{} - } - for _, read := range incoming.Storage { - key := storageKey{address: read.ContractAddress, key: read.Key} - if _, found := storageSeen[key]; !found { - existing.Storage = append(existing.Storage, read) - storageSeen[key] = struct{}{} - } - } - - nonceSeen := make(map[felt.Address]struct{}, len(existing.Nonces)) - for _, read := range existing.Nonces { - nonceSeen[read.ContractAddress] = struct{}{} - } - for _, read := range incoming.Nonces { - if _, found := nonceSeen[read.ContractAddress]; !found { - existing.Nonces = append(existing.Nonces, read) - nonceSeen[read.ContractAddress] = struct{}{} - } - } - - classHashSeen := make(map[felt.Address]struct{}, len(existing.ClassHashes)) - for _, read := range existing.ClassHashes { - classHashSeen[read.ContractAddress] = struct{}{} - } - for _, read := range incoming.ClassHashes { - if _, found := classHashSeen[read.ContractAddress]; !found { - existing.ClassHashes = append(existing.ClassHashes, read) - classHashSeen[read.ContractAddress] = struct{}{} - } - } - - declaredSeen := make(map[felt.ClassHash]struct{}, len(existing.DeclaredContracts)) - for _, read := range existing.DeclaredContracts { - declaredSeen[read.ClassHash] = struct{}{} - } - for _, read := range incoming.DeclaredContracts { - if _, found := declaredSeen[read.ClassHash]; !found { - existing.DeclaredContracts = append(existing.DeclaredContracts, read) - declaredSeen[read.ClassHash] = struct{}{} - } - } - return existing -} diff --git a/rpc/v10/trace_cache_initial_reads_test.go b/rpc/v10/trace_cache_initial_reads_test.go deleted file mode 100644 index 39d92defff..0000000000 --- a/rpc/v10/trace_cache_initial_reads_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package rpcv10 - -import ( - "testing" - - "github.com/NethermindEth/juno/core/felt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestMergeInitialReadsPreservesOriginalValues(t *testing.T) { - address1 := felt.FromUint64[felt.Address](1) - address2 := felt.FromUint64[felt.Address](2) - key1 := felt.FromUint64[felt.Felt](3) - key2 := felt.FromUint64[felt.Felt](4) - class1 := felt.FromUint64[felt.ClassHash](5) - class2 := felt.FromUint64[felt.ClassHash](6) - - existing := &InitialReads{ - Storage: []StorageEntry{{ - ContractAddress: address1, Key: key1, Value: felt.FromUint64[felt.Felt](10), - }}, - Nonces: []NonceEntry{{ContractAddress: address1, Nonce: felt.FromUint64[felt.Felt](11)}}, - ClassHashes: []ClassHashEntry{{ - ContractAddress: address1, ClassHash: class1, - }}, - DeclaredContracts: []DeclaredContractEntry{{ClassHash: class1, IsDeclared: false}}, - } - incoming := &InitialReads{ - Storage: []StorageEntry{ - {ContractAddress: address1, Key: key1, Value: felt.FromUint64[felt.Felt](100)}, - {ContractAddress: address2, Key: key2, Value: felt.FromUint64[felt.Felt](20)}, - }, - Nonces: []NonceEntry{ - {ContractAddress: address1, Nonce: felt.FromUint64[felt.Felt](101)}, - {ContractAddress: address2, Nonce: felt.FromUint64[felt.Felt](21)}, - }, - ClassHashes: []ClassHashEntry{ - {ContractAddress: address1, ClassHash: class2}, - {ContractAddress: address2, ClassHash: class2}, - }, - DeclaredContracts: []DeclaredContractEntry{ - {ClassHash: class1, IsDeclared: true}, - {ClassHash: class2, IsDeclared: false}, - }, - } - - merged := mergeInitialReads(existing, incoming) - require.Same(t, existing, merged) - require.Len(t, merged.Storage, 2) - assert.Equal(t, uint64(10), merged.Storage[0].Value.Uint64()) - assert.Equal(t, address2, merged.Storage[1].ContractAddress) - require.Len(t, merged.Nonces, 2) - assert.Equal(t, uint64(11), merged.Nonces[0].Nonce.Uint64()) - require.Len(t, merged.ClassHashes, 2) - assert.Equal(t, class1, merged.ClassHashes[0].ClassHash) - require.Len(t, merged.DeclaredContracts, 2) - assert.False(t, merged.DeclaredContracts[0].IsDeclared) -} diff --git a/rpc/v10/trace_cache_test.go b/rpc/v10/trace_cache_test.go index 325db6cc37..7aa0dd051b 100644 --- a/rpc/v10/trace_cache_test.go +++ b/rpc/v10/trace_cache_test.go @@ -2,7 +2,6 @@ package rpcv10 import ( "testing" - "time" "github.com/NethermindEth/juno/core/felt" "github.com/stretchr/testify/require" @@ -20,68 +19,29 @@ func blockTraceCacheState( } func TestBlockTraceRecordAppendPreservesPublishedPrefix(t *testing.T) { - baseHash := felt.FromUint64[felt.Felt](1) - suffixHash := felt.FromUint64[felt.Felt](2) - address := felt.FromUint64[felt.Address](3) - key := felt.FromUint64[felt.Felt](4) - baseValue := felt.FromUint64[felt.Felt](5) - record := &blockTraceRecord{ - traces: []TracedBlockTransaction{{TransactionHash: &baseHash}}, - initialReads: &InitialReads{Storage: []StorageEntry{{ - ContractAddress: address, Key: key, Value: baseValue, - }}}, - } - extension := TraceBlockTransactionsResponse{ - Traces: []TracedBlockTransaction{{TransactionHash: &suffixHash}}, - InitialReads: &InitialReads{Storage: []StorageEntry{{ - ContractAddress: address, - Key: felt.FromUint64[felt.Felt](6), - Value: felt.FromUint64[felt.Felt](7), - }}}, - } - - partial := record.view().response() - require.Nil(t, partial.InitialReads, "initial reads describe block pre-state only when complete") + cache := newBlockTraceCache(1) + blockHash := felt.FromUint64[felt.Felt](1) + secondHash := felt.FromUint64[felt.Felt](2) + // Spare capacity exercises appending to a shared backing array. + traces := make([]TracedBlockTransaction, 1, 4) + traces[0].TransactionHash = &blockHash + original := &blockTraceRecord{traces: traces} + cache.records.Add(blockHash, original) + partial := original.response() require.Equal(t, len(partial.Traces), cap(partial.Traces)) - response := record.append(extension, 2).response() + work := cache.lookupOrStart(blockHash, 1, false).work + response := work.commit(TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{{TransactionHash: &secondHash}}, + }, 2) require.Len(t, response.Traces, 2) - require.NotNil(t, response.InitialReads) - require.Len(t, response.InitialReads.Storage, 2) - require.Same(t, &baseHash, record.traces[0].TransactionHash) - require.Equal(t, uint64(5), record.initialReads.Storage[0].Value.Uint64()) - require.True(t, record.complete) - require.Len(t, partial.Traces, 1, "a previously returned prefix must not grow") -} - -func TestBlockTraceCacheReadsOtherRecordWhileOneIsLocked(t *testing.T) { - cache := newBlockTraceCache(2) - blockedHash := felt.FromUint64[felt.Felt](1) - otherHash := felt.FromUint64[felt.Felt](2) - completeResponse := func() TraceBlockTransactionsResponse { - return TraceBlockTransactionsResponse{ - Traces: make([]TracedBlockTransaction, 1), - InitialReads: emptyInitialReads(), - } - } - cache.storeComplete(blockedHash, completeResponse()) - cache.storeComplete(otherHash, completeResponse()) - - blockedRecord, found, _ := blockTraceCacheState(cache, blockedHash) - require.True(t, found) - blockedRecord.mu.Lock() - defer blockedRecord.mu.Unlock() - - otherDone := make(chan struct{}) - go func() { - defer close(otherDone) - _, _ = cache.traceAt(otherHash, 0) - }() - select { - case <-otherDone: - case <-time.After(time.Second): - t.Fatal("one record lock must not block another block's cache read") - } + require.Len(t, partial.Traces, 1) + require.Len(t, original.traces, 1) + require.Equal(t, blockHash, *partial.Traces[0].TransactionHash) + require.Equal(t, secondHash, *response.Traces[1].TransactionHash) + require.False(t, original.complete) + published, _, _ := blockTraceCacheState(cache, blockHash) + require.True(t, published.complete) } func TestBlockTraceCacheLookupTransitions(t *testing.T) { @@ -90,38 +50,54 @@ func TestBlockTraceCacheLookupTransitions(t *testing.T) { firstHash := felt.FromUint64[felt.Felt](2) secondHash := felt.FromUint64[felt.Felt](3) - first := cache.lookupOrStart(blockHash, 0) - require.Equal(t, traceCacheExtend, first.kind) + first := cache.lookupOrStart(blockHash, 0, false) + require.Equal(t, traceCacheExecute, first.kind) for _, target := range []uint64{0, 1} { - waiting := cache.lookupOrStart(blockHash, target) + waiting := cache.lookupOrStart(blockHash, target, false) require.Equal(t, traceCacheWait, waiting.kind) - require.True(t, first.work.flight.done == waiting.done) + require.True(t, first.work.flight == waiting.done) } prefix := first.work.commit(TraceBlockTransactionsResponse{ - Traces: []TracedBlockTransaction{{TransactionHash: &firstHash}}, - InitialReads: &InitialReads{}, + Traces: []TracedBlockTransaction{{TransactionHash: &firstHash}}, }, 2) require.Len(t, prefix.Traces, 1) require.Nil(t, prefix.InitialReads) select { - case <-first.work.flight.done: + case <-first.work.flight: default: t.Fatal("commit must wake flight waiters") } - hit := cache.lookupOrStart(blockHash, 0) + hit := cache.lookupOrStart(blockHash, 0, false) require.Equal(t, traceCacheHit, hit.kind) require.Len(t, hit.response.Traces, 1) - second := cache.lookupOrStart(blockHash, 1) - require.Equal(t, traceCacheExtend, second.kind) - require.Len(t, second.work.prefix(), 1) + second := cache.lookupOrStart(blockHash, 1, false) + require.Equal(t, traceCacheExecute, second.kind) + require.Len(t, second.work.record.traces, 1) complete := second.work.commit(TraceBlockTransactionsResponse{ - Traces: []TracedBlockTransaction{{TransactionHash: &secondHash}}, - InitialReads: &InitialReads{}, + Traces: []TracedBlockTransaction{{TransactionHash: &secondHash}}, }, 2) require.Len(t, complete.Traces, 2) + require.Nil(t, complete.InitialReads) + + withReads := cache.lookupOrStart(blockHash, 1, true) + require.Equal(t, traceCacheExecute, withReads.kind) + require.Empty(t, withReads.work.record.traces) + require.Equal(t, traceCacheHit, cache.lookupOrStart(blockHash, 1, false).kind) + withReads.work.abort() + require.Equal(t, traceCacheHit, cache.lookupOrStart(blockHash, 1, false).kind) + + withReads = cache.lookupOrStart(blockHash, 1, true) + complete = withReads.work.commit(TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{ + {TransactionHash: &firstHash}, + {TransactionHash: &secondHash}, + }, + InitialReads: emptyInitialReads(), + }, 2) require.NotNil(t, complete.InitialReads) + require.Equal(t, traceCacheHit, cache.lookupOrStart(blockHash, 1, true).kind) } diff --git a/rpc/v10/trace_progressive.go b/rpc/v10/trace_progressive.go index 00e1fd2ffb..2f67a1f58a 100644 --- a/rpc/v10/trace_progressive.go +++ b/rpc/v10/trace_progressive.go @@ -28,13 +28,17 @@ func (h *Handler) traceProgressiveBlock( "trace target index %d out of range for %d transactions", target, len(transactions), )) } + if returnInitialReads && target != uint64(len(transactions)-1) { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), + rpccore.ErrUnexpectedError.CloneWithData("initial reads require a complete block trace") + } blockHash := *header.Hash for { - lookup := h.blockTraceCache.lookupOrStart(blockHash, target) + lookup := h.blockTraceCache.lookupOrStart(blockHash, target, returnInitialReads) switch lookup.kind { case traceCacheHit: - return shapeTraceResponse(lookup.response, returnInitialReads), defaultExecutionHeader(), nil + return lookup.response, defaultExecutionHeader(), nil case traceCacheWait: select { case <-ctx.Done(): @@ -43,44 +47,34 @@ func (h *Handler) traceProgressiveBlock( case <-lookup.done: continue } - case traceCacheExtend: - response, responseHeader, rpcErr := h.executeTraceCacheWork( - lookup.work, header, transactions, target, + case traceCacheExecute: + work := lookup.work + //nolint:gocritic // safe to defer in loop: every execution path below returns or panics. + defer work.abort() + response, responseHeader, rpcErr := h.executeTraceRange( + header, transactions, work.record.traces, target, returnInitialReads, ) - return shapeTraceResponse(response, returnInitialReads), responseHeader, rpcErr + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, responseHeader, rpcErr + } + return work.commit(response, len(transactions)), responseHeader, nil default: panic("unknown trace cache lookup result") } } } -func (h *Handler) executeTraceCacheWork( - work *traceCacheWork, - header *core.Header, - transactions []core.Transaction, - target uint64, -) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { - // Always release waiters, including when VM execution or adaptation panics. - defer work.abort() - - response, responseHeader, rpcErr := h.executeTraceExtension( - header, transactions, work.prefix(), target, - ) - if rpcErr != nil { - return TraceBlockTransactionsResponse{}, responseHeader, rpcErr - } - - return work.commit(response, len(transactions)), responseHeader, nil -} - -func (h *Handler) executeTraceExtension( +// executeTraceRange traces from the end of cachedPrefix through target, inclusive. +// An empty prefix starts at the parent state, including when replaying to collect initial reads. +func (h *Handler) executeTraceRange( header *core.Header, transactions []core.Transaction, cachedPrefix []TracedBlockTransaction, target uint64, + returnInitialReads bool, ) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { start := uint64(len(cachedPrefix)) - base, baseCloser, err := h.bcReader.StateAtBlockHash(header.ParentHash) + parentState, parentCloser, err := h.bcReader.StateAtBlockHash(header.ParentHash) if err != nil { if errors.Is(err, db.ErrKeyNotFound) { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound @@ -88,38 +82,36 @@ func (h *Handler) executeTraceExtension( return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrInternal.CloneWithData(err) } - defer h.callAndLogErr(baseCloser, "Failed to close base state after trace extension") + defer h.callAndLogErr(parentCloser, "Failed to close parent state after trace execution") headState, headCloser, err := h.bcReader.HeadState() if err != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), jsonrpc.Err(jsonrpc.InternalError, err.Error()) } - defer h.callAndLogErr(headCloser, "Failed to close head state after trace extension") + defer h.callAndLogErr(headCloser, "Failed to close head state after trace execution") - executionState := base + executionState := parentState if start > 0 { checkpoint := checkpointFromTraces(cachedPrefix) - newClasses, rpcErr := checkpointClasses(&checkpoint, headState) + declaredClasses, rpcErr := loadCheckpointClasses(&checkpoint, headState) if rpcErr != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } - executionState = pending.NewState(&checkpoint, newClasses, base, header.Number) + executionState = pending.NewState(&checkpoint, declaredClasses, parentState, header.Number) } blockInfo, rpcErr := h.buildBlockInfo(header) if rpcErr != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } - // Always collect reads for future flagged requests; stitching retains Blockifier's earliest - // pre-state value. traces, vmInitialReads, responseHeader, rpcErr := traceTransactionsWithState( h.vm, transactions[start:target+1], executionState, headState, &blockInfo, - vm.TraceOptions{ReturnInitialReads: true}, + vm.TraceOptions{ReturnInitialReads: returnInitialReads}, start, ) if rpcErr != nil { @@ -136,17 +128,19 @@ func (h *Handler) executeTraceExtension( )) } } - if vmInitialReads == nil { - return TraceBlockTransactionsResponse{}, responseHeader, - rpccore.ErrUnexpectedError.CloneWithData("VM omitted initial reads for trace extension") + response := TraceBlockTransactionsResponse{Traces: traces} + if returnInitialReads { + if vmInitialReads == nil { + return TraceBlockTransactionsResponse{}, responseHeader, + rpccore.ErrUnexpectedError.CloneWithData("VM omitted initial reads for block trace") + } + adaptedReads := adaptVMInitialReads(vmInitialReads) + response.InitialReads = &adaptedReads } - adaptedReads := adaptVMInitialReads(vmInitialReads) - return TraceBlockTransactionsResponse{ - Traces: traces, InitialReads: &adaptedReads, - }, responseHeader, nil + return response, responseHeader, nil } -func checkpointClasses( +func loadCheckpointClasses( diff *core.StateDiff, classLookup core.StateReader, ) (map[felt.Felt]core.ClassDefinition, *jsonrpc.Error) { @@ -154,32 +148,24 @@ func checkpointClasses( map[felt.Felt]core.ClassDefinition, len(diff.DeclaredV0Classes)+len(diff.DeclaredV1Classes), ) - load := func(hash felt.Felt) *jsonrpc.Error { - if _, exists := classes[hash]; exists { - return nil - } - declared, err := classLookup.Class(&hash) - if err != nil { - return jsonrpc.Err(jsonrpc.InternalError, err.Error()) - } - classes[hash] = declared.Class - return nil - } for _, hash := range diff.DeclaredV0Classes { - if rpcErr := load(*hash); rpcErr != nil { - return nil, rpcErr - } + classes[*hash] = nil } for hash := range diff.DeclaredV1Classes { - if rpcErr := load(hash); rpcErr != nil { - return nil, rpcErr + classes[hash] = nil + } + for hash := range classes { + declared, err := classLookup.Class(&hash) + if err != nil { + return nil, jsonrpc.Err(jsonrpc.InternalError, err.Error()) } + classes[hash] = declared.Class } return classes, nil } // checkpointFromTraces rebuilds the continuation checkpoint from cached per-transaction state -// diffs. Progressive records only contain traces with non-nil state diffs; executeTraceExtension +// diffs. Progressive records only contain traces with non-nil state diffs; executeTraceRange // enforces this invariant before publishing an extension. The small recomputation cost avoids // retaining a duplicate cumulative state diff in the cache. func checkpointFromTraces(traces []TracedBlockTransaction) core.StateDiff { diff --git a/rpc/v10/trace_progressive_test.go b/rpc/v10/trace_progressive_test.go index c8638ac35e..a3727e33ce 100644 --- a/rpc/v10/trace_progressive_test.go +++ b/rpc/v10/trace_progressive_test.go @@ -53,10 +53,9 @@ func progressiveTestResults(transactions []core.Transaction) vm.ExecutionResults gas[index].L1Gas = transactions[index].Hash().Uint64() } return vm.ExecutionResults{ - Traces: traces, - GasConsumed: gas, - NumSteps: uint64(len(transactions)), - InitialReads: &vm.InitialReads{}, + Traces: traces, + GasConsumed: gas, + NumSteps: uint64(len(transactions)), } } @@ -67,7 +66,8 @@ func newProgressiveTestHandler( t.Helper() ctrl := gomock.NewController(t) reader := mocks.NewMockReader(ctrl) - state := mocks.NewMockStateReader(ctrl) + parentState := mocks.NewMockStateReader(ctrl) + headState := mocks.NewMockStateReader(ctrl) header := &core.Header{ Hash: felt.NewFromUint64[felt.Felt](100), ParentHash: felt.NewFromUint64[felt.Felt](99), @@ -77,10 +77,10 @@ func newProgressiveTestHandler( ProtocolVersion: "99.12.3", } reader.EXPECT().StateAtBlockHash(header.ParentHash). - Return(state, func() error { return nil }, nil).AnyTimes() - reader.EXPECT().HeadState().Return(state, func() error { return nil }, nil).AnyTimes() + Return(parentState, func() error { return nil }, nil).AnyTimes() + reader.EXPECT().HeadState().Return(headState, func() error { return nil }, nil).AnyTimes() handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) - return handler, header, progressiveTestTransactions(3), state + return handler, header, progressiveTestTransactions(3), headState } func TestMergeRPCStateDiff(t *testing.T) { @@ -94,8 +94,7 @@ func TestMergeRPCStateDiff(t *testing.T) { migratedClass := felt.FromUint64[felt.SierraClassHash](8) migratedCompiled := felt.FromUint64[felt.CasmClassHash](9) - converted := core.EmptyStateDiff() - mergeRPCStateDiff(&converted, &StateDiff{ + diff := StateDiff{ StorageDiffs: []StorageDiff{{ Address: address, StorageEntries: []Entry{{Key: key, Value: value}}, }}, @@ -109,7 +108,9 @@ func TestMergeRPCStateDiff(t *testing.T) { MigratedCompiledClasses: []MigratedCompiledClass{{ ClassHash: migratedClass, CompiledClassHash: migratedCompiled, }}, - }) + } + converted := core.EmptyStateDiff() + mergeRPCStateDiff(&converted, &diff) require.Equal(t, value, *converted.StorageDiffs[address][key]) require.Equal(t, nonce, *converted.Nonces[address]) @@ -119,7 +120,7 @@ func TestMergeRPCStateDiff(t *testing.T) { require.Equal(t, replacement, *converted.ReplacedClasses[address]) require.Equal(t, migratedCompiled, converted.MigratedClasses[migratedClass]) - value.SetUint64(99) + diff.StorageDiffs[0].StorageEntries[0].Value.SetUint64(99) require.Equal(t, uint64(3), converted.StorageDiffs[address][key].Uint64()) } @@ -220,27 +221,7 @@ func TestProgressiveTraceCacheDoesNotCacheFailedFirstExtension(t *testing.T) { require.False(t, inflight) } -func TestProgressiveTraceCacheRejectsMissingStateDiff(t *testing.T) { - virtualMachine := &progressiveTestVM{trace: func( - transactions []core.Transaction, - _ core.StateReader, - ) (vm.ExecutionResults, error) { - results := progressiveTestResults(transactions) - results.Traces[0].StateDiff = nil - return results, nil - }} - handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) - - _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) - require.NotNil(t, rpcErr) - require.Contains(t, rpcErr.Data, "VM omitted state diff for transaction trace 0") - - _, cached, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) - require.False(t, cached) - require.False(t, inflight) -} - -func TestProgressiveTraceCacheRejectsMismatchedVMResults(t *testing.T) { +func TestProgressiveTraceCacheRejectsMalformedVMResults(t *testing.T) { tests := []struct { name string traceCount int @@ -249,6 +230,7 @@ func TestProgressiveTraceCacheRejectsMismatchedVMResults(t *testing.T) { }{ {"too few traces", 1, 2, "unexpected number of transaction traces"}, {"too few gas results", 2, 1, "unexpected number of gas results"}, + {"missing state diff", 2, 2, "VM omitted state diff for transaction trace 0"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -264,6 +246,9 @@ func TestProgressiveTraceCacheRejectsMismatchedVMResults(t *testing.T) { _, _, rpcErr := handler.traceProgressiveBlock(t.Context(), header, transactions, 1, false) require.NotNil(t, rpcErr) require.Contains(t, rpcErr.Data, test.want) + _, cached, inflight := blockTraceCacheState(handler.blockTraceCache, *header.Hash) + require.False(t, cached) + require.False(t, inflight) }) } } @@ -281,12 +266,9 @@ func TestProgressiveTraceCachePanicClearsInflight(t *testing.T) { }} handler, header, transactions, _ := newProgressiveTestHandler(t, virtualMachine) - func() { - defer func() { - require.Equal(t, "trace panic", recover()) - }() + require.PanicsWithValue(t, "trace panic", func() { _, _, _ = handler.traceProgressiveBlock(t.Context(), header, transactions, 0, false) - }() + }) response, _, rpcErr := handler.traceProgressiveBlock( t.Context(), header, transactions, 0, false, @@ -296,6 +278,27 @@ func TestProgressiveTraceCachePanicClearsInflight(t *testing.T) { require.Equal(t, uint64(2), calls.Load()) } +func TestTraceFinalisedBlockRejectsInvalidTargetBeforeExecution(t *testing.T) { + ctrl := gomock.NewController(t) + reader := mocks.NewMockReader(ctrl) + virtualMachine := mocks.NewMockVM(ctrl) + header := &core.Header{Hash: felt.NewFromUint64[felt.Felt](100), ProtocolVersion: "99.12.3"} + transactions := progressiveTestTransactions(1) + reader.EXPECT().Network().Return(&networks.Mainnet).Times(2) + reader.EXPECT().TransactionsByBlockNumber(header.Number).Return(transactions, nil).Times(2) + handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) + + for name, target := range map[string]traceTarget{ + "index out of range": {index: 1, hash: felt.TransactionHash(*transactions[0].Hash())}, + "hash mismatch": {index: 0, hash: felt.FromUint64[felt.TransactionHash](999)}, + } { + t.Run(name, func(t *testing.T) { + _, _, rpcErr := handler.traceFinalisedBlock(t.Context(), header, &target, false) + require.Equal(t, rpccore.ErrTxnHashNotFound, rpcErr) + }) + } +} + func TestTraceFinalisedEmptyBlockReturnsWithoutCaching(t *testing.T) { ctrl := gomock.NewController(t) reader := mocks.NewMockReader(ctrl) @@ -310,7 +313,7 @@ func TestTraceFinalisedEmptyBlockReturnsWithoutCaching(t *testing.T) { handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) response, responseHeader, rpcErr := handler.traceFinalisedBlock( - t.Context(), header, true, + t.Context(), header, nil, true, ) require.Nil(t, rpcErr) require.Empty(t, response.Traces) @@ -321,10 +324,10 @@ func TestTraceFinalisedEmptyBlockReturnsWithoutCaching(t *testing.T) { require.Empty(t, response.InitialReads.DeclaredContracts) require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) - response, responseHeader, rpcErr = handler.traceFinalisedBlock(t.Context(), header, false) + response, responseHeader, rpcErr = handler.traceFinalisedBlock(t.Context(), header, nil, false) require.Nil(t, rpcErr) require.Empty(t, response.Traces) - require.Nil(t, response.InitialReads) + require.NotNil(t, response.InitialReads, "internal responses remain canonical") require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) } @@ -363,7 +366,7 @@ func TestProgressiveTraceCacheAllowsRecordEvictionDuringFlight(t *testing.T) { require.False(t, found, "the base record may be evicted while its owner retains it") require.True(t, inflight, "active work must remain discoverable through its flight") - waiting := handler.blockTraceCache.lookupOrStart(*header.Hash, 1) + waiting := handler.blockTraceCache.lookupOrStart(*header.Hash, 1, false) require.Equal(t, traceCacheWait, waiting.kind) close(release) @@ -412,7 +415,7 @@ func TestProgressiveTraceCacheMakesPrefixDeclarationsAvailableToSuffix(t *testin require.Equal(t, uint64(2), calls.Load()) } -func TestTransactionTraceIfHashMatchesValidatesCachedEntry(t *testing.T) { +func TestTransactionTraceResponseValidatesEntry(t *testing.T) { hash := felt.FromUint64[felt.TransactionHash](1) otherHash := felt.FromUint64[felt.Felt](2) trace := &TransactionTrace{} @@ -423,14 +426,16 @@ func TestTransactionTraceIfHashMatchesValidatesCachedEntry(t *testing.T) { } for name, cached := range tests { t.Run(name, func(t *testing.T) { - _, valid := transactionTraceIfHashMatches(cached, &hash) - require.False(t, valid) + _, header, rpcErr := transactionTraceResponse(cached, &hash, defaultExecutionHeader()) + require.Equal(t, rpccore.ErrTxnHashNotFound, rpcErr) + require.Nil(t, header) }) } - result, valid := transactionTraceIfHashMatches(TracedBlockTransaction{ + result, header, rpcErr := transactionTraceResponse(TracedBlockTransaction{ TransactionHash: (*felt.Felt)(&hash), TraceRoot: trace, - }, &hash) - require.True(t, valid) + }, &hash, defaultExecutionHeader()) + require.Nil(t, rpcErr) + require.Equal(t, "0", header.Get(ExecutionStepsHeader)) require.Equal(t, *trace, result) } diff --git a/rpc/v10/trace_test.go b/rpc/v10/trace_test.go index ef0fde1811..dbb4b2447f 100644 --- a/rpc/v10/trace_test.go +++ b/rpc/v10/trace_test.go @@ -428,12 +428,11 @@ func TestTraceTransaction(t *testing.T) { []*felt.Felt{}, &vm.BlockInfo{Header: header}, gomock.Any(), - vm.TraceOptions{ReturnInitialReads: true}).Return(vm.ExecutionResults{ - OverallFees: overallFee, - GasConsumed: gc, - Traces: []vm.TransactionTrace{vmTrace}, - NumSteps: stepsUsed, - InitialReads: &vm.InitialReads{}, + vm.TraceOptions{}).Return(vm.ExecutionResults{ + OverallFees: overallFee, + GasConsumed: gc, + Traces: []vm.TransactionTrace{vmTrace}, + NumSteps: stepsUsed, }, nil) trace, httpHeader, rpcErr := handler.TraceTransaction(t.Context(), hash) @@ -664,12 +663,8 @@ func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { } address := felt.FromUint64[felt.Felt](20) - addressTyped := felt.Address(address) key1 := felt.FromUint64[felt.Felt](21) - key2 := felt.FromUint64[felt.Felt](22) - zero := felt.Zero value1 := felt.FromUint64[felt.Felt](31) - value2 := felt.FromUint64[felt.Felt](32) nonce1 := felt.FromUint64[felt.Felt](1) prefixTraces := []vm.TransactionTrace{ @@ -678,28 +673,12 @@ func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { }}}}, {StateDiff: &vm.StateDiff{Nonces: []vm.Nonce{{ContractAddress: address, Nonce: nonce1}}}}, } - suffixTrace := vm.TransactionTrace{StateDiff: &vm.StateDiff{StorageDiffs: []vm.StorageDiff{{ - Address: address, StorageEntries: []vm.Entry{{Key: key2, Value: value2}}, - }}}} - prefixReads := &vm.InitialReads{ - Storage: []vm.InitialReadsStorageEntry{{ContractAddress: addressTyped, Key: key1, Value: zero}}, - Nonces: []vm.InitialReadsNonceEntry{{ContractAddress: addressTyped, Nonce: zero}}, - } - suffixReads := &vm.InitialReads{ - Storage: []vm.InitialReadsStorageEntry{ - {ContractAddress: addressTyped, Key: key1, Value: value1}, - {ContractAddress: addressTyped, Key: key2, Value: zero}, - }, - } - mockReader.EXPECT().Network().Return(&networks.Mainnet).AnyTimes() for _, target := range []uint64{1, 0, 2} { mockReader.EXPECT().BlockNumberAndIndexByTxHash(hashes[target]).Return(header.Number, target, nil) mockReader.EXPECT().BlockHeaderByNumber(header.Number).Return(header, nil) } - for range 2 { - mockReader.EXPECT().TransactionsByBlockNumber(header.Number).Return(transactions, nil) - } + mockReader.EXPECT().TransactionsByBlockNumber(header.Number).Return(transactions, nil).Times(2) mockReader.EXPECT().BlockHeaderByHash(header.Hash).Return(header, nil) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(baseState, nopCloser, nil).Times(2) mockReader.EXPECT().HeadState().Return(headState, nopCloser, nil).Times(2) @@ -707,10 +686,10 @@ func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { gomock.InOrder( mockVM.EXPECT().Trace( transactions[:2], []core.ClassDefinition(nil), []*felt.Felt{}, - &vm.BlockInfo{Header: header}, baseState, vm.TraceOptions{ReturnInitialReads: true}, + &vm.BlockInfo{Header: header}, baseState, vm.TraceOptions{}, ).Return(vm.ExecutionResults{ Traces: prefixTraces, GasConsumed: []core.GasConsumed{{L1Gas: 1}, {L1Gas: 2}}, - NumSteps: 10, InitialReads: prefixReads, + NumSteps: 10, }, nil), mockVM.EXPECT().Trace( transactions[2:], []core.ClassDefinition(nil), []*felt.Felt{}, @@ -723,10 +702,11 @@ func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { nonce, err := state.ContractNonce(&address) return err == nil && nonce.Equal(&nonce1) }), - vm.TraceOptions{ReturnInitialReads: true}, + vm.TraceOptions{}, ).Return(vm.ExecutionResults{ - Traces: []vm.TransactionTrace{suffixTrace}, GasConsumed: []core.GasConsumed{{L1Gas: 3}}, - NumSteps: 20, InitialReads: suffixReads, + Traces: []vm.TransactionTrace{{StateDiff: &vm.StateDiff{}}}, + GasConsumed: []core.GasConsumed{{L1Gas: 3}}, + NumSteps: 20, }, nil), ) @@ -749,15 +729,12 @@ func TestProgressiveTraceCacheExtendsPrefix(t *testing.T) { blockID := rpcv10.BlockIDFromHash(header.Hash) blockResponse, responseHeader, rpcErr := handler.TraceBlockTransactions( - t.Context(), &blockID, []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag}, + t.Context(), &blockID, nil, ) require.Nil(t, rpcErr) require.Equal(t, "0", responseHeader.Get(rpcv10.ExecutionStepsHeader)) require.Len(t, blockResponse.Traces, 3) - require.NotNil(t, blockResponse.InitialReads) - require.Len(t, blockResponse.InitialReads.Storage, 2) - require.Equal(t, zero, blockResponse.InitialReads.Storage[0].Value) - require.Equal(t, key2, blockResponse.InitialReads.Storage[1].Key) + require.Nil(t, blockResponse.InitialReads) } func TestTraceBlockTransactions(t *testing.T) { @@ -855,14 +832,13 @@ func TestTraceBlockTransactions(t *testing.T) { []*felt.Felt{}, &vm.BlockInfo{Header: header}, gomock.Any(), - vm.TraceOptions{ReturnInitialReads: true}). + vm.TraceOptions{}). Return(vm.ExecutionResults{ OverallFees: nil, DataAvailability: []core.DataAvailability{{}, {}}, GasConsumed: []core.GasConsumed{{}}, Traces: []vm.TransactionTrace{vmTrace}, NumSteps: stepsUsed, - InitialReads: &vm.InitialReads{}, }, nil) expectedTrace := rpcv10.AdaptVMTransactionTrace(&vmTrace) @@ -1531,8 +1507,9 @@ func TestTraceBlockTransactionsWithReturnInitialReads(t *testing.T) { mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() mockReader.EXPECT().BlockHeaderHashByNumber(uint64(90)).Return(revealedHeader.Hash, nil) + returnInitialReads := slices.Contains(test.traceFlags, rpcv10.TraceReturnInitialReadsFlag) mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{ReturnInitialReads: true}, + vm.TraceOptions{ReturnInitialReads: returnInitialReads}, ).Return(vm.ExecutionResults{ OverallFees: []*felt.Felt{&felt.Zero}, DataAvailability: []core.DataAvailability{{L1Gas: 0}}, @@ -1563,10 +1540,9 @@ func TestTraceBlockTransactionsWithReturnInitialReads(t *testing.T) { } } -// TestTraceBlockTransactionsInitialReadsCacheCoherence verifies that the -// block-trace cache does not poison RETURN_INITIAL_READS responses. The cache -// key is the block hash only, so a prior call without the flag used to cause -// subsequent calls with the flag to return empty initial reads. +// TestTraceBlockTransactionsInitialReadsCacheCoherence verifies that read-less cache entries are +// replayed when RETURN_INITIAL_READS is later requested and that read-populated entries serve both +// response shapes. func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { t.Parallel() n := &networks.Mainnet @@ -1599,6 +1575,7 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { parentHash := felt.FromUint64[felt.Felt](998) revealedHash := felt.FromUint64[felt.Felt](90) txHash := felt.FromUint64[felt.Felt](888) + secondTxHash := felt.FromUint64[felt.Felt](889) header := &core.Header{ SequencerAddress: n.BlockHashMetaInfo.FallBackSequencerAddress, L1GasPriceETH: &felt.Zero, @@ -1612,26 +1589,32 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { ProtocolVersion: "0.13.2", } block := &core.Block{ - Header: header, - Transactions: []core.Transaction{&core.InvokeTransaction{TransactionHash: &txHash}}, + Header: header, + Transactions: []core.Transaction{ + &core.InvokeTransaction{TransactionHash: &txHash}, + &core.InvokeTransaction{TransactionHash: &secondTxHash}, + }, } return block, &blockHash, &txHash, &core.Header{Hash: &revealedHash} } - execResultWithReads := func(reads *vm.InitialReads) vm.ExecutionResults { + execResultWithReads := func(transactionCount int, reads *vm.InitialReads) vm.ExecutionResults { + traces := make([]vm.TransactionTrace, transactionCount) + for i := range transactionCount { + traces[i].StateDiff = &vm.StateDiff{} + } + return vm.ExecutionResults{ - OverallFees: []*felt.Felt{&felt.Zero}, - DataAvailability: []core.DataAvailability{{L1Gas: 0}}, - GasConsumed: []core.GasConsumed{{L1Gas: 0, L1DataGas: 0, L2Gas: 0}}, - Traces: []vm.TransactionTrace{{StateDiff: &vm.StateDiff{}}}, - NumSteps: 100, - InitialReads: reads, + GasConsumed: make([]core.GasConsumed, transactionCount), + Traces: traces, + NumSteps: 100, + InitialReads: reads, } } - // TraceTransaction captures initial reads for continuation even though it does not return them. - // A follow-up block trace with RETURN_INITIAL_READS can therefore reuse the completed entry. - t.Run("TraceTransaction populates RETURN_INITIAL_READS cache", func(t *testing.T) { + // TraceTransaction caches no reads. A follow-up block trace with RETURN_INITIAL_READS must + // replay the block and replace the read-less entry. + t.Run("flagged request replays read-less cache entry", func(t *testing.T) { t.Parallel() mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) @@ -1652,13 +1635,20 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(block.Header, nil) mockReader.EXPECT().BlockHeaderByNumber(block.Number).Return(block.Header, nil) mockReader.EXPECT().TransactionsByBlockNumber(block.Number). - Return(block.Transactions, nil) - mockReader.EXPECT().StateAtBlockHash(block.ParentHash).Return(mockState, nopCloser, nil) - mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - - mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{ReturnInitialReads: true}, - ).Return(execResultWithReads(populatedVMReads()), nil) + Return(block.Transactions, nil).Times(2) + mockReader.EXPECT().StateAtBlockHash(block.ParentHash). + Return(mockState, nopCloser, nil).Times(2) + mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil).Times(2) + + gomock.InOrder( + mockVM.EXPECT().Trace( + block.Transactions[:1], gomock.Any(), gomock.Any(), gomock.Any(), mockState, + vm.TraceOptions{}, + ).Return(execResultWithReads(1, nil), nil), + mockVM.EXPECT().Trace(block.Transactions, gomock.Any(), gomock.Any(), gomock.Any(), mockState, + vm.TraceOptions{ReturnInitialReads: true}, + ).Return(execResultWithReads(len(block.Transactions), populatedVMReads()), nil), + ) handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) @@ -1674,9 +1664,8 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { require.Equal(t, expectedPopulatedReads, result.InitialReads) }) - // With the flag first, the cached entry already has initial reads, so a - // repeat call must be served from cache (VM invoked exactly once). - t.Run("cache reused when initial reads already cached", func(t *testing.T) { + // One VM call serves both response shapes without stripping reads from the cache. + t.Run("cache reused across flag changes", func(t *testing.T) { t.Parallel() mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) @@ -1692,7 +1681,7 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { BlockHeaderHashByNumber(uint64(90)). Return(revealedHeader.Hash, nil). AnyTimes() - mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(block.Header, nil).Times(2) + mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(block.Header, nil).Times(4) // The cached follow-up serves from the header alone, so transactions are read once. mockReader.EXPECT().TransactionsByBlockNumber(block.Number). Return(block.Transactions, nil) @@ -1701,62 +1690,23 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, vm.TraceOptions{ReturnInitialReads: true}, - ).Return(execResultWithReads(populatedVMReads()), nil) + ).Return(execResultWithReads(len(block.Transactions), populatedVMReads()), nil) handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) blockID := rpcv10.BlockIDFromHash(blockHash) - for range 2 { - result, _, err := handler.TraceBlockTransactions( - t.Context(), &blockID, - []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag}, - ) + for _, withReads := range []bool{true, true, false, true} { + var flags []rpcv10.TraceFlag + if withReads { + flags = []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag} + } + result, _, err := handler.TraceBlockTransactions(t.Context(), &blockID, flags) require.Nil(t, err) - require.Equal(t, expectedPopulatedReads, result.InitialReads) + if withReads { + require.Equal(t, expectedPopulatedReads, result.InitialReads) + } else { + require.Nil(t, result.InitialReads) + } } }) - - // With the flag first, then without: the second call must be served from - // cache and strip initial reads from the response. - t.Run("cache reused for flag-less follow-up", func(t *testing.T) { - t.Parallel() - mockCtrl := gomock.NewController(t) - t.Cleanup(mockCtrl.Finish) - - mockReader := mocks.NewMockReader(mockCtrl) - mockVM := mocks.NewMockVM(mockCtrl) - mockState := mocks.NewMockStateReader(mockCtrl) - block, blockHash, _, revealedHeader := buildBlock() - - mockReader.EXPECT().Network().Return(n).AnyTimes() - mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() - mockReader.EXPECT(). - BlockHeaderHashByNumber(uint64(90)). - Return(revealedHeader.Hash, nil). - AnyTimes() - mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(block.Header, nil).Times(2) - // The cached follow-up serves from the header alone, so transactions are read once. - mockReader.EXPECT().TransactionsByBlockNumber(block.Number). - Return(block.Transactions, nil) - mockReader.EXPECT().StateAtBlockHash(block.ParentHash).Return(mockState, nopCloser, nil) - mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - - mockVM.EXPECT().Trace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), mockState, - vm.TraceOptions{ReturnInitialReads: true}, - ).Return(execResultWithReads(populatedVMReads()), nil) - - handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) - - blockID := rpcv10.BlockIDFromHash(blockHash) - first, _, err := handler.TraceBlockTransactions( - t.Context(), &blockID, - []rpcv10.TraceFlag{rpcv10.TraceReturnInitialReadsFlag}, - ) - require.Nil(t, err) - require.Equal(t, expectedPopulatedReads, first.InitialReads) - - second, _, err := handler.TraceBlockTransactions(t.Context(), &blockID, nil) - require.Nil(t, err) - require.Nil(t, second.InitialReads) - }) } From 2c17da6be904108c9cb28074eca5ace1a5d94d4a Mon Sep 17 00:00:00 2001 From: danielntmd Date: Wed, 9 Sep 2026 11:51:58 -0700 Subject: [PATCH 4/6] perf(rpc): skip unrequested empty-block initial reads allocation --- rpc/v10/trace.go | 11 +++++++---- rpc/v10/trace_progressive_test.go | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index edaac6b859..900e9854d6 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -440,10 +440,13 @@ func (h *Handler) traceFinalisedBlock( } // Empty local blocks produce no traces or initial reads and are not cached. - return TraceBlockTransactionsResponse{ - Traces: []TracedBlockTransaction{}, - InitialReads: emptyInitialReads(), - }, defaultExecutionHeader(), nil + response := TraceBlockTransactionsResponse{ + Traces: []TracedBlockTransaction{}, + } + if returnInitialReads { + response.InitialReads = emptyInitialReads() + } + return response, defaultExecutionHeader(), nil } // fetchTracesFromFeederGateway fetches block traces from the feeder gateway diff --git a/rpc/v10/trace_progressive_test.go b/rpc/v10/trace_progressive_test.go index a3727e33ce..1d63b6163c 100644 --- a/rpc/v10/trace_progressive_test.go +++ b/rpc/v10/trace_progressive_test.go @@ -327,7 +327,7 @@ func TestTraceFinalisedEmptyBlockReturnsWithoutCaching(t *testing.T) { response, responseHeader, rpcErr = handler.traceFinalisedBlock(t.Context(), header, nil, false) require.Nil(t, rpcErr) require.Empty(t, response.Traces) - require.NotNil(t, response.InitialReads, "internal responses remain canonical") + require.Nil(t, response.InitialReads) require.Equal(t, "0", responseHeader.Get(ExecutionStepsHeader)) } From 6f499c949c672d8e77f9c12301826b4e6e96e354 Mon Sep 17 00:00:00 2001 From: danielntmd Date: Wed, 9 Sep 2026 12:36:18 -0700 Subject: [PATCH 5/6] perf(rpc): pass trace cache helper hashes by pointer --- rpc/v10/trace.go | 4 ++-- rpc/v10/trace_cache.go | 20 ++++++++++---------- rpc/v10/trace_progressive_test.go | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index 900e9854d6..ad60061a45 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -291,7 +291,7 @@ func (h *Handler) findAndTraceFinalisedTransaction( return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } - if cached, found := h.blockTraceCache.traceAt(*header.Hash, txIndex); found { + if cached, found := h.blockTraceCache.traceAt(header.Hash, txIndex); found { return transactionTraceResponse(cached, hash, defaultExecutionHeader()) } @@ -392,7 +392,7 @@ func (h *Handler) traceFinalisedBlock( target *traceTarget, returnInitialReads bool, ) (TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error) { - cacheKey := *header.Hash + cacheKey := header.Hash if target == nil { response, complete := h.blockTraceCache.completeResponse(cacheKey, returnInitialReads) if complete { diff --git a/rpc/v10/trace_cache.go b/rpc/v10/trace_cache.go index 44c8f5b874..8e0b99627b 100644 --- a/rpc/v10/trace_cache.go +++ b/rpc/v10/trace_cache.go @@ -54,7 +54,7 @@ func newBlockTraceCache(limit int) *blockTraceCache { } func (c *blockTraceCache) completeResponse( - blockHash felt.Felt, + blockHash *felt.Felt, requireInitialReads bool, ) (TraceBlockTransactionsResponse, bool) { record, found := c.record(blockHash) @@ -65,7 +65,7 @@ func (c *blockTraceCache) completeResponse( } func (c *blockTraceCache) traceAt( - blockHash felt.Felt, + blockHash *felt.Felt, index uint64, ) (TracedBlockTransaction, bool) { record, found := c.record(blockHash) @@ -104,7 +104,7 @@ func (c *blockTraceCache) lookupOrStart( // storeComplete takes ownership of the response containers. InitialReads must be non-nil. func (c *blockTraceCache) storeComplete( - blockHash felt.Felt, + blockHash *felt.Felt, response TraceBlockTransactionsResponse, ) { record := &blockTraceRecord{ @@ -114,17 +114,17 @@ func (c *blockTraceCache) storeComplete( } c.mu.Lock() defer c.mu.Unlock() - c.records.Add(blockHash, record) + c.records.Add(*blockHash, record) } -func (c *blockTraceCache) record(blockHash felt.Felt) (*blockTraceRecord, bool) { +func (c *blockTraceCache) record(blockHash *felt.Felt) (*blockTraceRecord, bool) { c.mu.Lock() defer c.mu.Unlock() - return c.records.Get(blockHash) + return c.records.Get(*blockHash) } -func (c *blockTraceCache) finishLocked(blockHash felt.Felt, flight chan struct{}) { - delete(c.flights, blockHash) +func (c *blockTraceCache) finishLocked(blockHash *felt.Felt, flight chan struct{}) { + delete(c.flights, *blockHash) close(flight) } @@ -149,7 +149,7 @@ func (w *traceCacheWork) commit( cache.mu.Lock() defer cache.mu.Unlock() cache.records.Add(w.hash, record) - cache.finishLocked(w.hash, w.flight) + cache.finishLocked(&w.hash, w.flight) return record.response() } @@ -160,5 +160,5 @@ func (w *traceCacheWork) abort() { if cache.flights[w.hash] != w.flight { return } - cache.finishLocked(w.hash, w.flight) + cache.finishLocked(&w.hash, w.flight) } diff --git a/rpc/v10/trace_progressive_test.go b/rpc/v10/trace_progressive_test.go index 1d63b6163c..30c51fc285 100644 --- a/rpc/v10/trace_progressive_test.go +++ b/rpc/v10/trace_progressive_test.go @@ -358,7 +358,7 @@ func TestProgressiveTraceCacheAllowsRecordEvictionDuringFlight(t *testing.T) { <-entered for index := range rpccore.TraceCacheSize { handler.blockTraceCache.storeComplete( - felt.FromUint64[felt.Felt](uint64(1_000+index)), + felt.NewFromUint64[felt.Felt](uint64(1_000+index)), TraceBlockTransactionsResponse{InitialReads: emptyInitialReads()}, ) } From 6e6b2f242277807bd07e3374476d060265980281 Mon Sep 17 00:00:00 2001 From: danielntmd Date: Wed, 9 Sep 2026 13:08:32 -0700 Subject: [PATCH 6/6] perf(rpc): reduce temporary trace data copies --- rpc/v10/adapt_trace.go | 12 ++++++++---- rpc/v10/trace.go | 6 +++--- rpc/v10/trace_progressive.go | 21 ++++++++++++++------- rpc/v10/trace_progressive_test.go | 4 ++-- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/rpc/v10/adapt_trace.go b/rpc/v10/adapt_trace.go index 673889dcca..4c46478097 100644 --- a/rpc/v10/adapt_trace.go +++ b/rpc/v10/adapt_trace.go @@ -250,7 +250,8 @@ func AdaptVMStateDiff(vmStateDiff *vm.StateDiff) StateDiff { // adaptVMInitialReads requires non-nil VM output; callers decide how missing reads are handled. func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { storage := make([]StorageEntry, len(vmInitialReads.Storage)) - for i, s := range vmInitialReads.Storage { + for i := range vmInitialReads.Storage { + s := &vmInitialReads.Storage[i] storage[i] = StorageEntry{ ContractAddress: s.ContractAddress, Key: s.Key, @@ -259,7 +260,8 @@ func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { } nonces := make([]NonceEntry, len(vmInitialReads.Nonces)) - for i, n := range vmInitialReads.Nonces { + for i := range vmInitialReads.Nonces { + n := &vmInitialReads.Nonces[i] nonces[i] = NonceEntry{ ContractAddress: n.ContractAddress, Nonce: n.Nonce, @@ -267,7 +269,8 @@ func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { } classHashes := make([]ClassHashEntry, len(vmInitialReads.ClassHashes)) - for i, ch := range vmInitialReads.ClassHashes { + for i := range vmInitialReads.ClassHashes { + ch := &vmInitialReads.ClassHashes[i] classHashes[i] = ClassHashEntry{ ContractAddress: ch.ContractAddress, ClassHash: ch.ClassHash, @@ -275,7 +278,8 @@ func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { } declaredContracts := make([]DeclaredContractEntry, len(vmInitialReads.DeclaredContracts)) - for i, dc := range vmInitialReads.DeclaredContracts { + for i := range vmInitialReads.DeclaredContracts { + dc := &vmInitialReads.DeclaredContracts[i] declaredContracts[i] = DeclaredContractEntry{ ClassHash: dc.ClassHash, IsDeclared: dc.IsDeclared, diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index ad60061a45..ff0de094f1 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -296,7 +296,7 @@ func (h *Handler) findAndTraceFinalisedTransaction( } response, responseHeader, rpcErr := h.traceFinalisedBlock( - ctx, header, &traceTarget{index: txIndex, hash: *hash}, false, + ctx, header, &traceTarget{index: txIndex, hash: hash}, false, ) if rpcErr != nil { return TransactionTrace{}, responseHeader, rpcErr @@ -381,7 +381,7 @@ func (h *Handler) findAndTraceInPreConfirmed( // traceTarget identifies the last transaction to trace and the hash expected at that index. type traceTarget struct { index uint64 - hash felt.TransactionHash + hash *felt.TransactionHash } // traceFinalisedBlock returns traces through target, or the whole block when target is nil. @@ -428,7 +428,7 @@ func (h *Handler) traceFinalisedBlock( if target != nil { // The tx-hash index and transaction list come from separate reads; validate before execution. if target.index >= uint64(len(transactions)) || - !transactions[target.index].Hash().Equal((*felt.Felt)(&target.hash)) { + !transactions[target.index].Hash().Equal((*felt.Felt)(target.hash)) { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrTxnHashNotFound } return h.traceProgressiveBlock(ctx, header, transactions, target.index, returnInitialReads) diff --git a/rpc/v10/trace_progressive.go b/rpc/v10/trace_progressive.go index 2f67a1f58a..1bce68757a 100644 --- a/rpc/v10/trace_progressive.go +++ b/rpc/v10/trace_progressive.go @@ -177,32 +177,39 @@ func checkpointFromTraces(traces []TracedBlockTransaction) core.StateDiff { } func mergeRPCStateDiff(result *core.StateDiff, diff *StateDiff) { - for _, storage := range diff.StorageDiffs { + for storageIndex := range diff.StorageDiffs { + storage := &diff.StorageDiffs[storageIndex] entries, found := result.StorageDiffs[storage.Address] if !found { entries = make(map[felt.Felt]*felt.Felt, len(storage.StorageEntries)) result.StorageDiffs[storage.Address] = entries } - for _, entry := range storage.StorageEntries { + for entryIndex := range storage.StorageEntries { + entry := &storage.StorageEntries[entryIndex] entries[entry.Key] = entry.Value.Clone() } } - for _, nonce := range diff.Nonces { + for nonceIndex := range diff.Nonces { + nonce := &diff.Nonces[nonceIndex] result.Nonces[nonce.ContractAddress] = nonce.Nonce.Clone() } - for _, deployed := range diff.DeployedContracts { + for deployedIndex := range diff.DeployedContracts { + deployed := &diff.DeployedContracts[deployedIndex] result.DeployedContracts[deployed.Address] = deployed.ClassHash.Clone() } for _, hash := range diff.DeprecatedDeclaredClasses { result.DeclaredV0Classes = append(result.DeclaredV0Classes, hash.Clone()) } - for _, declared := range diff.DeclaredClasses { + for declaredIndex := range diff.DeclaredClasses { + declared := &diff.DeclaredClasses[declaredIndex] result.DeclaredV1Classes[declared.ClassHash] = declared.CompiledClassHash.Clone() } - for _, replaced := range diff.ReplacedClasses { + for replacedIndex := range diff.ReplacedClasses { + replaced := &diff.ReplacedClasses[replacedIndex] result.ReplacedClasses[replaced.ContractAddress] = replaced.ClassHash.Clone() } - for _, migrated := range diff.MigratedCompiledClasses { + for migratedIndex := range diff.MigratedCompiledClasses { + migrated := &diff.MigratedCompiledClasses[migratedIndex] result.MigratedClasses[migrated.ClassHash] = migrated.CompiledClassHash } } diff --git a/rpc/v10/trace_progressive_test.go b/rpc/v10/trace_progressive_test.go index 30c51fc285..5399ed68c6 100644 --- a/rpc/v10/trace_progressive_test.go +++ b/rpc/v10/trace_progressive_test.go @@ -289,8 +289,8 @@ func TestTraceFinalisedBlockRejectsInvalidTargetBeforeExecution(t *testing.T) { handler := New(reader, nil, virtualMachine, log.NewNopZapLogger()) for name, target := range map[string]traceTarget{ - "index out of range": {index: 1, hash: felt.TransactionHash(*transactions[0].Hash())}, - "hash mismatch": {index: 0, hash: felt.FromUint64[felt.TransactionHash](999)}, + "index out of range": {index: 1, hash: (*felt.TransactionHash)(transactions[0].Hash())}, + "hash mismatch": {index: 0, hash: felt.NewFromUint64[felt.TransactionHash](999)}, } { t.Run(name, func(t *testing.T) { _, _, rpcErr := handler.traceFinalisedBlock(t.Context(), header, &target, false)