feat: enable valset update (#1658)

<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.

NOTE: PR titles should follow semantic commits:
https://www.conventionalcommits.org/en/v1.0.0/
-->

## Overview
Re-enable `ABCI update valset`.
Closes #1673
<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 

Ex: Closes #<issue number>
-->


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved handling of validator updates and state management to ensure
consistent and accurate validator information.

- **New Features**
- Enhanced validator and proposer logic for better performance and
reliability.

- **Tests**
- Updated node creation tests to include new parameters for better
simulation of real-world scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Matthew Sevey <[email protected]>

Author: 3 people

block/manager.go

@@ -61,8 +61,8 @@ const blockInChLength = 10000
 var initialBackoff = 100 * time.Millisecond
 
 var (
-	// ErrNoValidatorsInGenesis is used when no validators/proposers are found in genesis state
-	ErrNoValidatorsInGenesis = errors.New("no validators found in genesis")
+	// ErrNoValidatorsInState is used when no validators/proposers are found in state
+	ErrNoValidatorsInState = errors.New("no validators found in state")
 
 	// ErrNotProposer is used when the manager is not a proposer
 	ErrNotProposer = errors.New("not a proposer")
@@ -108,10 +108,6 @@ type Manager struct {
 
 	logger log.Logger
 
-	// Rollkit doesn't have "validators", but
-	// we store the sequencer in this struct for compatibility.
-	validatorSet *cmtypes.ValidatorSet
-
 	// For usage by Lazy Aggregator mode
 	buildingBlock bool
 	txsAvailable  <-chan struct{}
@@ -172,10 +168,6 @@ func NewManager(
 	//set block height in store
 	store.SetHeight(context.Background(), s.LastBlockHeight)
 
-	// genesis should have exactly one "validator", the centralized sequencer.
-	// this should have been validated in the above call to getInitialState.
-	valSet := types.GetValidatorSetFromGenesis(genesis)
-
 	if s.DAHeight < conf.DAStartHeight {
 		s.DAHeight = conf.DAStartHeight
 	}
@@ -205,31 +197,33 @@ func NewManager(
 		return nil, err
 	}
 
-	isProposer, err := isProposer(genesis, proposerKey)
-	if err != nil {
-		return nil, err
-	}
-
 	maxBlobSize, err := dalc.DA.MaxBlobSize(context.Background())
 	if err != nil {
 		return nil, err
 	}
 	// allow buffer for the block header and protocol encoding
 	maxBlobSize -= blockProtocolOverhead
 
-	exec := state.NewBlockExecutor(proposerAddress, genesis.ChainID, mempool, proxyApp, eventBus, maxBlobSize, logger, execMetrics, valSet.Hash())
+	exec := state.NewBlockExecutor(proposerAddress, genesis.ChainID, mempool, proxyApp, eventBus, maxBlobSize, logger, execMetrics)
 	if s.LastBlockHeight+1 == uint64(genesis.InitialHeight) {
 		res, err := exec.InitChain(genesis)
 		if err != nil {
 			return nil, err
 		}
+		if err := updateState(&s, res); err != nil {
+			return nil, err
+		}
 
-		updateState(&s, res)
 		if err := store.UpdateState(context.Background(), s); err != nil {
 			return nil, err
 		}
 	}
 
+	isProposer, err := isProposer(proposerKey, s)
+	if err != nil {
+		return nil, err
+	}
+
 	var txsAvailableCh <-chan struct{}
 	if mempool != nil {
 		txsAvailableCh = mempool.TxsAvailable()
@@ -261,7 +255,6 @@ func NewManager(
 		blockCache:    NewBlockCache(),
 		retrieveCh:    make(chan struct{}, 1),
 		logger:        logger,
-		validatorSet:  &valSet,
 		txsAvailable:  txsAvailableCh,
 		buildingBlock: false,
 		pendingBlocks: pendingBlocks,
@@ -285,15 +278,17 @@ func (m *Manager) SetDALC(dalc *da.DAClient) {
 }
 
 // isProposer returns whether or not the manager is a proposer
-func isProposer(genesis *cmtypes.GenesisDoc, signerPrivKey crypto.PrivKey) (bool, error) {
-	if len(genesis.Validators) == 0 {
-		return false, ErrNoValidatorsInGenesis
+func isProposer(signerPrivKey crypto.PrivKey, s types.State) (bool, error) {
+	if len(s.Validators.Validators) == 0 {
+		return false, ErrNoValidatorsInState
 	}
+
 	signerPubBytes, err := signerPrivKey.GetPublic().Raw()
 	if err != nil {
 		return false, err
 	}
-	return bytes.Equal(genesis.Validators[0].PubKey.Bytes(), signerPubBytes), nil
+
+	return bytes.Equal(s.Validators.Validators[0].PubKey.Bytes(), signerPubBytes), nil
 }
 
 // SetLastState is used to set lastState used by Manager.
@@ -786,7 +781,6 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 	)
 	height := m.store.Height()
 	newHeight := height + 1
-
 	// this is a special case, when first block is produced - there is no previous commit
 	if newHeight == uint64(m.genesis.InitialHeight) {
 		lastCommit = &types.Commit{}
@@ -823,9 +817,6 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 		}
 		m.logger.Debug("block info", "num_tx", len(block.Data.Txs))
 
-		block.SignedHeader.Validators = m.validatorSet
-		block.SignedHeader.ValidatorHash = m.validatorSet.Hash()
-
 		/*
 		   here we set the SignedHeader.DataHash, and SignedHeader.Commit as a hack
 		   to make the block pass ValidateBasic() when it gets called by applyBlock on line 681
@@ -836,6 +827,9 @@ func (m *Manager) publishBlock(ctx context.Context) error {
 			return err
 		}
 
+		block.SignedHeader.Validators = m.getLastStateValidators()
+		block.SignedHeader.ValidatorHash = block.SignedHeader.Validators.Hash()
+
 		commit, err = m.getCommit(block.SignedHeader.Header)
 		if err != nil {
 			return err
@@ -996,7 +990,6 @@ func (m *Manager) recordMetrics(block *types.Block) {
 	m.metrics.BlockSizeBytes.Set(float64(block.Size()))
 	m.metrics.CommittedHeight.Set(float64(block.Height()))
 }
-
 func (m *Manager) submitBlocksToDA(ctx context.Context) error {
 	submittedAllBlocks := false
 	var backoff time.Duration
@@ -1106,6 +1099,12 @@ func (m *Manager) exponentialBackoff(backoff time.Duration) time.Duration {
 	return backoff
 }
 
+func (m *Manager) getLastStateValidators() *cmtypes.ValidatorSet {
+	m.lastStateMtx.RLock()
+	defer m.lastStateMtx.RUnlock()
+	return m.lastState.Validators
+}
+
 // Updates the state stored in manager's store along the manager's lastState
 func (m *Manager) updateState(ctx context.Context, s types.State) error {
 	m.lastStateMtx.Lock()
@@ -1137,7 +1136,7 @@ func (m *Manager) applyBlock(ctx context.Context, block *types.Block) (types.Sta
 	return m.executor.ApplyBlock(ctx, m.lastState, block)
 }
 
-func updateState(s *types.State, res *abci.ResponseInitChain) {
+func updateState(s *types.State, res *abci.ResponseInitChain) error {
 	// If the app did not return an app hash, we keep the one set from the genesis doc in
 	// the state. We don't set appHash since we don't want the genesis doc app hash
 	// recorded in the genesis block. We should probably just remove GenesisDoc.AppHash.
@@ -1169,4 +1168,24 @@ func updateState(s *types.State, res *abci.ResponseInitChain) {
 	// We update the last results hash with the empty hash, to conform with RFC-6962.
 	s.LastResultsHash = merkle.HashFromByteSlices(nil)
 
+	vals, err := cmtypes.PB2TM.ValidatorUpdates(res.Validators)
+	if err != nil {
+		return err
+	}
+
+	// apply initchain valset change
+	nValSet := s.Validators.Copy()
+	err = nValSet.UpdateWithChangeSet(vals)
+	if err != nil {
+		return err
+	}
+	if len(nValSet.Validators) != 1 {
+		return fmt.Errorf("expected exactly one validator")
+	}
+
+	s.Validators = cmtypes.NewValidatorSet(nValSet.Validators)
+	s.NextValidators = cmtypes.NewValidatorSet(nValSet.Validators).CopyIncrementProposerPriority(1)
+	s.LastValidators = cmtypes.NewValidatorSet(nValSet.Validators)
+
+	return nil
 }

block/manager_test.go

@@ -72,6 +72,7 @@ func TestInitialStateClean(t *testing.T) {
 func TestInitialStateStored(t *testing.T) {
 	require := require.New(t)
 	genesisDoc, _ := types.GetGenesisWithPrivkey()
+	valset := types.GetRandomValidatorSet()
 	genesis := &cmtypes.GenesisDoc{
 		ChainID:       "myChain",
 		InitialHeight: 1,
@@ -82,7 +83,11 @@ func TestInitialStateStored(t *testing.T) {
 		ChainID:         "myChain",
 		InitialHeight:   1,
 		LastBlockHeight: 100,
+		Validators:      valset,
+		NextValidators:  valset,
+		LastValidators:  valset,
 	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	es, _ := store.NewDefaultInMemoryKVStore()
@@ -97,6 +102,7 @@ func TestInitialStateStored(t *testing.T) {
 
 func TestInitialStateUnexpectedHigherGenesis(t *testing.T) {
 	require := require.New(t)
+	valset := types.GetRandomValidatorSet()
 	genesisDoc, _ := types.GetGenesisWithPrivkey()
 	genesis := &cmtypes.GenesisDoc{
 		ChainID:       "myChain",
@@ -108,6 +114,9 @@ func TestInitialStateUnexpectedHigherGenesis(t *testing.T) {
 		ChainID:         "myChain",
 		InitialHeight:   1,
 		LastBlockHeight: 0,
+		Validators:      valset,
+		NextValidators:  valset,
+		LastValidators:  valset,
 	}
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -405,7 +414,7 @@ func Test_isProposer(t *testing.T) {
 	require := require.New(t)
 
 	type args struct {
-		genesis       *cmtypes.GenesisDoc
+		state         types.State
 		signerPrivKey crypto.PrivKey
 	}
 	tests := []struct {
@@ -418,10 +427,12 @@ func Test_isProposer(t *testing.T) {
 			name: "Signing key matches genesis proposer public key",
 			args: func() args {
 				genesisData, privKey := types.GetGenesisWithPrivkey()
+				s, err := types.NewFromGenesisDoc(genesisData)
+				require.NoError(err)
 				signingKey, err := types.PrivKeyToSigningKey(privKey)
 				require.NoError(err)
 				return args{
-					genesisData,
+					s,
 					signingKey,
 				}
 			}(),
@@ -432,11 +443,14 @@ func Test_isProposer(t *testing.T) {
 			name: "Signing key does not match genesis proposer public key",
 			args: func() args {
 				genesisData, _ := types.GetGenesisWithPrivkey()
+				s, err := types.NewFromGenesisDoc(genesisData)
+				require.NoError(err)
+
 				randomPrivKey := ed25519.GenPrivKey()
 				signingKey, err := types.PrivKeyToSigningKey(randomPrivKey)
 				require.NoError(err)
 				return args{
-					genesisData,
+					s,
 					signingKey,
 				}
 			}(),
@@ -448,20 +462,23 @@ func Test_isProposer(t *testing.T) {
 			args: func() args {
 				genesisData, privKey := types.GetGenesisWithPrivkey()
 				genesisData.Validators = nil
+				s, err := types.NewFromGenesisDoc(genesisData)
+				require.NoError(err)
+
 				signingKey, err := types.PrivKeyToSigningKey(privKey)
 				require.NoError(err)
 				return args{
-					genesisData,
+					s,
 					signingKey,
 				}
 			}(),
 			isProposer: false,
-			err:        ErrNoValidatorsInGenesis,
+			err:        ErrNoValidatorsInState,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			isProposer, err := isProposer(tt.args.genesis, tt.args.signerPrivKey)
+			isProposer, err := isProposer(tt.args.signerPrivKey, tt.args.state)
 			if !errors.Is(err, tt.err) {
 				t.Errorf("isProposer() error = %v, expected err %v", err, tt.err)
 				return

node/full_client_test.go

@@ -899,7 +899,13 @@ func TestStatus(t *testing.T) {
 	require.NotNil(node)
 	ctx := context.Background()
 
-	err = node.Store.UpdateState(ctx, types.State{})
+	validators := make([]*cmtypes.Validator, len(genesisDoc.Validators))
+	for i, val := range genesisDoc.Validators {
+		validators[i] = cmtypes.NewValidator(val.PubKey, val.Power)
+	}
+	validatorSet := cmtypes.NewValidatorSet(validators)
+
+	err = node.Store.UpdateState(ctx, types.State{LastValidators: validatorSet, NextValidators: validatorSet, Validators: validatorSet})
 	assert.NoError(err)
 
 	rpc := NewFullClient(node)