Ensure validator bytes are correctly verified prior Rio (#1749)

* Ensure validator bytes are correctly verified prior Rio

* Add new special block for Amoy

* Fix unit test

* Add new blocks

Author: cffls

builder/files/genesis-amoy.json

@@ -23,6 +23,7 @@
         "ahmedabadBlock": 11865856,
         "bhilaiBlock": 22765056,
         "rioBlock": 26272256,
+        "skipValidatorByteCheck": [26160367, 26161087, 26171567, 26173743, 26175647],
         "stateSyncConfirmationDelay": {
           "0": 128
         },

consensus/bor/bor.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"math/big"
+	"slices"
 	"sort"
 	"strconv"
 	"sync"
@@ -111,8 +112,8 @@ var (
 	// be modified via out-of-range or non-contiguous headers.
 	errOutOfRangeChain = errors.New("out of range or non-contiguous chain")
 
-	errUncleDetected = errors.New("uncles not allowed")
-	// errUnknownValidators = errors.New("unknown validators")
+	errUncleDetected     = errors.New("uncles not allowed")
+	errUnknownValidators = errors.New("unknown validators")
 )
 
 // SignerFn is a signer callback function to request a header to be signed by a
@@ -523,6 +524,69 @@ func (c *Bor) verifyCascadingFields(chain consensus.ChainHeaderReader, header *t
 		return ErrInvalidTimestamp
 	}
 
+	if !c.config.IsRio(header.Number) && !slices.Contains(c.config.SkipValidatorByteCheck, number) {
+		// Retrieve the snapshot needed to verify this header and cache it
+		snap, err := c.snapshot(chain, header, parents, false)
+		if err != nil {
+			return err
+		}
+
+		// Verify if the producer set in header's extra data matches with the list in span.
+		// We skip the check for 0th span as the producer set in contract v/s producer set
+		// in heimdall span is different which will lead a mismatch. Moreover, to make the
+		// validation stateless, we use the span from heimdall (via span store) instead of
+		// span from validator set genesis contract as both are supposed to be equivalent.
+		if number > zerothSpanEnd && IsSprintStart(number+1, c.config.CalculateSprint(number)) {
+			span, err := c.spanStore.spanByBlockNumber(context.Background(), number+1)
+			if err != nil {
+				return err
+			}
+
+			// Use producer set from span as it's equivalent to the data we get from genesis contract
+			selectedProducers := borSpan.ConvertHeimdallValidatorsToBorValidators(span.SelectedProducers)
+			newValidators := make([]*valset.Validator, len(selectedProducers))
+			for i, val := range selectedProducers {
+				newValidators[i] = &val
+			}
+			sort.Sort(valset.ValidatorsByAddress(newValidators))
+
+			headerVals, err := valset.ParseValidators(header.GetValidatorBytes(c.chainConfig))
+			if err != nil {
+				return err
+			}
+
+			if len(newValidators) != len(headerVals) {
+				log.Warn("Invalid validator set", "block number", number, "newValidators", newValidators, "headerVals", headerVals)
+				return errInvalidSpanValidators
+			}
+
+			for i, val := range newValidators {
+				if !bytes.Equal(val.HeaderBytes(), headerVals[i].HeaderBytes()) {
+					log.Warn("Invalid validator set", "block number", number, "index", i, "local validator", val, "header validator", headerVals[i])
+					return errInvalidSpanValidators
+				}
+			}
+		}
+
+		// verify the validator list in the last sprint block
+		if IsSprintStart(number, c.config.CalculateSprint(number)) && !slices.Contains(c.config.SkipValidatorByteCheck, number-1) {
+			parentValidatorBytes := parent.GetValidatorBytes(c.chainConfig)
+			validatorsBytes := make([]byte, len(snap.ValidatorSet.Validators)*validatorHeaderBytesLength)
+
+			currentValidators := snap.ValidatorSet.Copy().Validators
+			// sort validator by address
+			sort.Sort(valset.ValidatorsByAddress(currentValidators))
+
+			for i, validator := range currentValidators {
+				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(parentValidatorBytes, validatorsBytes) {
+				return &MismatchingValidatorsError{number - 1, validatorsBytes, parentValidatorBytes}
+			}
+		}
+	}
+
 	// All basic checks passed, verify the seal and return
 	return c.verifySeal(chain, header, parents)
 }
@@ -854,17 +918,9 @@ func (c *Bor) Prepare(chain consensus.ChainHeaderReader, header *types.Header) e
 
 	// get validator set if number
 	if IsSprintStart(number+1, c.config.CalculateSprint(number)) && !c.config.IsRio(header.Number) {
-		span, err := c.spanStore.spanByBlockNumber(context.Background(), number+1)
+		newValidators, err := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.ParentHash, number+1)
 		if err != nil {
-			return err
-		}
-
-		newValidators := make([]*valset.Validator, len(span.SelectedProducers))
-		for i, val := range span.SelectedProducers {
-			newValidators[i] = &valset.Validator{
-				Address:     common.HexToAddress(val.Signer),
-				VotingPower: val.VotingPower,
-			}
+			return errUnknownValidators
 		}
 
 		// sort validator by address

consensus/bor/span_store.go

@@ -215,6 +215,11 @@ func (s *SpanStore) spanById(ctx context.Context, spanId uint64) (*borTypes.Span
 			log.Warn("Unable to fetch span from heimdall", "id", spanId, "err", err)
 			return nil, err
 		}
+
+		if len(currentSpan.SelectedProducers) == 0 {
+			log.Warn("Span from Heimdall has empty SelectedProducers", "spanId", spanId, "selectedProducers", currentSpan.SelectedProducers, "validators", currentSpan.ValidatorSet.Validators, "startBlock", currentSpan.StartBlock, "endBlock", currentSpan.EndBlock)
+			return nil, fmt.Errorf("span %d has empty SelectedProducers, possibly incomplete", spanId)
+		}
 	}
 
 	if currentSpan == nil {
@@ -241,7 +246,7 @@ func (s *SpanStore) spanByBlockNumber(ctx context.Context, blockNumber uint64) (
 	// that we still check if the block number lies in the range of span before returning it.
 	estimatedSpanId := s.estimateSpanId(blockNumber)
 	defer func() {
-		if res != nil && err == nil {
+		if res != nil && len(res.SelectedProducers) > 0 && err == nil {
 			s.lastUsedSpan.Store(res)
 		}
 	}()

consensus/bor/span_store_test.go

@@ -1309,8 +1309,8 @@ func TestSpanStore_WaitForNewSpan(t *testing.T) {
 		}()
 
 		found, err := store.waitForNewSpan(150, author1Address, 1*time.Second)
-		require.NoError(t, err)
-		require.True(t, found)
+		require.Error(t, err)
+		require.False(t, found)
 	})
 
 	t.Run("target block not initially in span", func(t *testing.T) {

internal/cli/server/chains/amoy.go

@@ -64,6 +64,7 @@ var amoyTestnet = &Chain{
 					"0":        "0x0000000000000000000000000000000000000000",
 					"26272256": "0x7Ee41D8A25641000661B1EF5E6AE8A00400466B0",
 				},
+				SkipValidatorByteCheck: []uint64{26160367, 26161087, 26171567, 26173743, 26175647},
 				BlockAlloc: map[string]interface{}{
 					"11865856": map[string]interface{}{
 						// StateReceiver contract

params/config.go

@@ -366,6 +366,7 @@ var (
 				"0":        "0x0000000000000000000000000000000000000000",
 				"26272256": "0x7Ee41D8A25641000661B1EF5E6AE8A00400466B0",
 			},
+			SkipValidatorByteCheck: []uint64{26160367, 26161087, 26171567, 26173743, 26175647},
 			BlockAlloc: map[string]interface{}{
 				// write as interface since that is how it is decoded in genesis
 				"11865856": map[string]interface{}{
@@ -865,6 +866,7 @@ type BorConfig struct {
 	BlockAlloc                      map[string]interface{} `json:"blockAlloc"`
 	BurntContract                   map[string]string      `json:"burntContract"`              // governance contract where the token will be sent to and burnt in london fork
 	Coinbase                        map[string]string      `json:"coinbase"`                   // coinbase address
+	SkipValidatorByteCheck          []uint64               `json:"skipValidatorByteCheck"`     // skip validator byte check
 	JaipurBlock                     *big.Int               `json:"jaipurBlock"`                // Jaipur switch block (nil = no fork, 0 = already on jaipur)
 	DelhiBlock                      *big.Int               `json:"delhiBlock"`                 // Delhi switch block (nil = no fork, 0 = already on delhi)
 	IndoreBlock                     *big.Int               `json:"indoreBlock"`                // Indore switch block (nil = no fork, 0 = already on indore)