diff --git a/consensus/bor/bor.go b/consensus/bor/bor.go index a32543f087494a444337916ffbe2dde63c3a4169..994465155efaf65dbafb906181846db6564a8977 100644 --- a/consensus/bor/bor.go +++ b/consensus/bor/bor.go @@ -582,8 +582,7 @@ func (c *Bor) verifySeal(chain consensus.ChainReader, header *types.Header, pare return errUnauthorizedSigner } - proposer := snap.ValidatorSet.GetProposer().Address - if _, err = snap.getSignerSuccessionNumber(signer, proposer); err != nil { + if _, err = snap.GetSignerSuccessionNumber(signer); err != nil { return err } @@ -748,8 +747,7 @@ func (c *Bor) Seal(chain consensus.ChainReader, block *types.Block, results chan return errUnauthorizedSigner } - proposer := snap.ValidatorSet.GetProposer().Address - successionNumber, err := snap.getSignerSuccessionNumber(signer, proposer) + successionNumber, err := snap.GetSignerSuccessionNumber(signer) if err != nil { return err } @@ -760,7 +758,7 @@ func (c *Bor) Seal(chain consensus.ChainReader, block *types.Block, results chan delay += wiggle log.Info("Out-of-turn signing requested", "wiggle", common.PrettyDuration(wiggle)) - log.Info("Sealing block with", "number", number, "delay", delay, "headerDifficulty", header.Difficulty, "signer", signer.Hex(), "proposer", proposer.Hex()) + log.Info("Sealing block with", "number", number, "delay", delay, "headerDifficulty", header.Difficulty, "signer", signer.Hex(), "proposer", snap.ValidatorSet.GetProposer().Address.Hex()) // Sign all the things! sighash, err := signFn(accounts.Account{Address: signer}, accounts.MimetypeBor, BorRLP(header)) diff --git a/consensus/bor/bor_test/snapshot_test.go b/consensus/bor/bor_test/snapshot_test.go new file mode 100644 index 0000000000000000000000000000000000000000..264c63295299f6967f16c4fd124bd4266a2f3a63 --- /dev/null +++ b/consensus/bor/bor_test/snapshot_test.go @@ -0,0 +1,125 @@ +package bortest + +import ( + "math/rand" + "sort" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/maticnetwork/bor/common" + "github.com/maticnetwork/bor/consensus/bor" +) + +const ( + numVals = 100 +) + +func TestGetSignerSuccessionNumber_ProposerIsSigner(t *testing.T) { + validators := buildRandomValidatorSet(numVals) + validatorSet := bor.NewValidatorSet(validators) + snap := bor.Snapshot{ + ValidatorSet: validatorSet, + } + + // proposer is signer + signer := validatorSet.Proposer.Address + successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { + t.Fatalf("%s", err) + } + assert.Equal(t, 0, successionNumber) +} + +func TestGetSignerSuccessionNumber_SignerIndexIsLarger(t *testing.T) { + validators := buildRandomValidatorSet(numVals) + + // sort validators by address, which is what bor.NewValidatorSet also does + sort.Sort(bor.ValidatorsByAddress(validators)) + proposerIndex := 32 + signerIndex := 56 + // give highest ProposerPriority to a particular val, so that they become the proposer + validators[proposerIndex].VotingPower = 200 + snap := bor.Snapshot{ + ValidatorSet: bor.NewValidatorSet(validators), + } + + // choose a signer at an index greater than proposer index + signer := snap.ValidatorSet.Validators[signerIndex].Address + successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { + t.Fatalf("%s", err) + } + assert.Equal(t, signerIndex-proposerIndex, successionNumber) +} + +func TestGetSignerSuccessionNumber_SignerIndexIsSmaller(t *testing.T) { + validators := buildRandomValidatorSet(numVals) + proposerIndex := 98 + signerIndex := 11 + // give highest ProposerPriority to a particular val, so that they become the proposer + validators[proposerIndex].VotingPower = 200 + snap := bor.Snapshot{ + ValidatorSet: bor.NewValidatorSet(validators), + } + + // choose a signer at an index greater than proposer index + signer := snap.ValidatorSet.Validators[signerIndex].Address + successionNumber, err := snap.GetSignerSuccessionNumber(signer) + if err != nil { + t.Fatalf("%s", err) + } + assert.Equal(t, signerIndex+numVals-proposerIndex, successionNumber) +} + +func TestGetSignerSuccessionNumber_ProposerNotFound(t *testing.T) { + validators := buildRandomValidatorSet(numVals) + snap := bor.Snapshot{ + ValidatorSet: bor.NewValidatorSet(validators), + } + dummyProposerAddress := randomAddress() + snap.ValidatorSet.Proposer = &bor.Validator{Address: dummyProposerAddress} + // choose any signer + signer := snap.ValidatorSet.Validators[3].Address + _, err := snap.GetSignerSuccessionNumber(signer) + assert.NotNil(t, err) + e, ok := err.(*bor.ProposerNotFoundError) + assert.True(t, ok) + assert.Equal(t, dummyProposerAddress, e.Address) +} + +func TestGetSignerSuccessionNumber_SignerNotFound(t *testing.T) { + validators := buildRandomValidatorSet(numVals) + snap := bor.Snapshot{ + ValidatorSet: bor.NewValidatorSet(validators), + } + dummySignerAddress := randomAddress() + _, err := snap.GetSignerSuccessionNumber(dummySignerAddress) + assert.NotNil(t, err) + e, ok := err.(*bor.SignerNotFoundError) + assert.True(t, ok) + assert.Equal(t, dummySignerAddress, e.Address) +} + +func buildRandomValidatorSet(numVals int) []*bor.Validator { + rand.Seed(time.Now().Unix()) + validators := make([]*bor.Validator, numVals) + for i := 0; i < numVals; i++ { + validators[i] = &bor.Validator{ + Address: randomAddress(), + // cannot process validators with voting power 0, hence +1 + VotingPower: int64(rand.Intn(99) + 1), + } + } + + // sort validators by address, which is what bor.NewValidatorSet also does + sort.Sort(bor.ValidatorsByAddress(validators)) + return validators +} + +func randomAddress() common.Address { + bytes := make([]byte, 32) + rand.Read(bytes) + return common.BytesToAddress(bytes) +} diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index f0bd723ef54b4cca69aa237f0117a24917c5562f..08088d791103647145acc7e72744aa5a09863cec 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -158,8 +158,7 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { return nil, errUnauthorizedSigner } - proposer := snap.ValidatorSet.GetProposer().Address - if _, err = snap.getSignerSuccessionNumber(signer, proposer); err != nil { + if _, err = snap.GetSignerSuccessionNumber(signer); err != nil { return nil, err } @@ -186,16 +185,10 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { return snap, nil } -// getSignerSuccessionNumber returns the relative position of signer in terms of the in-turn proposer -func (s *Snapshot) getSignerSuccessionNumber(signer common.Address, proposer common.Address) (int, error) { +// GetSignerSuccessionNumber returns the relative position of signer in terms of the in-turn proposer +func (s *Snapshot) GetSignerSuccessionNumber(signer common.Address) (int, error) { validators := s.ValidatorSet.Validators - // bor.verifySeal and bor.Seal has the following commented out. - // TODO DISCUSS WITH JD if this is required - // If it is, we will also need to send in header.Number as parameter - // if number%c.config.Sprint != 0 { - // proposer = snap.Recents[number-1] - // } - + proposer := s.ValidatorSet.GetProposer().Address proposerIndex, _ := s.ValidatorSet.GetByAddress(proposer) if proposerIndex == -1 { return -1, &ProposerNotFoundError{proposer}