Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions block/internal/executing/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func NewExecutor(
return nil, fmt.Errorf("failed to get address: %w", err)
}

if !bytes.Equal(addr, genesis.ProposerAddress) {
if !genesis.HasScheduledProposer(addr) {
return nil, common.ErrNotProposer
}
}
Expand Down Expand Up @@ -696,6 +696,10 @@ func (e *Executor) RetrieveBatch(ctx context.Context) (*BatchData, error) {
func (e *Executor) CreateBlock(ctx context.Context, height uint64, batchData *BatchData) (*types.SignedHeader, *types.Data, error) {
currentState := e.getLastState()
headerTime := uint64(e.genesis.StartTime.UnixNano())
proposer, err := e.genesis.ProposerAtHeight(height)
if err != nil {
return nil, nil, fmt.Errorf("resolve proposer for height %d: %w", height, err)
}

var lastHeaderHash types.Hash
var lastDataHash types.Hash
Expand Down Expand Up @@ -728,22 +732,30 @@ func (e *Executor) CreateBlock(ctx context.Context, height uint64, batchData *Ba

// Get signer info and validator hash
var pubKey crypto.PubKey
var signerAddress []byte
var validatorHash types.Hash

if e.signer != nil {
var err error
pubKey, err = e.signer.GetPublic()
if err != nil {
return nil, nil, fmt.Errorf("failed to get public key: %w", err)
}

validatorHash, err = e.options.ValidatorHasherProvider(e.genesis.ProposerAddress, pubKey)
signerAddress, err = e.signer.GetAddress()
if err != nil {
return nil, nil, fmt.Errorf("failed to get signer address: %w", err)
}

if err := e.genesis.ValidateProposer(height, signerAddress, pubKey); err != nil {
return nil, nil, fmt.Errorf("signer does not match proposer schedule: %w", err)
}

validatorHash, err = e.options.ValidatorHasherProvider(proposer.Address, pubKey)
if err != nil {
return nil, nil, fmt.Errorf("failed to get validator hash: %w", err)
}
} else {
var err error
validatorHash, err = e.options.ValidatorHasherProvider(e.genesis.ProposerAddress, nil)
validatorHash, err = e.options.ValidatorHasherProvider(proposer.Address, nil)
if err != nil {
return nil, nil, fmt.Errorf("failed to get validator hash: %w", err)
}
Expand All @@ -763,13 +775,13 @@ func (e *Executor) CreateBlock(ctx context.Context, height uint64, batchData *Ba
},
LastHeaderHash: lastHeaderHash,
AppHash: currentState.AppHash,
ProposerAddress: e.genesis.ProposerAddress,
ProposerAddress: proposer.Address,
ValidatorHash: validatorHash,
},
Signature: lastSignature,
Signer: types.Signer{
PubKey: pubKey,
Address: e.genesis.ProposerAddress,
Address: proposer.Address,
},
}

Expand Down
221 changes: 221 additions & 0 deletions block/internal/executing/executor_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package executing

import (
"context"
"testing"
"time"

Expand All @@ -12,6 +13,7 @@ import (

"github.com/evstack/ev-node/block/internal/cache"
"github.com/evstack/ev-node/block/internal/common"
coreseq "github.com/evstack/ev-node/core/sequencer"
"github.com/evstack/ev-node/pkg/config"
"github.com/evstack/ev-node/pkg/genesis"
"github.com/evstack/ev-node/pkg/store"
Expand Down Expand Up @@ -121,3 +123,222 @@ func TestExecutor_NilBroadcasters(t *testing.T) {
assert.Equal(t, cacheManager, executor.cache)
assert.Equal(t, gen, executor.genesis)
}

func TestExecutor_CreateBlock_UsesScheduledProposerForHeight(t *testing.T) {
ds := sync.MutexWrap(datastore.NewMapDatastore())
memStore := store.New(ds)

cacheManager, err := cache.NewManager(config.DefaultConfig(), memStore, zerolog.Nop())
require.NoError(t, err)

metrics := common.NopMetrics()
oldAddr, oldSignerInfo, _ := buildTestSigner(t)
newAddr, newSignerInfo, newSigner := buildTestSigner(t)

entry1, err := genesis.NewProposerScheduleEntry(1, oldSignerInfo.PubKey)
require.NoError(t, err)
entry2, err := genesis.NewProposerScheduleEntry(2, newSignerInfo.PubKey)
require.NoError(t, err)

gen := genesis.Genesis{
ChainID: "test-chain",
InitialHeight: 1,
StartTime: time.Now().Add(-time.Second),
ProposerAddress: entry1.Address,
ProposerSchedule: []genesis.ProposerScheduleEntry{entry1, entry2},
DAEpochForcedInclusion: 1,
}

executor, err := NewExecutor(
memStore,
nil,
nil,
newSigner,
cacheManager,
metrics,
config.DefaultConfig(),
gen,
nil,
nil,
zerolog.Nop(),
common.DefaultBlockOptions(),
make(chan error, 1),
nil,
)
require.NoError(t, err)

prevHeader := &types.SignedHeader{
Header: types.Header{
Version: types.InitStateVersion,
BaseHeader: types.BaseHeader{
ChainID: gen.ChainID,
Height: 1,
Time: uint64(gen.StartTime.UnixNano()),
},
AppHash: []byte("state-root-0"),
ProposerAddress: oldAddr,
DataHash: common.DataHashForEmptyTxs,
},
Signature: types.Signature([]byte("sig-1")),
Signer: oldSignerInfo,
}
prevData := &types.Data{
Metadata: &types.Metadata{
ChainID: gen.ChainID,
Height: 1,
Time: prevHeader.BaseHeader.Time,
},
Txs: nil,
}

batch, err := memStore.NewBatch(context.Background())
require.NoError(t, err)
require.NoError(t, batch.SaveBlockData(prevHeader, prevData, &prevHeader.Signature))
require.NoError(t, batch.SetHeight(1))
require.NoError(t, batch.Commit())

executor.setLastState(types.State{
Version: types.InitStateVersion,
ChainID: gen.ChainID,
InitialHeight: gen.InitialHeight,
LastBlockHeight: 1,
LastBlockTime: prevHeader.Time(),
LastHeaderHash: prevHeader.Hash(),
AppHash: []byte("state-root-1"),
})

header, data, err := executor.CreateBlock(context.Background(), 2, &BatchData{
Batch: &coreseq.Batch{},
Time: time.Now(),
})
require.NoError(t, err)
require.Equal(t, newAddr, header.ProposerAddress)
require.Equal(t, newAddr, header.Signer.Address)
require.Equal(t, uint64(2), data.Height())
}
Comment on lines +143 to +218
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use fixed times in the new executor fixtures.

These tests only need ordered/non-zero times, so wall-clock values make the fixtures less reproducible.

🧪 Proposed direction
+	fixedStartTime := time.Unix(1_700_000_000, 0).UTC()
+	fixedBatchTime := fixedStartTime.Add(time.Second)

 	gen := genesis.Genesis{
 		ChainID:                "test-chain",
 		InitialHeight:          1,
-		StartTime:              time.Now().Add(-time.Second),
+		StartTime:              fixedStartTime,
 		ProposerAddress:        entry1.Address,
 		ProposerSchedule:       []genesis.ProposerScheduleEntry{entry1, entry2},
 		DAEpochForcedInclusion: 1,
 	}
@@
 	header, data, err := executor.CreateBlock(context.Background(), 2, &BatchData{
 		Batch: &coreseq.Batch{},
-		Time:  time.Now(),
+		Time:  fixedBatchTime,
 	})

Apply the same pattern to the other new genesis and BatchData fixtures in this file.

As per coding guidelines, Ensure tests are deterministic.

Also applies to: 237-343

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@block/internal/executing/executor_test.go` around lines 143 - 218, Tests use
wall-clock times (time.Now()) making fixtures non-deterministic; replace those
with a fixed timestamp constant (e.g., a single time variable declared in the
test like fixedTime := time.Unix(0, 0).UTC()) and use that for
genesis.StartTime, prevHeader/BaseHeader.Time, and BatchData.Time when
constructing genesis.Genesis, prevHeader, and the CreateBlock call; apply the
same fixed-time pattern to the other new genesis and BatchData fixtures in this
file so NewExecutor, setLastState, CreateBlock, BatchData and prevHeader use
deterministic times.


// TestNewExecutor_RejectsSignerOutsideSchedule verifies that a signer whose
// address does not appear anywhere in the proposer schedule cannot start the
// executor. This prevents a misconfigured replacement key from coming up as
// an aggregator on a chain it was never scheduled on.
func TestNewExecutor_RejectsSignerOutsideSchedule(t *testing.T) {
ds := sync.MutexWrap(datastore.NewMapDatastore())
memStore := store.New(ds)

cacheManager, err := cache.NewManager(config.DefaultConfig(), memStore, zerolog.Nop())
require.NoError(t, err)

_, scheduledSigner, _ := buildTestSigner(t)
_, _, strayerSigner := buildTestSigner(t)

entry, err := genesis.NewProposerScheduleEntry(1, scheduledSigner.PubKey)
require.NoError(t, err)

gen := genesis.Genesis{
ChainID: "test-chain",
InitialHeight: 1,
StartTime: time.Now(),
ProposerAddress: entry.Address,
ProposerSchedule: []genesis.ProposerScheduleEntry{entry},
DAEpochForcedInclusion: 1,
}

_, err = NewExecutor(
memStore, nil, nil, strayerSigner, cacheManager,
common.NopMetrics(), config.DefaultConfig(), gen,
nil, nil, zerolog.Nop(), common.DefaultBlockOptions(),
make(chan error, 1), nil,
)
require.ErrorIs(t, err, common.ErrNotProposer)
}

// TestExecutor_CreateBlock_RejectsSignerAtWrongHeight verifies that a signer
// which is scheduled (so startup succeeds) but not active at the current
// height cannot produce a block. This guards the per-height proposer check
// inside CreateBlock — without it, a rotation could be jumped ahead or
// rolled back by whichever signer the operator happens to start.
func TestExecutor_CreateBlock_RejectsSignerAtWrongHeight(t *testing.T) {
ds := sync.MutexWrap(datastore.NewMapDatastore())
memStore := store.New(ds)

cacheManager, err := cache.NewManager(config.DefaultConfig(), memStore, zerolog.Nop())
require.NoError(t, err)

oldAddr, oldSignerInfo, oldSigner := buildTestSigner(t)
_, newSignerInfo, _ := buildTestSigner(t)

entry1, err := genesis.NewProposerScheduleEntry(1, oldSignerInfo.PubKey)
require.NoError(t, err)
// Second entry activates at height 5. The old signer is scheduled at
// height 1 and is NOT the proposer for height 5+.
entry2, err := genesis.NewProposerScheduleEntry(5, newSignerInfo.PubKey)
require.NoError(t, err)

gen := genesis.Genesis{
ChainID: "test-chain",
InitialHeight: 1,
StartTime: time.Now().Add(-time.Second),
ProposerAddress: entry1.Address,
ProposerSchedule: []genesis.ProposerScheduleEntry{entry1, entry2},
DAEpochForcedInclusion: 1,
}

// Start the executor as the old signer — it IS in the schedule at
// height 1, so NewExecutor must accept it.
executor, err := NewExecutor(
memStore, nil, nil, oldSigner, cacheManager,
common.NopMetrics(), config.DefaultConfig(), gen,
nil, nil, zerolog.Nop(), common.DefaultBlockOptions(),
make(chan error, 1), nil,
)
require.NoError(t, err)

// Seed a height-4 block so CreateBlock(5) has a parent to reference.
prevHeader := &types.SignedHeader{
Header: types.Header{
Version: types.InitStateVersion,
BaseHeader: types.BaseHeader{
ChainID: gen.ChainID,
Height: 4,
Time: uint64(gen.StartTime.UnixNano()),
},
AppHash: []byte("state-root-4"),
ProposerAddress: oldAddr,
DataHash: common.DataHashForEmptyTxs,
},
Signature: types.Signature([]byte("sig-4")),
Signer: oldSignerInfo,
}
prevData := &types.Data{
Metadata: &types.Metadata{
ChainID: gen.ChainID,
Height: 4,
Time: prevHeader.BaseHeader.Time,
},
}

batch, err := memStore.NewBatch(context.Background())
require.NoError(t, err)
require.NoError(t, batch.SaveBlockData(prevHeader, prevData, &prevHeader.Signature))
require.NoError(t, batch.SetHeight(4))
require.NoError(t, batch.Commit())

executor.setLastState(types.State{
Version: types.InitStateVersion,
ChainID: gen.ChainID,
InitialHeight: gen.InitialHeight,
LastBlockHeight: 4,
LastBlockTime: prevHeader.Time(),
LastHeaderHash: prevHeader.Hash(),
AppHash: []byte("state-root-4"),
})

// Height 5 belongs to the NEW signer per the schedule — the old
// signer must be rejected even though it's a known schedule member.
_, _, err = executor.CreateBlock(context.Background(), 5, &BatchData{
Batch: &coreseq.Batch{},
Time: time.Now(),
})
require.Error(t, err)
require.Contains(t, err.Error(), "proposer")
}
9 changes: 4 additions & 5 deletions block/internal/submitting/da_submitter.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package submitting

import (
"bytes"
"context"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -476,10 +475,6 @@ func (s *DASubmitter) signData(ctx context.Context, unsignedDataList []*types.Si
return nil, nil, fmt.Errorf("failed to get address: %w", err)
}

if len(genesis.ProposerAddress) > 0 && !bytes.Equal(addr, genesis.ProposerAddress) {
return nil, nil, fmt.Errorf("signer address mismatch with genesis proposer")
}

signerInfo := types.Signer{
PubKey: pubKey,
Address: addr,
Expand All @@ -494,6 +489,10 @@ func (s *DASubmitter) signData(ctx context.Context, unsignedDataList []*types.Si
continue
}

if err := genesis.ValidateProposer(unsignedData.Height(), addr, pubKey); err != nil {
return nil, nil, fmt.Errorf("signer does not match proposer schedule for data at height %d: %w", unsignedData.Height(), err)
}

signature, err := signer.Sign(ctx, unsignedDataListBz[i])
if err != nil {
return nil, nil, fmt.Errorf("failed to sign data: %w", err)
Expand Down
Loading
Loading