From 72cbafbd16ce577cdbdc308234b9edc2733a44b3 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Fri, 24 Jan 2025 15:45:46 -0300 Subject: [PATCH 1/6] apply validator id from validatorSet contract --- consensus/bor/bor.go | 2 +- consensus/bor/heimdall/span/spanner.go | 144 ++++++++++++++++++++++++- consensus/bor/snapshot.go | 6 +- consensus/bor/valset/validator_set.go | 18 ++++ 4 files changed, 166 insertions(+), 4 deletions(-) diff --git a/consensus/bor/bor.go b/consensus/bor/bor.go index 829a5f67c4..0102a4a52d 100644 --- a/consensus/bor/bor.go +++ b/consensus/bor/bor.go @@ -620,7 +620,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 } diff --git a/consensus/bor/heimdall/span/spanner.go b/consensus/bor/heimdall/span/spanner.go index d7da3549f4..5ed619d71c 100644 --- a/consensus/bor/heimdall/span/spanner.go +++ b/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,141 @@ 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 +} + +// 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, blockNumber, 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, blockNumber uint64, 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 +246,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, diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index 87e3fb163c..72c7bc1ab1 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -1,6 +1,7 @@ package bor import ( + "context" "encoding/json" "github.com/ethereum/go-ethereum/consensus/bor/valset" @@ -100,7 +101,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 +158,9 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) { newVals, _ := valset.ParseValidators(validatorBytes) v := getUpdatedValidatorSet(snap.ValidatorSet.Copy(), newVals) v.IncrementProposerPriority(1) + + valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), header.Number.Uint64()) + v.IncludeIds(valsWithId) snap.ValidatorSet = v } } diff --git a/consensus/bor/valset/validator_set.go b/consensus/bor/valset/validator_set.go index b301470e56..7107679117 100644 --- a/consensus/bor/valset/validator_set.go +++ b/consensus/bor/valset/validator_set.go @@ -110,6 +110,24 @@ 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() { + panic("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] + } +} + func (vals *ValidatorSet) RescalePriorities(diffMax int64) { if vals.IsNilOrEmpty() { panic("empty validator set") From 7fee5576b2218713313c28b5f1bbc72247398391 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Fri, 24 Jan 2025 16:32:58 -0300 Subject: [PATCH 2/6] fix on lint --- consensus/bor/heimdall/span/spanner.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/bor/heimdall/span/spanner.go b/consensus/bor/heimdall/span/spanner.go index 5ed619d71c..d88db215f1 100644 --- a/consensus/bor/heimdall/span/spanner.go +++ b/consensus/bor/heimdall/span/spanner.go @@ -126,7 +126,6 @@ func (c *ChainSpanner) tryGetBorValidatorsWithId(ctx context.Context, blockNrOrH if err != nil { return nil, err } - } borValidatorsWithoutId, err := c.getBorValidatorsWithoutId(ctx, blockNrOrHash, blockNumber, toAddress, gas) @@ -234,7 +233,6 @@ func (c *ChainSpanner) getFirstEndBlock(ctx context.Context, blockNrOrHash rpc.B } 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" From 8255ce1e1f5184a534392bc185813448c750607a Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 27 Jan 2025 04:37:37 -0300 Subject: [PATCH 3/6] unit tests on new functions --- consensus/bor/abi/{interface.go => abi.go} | 1 + consensus/bor/abi/abi_mock.go | 68 +++++++ consensus/bor/heimdall/span/spanner.go | 4 +- consensus/bor/heimdall/span/spanner_test.go | 206 ++++++++++++++++++++ consensus/bor/valset/validator_set.go | 2 +- consensus/bor/valset/validator_set_test.go | 60 ++++++ 6 files changed, 338 insertions(+), 3 deletions(-) rename consensus/bor/abi/{interface.go => abi.go} (70%) create mode 100644 consensus/bor/abi/abi_mock.go create mode 100644 consensus/bor/heimdall/span/spanner_test.go diff --git a/consensus/bor/abi/interface.go b/consensus/bor/abi/abi.go similarity index 70% rename from consensus/bor/abi/interface.go rename to consensus/bor/abi/abi.go index bb05bf0b23..461bd81ac5 100644 --- a/consensus/bor/abi/interface.go +++ b/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 diff --git a/consensus/bor/abi/abi_mock.go b/consensus/bor/abi/abi_mock.go new file mode 100644 index 0000000000..ff163f51fa --- /dev/null +++ b/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) +} diff --git a/consensus/bor/heimdall/span/spanner.go b/consensus/bor/heimdall/span/spanner.go index d88db215f1..7734d016f3 100644 --- a/consensus/bor/heimdall/span/spanner.go +++ b/consensus/bor/heimdall/span/spanner.go @@ -138,7 +138,7 @@ func (c *ChainSpanner) tryGetBorValidatorsWithId(ctx context.Context, blockNrOrH valz := make([]*valset.Validator, producersCount) for i := 0; i < producersCount; i++ { - p, err := c.getProducersBySpanAndIndexMethod(ctx, blockNrOrHash, blockNumber, toAddress, gas, spanNumber, i) + p, err := c.getProducersBySpanAndIndexMethod(ctx, blockNrOrHash, toAddress, gas, spanNumber, i) // if fails, return validators without id if err != nil { return borValidatorsWithoutId, nil @@ -180,7 +180,7 @@ func (c *ChainSpanner) getSpanByBlock(ctx context.Context, blockNrOrHash rpc.Blo return spanNumber, nil } -func (c *ChainSpanner) getProducersBySpanAndIndexMethod(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash, blockNumber uint64, toAddress common.Address, gas hexutil.Uint64, spanNumber *big.Int, index int) (*contractValidator, error) { +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 { diff --git a/consensus/bor/heimdall/span/spanner_test.go b/consensus/bor/heimdall/span/spanner_test.go new file mode 100644 index 0000000000..d4747fb172 --- /dev/null +++ b/consensus/bor/heimdall/span/spanner_test.go @@ -0,0 +1,206 @@ +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 := ¶ms.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() +} diff --git a/consensus/bor/valset/validator_set.go b/consensus/bor/valset/validator_set.go index 7107679117..5842ba7e55 100644 --- a/consensus/bor/valset/validator_set.go +++ b/consensus/bor/valset/validator_set.go @@ -114,7 +114,7 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) { // validator queried on ValidatorSet contract func (vals *ValidatorSet) IncludeIds(valsWithId []*Validator) { if vals.IsNilOrEmpty() { - panic("empty validator set") + log.Warn("Empty validator set") } addressToId := make(map[common.Address]uint64) diff --git a/consensus/bor/valset/validator_set_test.go b/consensus/bor/valset/validator_set_test.go index 9397ca3e92..5c6ff2a1b9 100644 --- a/consensus/bor/valset/validator_set_test.go +++ b/consensus/bor/valset/validator_set_test.go @@ -197,3 +197,63 @@ 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") +} From 24fe3cc8fcad7490a5872ca925dbfca55b881fdd Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 27 Jan 2025 04:43:12 -0300 Subject: [PATCH 4/6] fix lint --- consensus/bor/heimdall/span/spanner_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/consensus/bor/heimdall/span/spanner_test.go b/consensus/bor/heimdall/span/spanner_test.go index d4747fb172..8e27716f67 100644 --- a/consensus/bor/heimdall/span/spanner_test.go +++ b/consensus/bor/heimdall/span/spanner_test.go @@ -72,7 +72,6 @@ func TestGetCurrentValidatorsByBlockNrOrHash(t *testing.T) { } return nil }).AnyTimes() - }, expectedValidators: []*valset.Validator{ { @@ -110,7 +109,6 @@ func TestGetCurrentValidatorsByBlockNrOrHash(t *testing.T) { ).DoAndReturn(func(v interface{}, name string, data []byte) error { return fmt.Errorf("failed") }).AnyTimes() - }, expectedValidators: []*valset.Validator{ { From a01c1802469082f700bd621f195b7d96f676cd62 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 27 Jan 2025 09:13:35 -0300 Subject: [PATCH 5/6] check empty ids before querying validator set contract --- consensus/bor/heimdall/span/spanner.go | 2 +- consensus/bor/snapshot.go | 8 +++- consensus/bor/valset/validator_set.go | 12 +++++ consensus/bor/valset/validator_set_test.go | 56 ++++++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/consensus/bor/heimdall/span/spanner.go b/consensus/bor/heimdall/span/spanner.go index 7734d016f3..99c361f044 100644 --- a/consensus/bor/heimdall/span/spanner.go +++ b/consensus/bor/heimdall/span/spanner.go @@ -111,7 +111,7 @@ func (c *ChainSpanner) GetCurrentValidatorsByBlockNrOrHash(ctx context.Context, return valz, nil } -// Try to get bor validators with Id from ValidatorSet contract by querying each element on mapping(uint256 => Validator[]) public producers +// 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) diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index 72c7bc1ab1..ae383cbf6f 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -5,6 +5,7 @@ import ( "encoding/json" "github.com/ethereum/go-ethereum/consensus/bor/valset" + "github.com/ethereum/go-ethereum/log" lru "github.com/hashicorp/golang-lru" @@ -159,8 +160,11 @@ func (s *Snapshot) apply(headers []*types.Header, c *Bor) (*Snapshot, error) { v := getUpdatedValidatorSet(snap.ValidatorSet.Copy(), newVals) v.IncrementProposerPriority(1) - valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), header.Number.Uint64()) - v.IncludeIds(valsWithId) + if v.CheckEmptyId() { + log.Warn("Empty id found on validator set. Querying on the validatorSet contract") + valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), header.Number.Uint64()) + v.IncludeIds(valsWithId) + } snap.ValidatorSet = v } } diff --git a/consensus/bor/valset/validator_set.go b/consensus/bor/valset/validator_set.go index 5842ba7e55..400a5ff8fb 100644 --- a/consensus/bor/valset/validator_set.go +++ b/consensus/bor/valset/validator_set.go @@ -128,6 +128,18 @@ func (vals *ValidatorSet) IncludeIds(valsWithId []*Validator) { } } +// 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") diff --git a/consensus/bor/valset/validator_set_test.go b/consensus/bor/valset/validator_set_test.go index 5c6ff2a1b9..f4e30eed8d 100644 --- a/consensus/bor/valset/validator_set_test.go +++ b/consensus/bor/valset/validator_set_test.go @@ -257,3 +257,59 @@ func TestValidatorSet_IncludeIds_EmptySet(t *testing.T) { 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) + } + }) + } +} From 1c8e610b3f799169c4179474b80358b6119bee8f Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 27 Jan 2025 09:48:26 -0300 Subject: [PATCH 6/6] querying the start block of next span --- consensus/bor/snapshot.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/bor/snapshot.go b/consensus/bor/snapshot.go index ae383cbf6f..796a91ac39 100644 --- a/consensus/bor/snapshot.go +++ b/consensus/bor/snapshot.go @@ -162,7 +162,7 @@ func (s *Snapshot) apply(headers []*types.Header, c *Bor) (*Snapshot, error) { if v.CheckEmptyId() { log.Warn("Empty id found on validator set. Querying on the validatorSet contract") - valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), header.Number.Uint64()) + valsWithId, _ := c.spanner.GetCurrentValidatorsByHash(context.Background(), header.Hash(), number+1) v.IncludeIds(valsWithId) } snap.ValidatorSet = v