fix(state): handle ValidatorSetUpdate with no validator changes (#774)

* fix(state): correctly update validators on

* doc(abci): describe ValidatorSetUpdate message

internal/state/current_round_state.go

@@ -4,11 +4,12 @@ import (
 	"bytes"
 	"fmt"
 
+	"github.com/rs/zerolog"
+
 	abci "github.com/dashpay/tenderdash/abci/types"
 	tmbytes "github.com/dashpay/tenderdash/libs/bytes"
 	tmtypes "github.com/dashpay/tenderdash/proto/tendermint/types"
 	"github.com/dashpay/tenderdash/types"
-	"github.com/rs/zerolog"
 )
 
 const (
@@ -341,6 +342,16 @@ func valsetUpdate(
 			nValSet = types.NewValidatorSetWithLocalNodeProTxHash(validatorUpdates, thresholdPubKey,
 				currentVals.QuorumType, quorumHash, nodeProTxHash)
 		}
+	} else {
+		// validators not changed, but we might have a new quorum hash or threshold public key
+		if !quorumHash.IsZero() {
+			nValSet.QuorumHash = quorumHash
+		}
+
+		if thresholdPubKey != nil {
+			nValSet.ThresholdPublicKey = thresholdPubKey
+		}
 	}
+
 	return nValSet, nil
 }

internal/state/execution.go

@@ -730,7 +730,7 @@ func execBlock(
 		logger.Error("executing block", "err", err)
 		return responseFinalizeBlock, err
 	}
-	logger.Info("executed block", "height", block.Height)
+	logger.Debug("executed block", "height", block.Height)
 
 	return responseFinalizeBlock, nil
 }

internal/state/state_test.go

@@ -333,6 +333,39 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) {
 	}
 }
 
+// TestEmptyValidatorUpdates tests that the validator set is updated correctly when there are no validator updates.
+func TestEmptyValidatorUpdates(t *testing.T) {
+	tearDown, _, state := setupTestCase(t)
+	defer tearDown(t)
+
+	firstNode := state.Validators.GetByIndex(0)
+	require.NotZero(t, firstNode.ProTxHash)
+	ctx := dash.ContextWithProTxHash(context.Background(), firstNode.ProTxHash)
+
+	newPrivKey := bls12381.GenPrivKeyFromSecret([]byte("test"))
+	newPubKey := newPrivKey.PubKey()
+	newQuorumHash := crypto.RandQuorumHash()
+
+	expectValidators := types.ValidatorListString(state.Validators.Validators)
+
+	resp := abci.ResponseProcessProposal{
+		ValidatorSetUpdate: &abci.ValidatorSetUpdate{
+			ValidatorUpdates:   nil,
+			ThresholdPublicKey: cryptoenc.MustPubKeyToProto(newPubKey),
+			QuorumHash:         newQuorumHash,
+		}}
+
+	changes, err := state.NewStateChangeset(
+		ctx,
+		sm.RoundParamsFromProcessProposal(&resp, nil, 0),
+	)
+	require.NoError(t, err)
+
+	assert.EqualValues(t, newQuorumHash, changes.NextValidators.QuorumHash, "quorum hash should be updated")
+	assert.EqualValues(t, newPubKey, changes.NextValidators.ThresholdPublicKey, "threshold public key should be updated")
+	assert.Equal(t, expectValidators, types.ValidatorListString(changes.NextValidators.Validators), "validator should not change")
+}
+
 //func TestProposerFrequency(t *testing.T) {
 //	ctx, cancel := context.WithCancel(context.Background())
 //	defer cancel()

proto/tendermint/abci/types.proto

@@ -798,6 +798,21 @@ message ValidatorUpdate {
   string node_address = 4 [(gogoproto.nullable) = true];
 }
 
+// ValidatorSetUpdate represents a change in the validator set.
+// It can be used to add, remove, or update a validator.
+//
+// Validator set update consists of multiple ValidatorUpdate records,
+// each of them can be used to add, remove, or update a validator, according to the
+// following rules:
+//
+// 1. If a validator with the same public key already exists in the validator set
+// and power is greater than 0, the existing validator will be updated with the new power.
+// 2. If a validator with the same public key already exists in the validator set
+// and power is 0, the existing validator will be removed from the validator set.
+// 3. If a validator with the same public key does not exist in the validator set and the power is greater than 0,
+// a new validator will be added to the validator set.
+// 4. As a special case, if quorum hash has changed, all existing validators will be removed before applying
+// the new validator set update.
 message ValidatorSetUpdate {
   repeated ValidatorUpdate    validator_updates    = 1 [(gogoproto.nullable) = false];
   tendermint.crypto.PublicKey threshold_public_key = 2 [(gogoproto.nullable) = false];

spec/abci++/api.md

@@ -1179,7 +1179,21 @@ Validator
 <a name="tendermint-abci-ValidatorSetUpdate"></a>
 
 ### ValidatorSetUpdate
-
+ValidatorSetUpdate represents a change in the validator set.
+It can be used to add, remove, or update a validator.
+
+Validator set update consists of multiple ValidatorUpdate records,
+each of them can be used to add, remove, or update a validator, according to the
+following rules:
+
+1. If a validator with the same public key already exists in the validator set
+and power is greater than 0, the existing validator will be updated with the new power.
+2. If a validator with the same public key already exists in the validator set
+and power is 0, the existing validator will be removed from the validator set.
+3. If a validator with the same public key does not exist in the validator set and the power is greater than 0,
+a new validator will be added to the validator set.
+4. As a special case, if quorum hash has changed, all existing validators will be removed before applying
+the new validator set update.
 
 
 | Field | Type | Label | Description |