Merge pull request #1415 from maticnetwork/lmartins/validator-id-on-snap-apply

Fix validator id on bor_getSnapshot, bor_getSnapshotAtHash and  bor_getCurrentValidators

consensus/bor/abi/abi.go

@@ -1,5 +1,6 @@
 package abi
 
+//go:generate mockgen -destination=./abi_mock.go -package=api . ABI
 type ABI interface {
 	Pack(name string, args ...interface{}) ([]byte, error)
 	UnpackIntoInterface(v interface{}, name string, data []byte) error

consensus/bor/abi/abi_mock.go

@@ -0,0 +1,68 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/ethereum/go-ethereum/consensus/bor/abi (interfaces: ABI)
+
+// Package abi is a generated GoMock package.
+package abi
+
+import (
+	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
+)
+
+// MockABI is a mock of ABI interface.
+type MockABI struct {
+	ctrl     *gomock.Controller
+	recorder *MockABIMockRecorder
+}
+
+// MockABIMockRecorder is the mock recorder for MockABI.
+type MockABIMockRecorder struct {
+	mock *MockABI
+}
+
+// NewMockABI creates a new mock instance.
+func NewMockABI(ctrl *gomock.Controller) *MockABI {
+	mock := &MockABI{ctrl: ctrl}
+	mock.recorder = &MockABIMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockABI) EXPECT() *MockABIMockRecorder {
+	return m.recorder
+}
+
+// Pack mocks base method.
+func (m *MockABI) Pack(arg0 string, arg1 ...interface{}) ([]byte, error) {
+	m.ctrl.T.Helper()
+	varargs := []interface{}{arg0}
+	for _, a := range arg1 {
+		varargs = append(varargs, a)
+	}
+	ret := m.ctrl.Call(m, "Pack", varargs...)
+	ret0, _ := ret[0].([]byte)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Pack indicates an expected call of Pack.
+func (mr *MockABIMockRecorder) Pack(arg0 interface{}, arg1 ...interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]interface{}{arg0}, arg1...)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Pack", reflect.TypeOf((*MockABI)(nil).Pack), varargs...)
+}
+
+// UnpackIntoInterface mocks base method.
+func (m *MockABI) UnpackIntoInterface(arg0 interface{}, arg1 string, arg2 []byte) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "UnpackIntoInterface", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UnpackIntoInterface indicates an expected call of UnpackIntoInterface.
+func (mr *MockABIMockRecorder) UnpackIntoInterface(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnpackIntoInterface", reflect.TypeOf((*MockABI)(nil).UnpackIntoInterface), arg0, arg1, arg2)
+}

consensus/bor/bor.go

@@ -617,7 +617,7 @@ func (c *Bor) snapshot(chain consensus.ChainHeaderReader, number uint64, hash co
 		headers[i], headers[len(headers)-1-i] = headers[len(headers)-1-i], headers[i]
 	}
 
-	snap, err := snap.apply(headers)
+	snap, err := snap.apply(headers, c)
 	if err != nil {
 		return nil, err
 	}

consensus/bor/heimdall/span/spanner.go

@@ -29,6 +29,13 @@ type ChainSpanner struct {
 	validatorContractAddress common.Address
 }
 
+// validator response on ValidatorSet contract
+type contractValidator struct {
+	Id     *big.Int
+	Power  *big.Int
+	Signer common.Address
+}
+
 func NewChainSpanner(ethAPI api.Caller, validatorSet abi.ABI, chainConfig *params.ChainConfig, validatorContractAddress common.Address) *ChainSpanner {
 	return &ChainSpanner{
 		ethAPI:                   ethAPI,
@@ -93,6 +100,139 @@ func (c *ChainSpanner) GetCurrentValidatorsByBlockNrOrHash(ctx context.Context,
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
+	toAddress := c.validatorContractAddress
+	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
+
+	valz, err := c.tryGetBorValidatorsWithId(ctx, blockNrOrHash, blockNumber, toAddress, gas)
+	if err != nil {
+		return nil, err
+	}
+
+	return valz, nil
+}
+
+// tryGetBorValidatorsWithId Try to get bor validators with Id from ValidatorSet contract by querying each element on mapping(uint256 => Validator[]) public producers
+// If fails then returns GetBorValidators without id
+func (c *ChainSpanner) tryGetBorValidatorsWithId(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, blockNumber uint64, toAddress common.Address, gas hexutil.Uint64) ([]*valset.Validator, error) {
+	firstEndBlock, err := c.getFirstEndBlock(ctx, blockNrOrHash, toAddress, gas)
+	if err != nil {
+		return nil, err
+	}
+	var spanNumber *big.Int
+	if big.NewInt(int64(blockNumber)).Cmp(firstEndBlock) <= 0 {
+		spanNumber = big.NewInt(0)
+	} else {
+		spanNumber, err = c.getSpanByBlock(ctx, blockNrOrHash, blockNumber, toAddress, gas)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	borValidatorsWithoutId, err := c.getBorValidatorsWithoutId(ctx, blockNrOrHash, blockNumber, toAddress, gas)
+	if err != nil {
+		return nil, err
+	}
+
+	producersCount := len(borValidatorsWithoutId)
+
+	valz := make([]*valset.Validator, producersCount)
+
+	for i := 0; i < producersCount; i++ {
+		p, err := c.getProducersBySpanAndIndexMethod(ctx, blockNrOrHash, toAddress, gas, spanNumber, i)
+		// if fails, return validators without id
+		if err != nil {
+			return borValidatorsWithoutId, nil
+		}
+
+		valz[i] = &valset.Validator{
+			ID:          p.Id.Uint64(),
+			Address:     p.Signer,
+			VotingPower: p.Power.Int64(),
+		}
+	}
+
+	return valz, nil
+}
+
+func (c *ChainSpanner) getSpanByBlock(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, blockNumber uint64, toAddress common.Address, gas hexutil.Uint64) (*big.Int, error) {
+	const getSpanByBlockMethod = "getSpanByBlock"
+	spanData, err := c.validatorSet.Pack(getSpanByBlockMethod, big.NewInt(0).SetUint64(blockNumber))
+	if err != nil {
+		log.Error("Unable to pack tx for getSpanByBlock", "error", err)
+		return nil, err
+	}
+
+	spanMsgData := (hexutil.Bytes)(spanData)
+
+	spanResult, err := c.ethAPI.Call(ctx, ethapi.TransactionArgs{
+		Gas:  &gas,
+		To:   &toAddress,
+		Data: &spanMsgData,
+	}, &blockNrOrHash, nil, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	var spanNumber *big.Int
+	if err := c.validatorSet.UnpackIntoInterface(&spanNumber, getSpanByBlockMethod, spanResult); err != nil {
+		return nil, err
+	}
+	return spanNumber, nil
+}
+
+func (c *ChainSpanner) getProducersBySpanAndIndexMethod(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, toAddress common.Address, gas hexutil.Uint64, spanNumber *big.Int, index int) (*contractValidator, error) {
+	const getProducersBySpanAndIndexMethod = "producers"
+	producerData, err := c.validatorSet.Pack(getProducersBySpanAndIndexMethod, spanNumber, big.NewInt(int64(index)))
+	if err != nil {
+		log.Error("Unable to pack tx for producers", "error", err)
+		return nil, err
+	}
+
+	producerMsgData := (hexutil.Bytes)(producerData)
+
+	result, err := c.ethAPI.Call(ctx, ethapi.TransactionArgs{
+		Gas:  &gas,
+		To:   &toAddress,
+		Data: &producerMsgData,
+	}, &blockNrOrHash, nil, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	var producer contractValidator
+	if err := c.validatorSet.UnpackIntoInterface(&producer, getProducersBySpanAndIndexMethod, result); err != nil {
+		return nil, err
+	}
+	return &producer, nil
+}
+
+func (c *ChainSpanner) getFirstEndBlock(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, toAddress common.Address, gas hexutil.Uint64) (*big.Int, error) {
+	const getFirstEndBlockMethod = "FIRST_END_BLOCK"
+	firstEndBlockData, err := c.validatorSet.Pack(getFirstEndBlockMethod)
+	if err != nil {
+		log.Error("Unable to pack tx for getFirstEndBlock", "error", err)
+		return nil, err
+	}
+
+	firstEndBlockMsgData := (hexutil.Bytes)(firstEndBlockData)
+
+	firstEndBlockResult, err := c.ethAPI.Call(ctx, ethapi.TransactionArgs{
+		Gas:  &gas,
+		To:   &toAddress,
+		Data: &firstEndBlockMsgData,
+	}, &blockNrOrHash, nil, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	var firstEndBlockNumber *big.Int
+	if err := c.validatorSet.UnpackIntoInterface(&firstEndBlockNumber, getFirstEndBlockMethod, firstEndBlockResult); err != nil {
+		return nil, err
+	}
+	return firstEndBlockNumber, nil
+}
+
+func (c *ChainSpanner) getBorValidatorsWithoutId(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, blockNumber uint64, toAddress common.Address, gas hexutil.Uint64) ([]*valset.Validator, error) {
 	// method
 	const method = "getBorValidators"
 
@@ -104,8 +244,6 @@ func (c *ChainSpanner) GetCurrentValidatorsByBlockNrOrHash(ctx context.Context,
 
 	// call
 	msgData := (hexutil.Bytes)(data)
-	toAddress := c.validatorContractAddress
-	gas := (hexutil.Uint64)(uint64(math.MaxUint64 / 2))
 
 	result, err := c.ethAPI.Call(ctx, ethapi.TransactionArgs{
 		Gas:  &gas,

consensus/bor/heimdall/span/spanner_test.go

@@ -0,0 +1,204 @@
+package span
+
+import (
+	"context"
+	"fmt"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/consensus/bor/abi"
+	"github.com/ethereum/go-ethereum/consensus/bor/api"
+	"github.com/ethereum/go-ethereum/consensus/bor/valset"
+	"github.com/ethereum/go-ethereum/params"
+	"github.com/ethereum/go-ethereum/rpc"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetCurrentValidatorsByBlockNrOrHash(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	chainConfig := &params.ChainConfig{}
+	validatorContractAddress := common.HexToAddress("0x1234567890123456789012345678901234567890")
+
+	testCases := []struct {
+		name               string
+		blockNumber        uint64
+		mockEthAPIExpected func(*api.MockCaller)
+		mockAbiExpected    func(*abi.MockABI)
+		expectedValidators []*valset.Validator
+		expectError        bool
+	}{
+		{
+			name:        "Successful retrieval of validators",
+			blockNumber: 1000,
+			mockEthAPIExpected: func(mockCaller *api.MockCaller) {
+				mockCaller.EXPECT().Call(
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+				).Return(common.FromHex("0x0000000000000000000000000000000000000000000000000000000000000000"), nil).AnyTimes()
+			},
+			mockAbiExpected: func(mockAbi *abi.MockABI) {
+				basicMocks(mockAbi)
+
+				callCount := 0
+				mockAbi.EXPECT().UnpackIntoInterface(
+					gomock.Any(),
+					gomock.Eq("producers"),
+					gomock.Any(),
+				).DoAndReturn(func(v interface{}, name string, data []byte) error {
+					defer func() { callCount++ }()
+
+					resp, _ := v.(*contractValidator)
+
+					if callCount == 0 {
+						*resp = contractValidator{
+							Id:     big.NewInt(1),
+							Signer: common.HexToAddress("0x1111111111111111111111111111111111111111"),
+							Power:  big.NewInt(10),
+						}
+					}
+					if callCount == 1 {
+						*resp = contractValidator{
+							Id:     big.NewInt(2),
+							Signer: common.HexToAddress("0x2222222222222222222222222222222222222222"),
+							Power:  big.NewInt(15),
+						}
+					}
+					return nil
+				}).AnyTimes()
+			},
+			expectedValidators: []*valset.Validator{
+				{
+					ID:          1,
+					Address:     common.HexToAddress("0x1111111111111111111111111111111111111111"),
+					VotingPower: 10,
+				},
+				{
+					ID:          2,
+					Address:     common.HexToAddress("0x2222222222222222222222222222222222222222"),
+					VotingPower: 15,
+				},
+			},
+			expectError: false,
+		},
+		{
+			name:        "Successful retrieval of validators without id",
+			blockNumber: 1000,
+			mockEthAPIExpected: func(mockCaller *api.MockCaller) {
+				mockCaller.EXPECT().Call(
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+					gomock.Any(),
+				).Return(common.FromHex("0x0000000000000000000000000000000000000000000000000000000000000000"), nil).AnyTimes()
+			},
+			mockAbiExpected: func(mockAbi *abi.MockABI) {
+				basicMocks(mockAbi)
+
+				mockAbi.EXPECT().UnpackIntoInterface(
+					gomock.Any(),
+					gomock.Eq("producers"),
+					gomock.Any(),
+				).DoAndReturn(func(v interface{}, name string, data []byte) error {
+					return fmt.Errorf("failed")
+				}).AnyTimes()
+			},
+			expectedValidators: []*valset.Validator{
+				{
+					ID:          0,
+					Address:     common.HexToAddress("0x1111111111111111111111111111111111111111"),
+					VotingPower: 10,
+				},
+				{
+					ID:          0,
+					Address:     common.HexToAddress("0x2222222222222222222222222222222222222222"),
+					VotingPower: 15,
+				},
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			mockEthAPI := api.NewMockCaller(ctrl)
+			mockValidatorSetABI := abi.NewMockABI(ctrl)
+
+			// Setup
+			chainSpanner := NewChainSpanner(
+				mockEthAPI,
+				mockValidatorSetABI,
+				chainConfig,
+				validatorContractAddress,
+			)
+
+			// Set up mock expectations
+			tc.mockEthAPIExpected(mockEthAPI)
+			tc.mockAbiExpected(mockValidatorSetABI)
+
+			blockNumber := rpc.BlockNumber(tc.blockNumber)
+			blockNrOrHash := rpc.BlockNumberOrHashWithNumber(blockNumber)
+
+			// Execute method
+			validators, err := chainSpanner.GetCurrentValidatorsByBlockNrOrHash(context.Background(), blockNrOrHash, tc.blockNumber)
+
+			// Assertions
+			if tc.expectError {
+				assert.Error(t, err)
+				assert.Nil(t, validators)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tc.expectedValidators, validators)
+			}
+		})
+	}
+}
+
+func basicMocks(mockAbi *abi.MockABI) {
+	mockAbi.EXPECT().Pack(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(common.FromHex("0x0000000000000000000000000000000000000000000000000000000000000000"), nil).AnyTimes()
+
+	mockAbi.EXPECT().UnpackIntoInterface(
+		gomock.Any(),
+		gomock.Eq("FIRST_END_BLOCK"),
+		gomock.Any(),
+	).DoAndReturn(func(v interface{}, name string, data []byte) error {
+		resp, _ := v.(**big.Int)
+		*resp = big.NewInt(999)
+		return nil
+	}).AnyTimes()
+
+	mockAbi.EXPECT().UnpackIntoInterface(
+		gomock.Any(),
+		gomock.Eq("getSpanByBlock"),
+		gomock.Any(),
+	).DoAndReturn(func(v interface{}, name string, data []byte) error {
+		resp, _ := v.(**big.Int)
+		*resp = big.NewInt(1)
+		return nil
+	}).AnyTimes()
+
+	mockAbi.EXPECT().UnpackIntoInterface(
+		gomock.Any(),
+		gomock.Eq("getBorValidators"),
+		gomock.Any(),
+	).DoAndReturn(func(v interface{}, name string, data []byte) error {
+		resp, _ := v.(*[]interface{})
+		ret0, _ := (*resp)[0].(*[]common.Address)
+		ret1, _ := (*resp)[1].(*[]*big.Int)
+
+		*ret0 = []common.Address{common.HexToAddress("0x1111111111111111111111111111111111111111"), common.HexToAddress("0x2222222222222222222222222222222222222222")}
+		*ret1 = []*big.Int{big.NewInt(10), big.NewInt(15)}
+
+		return nil
+	}).AnyTimes()
+}

consensus/bor/snapshot.go

@@ -1,9 +1,11 @@
 package bor
 
 import (
+	"context"
 	"encoding/json"
 
 	"github.com/ethereum/go-ethereum/consensus/bor/valset"
+	"github.com/ethereum/go-ethereum/log"
 
 	lru "github.com/hashicorp/golang-lru"
 
@@ -100,7 +102,7 @@ func (s *Snapshot) copy() *Snapshot {
 	return cpy
 }
 
-func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) {
+func (s *Snapshot) apply(headers []*types.Header, c *Bor) (*Snapshot, error) {
 	// Allow passing in no headers for cleaner code
 	if len(headers) == 0 {
 		return s, nil
@@ -157,6 +159,12 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) {
 			newVals, _ := valset.ParseValidators(validatorBytes)
 			v := getUpdatedValidatorSet(snap.ValidatorSet.Copy(), newVals)
 			v.IncrementProposerPriority(1)
+
+			if v.CheckEmptyId() {
+				log.Warn("Empty id found on validator set. Querying on the validatorSet contract")
+				valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), number+1)
+				v.IncludeIds(valsWithId)
+			}
 			snap.ValidatorSet = v
 		}
 	}

consensus/bor/valset/validator_set.go

@@ -110,6 +110,36 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) {
 	vals.Proposer = proposer
 }
 
+// IncludeIds include the proper Id of each validator by getting the id from
+// validator queried on ValidatorSet contract
+func (vals *ValidatorSet) IncludeIds(valsWithId []*Validator) {
+	if vals.IsNilOrEmpty() {
+		log.Warn("Empty validator set")
+	}
+
+	addressToId := make(map[common.Address]uint64)
+
+	for _, val := range valsWithId {
+		addressToId[val.Address] = val.ID
+	}
+
+	for _, val := range vals.Validators {
+		val.ID = addressToId[val.Address]
+	}
+}
+
+// CheckEmptyId checks if any validator in the ValidatorSet has an empty ID (ID == 0).
+// Returns true if at least one validator has an empty ID.
+// Returns false if all validators have non-zero IDs or if the ValidatorSet is empty.
+func (vals *ValidatorSet) CheckEmptyId() bool {
+	for _, val := range vals.Validators {
+		if val.ID == 0 {
+			return true
+		}
+	}
+	return false
+}
+
 func (vals *ValidatorSet) RescalePriorities(diffMax int64) {
 	if vals.IsNilOrEmpty() {
 		panic("empty validator set")

consensus/bor/valset/validator_set_test.go

@@ -197,3 +197,119 @@ func TestUpdateWithChangeSet(t *testing.T) {
 	_, updatedTempVal := valSet.GetByAddress(tempVal.Address)
 	require.Equal(t, int64(250), updatedTempVal.VotingPower)
 }
+
+func TestValidatorSet_IncludeIds(t *testing.T) {
+	v1 := &Validator{
+		Address:          common.HexToAddress("0x1111111111111111111111111111111111111111"),
+		VotingPower:      100,
+		ProposerPriority: 0,
+		ID:               0,
+	}
+	v2 := &Validator{
+		Address:          common.HexToAddress("0x2222222222222222222222222222222222222222"),
+		VotingPower:      200,
+		ProposerPriority: 0,
+		ID:               0,
+	}
+
+	valSet := NewValidatorSet([]*Validator{v1, v2})
+
+	valsWithId := []*Validator{
+		{
+			Address:          v1.Address,
+			ID:               10, // new ID for v1
+			VotingPower:      999,
+			ProposerPriority: 999,
+		},
+		{
+			Address:          v2.Address,
+			ID:               20, // new ID for v2
+			VotingPower:      999,
+			ProposerPriority: 999,
+		},
+		{
+			Address:     common.HexToAddress("0x3333333333333333333333333333333333333333"),
+			ID:          30,
+			VotingPower: 300,
+		},
+	}
+
+	valSet.IncludeIds(valsWithId)
+
+	assert.Equal(t, uint64(10), valSet.Validators[0].ID, "v1 ID should be updated to 10")
+	assert.Equal(t, uint64(20), valSet.Validators[1].ID, "v2 ID should be updated to 20")
+
+	assert.Equal(t, 2, len(valSet.Validators), "No extra validators should be added")
+
+	assert.Equal(t, int64(100), valSet.Validators[0].VotingPower)
+	assert.Equal(t, int64(200), valSet.Validators[1].VotingPower)
+}
+
+func TestValidatorSet_IncludeIds_EmptySet(t *testing.T) {
+	valSet := NewValidatorSet(nil) // empty set
+
+	valSet.IncludeIds([]*Validator{
+		{
+			Address: common.HexToAddress("0xabcdef"),
+			ID:      42,
+		},
+	})
+
+	assert.Equal(t, 0, valSet.Size(), "ValidatorSet remains empty")
+}
+
+func TestCheckEmptyId(t *testing.T) {
+	tests := []struct {
+		name         string
+		validatorSet ValidatorSet
+		expected     bool
+	}{
+		{
+			name:         "Empty ValidatorSet",
+			validatorSet: ValidatorSet{Validators: []*Validator{}},
+			expected:     false,
+		},
+		{
+			name: "All Validators with Non-Zero IDs",
+			validatorSet: ValidatorSet{
+				Validators: []*Validator{
+					{ID: 1},
+					{ID: 2},
+					{ID: 3},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "One Validator with ID Zero",
+			validatorSet: ValidatorSet{
+				Validators: []*Validator{
+					{ID: 0},
+					{ID: 2},
+					{ID: 3},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "All Validators with ID Zero",
+			validatorSet: ValidatorSet{
+				Validators: []*Validator{
+					{ID: 0},
+					{ID: 0},
+					{ID: 0},
+				},
+			},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.validatorSet.CheckEmptyId()
+			if result != tt.expected {
+				t.Errorf("expected %v, got %v", tt.expected, result)
+			}
+		})
+	}
+}