Fix RLP marshal byte array and enhancements to stake manager

consensus/polybft/consensus_runtime.go

@@ -229,7 +229,6 @@ func (c *consensusRuntime) initStakeManager(logger hcf.Logger) error {
 	c.stakeManager = newStakeManager(
 		logger.Named("stake-manager"),
 		c.state,
-		c.config.blockchain,
 		rootRelayer,
 		wallet.NewEcdsaSigner(c.config.Key),
 		contracts.ValidatorSetContract,

consensus/polybft/mocks_test.go

@@ -210,12 +210,6 @@ type systemStateMock struct {
 	mock.Mock
 }
 
-func (m *systemStateMock) GetStakeOnValidatorSet(validatorAddr types.Address) (*big.Int, error) {
-	args := m.Called()
-
-	return args.Get(0).(*big.Int), args.Error(1) //nolint:forcetypeassert
-}
-
 func (m *systemStateMock) GetNextCommittedIndex() (uint64, error) {
 	args := m.Called()
 

consensus/polybft/stake_manager.go

@@ -51,7 +51,6 @@ type stakeManager struct {
 	logger                  hclog.Logger
 	state                   *State
 	rootChainRelayer        txrelayer.TxRelayer
-	blockchain              blockchainBackend
 	key                     ethgo.Key
 	validatorSetContract    types.Address
 	supernetManagerContract types.Address
@@ -62,7 +61,6 @@ type stakeManager struct {
 func newStakeManager(
 	logger hclog.Logger,
 	state *State,
-	blockchain blockchainBackend,
 	rootchainRelayer txrelayer.TxRelayer,
 	key ethgo.Key,
 	validatorSetAddr, supernetManagerAddr types.Address,
@@ -71,7 +69,6 @@ func newStakeManager(
 	return &stakeManager{
 		logger:                  logger,
 		state:                   state,
-		blockchain:              blockchain,
 		rootChainRelayer:        rootchainRelayer,
 		key:                     key,
 		validatorSetContract:    validatorSetAddr,
@@ -116,35 +113,22 @@ func (s *stakeManager) PostBlock(req *PostBlockRequest) error {
 
 	stakeMap := fullValidatorSet.Validators
 
-	updatedValidatorsStake := make(map[types.Address]struct{}, 0)
+	s.logger.Debug("full validator set before", "block", fullValidatorSet.BlockNumber, "data", stakeMap)
 
 	for _, event := range events {
 		if event.IsStake() {
-			updatedValidatorsStake[event.To] = struct{}{}
-		} else if event.IsUnstake() {
-			updatedValidatorsStake[event.From] = struct{}{}
-		} else {
-			s.logger.Debug("Found a transfer event that represents neither stake nor unstake")
-		}
-	}
+			s.logger.Debug("Stake transfer event", "To", event.To, "Value", event.Value)
 
-	// this is a temporary solution (a workaround) for a bug where amount
-	// in transfer event is not correctly generated (unknown 4 bytes are added to begging of Data array)
-	if len(updatedValidatorsStake) > 0 {
-		provider, err := s.blockchain.GetStateProviderForBlock(req.FullBlock.Block.Header)
-		if err != nil {
-			return err
-		}
-
-		systemState := s.blockchain.GetSystemState(provider)
-
-		for a := range updatedValidatorsStake {
-			stake, err := systemState.GetStakeOnValidatorSet(a)
-			if err != nil {
-				return fmt.Errorf("could not retrieve balance of validator %v on ValidatorSet", a)
-			}
+			// then this amount was minted To validator address
+			stakeMap.addStake(event.To, event.Value)
+		} else if event.IsUnstake() {
+			s.logger.Debug("Unstake transfer event", "From", event.From, "Value", event.Value)
 
-			stakeMap.setStake(a, stake)
+			// then this amount was burned From validator address
+			stakeMap.removeStake(event.From, event.Value)
+		} else {
+			// this should not happen, but lets log it if it does
+			s.logger.Warn("Found a transfer event that represents neither stake nor unstake")
 		}
 	}
 
@@ -159,6 +143,8 @@ func (s *stakeManager) PostBlock(req *PostBlockRequest) error {
 		data.IsActive = data.VotingPower.Cmp(bigZero) > 0
 	}
 
+	s.logger.Debug("full validator set after", "block", req.FullBlock.Block.Number(), "data", stakeMap)
+
 	return s.state.StakeStore.insertFullValidatorSet(validatorSetState{
 		EpochID:     req.Epoch,
 		BlockNumber: req.FullBlock.Block.Number(),
@@ -180,7 +166,7 @@ func (s *stakeManager) UpdateValidatorSet(epoch uint64, oldValidatorSet AccountS
 	stakeMap := fullValidatorSet.Validators
 
 	// slice of all validator set
-	newValidatorSet := stakeMap.getActiveValidators(s.maxValidatorSetSize)
+	newValidatorSet := stakeMap.getSorted(s.maxValidatorSetSize)
 	// set of all addresses that will be in next validator set
 	addressesSet := make(map[types.Address]struct{}, len(newValidatorSet))
 
@@ -256,9 +242,7 @@ func (s *stakeManager) getTransferEventsFromReceipts(receipts []*types.Receipt)
 
 			var transferEvent contractsapi.TransferEvent
 
-			convertedLog := convertLog(log)
-
-			doesMatch, err := transferEvent.ParseLog(convertedLog)
+			doesMatch, err := transferEvent.ParseLog(convertLog(log))
 			if err != nil {
 				return nil, err
 			}
@@ -303,10 +287,6 @@ func (s *stakeManager) getBlsKey(address types.Address) (*bls.PublicKey, error)
 		return nil, err
 	}
 
-	//nolint:godox
-	// TODO - @goran-ethernal change this to use the generated stub
-	// once we remove old ChildValidatorSet stubs and generate new ones
-	// from the new contract
 	output, ok := decoded.(map[string]interface{})
 	if !ok {
 		return nil, fmt.Errorf("could not convert decoded outputs to map")
@@ -353,24 +333,29 @@ func newValidatorStakeMap(validatorSet AccountSet) validatorStakeMap {
 	return stakeMap
 }
 
-// setStake sets given amount of stake to a validator defined by address
-func (sc *validatorStakeMap) setStake(address types.Address, amount *big.Int) {
-	isActive := amount.Cmp(bigZero) > 0
-
+// addStake adds given amount to a validator defined by address
+func (sc *validatorStakeMap) addStake(address types.Address, amount *big.Int) {
 	if metadata, exists := (*sc)[address]; exists {
-		metadata.VotingPower = amount
-		metadata.IsActive = isActive
+		metadata.VotingPower.Add(metadata.VotingPower, amount)
+		metadata.IsActive = metadata.VotingPower.Cmp(bigZero) > 0
 	} else {
 		(*sc)[address] = &ValidatorMetadata{
 			VotingPower: new(big.Int).Set(amount),
 			Address:     address,
-			IsActive:    isActive,
+			IsActive:    amount.Cmp(bigZero) > 0,
 		}
 	}
 }
 
-// getActiveValidators returns all validators (*ValidatorMetadata) in sorted order
-func (sc validatorStakeMap) getActiveValidators(maxValidatorSetSize int) AccountSet {
+// removeStake removes given amount from validator defined by address
+func (sc *validatorStakeMap) removeStake(address types.Address, amount *big.Int) {
+	stakeData := (*sc)[address]
+	stakeData.VotingPower.Sub(stakeData.VotingPower, amount)
+	stakeData.IsActive = stakeData.VotingPower.Cmp(bigZero) > 0
+}
+
+// getSorted returns validators (*ValidatorMetadata) in sorted order
+func (sc validatorStakeMap) getSorted(maxValidatorSetSize int) AccountSet {
 	activeValidators := make(AccountSet, 0, len(sc))
 
 	for _, v := range sc {
@@ -402,7 +387,7 @@ func (sc validatorStakeMap) getActiveValidators(maxValidatorSetSize int) Account
 func (sc validatorStakeMap) String() string {
 	var sb strings.Builder
 
-	for _, x := range sc {
+	for _, x := range sc.getSorted(len(sc)) {
 		bls := ""
 		if x.BlsKey != nil {
 			bls = hex.EncodeToString(x.BlsKey.Marshal())

consensus/polybft/stake_manager_test.go

@@ -60,26 +60,18 @@ func TestStakeManager_PostBlock(t *testing.T) {
 		epoch             = uint64(1)
 		block             = uint64(10)
 		newStake          = uint64(100)
-		zeroStake         = uint64(0)
 		firstValidator    = uint64(0)
 		secondValidator   = uint64(1)
 	)
 
-	validators := newTestValidatorsWithAliases(t, allAliases)
 	state := newTestState(t)
 	t.Run("PostBlock - unstake to zero", func(t *testing.T) {
 		t.Parallel()
-		systemStateMock := new(systemStateMock)
-		systemStateMock.On("GetStakeOnValidatorSet", mock.Anything).Return(big.NewInt(int64(zeroStake)), nil).Once()
-
-		blockchainMock := new(blockchainMock)
-		blockchainMock.On("GetStateProviderForBlock", mock.Anything).Return(new(stateProviderMock)).Once()
-		blockchainMock.On("GetSystemState", mock.Anything, mock.Anything).Return(systemStateMock)
 
+		validators := newTestValidatorsWithAliases(t, allAliases)
 		stakeManager := newStakeManager(
 			hclog.NewNullLogger(),
 			state,
-			blockchainMock,
 			nil,
 			wallet.NewEcdsaSigner(validators.getValidator("A").Key()),
 			types.StringToAddress("0x0001"), types.StringToAddress("0x0002"),
@@ -98,7 +90,7 @@ func TestStakeManager_PostBlock(t *testing.T) {
 					stakeManager.validatorSetContract,
 					validators.getValidator(initialSetAliases[firstValidator]).Address(),
 					types.ZeroAddress,
-					zeroStake,
+					1, // initial validator stake was 1
 				),
 			},
 		}
@@ -126,23 +118,14 @@ func TestStakeManager_PostBlock(t *testing.T) {
 		require.NotNil(t, firstValidatorMeta)
 		require.Equal(t, bigZero, firstValidatorMeta.VotingPower)
 		require.False(t, firstValidatorMeta.IsActive)
-
-		systemStateMock.AssertExpectations(t)
-		blockchainMock.AssertExpectations(t)
 	})
 	t.Run("PostBlock - add stake to one validator", func(t *testing.T) {
 		t.Parallel()
-		systemStateMock := new(systemStateMock)
-		systemStateMock.On("GetStakeOnValidatorSet", mock.Anything).Return(big.NewInt(250), nil).Once()
-
-		blockchainMock := new(blockchainMock)
-		blockchainMock.On("GetStateProviderForBlock", mock.Anything).Return(new(stateProviderMock)).Once()
-		blockchainMock.On("GetSystemState", mock.Anything, mock.Anything).Return(systemStateMock)
 
+		validators := newTestValidatorsWithAliases(t, allAliases)
 		stakeManager := newStakeManager(
 			hclog.NewNullLogger(),
 			state,
-			blockchainMock,
 			nil,
 			wallet.NewEcdsaSigner(validators.getValidator("A").Key()),
 			types.StringToAddress("0x0001"), types.StringToAddress("0x0002"),
@@ -187,21 +170,14 @@ func TestStakeManager_PostBlock(t *testing.T) {
 			}
 		}
 		require.NotNil(t, firstValidaotor)
-		require.Equal(t, big.NewInt(250), firstValidaotor.VotingPower)
+		require.Equal(t, big.NewInt(251), firstValidaotor.VotingPower) // 250 + initial 1
 		require.True(t, firstValidaotor.IsActive)
-
-		systemStateMock.AssertExpectations(t)
-		blockchainMock.AssertExpectations(t)
 	})
 
 	t.Run("PostBlock - add validator and stake", func(t *testing.T) {
 		t.Parallel()
-		systemStateMock := new(systemStateMock)
-		systemStateMock.On("GetStakeOnValidatorSet", mock.Anything).Return(big.NewInt(int64(newStake)), nil).Times(len(allAliases))
 
-		blockchainMock := new(blockchainMock)
-		blockchainMock.On("GetStateProviderForBlock", mock.Anything).Return(new(stateProviderMock)).Once()
-		blockchainMock.On("GetSystemState", mock.Anything, mock.Anything).Return(systemStateMock)
+		validators := newTestValidatorsWithAliases(t, allAliases, []uint64{1, 2, 3, 4, 5, 6})
 
 		txRelayerMock := newDummyStakeTxRelayer(t, func() *ValidatorMetadata {
 			return validators.getValidator("F").ValidatorMetadata()
@@ -214,7 +190,6 @@ func TestStakeManager_PostBlock(t *testing.T) {
 		stakeManager := newStakeManager(
 			hclog.NewNullLogger(),
 			state,
-			blockchainMock,
 			txRelayerMock,
 			wallet.NewEcdsaSigner(validators.getValidator("A").Key()),
 			types.StringToAddress("0x0001"), types.StringToAddress("0x0002"),
@@ -252,12 +227,10 @@ func TestStakeManager_PostBlock(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, fullValidatorSet.Validators, len(allAliases))
 
-		for _, v := range fullValidatorSet.Validators {
-			require.Equal(t, newStake, v.VotingPower.Uint64())
+		validatorsCount := validators.toValidatorSet().Len()
+		for i, v := range fullValidatorSet.Validators.getSorted(validatorsCount) {
+			require.Equal(t, newStake+uint64(validatorsCount)-uint64(i)-1, v.VotingPower.Uint64())
 		}
-
-		systemStateMock.AssertExpectations(t)
-		blockchainMock.AssertExpectations(t)
 	})
 }
 
@@ -275,7 +248,6 @@ func TestStakeManager_UpdateValidatorSet(t *testing.T) {
 		hclog.NewNullLogger(),
 		state,
 		nil,
-		nil,
 		wallet.NewEcdsaSigner(validators.getValidator("A").Key()),
 		types.StringToAddress("0x0001"), types.StringToAddress("0x0002"),
 		10,
@@ -420,7 +392,7 @@ func TestStakeCounter_ShouldBeDeterministic(t *testing.T) {
 		test := func() []*ValidatorMetadata {
 			stakeCounter := newValidatorStakeMap(validators.getPublicIdentities("A", "B", "C", "D", "E"))
 
-			return stakeCounter.getActiveValidators(maxValidatorSetSize)
+			return stakeCounter.getSorted(maxValidatorSetSize)
 		}
 
 		initialSlice := test()

consensus/polybft/system_state.go

@@ -1,7 +1,6 @@
 package polybft
 
 import (
-	"encoding/json"
 	"fmt"
 	"math/big"
 
@@ -27,8 +26,6 @@ type SystemState interface {
 	GetEpoch() (uint64, error)
 	// GetNextCommittedIndex retrieves next committed bridge state sync index
 	GetNextCommittedIndex() (uint64, error)
-	// GetStakeOnValidatorSet retrieves stake of given validator on ValidatorSet contract
-	GetStakeOnValidatorSet(validatorAddr types.Address) (*big.Int, error)
 }
 
 var _ SystemState = &SystemStateImpl{}
@@ -55,21 +52,6 @@ func NewSystemState(valSetAddr types.Address, stateRcvAddr types.Address, provid
 	return s
 }
 
-// GetStakeOnValidatorSet retrieves stake of given validator on ValidatorSet contract
-func (s *SystemStateImpl) GetStakeOnValidatorSet(validatorAddr types.Address) (*big.Int, error) {
-	rawResult, err := s.validatorContract.Call("balanceOf", ethgo.Latest, validatorAddr)
-	if err != nil {
-		return nil, err
-	}
-
-	balance, isOk := rawResult["0"].(*big.Int)
-	if !isOk {
-		return nil, fmt.Errorf("failed to decode balance")
-	}
-
-	return balance, nil
-}
-
 // GetEpoch retrieves current epoch number from the smart contract
 func (s *SystemStateImpl) GetEpoch() (uint64, error) {
 	rawResult, err := s.validatorContract.Call("currentEpochId", ethgo.Latest)
@@ -100,23 +82,3 @@ func (s *SystemStateImpl) GetNextCommittedIndex() (uint64, error) {
 
 	return nextCommittedIndex.Uint64() + 1, nil
 }
-
-func buildLogsFromReceipts(entry []*types.Receipt, header *types.Header) []*types.Log {
-	var logs []*types.Log
-
-	for _, taskReceipt := range entry {
-		for _, taskLog := range taskReceipt.Logs {
-			log := new(types.Log)
-			*log = *taskLog
-
-			data := map[string]interface{}{
-				"Hash":   header.Hash,
-				"Number": header.Number,
-			}
-			log.Data, _ = json.Marshal(&data)
-			logs = append(logs, log)
-		}
-	}
-
-	return logs
-}