comments and amendments

Author: tac0turtle

block/internal/executing/executor_test.go

@@ -216,3 +216,129 @@ func TestExecutor_CreateBlock_UsesScheduledProposerForHeight(t *testing.T) {
 	require.Equal(t, newAddr, header.Signer.Address)
 	require.Equal(t, uint64(2), data.Height())
 }
+
+// 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")
+}

block/internal/syncing/p2p_handler_test.go

@@ -253,6 +253,44 @@ func TestP2PHandler_ProcessHeight_AllowsScheduledProposerRotation(t *testing.T)
 	require.Equal(t, nextAddr, events[0].Header.ProposerAddress)
 }
 
+// TestP2PHandler_ProcessHeight_RejectsScheduledProposerBeforeActivation verifies
+// the counterpart to the rotation-allows test: a signer that IS in the schedule
+// but only active at a later height must not be accepted for blocks before the
+// activation height. Without the per-height check, any scheduled signer could
+// forge blocks outside their active window.
+func TestP2PHandler_ProcessHeight_RejectsScheduledProposerBeforeActivation(t *testing.T) {
+	p := setupP2P(t)
+	ctx := context.Background()
+
+	nextAddr, nextPub, nextSigner := buildTestSigner(t)
+
+	entry1, err := genesis.NewProposerScheduleEntry(p.Genesis.InitialHeight, p.ProposerPub)
+	require.NoError(t, err)
+	entry2, err := genesis.NewProposerScheduleEntry(11, nextPub)
+	require.NoError(t, err)
+
+	p.Genesis.ProposerAddress = entry1.Address
+	p.Genesis.ProposerSchedule = []genesis.ProposerScheduleEntry{entry1, entry2}
+	p.Genesis.DAEpochForcedInclusion = 1
+	require.NoError(t, p.Genesis.Validate())
+	p.Handler.genesis = p.Genesis
+
+	// entry2 is scheduled but only active at height 11. Height 10 still
+	// belongs to entry1, so a header from the next signer at height 10
+	// must be rejected.
+	header := p2pMakeSignedHeader(t, p.Genesis.ChainID, 10, nextAddr, nextPub, nextSigner)
+	header.DataHash = common.DataHashForEmptyTxs
+
+	p.HeaderStore.EXPECT().GetByHeight(mock.Anything, uint64(10)).Return(header, nil).Once()
+
+	ch := make(chan common.DAHeightEvent, 1)
+	err = p.Handler.ProcessHeight(ctx, 10, ch)
+	require.Error(t, err)
+
+	require.Empty(t, collectEvents(t, ch, 50*time.Millisecond))
+	p.DataStore.AssertNotCalled(t, "GetByHeight", mock.Anything, uint64(10))
+}
+
 func TestP2PHandler_ProcessedHeightSkipsPreviouslyHandledBlocks(t *testing.T) {
 	p := setupP2P(t)
 	ctx := t.Context()

docs/adr/adr-023-proposer-key-rotation.md

@@ -27,18 +27,32 @@ and validation become ambiguous.
 
 ### 2. Re-issue a new genesis on each rotation
 
-This treats every proposer rotation like a chain restart. It is operationally heavy, conflates upgrades with
-rotations, and breaks continuity for nodes syncing historical data.
+This treats every proposer rotation like a chain restart: a new `chain_id`, state reset back to `initial_height`,
+and existing block history discarded. It is operationally heavy, conflates upgrades with rotations, and breaks
+continuity for nodes syncing historical data.
 
 ### 3. Height-indexed proposer schedule in genesis (Chosen)
 
-Record proposer changes as an ordered schedule indexed by activation height. This preserves chain continuity while
-making rotation rules explicit and replayable from genesis.
+Record proposer changes as an ordered schedule indexed by activation height. The `genesis.json` file is updated
+with a new schedule entry and redistributed, but the chain keeps its `chain_id`, continues from the current
+height, preserves all block history, and fresh nodes can still validate the entire chain end-to-end across
+rotation boundaries. The rollout is still coordinated — every node must receive the updated `genesis.json` and
+restart before the activation height — but none of the chain's state or provenance is reset.
 
 ## Decision
 
 ev-node now supports proposer rotation through a `proposer_schedule` field in genesis.
 
+### What this is not
+
+This is **not** a re-genesis. Re-genesis — in the sense we mean it above — would involve issuing a new `chain_id`,
+resetting height to `initial_height`, and discarding existing block history. Proposer key rotation does none of
+that: the `chain_id` is unchanged, block height keeps progressing, all previous blocks remain valid, and fresh
+nodes can sync the chain from genesis across any number of rotation boundaries.
+
+The `genesis.json` file itself is updated (a new `proposer_schedule` entry is appended) and operators must
+restart every node to reload it. The file changes; the chain's state does not.
+
 Each entry declares:
 
 - `start_height`
@@ -137,7 +151,7 @@ Implemented
 
 - proposer schedule changes are consensus-visible and require coordinated rollout
 - operators must distribute updated genesis/config before activation height
-- emergency rotation still requires preplanned scheduling or a later authority-based mechanism
+- emergency rotation still requires prior scheduling or a later authority-based mechanism
 
 ### Neutral