Add block number to pending block (#1655)

Co-authored-by: Ömer Faruk IRMAK <[email protected]>

blockchain/blockchain.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"sync/atomic"
+	"time"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/NethermindEth/juno/core"
@@ -930,7 +931,8 @@ func (b *Blockchain) storeEmptyPending(txn db.Transaction, latestHeader *core.He
 		Header: &core.Header{
 			ParentHash:       latestHeader.Hash,
 			SequencerAddress: latestHeader.SequencerAddress,
-			Timestamp:        latestHeader.Timestamp + 1,
+			Number:           latestHeader.Number + 1,
+			Timestamp:        uint64(time.Now().Unix()),
 			ProtocolVersion:  latestHeader.ProtocolVersion,
 			EventsBloom:      core.EventsBloom(receipts),
 			GasPrice:         latestHeader.GasPrice,
@@ -971,9 +973,13 @@ func (b *Blockchain) StorePending(pending *Pending) error {
 			return ErrParentDoesNotMatchHead
 		}
 
-		existingPending, err := b.pendingBlock(txn)
-		if err == nil && existingPending.Block.TransactionCount >= pending.Block.TransactionCount {
-			return nil // ignore the incoming pending if it has fewer transactions than the one we already have
+		if existingPending, err := b.pendingBlock(txn); err == nil {
+			if existingPending.Block.TransactionCount >= pending.Block.TransactionCount {
+				return nil // ignore the incoming pending if it has fewer transactions than the one we already have
+			}
+			pending.Block.Number = existingPending.Block.Number // Just in case the number is not set.
+		} else if !errors.Is(err, db.ErrKeyNotFound) { // Allow StorePending before block zero.
+			return err
 		}
 
 		return b.storePending(txn, pending)

blockchain/blockchain_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/NethermindEth/juno/blockchain"
 	"github.com/NethermindEth/juno/clients/feeder"
@@ -665,19 +666,31 @@ func TestPending(t *testing.T) {
 	t.Run("no pending block means pending state matches head state", func(t *testing.T) {
 		pending, pErr := chain.Pending()
 		require.NoError(t, pErr)
-		require.Equal(t, b.Timestamp+1, pending.Block.Timestamp)
-		require.Equal(t, b.SequencerAddress, pending.Block.SequencerAddress)
-		require.Equal(t, b.GasPrice, pending.Block.GasPrice)
-		require.Equal(t, b.GasPriceSTRK, pending.Block.GasPriceSTRK)
-		require.Equal(t, b.ProtocolVersion, pending.Block.ProtocolVersion)
-		require.Equal(t, su.NewRoot, pending.StateUpdate.OldRoot)
-		require.Empty(t, pending.StateUpdate.StateDiff.Nonces)
-		require.Empty(t, pending.StateUpdate.StateDiff.StorageDiffs)
-		require.Empty(t, pending.StateUpdate.StateDiff.ReplacedClasses)
-		require.Empty(t, pending.StateUpdate.StateDiff.DeclaredV0Classes)
-		require.Empty(t, pending.StateUpdate.StateDiff.DeclaredV1Classes)
-		require.Empty(t, pending.StateUpdate.StateDiff.DeployedContracts)
-		require.Empty(t, pending.NewClasses)
+
+		require.LessOrEqual(t, pending.Block.Timestamp, uint64(time.Now().Unix()))
+		require.GreaterOrEqual(t, pending.Block.Timestamp, b.Timestamp)
+		receipts := make([]*core.TransactionReceipt, 0)
+		require.Equal(t, blockchain.Pending{
+			Block: &core.Block{
+				Header: &core.Header{
+					ParentHash:       b.Hash,
+					SequencerAddress: b.SequencerAddress,
+					Number:           b.Number + 1,
+					Timestamp:        pending.Block.Timestamp, // Tested above.
+					ProtocolVersion:  b.ProtocolVersion,
+					EventsBloom:      core.EventsBloom(receipts),
+					GasPrice:         b.GasPrice,
+					GasPriceSTRK:     b.GasPriceSTRK,
+				},
+				Transactions: make([]core.Transaction, 0),
+				Receipts:     receipts,
+			},
+			StateUpdate: &core.StateUpdate{
+				OldRoot:   su.NewRoot,
+				StateDiff: core.EmptyStateDiff(),
+			},
+			NewClasses: make(map[felt.Felt]core.Class, 0),
+		}, pending)
 
 		// PendingState matches head state.
 		require.NoError(t, pErr)
@@ -760,6 +773,45 @@ func TestPending(t *testing.T) {
 	})
 }
 
+func TestStorePendingIncludesNumber(t *testing.T) {
+	network := utils.Mainnet
+	chain := blockchain.New(pebble.NewMemTest(t), network)
+
+	// Store pending genesis.
+	require.NoError(t, chain.StorePending(&blockchain.Pending{
+		Block: &core.Block{
+			Header: &core.Header{
+				ParentHash: new(felt.Felt),
+				Hash:       new(felt.Felt),
+			},
+		},
+	}))
+	pending, err := chain.Pending()
+	require.NoError(t, err)
+	require.Equal(t, uint64(0), pending.Block.Number)
+
+	// Add block zero.
+	gw := adaptfeeder.New(feeder.NewTestClient(t, network))
+	b, err := gw.BlockByNumber(context.Background(), 0)
+	require.NoError(t, err)
+	su, err := gw.StateUpdate(context.Background(), 0)
+	require.NoError(t, err)
+	require.NoError(t, chain.Store(b, nil, su, nil))
+
+	// Store pending.
+	require.NoError(t, chain.StorePending(&blockchain.Pending{
+		Block: &core.Block{
+			Header: &core.Header{
+				ParentHash: b.Hash,
+				Hash:       new(felt.Felt),
+			},
+		},
+	}))
+	pending, err = chain.Pending()
+	require.NoError(t, err)
+	require.Equal(t, uint64(1), pending.Block.Number)
+}
+
 func TestMakeStateDiffForEmptyBlock(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	t.Cleanup(mockCtrl.Finish)

rpc/handlers.go

@@ -1229,17 +1229,8 @@ func (h *Handler) Call(call FunctionCall, id BlockID) ([]*felt.Felt, *jsonrpc.Er
 		return nil, ErrContractNotFound
 	}
 
-	blockNumber := header.Number
-	if id.Pending {
-		height, hErr := h.bcReader.Height()
-		if hErr != nil {
-			return nil, ErrBlockNotFound
-		}
-		blockNumber = height + 1
-	}
-
 	res, err := h.vm.Call(&call.ContractAddress, classHash, &call.EntryPointSelector,
-		call.Calldata, blockNumber, header.Timestamp, state, h.bcReader.Network())
+		call.Calldata, header.Number, header.Timestamp, state, h.bcReader.Network())
 	if err != nil {
 		if errors.Is(err, utils.ErrResourceBusy) {
 			return nil, ErrInternal.CloneWithData(err.Error())
@@ -1441,7 +1432,7 @@ func (h *Handler) LegacySimulateTransactions(id BlockID, transactions []Broadcas
 	return res, err
 }
 
-func (h *Handler) simulateTransactions(id BlockID, transactions []BroadcastedTransaction, //nolint: gocyclo
+func (h *Handler) simulateTransactions(id BlockID, transactions []BroadcastedTransaction,
 	simulationFlags []SimulationFlag, legacyTraceJSON, errOnRevert bool,
 ) ([]SimulatedTransaction, *jsonrpc.Error) {
 	skipFeeCharge := slices.Contains(simulationFlags, SkipFeeChargeFlag)
@@ -1478,20 +1469,11 @@ func (h *Handler) simulateTransactions(id BlockID, transactions []BroadcastedTra
 		}
 	}
 
-	blockNumber := header.Number
-	if id.Pending {
-		height, hErr := h.bcReader.Height()
-		if hErr != nil {
-			return nil, ErrBlockNotFound
-		}
-		blockNumber = height + 1
-	}
-
 	sequencerAddress := header.SequencerAddress
 	if sequencerAddress == nil {
 		sequencerAddress = core.NetworkBlockHashMetaInfo(h.bcReader.Network()).FallBackSequencerAddress
 	}
-	overallFees, traces, err := h.vm.Execute(txns, classes, blockNumber, header.Timestamp, sequencerAddress,
+	overallFees, traces, err := h.vm.Execute(txns, classes, header.Number, header.Timestamp, sequencerAddress,
 		state, h.bcReader.Network(), paidFeesOnL1, skipFeeCharge, skipValidate, errOnRevert, header.GasPrice,
 		header.GasPriceSTRK, legacyTraceJSON)
 	if err != nil {
@@ -1593,16 +1575,7 @@ func (h *Handler) traceBlockTransactions(ctx context.Context, block *core.Block,
 	}
 	defer h.callAndLogErr(closer, "Failed to close state in traceBlockTransactions")
 
-	blockNumber := block.Number
-	if isPending {
-		height, hErr := h.bcReader.Height()
-		if hErr != nil {
-			return nil, ErrBlockNotFound
-		}
-		blockNumber = height + 1
-	}
-
-	if state, err = prependBlockHashToState(h.bcReader, blockNumber, state); err != nil {
+	if state, err = prependBlockHashToState(h.bcReader, block.Number, state); err != nil {
 		return nil, jsonrpc.Err(jsonrpc.InternalError, err.Error())
 	}
 
@@ -1642,7 +1615,7 @@ func (h *Handler) traceBlockTransactions(ctx context.Context, block *core.Block,
 		sequencerAddress = core.NetworkBlockHashMetaInfo(h.bcReader.Network()).FallBackSequencerAddress
 	}
 
-	_, traces, err := h.vm.Execute(block.Transactions, classes, blockNumber, block.Header.Timestamp,
+	_, traces, err := h.vm.Execute(block.Transactions, classes, block.Number, block.Header.Timestamp,
 		sequencerAddress, state, h.bcReader.Network(), paidFeesOnL1, false, false, false, block.Header.GasPrice,
 		block.Header.GasPriceSTRK, legacyJSON)
 	if err != nil {

rpc/handlers_test.go

@@ -3336,8 +3336,6 @@ func TestTraceBlockTransactions(t *testing.T) {
 		headState := mocks.NewMockStateHistoryReader(mockCtrl)
 		headState.EXPECT().Class(declareTx.ClassHash).Return(declaredClass, nil)
 		mockReader.EXPECT().PendingState().Return(headState, nopCloser, nil)
-		const height uint64 = 8
-		mockReader.EXPECT().Height().Return(height, nil)
 
 		sequencerAddress := core.NetworkBlockHashMetaInfo(network).FallBackSequencerAddress
 		paidL1Fees := []*felt.Felt{(&felt.Felt{}).SetUint64(1)}
@@ -3356,7 +3354,7 @@ func TestTraceBlockTransactions(t *testing.T) {
 		}`)
 		vmTrace := vm.TransactionTrace{}
 		require.NoError(t, json.Unmarshal(vmTraceJSON, &vmTrace))
-		mockVM.EXPECT().Execute(block.Transactions, []core.Class{declaredClass.Class}, height+1, header.Timestamp, sequencerAddress,
+		mockVM.EXPECT().Execute(block.Transactions, []core.Class{declaredClass.Class}, header.Number, header.Timestamp, sequencerAddress,
 			gomock.Any(), network, paidL1Fees, false, false, false, header.GasPrice, header.GasPriceSTRK, false).Return(nil, []vm.TransactionTrace{vmTrace, vmTrace}, nil)
 
 		result, err := handler.TraceBlockTransactions(context.Background(), rpc.BlockID{Hash: blockHash})
@@ -3754,8 +3752,6 @@ func TestThrottledVMError(t *testing.T) {
 		headState := mocks.NewMockStateHistoryReader(mockCtrl)
 		headState.EXPECT().Class(declareTx.ClassHash).Return(declaredClass, nil)
 		mockReader.EXPECT().PendingState().Return(headState, nopCloser, nil)
-		const height uint64 = 8
-		mockReader.EXPECT().Height().Return(height, nil)
 		_, rpcErr := handler.TraceBlockTransactions(context.Background(), rpc.BlockID{Hash: blockHash})
 		assert.Equal(t, utils.ErrResourceBusy.Error(), rpcErr.Data)
 	})