fix: getBlockWithTx* fetch block header and txns separately

blockchain/blockchain.go

@@ -32,13 +32,16 @@ type Reader interface {
 	BlockHeaderByHash(hash *felt.Felt) (header *core.Header, err error)
 
 	BlockNumberByHash(hash *felt.Felt) (uint64, error)
-
-	TransactionByHash(hash *felt.Felt) (transaction core.Transaction, err error)
-	TransactionByBlockNumberAndIndex(blockNumber, index uint64) (transaction core.Transaction, err error)
 	BlockNumberAndIndexByTxHash(
 		hash *felt.TransactionHash,
 	) (blockNumber uint64, index uint64, err error)
 
+	TransactionByHash(hash *felt.Felt) (transaction core.Transaction, err error)
+	TransactionByBlockNumberAndIndex(
+		blockNumber, index uint64,
+	) (transaction core.Transaction, err error)
+	TransactionsByBlockNumber(blockNumber uint64) (transactions []core.Transaction, err error)
+
 	Receipt(hash *felt.Felt) (receipt *core.TransactionReceipt, blockHash *felt.Felt, blockNumber uint64, err error)
 	ReceiptByBlockNumberAndIndex(
 		blockNumber, index uint64,
@@ -226,6 +229,12 @@ func (b *Blockchain) TransactionByHash(hash *felt.Felt) (core.Transaction, error
 	return b.transactionLayout.TransactionByHash(b.database, (*felt.TransactionHash)(hash))
 }
 
+// TransactionsByBlockNumber gets all transactions for a given block number
+func (b *Blockchain) TransactionsByBlockNumber(number uint64) ([]core.Transaction, error) {
+	b.listener.OnRead("TransactionsByBlockNumber")
+	return b.transactionLayout.TransactionsByBlockNumber(b.database, number)
+}
+
 // BlockNumberAndIndexByTxHash gets transaction block number and index by Tx hash
 func (b *Blockchain) BlockNumberAndIndexByTxHash(
 	hash *felt.TransactionHash,

blockchain/blockchain_test.go

@@ -394,6 +394,25 @@ func TestTransactionAndReceipt(t *testing.T) {
 		}
 	})
 
+	t.Run("TransactionsByBlockNumber returns empty for non-existent block", func(t *testing.T) {
+		txns, err := chain.TransactionsByBlockNumber(32)
+		require.NoError(t, err)
+		assert.Empty(t, txns)
+	})
+
+	t.Run("TransactionsByBlockNumber returns all transactions for a block", func(t *testing.T) {
+		for i := range uint64(3) {
+			t.Run(fmt.Sprintf("mainnet block %v", i), func(t *testing.T) {
+				block, err := gw.BlockByNumber(t.Context(), i)
+				require.NoError(t, err)
+
+				txns, err := chain.TransactionsByBlockNumber(i)
+				require.NoError(t, err)
+				assert.Equal(t, block.Transactions, txns)
+			})
+		}
+	})
+
 	t.Run("GetReceipt returns expected receipt", func(t *testing.T) {
 		for i := range uint64(3) {
 			t.Run(fmt.Sprintf("mainnet block %v", i), func(t *testing.T) {
@@ -754,6 +773,11 @@ func TestRevert(t *testing.T) {
 		_, err := chain.TransactionByBlockNumberAndIndex(revertedHeight, 0)
 		require.Error(t, err)
 	})
+	t.Run("TransactionsByBlockNumber should return empty for reverted height", func(t *testing.T) {
+		txns, err := chain.TransactionsByBlockNumber(revertedHeight)
+		require.NoError(t, err)
+		assert.Empty(t, txns)
+	})
 
 	require.NoError(t, chain.RevertHead())
 	require.NoError(t, chain.RevertHead())

rpc/v10/block.go

@@ -68,34 +68,45 @@ type BlockWithReceipts struct {
 // It follows the specification defined here:
 // https://github.com/starkware-libs/starknet-specs/blob/cce1563eff702c87590bad3a48382d2febf1f7d9/api/starknet_api_openrpc.json#L25
 func (h *Handler) BlockWithTxHashes(id *rpcv9.BlockID) (*BlockWithTxHashes, *jsonrpc.Error) {
-	block, rpcErr := h.blockByID(id)
+	header, rpcErr := h.blockHeaderByID(id)
 	if rpcErr != nil {
 		return nil, rpcErr
 	}
 
-	txnHashes := make([]*felt.Felt, len(block.Transactions))
-	for index, txn := range block.Transactions {
+	var numID rpcv9.BlockID
+	if id.IsPreConfirmed() {
+		numID = *id
+	} else {
+		numID = rpcv9.BlockIDFromNumber(header.Number)
+	}
+	blockTxns, rpcErr := h.blockTxnsByNumber(&numID)
+	if rpcErr != nil {
+		return nil, rpcErr
+	}
+
+	txnHashes := make([]*felt.Felt, header.TransactionCount)
+	for index, txn := range blockTxns {
 		txnHashes[index] = txn.Hash()
 	}
 
-	status, rpcErr := h.blockStatus(id, block)
+	status, rpcErr := h.blockStatus(id, header.Number)
 	if rpcErr != nil {
 		return nil, rpcErr
 	}
 
 	var commitments *core.BlockCommitments
 	var stateDiff *core.StateDiff
-	if block.Hash != nil {
+	if header.Hash != nil {
 		var err error
-		commitments, stateDiff, err = h.getCommitmentsAndStateDiff(block.Number)
+		commitments, stateDiff, err = h.getCommitmentsAndStateDiff(header.Number)
 		if err != nil {
 			return nil, rpccore.ErrInternal.CloneWithData(err)
 		}
 	}
 
 	return &BlockWithTxHashes{
 		Status:      status,
-		BlockHeader: AdaptBlockHeader(block.Header, commitments, stateDiff),
+		BlockHeader: AdaptBlockHeader(header, commitments, stateDiff),
 		TxnHashes:   txnHashes,
 	}, nil
 }
@@ -115,7 +126,7 @@ func (h *Handler) BlockWithReceipts(
 		return nil, rpcErr
 	}
 
-	blockStatus, rpcErr := h.blockStatus(id, block)
+	blockStatus, rpcErr := h.blockStatus(id, block.Number)
 	if rpcErr != nil {
 		return nil, rpcErr
 	}
@@ -176,42 +187,53 @@ func (h *Handler) BlockWithTxs(
 ) (*BlockWithTxs, *jsonrpc.Error) {
 	includeProofFacts := responseFlags.IncludeProofFacts
 
-	block, rpcErr := h.blockByID(blockID)
+	header, rpcErr := h.blockHeaderByID(blockID)
 	if rpcErr != nil {
 		return nil, rpcErr
 	}
 
-	txs := make([]*Transaction, len(block.Transactions))
-	for index, txn := range block.Transactions {
+	var numID rpcv9.BlockID
+	if blockID.IsPreConfirmed() {
+		numID = *blockID
+	} else {
+		numID = rpcv9.BlockIDFromNumber(header.Number)
+	}
+	blockTxns, rpcErr := h.blockTxnsByNumber(&numID)
+	if rpcErr != nil {
+		return nil, rpcErr
+	}
+
+	txs := make([]*Transaction, len(blockTxns))
+	for index, txn := range blockTxns {
 		adaptedTx := AdaptTransaction(txn, includeProofFacts)
 		txs[index] = &adaptedTx
 	}
 
-	status, rpcErr := h.blockStatus(blockID, block)
+	status, rpcErr := h.blockStatus(blockID, header.Number)
 	if rpcErr != nil {
 		return nil, rpcErr
 	}
 
 	var commitments *core.BlockCommitments
 	var stateDiff *core.StateDiff
 	var err error
-	if block.Hash != nil {
-		commitments, stateDiff, err = h.getCommitmentsAndStateDiff(block.Number)
+	if header.Hash != nil {
+		commitments, stateDiff, err = h.getCommitmentsAndStateDiff(header.Number)
 		if err != nil {
 			return nil, rpccore.ErrInternal.CloneWithData(err)
 		}
 	}
 
 	return &BlockWithTxs{
 		Status:       status,
-		BlockHeader:  AdaptBlockHeader(block.Header, commitments, stateDiff),
+		BlockHeader:  AdaptBlockHeader(header, commitments, stateDiff),
 		Transactions: txs,
 	}, nil
 }
 
 func (h *Handler) blockStatus(
 	id *rpcv9.BlockID,
-	block *core.Block,
+	blockNumber uint64,
 ) (rpcv9.BlockStatus, *jsonrpc.Error) {
 	l1H, jsonErr := h.l1Head()
 	if jsonErr != nil {
@@ -221,7 +243,7 @@ func (h *Handler) blockStatus(
 	status := rpcv9.BlockAcceptedL2
 	if id.IsPreConfirmed() {
 		status = rpcv9.BlockPreConfirmed
-	} else if isL1Verified(block.Number, l1H) {
+	} else if isL1Verified(blockNumber, l1H) {
 		status = rpcv9.BlockAcceptedL1
 	}
 

rpc/v10/block_test.go

@@ -418,15 +418,27 @@ func setupMockBlockTest(
 		mockSyncReader.EXPECT().PendingData().Return(
 			&blockAsPreConfirmed,
 			nil,
-		)
+		).AnyTimes()
 	case blockID.IsLatest():
-		mockChain.EXPECT().Head().Return(block, nil)
+		mockChain.EXPECT().Head().Return(block, nil).AnyTimes()
+		mockChain.EXPECT().HeadsHeader().Return(block.Header, nil).AnyTimes()
+		mockChain.EXPECT().TransactionsByBlockNumber(block.Number).Return(
+			block.Transactions, nil).AnyTimes()
 	case blockID.IsHash():
-		mockChain.EXPECT().BlockByHash(block.Hash).Return(block, nil)
+		mockChain.EXPECT().BlockByHash(block.Hash).Return(block, nil).AnyTimes()
+		mockChain.EXPECT().BlockHeaderByHash(block.Hash).Return(block.Header, nil).AnyTimes()
+		mockChain.EXPECT().TransactionsByBlockNumber(block.Number).Return(
+			block.Transactions, nil).AnyTimes()
 	case blockID.IsL1Accepted():
-		mockChain.EXPECT().BlockByNumber(block.Number).Return(block, nil)
+		mockChain.EXPECT().BlockByNumber(block.Number).Return(block, nil).AnyTimes()
+		mockChain.EXPECT().BlockHeaderByNumber(block.Number).Return(block.Header, nil).AnyTimes()
+		mockChain.EXPECT().TransactionsByBlockNumber(block.Number).Return(
+			block.Transactions, nil).AnyTimes()
 	default:
-		mockChain.EXPECT().BlockByNumber(block.Number).Return(block, nil)
+		mockChain.EXPECT().BlockByNumber(block.Number).Return(block, nil).AnyTimes()
+		mockChain.EXPECT().BlockHeaderByNumber(block.Number).Return(block.Header, nil).AnyTimes()
+		mockChain.EXPECT().TransactionsByBlockNumber(block.Number).Return(
+			block.Transactions, nil).AnyTimes()
 	}
 }
 
@@ -523,6 +535,42 @@ func TestBlockWithTxHashes(t *testing.T) {
 	}
 }
 
+func TestBlockWithTxHashes_TxnsFetchError(t *testing.T) {
+	blockNumber := uint64(123)
+	header := &core.Header{Number: blockNumber}
+
+	t.Run("TransactionsByBlockNumber returns ErrKeyNotFound", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+		mockReader := mocks.NewMockReader(mockCtrl)
+		handler := rpcv10.New(mockReader, nil, nil, nil)
+
+		id := rpcv9.BlockIDFromNumber(blockNumber)
+		mockReader.EXPECT().BlockHeaderByNumber(blockNumber).Return(header, nil)
+		mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).Return(nil, db.ErrKeyNotFound)
+
+		block, rpcErr := handler.BlockWithTxHashes(&id)
+		assert.Nil(t, block)
+		assert.Equal(t, rpccore.ErrBlockNotFound, rpcErr)
+	})
+
+	t.Run("TransactionsByBlockNumber returns internal error", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+		mockReader := mocks.NewMockReader(mockCtrl)
+		handler := rpcv10.New(mockReader, nil, nil, nil)
+
+		id := rpcv9.BlockIDFromNumber(blockNumber)
+		internalErr := errors.New("some internal error")
+		mockReader.EXPECT().BlockHeaderByNumber(blockNumber).Return(header, nil)
+		mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).Return(nil, internalErr)
+
+		block, rpcErr := handler.BlockWithTxHashes(&id)
+		assert.Nil(t, block)
+		assert.Equal(t, rpccore.ErrInternal.CloneWithData(internalErr), rpcErr)
+	})
+}
+
 //nolint:dupl // Shares similar structure with other tests but tests different method
 func TestBlockWithTxs_ErrorCases(t *testing.T) {
 	errTests := map[string]rpcv9.BlockID{
@@ -634,6 +682,42 @@ func TestBlockWithTxs(t *testing.T) {
 	}
 }
 
+func TestBlockWithTxs_TxnsFetchError(t *testing.T) {
+	blockNumber := uint64(123)
+	header := &core.Header{Number: blockNumber}
+
+	t.Run("TransactionsByBlockNumber returns ErrKeyNotFound", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+		mockReader := mocks.NewMockReader(mockCtrl)
+		handler := rpcv10.New(mockReader, nil, nil, nil)
+
+		id := rpcv9.BlockIDFromNumber(blockNumber)
+		mockReader.EXPECT().BlockHeaderByNumber(blockNumber).Return(header, nil)
+		mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).Return(nil, db.ErrKeyNotFound)
+
+		block, rpcErr := handler.BlockWithTxs(&id, rpcv10.ResponseFlags{})
+		assert.Nil(t, block)
+		assert.Equal(t, rpccore.ErrBlockNotFound, rpcErr)
+	})
+
+	t.Run("TransactionsByBlockNumber returns internal error", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		t.Cleanup(mockCtrl.Finish)
+		mockReader := mocks.NewMockReader(mockCtrl)
+		handler := rpcv10.New(mockReader, nil, nil, nil)
+
+		id := rpcv9.BlockIDFromNumber(blockNumber)
+		internalErr := errors.New("some internal error")
+		mockReader.EXPECT().BlockHeaderByNumber(blockNumber).Return(header, nil)
+		mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).Return(nil, internalErr)
+
+		block, rpcErr := handler.BlockWithTxs(&id, rpcv10.ResponseFlags{})
+		assert.Nil(t, block)
+		assert.Equal(t, rpccore.ErrInternal.CloneWithData(internalErr), rpcErr)
+	})
+}
+
 //nolint:dupl // Shares similar structure with other tests but tests different method
 func TestBlockWithReceipts_ErrorCases(t *testing.T) {
 	errTests := map[string]rpcv9.BlockID{
@@ -763,7 +847,9 @@ func TestRpcBlockAdaptation(t *testing.T) {
 		)
 
 		block.Header.SequencerAddress = nil
-		mockReader.EXPECT().Head().Return(block, nil).Times(2)
+		mockReader.EXPECT().HeadsHeader().Return(block.Header, nil).Times(2)
+		mockReader.EXPECT().TransactionsByBlockNumber(block.Number).Return(
+			block.Transactions, nil).Times(2)
 		mockReader.EXPECT().L1Head().Return(core.L1Head{}, db.ErrKeyNotFound).Times(2)
 		mockReader.EXPECT().BlockCommitmentsByNumber(block.Number).Return(commitments, nil).Times(2)
 		mockReader.EXPECT().StateUpdateByNumber(block.Number).Return(stateUpdate, nil).Times(2)
@@ -789,7 +875,8 @@ func TestBlockWithTxHashesV013(t *testing.T) {
 	t.Cleanup(mockCtrl.Finish)
 	mockReader := mocks.NewMockReader(mockCtrl)
 
-	mockReader.EXPECT().BlockByNumber(gomock.Any()).Return(block, nil)
+	mockReader.EXPECT().BlockHeaderByNumber(blockNumber).Return(block.Header, nil)
+	mockReader.EXPECT().TransactionsByBlockNumber(blockNumber).Return(block.Transactions, nil)
 	mockReader.EXPECT().L1Head().Return(core.L1Head{}, nil)
 	mockReader.EXPECT().BlockCommitmentsByNumber(blockNumber).Return(commitments, nil)
 	mockReader.EXPECT().StateUpdateByNumber(blockNumber).Return(stateUpdate, nil)
@@ -929,7 +1016,10 @@ func TestBlockWithTxsWithResponseFlags(t *testing.T) {
 	mockSyncReader := mocks.NewMockSyncReader(mockCtrl)
 
 	blockID := rpcv9.BlockIDFromNumber(block.Header.Number)
-	mockReader.EXPECT().BlockByNumber(block.Header.Number).Return(block, nil).AnyTimes()
+	mockReader.EXPECT().BlockHeaderByNumber(block.Header.Number).Return(block.Header, nil).AnyTimes()
+	mockReader.EXPECT().TransactionsByBlockNumber(
+		block.Header.Number,
+	).Return(block.Transactions, nil).AnyTimes()
 	mockReader.EXPECT().Network().Return(network).AnyTimes()
 	mockReader.EXPECT().L1Head().Return(core.L1Head{}, nil).AnyTimes()