diff --git a/blockchain/blockchain.go b/blockchain/blockchain.go index 9a4578aa51..8ae7885df3 100644 --- a/blockchain/blockchain.go +++ b/blockchain/blockchain.go @@ -56,6 +56,11 @@ type Reader interface { blockNumber, index uint64, ) (transaction core.Transaction, err error) TransactionsByBlockNumber(blockNumber uint64) (transactions []core.Transaction, err error) + TransactionsAndReceiptsByBlockNumber(blockNumber uint64) ( + transactions []core.Transaction, + receipts []*core.TransactionReceipt, + err error, + ) TransactionHashesByBlockNumber(blockNumber uint64) (hashes []felt.Felt, err error) Receipt( @@ -299,6 +304,14 @@ func (b *Blockchain) TransactionsByBlockNumber(number uint64) ([]core.Transactio return core.GetTransactionsByBlockNumber(b.database, number) } +// TransactionsAndReceiptsByBlockNumber gets all transactions and receipts for a given block number +func (b *Blockchain) TransactionsAndReceiptsByBlockNumber( + number uint64, +) ([]core.Transaction, []*core.TransactionReceipt, error) { + b.listener.OnRead("TransactionsAndReceiptsByBlockNumber") + return core.GetTransactionsAndReceiptsByBlockNumber(b.database, number) +} + // BlockNumberAndIndexByTxHash gets transaction block number and index by Tx hash func (b *Blockchain) BlockNumberAndIndexByTxHash( hash *felt.TransactionHash, diff --git a/core/accessors.go b/core/accessors.go index 0607b5a7bc..c5e5a59f71 100644 --- a/core/accessors.go +++ b/core/accessors.go @@ -513,6 +513,28 @@ func GetTransactionHashesByBlockNumber( return BlockTransactionsAllTransactionHashesPartialBucket.Get(r, blockNumber, struct{}{}) } +// GetTransactionsAndReceiptsByBlockNumber returns all transactions and receipts in a given block. +// Both live under the same key, so this reads the block only once. +func GetTransactionsAndReceiptsByBlockNumber( + r db.KeyValueReader, + blockNumber uint64, +) ([]Transaction, []*TransactionReceipt, error) { + result, err := BlockTransactionsAllTransactionsAndReceiptsPartialBucket.Get( + r, + blockNumber, + struct{}{}, + ) + if err != nil { + return nil, nil, fmt.Errorf( + "getting transactions and receipts of block %d: %w", + blockNumber, + err, + ) + } + + return result.Transactions, result.Receipts, nil +} + // GetReceiptByBlockAndIndex returns a receipt by block number and transaction index func GetReceiptByBlockAndIndex( r db.KeyValueReader, @@ -555,12 +577,7 @@ func GetBlockByNumber(r db.KeyValueReader, blockNumber uint64) (*Block, error) { return nil, err } - txs, err := GetTransactionsByBlockNumber(r, blockNumber) - if err != nil { - return nil, err - } - - receipts, err := GetReceiptsByBlockNumber(r, blockNumber) + txs, receipts, err := GetTransactionsAndReceiptsByBlockNumber(r, blockNumber) if err != nil { return nil, err } diff --git a/core/block_transaction.go b/core/block_transaction.go index 826249d525..4718c31157 100644 --- a/core/block_transaction.go +++ b/core/block_transaction.go @@ -41,6 +41,12 @@ type TransactionAndReceipt struct { Receipt *TransactionReceipt } +// TransactionsAndReceipts holds every transaction and receipt of a single block. +type TransactionsAndReceipts struct { + Transactions []Transaction + Receipts []*TransactionReceipt +} + func NewBlockTransactionsFromIterators[T, R any]( transactions iter.Seq2[T, error], receipts iter.Seq2[R, error], diff --git a/core/block_transaction_serializer.go b/core/block_transaction_serializer.go index 2c947ba960..57196dd192 100644 --- a/core/block_transaction_serializer.go +++ b/core/block_transaction_serializer.go @@ -112,6 +112,27 @@ func (extractAllReceipts) extract(b *BlockTransactions, _ struct{}) ([]*Transact return b.Receipts().All() } +type extractAllTransactionsAndReceipts struct{} + +// extract decodes both halves of the entry in one pass, so reading them together needs neither a +// second lookup nor a copy of the whole block blob. +func (extractAllTransactionsAndReceipts) extract( + b *BlockTransactions, + _ struct{}, +) (TransactionsAndReceipts, error) { + transactions, err := b.Transactions().All() + if err != nil { + return TransactionsAndReceipts{}, fmt.Errorf("extracting transactions: %w", err) + } + + receipts, err := b.Receipts().All() + if err != nil { + return TransactionsAndReceipts{}, fmt.Errorf("extracting receipts: %w", err) + } + + return TransactionsAndReceipts{Transactions: transactions, Receipts: receipts}, nil +} + type extractAllTransactionEvents struct{} func (extractAllTransactionEvents) extract( @@ -190,6 +211,11 @@ var ( struct{}, []*TransactionReceipt, ]{} + BlockTransactionsAllTransactionsAndReceiptsPartialSerializer = blockTransactionsPartialSerializer[ + extractAllTransactionsAndReceipts, + struct{}, + TransactionsAndReceipts, + ]{} BlockTransactionsAllTransactionEventsPartialSerializer = blockTransactionsPartialSerializer[ extractAllTransactionEvents, struct{}, diff --git a/core/block_transaction_test.go b/core/block_transaction_test.go index 027cf12baa..7506ab2a93 100644 --- a/core/block_transaction_test.go +++ b/core/block_transaction_test.go @@ -149,6 +149,16 @@ func TestBlockTransactionsSerializer(t *testing.T) { ) }) + t.Run("BlockTransactionsAllTransactionsAndReceiptsPartialSerializer", func(t *testing.T) { + assertPartialSerializer( + t, + core.BlockTransactionsAllTransactionsAndReceiptsPartialSerializer, + struct{}{}, + core.TransactionsAndReceipts{Transactions: transactions, Receipts: receipts}, + serialised, + ) + }) + t.Run("BlockTransactionsAllTransactionEventsPartialSerializer", func(t *testing.T) { expected := make([]core.TransactionEvents, len(receipts)) for i, receipt := range receipts { diff --git a/core/pending/pending.go b/core/pending/pending.go index 73d9c3f64d..76d8a8252d 100644 --- a/core/pending/pending.go +++ b/core/pending/pending.go @@ -116,9 +116,11 @@ func (p *PreConfirmed) GetTransactionStateDiffs() []*core.StateDiff { // TransactionByHash locates a transaction by hash in the block and returns // it together with its index. Returns ErrTransactionNotFound when missing. -func (p *PreConfirmed) TransactionByHash(hash *felt.Felt) (core.Transaction, uint, error) { +func (p *PreConfirmed) TransactionByHash( + hash *felt.TransactionHash, +) (core.Transaction, uint, error) { for i, tx := range p.Block.Transactions { - if tx.Hash().Equal(hash) { + if tx.Hash().Equal((*felt.Felt)(hash)) { return tx, uint(i), nil } } @@ -126,10 +128,10 @@ func (p *PreConfirmed) TransactionByHash(hash *felt.Felt) (core.Transaction, uin } func (p *PreConfirmed) ReceiptByHash( - hash *felt.Felt, + hash *felt.TransactionHash, ) (*core.TransactionReceipt, error) { for _, receipt := range p.Block.Receipts { - if receipt.TransactionHash.Equal(hash) { + if receipt.TransactionHash.Equal((*felt.Felt)(hash)) { return receipt, nil } } diff --git a/core/pending/pending_test.go b/core/pending/pending_test.go index 2314ddc9c2..f282848c33 100644 --- a/core/pending/pending_test.go +++ b/core/pending/pending_test.go @@ -10,11 +10,11 @@ import ( ) func TestPreConfirmedTransactionByHash(t *testing.T) { - preConfirmedTxHash := felt.FromUint64[felt.Felt](2) - nonExistingTxHash := felt.FromUint64[felt.Felt](4) + preConfirmedTxHash := felt.FromUint64[felt.TransactionHash](2) + nonExistingTxHash := felt.FromUint64[felt.TransactionHash](4) preConfirmedTx := &core.InvokeTransaction{ - TransactionHash: &preConfirmedTxHash, + TransactionHash: (*felt.Felt)(&preConfirmedTxHash), } preConfirmed := &pending.PreConfirmed{ @@ -41,11 +41,11 @@ func TestPreConfirmedTransactionByHash(t *testing.T) { } func TestPreConfirmedReceiptByHash(t *testing.T) { - preConfirmedReceiptHash := felt.FromUint64[felt.Felt](2) - nonExistingReceiptHash := felt.FromUint64[felt.Felt](3) + preConfirmedReceiptHash := felt.FromUint64[felt.TransactionHash](2) + nonExistingReceiptHash := felt.FromUint64[felt.TransactionHash](3) preConfirmedReceipt := core.TransactionReceipt{ - TransactionHash: &preConfirmedReceiptHash, + TransactionHash: (*felt.Felt)(&preConfirmedReceiptHash), } preConfirmedBlockNumber := uint64(2) diff --git a/core/typed_buckets.go b/core/typed_buckets.go index 672c755299..e4f80ab4e5 100644 --- a/core/typed_buckets.go +++ b/core/typed_buckets.go @@ -197,6 +197,11 @@ var BlockTransactionsAllReceiptsPartialBucket = partial.NewPartialBucket( BlockTransactionsAllReceiptsPartialSerializer, ) +var BlockTransactionsAllTransactionsAndReceiptsPartialBucket = partial.NewPartialBucket( + BlockTransactionsBucket.Bucket, + BlockTransactionsAllTransactionsAndReceiptsPartialSerializer, +) + var BlockTransactionsAllTransactionEventsPartialBucket = partial.NewPartialBucket( BlockTransactionsBucket.Bucket, BlockTransactionsAllTransactionEventsPartialSerializer, diff --git a/mocks/mock_blockchain.go b/mocks/mock_blockchain.go index a71931fd55..2fe1e8f9e9 100644 --- a/mocks/mock_blockchain.go +++ b/mocks/mock_blockchain.go @@ -470,6 +470,22 @@ func (mr *MockReaderMockRecorder) TransactionHashesByBlockNumber(blockNumber any return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TransactionHashesByBlockNumber", reflect.TypeOf((*MockReader)(nil).TransactionHashesByBlockNumber), blockNumber) } +// TransactionsAndReceiptsByBlockNumber mocks base method. +func (m *MockReader) TransactionsAndReceiptsByBlockNumber(blockNumber uint64) ([]core.Transaction, []*core.TransactionReceipt, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TransactionsAndReceiptsByBlockNumber", blockNumber) + ret0, _ := ret[0].([]core.Transaction) + ret1, _ := ret[1].([]*core.TransactionReceipt) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// TransactionsAndReceiptsByBlockNumber indicates an expected call of TransactionsAndReceiptsByBlockNumber. +func (mr *MockReaderMockRecorder) TransactionsAndReceiptsByBlockNumber(blockNumber any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TransactionsAndReceiptsByBlockNumber", reflect.TypeOf((*MockReader)(nil).TransactionsAndReceiptsByBlockNumber), blockNumber) +} + // TransactionsByBlockNumber mocks base method. func (m *MockReader) TransactionsByBlockNumber(blockNumber uint64) ([]core.Transaction, error) { m.ctrl.T.Helper() diff --git a/rpc/rpccore/rpccore.go b/rpc/rpccore/rpccore.go index 6b92f6bdfe..68bfd472ea 100644 --- a/rpc/rpccore/rpccore.go +++ b/rpc/rpccore/rpccore.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" - "github.com/NethermindEth/juno/core/felt" "github.com/NethermindEth/juno/jsonrpc" "github.com/NethermindEth/juno/l1/eth" ) @@ -35,10 +34,6 @@ type L1Client interface { TransactionReceipt(ctx context.Context, txHash eth.Hash) (eth.Receipt, error) } -type TraceCacheKey struct { - BlockHash felt.Felt -} - var ( ErrContractNotFound = &jsonrpc.Error{Code: 20, Message: "Contract not found"} ErrEntrypointNotFound = &jsonrpc.Error{ diff --git a/rpc/v10/adapt_trace.go b/rpc/v10/adapt_trace.go index 43fafefdc3..801d8eee65 100644 --- a/rpc/v10/adapt_trace.go +++ b/rpc/v10/adapt_trace.go @@ -303,14 +303,14 @@ func adaptVMInitialReads(vmInitialReads *vm.InitialReads) InitialReads { *****************************************************/ func AdaptFeederBlockTrace( - block *core.Block, + transactions []core.Transaction, blockTrace *starknet.BlockTrace, ) ([]TracedBlockTransaction, error) { if blockTrace == nil { return nil, nil } - if len(block.Transactions) != len(blockTrace.Traces) { + if len(transactions) != len(blockTrace.Traces) { return nil, errors.New("mismatched number of txs and traces") } @@ -320,7 +320,7 @@ func AdaptFeederBlockTrace( feederTrace := &blockTrace.Traces[index] trace := TransactionTrace{ - Type: transactionTypeFrom(block.Transactions[index]), + Type: transactionTypeFrom(transactions[index]), } if feederTrace.FeeTransferInvocation != nil && trace.Type != TxnL1Handler { diff --git a/rpc/v10/handlers.go b/rpc/v10/handlers.go index f5e27860d1..018665e9c1 100644 --- a/rpc/v10/handlers.go +++ b/rpc/v10/handlers.go @@ -11,6 +11,7 @@ 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" @@ -42,9 +43,7 @@ type Handler struct { idgen func() string subscriptions stdsync.Map // map[string]*subscription - // todo(rdr): why do we have the `TraceCacheKey` type and why it feels uncomfortable - // to use. It makes no sense, why not use `Felt` or `Hash` directly? - blockTraceCache *lru.Cache[rpccore.TraceCacheKey, TraceBlockTransactionsResponse] + blockTraceCache *lru.Cache[felt.Felt, TraceBlockTransactionsResponse] // todo(rdr): Can this cache be genericified and can it be applied to the `blockTraceCache` submittedTransactionsCache *rpccore.TransactionCache @@ -86,7 +85,7 @@ func New( l1Heads: feed.New[*core.L1Head](), blockTraceCache: lru.New[ - rpccore.TraceCacheKey, + felt.Felt, TraceBlockTransactionsResponse, ](rpccore.TraceCacheSize), filterLimit: math.MaxUint, diff --git a/rpc/v10/handlers_test.go b/rpc/v10/handlers_test.go index e096127fe3..914ca36a46 100644 --- a/rpc/v10/handlers_test.go +++ b/rpc/v10/handlers_test.go @@ -77,7 +77,9 @@ func TestThrottledVMError(t *testing.T) { Transactions: []core.Transaction{l1Tx, declareTx}, } - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil) + mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) state := mocks.NewMockStateReader(mockCtrl) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(state, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) diff --git a/rpc/v10/trace.go b/rpc/v10/trace.go index 04e217d9c1..b9e81b440c 100644 --- a/rpc/v10/trace.go +++ b/rpc/v10/trace.go @@ -12,6 +12,7 @@ import ( "github.com/NethermindEth/juno/blockchain/networks" "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" @@ -40,18 +41,20 @@ type TransactionTrace struct { // It follows the specification defined here: // https://github.com/starkware-libs/starknet-specs/blob/9377851884da5c81f757b6ae0ed47e84f9e7c058/api/starknet_trace_api_openrpc.json#L11 func (h *Handler) TraceTransaction( - ctx context.Context, hash *felt.Felt, + ctx context.Context, hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { httpHeader := defaultExecutionHeader() - if trace, header, err := h.findAndTraceFinalisedTransaction(ctx, hash); err == nil { + trace, header, err := h.findAndTraceFinalisedTransaction(ctx, hash) + if err == nil { return trace, header, nil - } else if err != rpccore.ErrTxnHashNotFound { - return TransactionTrace{}, httpHeader, rpccore.ErrTxnHashNotFound + } + if err != rpccore.ErrTxnHashNotFound { + return TransactionTrace{}, httpHeader, err } - // Try to find and trace transaction in preconfirmed - trace, header, err := h.findAndTraceInPreConfirmed(hash) + // Not in a finalised block, so try the pre_confirmed chain. + trace, header, err = h.findAndTraceInPreConfirmed(hash) if err != nil { return TransactionTrace{}, httpHeader, err } @@ -132,13 +135,15 @@ func (h *Handler) TraceBlockTransactions( return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrCallOnPreConfirmed } - block, rpcErr := h.blockByID(id) + // Resolve the block id once: the header pins the block number, so the reads that follow it + // go by number and a tag like `latest` cannot move between them. + header, rpcErr := h.blockHeaderByID(id) if rpcErr != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } returnInitialReads := slices.Contains(traceFlags, TraceReturnInitialReadsFlag) - return h.traceBlockTransactions(ctx, block, returnInitialReads) + return h.traceFinalisedBlock(ctx, header, returnInitialReads) } /**************************************************** @@ -252,9 +257,9 @@ func fetchDeclaredClassesAndL1Fees( // findAndTraceFinalisedTransaction searches for a transaction in // finalised blocks and returns its trace. func (h *Handler) findAndTraceFinalisedTransaction( - ctx context.Context, hash *felt.Felt, + ctx context.Context, hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { - _, blockHash, _, err := h.bcReader.Receipt(hash) + blockNumber, txIndex, err := h.bcReader.BlockNumberAndIndexByTxHash(hash) if err != nil { if !errors.Is(err, db.ErrKeyNotFound) { return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) @@ -262,22 +267,27 @@ func (h *Handler) findAndTraceFinalisedTransaction( return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } - block, err := h.bcReader.BlockByHash(blockHash) + header, err := h.bcReader.BlockHeaderByNumber(blockNumber) if err != nil { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound - } - - txIndex, rpcErr := findTransactionInBlock(block, hash) - if rpcErr != nil { - return TransactionTrace{}, defaultExecutionHeader(), rpccore.ErrTxnHashNotFound + if errors.Is(err, db.ErrKeyNotFound) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } - blockTracesResp, httpHeader, rpcErr := h.traceBlockTransactions(ctx, block, false) + blockTracesResp, httpHeader, rpcErr := h.traceFinalisedBlock(ctx, header, false) if rpcErr != nil { return TransactionTrace{}, nil, 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)) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return *blockTraces[txIndex].TraceRoot, httpHeader, nil } @@ -287,22 +297,25 @@ func (h *Handler) findAndTraceFinalisedTransaction( // entry's diff from chain bottom up to entry's block, then the entry's own // transaction-level diffs up to (but not including) txIndex. func (h *Handler) findAndTraceInPreConfirmed( - hash *felt.Felt, + hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { chain, err := h.syncReader.PreConfirmedChain() if err != nil { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + if errors.Is(err, db.ErrKeyNotFound) || errors.Is(err, pending.ErrPreConfirmedNotFound) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } for entry := range chain.NewestFirst() { - _, txIndex, err := entry.TransactionByHash(hash) + transaction, transactionIndex, err := entry.TransactionByHash(hash) if err != nil { continue } state, baseCloser, err := chain.PreConfirmedStateBeforeIndexAt( entry.Block.Number, - txIndex, + transactionIndex, h.bcReader, ) if err != nil { @@ -318,7 +331,7 @@ func (h *Handler) findAndTraceInPreConfirmed( traces, _, httpHeader, rpcErr := traceTransactionsWithState( h.vm, - []core.Transaction{entry.Block.Transactions[txIndex]}, + []core.Transaction{transaction}, state, // execution state state, // class lookup state (same for preconfirmed) &blockInfo, @@ -336,15 +349,17 @@ func (h *Handler) findAndTraceInPreConfirmed( Block Tracing Helpers *****************************************************/ -// traceBlockTransactions gets the trace for a block. The block will always be traced locally except +// 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 -func (h *Handler) traceBlockTransactions( - ctx context.Context, block *core.Block, returnInitialReads bool, +func (h *Handler) traceFinalisedBlock( + ctx context.Context, + 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 := rpccore.TraceCacheKey{BlockHash: *block.Hash} + cacheKey := *header.Hash cachedResponse, hit := h.blockTraceCache.Get(cacheKey) if hit && (!returnInitialReads || cachedResponse.InitialReads != nil) { if returnInitialReads { @@ -356,7 +371,7 @@ func (h *Handler) traceBlockTransactions( }, defaultExecutionHeader(), nil } - fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(block, h.bcReader.Network()) + fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(header, h.bcReader.Network()) if err != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), @@ -364,45 +379,63 @@ func (h *Handler) traceBlockTransactions( } if fetchFromFeederGW { - // Feeder gateway can't supply initial reads. If we already have cached - // traces from a previous fetch, reuse them instead of re-hitting the GW. - traces := cachedResponse.Traces - if !hit { - var fetchErr *jsonrpc.Error - traces, fetchErr = h.fetchTracesFromFeederGateway(ctx, block) - if fetchErr != nil { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), fetchErr - } - h.blockTraceCache.Add( - rpccore.TraceCacheKey{BlockHash: *block.Hash}, - TraceBlockTransactionsResponse{ - Traces: traces, - InitialReads: nil, - }, - ) + traces, rpcErr := h.fetchTracesFromFeederGateway(ctx, header) + if rpcErr != nil { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } - var initialReads *InitialReads - if returnInitialReads { - initialReads = &InitialReads{} - } - return TraceBlockTransactionsResponse{ + // 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{ Traces: traces, - InitialReads: initialReads, - }, defaultExecutionHeader(), nil + InitialReads: &InitialReads{}, + } + h.blockTraceCache.Add(cacheKey, cached) + + response := cached + if !returnInitialReads { + response.InitialReads = nil + } + + return response, defaultExecutionHeader(), 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 h.traceBlockWithVM(block, returnInitialReads) + return response, httpHeader, nil } -// traceBlockWithVM traces a block using the local VM and stores the result in the block cache. -func (h *Handler) traceBlockWithVM(block *core.Block, returnInitialReads bool) ( - TraceBlockTransactionsResponse, http.Header, *jsonrpc.Error, -) { +// 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(block.ParentHash) + state, closer, err := h.bcReader.StateAtBlockHash(header.ParentHash) if err != nil { - return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound + if errors.Is(err, db.ErrKeyNotFound) { + return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpccore.ErrBlockNotFound + } + + return TraceBlockTransactionsResponse{}, + defaultExecutionHeader(), + rpccore.ErrInternal.CloneWithData(err) } defer h.callAndLogErr(closer, "Failed to close state in traceBlockTransactions") @@ -421,14 +454,14 @@ func (h *Handler) traceBlockWithVM(block *core.Block, returnInitialReads bool) ( defer h.callAndLogErr(headStateCloser, "Failed to close head state in traceBlockTransactions") // Create block info - blockInfo, rpcErr := h.buildBlockInfo(block.Header) + blockInfo, rpcErr := h.buildBlockInfo(header) if rpcErr != nil { return TraceBlockTransactionsResponse{}, defaultExecutionHeader(), rpcErr } traces, vmInitialReads, httpHeader, rpcErr := traceTransactionsWithState( h.vm, - block.Transactions, + transactions, state, headState, &blockInfo, @@ -440,18 +473,9 @@ func (h *Handler) traceBlockWithVM(block *core.Block, returnInitialReads bool) ( var adaptedInitialReads *InitialReads if vmInitialReads != nil && returnInitialReads { - adapted := adaptVMInitialReads(vmInitialReads) - adaptedInitialReads = &adapted + adaptedInitialReads = new(adaptVMInitialReads(vmInitialReads)) } - h.blockTraceCache.Add( - rpccore.TraceCacheKey{BlockHash: *block.Hash}, - TraceBlockTransactionsResponse{ - Traces: traces, - InitialReads: adaptedInitialReads, - }, - ) - return TraceBlockTransactionsResponse{ Traces: traces, InitialReads: adaptedInitialReads, @@ -461,25 +485,31 @@ func (h *Handler) traceBlockWithVM(block *core.Block, returnInitialReads bool) ( // fetchTracesFromFeederGateway fetches block traces from the feeder gateway // and fills in missing data. func (h *Handler) fetchTracesFromFeederGateway( - ctx context.Context, block *core.Block, + ctx context.Context, header *core.Header, ) ([]TracedBlockTransaction, *jsonrpc.Error) { if h.feederClient == nil { return nil, rpccore.ErrInternal.CloneWithData("no feeder client configured") } - blockTrace, err := h.feederClient.BlockTrace(ctx, block.Hash.String()) + blockTrace, err := h.feederClient.BlockTrace(ctx, header.Hash.String()) if err != nil { return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) } - traces, err := AdaptFeederBlockTrace(block, &blockTrace) + transactions, receipts, err := h.bcReader.TransactionsAndReceiptsByBlockNumber(header.Number) if err != nil { - return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + if errors.Is(err, db.ErrKeyNotFound) { + return nil, rpccore.ErrBlockNotFound + } + return nil, rpccore.ErrInternal.CloneWithData(err) } - traces = fillFeederGatewayData(traces, block.Receipts) + traces, err := AdaptFeederBlockTrace(transactions, &blockTrace) + if err != nil { + return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + } - return traces, nil + return fillFeederGatewayData(traces, receipts), nil } // buildBlockInfo builds block info for VM execution. @@ -498,22 +528,22 @@ func (h *Handler) buildBlockInfo(header *core.Header) (vm.BlockInfo, *jsonrpc.Er // shouldFetchTracesFromFeederGateway determines if // traces for a block should be fetched from the feeder gateway. func shouldFetchTracesFromFeederGateway( - block *core.Block, + header *core.Header, network *networks.Network, ) (bool, error) { - blockVer, err := core.ParseBlockVersion(block.ProtocolVersion) + blockVer, err := core.ParseBlockVersion(header.ProtocolVersion) if err != nil { return false, err } // We rely on the feeder gateway for Starknet version strictly older than "0.13.1.1" fetchFromFeederGW := blockVer.LessThan(core.Ver0_13_2) && - block.ProtocolVersion != "0.13.1.1" + header.ProtocolVersion != "0.13.1.1" // This specific block range caused a re-org, also related with Cairo 0 and we have to // depend on the Sequencer to provide the correct traces fetchFromFeederGW = fetchFromFeederGW || - (block.Number >= 1943705 && - block.Number <= 1952704 && + (header.Number >= 1943705 && + header.Number <= 1952704 && *network == networks.Mainnet) return fetchFromFeederGW, nil @@ -563,16 +593,3 @@ func defaultExecutionHeader() http.Header { header.Set(ExecutionStepsHeader, "0") return header } - -// findTransactionInBlock locates the index of a transaction with the given hash in a block. -// -// Returns the index of the transaction and nil if found, or 0 and ErrTxnHashNotFound if not found. -func findTransactionInBlock(block *core.Block, hash *felt.Felt) (uint, *jsonrpc.Error) { - txIndex := slices.IndexFunc(block.Transactions, func(tx core.Transaction) bool { - return tx.Hash().Equal(hash) - }) - if txIndex == -1 { - return 0, rpccore.ErrTxnHashNotFound - } - return uint(txIndex), nil -} diff --git a/rpc/v10/trace_test.go b/rpc/v10/trace_test.go index d0ff103e13..3ffed0b521 100644 --- a/rpc/v10/trace_test.go +++ b/rpc/v10/trace_test.go @@ -145,9 +145,21 @@ func AssertTracedBlockTransactions( mockReader := mocks.NewMockReader(mockCtrl) - mockReader.EXPECT().BlockByNumber(gomock.Any()). - DoAndReturn(func(number uint64) (block *core.Block, err error) { - block, err = gateway.BlockByNumber(t.Context(), number) + mockReader.EXPECT().BlockHeaderByNumber(gomock.Any()). + DoAndReturn(func(number uint64) (*core.Header, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, err + } + return block.Header, nil + }).AnyTimes() + + mockReader.EXPECT().TransactionsAndReceiptsByBlockNumber(gomock.Any()). + DoAndReturn(func(number uint64) ([]core.Transaction, []*core.TransactionReceipt, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, nil, err + } // Simulate gas consumption in block receipts for _, receipt := range block.Receipts { @@ -157,7 +169,7 @@ func AssertTracedBlockTransactions( L1DataGas: 15, } } - return block, err + return block.Transactions, block.Receipts, nil }).AnyTimes() mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() @@ -193,9 +205,13 @@ func TestTraceBlockTransactionsReturnsError(t *testing.T) { blockNumber := uint64(40000) - mockReader.EXPECT().BlockByNumber(gomock.Any()). - DoAndReturn(func(number uint64) (block *core.Block, err error) { - return gateway.BlockByNumber(t.Context(), number) + mockReader.EXPECT().BlockHeaderByNumber(gomock.Any()). + DoAndReturn(func(number uint64) (*core.Header, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, err + } + return block.Header, nil }) mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() mockReader.EXPECT().Network().Return(&network) @@ -330,9 +346,10 @@ func TestTraceTransaction(t *testing.T) { t.Run("not found", func(t *testing.T) { t.Run("key not found", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xBBBB") - // Receipt() returns error related to db - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xBBBB") + // The tx-hash index lookup misses + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) preConfirmed := pending.NewPreConfirmed(&core.Block{}, nil, nil, "") mockSyncReader.EXPECT().PreConfirmedChain().Return(mustNewChain(t, &preConfirmed), nil) @@ -343,27 +360,23 @@ func TestTraceTransaction(t *testing.T) { }) t.Run("other error", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xBBBB") - // Receipt() returns some other error - mockReader.EXPECT().Receipt(hash).Return( - nil, - nil, - uint64(0), - errors.New("database error"), - ) + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xBBBB") + // The tx-hash index lookup fails for a non-missing-key reason + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), errors.New("database error")) trace, httpHeader, err := handler.TraceTransaction(t.Context(), hash) assert.Empty(t, trace) - assert.Equal(t, rpccore.ErrTxnHashNotFound, err) + assert.Equal(t, rpccore.ErrInternal.CloneWithData(errors.New("database error")), err) assert.Equal(t, httpHeader.Get(rpcv10.ExecutionStepsHeader), "0") }) }) t.Run("ok", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]( + hash := felt.NewUnsafeFromString[felt.TransactionHash]( "0x37b244ea7dc6b3f9735fba02d183ef0d6807a572dd91a63cc1b14b923c1ac0", ) tx := &core.DeclareTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), ClassHash: felt.NewUnsafeFromString[felt.Felt]("0x000000000"), Version: new(core.TransactionVersion).SetUint64(1), } @@ -385,8 +398,10 @@ func TestTraceTransaction(t *testing.T) { Class: &core.SierraClass{}, } - mockReader.EXPECT().Receipt(hash).Return(nil, header.Hash, header.Number, nil) - mockReader.EXPECT().BlockByHash(header.Hash).Return(block, nil) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash).Return(header.Number, uint64(0), nil) + mockReader.EXPECT().BlockHeaderByNumber(header.Number).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(nil, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) @@ -427,9 +442,11 @@ func TestTraceTransaction(t *testing.T) { }) t.Run("pre_confirmed block", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xceb6a374aff2bbb3537cf35f50df8634b2354a21") + hash := felt.NewUnsafeFromString[felt.TransactionHash]( + "0xceb6a374aff2bbb3537cf35f50df8634b2354a21", + ) tx := &core.InvokeTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), Version: new(core.TransactionVersion).SetUint64(1), } @@ -448,7 +465,8 @@ func TestTraceTransaction(t *testing.T) { Transactions: []core.Transaction{tx}, } - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) preConfirmedStateDiff := core.EmptyStateDiff() preConfirmed := pending.PreConfirmed{ Block: block, @@ -500,9 +518,9 @@ func TestTraceTransaction(t *testing.T) { // the tip. findAndTraceInPreConfirmed must walk the chain newest-first and // reconstruct state at the matching entry. t.Run("pre_confirmed multi-block chain - tx in non-tip entry", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xdeadbeef") + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xdeadbeef") tx := &core.InvokeTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), Version: new(core.TransactionVersion).SetUint64(1), } @@ -529,7 +547,8 @@ func TestTraceTransaction(t *testing.T) { StateUpdate: &core.StateUpdate{StateDiff: &tipDiff}, } - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) mockSyncReader.EXPECT().PreConfirmedChain(). Return(mustNewChain(t, &baseEntry, &tipEntry), nil) // Base resolution: bottom (= baseHeader.Number) - 1. @@ -568,20 +587,37 @@ func TestTraceTransaction(t *testing.T) { gateway := adaptfeeder.New(client) // Tx at index 3 in the block - revertedTxHash := felt.NewUnsafeFromString[felt.Felt]( + revertedTxHash := felt.NewUnsafeFromString[felt.TransactionHash]( "0x2f00c7f28df2197196440747f97baa63d0851e3b0cfc2efedb6a88a7ef78cb1", ) blockNumber := uint64(18) - blockHash := felt.NewUnsafeFromString[felt.Felt]( - "0x5beb56c7d9a9fc066e695c3fc467f45532cace83d9979db4ccfd6b77ca476af", - ) - mockReader.EXPECT().Receipt(revertedTxHash).Return(nil, blockHash, blockNumber, nil) - mockReader.EXPECT().BlockByHash(blockHash). - DoAndReturn(func(_ *felt.Felt) (block *core.Block, err error) { - return gateway.BlockByNumber(t.Context(), blockNumber) + gatewayBlock, gatewayErr := gateway.BlockByNumber(t.Context(), blockNumber) + require.NoError(t, gatewayErr) + revertedTxIndex := slices.IndexFunc(gatewayBlock.Transactions, func(tx core.Transaction) bool { + return tx.Hash().Equal((*felt.Felt)(revertedTxHash)) + }) + require.NotEqual(t, -1, revertedTxIndex) + + mockReader.EXPECT().BlockNumberAndIndexByTxHash(revertedTxHash). + Return(blockNumber, uint64(revertedTxIndex), nil) + mockReader.EXPECT().BlockHeaderByNumber(blockNumber). + DoAndReturn(func(number uint64) (*core.Header, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, err + } + return block.Header, nil }) + mockReader.EXPECT().TransactionsAndReceiptsByBlockNumber(blockNumber). + DoAndReturn(func(number uint64) ([]core.Transaction, []*core.TransactionReceipt, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, nil, err + } + return block.Transactions, block.Receipts, nil + }).AnyTimes() expectedRevertedTrace := readTestData[rpcv10.TransactionTrace]( t, @@ -671,7 +707,9 @@ func TestTraceBlockTransactions(t *testing.T) { Class: &core.SierraClass{}, } - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil) + mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(nil, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) @@ -996,7 +1034,7 @@ func TestAdaptFeederBlockTrace(t *testing.T) { t.Run("nil block trace", func(t *testing.T) { block := &core.Block{} - res, err := rpcv10.AdaptFeederBlockTrace(block, nil) + res, err := rpcv10.AdaptFeederBlockTrace(block.Transactions, nil) require.Nil(t, res) require.Nil(t, err) }) @@ -1007,7 +1045,7 @@ func TestAdaptFeederBlockTrace(t *testing.T) { } blockTrace := &starknet.BlockTrace{} - res, err := rpcv10.AdaptFeederBlockTrace(block, blockTrace) + res, err := rpcv10.AdaptFeederBlockTrace(block.Transactions, blockTrace) require.Nil(t, res) require.Equal(t, errors.New("mismatched number of txs and traces"), err) }) @@ -1065,7 +1103,7 @@ func TestAdaptFeederBlockTrace(t *testing.T) { }, } - res, err := rpcv10.AdaptFeederBlockTrace(block, blockTrace) + res, err := rpcv10.AdaptFeederBlockTrace(block.Transactions, blockTrace) require.Nil(t, err) require.Equal(t, expectedAdaptedTrace, res) }) @@ -1110,7 +1148,7 @@ func TestAdaptFeederBlockTrace(t *testing.T) { }, } - res, err := rpcv10.AdaptFeederBlockTrace(block, blockTrace) + res, err := rpcv10.AdaptFeederBlockTrace(block.Transactions, blockTrace) require.Nil(t, err) require.Equal(t, expectedAdaptedTrace, res) }) @@ -1356,7 +1394,9 @@ func TestTraceBlockTransactionsWithReturnInitialReads(t *testing.T) { revealedHeader := &core.Header{Hash: &revealedHash} mockReader.EXPECT().Network().Return(n).AnyTimes() - mockReader.EXPECT().BlockByHash(&blockHash).Return(block, nil) + mockReader.EXPECT().BlockHeaderByHash(&blockHash).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) mockReader.EXPECT().StateAtBlockHash(&parentHash).Return(mockState, nopCloser, nil) mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() @@ -1477,8 +1517,12 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { BlockHeaderHashByNumber(uint64(90)). Return(revealedHeader.Hash, nil). AnyTimes() - mockReader.EXPECT().Receipt(txHash).Return(nil, blockHash, block.Number, nil) - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil).Times(2) + mockReader.EXPECT().BlockNumberAndIndexByTxHash((*felt.TransactionHash)(txHash)). + Return(block.Number, uint64(0), nil) + 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) @@ -1495,7 +1539,7 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { handler := rpcv10.New(mockReader, nil, mockVM, log.NewNopZapLogger()) - _, _, err := handler.TraceTransaction(t.Context(), txHash) + _, _, err := handler.TraceTransaction(t.Context(), (*felt.TransactionHash)(txHash)) require.Nil(t, err) blockID := rpcv10.BlockIDFromHash(blockHash) @@ -1525,7 +1569,10 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { BlockHeaderHashByNumber(uint64(90)). Return(revealedHeader.Hash, nil). AnyTimes() - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil).Times(2) + 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) @@ -1564,7 +1611,10 @@ func TestTraceBlockTransactionsInitialReadsCacheCoherence(t *testing.T) { BlockHeaderHashByNumber(uint64(90)). Return(revealedHeader.Hash, nil). AnyTimes() - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil).Times(2) + 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) diff --git a/rpc/v10/transaction_test.go b/rpc/v10/transaction_test.go index 2fdc5a7046..820104411d 100644 --- a/rpc/v10/transaction_test.go +++ b/rpc/v10/transaction_test.go @@ -1133,7 +1133,7 @@ func TestTransactionReceiptByHash(t *testing.T) { preConfirmed := test.preConfirmedFn(t, loadedBlock) mockSyncReader.EXPECT().PreConfirmedChain().Return(mustNewChain(t, preConfirmed), nil) - _, err := preConfirmed.ReceiptByHash(transaction.Hash()) + _, err := preConfirmed.ReceiptByHash((*felt.TransactionHash)(transaction.Hash())) if err != nil { // receipt belong to canonical block mock expectations mockReader.EXPECT().BlockNumberAndIndexByTxHash( diff --git a/rpc/v8/handlers.go b/rpc/v8/handlers.go index ee93366567..b63e7957d1 100644 --- a/rpc/v8/handlers.go +++ b/rpc/v8/handlers.go @@ -11,6 +11,7 @@ import ( "github.com/NethermindEth/juno/blockchain" "github.com/NethermindEth/juno/clients/feeder" "github.com/NethermindEth/juno/core" + "github.com/NethermindEth/juno/core/felt" pendingpkg "github.com/NethermindEth/juno/core/pending" "github.com/NethermindEth/juno/feed" "github.com/NethermindEth/juno/jsonrpc" @@ -43,7 +44,7 @@ type Handler struct { idgen func() string subscriptions stdsync.Map // map[string]*subscription - blockTraceCache *lru.Cache[rpccore.TraceCacheKey, []TracedBlockTransaction] + blockTraceCache *lru.Cache[felt.Felt, []TracedBlockTransaction] submittedTransactionsCache *rpccore.TransactionCache filterLimit uint @@ -82,7 +83,7 @@ func New( l1Heads: feed.New[*core.L1Head](), blockTraceCache: lru.New[ - rpccore.TraceCacheKey, + felt.Felt, []TracedBlockTransaction, ](rpccore.TraceCacheSize), filterLimit: math.MaxUint, diff --git a/rpc/v8/trace.go b/rpc/v8/trace.go index 47f1261036..60856994c2 100644 --- a/rpc/v8/trace.go +++ b/rpc/v8/trace.go @@ -148,7 +148,7 @@ func (h *Handler) traceBlockTransactions( isPending := block.Hash == nil if !isPending { // Check if it was already traced - traces, hit := h.blockTraceCache.Get(rpccore.TraceCacheKey{BlockHash: *block.Hash}) + traces, hit := h.blockTraceCache.Get(*block.Hash) if hit { return traces, defaultExecutionHeader(), nil } @@ -175,7 +175,7 @@ func (h *Handler) traceBlockTransactions( if err != nil { return nil, defaultExecutionHeader(), err } - h.blockTraceCache.Add(rpccore.TraceCacheKey{BlockHash: *block.Hash}, traces) + h.blockTraceCache.Add(*block.Hash, traces) return traces, defaultExecutionHeader(), nil } } @@ -268,7 +268,7 @@ func (h *Handler) traceBlockTransactionWithVM(block *core.Block) ( } if !isPending { - h.blockTraceCache.Add(rpccore.TraceCacheKey{BlockHash: *block.Hash}, result) + h.blockTraceCache.Add(*block.Hash, result) } return result, httpHeader, nil diff --git a/rpc/v9/adapters.go b/rpc/v9/adapters.go index efe4be1f04..3b93834ac9 100644 --- a/rpc/v9/adapters.go +++ b/rpc/v9/adapters.go @@ -3,6 +3,7 @@ package rpcv9 import ( "errors" + "github.com/NethermindEth/juno/core" "github.com/NethermindEth/juno/core/felt" "github.com/NethermindEth/juno/starknet" "github.com/NethermindEth/juno/vm" @@ -157,12 +158,15 @@ func AdaptVMExecutionResources(r *vm.ExecutionResources) ExecutionResources { Feeder Adapters *****************************************************/ -func AdaptFeederBlockTrace(block *BlockWithTxs, blockTrace *starknet.BlockTrace) ([]TracedBlockTransaction, error) { +func AdaptFeederBlockTrace( + transactions []core.Transaction, + blockTrace *starknet.BlockTrace, +) ([]TracedBlockTransaction, error) { if blockTrace == nil { return nil, nil } - if len(block.Transactions) != len(blockTrace.Traces) { + if len(transactions) != len(blockTrace.Traces) { return nil, errors.New("mismatched number of txs and traces") } @@ -172,7 +176,7 @@ func AdaptFeederBlockTrace(block *BlockWithTxs, blockTrace *starknet.BlockTrace) feederTrace := &blockTrace.Traces[index] trace := TransactionTrace{ - Type: block.Transactions[index].Type, + Type: transactionTypeFrom(transactions[index]), } if feederTrace.FeeTransferInvocation != nil && trace.Type != TxnL1Handler { diff --git a/rpc/v9/handlers.go b/rpc/v9/handlers.go index 7c22fdccb2..517aa4f0f0 100644 --- a/rpc/v9/handlers.go +++ b/rpc/v9/handlers.go @@ -11,6 +11,7 @@ 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" @@ -43,9 +44,7 @@ type Handler struct { idgen func() string subscriptions stdsync.Map // map[string]*subscription - // todo(rdr): why do we have the `TraceCacheKey` type and why it feels uncomfortable - // to use. It makes no sense, why not use `Felt` or `Hash` directly? - blockTraceCache *lru.Cache[rpccore.TraceCacheKey, []TracedBlockTransaction] + blockTraceCache *lru.Cache[felt.Felt, []TracedBlockTransaction] // todo(rdr): Can this cache be genericified and can it be applied to the `blockTraceCache` submittedTransactionsCache *rpccore.TransactionCache @@ -85,7 +84,7 @@ func New( l1Heads: feed.New[*core.L1Head](), blockTraceCache: lru.New[ - rpccore.TraceCacheKey, + felt.Felt, []TracedBlockTransaction, ](rpccore.TraceCacheSize), filterLimit: math.MaxUint, diff --git a/rpc/v9/handlers_test.go b/rpc/v9/handlers_test.go index 13fbd7375a..c994e07b33 100644 --- a/rpc/v9/handlers_test.go +++ b/rpc/v9/handlers_test.go @@ -86,7 +86,9 @@ func TestThrottledVMError(t *testing.T) { Transactions: []core.Transaction{l1Tx, declareTx}, } - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil) + mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) state := mocks.NewMockStateReader(mockCtrl) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(state, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) diff --git a/rpc/v9/trace.go b/rpc/v9/trace.go index 21f5d4b895..09d98d54a7 100644 --- a/rpc/v9/trace.go +++ b/rpc/v9/trace.go @@ -5,13 +5,13 @@ import ( "encoding/json" "errors" "net/http" - "slices" "strconv" "github.com/NethermindEth/juno/blockchain" "github.com/NethermindEth/juno/blockchain/networks" "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" @@ -84,16 +84,20 @@ type OrderedL2toL1Message struct { // It follows the specification defined here: // https://github.com/starkware-libs/starknet-specs/blob/9377851884da5c81f757b6ae0ed47e84f9e7c058/api/starknet_trace_api_openrpc.json#L11 func (h *Handler) TraceTransaction( - ctx context.Context, hash *felt.Felt, + ctx context.Context, hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { httpHeader := defaultExecutionHeader() - if trace, header, err := h.findAndTraceFinalisedTransaction(ctx, hash); err == nil { + trace, header, err := h.findAndTraceFinalisedTransaction(ctx, hash) + if err == nil { return trace, header, nil - } else if err != rpccore.ErrTxnHashNotFound { - return TransactionTrace{}, httpHeader, rpccore.ErrTxnHashNotFound } - trace, header, err := h.findAndTraceInPreConfirmed(hash) + if err != rpccore.ErrTxnHashNotFound { + return TransactionTrace{}, httpHeader, err + } + + // Not in a finalised block, so try the pre_confirmed chain. + trace, header, err = h.findAndTraceInPreConfirmed(hash) if err != nil { return TransactionTrace{}, httpHeader, err } @@ -111,12 +115,14 @@ func (h *Handler) TraceBlockTransactions( return nil, defaultExecutionHeader(), rpccore.ErrCallOnPreConfirmed } - block, rpcErr := h.blockByID(id) + // Resolve the block id once: the header pins the block number, so the reads that follow it + // go by number and a tag like `latest` cannot move between them. + header, rpcErr := h.blockHeaderByID(id) if rpcErr != nil { return nil, defaultExecutionHeader(), rpcErr } - return h.traceBlockTransactions(ctx, block) + return h.traceFinalisedBlock(ctx, header) } // https://github.com/starkware-libs/starknet-specs/blob/9377851884da5c81f757b6ae0ed47e84f9e7c058/api/starknet_api_openrpc.json#L579 @@ -286,9 +292,9 @@ func fetchDeclaredClassesAndL1Fees( // findAndTraceFinalisedTransaction searches for a transaction in // finalised blocks and returns its trace. func (h *Handler) findAndTraceFinalisedTransaction( - ctx context.Context, hash *felt.Felt, + ctx context.Context, hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { - _, blockHash, _, err := h.bcReader.Receipt(hash) + blockNumber, txIndex, err := h.bcReader.BlockNumberAndIndexByTxHash(hash) if err != nil { if !errors.Is(err, db.ErrKeyNotFound) { return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) @@ -296,19 +302,24 @@ func (h *Handler) findAndTraceFinalisedTransaction( return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } - block, err := h.bcReader.BlockByHash(blockHash) + header, err := h.bcReader.BlockHeaderByNumber(blockNumber) if err != nil { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + if errors.Is(err, db.ErrKeyNotFound) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } - txIndex, rpcErr := findTransactionInBlock(block, hash) + blockTraces, httpHeader, rpcErr := h.traceFinalisedBlock(ctx, header) if rpcErr != nil { - return TransactionTrace{}, defaultExecutionHeader(), rpccore.ErrTxnHashNotFound + return TransactionTrace{}, nil, rpcErr } - blockTraces, httpHeader, rpcErr := h.traceBlockTransactions(ctx, block) - if rpcErr != nil { - return TransactionTrace{}, nil, 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. + if txIndex >= uint64(len(blockTraces)) || + !blockTraces[txIndex].TransactionHash.Equal((*felt.Felt)(hash)) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound } return *blockTraces[txIndex].TraceRoot, httpHeader, nil @@ -320,11 +331,14 @@ func (h *Handler) findAndTraceFinalisedTransaction( // entry's diff from chain bottom up to entry's block, then the entry's own // transaction-level diffs up to (but not including) txIndex. func (h *Handler) findAndTraceInPreConfirmed( - hash *felt.Felt, + hash *felt.TransactionHash, ) (TransactionTrace, http.Header, *jsonrpc.Error) { chain, err := h.syncReader.PreConfirmedChain() if err != nil { - return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + if errors.Is(err, db.ErrKeyNotFound) || errors.Is(err, pending.ErrPreConfirmedNotFound) { + return TransactionTrace{}, nil, rpccore.ErrTxnHashNotFound + } + return TransactionTrace{}, nil, rpccore.ErrInternal.CloneWithData(err) } for entry := range chain.NewestFirst() { @@ -368,42 +382,60 @@ func (h *Handler) findAndTraceInPreConfirmed( Block Tracing Helpers *****************************************************/ -// traceBlockTransactions gets the trace for a block. The block will always be traced locally except +// 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 -func (h *Handler) traceBlockTransactions( - ctx context.Context, block *core.Block, +func (h *Handler) traceFinalisedBlock( + ctx context.Context, header *core.Header, ) ([]TracedBlockTransaction, http.Header, *jsonrpc.Error) { // Check if it was already traced - traces, hit := h.blockTraceCache.Get(rpccore.TraceCacheKey{BlockHash: *block.Hash}) - if hit { + cacheKey := *header.Hash + if traces, hit := h.blockTraceCache.Get(cacheKey); hit { return traces, defaultExecutionHeader(), nil } - fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(block, h.bcReader.Network()) + fetchFromFeederGW, err := shouldFetchTracesFromFeederGateway(header, h.bcReader.Network()) if err != nil { return nil, defaultExecutionHeader(), rpccore.ErrUnexpectedError.CloneWithData(err.Error()) } + var ( + traces []TracedBlockTransaction + httpHeader = defaultExecutionHeader() + rpcErr *jsonrpc.Error + ) if fetchFromFeederGW { - traces, err := h.fetchTracesFromFeederGateway(ctx, block) - if err != nil { - return nil, defaultExecutionHeader(), err + traces, rpcErr = h.fetchTracesFromFeederGateway(ctx, header) + } else { + transactions, txErr := h.bcReader.TransactionsByBlockNumber(header.Number) + if txErr != nil { + if errors.Is(txErr, db.ErrKeyNotFound) { + return nil, httpHeader, rpccore.ErrBlockNotFound + } + return nil, httpHeader, rpccore.ErrInternal.CloneWithData(txErr) } - h.blockTraceCache.Add(rpccore.TraceCacheKey{BlockHash: *block.Hash}, traces) - return traces, defaultExecutionHeader(), nil + traces, httpHeader, rpcErr = h.traceBlockWithVM(header, transactions) + } + if rpcErr != nil { + return nil, httpHeader, rpcErr } - return h.traceBlockWithVM(block) + h.blockTraceCache.Add(cacheKey, traces) + + return traces, httpHeader, nil } -// traceBlockWithVM traces a block using the local VM and stores the result in the block cache. -func (h *Handler) traceBlockWithVM(block *core.Block) ( - []TracedBlockTransaction, http.Header, *jsonrpc.Error, -) { +// traceBlockWithVM traces a block using the local VM. Caching is the caller's responsibility. +func (h *Handler) traceBlockWithVM( + header *core.Header, + transactions []core.Transaction, +) ([]TracedBlockTransaction, http.Header, *jsonrpc.Error) { // Prepare execution state - state, closer, err := h.bcReader.StateAtBlockHash(block.ParentHash) + state, closer, err := h.bcReader.StateAtBlockHash(header.ParentHash) if err != nil { - return nil, defaultExecutionHeader(), rpccore.ErrBlockNotFound + if errors.Is(err, db.ErrKeyNotFound) { + return nil, defaultExecutionHeader(), rpccore.ErrBlockNotFound + } + return nil, defaultExecutionHeader(), rpccore.ErrInternal.CloneWithData(err) } defer h.callAndLogErr(closer, "Failed to close state in traceBlockTransactions") @@ -420,14 +452,14 @@ func (h *Handler) traceBlockWithVM(block *core.Block) ( defer h.callAndLogErr(headStateCloser, "Failed to close head state in traceBlockTransactions") // Create block info - blockInfo, rpcErr := h.buildBlockInfo(block.Header) + blockInfo, rpcErr := h.buildBlockInfo(header) if rpcErr != nil { return nil, defaultExecutionHeader(), rpcErr } traces, httpHeader, rpcErr := traceTransactionsWithState( h.vm, - block.Transactions, + transactions, state, headState, &blockInfo, @@ -436,41 +468,37 @@ func (h *Handler) traceBlockWithVM(block *core.Block) ( return nil, httpHeader, rpcErr } - h.blockTraceCache.Add(rpccore.TraceCacheKey{BlockHash: *block.Hash}, traces) - return traces, httpHeader, nil } // fetchTracesFromFeederGateway fetches block traces from the feeder gateway // and fills in missing data. func (h *Handler) fetchTracesFromFeederGateway( - ctx context.Context, block *core.Block, + ctx context.Context, header *core.Header, ) ([]TracedBlockTransaction, *jsonrpc.Error) { - // todo(rdr): this feels unnatural, why if I have the `core.Block` should I still - // try to go for the rpcBlock? Ideally we extract all the info directly from `core.Block` - blockID := BlockIDFromHash(block.Hash) - rpcBlock, rpcErr := h.BlockWithTxs(&blockID) - if rpcErr != nil { - return nil, rpcErr - } - if h.feederClient == nil { return nil, rpccore.ErrInternal.CloneWithData("no feeder client configured") } - blockTrace, err := h.feederClient.BlockTrace(ctx, block.Hash.String()) + blockTrace, err := h.feederClient.BlockTrace(ctx, header.Hash.String()) if err != nil { return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) } - traces, err := AdaptFeederBlockTrace(rpcBlock, &blockTrace) + transactions, receipts, err := h.bcReader.TransactionsAndReceiptsByBlockNumber(header.Number) if err != nil { - return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + if errors.Is(err, db.ErrKeyNotFound) { + return nil, rpccore.ErrBlockNotFound + } + return nil, rpccore.ErrInternal.CloneWithData(err) } - traces = fillFeederGatewayData(traces, block.Receipts) + traces, err := AdaptFeederBlockTrace(transactions, &blockTrace) + if err != nil { + return nil, rpccore.ErrUnexpectedError.CloneWithData(err.Error()) + } - return traces, nil + return fillFeederGatewayData(traces, receipts), nil } // buildBlockInfo builds block info for VM execution. @@ -489,22 +517,22 @@ func (h *Handler) buildBlockInfo(header *core.Header) (vm.BlockInfo, *jsonrpc.Er // shouldFetchTracesFromFeederGateway determines if // traces for a block should be fetched from the feeder gateway. func shouldFetchTracesFromFeederGateway( - block *core.Block, + header *core.Header, network *networks.Network, ) (bool, error) { - blockVer, err := core.ParseBlockVersion(block.ProtocolVersion) + blockVer, err := core.ParseBlockVersion(header.ProtocolVersion) if err != nil { return false, err } // We rely on the feeder gateway for Starknet version strictly older than "0.13.1.1" fetchFromFeederGW := blockVer.LessThan(core.Ver0_13_2) && - block.ProtocolVersion != "0.13.1.1" + header.ProtocolVersion != "0.13.1.1" // This specific block range caused a re-org, also related with Cairo 0 and we have to // depend on the Sequencer to provide the correct traces fetchFromFeederGW = fetchFromFeederGW || - (block.Number >= 1943705 && - block.Number <= 1952704 && + (header.Number >= 1943705 && + header.Number <= 1952704 && *network == networks.Mainnet) return fetchFromFeederGW, nil @@ -554,16 +582,3 @@ func defaultExecutionHeader() http.Header { header.Set(ExecutionStepsHeader, "0") return header } - -// findTransactionInBlock locates the index of a transaction with the given hash in a block. -// -// Returns the index of the transaction and nil if found, or 0 and ErrTxnHashNotFound if not found. -func findTransactionInBlock(block *core.Block, hash *felt.Felt) (uint, *jsonrpc.Error) { - txIndex := slices.IndexFunc(block.Transactions, func(tx core.Transaction) bool { - return tx.Hash().Equal(hash) - }) - if txIndex == -1 { - return 0, rpccore.ErrTxnHashNotFound - } - return uint(txIndex), nil -} diff --git a/rpc/v9/trace_test.go b/rpc/v9/trace_test.go index 6ec1491495..14670f1574 100644 --- a/rpc/v9/trace_test.go +++ b/rpc/v9/trace_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "testing" "github.com/NethermindEth/juno/blockchain" @@ -115,43 +116,38 @@ func AssertTracedBlockTransactions( mockReader := mocks.NewMockReader(mockCtrl) - mockReader.EXPECT().BlockByNumber(gomock.Any()).DoAndReturn(func(number uint64) (block *core.Block, err error) { - block, err = gateway.BlockByNumber(t.Context(), number) + mockReader.EXPECT().BlockHeaderByNumber(gomock.Any()).DoAndReturn( + func(number uint64) (*core.Header, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, err + } + return block.Header, nil + }).AnyTimes() + + mockReader.EXPECT().TransactionsAndReceiptsByBlockNumber(gomock.Any()).DoAndReturn( + func(number uint64) ([]core.Transaction, []*core.TransactionReceipt, error) { + block, err := gateway.BlockByNumber(t.Context(), number) + if err != nil { + return nil, nil, err + } - // Simulate gas consumption in block receipts - for _, receipt := range block.Receipts { - receipt.ExecutionResources.TotalGasConsumed = &core.GasConsumed{ - L1Gas: 5, - L2Gas: 10, - L1DataGas: 15, + // Simulate gas consumption in block receipts + for _, receipt := range block.Receipts { + receipt.ExecutionResources.TotalGasConsumed = &core.GasConsumed{ + L1Gas: 5, + L2Gas: 10, + L1DataGas: 15, + } } - } - return block, err - }).AnyTimes() + return block.Transactions, block.Receipts, nil + }).AnyTimes() mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() mockReader.EXPECT().Network().Return(n).AnyTimes() for description, test := range tests { t.Run(description, func(t *testing.T) { - blockHash := felt.NewUnsafeFromString[felt.Felt](test.blockHash) - mockReader.EXPECT().BlockHeaderByHash(blockHash).DoAndReturn( - func(_ *felt.Felt) (*core.Header, error) { - block, err := mockReader.BlockByNumber(test.blockNumber) - if err != nil { - return nil, err - } - return block.Header, nil - }) - mockReader.EXPECT().TransactionsByBlockNumber(test.blockNumber).DoAndReturn( - func(number uint64) ([]core.Transaction, error) { - block, err := mockReader.BlockByNumber(test.blockNumber) - if err != nil { - return nil, err - } - return block.Transactions, nil - }) - handler := rpc.New(mockReader, nil, nil, nil) handler = handler.WithFeeder(client) blockID := blockIDNumber(t, test.blockNumber) @@ -181,27 +177,15 @@ func TestTraceBlockTransactionsReturnsError(t *testing.T) { blockNumber := uint64(40000) - mockReader.EXPECT().BlockByNumber(gomock.Any()).DoAndReturn( - func(number uint64) (block *core.Block, err error) { - return gateway.BlockByNumber(t.Context(), number) - }) - mockReader.EXPECT().BlockHeaderByHash(gomock.Any()).DoAndReturn( - func(hash *felt.Felt) (*core.Header, error) { - block, err := gateway.BlockByNumber(t.Context(), blockNumber) + mockReader.EXPECT().BlockHeaderByNumber(blockNumber).DoAndReturn( + func(number uint64) (*core.Header, error) { + block, err := gateway.BlockByNumber(t.Context(), number) if err != nil { return nil, err } return block.Header, nil }) - mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).DoAndReturn( - func(number uint64) ([]core.Transaction, error) { - block, err := gateway.BlockByNumber(t.Context(), blockNumber) - if err != nil { - return nil, err - } - return block.Transactions, nil - }) - mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound) + mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).AnyTimes() mockReader.EXPECT().Network().Return(&network) // No feeder client is set @@ -335,9 +319,10 @@ func TestTraceTransaction(t *testing.T) { t.Run("not found", func(t *testing.T) { t.Run("key not found", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xBBBB") + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xBBBB") // Receipt() returns error related to db - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) preConfirmed := pending.NewPreConfirmed(&core.Block{}, nil, nil, "") mockSyncReader.EXPECT().PreConfirmedChain().Return(mustNewChain(t, &preConfirmed), nil) @@ -348,20 +333,23 @@ func TestTraceTransaction(t *testing.T) { }) t.Run("other error", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xBBBB") - // Receipt() returns some other error - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), errors.New("database error")) + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xBBBB") + // The tx-hash index lookup fails for a non-missing-key reason + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), errors.New("database error")) trace, httpHeader, err := handler.TraceTransaction(t.Context(), hash) assert.Empty(t, trace) - assert.Equal(t, rpccore.ErrTxnHashNotFound, err) + assert.Equal(t, rpccore.ErrInternal.CloneWithData(errors.New("database error")), err) assert.Equal(t, httpHeader.Get(rpc.ExecutionStepsHeader), "0") }) }) t.Run("ok", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0x37b244ea7dc6b3f9735fba02d183ef0d6807a572dd91a63cc1b14b923c1ac0") + hash := felt.NewUnsafeFromString[felt.TransactionHash]( + "0x37b244ea7dc6b3f9735fba02d183ef0d6807a572dd91a63cc1b14b923c1ac0", + ) tx := &core.DeclareTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), ClassHash: felt.NewUnsafeFromString[felt.Felt]("0x000000000"), Version: new(core.TransactionVersion).SetUint64(1), } @@ -383,8 +371,10 @@ func TestTraceTransaction(t *testing.T) { Class: &core.SierraClass{}, } - mockReader.EXPECT().Receipt(hash).Return(nil, header.Hash, header.Number, nil) - mockReader.EXPECT().BlockByHash(header.Hash).Return(block, nil) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash).Return(header.Number, uint64(0), nil) + mockReader.EXPECT().BlockHeaderByNumber(header.Number).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(nil, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) @@ -426,9 +416,11 @@ func TestTraceTransaction(t *testing.T) { }) t.Run("pre_confirmed block", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xceb6a374aff2bbb3537cf35f50df8634b2354a21") + hash := felt.NewUnsafeFromString[felt.TransactionHash]( + "0xceb6a374aff2bbb3537cf35f50df8634b2354a21", + ) tx := &core.InvokeTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), Version: new(core.TransactionVersion).SetUint64(1), } @@ -447,7 +439,8 @@ func TestTraceTransaction(t *testing.T) { Transactions: []core.Transaction{tx}, } - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) preConfirmedStateDiff := core.EmptyStateDiff() preConfirmed := pending.PreConfirmed{ Block: block, @@ -500,9 +493,9 @@ func TestTraceTransaction(t *testing.T) { // the tip. findAndTraceInPreConfirmed must walk newest-first and // reconstruct state at the matching entry. t.Run("pre_confirmed multi-block chain - tx in non-tip entry", func(t *testing.T) { - hash := felt.NewUnsafeFromString[felt.Felt]("0xdeadbeef") + hash := felt.NewUnsafeFromString[felt.TransactionHash]("0xdeadbeef") tx := &core.InvokeTransaction{ - TransactionHash: hash, + TransactionHash: (*felt.Felt)(hash), Version: new(core.TransactionVersion).SetUint64(1), } @@ -529,7 +522,8 @@ func TestTraceTransaction(t *testing.T) { StateUpdate: &core.StateUpdate{StateDiff: &tipDiff}, } - mockReader.EXPECT().Receipt(hash).Return(nil, nil, uint64(0), db.ErrKeyNotFound) + mockReader.EXPECT().BlockNumberAndIndexByTxHash(hash). + Return(uint64(0), uint64(0), db.ErrKeyNotFound) mockSyncReader.EXPECT().PreConfirmedChain(). Return(mustNewChain(t, &baseEntry, &tipEntry), nil) mockReader.EXPECT().StateAtBlockNumber(baseHeader.Number-1). @@ -568,34 +562,25 @@ func TestTraceTransaction(t *testing.T) { gateway := adaptfeeder.New(client) // Tx at index 3 in the block - revertedTxHash := felt.NewUnsafeFromString[felt.Felt]("0x2f00c7f28df2197196440747f97baa63d0851e3b0cfc2efedb6a88a7ef78cb1") + revertedTxHash := felt.NewUnsafeFromString[felt.TransactionHash]( + "0x2f00c7f28df2197196440747f97baa63d0851e3b0cfc2efedb6a88a7ef78cb1", + ) blockNumber := uint64(18) - blockHash := felt.NewUnsafeFromString[felt.Felt]("0x5beb56c7d9a9fc066e695c3fc467f45532cace83d9979db4ccfd6b77ca476af") - mockReader.EXPECT().Receipt(revertedTxHash).Return(nil, blockHash, blockNumber, nil) - mockReader.EXPECT().BlockByHash(blockHash).DoAndReturn(func(_ *felt.Felt) (block *core.Block, err error) { - return gateway.BlockByNumber(t.Context(), blockNumber) + gatewayBlock, gatewayErr := gateway.BlockByNumber(t.Context(), blockNumber) + require.NoError(t, gatewayErr) + revertedTxIndex := slices.IndexFunc(gatewayBlock.Transactions, func(tx core.Transaction) bool { + return tx.Hash().Equal((*felt.Felt)(revertedTxHash)) }) - mockReader.EXPECT().BlockHeaderByHash(blockHash).DoAndReturn( - func(_ *felt.Felt) (*core.Header, error) { - block, err := gateway.BlockByNumber(t.Context(), blockNumber) - if err != nil { - return nil, err - } - return block.Header, nil - }) - mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).DoAndReturn( - func(number uint64) ([]core.Transaction, error) { - block, err := gateway.BlockByNumber(t.Context(), blockNumber) - if err != nil { - return nil, err - } - return block.Transactions, nil - }) - mockReader.EXPECT().L1Head().Return(core.L1Head{ - BlockNumber: 19, // Doesn't really matter for this test - }, nil) + require.NotEqual(t, -1, revertedTxIndex) + + mockReader.EXPECT().BlockNumberAndIndexByTxHash(revertedTxHash). + Return(blockNumber, uint64(revertedTxIndex), nil) + mockReader.EXPECT().BlockHeaderByNumber(blockNumber). + Return(gatewayBlock.Header, nil) + mockReader.EXPECT().TransactionsAndReceiptsByBlockNumber(blockNumber). + Return(gatewayBlock.Transactions, gatewayBlock.Receipts, nil) expectedRevertedTrace := rpc.TransactionTrace{ Type: rpc.TxnInvoke, @@ -775,7 +760,9 @@ func TestTraceBlockTransactions(t *testing.T) { Class: &core.SierraClass{}, } - mockReader.EXPECT().BlockByHash(blockHash).Return(block, nil) + mockReader.EXPECT().BlockHeaderByHash(blockHash).Return(header, nil) + mockReader.EXPECT().TransactionsByBlockNumber(header.Number). + Return(block.Transactions, nil) mockReader.EXPECT().StateAtBlockHash(header.ParentHash).Return(nil, nopCloser, nil) headState := mocks.NewMockStateReader(mockCtrl) @@ -1099,34 +1086,22 @@ func TestAdaptVMTransactionTrace(t *testing.T) { func TestAdaptFeederBlockTrace(t *testing.T) { t.Run("nil block trace", func(t *testing.T) { - block := &rpc.BlockWithTxs{} - - res, err := rpc.AdaptFeederBlockTrace(block, nil) + res, err := rpc.AdaptFeederBlockTrace(nil, nil) require.Nil(t, res) require.Nil(t, err) }) t.Run("inconsistent blockWithTxs and blockTrace", func(t *testing.T) { - blockWithTxs := &rpc.BlockWithTxs{ - Transactions: []rpc.Transaction{ - {}, - }, - } + transactions := []core.Transaction{&core.InvokeTransaction{}} blockTrace := &starknet.BlockTrace{} - res, err := rpc.AdaptFeederBlockTrace(blockWithTxs, blockTrace) + res, err := rpc.AdaptFeederBlockTrace(transactions, blockTrace) require.Nil(t, res) require.Equal(t, errors.New("mismatched number of txs and traces"), err) }) t.Run("L1_HANDLER tx gets successfully adapted", func(t *testing.T) { - blockWithTxs := &rpc.BlockWithTxs{ - Transactions: []rpc.Transaction{ - { - Type: rpc.TxnL1Handler, - }, - }, - } + transactions := []core.Transaction{&core.L1HandlerTransaction{}} blockTrace := &starknet.BlockTrace{ Traces: []starknet.TransactionTrace{ { @@ -1176,19 +1151,13 @@ func TestAdaptFeederBlockTrace(t *testing.T) { }, } - res, err := rpc.AdaptFeederBlockTrace(blockWithTxs, blockTrace) + res, err := rpc.AdaptFeederBlockTrace(transactions, blockTrace) require.Nil(t, err) require.Equal(t, expectedAdaptedTrace, res) }) t.Run("INVOKE tx gets successfully adapted (with revert error)", func(t *testing.T) { - blockWithTxs := &rpc.BlockWithTxs{ - Transactions: []rpc.Transaction{ - { - Type: rpc.TxnInvoke, - }, - }, - } + transactions := []core.Transaction{&core.InvokeTransaction{}} blockTrace := &starknet.BlockTrace{ Traces: []starknet.TransactionTrace{ { @@ -1225,7 +1194,7 @@ func TestAdaptFeederBlockTrace(t *testing.T) { }, } - res, err := rpc.AdaptFeederBlockTrace(blockWithTxs, blockTrace) + res, err := rpc.AdaptFeederBlockTrace(transactions, blockTrace) require.Nil(t, err) require.Equal(t, expectedAdaptedTrace, res) }) diff --git a/utils/lru/lru_bench_test.go b/utils/lru/lru_bench_test.go index b72dc48f19..6314609410 100644 --- a/utils/lru/lru_bench_test.go +++ b/utils/lru/lru_bench_test.go @@ -13,7 +13,7 @@ import ( // Key shapes match real callsites: // - int baseline // - bloomKey {u64, u64} blockchain.EventFiltersCacheKey -// - feltKey [4]uint64 rpccore.TraceCacheKey (felt.Felt underlies as [4]uint64) +// - feltKey [4]uint64 the trace cache's felt.Felt key (underlies as [4]uint64) // // Sizes mirror production: // - 16 AggregatedBloomFilterCacheSize