From 68570c7f133980223211afded7b35c346849c4fb Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Tue, 22 Aug 2023 22:01:46 +0200 Subject: [PATCH 01/15] rewardValidatorTx cleanup --- .../txs/executor/proposal_tx_executor.go | 650 ++++++++---------- .../txs/executor/staker_tx_verification.go | 89 --- .../staker_tx_verification_helpers.go | 258 +++++++ 3 files changed, 528 insertions(+), 469 deletions(-) create mode 100644 vms/platformvm/txs/executor/staker_tx_verification_helpers.go diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index eddd3921e5e0..cd77e56dc1e7 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -6,17 +6,19 @@ package executor import ( "errors" "fmt" + "math" "time" "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/constants" - "github.com/ava-labs/avalanchego/utils/math" "github.com/ava-labs/avalanchego/vms/components/avax" "github.com/ava-labs/avalanchego/vms/components/verify" "github.com/ava-labs/avalanchego/vms/platformvm/reward" "github.com/ava-labs/avalanchego/vms/platformvm/state" "github.com/ava-labs/avalanchego/vms/platformvm/txs" + + safemath "github.com/ava-labs/avalanchego/utils/math" ) const ( @@ -320,33 +322,34 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error if !currentStakerIterator.Next() { return fmt.Errorf("failed to get next staker to remove: %w", database.ErrNotFound) } - stakerToRemove := currentStakerIterator.Value() + stakerToReward := currentStakerIterator.Value() currentStakerIterator.Release() - if stakerToRemove.TxID != tx.TxID { + if stakerToReward.TxID != tx.TxID { return fmt.Errorf( "%w: %s != %s", ErrRemoveWrongStaker, - stakerToRemove.TxID, + stakerToReward.TxID, tx.TxID, ) } // Verify that the chain's timestamp is the validator's end time currentChainTime := e.OnCommitState.GetTimestamp() - if !stakerToRemove.EndTime.Equal(currentChainTime) { + if !stakerToReward.NextTime.Equal(currentChainTime) { return fmt.Errorf( "%w: TxID = %s with %s < %s", ErrRemoveStakerTooEarly, tx.TxID, currentChainTime, - stakerToRemove.EndTime, + stakerToReward.NextTime, ) } + // retrieve primaryNetworkValidator before possibly removing it. primaryNetworkValidator, err := e.OnCommitState.GetCurrentValidator( constants.PrimaryNetworkID, - stakerToRemove.NodeID, + stakerToReward.NodeID, ) if err != nil { // This should never error because the staker set is in memory and @@ -354,181 +357,276 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error return err } - stakerTx, _, err := e.OnCommitState.GetTx(stakerToRemove.TxID) + stakerTx, _, err := e.OnCommitState.GetTx(stakerToReward.TxID) if err != nil { return fmt.Errorf("failed to get next removed staker tx: %w", err) } + // Invariant: A [txs.DelegatorTx] does not also implement the + // [txs.ValidatorTx] interface. switch uStakerTx := stakerTx.Unsigned.(type) { case txs.ValidatorTx: - e.OnCommitState.DeleteCurrentValidator(stakerToRemove) - e.OnAbortState.DeleteCurrentValidator(stakerToRemove) + if err := e.rewardValidatorTx(uStakerTx, stakerToReward); err != nil { + return err + } + case txs.DelegatorTx: + if err := e.rewardDelegatorTx(uStakerTx, stakerToReward); err != nil { + return err + } + default: + // Invariant: Permissioned stakers are removed by the advancement of + // time and the current chain timestamp is == this staker's + // EndTime. This means only permissionless stakers should be + // left in the staker set. + return ErrShouldBePermissionlessStaker + } + + // If the reward is aborted, then the current supply should be decreased. + currentSupply, err := e.OnAbortState.GetCurrentSupply(stakerToReward.SubnetID) + if err != nil { + return err + } + newSupply, err := safemath.Sub(currentSupply, stakerToReward.PotentialReward) + if err != nil { + return err + } + e.OnAbortState.SetCurrentSupply(stakerToReward.SubnetID, newSupply) - stake := uStakerTx.Stake() - outputs := uStakerTx.Outputs() + // handle staker lifecycle + e.OnCommitState.DeleteCurrentValidator(stakerToReward) + e.OnAbortState.DeleteCurrentValidator(stakerToReward) + + // handle option preference + shouldCommit, err := e.calculateProposalPreference(stakerToReward, primaryNetworkValidator) + if err != nil { + return err + } + + e.PrefersCommit = shouldCommit + return nil +} + +func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, validator *state.Staker) error { + var ( + txID = validator.TxID + stake = uValidatorTx.Stake() + outputs = uValidatorTx.Outputs() // Invariant: The staked asset must be equal to the reward asset. - stakeAsset := stake[0].Asset + stakeAsset = stake[0].Asset + ) - // Refund the stake here - for i, out := range stake { - utxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + i), - }, - Asset: out.Asset, - Out: out.Output(), - } - e.OnCommitState.AddUTXO(utxo) - e.OnAbortState.AddUTXO(utxo) + // Refund the stake only when validator is about to leave + // the staking set + for i, out := range stake { + utxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(len(outputs) + i), + }, + Asset: out.Asset, + Out: out.Output(), } + e.OnCommitState.AddUTXO(utxo) + e.OnAbortState.AddUTXO(utxo) + } - offset := 0 + utxosOffset := len(outputs) + len(stake) + currentRewardUTXOs, err := e.OnCommitState.GetRewardUTXOs(txID) + if err != nil { + return fmt.Errorf("failed to create output: %w", err) + } + utxosOffset += len(currentRewardUTXOs) - // Provide the reward here - if stakerToRemove.PotentialReward > 0 { - validationRewardsOwner := uStakerTx.ValidationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(stakerToRemove.PotentialReward, validationRewardsOwner) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - out, ok := outIntf.(verify.State) - if !ok { - return ErrInvalidState - } + // Provide the reward here + rewardToPayBack := validator.PotentialReward + if rewardToPayBack > 0 { + validationRewardsOwner := uValidatorTx.ValidationRewardsOwner() + outIntf, err := e.Fx.CreateOutput(rewardToPayBack, validationRewardsOwner) + if err != nil { + return fmt.Errorf("failed to create output: %w", err) + } + out, ok := outIntf.(verify.State) + if !ok { + return ErrInvalidState + } - utxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + len(stake)), - }, - Asset: stakeAsset, - Out: out, - } + utxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(utxosOffset), + }, + Asset: stakeAsset, + Out: out, + } + e.OnCommitState.AddUTXO(utxo) + e.OnCommitState.AddRewardUTXO(txID, utxo) - e.OnCommitState.AddUTXO(utxo) - e.OnCommitState.AddRewardUTXO(tx.TxID, utxo) + utxosOffset++ + } - offset++ - } + // Provide the accrued delegatee rewards from successful delegations here. + delegateeRewardToPayBack, err := e.OnCommitState.GetDelegateeReward( + validator.SubnetID, + validator.NodeID, + ) + if err != nil && err != database.ErrNotFound { + return fmt.Errorf("failed to fetch accrued delegatee rewards: %w", err) + } - // Provide the accrued delegatee rewards from successful delegations here. - delegateeReward, err := e.OnCommitState.GetDelegateeReward( - stakerToRemove.SubnetID, - stakerToRemove.NodeID, - ) + if delegateeRewardToPayBack > 0 { + delegationRewardsOwner := uValidatorTx.DelegationRewardsOwner() + outIntf, err := e.Fx.CreateOutput(delegateeRewardToPayBack, delegationRewardsOwner) if err != nil { - return fmt.Errorf("failed to fetch accrued delegatee rewards: %w", err) + return fmt.Errorf("failed to create output: %w", err) + } + out, ok := outIntf.(verify.State) + if !ok { + return ErrInvalidState } - if delegateeReward > 0 { - delegationRewardsOwner := uStakerTx.DelegationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - out, ok := outIntf.(verify.State) - if !ok { - return ErrInvalidState - } + onCommitUtxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(utxosOffset), + }, + Asset: stakeAsset, + Out: out, + } + e.OnCommitState.AddUTXO(onCommitUtxo) + e.OnCommitState.AddRewardUTXO(txID, onCommitUtxo) + + // Note: There is no [offset] if the RewardValidatorTx is + // aborted, because the validator reward is not awarded. + onAbortUtxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(utxosOffset - 1), + }, + Asset: stakeAsset, + Out: out, + } + e.OnAbortState.AddUTXO(onAbortUtxo) + e.OnAbortState.AddRewardUTXO(txID, onAbortUtxo) + } + return nil +} - onCommitUtxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + len(stake) + offset), - }, - Asset: stakeAsset, - Out: out, - } - e.OnCommitState.AddUTXO(onCommitUtxo) - e.OnCommitState.AddRewardUTXO(tx.TxID, onCommitUtxo) +func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, delegator *state.Staker) error { + var ( + txID = delegator.TxID + stake = uDelegatorTx.Stake() + outputs = uDelegatorTx.Outputs() + // Invariant: The staked asset must be equal to the reward asset. + stakeAsset = stake[0].Asset + ) - onAbortUtxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - // Note: There is no [offset] if the RewardValidatorTx is - // aborted, because the validator reward is not awarded. - OutputIndex: uint32(len(outputs) + len(stake)), - }, - Asset: stakeAsset, - Out: out, - } - e.OnAbortState.AddUTXO(onAbortUtxo) - e.OnAbortState.AddRewardUTXO(tx.TxID, onAbortUtxo) + // Refund the stake only when delegator is about to leave + // the staking set + for i, out := range stake { + utxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(len(outputs) + i), + }, + Asset: out.Asset, + Out: out.Output(), } - // Invariant: A [txs.DelegatorTx] does not also implement the - // [txs.ValidatorTx] interface. - case txs.DelegatorTx: - e.OnCommitState.DeleteCurrentDelegator(stakerToRemove) - e.OnAbortState.DeleteCurrentDelegator(stakerToRemove) + e.OnCommitState.AddUTXO(utxo) + e.OnAbortState.AddUTXO(utxo) + } - stake := uStakerTx.Stake() - outputs := uStakerTx.Outputs() - stakeAsset := stake[0].Asset + // We're (possibly) rewarding a delegator, so we need to fetch + // the validator they are delegated to. + validator, err := e.OnCommitState.GetCurrentValidator(delegator.SubnetID, delegator.NodeID) + if err != nil { + return fmt.Errorf("failed to get whether %s is a validator: %w", delegator.NodeID, err) + } - // Refund the stake here - for i, out := range stake { - utxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + i), - }, - Asset: out.Asset, - Out: out.Output(), - } - e.OnCommitState.AddUTXO(utxo) - e.OnAbortState.AddUTXO(utxo) - } + vdrTxIntf, _, err := e.OnCommitState.GetTx(validator.TxID) + if err != nil { + return fmt.Errorf("failed to get whether %s is a validator: %w", delegator.NodeID, err) + } - // We're removing a delegator, so we need to fetch the validator they - // are delegated to. - vdrStaker, err := e.OnCommitState.GetCurrentValidator( - stakerToRemove.SubnetID, - stakerToRemove.NodeID, - ) - if err != nil { - return fmt.Errorf( - "failed to get whether %s is a validator: %w", - stakerToRemove.NodeID, - err, - ) - } + // Invariant: Delegators must only be able to reference validator + // transactions that implement [txs.ValidatorTx]. All + // validator transactions implement this interface except the + // AddSubnetValidatorTx. + vdrTx, ok := vdrTxIntf.Unsigned.(txs.ValidatorTx) + if !ok { + return ErrWrongTxType + } + + // Calculate split of reward between delegator/delegatee + delegatorReward, delegateeReward := splitAmountByShares(delegator.PotentialReward, vdrTx.Shares(), math.MaxUint64) + + // following Continuous staking fork activation multiple rewards UTXOS + // can be cumulated, each related to a different staking period. We make + // sure to index the reward UTXOs correctly by appending them to previous ones. + utxosOffset := len(outputs) + len(stake) + currentRewardUTXOs, err := e.OnCommitState.GetRewardUTXOs(delegator.TxID) + if err != nil { + return fmt.Errorf("failed to create output: %w", err) + } + utxosOffset += len(currentRewardUTXOs) - vdrTxIntf, _, err := e.OnCommitState.GetTx(vdrStaker.TxID) + // Reward the delegator here + delRewardToPayBack := delegatorReward + if delRewardToPayBack > 0 { + rewardsOwner := uDelegatorTx.RewardsOwner() + outIntf, err := e.Fx.CreateOutput(delRewardToPayBack, rewardsOwner) if err != nil { - return fmt.Errorf( - "failed to get whether %s is a validator: %w", - stakerToRemove.NodeID, - err, - ) + return fmt.Errorf("failed to create output: %w", err) } - - // Invariant: Delegators must only be able to reference validator - // transactions that implement [txs.ValidatorTx]. All - // validator transactions implement this interface except the - // AddSubnetValidatorTx. - vdrTx, ok := vdrTxIntf.Unsigned.(txs.ValidatorTx) + out, ok := outIntf.(verify.State) if !ok { - return ErrWrongTxType + return ErrInvalidState } - - // Calculate split of reward between delegator/delegatee - // The delegator gives stake to the validatee - validatorShares := vdrTx.Shares() - delegatorShares := reward.PercentDenominator - uint64(validatorShares) // parentTx.Shares <= reward.PercentDenominator so no underflow - delegatorReward := delegatorShares * (stakerToRemove.PotentialReward / reward.PercentDenominator) // delegatorShares <= reward.PercentDenominator so no overflow - // Delay rounding as long as possible for small numbers - if optimisticReward, err := math.Mul64(delegatorShares, stakerToRemove.PotentialReward); err == nil { - delegatorReward = optimisticReward / reward.PercentDenominator + utxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(utxosOffset), + }, + Asset: stakeAsset, + Out: out, } - delegateeReward := stakerToRemove.PotentialReward - delegatorReward // delegatorReward <= reward so no underflow - offset := 0 + e.OnCommitState.AddUTXO(utxo) + e.OnCommitState.AddRewardUTXO(txID, utxo) + + utxosOffset++ + } + + // Reward the delegatee here + if delegateeReward > 0 { + if validator.StartTime.After(e.Config.CortinaTime) { + previousDelegateeReward, err := e.OnCommitState.GetDelegateeReward( + validator.SubnetID, + validator.NodeID, + ) + if err != nil { + return fmt.Errorf("failed to get delegatee reward: %w", err) + } - // Reward the delegator here - if delegatorReward > 0 { - rewardsOwner := uStakerTx.RewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegatorReward, rewardsOwner) + // Invariant: The rewards calculator can never return a + // [potentialReward] that would overflow the + // accumulated rewards. + newDelegateeReward := previousDelegateeReward + delegateeReward + + // For any validators starting after [CortinaTime], we defer rewarding the + // [rewardToPayBack] until their staking period is over. + err = e.OnCommitState.SetDelegateeReward( + validator.SubnetID, + validator.NodeID, + newDelegateeReward, + ) + if err != nil { + return fmt.Errorf("failed to update delegatee reward: %w", err) + } + } else { + // For any validators who started prior to [CortinaTime], we issue the + // [delegateeReward] immediately. + delegationRewardsOwner := vdrTx.DelegationRewardsOwner() + outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) if err != nil { return fmt.Errorf("failed to create output: %w", err) } @@ -538,98 +636,30 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error } utxo := &avax.UTXO{ UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + len(stake)), + TxID: txID, + OutputIndex: uint32(utxosOffset), }, Asset: stakeAsset, Out: out, } e.OnCommitState.AddUTXO(utxo) - e.OnCommitState.AddRewardUTXO(tx.TxID, utxo) - - offset++ - } - - // Reward the delegatee here - if delegateeReward > 0 { - if vdrStaker.StartTime.After(e.Config.CortinaTime) { - previousDelegateeReward, err := e.OnCommitState.GetDelegateeReward( - vdrStaker.SubnetID, - vdrStaker.NodeID, - ) - if err != nil { - return fmt.Errorf("failed to get delegatee reward: %w", err) - } - - // Invariant: The rewards calculator can never return a - // [potentialReward] that would overflow the - // accumulated rewards. - newDelegateeReward := previousDelegateeReward + delegateeReward - - // For any validators starting after [CortinaTime], we defer rewarding the - // [delegateeReward] until their staking period is over. - err = e.OnCommitState.SetDelegateeReward( - vdrStaker.SubnetID, - vdrStaker.NodeID, - newDelegateeReward, - ) - if err != nil { - return fmt.Errorf("failed to update delegatee reward: %w", err) - } - } else { - // For any validators who started prior to [CortinaTime], we issue the - // [delegateeReward] immediately. - delegationRewardsOwner := vdrTx.DelegationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - out, ok := outIntf.(verify.State) - if !ok { - return ErrInvalidState - } - utxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: tx.TxID, - OutputIndex: uint32(len(outputs) + len(stake) + offset), - }, - Asset: stakeAsset, - Out: out, - } - - e.OnCommitState.AddUTXO(utxo) - e.OnCommitState.AddRewardUTXO(tx.TxID, utxo) - } + e.OnCommitState.AddRewardUTXO(txID, utxo) } - default: - // Invariant: Permissioned stakers are removed by the advancement of - // time and the current chain timestamp is == this staker's - // EndTime. This means only permissionless stakers should be - // left in the staker set. - return ErrShouldBePermissionlessStaker - } - - // If the reward is aborted, then the current supply should be decreased. - currentSupply, err := e.OnAbortState.GetCurrentSupply(stakerToRemove.SubnetID) - if err != nil { - return err } - newSupply, err := math.Sub(currentSupply, stakerToRemove.PotentialReward) - if err != nil { - return err - } - e.OnAbortState.SetCurrentSupply(stakerToRemove.SubnetID, newSupply) + return nil +} +func (e *ProposalTxExecutor) calculateProposalPreference(stakerToReward, primaryNetworkValidator *state.Staker) (bool, error) { var expectedUptimePercentage float64 - if stakerToRemove.SubnetID != constants.PrimaryNetworkID { - transformSubnetIntf, err := e.OnCommitState.GetSubnetTransformation(stakerToRemove.SubnetID) + if stakerToReward.SubnetID != constants.PrimaryNetworkID { + transformSubnetIntf, err := e.OnCommitState.GetSubnetTransformation(stakerToReward.SubnetID) if err != nil { - return err + return false, fmt.Errorf("failed to calculate uptime: %w", err) } transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) if !ok { - return ErrIsNotTransformSubnetTx + return false, fmt.Errorf("failed to calculate uptime: %w", err) } expectedUptimePercentage = float64(transformSubnet.UptimeRequirement) / reward.PercentDenominator @@ -644,164 +674,24 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error primaryNetworkValidator.StartTime, ) if err != nil { - return fmt.Errorf("failed to calculate uptime: %w", err) - } - - e.PrefersCommit = uptime >= expectedUptimePercentage - return nil -} - -// GetNextStakerChangeTime returns the next time a staker will be either added -// or removed to/from the current validator set. -func GetNextStakerChangeTime(state state.Chain) (time.Time, error) { - currentStakerIterator, err := state.GetCurrentStakerIterator() - if err != nil { - return time.Time{}, err - } - defer currentStakerIterator.Release() - - pendingStakerIterator, err := state.GetPendingStakerIterator() - if err != nil { - return time.Time{}, err - } - defer pendingStakerIterator.Release() - - hasCurrentStaker := currentStakerIterator.Next() - hasPendingStaker := pendingStakerIterator.Next() - switch { - case hasCurrentStaker && hasPendingStaker: - nextCurrentTime := currentStakerIterator.Value().NextTime - nextPendingTime := pendingStakerIterator.Value().NextTime - if nextCurrentTime.Before(nextPendingTime) { - return nextCurrentTime, nil - } - return nextPendingTime, nil - case hasCurrentStaker: - return currentStakerIterator.Value().NextTime, nil - case hasPendingStaker: - return pendingStakerIterator.Value().NextTime, nil - default: - return time.Time{}, database.ErrNotFound - } -} - -// GetValidator returns information about the given validator, which may be a -// current validator or pending validator. -func GetValidator(state state.Chain, subnetID ids.ID, nodeID ids.NodeID) (*state.Staker, error) { - validator, err := state.GetCurrentValidator(subnetID, nodeID) - if err == nil { - // This node is currently validating the subnet. - return validator, nil - } - if err != database.ErrNotFound { - // Unexpected error occurred. - return nil, err - } - return state.GetPendingValidator(subnetID, nodeID) -} - -// overDelegated returns true if [validator] will be overdelegated when adding [delegator]. -// -// A [validator] would become overdelegated if: -// - the maximum total weight on [validator] exceeds [weightLimit] -func overDelegated( - state state.Chain, - validator *state.Staker, - weightLimit uint64, - delegator *state.Staker, -) (bool, error) { - maxWeight, err := GetMaxWeight(state, validator, delegator.StartTime, delegator.EndTime) - if err != nil { - return true, err - } - newMaxWeight, err := math.Add64(maxWeight, delegator.Weight) - if err != nil { - return true, err + return false, fmt.Errorf("failed to calculate uptime: %w", err) } - return newMaxWeight > weightLimit, nil + return uptime >= expectedUptimePercentage, nil } -// GetMaxWeight returns the maximum total weight of the [validator], including -// its own weight, between [startTime] and [endTime]. -// The weight changes are applied in the order they will be applied as chain -// time advances. -// Invariant: -// - [validator.StartTime] <= [startTime] < [endTime] <= [validator.EndTime] -func GetMaxWeight( - chainState state.Chain, - validator *state.Staker, - startTime time.Time, - endTime time.Time, -) (uint64, error) { - currentDelegatorIterator, err := chainState.GetCurrentDelegatorIterator(validator.SubnetID, validator.NodeID) - if err != nil { - return 0, err - } +func splitAmountByShares(totalAmount uint64, shares uint32, sharesAmountCap uint64) (uint64, uint64) { + remainderShares := reward.PercentDenominator - uint64(shares) + remainderAmount := remainderShares * (totalAmount / reward.PercentDenominator) - // TODO: We can optimize this by moving the current total weight to be - // stored in the validator state. - // - // Calculate the current total weight on this validator, including the - // weight of the actual validator and the sum of the weights of all of the - // currently active delegators. - currentWeight := validator.Weight - for currentDelegatorIterator.Next() { - currentDelegator := currentDelegatorIterator.Value() - - currentWeight, err = math.Add64(currentWeight, currentDelegator.Weight) - if err != nil { - currentDelegatorIterator.Release() - return 0, err - } + // Delay rounding as long as possible for small numbers + if optimisticReward, err := safemath.Mul64(remainderShares, totalAmount); err == nil { + remainderAmount = optimisticReward / reward.PercentDenominator } - currentDelegatorIterator.Release() - currentDelegatorIterator, err = chainState.GetCurrentDelegatorIterator(validator.SubnetID, validator.NodeID) - if err != nil { - return 0, err - } - pendingDelegatorIterator, err := chainState.GetPendingDelegatorIterator(validator.SubnetID, validator.NodeID) - if err != nil { - currentDelegatorIterator.Release() - return 0, err - } - delegatorChangesIterator := state.NewStakerDiffIterator(currentDelegatorIterator, pendingDelegatorIterator) - defer delegatorChangesIterator.Release() - - // Iterate over the future stake weight changes and calculate the maximum - // total weight on the validator, only including the points in the time - // range [startTime, endTime]. - var currentMax uint64 - for delegatorChangesIterator.Next() { - delegator, isAdded := delegatorChangesIterator.Value() - // [delegator.NextTime] > [endTime] - if delegator.NextTime.After(endTime) { - // This delegation change (and all following changes) occurs after - // [endTime]. Since we're calculating the max amount staked in - // [startTime, endTime], we can stop. - break - } - - // [delegator.NextTime] >= [startTime] - if !delegator.NextTime.Before(startTime) { - // We have advanced time to be at the inside of the delegation - // window. Make sure that the max weight is updated accordingly. - currentMax = math.Max(currentMax, currentWeight) - } - - var op func(uint64, uint64) (uint64, error) - if isAdded { - op = math.Add64 - } else { - op = math.Sub[uint64] - } - currentWeight, err = op(currentWeight, delegator.Weight) - if err != nil { - return 0, err - } + amountFromShares := totalAmount - remainderAmount + if amountFromShares > sharesAmountCap { + remainderAmount += amountFromShares - sharesAmountCap + amountFromShares = sharesAmountCap } - // Because we assume [startTime] < [endTime], we have advanced time to - // be at the end of the delegation window. Make sure that the max weight is - // updated accordingly. - return math.Max(currentMax, currentWeight), nil + return remainderAmount, amountFromShares } diff --git a/vms/platformvm/txs/executor/staker_tx_verification.go b/vms/platformvm/txs/executor/staker_tx_verification.go index 304c2d73edf8..6ccbb768c543 100644 --- a/vms/platformvm/txs/executor/staker_tx_verification.go +++ b/vms/platformvm/txs/executor/staker_tx_verification.go @@ -6,7 +6,6 @@ package executor import ( "errors" "fmt" - "time" stdmath "math" @@ -577,50 +576,6 @@ func verifyAddPermissionlessValidatorTx( return nil } -type addValidatorRules struct { - assetID ids.ID - minValidatorStake uint64 - maxValidatorStake uint64 - minStakeDuration time.Duration - maxStakeDuration time.Duration - minDelegationFee uint32 -} - -func getValidatorRules( - backend *Backend, - chainState state.Chain, - subnetID ids.ID, -) (*addValidatorRules, error) { - if subnetID == constants.PrimaryNetworkID { - return &addValidatorRules{ - assetID: backend.Ctx.AVAXAssetID, - minValidatorStake: backend.Config.MinValidatorStake, - maxValidatorStake: backend.Config.MaxValidatorStake, - minStakeDuration: backend.Config.MinStakeDuration, - maxStakeDuration: backend.Config.MaxStakeDuration, - minDelegationFee: backend.Config.MinDelegationFee, - }, nil - } - - transformSubnetIntf, err := chainState.GetSubnetTransformation(subnetID) - if err != nil { - return nil, err - } - transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) - if !ok { - return nil, ErrIsNotTransformSubnetTx - } - - return &addValidatorRules{ - assetID: transformSubnet.AssetID, - minValidatorStake: transformSubnet.MinValidatorStake, - maxValidatorStake: transformSubnet.MaxValidatorStake, - minStakeDuration: time.Duration(transformSubnet.MinStakeDuration) * time.Second, - maxStakeDuration: time.Duration(transformSubnet.MaxStakeDuration) * time.Second, - minDelegationFee: transformSubnet.MinDelegationFee, - }, nil -} - // verifyAddPermissionlessDelegatorTx carries out the validation for an // AddPermissionlessDelegatorTx. func verifyAddPermissionlessDelegatorTx( @@ -764,47 +719,3 @@ func verifyAddPermissionlessDelegatorTx( return nil } - -type addDelegatorRules struct { - assetID ids.ID - minDelegatorStake uint64 - maxValidatorStake uint64 - minStakeDuration time.Duration - maxStakeDuration time.Duration - maxValidatorWeightFactor byte -} - -func getDelegatorRules( - backend *Backend, - chainState state.Chain, - subnetID ids.ID, -) (*addDelegatorRules, error) { - if subnetID == constants.PrimaryNetworkID { - return &addDelegatorRules{ - assetID: backend.Ctx.AVAXAssetID, - minDelegatorStake: backend.Config.MinDelegatorStake, - maxValidatorStake: backend.Config.MaxValidatorStake, - minStakeDuration: backend.Config.MinStakeDuration, - maxStakeDuration: backend.Config.MaxStakeDuration, - maxValidatorWeightFactor: MaxValidatorWeightFactor, - }, nil - } - - transformSubnetIntf, err := chainState.GetSubnetTransformation(subnetID) - if err != nil { - return nil, err - } - transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) - if !ok { - return nil, ErrIsNotTransformSubnetTx - } - - return &addDelegatorRules{ - assetID: transformSubnet.AssetID, - minDelegatorStake: transformSubnet.MinDelegatorStake, - maxValidatorStake: transformSubnet.MaxValidatorStake, - minStakeDuration: time.Duration(transformSubnet.MinStakeDuration) * time.Second, - maxStakeDuration: time.Duration(transformSubnet.MaxStakeDuration) * time.Second, - maxValidatorWeightFactor: transformSubnet.MaxValidatorWeightFactor, - }, nil -} diff --git a/vms/platformvm/txs/executor/staker_tx_verification_helpers.go b/vms/platformvm/txs/executor/staker_tx_verification_helpers.go new file mode 100644 index 000000000000..a4d1e8fb596c --- /dev/null +++ b/vms/platformvm/txs/executor/staker_tx_verification_helpers.go @@ -0,0 +1,258 @@ +// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package executor + +import ( + "time" + + "github.com/ava-labs/avalanchego/database" + "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/math" + "github.com/ava-labs/avalanchego/vms/platformvm/state" + "github.com/ava-labs/avalanchego/vms/platformvm/txs" +) + +type addValidatorRules struct { + assetID ids.ID + minValidatorStake uint64 + maxValidatorStake uint64 + minStakeDuration time.Duration + maxStakeDuration time.Duration + minDelegationFee uint32 +} + +func getValidatorRules( + backend *Backend, + chainState state.Chain, + subnetID ids.ID, +) (*addValidatorRules, error) { + if subnetID == constants.PrimaryNetworkID { + return &addValidatorRules{ + assetID: backend.Ctx.AVAXAssetID, + minValidatorStake: backend.Config.MinValidatorStake, + maxValidatorStake: backend.Config.MaxValidatorStake, + minStakeDuration: backend.Config.MinStakeDuration, + maxStakeDuration: backend.Config.MaxStakeDuration, + minDelegationFee: backend.Config.MinDelegationFee, + }, nil + } + + transformSubnetIntf, err := chainState.GetSubnetTransformation(subnetID) + if err != nil { + return nil, err + } + transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) + if !ok { + return nil, ErrIsNotTransformSubnetTx + } + + return &addValidatorRules{ + assetID: transformSubnet.AssetID, + minValidatorStake: transformSubnet.MinValidatorStake, + maxValidatorStake: transformSubnet.MaxValidatorStake, + minStakeDuration: time.Duration(transformSubnet.MinStakeDuration) * time.Second, + maxStakeDuration: time.Duration(transformSubnet.MaxStakeDuration) * time.Second, + minDelegationFee: transformSubnet.MinDelegationFee, + }, nil +} + +type addDelegatorRules struct { + assetID ids.ID + minDelegatorStake uint64 + maxValidatorStake uint64 + minStakeDuration time.Duration + maxStakeDuration time.Duration + maxValidatorWeightFactor byte +} + +func getDelegatorRules( + backend *Backend, + chainState state.Chain, + subnetID ids.ID, +) (*addDelegatorRules, error) { + if subnetID == constants.PrimaryNetworkID { + return &addDelegatorRules{ + assetID: backend.Ctx.AVAXAssetID, + minDelegatorStake: backend.Config.MinDelegatorStake, + maxValidatorStake: backend.Config.MaxValidatorStake, + minStakeDuration: backend.Config.MinStakeDuration, + maxStakeDuration: backend.Config.MaxStakeDuration, + maxValidatorWeightFactor: MaxValidatorWeightFactor, + }, nil + } + + transformSubnetIntf, err := chainState.GetSubnetTransformation(subnetID) + if err != nil { + return nil, err + } + transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) + if !ok { + return nil, ErrIsNotTransformSubnetTx + } + + return &addDelegatorRules{ + assetID: transformSubnet.AssetID, + minDelegatorStake: transformSubnet.MinDelegatorStake, + maxValidatorStake: transformSubnet.MaxValidatorStake, + minStakeDuration: time.Duration(transformSubnet.MinStakeDuration) * time.Second, + maxStakeDuration: time.Duration(transformSubnet.MaxStakeDuration) * time.Second, + maxValidatorWeightFactor: transformSubnet.MaxValidatorWeightFactor, + }, nil +} + +// GetNextStakerChangeTime returns the next time a staker will be either added +// or removed to/from the current validator set. +func GetNextStakerChangeTime(state state.Chain) (time.Time, error) { + currentStakerIterator, err := state.GetCurrentStakerIterator() + if err != nil { + return time.Time{}, err + } + defer currentStakerIterator.Release() + + pendingStakerIterator, err := state.GetPendingStakerIterator() + if err != nil { + return time.Time{}, err + } + defer pendingStakerIterator.Release() + + hasCurrentStaker := currentStakerIterator.Next() + hasPendingStaker := pendingStakerIterator.Next() + switch { + case hasCurrentStaker && hasPendingStaker: + nextCurrentTime := currentStakerIterator.Value().NextTime + nextPendingTime := pendingStakerIterator.Value().NextTime + if nextCurrentTime.Before(nextPendingTime) { + return nextCurrentTime, nil + } + return nextPendingTime, nil + case hasCurrentStaker: + return currentStakerIterator.Value().NextTime, nil + case hasPendingStaker: + return pendingStakerIterator.Value().NextTime, nil + default: + return time.Time{}, database.ErrNotFound + } +} + +// GetValidator returns information about the given validator, which may be a +// current validator or pending validator. +func GetValidator(state state.Chain, subnetID ids.ID, nodeID ids.NodeID) (*state.Staker, error) { + validator, err := state.GetCurrentValidator(subnetID, nodeID) + if err == nil { + // This node is currently validating the subnet. + return validator, nil + } + if err != database.ErrNotFound { + // Unexpected error occurred. + return nil, err + } + return state.GetPendingValidator(subnetID, nodeID) +} + +// overDelegated returns true if [validator] will be overdelegated when adding [delegator]. +// +// A [validator] would become overdelegated if: +// - the maximum total weight on [validator] exceeds [weightLimit] +func overDelegated( + state state.Chain, + validator *state.Staker, + weightLimit uint64, + delegator *state.Staker, +) (bool, error) { + maxWeight, err := GetMaxWeight(state, validator, delegator.StartTime, delegator.EndTime) + if err != nil { + return true, err + } + newMaxWeight, err := math.Add64(maxWeight, delegator.Weight) + if err != nil { + return true, err + } + return newMaxWeight > weightLimit, nil +} + +// GetMaxWeight returns the maximum total weight of the [validator], including +// its own weight, between [startTime] and [endTime]. +// The weight changes are applied in the order they will be applied as chain +// time advances. +// Invariant: +// - [validator.StartTime] <= [startTime] < [endTime] <= [validator.EndTime] +func GetMaxWeight( + chainState state.Chain, + validator *state.Staker, + startTime time.Time, + endTime time.Time, +) (uint64, error) { + currentDelegatorIterator, err := chainState.GetCurrentDelegatorIterator(validator.SubnetID, validator.NodeID) + if err != nil { + return 0, err + } + + // TODO: We can optimize this by moving the current total weight to be + // stored in the validator state. + // + // Calculate the current total weight on this validator, including the + // weight of the actual validator and the sum of the weights of all of the + // currently active delegators. + currentWeight := validator.Weight + for currentDelegatorIterator.Next() { + currentDelegator := currentDelegatorIterator.Value() + + currentWeight, err = math.Add64(currentWeight, currentDelegator.Weight) + if err != nil { + currentDelegatorIterator.Release() + return 0, err + } + } + currentDelegatorIterator.Release() + + currentDelegatorIterator, err = chainState.GetCurrentDelegatorIterator(validator.SubnetID, validator.NodeID) + if err != nil { + return 0, err + } + pendingDelegatorIterator, err := chainState.GetPendingDelegatorIterator(validator.SubnetID, validator.NodeID) + if err != nil { + currentDelegatorIterator.Release() + return 0, err + } + delegatorChangesIterator := state.NewStakerDiffIterator(currentDelegatorIterator, pendingDelegatorIterator) + defer delegatorChangesIterator.Release() + + // Iterate over the future stake weight changes and calculate the maximum + // total weight on the validator, only including the points in the time + // range [startTime, endTime]. + var currentMax uint64 + for delegatorChangesIterator.Next() { + delegator, isAdded := delegatorChangesIterator.Value() + // [delegator.NextTime] > [endTime] + if delegator.NextTime.After(endTime) { + // This delegation change (and all following changes) occurs after + // [endTime]. Since we're calculating the max amount staked in + // [startTime, endTime], we can stop. + break + } + + // [delegator.NextTime] >= [startTime] + if !delegator.NextTime.Before(startTime) { + // We have advanced time to be at the inside of the delegation + // window. Make sure that the max weight is updated accordingly. + currentMax = math.Max(currentMax, currentWeight) + } + + var op func(uint64, uint64) (uint64, error) + if isAdded { + op = math.Add64 + } else { + op = math.Sub[uint64] + } + currentWeight, err = op(currentWeight, delegator.Weight) + if err != nil { + return 0, err + } + } + // Because we assume [startTime] < [endTime], we have advanced time to + // be at the end of the delegation window. Make sure that the max weight is + // updated accordingly. + return math.Max(currentMax, currentWeight), nil +} From 337a54fe65b5b36c457344877cd96aaf2fc9e516 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Tue, 22 Aug 2023 22:28:26 +0200 Subject: [PATCH 02/15] fix + nit --- .../txs/executor/proposal_tx_executor.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index cd77e56dc1e7..fd02f4aefcdb 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -392,9 +392,14 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error } e.OnAbortState.SetCurrentSupply(stakerToReward.SubnetID, newSupply) - // handle staker lifecycle - e.OnCommitState.DeleteCurrentValidator(stakerToReward) - e.OnAbortState.DeleteCurrentValidator(stakerToReward) + // here both commit and abort state supplies are correct. Handle staker lifecycle. + if _, ok := stakerTx.Unsigned.(txs.ValidatorTx); ok { + e.OnCommitState.DeleteCurrentValidator(stakerToReward) + e.OnAbortState.DeleteCurrentValidator(stakerToReward) + } else { // must be txs.DelegatorTx due to switch check above + e.OnCommitState.DeleteCurrentDelegator(stakerToReward) + e.OnAbortState.DeleteCurrentDelegator(stakerToReward) + } // handle option preference shouldCommit, err := e.calculateProposalPreference(stakerToReward, primaryNetworkValidator) @@ -559,9 +564,6 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del // Calculate split of reward between delegator/delegatee delegatorReward, delegateeReward := splitAmountByShares(delegator.PotentialReward, vdrTx.Shares(), math.MaxUint64) - // following Continuous staking fork activation multiple rewards UTXOS - // can be cumulated, each related to a different staking period. We make - // sure to index the reward UTXOs correctly by appending them to previous ones. utxosOffset := len(outputs) + len(stake) currentRewardUTXOs, err := e.OnCommitState.GetRewardUTXOs(delegator.TxID) if err != nil { From 133a58d59dd82fa0282c8aa90d1e126b81ffac4c Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Tue, 22 Aug 2023 22:39:35 +0200 Subject: [PATCH 03/15] another fix --- vms/platformvm/blocks/executor/proposal_block_test.go | 2 ++ vms/platformvm/txs/executor/proposal_tx_executor.go | 10 ---------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/vms/platformvm/blocks/executor/proposal_block_test.go b/vms/platformvm/blocks/executor/proposal_block_test.go index e96af6d57d87..17642db4c16c 100644 --- a/vms/platformvm/blocks/executor/proposal_block_test.go +++ b/vms/platformvm/blocks/executor/proposal_block_test.go @@ -95,6 +95,7 @@ func TestApricotProposalBlockTimeVerification(t *testing.T) { NodeID: utx.NodeID(), SubnetID: utx.SubnetID(), StartTime: utx.StartTime(), + NextTime: chainTime, EndTime: chainTime, }).Times(2) currentStakersIt.EXPECT().Release() @@ -104,6 +105,7 @@ func TestApricotProposalBlockTimeVerification(t *testing.T) { NodeID: utx.NodeID(), SubnetID: utx.SubnetID(), StartTime: utx.StartTime(), + NextTime: chainTime, EndTime: chainTime, }, nil) onParentAccept.EXPECT().GetTx(addValTx.ID()).Return(addValTx, status.Committed, nil) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index fd02f4aefcdb..bf5b8205a7ce 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -436,11 +436,6 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val } utxosOffset := len(outputs) + len(stake) - currentRewardUTXOs, err := e.OnCommitState.GetRewardUTXOs(txID) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - utxosOffset += len(currentRewardUTXOs) // Provide the reward here rewardToPayBack := validator.PotentialReward @@ -565,11 +560,6 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del delegatorReward, delegateeReward := splitAmountByShares(delegator.PotentialReward, vdrTx.Shares(), math.MaxUint64) utxosOffset := len(outputs) + len(stake) - currentRewardUTXOs, err := e.OnCommitState.GetRewardUTXOs(delegator.TxID) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - utxosOffset += len(currentRewardUTXOs) // Reward the delegator here delRewardToPayBack := delegatorReward From 40cfa7561fe82d46744c3535354bf42b8dcf3c3f Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Wed, 23 Aug 2023 00:42:38 +0200 Subject: [PATCH 04/15] nit --- vms/platformvm/txs/executor/proposal_tx_executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index bf5b8205a7ce..0a75cde5b3b8 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -469,7 +469,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val validator.SubnetID, validator.NodeID, ) - if err != nil && err != database.ErrNotFound { + if err != nil { return fmt.Errorf("failed to fetch accrued delegatee rewards: %w", err) } From fa0515ebefe7ebb642554eb8943bbfc1a5f7e2cc Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Wed, 30 Aug 2023 16:43:42 +0200 Subject: [PATCH 05/15] nits --- .../txs/executor/proposal_tx_executor.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index 0a75cde5b3b8..ef2a2ac28a22 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -402,7 +402,7 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error } // handle option preference - shouldCommit, err := e.calculateProposalPreference(stakerToReward, primaryNetworkValidator) + shouldCommit, err := e.shouldBeRewarded(stakerToReward, primaryNetworkValidator) if err != nil { return err } @@ -438,10 +438,10 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val utxosOffset := len(outputs) + len(stake) // Provide the reward here - rewardToPayBack := validator.PotentialReward - if rewardToPayBack > 0 { + reward := validator.PotentialReward + if reward > 0 { validationRewardsOwner := uValidatorTx.ValidationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(rewardToPayBack, validationRewardsOwner) + outIntf, err := e.Fx.CreateOutput(reward, validationRewardsOwner) if err != nil { return fmt.Errorf("failed to create output: %w", err) } @@ -465,7 +465,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val } // Provide the accrued delegatee rewards from successful delegations here. - delegateeRewardToPayBack, err := e.OnCommitState.GetDelegateeReward( + delegateeReward, err := e.OnCommitState.GetDelegateeReward( validator.SubnetID, validator.NodeID, ) @@ -473,9 +473,9 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val return fmt.Errorf("failed to fetch accrued delegatee rewards: %w", err) } - if delegateeRewardToPayBack > 0 { + if delegateeReward > 0 { delegationRewardsOwner := uValidatorTx.DelegationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegateeRewardToPayBack, delegationRewardsOwner) + outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) if err != nil { return fmt.Errorf("failed to create output: %w", err) } @@ -562,10 +562,10 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del utxosOffset := len(outputs) + len(stake) // Reward the delegator here - delRewardToPayBack := delegatorReward - if delRewardToPayBack > 0 { + reward := delegatorReward + if reward > 0 { rewardsOwner := uDelegatorTx.RewardsOwner() - outIntf, err := e.Fx.CreateOutput(delRewardToPayBack, rewardsOwner) + outIntf, err := e.Fx.CreateOutput(reward, rewardsOwner) if err != nil { return fmt.Errorf("failed to create output: %w", err) } @@ -605,7 +605,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del newDelegateeReward := previousDelegateeReward + delegateeReward // For any validators starting after [CortinaTime], we defer rewarding the - // [rewardToPayBack] until their staking period is over. + // [reward] until their staking period is over. err = e.OnCommitState.SetDelegateeReward( validator.SubnetID, validator.NodeID, @@ -642,7 +642,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del return nil } -func (e *ProposalTxExecutor) calculateProposalPreference(stakerToReward, primaryNetworkValidator *state.Staker) (bool, error) { +func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkValidator *state.Staker) (bool, error) { var expectedUptimePercentage float64 if stakerToReward.SubnetID != constants.PrimaryNetworkID { transformSubnetIntf, err := e.OnCommitState.GetSubnetTransformation(stakerToReward.SubnetID) @@ -651,7 +651,7 @@ func (e *ProposalTxExecutor) calculateProposalPreference(stakerToReward, primary } transformSubnet, ok := transformSubnetIntf.Unsigned.(*txs.TransformSubnetTx) if !ok { - return false, fmt.Errorf("failed to calculate uptime: %w", err) + return false, fmt.Errorf("failed to calculate uptime: %w", ErrIsNotTransformSubnetTx) } expectedUptimePercentage = float64(transformSubnet.UptimeRequirement) / reward.PercentDenominator From 57ff17b5a2a1586cbb4db7b80fc61000c65f4866 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Thu, 31 Aug 2023 10:25:26 +0200 Subject: [PATCH 06/15] reverted utxos offset handling --- .../txs/executor/proposal_tx_executor.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index ef2a2ac28a22..efbc5d9be5a7 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -435,7 +435,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val e.OnAbortState.AddUTXO(utxo) } - utxosOffset := len(outputs) + len(stake) + utxosOffset := 0 // Provide the reward here reward := validator.PotentialReward @@ -453,7 +453,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val utxo := &avax.UTXO{ UTXOID: avax.UTXOID{ TxID: txID, - OutputIndex: uint32(utxosOffset), + OutputIndex: uint32(len(outputs) + len(stake)), }, Asset: stakeAsset, Out: out, @@ -487,7 +487,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val onCommitUtxo := &avax.UTXO{ UTXOID: avax.UTXOID{ TxID: txID, - OutputIndex: uint32(utxosOffset), + OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), }, Asset: stakeAsset, Out: out, @@ -500,7 +500,7 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val onAbortUtxo := &avax.UTXO{ UTXOID: avax.UTXOID{ TxID: txID, - OutputIndex: uint32(utxosOffset - 1), + OutputIndex: uint32(len(outputs) + len(stake)), }, Asset: stakeAsset, Out: out, @@ -559,7 +559,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del // Calculate split of reward between delegator/delegatee delegatorReward, delegateeReward := splitAmountByShares(delegator.PotentialReward, vdrTx.Shares(), math.MaxUint64) - utxosOffset := len(outputs) + len(stake) + utxosOffset := 0 // Reward the delegator here reward := delegatorReward @@ -576,7 +576,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del utxo := &avax.UTXO{ UTXOID: avax.UTXOID{ TxID: txID, - OutputIndex: uint32(utxosOffset), + OutputIndex: uint32(len(outputs) + len(stake)), }, Asset: stakeAsset, Out: out, @@ -629,7 +629,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del utxo := &avax.UTXO{ UTXOID: avax.UTXOID{ TxID: txID, - OutputIndex: uint32(utxosOffset), + OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), }, Asset: stakeAsset, Out: out, From 50b8bdfcf7877d8bc0664f303a8313ed55ff6187 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Fri, 8 Sep 2023 17:23:01 +0200 Subject: [PATCH 07/15] exported SplitByShare --- vms/platformvm/txs/executor/proposal_tx_executor.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index efbc5d9be5a7..d46cc3160e52 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -6,7 +6,6 @@ package executor import ( "errors" "fmt" - "math" "time" "github.com/ava-labs/avalanchego/database" @@ -557,7 +556,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del } // Calculate split of reward between delegator/delegatee - delegatorReward, delegateeReward := splitAmountByShares(delegator.PotentialReward, vdrTx.Shares(), math.MaxUint64) + delegatorReward, delegateeReward := SplitAmountByShares(delegator.PotentialReward, vdrTx.Shares()) utxosOffset := 0 @@ -671,7 +670,7 @@ func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkVali return uptime >= expectedUptimePercentage, nil } -func splitAmountByShares(totalAmount uint64, shares uint32, sharesAmountCap uint64) (uint64, uint64) { +func SplitAmountByShares(totalAmount uint64, shares uint32) (uint64, uint64) { remainderShares := reward.PercentDenominator - uint64(shares) remainderAmount := remainderShares * (totalAmount / reward.PercentDenominator) @@ -681,9 +680,5 @@ func splitAmountByShares(totalAmount uint64, shares uint32, sharesAmountCap uint } amountFromShares := totalAmount - remainderAmount - if amountFromShares > sharesAmountCap { - remainderAmount += amountFromShares - sharesAmountCap - amountFromShares = sharesAmountCap - } return remainderAmount, amountFromShares } From 9c0935441a32666c2aa73384d5e392c16de3e4a3 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Fri, 8 Sep 2023 17:28:09 +0200 Subject: [PATCH 08/15] moved Split to reward package --- vms/platformvm/reward/calculator.go | 15 +++++++++++++++ .../txs/executor/proposal_tx_executor.go | 15 +-------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/vms/platformvm/reward/calculator.go b/vms/platformvm/reward/calculator.go index 25c560d2b8ff..8dc24b19b2be 100644 --- a/vms/platformvm/reward/calculator.go +++ b/vms/platformvm/reward/calculator.go @@ -6,6 +6,8 @@ package reward import ( "math/big" "time" + + safemath "github.com/ava-labs/avalanchego/utils/math" ) var _ Calculator = (*calculator)(nil) @@ -67,3 +69,16 @@ func (c *calculator) Calculate(stakedDuration time.Duration, stakedAmount, curre return finalReward } + +func Split(totalAmount uint64, shares uint32) (uint64, uint64) { + remainderShares := PercentDenominator - uint64(shares) + remainderAmount := remainderShares * (totalAmount / PercentDenominator) + + // Delay rounding as long as possible for small numbers + if optimisticReward, err := safemath.Mul64(remainderShares, totalAmount); err == nil { + remainderAmount = optimisticReward / PercentDenominator + } + + amountFromShares := totalAmount - remainderAmount + return remainderAmount, amountFromShares +} diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index d46cc3160e52..8c194aca1c3b 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -556,7 +556,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del } // Calculate split of reward between delegator/delegatee - delegatorReward, delegateeReward := SplitAmountByShares(delegator.PotentialReward, vdrTx.Shares()) + delegatorReward, delegateeReward := reward.Split(delegator.PotentialReward, vdrTx.Shares()) utxosOffset := 0 @@ -669,16 +669,3 @@ func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkVali } return uptime >= expectedUptimePercentage, nil } - -func SplitAmountByShares(totalAmount uint64, shares uint32) (uint64, uint64) { - remainderShares := reward.PercentDenominator - uint64(shares) - remainderAmount := remainderShares * (totalAmount / reward.PercentDenominator) - - // Delay rounding as long as possible for small numbers - if optimisticReward, err := safemath.Mul64(remainderShares, totalAmount); err == nil { - remainderAmount = optimisticReward / reward.PercentDenominator - } - - amountFromShares := totalAmount - remainderAmount - return remainderAmount, amountFromShares -} From d6c9e23807f577acbc0bf486b52c513d97f32f92 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Fri, 8 Sep 2023 17:49:34 +0200 Subject: [PATCH 09/15] nits --- .../txs/executor/proposal_tx_executor.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index 8c194aca1c3b..c429bc6e3830 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -368,10 +368,19 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error if err := e.rewardValidatorTx(uStakerTx, stakerToReward); err != nil { return err } + + // Handle staker lifecycle. + e.OnCommitState.DeleteCurrentValidator(stakerToReward) + e.OnAbortState.DeleteCurrentValidator(stakerToReward) + case txs.DelegatorTx: if err := e.rewardDelegatorTx(uStakerTx, stakerToReward); err != nil { return err } + + // Handle staker lifecycle. + e.OnCommitState.DeleteCurrentDelegator(stakerToReward) + e.OnAbortState.DeleteCurrentDelegator(stakerToReward) default: // Invariant: Permissioned stakers are removed by the advancement of // time and the current chain timestamp is == this staker's @@ -391,15 +400,6 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error } e.OnAbortState.SetCurrentSupply(stakerToReward.SubnetID, newSupply) - // here both commit and abort state supplies are correct. Handle staker lifecycle. - if _, ok := stakerTx.Unsigned.(txs.ValidatorTx); ok { - e.OnCommitState.DeleteCurrentValidator(stakerToReward) - e.OnAbortState.DeleteCurrentValidator(stakerToReward) - } else { // must be txs.DelegatorTx due to switch check above - e.OnCommitState.DeleteCurrentDelegator(stakerToReward) - e.OnAbortState.DeleteCurrentDelegator(stakerToReward) - } - // handle option preference shouldCommit, err := e.shouldBeRewarded(stakerToReward, primaryNetworkValidator) if err != nil { From 8c2814fffa4bd7517b396c4771f81d60fbd46217 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Mon, 18 Sep 2023 12:19:32 +0200 Subject: [PATCH 10/15] nits --- vms/platformvm/reward/calculator.go | 6 +- .../txs/executor/proposal_tx_executor.go | 116 +++++++++--------- 2 files changed, 60 insertions(+), 62 deletions(-) diff --git a/vms/platformvm/reward/calculator.go b/vms/platformvm/reward/calculator.go index 8dc24b19b2be..de50f870129c 100644 --- a/vms/platformvm/reward/calculator.go +++ b/vms/platformvm/reward/calculator.go @@ -7,7 +7,7 @@ import ( "math/big" "time" - safemath "github.com/ava-labs/avalanchego/utils/math" + "github.com/ava-labs/avalanchego/utils/math" ) var _ Calculator = (*calculator)(nil) @@ -70,12 +70,14 @@ func (c *calculator) Calculate(stakedDuration time.Duration, stakedAmount, curre return finalReward } +// [Split] splits [totalAmount] according to [shares] percentage. +// It returns the absolute amounts corresponding to [shares] and [PercentDenominator-shares]. func Split(totalAmount uint64, shares uint32) (uint64, uint64) { remainderShares := PercentDenominator - uint64(shares) remainderAmount := remainderShares * (totalAmount / PercentDenominator) // Delay rounding as long as possible for small numbers - if optimisticReward, err := safemath.Mul64(remainderShares, totalAmount); err == nil { + if optimisticReward, err := math.Mul64(remainderShares, totalAmount); err == nil { remainderAmount = optimisticReward / PercentDenominator } diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index c429bc6e3830..c12afe12bd8c 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -11,13 +11,12 @@ import ( "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/math" "github.com/ava-labs/avalanchego/vms/components/avax" "github.com/ava-labs/avalanchego/vms/components/verify" "github.com/ava-labs/avalanchego/vms/platformvm/reward" "github.com/ava-labs/avalanchego/vms/platformvm/state" "github.com/ava-labs/avalanchego/vms/platformvm/txs" - - safemath "github.com/ava-labs/avalanchego/utils/math" ) const ( @@ -335,13 +334,13 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error // Verify that the chain's timestamp is the validator's end time currentChainTime := e.OnCommitState.GetTimestamp() - if !stakerToReward.NextTime.Equal(currentChainTime) { + if !stakerToReward.EndTime.Equal(currentChainTime) { return fmt.Errorf( "%w: TxID = %s with %s < %s", ErrRemoveStakerTooEarly, tx.TxID, currentChainTime, - stakerToReward.NextTime, + stakerToReward.EndTime, ) } @@ -394,20 +393,15 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error if err != nil { return err } - newSupply, err := safemath.Sub(currentSupply, stakerToReward.PotentialReward) + newSupply, err := math.Sub(currentSupply, stakerToReward.PotentialReward) if err != nil { return err } e.OnAbortState.SetCurrentSupply(stakerToReward.SubnetID, newSupply) // handle option preference - shouldCommit, err := e.shouldBeRewarded(stakerToReward, primaryNetworkValidator) - if err != nil { - return err - } - - e.PrefersCommit = shouldCommit - return nil + e.PrefersCommit, err = e.shouldBeRewarded(stakerToReward, primaryNetworkValidator) + return err } func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, validator *state.Staker) error { @@ -587,56 +581,58 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del utxosOffset++ } + if delegateeReward == 0 { + return nil + } + // Reward the delegatee here - if delegateeReward > 0 { - if validator.StartTime.After(e.Config.CortinaTime) { - previousDelegateeReward, err := e.OnCommitState.GetDelegateeReward( - validator.SubnetID, - validator.NodeID, - ) - if err != nil { - return fmt.Errorf("failed to get delegatee reward: %w", err) - } - - // Invariant: The rewards calculator can never return a - // [potentialReward] that would overflow the - // accumulated rewards. - newDelegateeReward := previousDelegateeReward + delegateeReward - - // For any validators starting after [CortinaTime], we defer rewarding the - // [reward] until their staking period is over. - err = e.OnCommitState.SetDelegateeReward( - validator.SubnetID, - validator.NodeID, - newDelegateeReward, - ) - if err != nil { - return fmt.Errorf("failed to update delegatee reward: %w", err) - } - } else { - // For any validators who started prior to [CortinaTime], we issue the - // [delegateeReward] immediately. - delegationRewardsOwner := vdrTx.DelegationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - out, ok := outIntf.(verify.State) - if !ok { - return ErrInvalidState - } - utxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: txID, - OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), - }, - Asset: stakeAsset, - Out: out, - } - - e.OnCommitState.AddUTXO(utxo) - e.OnCommitState.AddRewardUTXO(txID, utxo) + if validator.StartTime.After(e.Config.CortinaTime) { + previousDelegateeReward, err := e.OnCommitState.GetDelegateeReward( + validator.SubnetID, + validator.NodeID, + ) + if err != nil { + return fmt.Errorf("failed to get delegatee reward: %w", err) } + + // Invariant: The rewards calculator can never return a + // [potentialReward] that would overflow the + // accumulated rewards. + newDelegateeReward := previousDelegateeReward + delegateeReward + + // For any validators starting after [CortinaTime], we defer rewarding the + // [reward] until their staking period is over. + err = e.OnCommitState.SetDelegateeReward( + validator.SubnetID, + validator.NodeID, + newDelegateeReward, + ) + if err != nil { + return fmt.Errorf("failed to update delegatee reward: %w", err) + } + } else { + // For any validators who started prior to [CortinaTime], we issue the + // [delegateeReward] immediately. + delegationRewardsOwner := vdrTx.DelegationRewardsOwner() + outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) + if err != nil { + return fmt.Errorf("failed to create output: %w", err) + } + out, ok := outIntf.(verify.State) + if !ok { + return ErrInvalidState + } + utxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), + }, + Asset: stakeAsset, + Out: out, + } + + e.OnCommitState.AddUTXO(utxo) + e.OnCommitState.AddRewardUTXO(txID, utxo) } return nil } From 1848bb14e983b15279b748789f4ee8e1f3020d78 Mon Sep 17 00:00:00 2001 From: Alberto Benegiamo Date: Mon, 18 Sep 2023 17:12:26 +0200 Subject: [PATCH 11/15] fixed comment --- vms/platformvm/reward/calculator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/reward/calculator.go b/vms/platformvm/reward/calculator.go index de50f870129c..8db835ee0194 100644 --- a/vms/platformvm/reward/calculator.go +++ b/vms/platformvm/reward/calculator.go @@ -71,7 +71,7 @@ func (c *calculator) Calculate(stakedDuration time.Duration, stakedAmount, curre } // [Split] splits [totalAmount] according to [shares] percentage. -// It returns the absolute amounts corresponding to [shares] and [PercentDenominator-shares]. +// It returns the absolute amounts corresponding to [PercentDenominator-shares] and [shares]. func Split(totalAmount uint64, shares uint32) (uint64, uint64) { remainderShares := PercentDenominator - uint64(shares) remainderAmount := remainderShares * (totalAmount / PercentDenominator) From e6e5d35bdece1fa1248e219eced57bc7e92ed5bc Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 19 Sep 2023 11:50:32 -0400 Subject: [PATCH 12/15] Add tests --- vms/platformvm/reward/calculator.go | 8 ++- vms/platformvm/reward/calculator_test.go | 63 +++++++++++++++++++ .../txs/executor/proposal_tx_executor.go | 2 +- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/vms/platformvm/reward/calculator.go b/vms/platformvm/reward/calculator.go index 8db835ee0194..83524b374194 100644 --- a/vms/platformvm/reward/calculator.go +++ b/vms/platformvm/reward/calculator.go @@ -70,8 +70,10 @@ func (c *calculator) Calculate(stakedDuration time.Duration, stakedAmount, curre return finalReward } -// [Split] splits [totalAmount] according to [shares] percentage. -// It returns the absolute amounts corresponding to [PercentDenominator-shares] and [shares]. +// [Split] [totalAmount] into [totalAmount * shares percentage] and the +// remainder. +// +// Invariant: [shares] <= [PercentDenominator] func Split(totalAmount uint64, shares uint32) (uint64, uint64) { remainderShares := PercentDenominator - uint64(shares) remainderAmount := remainderShares * (totalAmount / PercentDenominator) @@ -82,5 +84,5 @@ func Split(totalAmount uint64, shares uint32) (uint64, uint64) { } amountFromShares := totalAmount - remainderAmount - return remainderAmount, amountFromShares + return amountFromShares, remainderAmount } diff --git a/vms/platformvm/reward/calculator_test.go b/vms/platformvm/reward/calculator_test.go index cb16435af9c3..592412ce6476 100644 --- a/vms/platformvm/reward/calculator_test.go +++ b/vms/platformvm/reward/calculator_test.go @@ -170,3 +170,66 @@ func TestRewardsMint(t *testing.T) { ) require.Equal(t, maxSupply-initialSupply, rewards) } + +func TestSplit(t *testing.T) { + tests := []struct { + amount uint64 + shares uint32 + expectedSplit uint64 + }{ + { + amount: 1000, + shares: PercentDenominator / 2, + expectedSplit: 500, + }, + { + amount: 1, + shares: PercentDenominator, + expectedSplit: 1, + }, + { + amount: 1, + shares: PercentDenominator - 1, + expectedSplit: 1, + }, + { + amount: 1, + shares: 1, + expectedSplit: 1, + }, + { + amount: 1, + shares: 0, + expectedSplit: 0, + }, + { + amount: 9223374036974675809, + shares: 2, + expectedSplit: 18446748749757, + }, + { + amount: 9223374036974675809, + shares: PercentDenominator, + expectedSplit: 9223374036974675809, + }, + { + amount: 9223372036855275808, + shares: PercentDenominator - 2, + expectedSplit: 9223353590111202098, + }, + { + amount: 9223372036855275808, + shares: 2, + expectedSplit: 18446744349518, + }, + } + for _, test := range tests { + t.Run(fmt.Sprintf("%d_%d", test.amount, test.shares), func(t *testing.T) { + require := require.New(t) + + split, remainder := Split(test.amount, test.shares) + require.Equal(test.expectedSplit, split, int(split)) + require.Equal(test.amount-test.expectedSplit, remainder) + }) + } +} diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index c12afe12bd8c..27e50e4ffd37 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -550,7 +550,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del } // Calculate split of reward between delegator/delegatee - delegatorReward, delegateeReward := reward.Split(delegator.PotentialReward, vdrTx.Shares()) + delegateeReward, delegatorReward := reward.Split(delegator.PotentialReward, vdrTx.Shares()) utxosOffset := 0 From 4fc4710198b83f0adf8b76c3d7da07cb5303bba8 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 19 Sep 2023 18:07:12 -0400 Subject: [PATCH 13/15] nits --- .../txs/executor/proposal_tx_executor.go | 73 +++++++++---------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index 27e50e4ffd37..de6341d8e148 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -371,7 +371,6 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error // Handle staker lifecycle. e.OnCommitState.DeleteCurrentValidator(stakerToReward) e.OnAbortState.DeleteCurrentValidator(stakerToReward) - case txs.DelegatorTx: if err := e.rewardDelegatorTx(uStakerTx, stakerToReward); err != nil { return err @@ -466,41 +465,43 @@ func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, val return fmt.Errorf("failed to fetch accrued delegatee rewards: %w", err) } - if delegateeReward > 0 { - delegationRewardsOwner := uValidatorTx.DelegationRewardsOwner() - outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) - if err != nil { - return fmt.Errorf("failed to create output: %w", err) - } - out, ok := outIntf.(verify.State) - if !ok { - return ErrInvalidState - } - - onCommitUtxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: txID, - OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), - }, - Asset: stakeAsset, - Out: out, - } - e.OnCommitState.AddUTXO(onCommitUtxo) - e.OnCommitState.AddRewardUTXO(txID, onCommitUtxo) + if delegateeReward == 0 { + return nil + } - // Note: There is no [offset] if the RewardValidatorTx is - // aborted, because the validator reward is not awarded. - onAbortUtxo := &avax.UTXO{ - UTXOID: avax.UTXOID{ - TxID: txID, - OutputIndex: uint32(len(outputs) + len(stake)), - }, - Asset: stakeAsset, - Out: out, - } - e.OnAbortState.AddUTXO(onAbortUtxo) - e.OnAbortState.AddRewardUTXO(txID, onAbortUtxo) + delegationRewardsOwner := uValidatorTx.DelegationRewardsOwner() + outIntf, err := e.Fx.CreateOutput(delegateeReward, delegationRewardsOwner) + if err != nil { + return fmt.Errorf("failed to create output: %w", err) } + out, ok := outIntf.(verify.State) + if !ok { + return ErrInvalidState + } + + onCommitUtxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(len(outputs) + len(stake) + utxosOffset), + }, + Asset: stakeAsset, + Out: out, + } + e.OnCommitState.AddUTXO(onCommitUtxo) + e.OnCommitState.AddRewardUTXO(txID, onCommitUtxo) + + // Note: There is no [offset] if the RewardValidatorTx is + // aborted, because the validator reward is not awarded. + onAbortUtxo := &avax.UTXO{ + UTXOID: avax.UTXOID{ + TxID: txID, + OutputIndex: uint32(len(outputs) + len(stake)), + }, + Asset: stakeAsset, + Out: out, + } + e.OnAbortState.AddUTXO(onAbortUtxo) + e.OnAbortState.AddRewardUTXO(txID, onAbortUtxo) return nil } @@ -638,7 +639,7 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del } func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkValidator *state.Staker) (bool, error) { - var expectedUptimePercentage float64 + expectedUptimePercentage := e.Config.UptimePercentage if stakerToReward.SubnetID != constants.PrimaryNetworkID { transformSubnetIntf, err := e.OnCommitState.GetSubnetTransformation(stakerToReward.SubnetID) if err != nil { @@ -650,8 +651,6 @@ func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkVali } expectedUptimePercentage = float64(transformSubnet.UptimeRequirement) / reward.PercentDenominator - } else { - expectedUptimePercentage = e.Config.UptimePercentage } // TODO: calculate subnet uptimes From 1f396e11d51ae5b18e990270b8a3ba9d46d40786 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 19 Sep 2023 18:09:50 -0400 Subject: [PATCH 14/15] cleanup --- vms/platformvm/reward/calculator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vms/platformvm/reward/calculator.go b/vms/platformvm/reward/calculator.go index 83524b374194..30ba7c3270d7 100644 --- a/vms/platformvm/reward/calculator.go +++ b/vms/platformvm/reward/calculator.go @@ -70,8 +70,7 @@ func (c *calculator) Calculate(stakedDuration time.Duration, stakedAmount, curre return finalReward } -// [Split] [totalAmount] into [totalAmount * shares percentage] and the -// remainder. +// Split [totalAmount] into [totalAmount * shares percentage] and the remainder. // // Invariant: [shares] <= [PercentDenominator] func Split(totalAmount uint64, shares uint32) (uint64, uint64) { From 58086294e6f33fa62ca8b088942b9a13fe75cbd0 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 19 Sep 2023 18:10:47 -0400 Subject: [PATCH 15/15] cleanup --- vms/platformvm/reward/calculator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vms/platformvm/reward/calculator_test.go b/vms/platformvm/reward/calculator_test.go index 592412ce6476..1462bd2e3664 100644 --- a/vms/platformvm/reward/calculator_test.go +++ b/vms/platformvm/reward/calculator_test.go @@ -228,7 +228,7 @@ func TestSplit(t *testing.T) { require := require.New(t) split, remainder := Split(test.amount, test.shares) - require.Equal(test.expectedSplit, split, int(split)) + require.Equal(test.expectedSplit, split) require.Equal(test.amount-test.expectedSplit, remainder) }) }