Refactor Transition (#1212)

* Update validation names + reorder staker ops

* moved withdrawable calculations to validation

* Decouple calculation and application in housekeep

* added todo + cleanup

* Decouple calculation and application in housekeep

* added todo + cleanup

* ensure activations use calculated exits

* Refactor staker transitions

* move validation ops to validation service

* reuse housekeeping approach

* removing public AddLeaderGroup

* Decouple calculation and application in housekeep

* added todo + cleanup

* Update validation names + reorder staker ops

* moved withdrawable calculations to validation

* Decouple calculation and application in housekeep

* added todo + cleanup

* ensure activations use calculated exits

* Refactor staker transitions

* move validation ops to validation service

* reuse housekeeping approach

* removing public AddLeaderGroup

* revert e2e test

* add ll todo

* revert e2e change

* ActivateNextValidator as a public method

* Update builtin/staker/validation/service.go

Co-authored-by: Paolo Galli <paolo.galli@vechain.org>

* pr comments

---------

Co-authored-by: Paolo Galli <paolo.galli@vechain.org>

Author: otherview

builtin/staker/housekeep.go

@@ -15,7 +15,10 @@ import (
 	"github.com/vechain/thor/v2/thor"
 )
 
+//
 // State transition types
+//
+
 type EpochTransition struct {
 	Block           uint32
 	Renewals        []ValidatorRenewal
@@ -38,7 +41,7 @@ func (s *Staker) Housekeep(currentBlock uint32) (bool, map[thor.Address]*validat
 
 	logger.Info("🏠performing housekeeping", "block", currentBlock)
 
-	transition, err := s.ComputeEpochTransition(currentBlock)
+	transition, err := s.computeEpochTransition(currentBlock)
 	if err != nil {
 		return false, nil, err
 	}
@@ -47,7 +50,7 @@ func (s *Staker) Housekeep(currentBlock uint32) (bool, map[thor.Address]*validat
 		return false, nil, nil
 	}
 
-	if err := s.ApplyEpochTransition(transition); err != nil {
+	if err := s.applyEpochTransition(transition); err != nil {
 		return false, nil, err
 	}
 
@@ -58,12 +61,9 @@ func (s *Staker) Housekeep(currentBlock uint32) (bool, map[thor.Address]*validat
 	return true, activeValidators, nil
 }
 
-// ComputeEpochTransition calculates all state changes needed for an epoch transition
-func (s *Staker) ComputeEpochTransition(currentBlock uint32) (*EpochTransition, error) {
+// computeEpochTransition calculates all state changes needed for an epoch transition
+func (s *Staker) computeEpochTransition(currentBlock uint32) (*EpochTransition, error) {
 	var err error
-	if currentBlock%epochLength != 0 {
-		return nil, nil // No transition needed
-	}
 
 	transition := &EpochTransition{Block: currentBlock}
 
@@ -80,7 +80,7 @@ func (s *Staker) ComputeEpochTransition(currentBlock uint32) (*EpochTransition,
 	}
 
 	// 3. Compute all activations
-	transition.ActivationCount, err = s.computeActivations(transition.ExitValidator != nil)
+	transition.ActivationCount, err = s.computeActivationCount(transition.ExitValidator != nil)
 	if err != nil {
 		return nil, err
 	}
@@ -156,7 +156,8 @@ func (s *Staker) computeExits(currentBlock uint32) (*thor.Address, error) {
 	return nil, nil
 }
 
-func (s *Staker) computeActivations(hasValidatorExited bool) (int64, error) {
+// computeActivationCount calculates how many validators can be activated
+func (s *Staker) computeActivationCount(hasValidatorExited bool) (int64, error) {
 	// Calculate how many validators can be activated
 	queuedSize, err := s.QueuedGroupSize()
 	if err != nil {
@@ -193,8 +194,8 @@ func (s *Staker) computeActivations(hasValidatorExited bool) (int64, error) {
 	return queuedCount, nil
 }
 
-// ApplyEpochTransition applies all computed changes
-func (s *Staker) ApplyEpochTransition(transition *EpochTransition) error {
+// applyEpochTransition applies all computed changes
+func (s *Staker) applyEpochTransition(transition *EpochTransition) error {
 	logger.Info("applying epoch transition", "block", transition.Block)
 
 	// Apply renewals
@@ -243,8 +244,7 @@ func (s *Staker) ApplyEpochTransition(transition *EpochTransition) error {
 	}
 
 	for range transition.ActivationCount {
-		_, err := s.ActivateNextValidator(transition.Block, maxLeaderGroupSize)
-		if err != nil {
+		if _, err := s.ActivateNextValidator(transition.Block, maxLeaderGroupSize); err != nil {
 			return err
 		}
 	}
@@ -273,134 +273,27 @@ func (s *Staker) buildActiveValidatorsFromTransition(transition *EpochTransition
 	return activeValidators
 }
 
-// Transition activates the staker contract when sufficient validators are queued
-func (s *Staker) Transition(currentBlock uint32) (bool, error) {
-	active, err := s.IsPoSActive()
-	if err != nil {
-		return false, err
-	}
-	if active {
-		return false, nil
-	}
-
-	maxProposers, err := s.params.Get(thor.KeyMaxBlockProposers)
-	if err != nil || maxProposers.Cmp(big.NewInt(0)) == 0 {
-		maxProposers = big.NewInt(0).SetUint64(thor.InitialMaxBlockProposers)
-	}
-
-	queueSize, err := s.validationService.QueuedGroupSize()
-	if err != nil {
-		return false, err
-	}
-
-	// if the queue size is not AT LEAST 2/3 of the maxProposers, then return nil
-	minimum := big.NewFloat(0).SetInt(maxProposers)
-	minimum.Mul(minimum, big.NewFloat(2))
-	minimum.Quo(minimum, big.NewFloat(3))
-	if big.NewFloat(0).SetInt(queueSize).Cmp(minimum) < 0 {
-		return false, nil
-	}
-
-	// Use existing activateValidators method for transition
-	ids, err := s.activateValidators(currentBlock)
-	if err != nil {
-		return false, err
-	}
-	logger.Info("activated validations", "count", len(ids))
-
-	return true, nil
-}
-
-// activateValidators is kept for the Transition method
-func (s *Staker) activateValidators(currentBlock uint32) ([]*thor.Address, error) {
-	queuedSize, err := s.QueuedGroupSize()
-	if err != nil {
-		return nil, err
-	}
-	leaderSize, err := s.LeaderGroupSize()
-	if err != nil {
-		return nil, err
-	}
-	maxSize, err := s.params.Get(thor.KeyMaxBlockProposers)
-	if err != nil {
-		return nil, err
-	}
-	if leaderSize.Cmp(maxSize) >= 0 {
-		return nil, nil
-	}
-
-	// no one is in the queue
-	if queuedSize.Cmp(big.NewInt(0)) <= 0 {
-		return nil, nil
-	}
-
-	queuedCount := queuedSize.Int64()
-	leaderDelta := maxSize.Int64() - leaderSize.Int64()
-	if leaderDelta > 0 {
-		if leaderDelta < queuedCount {
-			queuedCount = leaderDelta
-		}
-	} else {
-		return nil, nil
-	}
-
-	activated := make([]*thor.Address, queuedCount)
-	maxLeaderGroupSize, err := s.params.Get(thor.KeyMaxBlockProposers)
-	if err != nil {
-		return nil, err
-	}
-
-	for i := int64(0); i < queuedCount; i++ {
-		id, err := s.ActivateNextValidator(currentBlock, maxLeaderGroupSize)
-		if err != nil {
-			return nil, err
-		}
-		activated[i] = id
-	}
-
-	return activated, nil
-}
-
+// TODO this should be a public method, it's ever only used in the pause test
 func (s *Staker) ActivateNextValidator(currentBlk uint32, maxLeaderGroupSize *big.Int) (*thor.Address, error) {
-	validatorID, val, err := s.validationService.NextToActivate(maxLeaderGroupSize)
+	validatorID, err := s.validationService.NextToActivate(maxLeaderGroupSize)
 	if err != nil {
 		return nil, err
 	}
 	logger.Debug("activating validator", "validatorID", validatorID, "block", currentBlk)
 
+	// renew the current delegations aggregation
 	aggRenew, err := s.aggregationService.Renew(*validatorID)
 	if err != nil {
 		return nil, err
 	}
 
-	// update the validator values
-	// TODO move this to the validatorservice at some point
-	validatorLocked := big.NewInt(0).Add(val.LockedVET, val.QueuedVET)
-	val.QueuedVET = big.NewInt(0)
-	val.LockedVET = validatorLocked
-	// x2 multiplier for validator's stake
-	validatorWeight := big.NewInt(0).Mul(validatorLocked, validatorWeightMultiplier)
-	val.Weight = big.NewInt(0).Add(validatorWeight, aggRenew.NewLockedWeight)
-
-	// update the validator statuses
-	val.Status = validation.StatusActive
-	val.Online = true
-	val.StartBlock = currentBlk
-	// add to the active list
-	added, err := s.validationService.AddLeaderGroup(*validatorID, val)
+	// Activate the validator using the validation service
+	validatorRenewal, err := s.validationService.ActivateValidator(*validatorID, currentBlk, aggRenew)
 	if err != nil {
 		return nil, err
 	}
-	if !added {
-		return nil, errors.New("failed to add validator to active list")
-	}
 
-	validatorRenewal := &delta.Renewal{
-		NewLockedVET:         val.LockedVET,
-		NewLockedWeight:      val.Weight,
-		QueuedDecrease:       val.LockedVET,
-		QueuedDecreaseWeight: big.NewInt(0).Mul(val.LockedVET, validatorWeightMultiplier), // Only decrease validator's own weight
-	}
+	// Update global stats with both validator and delegation renewals
 	if err = s.globalStatsService.UpdateTotals(validatorRenewal, aggRenew); err != nil {
 		return nil, err
 	}

builtin/staker/transition.go

@@ -0,0 +1,67 @@
+// Copyright (c) 2018 The VeChainThor developers
+
+// Distributed under the GNU Lesser General Public License v3.0 software license, see the accompanying
+// file LICENSE or <https://www.gnu.org/licenses/lgpl-3.0.html>
+
+package staker
+
+import (
+	"math/big"
+
+	"github.com/vechain/thor/v2/thor"
+)
+
+// Transition activates the staker contract when sufficient validators are queued
+func (s *Staker) Transition(currentBlock uint32) (bool, error) {
+	// TODO review how to change this elegantly for unit tests
+	// if this check is enabled the epochLength is defaulted to 180 blocks
+	// which breaks most of tests that rely on a HAYABUSA_TP = 1
+	//
+	//if currentBlock%epochLength != 0 {
+	//	return false, nil // No transition needed
+	//}
+
+	active, err := s.IsPoSActive()
+	if err != nil {
+		return false, err
+	}
+	if active {
+		return false, nil
+	}
+
+	maxProposers, err := s.params.Get(thor.KeyMaxBlockProposers)
+	if err != nil {
+		return false, err
+	}
+
+	if maxProposers.Cmp(big.NewInt(0)) == 0 {
+		maxProposers = big.NewInt(0).SetUint64(thor.InitialMaxBlockProposers)
+	}
+
+	queueSize, err := s.validationService.QueuedGroupSize()
+	if err != nil {
+		return false, err
+	}
+
+	// Transitions is not possible if the queue size is not AT LEAST 2/3 of the maxProposers
+	// queueSize >= 2/3 * maxProposers
+	// queueSize * 3 >= maxProposers * 2
+	if queueSize.Int64()*3 < maxProposers.Int64()*2 { // these figures will never surpass int64
+		return false, nil
+	}
+
+	// Use the epoch transition pattern as housekeeping
+	transition, err := s.computeEpochTransition(currentBlock)
+	if err != nil {
+		return false, err
+	}
+
+	// Apply the transition
+	if err := s.applyEpochTransition(transition); err != nil {
+		return false, err
+	}
+
+	logger.Info("activated validations", "count", transition.ActivationCount)
+
+	return true, nil
+}

builtin/staker/validation/linked_list.go

@@ -81,6 +81,7 @@ func (l *LinkedList) Remove(validator thor.Address, validation *Validation) (rem
 			return false, err
 		}
 		prevEntry.Next = next
+		// TODO keep the LL separated from the Validation storage ops
 		if err := l.repo.SetValidation(*prev, prevEntry, false); err != nil {
 			return false, err
 		}
@@ -94,6 +95,7 @@ func (l *LinkedList) Remove(validator thor.Address, validation *Validation) (rem
 			return false, err
 		}
 		nextEntry.Prev = prev
+		// TODO keep the LL separated from the Validation storage ops
 		if err := l.repo.SetValidation(*next, nextEntry, false); err != nil {
 			return false, err
 		}
@@ -128,6 +130,7 @@ func (l *LinkedList) Add(newTail thor.Address, validation *Validation) (added bo
 		// list is currently empty, set this entry to head & tail
 		l.head.Set(&newTail, false)
 		l.tail.Set(&newTail, false)
+		// TODO keep the LL separated from the Validation storage ops
 		return true, l.repo.SetValidation(newTail, validation, false)
 	}
 
@@ -138,6 +141,7 @@ func (l *LinkedList) Add(newTail thor.Address, validation *Validation) (added bo
 	oldTail.Next = &newTail
 	validation.Prev = &oldTailID
 
+	// TODO keep the LL separated from the Validation storage ops
 	if err := l.repo.SetValidation(oldTailID, oldTail, false); err != nil {
 		return false, err
 	}