Skip to content

Commit

Permalink
cr fix
Browse files Browse the repository at this point in the history
  • Loading branch information
igorcrevar committed May 4, 2023
1 parent ba933ad commit e525f9c
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 41 deletions.
10 changes: 5 additions & 5 deletions consensus/polybft/stake_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (s *stakeManager) PostBlock(req *PostBlockRequest) error {
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")
s.logger.Warn("Found a transfer event that represents neither stake nor unstake")
}
}

Expand Down Expand Up @@ -166,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))

Expand Down Expand Up @@ -354,8 +354,8 @@ func (sc *validatorStakeMap) removeStake(address types.Address, amount *big.Int)
stakeData.IsActive = stakeData.VotingPower.Cmp(bigZero) > 0
}

// getActiveValidators returns all validators (*ValidatorMetadata) in sorted order
func (sc validatorStakeMap) getActiveValidators(maxValidatorSetSize int) AccountSet {
// getSorted returns all validators (*ValidatorMetadata) in sorted order
func (sc validatorStakeMap) getSorted(maxValidatorSetSize int) AccountSet {
activeValidators := make(AccountSet, 0, len(sc))

for _, v := range sc {
Expand Down Expand Up @@ -387,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())
Expand Down
40 changes: 4 additions & 36 deletions consensus/polybft/stake_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ 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) {
Expand Down Expand Up @@ -65,11 +64,11 @@ func TestStakeManager_PostBlock(t *testing.T) {
secondValidator = uint64(1)
)

validators := newTestValidatorsWithAliases(t, allAliases)
state := newTestState(t)
t.Run("PostBlock - unstake to zero", func(t *testing.T) {
t.Parallel()

validators := newTestValidatorsWithAliases(t, allAliases)
stakeManager := newStakeManager(
hclog.NewNullLogger(),
state,
Expand Down Expand Up @@ -123,6 +122,7 @@ func TestStakeManager_PostBlock(t *testing.T) {
t.Run("PostBlock - add stake to one validator", func(t *testing.T) {
t.Parallel()

validators := newTestValidatorsWithAliases(t, allAliases)
stakeManager := newStakeManager(
hclog.NewNullLogger(),
state,
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestStakeManager_PostBlock(t *testing.T) {
require.Len(t, fullValidatorSet.Validators, len(allAliases))

validatorsCount := validators.toValidatorSet().Len()
for i, v := range fullValidatorSet.Validators.getActiveValidators(validatorsCount) {
for i, v := range fullValidatorSet.Validators.getSorted(validatorsCount) {
require.Equal(t, newStake+uint64(validatorsCount)-uint64(i)-1, v.VotingPower.Uint64())
}
})
Expand Down Expand Up @@ -392,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()
Expand Down Expand Up @@ -484,35 +484,3 @@ 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)])
}
33 changes: 33 additions & 0 deletions types/rlp_encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/umbracle/fastrlp"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type codec interface {
Expand Down Expand Up @@ -357,3 +358,35 @@ func testRLPData(arena *fastrlp.Arena, omitValues map[string]bool) []byte {

return testData
}

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)])
}

0 comments on commit e525f9c

Please sign in to comment.