fix: Check validator bytes at first block of sprint (MAT-1311) (#56)

* Check validator bytes at first block of sprint

* not required to get parent again

* fix tests

* fix build error

consensus/bor/bor.go

@@ -105,10 +105,6 @@ var (
 	// invalid list of validators (i.e. non divisible by 40 bytes).
 	errInvalidSpanValidators = errors.New("invalid validator list on sprint end block")
 
-	// errMismatchingSprintValidators is returned if a sprint block contains a
-	// list of validators different than the one the local node calculated.
-	errMismatchingSprintValidators = errors.New("mismatching validator list on sprint block")
-
 	// errInvalidMixDigest is returned if a block's mix digest is non-zero.
 	errInvalidMixDigest = errors.New("non-zero mix digest")
 
@@ -436,9 +432,9 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainReader, header *types.H
 		return err
 	}
 
-	isSprintEnd := (number+1)%c.config.Sprint == 0
 	// verify the validator list in the last sprint block
-	if isSprintEnd {
+	if isSprintStart(number, c.config.Sprint) {
+		parentValidatorBytes := parent.Extra[extraVanity : len(parent.Extra)-extraSeal]
 		validatorsBytes := make([]byte, len(snap.ValidatorSet.Validators)*validatorHeaderBytesLength)
 
 		currentValidators := snap.ValidatorSet.Copy().Validators
@@ -448,8 +444,8 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainReader, header *types.H
 			copy(validatorsBytes[i*validatorHeaderBytesLength:], validator.HeaderBytes())
 		}
 		// 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
+		if !bytes.Equal(parentValidatorBytes, validatorsBytes) {
+			return &MismatchingValidatorsError{number - 1, validatorsBytes, parentValidatorBytes}
 		}
 	}
 
@@ -1417,3 +1413,7 @@ func getUpdatedValidatorSet(oldValidatorSet *ValidatorSet, newVals []*Validator)
 	v.UpdateWithChangeSet(changes)
 	return v
 }
+
+func isSprintStart(number, sprint uint64) bool {
+	return number%sprint == 0
+}

consensus/bor/bor_test/bor_test.go

@@ -24,31 +24,34 @@ func TestCommitSpan(t *testing.T) {
 	// Mock HeimdallClient.FetchWithRetry to return span data from span.json
 	res, heimdallSpan := loadSpanFromFile(t)
 	h := &mocks.IHeimdallClient{}
-	h.On("FetchWithRetry", "bor", "span", "1").
-		Return(res, nil).
-		Times(2) // both FinalizeAndAssemble and chain.InsertChain call HeimdallClient.FetchWithRetry. @todo Investigate this in depth
+	// FetchWithRetry is invoked 3 times
+	// 1. bor.FinalizeAndAssemble to prepare a new block when calling insertNewBlock
+	// 2. bor.Finalize via(bc.insertChain => bc.processor.Process)
+	// 3. bor.FinalizeAndAssemble via worker.commit
+	h.On("FetchWithRetry", "bor", "span", "1").Return(res, nil).Times(3)
 	_bor.SetHeimdallClient(h)
 
 	db := init.ethereum.ChainDb()
-	block := init.genesis.ToBlock(db)
-	// Build 1st block's header
-	header := buildMinimalNextHeader(t, block, init.genesis.Config.Bor)
-
 	statedb, err := chain.State()
 	if err != nil {
 		t.Fatalf("%s", err)
 	}
-
 	_key, _ := hex.DecodeString(privKey)
-	insertNewBlock(t, _bor, chain, header, statedb, _key)
 
-	assert.True(t, h.AssertNumberOfCalls(t, "FetchWithRetry", 2))
-	validators, err := _bor.GetCurrentValidators(1, 256) // new span starts at 256
+	block := init.genesis.ToBlock(db)
+	// Insert sprintSize # of blocks so that span is fetched at the start of a new sprint
+	for i := uint64(1); i <= sprintSize; i++ {
+		header := buildMinimalNextHeader(t, block, init.genesis.Config.Bor)
+		block = insertNewBlock(t, _bor, chain, header, statedb, _key)
+	}
+
+	assert.True(t, h.AssertNumberOfCalls(t, "FetchWithRetry", 3))
+	validators, err := _bor.GetCurrentValidators(sprintSize, 256) // new span starts at 256
 	if err != nil {
 		t.Fatalf("%s", err)
 	}
 
-	assert.Equal(t, len(validators), 3)
+	assert.Equal(t, 3, len(validators))
 	for i, validator := range validators {
 		assert.Equal(t, validator.Address.Bytes(), heimdallSpan.SelectedProducers[i].Address.Bytes())
 		assert.Equal(t, validator.VotingPower, heimdallSpan.SelectedProducers[i].VotingPower)
@@ -85,29 +88,17 @@ func TestIsValidatorAction(t *testing.T) {
 	h.On("FetchWithRetry", "bor", "span", "1").Return(res, nil)
 	_bor.SetHeimdallClient(h)
 
-	// Build 1st block's header
 	db := init.ethereum.ChainDb()
-	block := init.genesis.ToBlock(db)
-
-	header := buildMinimalNextHeader(t, block, init.genesis.Config.Bor)
 	statedb, err := chain.State()
 	if err != nil {
 		t.Fatalf("%s", err)
 	}
-
 	_key, _ := hex.DecodeString(privKey)
-	insertNewBlock(t, _bor, chain, header, statedb, _key)
-	block = types.NewBlockWithHeader(header)
 
-	var headers []*types.Header
-	for i := int64(2); i <= 255; i++ {
+	block := init.genesis.ToBlock(db)
+	for i := uint64(1); i <= spanSize; i++ {
 		header := buildMinimalNextHeader(t, block, init.genesis.Config.Bor)
-		headers = append(headers, header)
-		block = types.NewBlockWithHeader(header)
-	}
-	t.Logf("inserting %v headers", len(headers))
-	if _, err := chain.InsertHeaderChain(headers, 0); err != nil {
-		t.Fatalf("%s", err)
+		block = insertNewBlock(t, _bor, chain, header, statedb, _key)
 	}
 
 	for _, validator := range heimdallSpan.SelectedProducers {

consensus/bor/bor_test/helper.go

@@ -5,8 +5,10 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"math/big"
+	"sort"
 	"testing"
 
+	"github.com/maticnetwork/bor/common"
 	"github.com/maticnetwork/bor/consensus/bor"
 	"github.com/maticnetwork/bor/core"
 	"github.com/maticnetwork/bor/core/state"
@@ -20,10 +22,14 @@ import (
 )
 
 var (
-	extraSeal = 65 // Fixed number of extra-data suffix bytes reserved for signer seal
-	privKey   = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291"
-	key, _    = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
-	addr      = crypto.PubkeyToAddress(key.PublicKey) // 0x71562b71999873DB5b286dF957af199Ec94617F7
+	// The genesis for tests was generated with following parameters
+	extraSeal                         = 65 // Fixed number of extra-data suffix bytes reserved for signer seal
+	privKey                           = "b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291"
+	key, _                            = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
+	addr                              = crypto.PubkeyToAddress(key.PublicKey) // 0x71562b71999873DB5b286dF957af199Ec94617F7
+	validatorHeaderBytesLength        = common.AddressLength + 20             // address + power
+	sprintSize                 uint64 = 4
+	spanSize                   uint64 = 8
 )
 
 type initializeData struct {
@@ -82,7 +88,7 @@ func buildEthereumInstance(t *testing.T, db ethdb.Database) *initializeData {
 	}
 }
 
-func insertNewBlock(t *testing.T, _bor *bor.Bor, chain *core.BlockChain, header *types.Header, statedb *state.StateDB, privKey []byte) {
+func insertNewBlock(t *testing.T, _bor *bor.Bor, chain *core.BlockChain, header *types.Header, statedb *state.StateDB, privKey []byte) *types.Block {
 	_, err := _bor.FinalizeAndAssemble(chain, header, statedb, nil, nil, nil)
 	if err != nil {
 		t.Fatalf("%s", err)
@@ -98,6 +104,7 @@ func insertNewBlock(t *testing.T, _bor *bor.Bor, chain *core.BlockChain, header
 	if _, err := chain.InsertChain([]*types.Block{block}); err != nil {
 		t.Fatalf("%s", err)
 	}
+	return block
 }
 
 func buildMinimalNextHeader(t *testing.T, block *types.Block, borConfig *params.BorConfig) *types.Header {
@@ -105,15 +112,25 @@ 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)
-	isSprintEnd := (header.Number.Uint64()+1)%borConfig.Sprint == 0
+	header.Extra = make([]byte, 32+65) // vanity + extraSeal
+
+	currentValidators := []*bor.Validator{bor.NewValidator(addr, 10)}
+	isSpanEnd := (header.Number.Uint64()+1)%spanSize == 0
+	isSprintEnd := (header.Number.Uint64()+1)%sprintSize == 0
+	if isSpanEnd {
+		_, heimdallSpan := loadSpanFromFile(t)
+		currentValidators = heimdallSpan.ValidatorSet.Validators
+	} else if header.Number.Uint64()%spanSize == 0 {
+		header.Difficulty = new(big.Int).SetInt64(5)
+	}
 	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
+		sort.Sort(bor.ValidatorsByAddress(currentValidators))
+		validatorBytes := make([]byte, len(currentValidators)*validatorHeaderBytesLength)
+		header.Extra = make([]byte, 32+len(validatorBytes)+65) // vanity + validatorBytes + extraSeal
+		for i, val := range currentValidators {
+			copy(validatorBytes[i*validatorHeaderBytesLength:], val.HeaderBytes())
+		}
+		copy(header.Extra[32:], validatorBytes)
 	}
 	_key, _ := hex.DecodeString(privKey)
 	sig, err := secp256k1.Sign(crypto.Keccak256(bor.BorRLP(header)), _key)

consensus/bor/bor_test/span.json

@@ -47,7 +47,7 @@
 				"endEpoch": 0,
 				"power": 10000,
 				"pubKey": "0x0469536ae98030a7e83ec5ef3baffed2d05a32e31d978e58486f6bdb0fbbf240293838325116090190c0639db03f9cbd8b9aecfd269d016f46e3a2287fbf9ad232",
-				"signer": "0x1c4f0f054a0d6a1415382dc0fd83c6535188b220",
+				"signer": "0x71562b71999873DB5b286dF957af199Ec94617F7",
 				"last_updated": 0,
 				"accum": 10000
 			}],

consensus/bor/errors.go

@@ -79,3 +79,20 @@ func (e *SealingInFlightError) Error() string {
 		e.Number,
 	)
 }
+
+// MismatchingValidatorsError is returned if a last block in sprint contains a
+// list of validators different from the one that local node calculated
+type MismatchingValidatorsError struct {
+	Number             uint64
+	ValidatorSetSnap   []byte
+	ValidatorSetHeader []byte
+}
+
+func (e *MismatchingValidatorsError) Error() string {
+	return fmt.Sprintf(
+		"Mismatching validators at block %d\nValidatorBytes from snapshot: %x\nValidatorBytes in Header: %x\n",
+		e.Number,
+		e.ValidatorSetSnap,
+		e.ValidatorSetHeader,
+	)
+}