Merge pull request #51 from maticnetwork/dev-audit-fixes

MAT-1248: Audit fixes

accounts/scwallet/hub.go

@@ -41,11 +41,11 @@ import (
 	"sync"
 	"time"
 
+	pcsc "github.com/gballet/go-libpcsclite"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/event"
 	"github.com/maticnetwork/bor/log"
-	pcsc "github.com/gballet/go-libpcsclite"
 )
 
 // Scheme is the URI prefix for smartcard wallets.

accounts/scwallet/securechannel.go

@@ -25,8 +25,8 @@ import (
 	"crypto/sha512"
 	"fmt"
 
-	"github.com/maticnetwork/bor/crypto"
 	pcsc "github.com/gballet/go-libpcsclite"
+	"github.com/maticnetwork/bor/crypto"
 	"github.com/wsddn/go-ecdh"
 	"golang.org/x/crypto/pbkdf2"
 	"golang.org/x/text/unicode/norm"

accounts/scwallet/wallet.go

@@ -33,13 +33,13 @@ import (
 	"sync"
 	"time"
 
+	pcsc "github.com/gballet/go-libpcsclite"
 	ethereum "github.com/maticnetwork/bor"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/core/types"
 	"github.com/maticnetwork/bor/crypto"
 	"github.com/maticnetwork/bor/log"
-	pcsc "github.com/gballet/go-libpcsclite"
 	"github.com/status-im/keycard-go/derivationpath"
 )
 

accounts/usbwallet/hub.go

@@ -23,10 +23,10 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/karalabe/usb"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/event"
 	"github.com/maticnetwork/bor/log"
-	"github.com/karalabe/usb"
 )
 
 // LedgerScheme is the protocol scheme prefixing account and wallet URLs.

accounts/usbwallet/trezor.go

@@ -27,13 +27,13 @@ import (
 	"io"
 	"math/big"
 
+	"github.com/golang/protobuf/proto"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/accounts/usbwallet/trezor"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/common/hexutil"
 	"github.com/maticnetwork/bor/core/types"
 	"github.com/maticnetwork/bor/log"
-	"github.com/golang/protobuf/proto"
 )
 
 // ErrTrezorPINNeeded is returned if opening the trezor requires a PIN code. In

accounts/usbwallet/wallet.go

@@ -25,13 +25,13 @@ import (
 	"sync"
 	"time"
 
+	"github.com/karalabe/usb"
 	ethereum "github.com/maticnetwork/bor"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/core/types"
 	"github.com/maticnetwork/bor/crypto"
 	"github.com/maticnetwork/bor/log"
-	"github.com/karalabe/usb"
 )
 
 // Maximum time between wallet health checks to detect USB unplugs.

consensus/bor/bor.go

@@ -201,8 +201,8 @@ func encodeSigHeader(w io.Writer, header *types.Header) {
 // CalcDifficulty is the difficulty adjustment algorithm. It returns the difficulty
 // that a new block should have based on the previous blocks in the chain and the
 // current signer.
-func CalcDifficulty(snap *Snapshot, signer common.Address, epoch uint64) *big.Int {
-	return big.NewInt(0).SetUint64(snap.inturn(snap.Number+1, signer, epoch))
+func CalcDifficulty(snap *Snapshot, signer common.Address, sprint uint64) *big.Int {
+	return big.NewInt(0).SetUint64(snap.inturn(snap.Number+1, signer, sprint))
 }
 
 // CalcProducerDelay is the block delay algorithm based on block time and period / producerDelay values in genesis
@@ -346,12 +346,9 @@ func (c *Bor) verifyHeader(chain consensus.ChainReader, header *types.Header, pa
 	if header.Time > uint64(time.Now().Unix()) {
 		return consensus.ErrFutureBlock
 	}
-	// Check that the extra-data contains both the vanity and signature
-	if len(header.Extra) < extraVanity {
-		return errMissingVanity
-	}
-	if len(header.Extra) < extraVanity+extraSeal {
-		return errMissingSignature
+
+	if err := validateHeaderExtraField(header.Extra); err != nil {
+		return err
 	}
 
 	// check extr adata
@@ -387,6 +384,18 @@ func (c *Bor) verifyHeader(chain consensus.ChainReader, header *types.Header, pa
 	return c.verifyCascadingFields(chain, header, parents)
 }
 
+// validateHeaderExtraField validates that the extra-data contains both the vanity and signature.
+// header.Extra = header.Vanity + header.ProducerBytes (optional) + header.Seal
+func validateHeaderExtraField(extraBytes []byte) error {
+	if len(extraBytes) < extraVanity {
+		return errMissingVanity
+	}
+	if len(extraBytes) < extraVanity+extraSeal {
+		return errMissingSignature
+	}
+	return nil
+}
+
 // verifyCascadingFields verifies all the header fields that are not standalone,
 // rather depend on a batch of previous headers. The caller may optionally pass
 // in a batch of parents (ascending order) to avoid looking those up from the
@@ -420,8 +429,9 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainReader, header *types.H
 		return err
 	}
 
-	// If the block is a sprint end block, verify the validator list
-	if number%c.config.Sprint == 0 {
+	isSprintEnd := (number+1)%c.config.Sprint == 0
+	// verify the validator list in the last sprint block
+	if isSprintEnd {
 		validatorsBytes := make([]byte, len(snap.ValidatorSet.Validators)*validatorHeaderBytesLength)
 
 		currentValidators := snap.ValidatorSet.Copy().Validators
@@ -430,14 +440,9 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainReader, header *types.H
 		for i, validator := range currentValidators {
 			copy(validatorsBytes[i*validatorHeaderBytesLength:], validator.HeaderBytes())
 		}
-
-		extraSuffix := len(header.Extra) - extraSeal
-
-		// fmt.Println("validatorsBytes ==> verify seal ==> ", hex.EncodeToString(validatorsBytes))
-		// fmt.Println("header.Extra ==> verify seal ==> ", hex.EncodeToString(header.Extra[extraVanity:extraSuffix]))
-
-		if !bytes.Equal(header.Extra[extraVanity:extraSuffix], validatorsBytes) {
-			// return errMismatchingSprintValidators
+		// len(header.Extra) >= extraVanity+extraSeal has already been validated in validateHeaderExtraField, so this won't result in a panic
+		if !bytes.Equal(header.Extra[extraVanity : len(header.Extra)-extraSeal], validatorsBytes) {
+			return errMismatchingSprintValidators
 		}
 	}
 
@@ -474,7 +479,7 @@ func (c *Bor) snapshot(chain consensus.ChainReader, number uint64, hash common.H
 		// up more headers than allowed to be reorged (chain reinit from a freezer),
 		// consider the checkpoint trusted and snapshot it.
 		// TODO fix this
-		if number == 0 /* || (number%c.config.Sprint == 0 && (len(headers) > params.ImmutabilityThreshold || chain.GetHeaderByNumber(number-1) == nil)) */ {
+		if number == 0 {
 			checkpoint := chain.GetHeaderByNumber(number)
 			if checkpoint != nil {
 				// get checkpoint data
@@ -582,27 +587,8 @@ func (c *Bor) verifySeal(chain consensus.ChainReader, header *types.Header, pare
 		return errUnauthorizedSigner
 	}
 
-	// check if signer is correct
-	validators := snap.ValidatorSet.Validators
-	// proposer will be the last signer if block is not epoch block
-	proposer := snap.ValidatorSet.GetProposer().Address
-	if number%c.config.Sprint != 0 {
-		// proposer = snap.Recents[number-1]
-	}
-	proposerIndex, _ := snap.ValidatorSet.GetByAddress(proposer)
-	signerIndex, _ := snap.ValidatorSet.GetByAddress(signer)
-	limit := len(validators)/2 + 1
-
-	// temp index
-	tempIndex := signerIndex
-	if proposerIndex != tempIndex && limit > 0 {
-		if tempIndex < proposerIndex {
-			tempIndex = tempIndex + len(validators)
-		}
-
-		if tempIndex-proposerIndex > limit {
-			return errRecentlySigned
-		}
+	if _, err = snap.GetSignerSuccessionNumber(signer); err != nil {
+		return err
 	}
 
 	// Ensure that the difficulty corresponds to the turn-ness of the signer
@@ -766,35 +752,18 @@ func (c *Bor) Seal(chain consensus.ChainReader, block *types.Block, results chan
 		return errUnauthorizedSigner
 	}
 
-	validators := snap.ValidatorSet.Validators
-	// proposer will be the last signer if block is not epoch block
-	proposer := snap.ValidatorSet.GetProposer().Address
-	if number%c.config.Sprint != 0 {
-		// proposer = snap.Recents[number-1]
-	}
-
-	proposerIndex, _ := snap.ValidatorSet.GetByAddress(proposer)
-	signerIndex, _ := snap.ValidatorSet.GetByAddress(signer)
-	limit := len(validators)/2 + 1
-
-	// temp index
-	tempIndex := signerIndex
-	if tempIndex < proposerIndex {
-		tempIndex = tempIndex + len(validators)
-	}
-
-	if limit > 0 && tempIndex-proposerIndex > limit {
-		log.Info("Signed recently, must wait for others")
-		return nil
+	successionNumber, err := snap.GetSignerSuccessionNumber(signer)
+	if err != nil {
+		return err
 	}
 
 	// Sweet, the protocol permits us to sign the block, wait for our time
 	delay := time.Unix(int64(header.Time), 0).Sub(time.Now()) // nolint: gosimple
-	wiggle := time.Duration(2*c.config.Period) * time.Second * time.Duration(tempIndex-proposerIndex)
+	wiggle := time.Duration(2*c.config.Period) * time.Second * time.Duration(successionNumber)
 	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))
@@ -854,7 +823,7 @@ func (c *Bor) Close() error {
 	return nil
 }
 
-// Checks if new span is pending
+// Checks if "force" proposeSpan has been set
 func (c *Bor) isSpanPending(snapshotNumber uint64) (bool, error) {
 	blockNr := rpc.BlockNumber(snapshotNumber)
 	method := "spanProposalPending"
@@ -866,14 +835,10 @@ func (c *Bor) isSpanPending(snapshotNumber uint64) (bool, error) {
 		return false, err
 	}
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel() // cancel when we are finished consuming integers
-
-	// call
 	msgData := (hexutil.Bytes)(data)
 	toAddress := common.HexToAddress(c.config.ValidatorContract)
 	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
-	result, err := c.ethAPI.Call(ctx, ethapi.CallArgs{
+	result, err := c.ethAPI.Call(context.Background(), ethapi.CallArgs{
 		Gas:  &gas,
 		To:   &toAddress,
 		Data: &msgData,
@@ -904,14 +869,10 @@ func (c *Bor) GetCurrentSpan(snapshotNumber uint64) (*Span, error) {
 		return nil, err
 	}
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel() // cancel when we are finished consuming integers
-
-	// call
 	msgData := (hexutil.Bytes)(data)
 	toAddress := common.HexToAddress(c.config.ValidatorContract)
 	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
-	result, err := c.ethAPI.Call(ctx, ethapi.CallArgs{
+	result, err := c.ethAPI.Call(context.Background(), ethapi.CallArgs{
 		Gas:  &gas,
 		To:   &toAddress,
 		Data: &msgData,
@@ -954,14 +915,11 @@ func (c *Bor) GetCurrentValidators(snapshotNumber uint64, blockNumber uint64) ([
 		return nil, err
 	}
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel() // cancel when we are finished consuming integers
-
 	// call
 	msgData := (hexutil.Bytes)(data)
 	toAddress := common.HexToAddress(c.config.ValidatorContract)
 	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
-	result, err := c.ethAPI.Call(ctx, ethapi.CallArgs{
+	result, err := c.ethAPI.Call(context.Background(), ethapi.CallArgs{
 		Gas:  &gas,
 		To:   &toAddress,
 		Data: &msgData,
@@ -1144,13 +1102,10 @@ func (c *Bor) GetPendingStateProposals(snapshotNumber uint64) ([]*big.Int, error
 		return nil, err
 	}
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel() // cancel when we are finished consuming integers
-
 	msgData := (hexutil.Bytes)(data)
 	toAddress := common.HexToAddress(c.config.StateReceiverContract)
 	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
-	result, err := c.ethAPI.Call(ctx, ethapi.CallArgs{
+	result, err := c.ethAPI.Call(context.Background(), ethapi.CallArgs{
 		Gas:  &gas,
 		To:   &toAddress,
 		Data: &msgData,

consensus/bor/bor_test/helper.go

@@ -105,7 +105,16 @@ func buildMinimalNextHeader(t *testing.T, block *types.Block, borConfig *params.
 	header.Number.Add(header.Number, big.NewInt(1))
 	header.ParentHash = block.Hash()
 	header.Time += bor.CalcProducerDelay(header.Number.Uint64(), borConfig.Period, borConfig.Sprint, borConfig.ProducerDelay)
-	header.Extra = make([]byte, 97) // vanity (32) + extraSeal (65)
+	isSprintEnd := (header.Number.Uint64()+1)%borConfig.Sprint == 0
+	if isSprintEnd {
+		header.Extra = make([]byte, 32 + 40 + 65) // vanity + validatorBytes + extraSeal
+		// the genesis file was initialized with a validator 0x71562b71999873db5b286df957af199ec94617f7 with power 10
+		// So, if you change ./genesis.json, do change the following as well
+		validatorBytes, _ := hex.DecodeString("71562b71999873db5b286df957af199ec94617f7000000000000000000000000000000000000000a")
+		copy(header.Extra[32:72], validatorBytes)
+	} else {
+		header.Extra = make([]byte, 32 + 65) // vanity + extraSeal
+	}
 	_key, _ := hex.DecodeString(privKey)
 	sig, err := secp256k1.Sign(crypto.Keccak256(bor.BorRLP(header)), _key)
 	if err != nil {

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)
+}

consensus/bor/errors.go

@@ -0,0 +1,42 @@
+package bor
+
+import (
+	"fmt"
+
+	"github.com/maticnetwork/bor/common"
+)
+
+// Will include any new bor consensus errors here in an attempt to make error messages more descriptive
+
+// ProposerNotFoundError is returned if the given proposer address is not present in the validator set
+type ProposerNotFoundError struct {
+	Address common.Address
+}
+
+func (e *ProposerNotFoundError) Error() string {
+	return fmt.Sprintf("Proposer: %s not found", e.Address.Hex())
+}
+
+// SignerNotFoundError is returned when the signer address is not present in the validator set
+type SignerNotFoundError struct {
+	Address common.Address
+}
+
+func (e *SignerNotFoundError) Error() string {
+	return fmt.Sprintf("Signer: %s not found", e.Address.Hex())
+}
+
+// TotalVotingPowerExceededError is returned when the maximum allowed total voting power is exceeded
+type TotalVotingPowerExceededError struct {
+	Sum        int64
+	Validators []*Validator
+}
+
+func (e *TotalVotingPowerExceededError) Error() string {
+	return fmt.Sprintf(
+		"Total voting power should be guarded to not exceed %v; got: %v; for validator set: %v",
+		MaxTotalVotingPower,
+		e.Sum,
+		e.Validators,
+	)
+}

consensus/bor/snapshot.go

@@ -87,7 +87,9 @@ func loadSnapshot(config *params.BorConfig, sigcache *lru.ARCCache, db ethdb.Dat
 	snap.ethAPI = ethAPI
 
 	// update total voting power
-	snap.ValidatorSet.updateTotalVotingPower()
+	if err := snap.ValidatorSet.updateTotalVotingPower(); err != nil {
+		return nil, err
+	}
 
 	return snap, nil
 }
@@ -156,28 +158,8 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) {
 			return nil, errUnauthorizedSigner
 		}
 
-		//
-		// Check validator
-		//
-
-		validators := snap.ValidatorSet.Validators
-		// proposer will be the last signer if block is not epoch block
-		proposer := snap.ValidatorSet.GetProposer().Address
-		proposerIndex, _ := snap.ValidatorSet.GetByAddress(proposer)
-		signerIndex, _ := snap.ValidatorSet.GetByAddress(signer)
-		limit := len(validators)/2 + 1
-
-		// temp index
-		tempIndex := signerIndex
-		if proposerIndex != tempIndex && limit > 0 {
-			if tempIndex < proposerIndex {
-				tempIndex = tempIndex + len(validators)
-			}
-
-			if tempIndex-proposerIndex > limit {
-				log.Info("Invalid signer: error while applying headers", "proposerIndex", validators[proposerIndex].Address.Hex(), "signerIndex", validators[signerIndex].Address.Hex())
-				return nil, errRecentlySigned
-			}
+		if _, err = snap.GetSignerSuccessionNumber(signer); err != nil {
+			return nil, err
 		}
 
 		// add recents
@@ -185,6 +167,9 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) {
 
 		// change validator set and change proposer
 		if number > 0 && (number+1)%s.config.Sprint == 0 {
+			if err := validateHeaderExtraField(header.Extra); err != nil {
+				return nil, err
+			}
 			validatorBytes := header.Extra[extraVanity : len(header.Extra)-extraSeal]
 
 			// get validators from headers and use that for new validator set
@@ -200,6 +185,34 @@ 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) (int, error) {
+	validators := s.ValidatorSet.Validators
+	proposer := s.ValidatorSet.GetProposer().Address
+	proposerIndex, _ := s.ValidatorSet.GetByAddress(proposer)
+	if proposerIndex == -1 {
+		return -1, &ProposerNotFoundError{proposer}
+	}
+	signerIndex, _ := s.ValidatorSet.GetByAddress(signer)
+	if signerIndex == -1 {
+		return -1, &SignerNotFoundError{signer}
+	}
+	limit := len(validators)/2 + 1
+
+	tempIndex := signerIndex
+	if proposerIndex != tempIndex && limit > 0 {
+		if tempIndex < proposerIndex {
+			tempIndex = tempIndex + len(validators)
+		}
+
+		if tempIndex-proposerIndex > limit {
+			log.Info("errRecentlySigned", "proposerIndex", validators[proposerIndex].Address.Hex(), "signerIndex", validators[signerIndex].Address.Hex())
+			return -1, errRecentlySigned
+		}
+	}
+	return tempIndex - proposerIndex, nil
+}
+
 // signers retrieves the list of authorized signers in ascending order.
 func (s *Snapshot) signers() []common.Address {
 	sigs := make([]common.Address, 0, len(s.ValidatorSet.Validators))

consensus/bor/validator.go

@@ -2,7 +2,7 @@ package bor
 
 import (
 	"bytes"
-	"encoding/json"
+	// "encoding/json"
 	"errors"
 	"fmt"
 	"math/big"
@@ -13,8 +13,6 @@ import (
 )
 
 // Validator represets Volatile state for each Validator
-// NOTE: The ProposerPriority is not included in Validator.Hash();
-// make sure to update that method if changes are made here
 type Validator struct {
 	ID               uint64         `json:"ID"`
 	Address          common.Address `json:"signer"`
@@ -31,18 +29,23 @@ func NewValidator(address common.Address, votingPower int64) *Validator {
 	}
 }
 
-// Creates a new copy of the validator so we can mutate ProposerPriority.
+// Copy creates a new copy of the validator so we can mutate ProposerPriority.
 // Panics if the validator is nil.
 func (v *Validator) Copy() *Validator {
 	vCopy := *v
 	return &vCopy
 }
 
-// Returns the one with higher ProposerPriority.
-func (v *Validator) CompareProposerPriority(other *Validator) *Validator {
+// Cmp returns the one validator with a higher ProposerPriority.
+// If ProposerPriority is same, it returns the validator with lexicographically smaller address
+func (v *Validator) Cmp(other *Validator) *Validator {
+	// if both of v and other are nil, nil will be returned and that could possibly lead to nil pointer dereference bubbling up the stack
 	if v == nil {
 		return other
 	}
+	if other == nil {
+		return v
+	}
 	if v.ProposerPriority > other.ProposerPriority {
 		return v
 	} else if v.ProposerPriority < other.ProposerPriority {
@@ -55,7 +58,6 @@ func (v *Validator) CompareProposerPriority(other *Validator) *Validator {
 			return other
 		} else {
 			panic("Cannot compare identical validators")
-			return nil
 		}
 	}
 }
@@ -80,18 +82,6 @@ func ValidatorListString(vals []*Validator) string {
 	return strings.Join(chunks, ",")
 }
 
-// Bytes computes the unique encoding of a validator with a given voting power.
-// These are the bytes that gets hashed in consensus. It excludes address
-// as its redundant with the pubkey. This also excludes ProposerPriority
-// which changes every round.
-func (v *Validator) Bytes() []byte {
-	b, err := json.Marshal(v)
-	if err != nil {
-		return b
-	}
-	return nil
-}
-
 // HeaderBytes return header bytes
 func (v *Validator) HeaderBytes() []byte {
 	result := make([]byte, 40)

consensus/bor/validator_set.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/maticnetwork/bor/common"
+	"github.com/maticnetwork/bor/log"
 )
 
 // MaxTotalVotingPower - the maximum allowed total voting power.
@@ -182,7 +183,7 @@ func computeMaxMinPriorityDiff(vals *ValidatorSet) int64 {
 func (vals *ValidatorSet) getValWithMostPriority() *Validator {
 	var res *Validator
 	for _, val := range vals.Validators {
-		res = res.CompareProposerPriority(val)
+		res = res.Cmp(val)
 	}
 	return res
 }
@@ -256,28 +257,29 @@ func (vals *ValidatorSet) Size() int {
 }
 
 // Force recalculation of the set's total voting power.
-func (vals *ValidatorSet) updateTotalVotingPower() {
+func (vals *ValidatorSet) updateTotalVotingPower() error {
 
 	sum := int64(0)
 	for _, val := range vals.Validators {
 		// mind overflow
 		sum = safeAddClip(sum, val.VotingPower)
 		if sum > MaxTotalVotingPower {
-			panic(fmt.Sprintf(
-				"Total voting power should be guarded to not exceed %v; got: %v",
-				MaxTotalVotingPower,
-				sum))
+			return &TotalVotingPowerExceededError{sum, vals.Validators}
 		}
 	}
-
 	vals.totalVotingPower = sum
+	return nil
 }
 
 // TotalVotingPower returns the sum of the voting powers of all validators.
 // It recomputes the total voting power if required.
 func (vals *ValidatorSet) TotalVotingPower() int64 {
 	if vals.totalVotingPower == 0 {
-		vals.updateTotalVotingPower()
+		log.Info("invoking updateTotalVotingPower before returning it")
+		if err := vals.updateTotalVotingPower(); err != nil {
+			// Can/should we do better?
+			panic(err)
+		}
 	}
 	return vals.totalVotingPower
 }
@@ -298,7 +300,7 @@ func (vals *ValidatorSet) findProposer() *Validator {
 	var proposer *Validator
 	for _, val := range vals.Validators {
 		if proposer == nil || !bytes.Equal(val.Address.Bytes(), proposer.Address.Bytes()) {
-			proposer = proposer.CompareProposerPriority(val)
+			proposer = proposer.Cmp(val)
 		}
 	}
 	return proposer
@@ -562,7 +564,9 @@ func (vals *ValidatorSet) updateWithChangeSet(changes []*Validator, allowDeletes
 	vals.applyUpdates(updates)
 	vals.applyRemovals(deletes)
 
-	vals.updateTotalVotingPower()
+	if err := vals.updateTotalVotingPower(); err != nil {
+		return err
+	}
 
 	// Scale and center.
 	vals.RescalePriorities(PriorityWindowSizeFactor * vals.TotalVotingPower())

consensus/clique/clique.go

@@ -26,6 +26,7 @@ import (
 	"sync"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/maticnetwork/bor/accounts"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/common/hexutil"
@@ -39,7 +40,6 @@ import (
 	"github.com/maticnetwork/bor/params"
 	"github.com/maticnetwork/bor/rlp"
 	"github.com/maticnetwork/bor/rpc"
-	lru "github.com/hashicorp/golang-lru"
 	"golang.org/x/crypto/sha3"
 )
 

consensus/clique/snapshot.go

@@ -22,12 +22,12 @@ import (
 	"sort"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/core/types"
 	"github.com/maticnetwork/bor/ethdb"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/params"
-	lru "github.com/hashicorp/golang-lru"
 )
 
 // Vote represents a single vote that an authorized signer made to modify the

consensus/ethash/ethash.go

@@ -34,13 +34,13 @@ import (
 	"unsafe"
 
 	mmap "github.com/edsrzf/mmap-go"
+	"github.com/hashicorp/golang-lru/simplelru"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/consensus"
 	"github.com/maticnetwork/bor/core/types"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/metrics"
 	"github.com/maticnetwork/bor/rpc"
-	"github.com/hashicorp/golang-lru/simplelru"
 )
 
 var ErrInvalidDumpMagic = errors.New("invalid dump magic")

core/headerchain.go

@@ -26,6 +26,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/consensus"
 	"github.com/maticnetwork/bor/core/rawdb"
@@ -33,7 +34,6 @@ import (
 	"github.com/maticnetwork/bor/ethdb"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/params"
-	lru "github.com/hashicorp/golang-lru"
 )
 
 const (

core/rawdb/freezer_table.go

@@ -26,10 +26,10 @@ import (
 	"sync"
 	"sync/atomic"
 
+	"github.com/golang/snappy"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/metrics"
-	"github.com/golang/snappy"
 )
 
 var (

core/state/database.go

@@ -19,10 +19,10 @@ package state
 import (
 	"fmt"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/ethdb"
 	"github.com/maticnetwork/bor/trie"
-	lru "github.com/hashicorp/golang-lru"
 )
 
 const (

graphql/service.go

@@ -21,12 +21,12 @@ import (
 	"net"
 	"net/http"
 
+	"github.com/graph-gophers/graphql-go"
+	"github.com/graph-gophers/graphql-go/relay"
 	"github.com/maticnetwork/bor/internal/ethapi"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/p2p"
 	"github.com/maticnetwork/bor/rpc"
-	"github.com/graph-gophers/graphql-go"
-	"github.com/graph-gophers/graphql-go/relay"
 )
 
 // Service encapsulates a GraphQL service.

internal/debug/flags.go

@@ -24,10 +24,10 @@ import (
 	"os"
 	"runtime"
 
+	"github.com/fjl/memsize/memsizeui"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/metrics"
 	"github.com/maticnetwork/bor/metrics/exp"
-	"github.com/fjl/memsize/memsizeui"
 	colorable "github.com/mattn/go-colorable"
 	"github.com/mattn/go-isatty"
 	"gopkg.in/urfave/cli.v1"

light/lightchain.go

@@ -26,6 +26,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/consensus"
 	"github.com/maticnetwork/bor/core"
@@ -37,7 +38,6 @@ import (
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/params"
 	"github.com/maticnetwork/bor/rlp"
-	lru "github.com/hashicorp/golang-lru"
 )
 
 var (

metrics/influxdb/influxdb.go

@@ -5,9 +5,9 @@ import (
 	uurl "net/url"
 	"time"
 
+	"github.com/influxdata/influxdb/client"
 	"github.com/maticnetwork/bor/log"
 	"github.com/maticnetwork/bor/metrics"
-	"github.com/influxdata/influxdb/client"
 )
 
 type reporter struct {

p2p/nat/nat.go

@@ -25,8 +25,8 @@ import (
 	"sync"
 	"time"
 
-	"github.com/maticnetwork/bor/log"
 	"github.com/jackpal/go-nat-pmp"
+	"github.com/maticnetwork/bor/log"
 )
 
 // An implementation of nat.Interface can map local ports to ports

p2p/rlpx.go

@@ -35,11 +35,11 @@ import (
 	"sync"
 	"time"
 
+	"github.com/golang/snappy"
 	"github.com/maticnetwork/bor/common/bitutil"
 	"github.com/maticnetwork/bor/crypto"
 	"github.com/maticnetwork/bor/crypto/ecies"
 	"github.com/maticnetwork/bor/rlp"
-	"github.com/golang/snappy"
 	"golang.org/x/crypto/sha3"
 )
 

p2p/simulations/http.go

@@ -29,12 +29,12 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/julienschmidt/httprouter"
 	"github.com/maticnetwork/bor/event"
 	"github.com/maticnetwork/bor/p2p"
 	"github.com/maticnetwork/bor/p2p/enode"
 	"github.com/maticnetwork/bor/p2p/simulations/adapters"
 	"github.com/maticnetwork/bor/rpc"
-	"github.com/julienschmidt/httprouter"
 	"golang.org/x/net/websocket"
 )