types: validate Validator#Address field (backport #1715) (#1722)

* types: validate Validator#Address field (#1715) * types: validate Validator#Address field * fix TestProposerSelection3 * add a changelog entry * fix two more tests * Update .changelog/unreleased/improvements/1715-validate-validator-address Co-authored-by: Thane Thomson <connect@thanethomson.com> --------- Co-authored-by: Thane Thomson <connect@thanethomson.com> (cherry picked from commit 63fe7bf) # Conflicts: # internal/state/store_test.go # internal/store/store_test.go * fix conflicts * fix test * golint --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
cometbft · Dec 2, 2023 · 936d5cc · 936d5cc
1 parent cce2e5d
commit 936d5cc
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 12 deletions.
diff --git a/.changelog/unreleased/improvements/1715-validate-validator-address b/.changelog/unreleased/improvements/1715-validate-validator-address
@@ -0,0 +1 @@
+- `[types]` Validate `Validator#Address` in `ValidateBasic` ([\#1715](https://github.com/cometbft/cometbft/pull/1715))
diff --git a/state/store_test.go b/state/store_test.go
@@ -12,9 +12,7 @@ import (
 
 	abci "github.com/cometbft/cometbft/abci/types"
 	cfg "github.com/cometbft/cometbft/config"
-	"github.com/cometbft/cometbft/crypto"
 	"github.com/cometbft/cometbft/crypto/ed25519"
-	cmtrand "github.com/cometbft/cometbft/libs/rand"
 	cmtstate "github.com/cometbft/cometbft/proto/tendermint/state"
 	sm "github.com/cometbft/cometbft/state"
 	"github.com/cometbft/cometbft/types"
@@ -117,7 +115,7 @@ func TestPruneStates(t *testing.T) {
 
 			// Generate a bunch of state data. Validators change for heights ending with 3, and
 			// parameters when ending with 5.
-			validator := &types.Validator{Address: cmtrand.Bytes(crypto.AddressSize), VotingPower: 100, PubKey: pk}
+			validator := &types.Validator{Address: pk.Address(), VotingPower: 100, PubKey: pk}
 			validatorSet := &types.ValidatorSet{
 				Validators: []*types.Validator{validator},
 				Proposer:   validator,

diff --git a/types/validator.go b/types/validator.go
@@ -46,8 +46,9 @@ func (v *Validator) ValidateBasic() error {
 		return errors.New("validator has negative voting power")
 	}
 
-	if len(v.Address) != crypto.AddressSize {
-		return fmt.Errorf("validator address is the wrong size: %v", v.Address)
+	addr := v.PubKey.Address()
+	if !bytes.Equal(v.Address, addr) {
+		return fmt.Errorf("validator address is incorrectly derived from pubkey. Exp: %v, got %v", addr, v.Address)
 	}
 
 	return nil

diff --git a/types/validator_set_test.go b/types/validator_set_test.go
@@ -300,18 +300,22 @@ func TestProposerSelection2(t *testing.T) {
 }
 
 func TestProposerSelection3(t *testing.T) {
-	vset := NewValidatorSet([]*Validator{
+	vals := []*Validator{
 		newValidator([]byte("avalidator_address12"), 1),
 		newValidator([]byte("bvalidator_address12"), 1),
 		newValidator([]byte("cvalidator_address12"), 1),
 		newValidator([]byte("dvalidator_address12"), 1),
-	})
+	}
 
-	proposerOrder := make([]*Validator, 4)
 	for i := 0; i < 4; i++ {
-		// need to give all validators to have keys
 		pk := ed25519.GenPrivKey().PubKey()
-		vset.Validators[i].PubKey = pk
+		vals[i].PubKey = pk
+		vals[i].Address = pk.Address()
+	}
+	sort.Sort(ValidatorsByAddress(vals))
+	vset := NewValidatorSet(vals)
+	proposerOrder := make([]*Validator, 4)
+	for i := 0; i < 4; i++ {
 		proposerOrder[i] = vset.GetProposer()
 		vset.IncrementProposerPriority(1)
 	}

diff --git a/types/validator_test.go b/types/validator_test.go
@@ -1,6 +1,7 @@
 package types
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -74,15 +75,15 @@ func TestValidatorValidateBasic(t *testing.T) {
 				Address: nil,
 			},
 			err: true,
-			msg: "validator address is the wrong size: ",
+			msg: fmt.Sprintf("validator address is incorrectly derived from pubkey. Exp: %v, got ", pubKey.Address()),
 		},
 		{
 			val: &Validator{
 				PubKey:  pubKey,
 				Address: []byte{'a'},
 			},
 			err: true,
-			msg: "validator address is the wrong size: 61",
+			msg: fmt.Sprintf("validator address is incorrectly derived from pubkey. Exp: %v, got 61", pubKey.Address()),
 		},
 	}