Skip to content

Commit

Permalink
types: validate Validator#Address field (cometbft#1715)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
melekes and thanethomson authored Dec 1, 2023
1 parent 4199a82 commit 63fe7bf
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `[types]` Validate `Validator#Address` in `ValidateBasic` ([\#1715](https://github.com/cometbft/cometbft/pull/1715))
4 changes: 1 addition & 3 deletions internal/state/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ import (

abci "github.com/cometbft/cometbft/abci/types"
cmtstate "github.com/cometbft/cometbft/api/cometbft/state/v1"
"github.com/cometbft/cometbft/crypto"
"github.com/cometbft/cometbft/crypto/ed25519"
cmtrand "github.com/cometbft/cometbft/internal/rand"
sm "github.com/cometbft/cometbft/internal/state"
"github.com/cometbft/cometbft/internal/store"
"github.com/cometbft/cometbft/internal/test"
Expand Down Expand Up @@ -135,7 +133,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,
Expand Down
2 changes: 1 addition & 1 deletion internal/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ func TestPruningService(t *testing.T) {
// Generate a bunch of state data.
// This is needed because the pruning is expecting to load the state from the database thus
// We have to have acceptable values for all fields of the state
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,
Expand Down
5 changes: 3 additions & 2 deletions types/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions types/validator_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,18 +299,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)
}
Expand Down
5 changes: 3 additions & 2 deletions types/validator_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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()),
},
}

Expand Down

0 comments on commit 63fe7bf

Please sign in to comment.