fix: authority balance check in packer (#1288)

Author: darrenkel

builtin/authority/authority.go

@@ -6,8 +6,6 @@
 package authority
 
 import (
-	"math/big"
-
 	"github.com/ethereum/go-ethereum/rlp"
 
 	"github.com/vechain/thor/v2/state"
@@ -200,8 +198,10 @@ func (a *Authority) Update(nodeMaster thor.Address, active bool) (bool, error) {
 	return true, nil
 }
 
+type BalanceChecker func(master, endorser thor.Address) (bool, error)
+
 // Candidates picks a batch of candidates up to limit, that satisfy given endorsement.
-func (a *Authority) Candidates(endorsement *big.Int, limit uint64) ([]*Candidate, error) {
+func (a *Authority) Candidates(checker BalanceChecker, limit uint64) ([]*Candidate, error) {
 	ptr, err := a.getAddressPtr(headKey)
 	if err != nil {
 		return nil, err
@@ -212,11 +212,11 @@ func (a *Authority) Candidates(endorsement *big.Int, limit uint64) ([]*Candidate
 		if err != nil {
 			return nil, err
 		}
-		bal, err := a.state.GetBalance(entry.Endorsor)
+		ok, err := checker(*ptr, entry.Endorsor)
 		if err != nil {
 			return nil, err
 		}
-		if bal.Cmp(endorsement) >= 0 {
+		if ok {
 			candidates = append(candidates, &Candidate{
 				NodeMaster: *ptr,
 				Endorsor:   entry.Endorsor,

builtin/authority/authority_test.go

@@ -33,6 +33,17 @@ func TestAuthority(t *testing.T) {
 	st.SetBalance(p3, big.NewInt(30))
 
 	aut := New(thor.BytesToAddress([]byte("aut")), st)
+
+	checker := func(amount *big.Int) BalanceChecker {
+		return func(master, endorser thor.Address) (bool, error) {
+			bal, err := st.GetBalance(endorser)
+			if err != nil {
+				return false, err
+			}
+			return bal.Cmp(amount) >= 0, nil
+		}
+	}
+
 	tests := []struct {
 		ret      any
 		expected any
@@ -41,16 +52,16 @@ func TestAuthority(t *testing.T) {
 		{M(aut.Get(p1)), M(true, p1, thor.Bytes32{}, true, nil)},
 		{M(aut.Add(p2, p2, thor.Bytes32{})), M(true, nil)},
 		{M(aut.Add(p3, p3, thor.Bytes32{})), M(true, nil)},
-		{M(aut.Candidates(big.NewInt(10), thor.InitialMaxBlockProposers)), M(
+		{M(aut.Candidates(checker(big.NewInt(10)), thor.InitialMaxBlockProposers)), M(
 			[]*Candidate{{p1, p1, thor.Bytes32{}, true}, {p2, p2, thor.Bytes32{}, true}, {p3, p3, thor.Bytes32{}, true}}, nil,
 		)},
-		{M(aut.Candidates(big.NewInt(20), thor.InitialMaxBlockProposers)), M(
+		{M(aut.Candidates(checker(big.NewInt(20)), thor.InitialMaxBlockProposers)), M(
 			[]*Candidate{{p2, p2, thor.Bytes32{}, true}, {p3, p3, thor.Bytes32{}, true}}, nil,
 		)},
-		{M(aut.Candidates(big.NewInt(30), thor.InitialMaxBlockProposers)), M(
+		{M(aut.Candidates(checker(big.NewInt(30)), thor.InitialMaxBlockProposers)), M(
 			[]*Candidate{{p3, p3, thor.Bytes32{}, true}}, nil,
 		)},
-		{M(aut.Candidates(big.NewInt(10), 2)), M(
+		{M(aut.Candidates(checker(big.NewInt(10)), 2)), M(
 			[]*Candidate{{p1, p1, thor.Bytes32{}, true}, {p2, p2, thor.Bytes32{}, true}}, nil,
 		)},
 		{M(aut.Get(p1)), M(true, p1, thor.Bytes32{}, true, nil)},
@@ -60,7 +71,7 @@ func TestAuthority(t *testing.T) {
 		{M(aut.Get(p1)), M(true, p1, thor.Bytes32{}, true, nil)},
 		{M(aut.Revoke(p1)), M(true, nil)},
 		{M(aut.Get(p1)), M(false, p1, thor.Bytes32{}, false, nil)},
-		{M(aut.Candidates(&big.Int{}, thor.InitialMaxBlockProposers)), M(
+		{M(aut.Candidates(checker(&big.Int{}), thor.InitialMaxBlockProposers)), M(
 			[]*Candidate{{p2, p2, thor.Bytes32{}, true}, {p3, p3, thor.Bytes32{}, true}}, nil,
 		)},
 		{

builtin/staker/staker.go

@@ -40,6 +40,7 @@ func SetLogger(l log.Logger) {
 // Staker implements native methods of `Staker` contract.
 type Staker struct {
 	params *params.Params
+	state  *state.State
 
 	aggregationService *aggregation.Service
 	globalStatsService *globalstats.Service
@@ -53,6 +54,7 @@ func New(addr thor.Address, state *state.State, params *params.Params, charger *
 
 	return &Staker{
 		params: params,
+		state:  state,
 
 		aggregationService: aggregation.New(sctx),
 		globalStatsService: globalstats.New(sctx),

builtin/staker/transition.go

@@ -8,6 +8,7 @@ package staker
 import (
 	"math/big"
 
+	"github.com/vechain/thor/v2/builtin/authority"
 	"github.com/vechain/thor/v2/thor"
 )
 
@@ -61,3 +62,37 @@ func (s *Staker) transition(currentBlock uint32) (bool, error) {
 
 	return true, nil
 }
+
+var bigE18 = big.NewInt(1e18)
+
+// TransitionPeriodBalanceCheck returns a BalanceChecker function that checks if an endorser has enough VET soft staked
+// for the transition period whereby the endorser can leverage queued VET to meet the requirement.
+// It defaults to checking the account balance first and then checks the queued VET if in transition period.
+func (s *Staker) TransitionPeriodBalanceCheck(fc *thor.ForkConfig, currentBlock uint32, endorsement *big.Int) authority.BalanceChecker {
+	return func(validator, endorser thor.Address) (bool, error) {
+		balance, err := s.state.GetBalance(endorser)
+		if err != nil {
+			return false, err
+		}
+		if balance.Cmp(endorsement) >= 0 {
+			return true, nil
+		}
+		if currentBlock < fc.HAYABUSA { // before HAYABUSA fork, we only check the account balance
+			return false, nil
+		}
+		validation, err := s.validationService.GetValidation(validator)
+		if err != nil {
+			return false, err
+		}
+		if validation.IsEmpty() {
+			return false, nil
+		}
+		if validation.Endorser != endorser {
+			return false, nil // endorser mismatch
+		}
+		queuedVET := big.NewInt(0).SetUint64(validation.QueuedVET)
+		queuedVET.Mul(queuedVET, bigE18) // convert to wei
+
+		return queuedVET.Cmp(endorsement) >= 0, nil
+	}
+}

builtin/staker/transition_test.go

@@ -72,3 +72,84 @@ func TestTransition(t *testing.T) {
 	st.SetRawStorage(stakerAddr, activeCountSlot, rlp.RawValue{0x0})
 	st.SetRawStorage(stakerAddr, queuedCountSlot, rlp.RawValue{0xFF})
 }
+
+func TestStaker_TransitionPeriodBalanceCheck(t *testing.T) {
+	fc := &thor.ForkConfig{
+		HAYABUSA: 10,
+	}
+	tp := uint32(10)
+	thor.SetConfig(thor.Config{HayabusaTP: &tp})
+	bigE18 := big.NewInt(1e18)
+
+	endorsement := big.NewInt(1000)
+	endorser := datagen.RandAddress()
+	master := datagen.RandAddress()
+
+	tests := []struct {
+		name         string
+		currentBlock uint32
+		preTestHook  func(staker *Staker)
+		ok           bool
+	}{
+		{
+			name:         "before hayabusa, validator has greater than funds",
+			currentBlock: 5,
+			preTestHook: func(staker *Staker) {
+				assert.NoError(t, staker.state.SetBalance(endorser, big.NewInt(2000)))
+			},
+			ok: true,
+		},
+		{
+			name:         "before hayabusa, validator funds are too low",
+			currentBlock: 5,
+			preTestHook: func(staker *Staker) {
+				assert.NoError(t, staker.state.SetBalance(endorser, big.NewInt(500)))
+			},
+			ok: false,
+		},
+		{
+			name:         "before hayabusa, validator has exactly enough funds",
+			currentBlock: 5,
+			preTestHook: func(staker *Staker) {
+				assert.NoError(t, staker.state.SetBalance(endorser, big.NewInt(1000)))
+			},
+			ok: true,
+		},
+		{
+			name:         "during transition period, validator has not staked, has enough funds",
+			currentBlock: 15,
+			preTestHook: func(staker *Staker) {
+				assert.NoError(t, staker.state.SetBalance(endorser, big.NewInt(2000)))
+			},
+			ok: true,
+		},
+		{
+			name:         "during transition period, validator has not staked, has insufficient funds",
+			currentBlock: 15,
+			preTestHook: func(staker *Staker) {
+				assert.NoError(t, staker.state.SetBalance(endorser, big.NewInt(500)))
+			},
+			ok: false,
+		},
+		{
+			name:         "during transition period, validator has staked",
+			currentBlock: 15,
+			preTestHook: func(staker *Staker) {
+				stake := big.NewInt(0).Div(MinStake, bigE18).Uint64()
+				assert.NoError(t, staker.AddValidation(master, endorser, thor.MediumStakingPeriod(), stake))
+			},
+			ok: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			staker, _ := newStaker(t, 1, 1, false)
+			tt.preTestHook(staker)
+			balanceCheck := staker.TransitionPeriodBalanceCheck(fc, tt.currentBlock, endorsement)
+			ok, err := balanceCheck(master, endorser)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.ok, ok)
+		})
+	}
+}

consensus/poa_validator.go

@@ -7,7 +7,6 @@ package consensus
 
 import (
 	"fmt"
-	"math/big"
 
 	"github.com/vechain/thor/v2/block"
 	"github.com/vechain/thor/v2/builtin"
@@ -34,8 +33,14 @@ func (c *Consensus) validateAuthorityProposer(header *block.Header, parent *bloc
 		}
 		candidates = poa.NewCandidates(list)
 	}
+	staker := builtin.Staker.Native(st)
+	endorsement, err := builtin.Params.Native(st).Get(thor.KeyProposerEndorsement)
+	if err != nil {
+		return nil, err
+	}
+	balanceCheck := staker.TransitionPeriodBalanceCheck(c.forkConfig, header.Number(), endorsement)
 
-	proposers, err := candidates.Pick(st, c.authorityBalanceCheck(header, st, signer))
+	proposers, err := candidates.Pick(st, balanceCheck)
 	if err != nil {
 		return nil, err
 	}
@@ -125,36 +130,3 @@ func (c *Consensus) authorityCacheHandler(candidates *poa.Candidates, header *bl
 
 	return nil
 }
-
-func (c *Consensus) authorityBalanceCheck(header *block.Header, st *state.State, signer thor.Address) poa.BalancerChecker {
-	transitionPeriod := header.Number() >= c.forkConfig.HAYABUSA
-	staker := builtin.Staker.Native(st)
-
-	return func(endorsor thor.Address, minBalance *big.Int) (bool, error) {
-		bal, err := st.GetBalance(endorsor)
-		if err != nil {
-			return false, err
-		}
-		hasAccountBalance := bal.Cmp(minBalance) >= 0
-		if hasAccountBalance {
-			return true, nil
-		}
-		if !transitionPeriod { // before HAYABUSA fork, we only check the account balance
-			return false, nil
-		}
-		// `signer` is the node master, not the endorsor
-		// We are checking if the signer of the block has a `Validation` entry with a stake
-		// NOT if the given endorsor has a staked
-		validator, err := staker.GetValidation(signer)
-		if err != nil {
-			return false, err
-		}
-		if validator.IsEmpty() || validator.QueuedVET == 0 {
-			return false, nil
-		}
-
-		// convert the queued VET to wei
-		queuedVET := new(big.Int).Mul(new(big.Int).SetUint64(validator.QueuedVET), big.NewInt(1e18))
-		return queuedVET.Cmp(minBalance) >= 0, nil
-	}
-}