Skip to content

Commit

Permalink
Fix critical proposer selection bug #4259 (#4265)
Browse files Browse the repository at this point in the history 
* fix critical proposer selection bug #4259

* gofmt

* add 1 more validator to make it 5

* more tests

* Fixed archivedProposerIndex

* Fixed TestFilterAttestation_OK

* Refactor ComputeProposerIndex, add regression test for potential out of range panic

* handle case of nil validator

* Update validators_test.go
  • Loading branch information
@prestonvanloon
prestonvanloon authored Dec 17, 2019
1 parent 73d5f07 commit e644dab
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 27 deletions.
23 changes: 17 additions & 6 deletions beacon-chain/core/helpers/validators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,14 @@ func BeaconProposerIndex(state *pb.BeaconState) (uint64, error) {
return 0, errors.Wrap(err, "could not get active indices")
}

return ComputeProposerIndex(state, indices, seedWithSlotHash)
return ComputeProposerIndex(state.Validators, indices, seedWithSlotHash)
}

// ComputeProposerIndex returns the index sampled by effective balance, which is used to calculate proposer.
//
// Note: This method signature deviates slightly from the spec recommended definition. The full
// state object is not required to compute the proposer index.
//
// Spec pseudocode definition:
// def compute_proposer_index(state: BeaconState, indices: Sequence[ValidatorIndex], seed: Hash) -> ValidatorIndex:
// """
Expand All @@ -186,21 +189,29 @@ func BeaconProposerIndex(state *pb.BeaconState) (uint64, error) {
// if effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte:
// return ValidatorIndex(candidate_index)
// i += 1
func ComputeProposerIndex(state *pb.BeaconState, indices []uint64, seed [32]byte) (uint64, error) {
length := uint64(len(indices))
func ComputeProposerIndex(validators []*ethpb.Validator, activeIndices []uint64, seed [32]byte) (uint64, error) {
length := uint64(len(activeIndices))
if length == 0 {
return 0, errors.New("empty indices list")
return 0, errors.New("empty active indices list")
}
maxRandomByte := uint64(1<<8 - 1)

for i := uint64(0); ; i++ {
candidateIndex, err := ComputeShuffledIndex(i%length, length, seed, true)
candidateIndex, err := ComputeShuffledIndex(i%length, length, seed, true /* shuffle */)
if err != nil {
return 0, err
}
candidateIndex = activeIndices[candidateIndex]
if int(candidateIndex) >= len(validators) {
return 0, errors.New("active index out of range")
}
b := append(seed[:], bytesutil.Bytes8(i/32)...)
randomByte := hashutil.Hash(b)[i%32]
effectiveBal := state.Validators[candidateIndex].EffectiveBalance
v := validators[candidateIndex]
var effectiveBal uint64
if v != nil {
effectiveBal = v.EffectiveBalance
}
if effectiveBal*maxRandomByte >= params.BeaconConfig().MaxEffectiveBalance*uint64(randomByte) {
return candidateIndex, nil
}
Expand Down
123 changes: 123 additions & 0 deletions beacon-chain/core/helpers/validators_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,3 +405,126 @@ func TestActiveValidatorIndices(t *testing.T) {
})
}
}

func TestComputeProposerIndex(t *testing.T) {
seed := bytesutil.ToBytes32([]byte("seed"))
type args struct {
validators []*ethpb.Validator
indices []uint64
seed [32]byte
}
tests := []struct {
name string
args args
want uint64
wantErr bool
}{
{
name: "all_active_indices",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{0, 1, 2, 3, 4},
seed: seed,
},
want: 2,
},
{ // Regression test for https://github.com/prysmaticlabs/prysm/issues/4259.
name: "1_active_index",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{3},
seed: seed,
},
want: 3,
},
{
name: "empty_active_indices",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{},
seed: seed,
},
wantErr: true,
},
{
name: "active_indices_out_of_range",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{100},
seed: seed,
},
wantErr: true,
},
{
name: "second_half_active",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{5, 6, 7, 8, 9},
seed: seed,
},
want: 7,
},
{
name: "nil_validator",
args: args{
validators: []*ethpb.Validator{
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
nil, // Should never happen, but would cause a panic when it does happen.
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
&ethpb.Validator{EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance},
},
indices: []uint64{0, 1, 2, 3, 4},
seed: seed,
},
want: 4,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ComputeProposerIndex(tt.args.validators, tt.args.indices, tt.args.seed)
if (err != nil) != tt.wantErr {
t.Errorf("ComputeProposerIndex() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("ComputeProposerIndex() got = %v, want %v", got, tt.want)
}
})
}
}
26 changes: 5 additions & 21 deletions beacon-chain/rpc/beacon/assignments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,27 +228,11 @@ func archivedValidatorCommittee(
return nil, 0, 0, 0, fmt.Errorf("could not find committee for validator index %d", validatorIndex)
}

// helpers.ComputeProposerIndex wrapper.
func archivedProposerIndex(activeIndices []uint64, activeBalances []uint64, seed [32]byte) (uint64, error) {
length := uint64(len(activeIndices))
if length == 0 {
return 0, errors.New("empty indices list")
}
maxRandomByte := uint64(1<<8 - 1)
for i := uint64(0); ; i++ {
candidateIndex, err := helpers.ComputeShuffledIndex(i%length, length, seed, true)
if err != nil {
return 0, err
}
b := append(seed[:], bytesutil.Bytes8(i/32)...)
randomByte := hashutil.Hash(b)[i%32]
effectiveBalance := activeBalances[candidateIndex]
if effectiveBalance >= params.BeaconConfig().MaxEffectiveBalance {
// if the actual balance is greater than or equal to the max effective balance,
// we just determine the proposer index using config.MaxEffectiveBalance.
effectiveBalance = params.BeaconConfig().MaxEffectiveBalance
}
if effectiveBalance*maxRandomByte >= params.BeaconConfig().MaxEffectiveBalance*uint64(randomByte) {
return candidateIndex, nil
}
validators := make([]*ethpb.Validator, len(activeBalances))
for i, bal := range activeBalances {
validators[i] = &ethpb.Validator{EffectiveBalance: bal}
}
return helpers.ComputeProposerIndex(validators, activeIndices, seed)
}

0 comments on commit e644dab

Please sign in to comment.