different stake in stake manager handling + marshal fix + additional …

…logs

Makefile

@@ -59,7 +59,9 @@ test-e2e-polybft:
     # We can not build with race because of a bug in boltdb dependency
 	go build -o artifacts/polygon-edge .
 	env EDGE_BINARY=${PWD}/artifacts/polygon-edge E2E_TESTS=true E2E_LOGS=true \
-	go test -v -timeout=1h30m ./e2e-polybft/e2e/...
+		go test -v -timeout 1h -run TestE2E_Bridge_ChangeVotingPower github.com/0xPolygon/polygon-edge/e2e-polybft/e2e
+	env EDGE_BINARY=${PWD}/artifacts/polygon-edge E2E_TESTS=true E2E_LOGS=true \
+		go test -v -timeout 1h -run TestE2E_Consensus_RegisterValidator github.com/0xPolygon/polygon-edge/e2e-polybft/e2e
 
 .PHONY: test-property-polybft
 test-property-polybft:

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/fsm_test.go

@@ -1107,7 +1107,7 @@ func TestFSM_Insert_InvalidNode(t *testing.T) {
 	validatorSet := NewValidatorSet(validatorsMetadata[0:len(validatorsMetadata)-1], hclog.NewNullLogger())
 
 	fsm := &fsm{parent: parent, blockBuilder: mBlockBuilder, backend: &blockchainMock{},
-		validators: validatorSet,
+		validators: validatorSet, logger: hclog.NewNullLogger(),
 	}
 
 	proposal := buildBlock.Block.MarshalRLP()

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,
@@ -109,45 +106,32 @@ func (s *stakeManager) PostBlock(req *PostBlockRequest) error {
 	s.logger.Debug("Gotten transfer (stake changed) events from logs on block",
 		"eventsNum", len(events), "block", req.FullBlock.Block.Number())
 
+	for _, e := range events {
+		s.logger.Debug("Transfer event", "From", e.From, "To", e.To, "Value", e.Value)
+	}
+
 	fullValidatorSet, err := s.state.StakeStore.getFullValidatorSet()
 	if err != nil {
 		return err
 	}
 
 	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{}{}
+			// then this amount was minted To validator address
+			stakeMap.addStake(event.To, event.Value)
 		} else if event.IsUnstake() {
-			updatedValidatorsStake[event.From] = struct{}{}
+			// 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.Debug("Found a transfer event that represents neither stake nor unstake")
 		}
 	}
 
-	// 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)
-			}
-
-			stakeMap.setStake(a, stake)
-		}
-	}
-
 	for addr, data := range stakeMap {
 		if data.BlsKey == nil {
 			data.BlsKey, err = s.getBlsKey(data.Address)
@@ -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(),
@@ -256,6 +242,8 @@ func (s *stakeManager) getTransferEventsFromReceipts(receipts []*types.Receipt)
 
 			var transferEvent contractsapi.TransferEvent
 
+			s.logger.Debug("Transfer event log before parse", "Log", log.Data)
+
 			convertedLog := convertLog(log)
 
 			doesMatch, err := transferEvent.ParseLog(convertedLog)
@@ -267,6 +255,8 @@ func (s *stakeManager) getTransferEventsFromReceipts(receipts []*types.Receipt)
 				continue
 			}
 
+			s.logger.Debug("Transfer event log after parse", "Log", log.Data)
+
 			events = append(events, &transferEvent)
 		}
 	}
@@ -353,22 +343,27 @@ 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,
 		}
 	}
 }
 
+// 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
+}
+
 // getActiveValidators returns all validators (*ValidatorMetadata) in sorted order
 func (sc validatorStakeMap) getActiveValidators(maxValidatorSetSize int) AccountSet {
 	activeValidators := make(AccountSet, 0, len(sc))

consensus/polybft/stake_manager_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/umbracle/ethgo"
 	"github.com/umbracle/ethgo/abi"
 	"github.com/umbracle/ethgo/jsonrpc"
+	"github.com/umbracle/fastrlp"
 )
 
 func TestStakeManager_PostEpoch(t *testing.T) {
@@ -60,7 +61,6 @@ func TestStakeManager_PostBlock(t *testing.T) {
 		epoch             = uint64(1)
 		block             = uint64(10)
 		newStake          = uint64(100)
-		zeroStake         = uint64(0)
 		firstValidator    = uint64(0)
 		secondValidator   = uint64(1)
 	)
@@ -69,17 +69,10 @@ func TestStakeManager_PostBlock(t *testing.T) {
 	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)
 
 		stakeManager := newStakeManager(
 			hclog.NewNullLogger(),
 			state,
-			blockchainMock,
 			nil,
 			wallet.NewEcdsaSigner(validators.getValidator("A").Key()),
 			types.StringToAddress("0x0001"), types.StringToAddress("0x0002"),
@@ -98,7 +91,7 @@ func TestStakeManager_PostBlock(t *testing.T) {
 					stakeManager.validatorSetContract,
 					validators.getValidator(initialSetAliases[firstValidator]).Address(),
 					types.ZeroAddress,
-					zeroStake,
+					1, // remove initial 1
 				),
 			},
 		}
@@ -126,23 +119,13 @@ 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)
 
 		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.getActiveValidators(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,
@@ -512,3 +484,35 @@ func (d *dummyStakeTxRelayer) SendTransactionLocal(txn *ethgo.Transaction) (*eth
 func (d *dummyStakeTxRelayer) Client() *jsonrpc.Client {
 	return nil
 }
+
+func Test_MarshalBug(t *testing.T) {
+	t.Parallel()
+
+	marshal := func(obj func(*fastrlp.Arena) *fastrlp.Value) []byte {
+		ar := fastrlp.DefaultArenaPool.Get()
+		defer fastrlp.DefaultArenaPool.Put(ar)
+
+		return obj(ar).MarshalTo(nil)
+	}
+
+	emptyArray := [8]byte{}
+	corruptedSlice := make([]byte, 32)
+	corruptedSlice[29], corruptedSlice[30], corruptedSlice[31] = 5, 126, 64
+	intOfCorruption := uint64(18_446_744_073_709_551_615) // 2^64-1
+
+	marshalOne := func(ar *fastrlp.Arena) *fastrlp.Value {
+		return ar.NewBytes(corruptedSlice)
+	}
+
+	marshalTwo := func(ar *fastrlp.Arena) *fastrlp.Value {
+		return ar.NewUint(intOfCorruption)
+	}
+
+	marshal(marshalOne)
+
+	require.Equal(t, emptyArray[:], corruptedSlice[:len(emptyArray)])
+
+	marshal(marshalTwo) // without fixing this, marshaling will cause corruption of the corrupted slice
+
+	require.NotEqual(t, emptyArray[:], corruptedSlice[:len(emptyArray)])
+}

consensus/polybft/system_state.go

@@ -1,7 +1,6 @@
 package polybft
 
 import (
-	"encoding/json"
 	"fmt"
 	"math/big"
 
@@ -100,23 +99,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
-}