Skip to content

Commit

Permalink
Improve delegation error message to specify invalid times or over del…
Browse files Browse the repository at this point in the history 
…egated (ava-labs#1606)
  • Loading branch information
@felipemadero
felipemadero committed Jun 19, 2023
1 parent b85b31c commit c7e1c6a
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 62 deletions.
24 changes: 7 additions & 17 deletions vms/platformvm/txs/executor/proposal_tx_executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,35 +700,25 @@ func GetValidator(state state.Chain, subnetID ids.ID, nodeID ids.NodeID) (*state
return state.GetPendingValidator(subnetID, nodeID)
}

// canDelegate returns true if [delegator] can be added as a delegator of
// [validator].
// overDelegated returns true if [validator] will be overdelegated when adding [delegator].
//
// A [delegator] can be added if:
// - [delegator]'s start time is not before [validator]'s start time
// - [delegator]'s end time is not after [validator]'s end time
// - the maximum total weight on [validator] will not exceed [weightLimit]
func canDelegate(
// 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) {
if delegator.StartTime.Before(validator.StartTime) {
return false, nil
}
if delegator.EndTime.After(validator.EndTime) {
return false, nil
}

maxWeight, err := GetMaxWeight(state, validator, delegator.StartTime, delegator.EndTime)
if err != nil {
return false, err
return true, err
}
newMaxWeight, err := math.Add64(maxWeight, delegator.Weight)
if err != nil {
return false, err
return true, err
}
return newMaxWeight <= weightLimit, nil
return newMaxWeight > weightLimit, nil
}

// GetMaxWeight returns the maximum total weight of the [validator], including
Expand Down
12 changes: 6 additions & 6 deletions vms/platformvm/txs/executor/proposal_tx_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: nil,
AP3Time: defaultGenesisTime,
expectedErr: ErrOverDelegated,
expectedErr: ErrPeriodMismatch,
},
{
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: addMinStakeValidator,
AP3Time: defaultGenesisTime,
expectedErr: ErrOverDelegated,
expectedErr: ErrPeriodMismatch,
},
{
description: "delegator stops before validator",
Expand All @@ -162,7 +162,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: addMinStakeValidator,
AP3Time: defaultGenesisTime,
expectedErr: ErrOverDelegated,
expectedErr: ErrPeriodMismatch,
},
{
description: "valid",
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand Down Expand Up @@ -444,7 +444,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand Down Expand Up @@ -474,7 +474,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand Down
44 changes: 35 additions & 9 deletions vms/platformvm/txs/executor/staker_tx_verification.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ var (
ErrStakeTooLong = errors.New("staking period is too long")
ErrFlowCheckFailed = errors.New("flow check failed")
ErrFutureStakeTime = fmt.Errorf("staker is attempting to start staking more than %s ahead of the current chain time", MaxFutureStartTime)
ErrValidatorSubset = errors.New("all subnets' staking period must be a subset of the primary network")
ErrNotValidator = errors.New("isn't a current or pending validator")
ErrRemovePermissionlessValidator = errors.New("attempting to remove permissionless validator")
ErrStakeOverflow = errors.New("validator stake exceeds limit")
ErrPeriodMismatch = errors.New("proposed staking period is not inside dependant staking period")
ErrOverDelegated = errors.New("validator would be over delegated")
ErrIsNotTransformSubnetTx = errors.New("is not a transform subnet tx")
ErrTimestampNotBeforeStartTime = errors.New("chain timestamp not before start time")
Expand Down Expand Up @@ -216,8 +216,13 @@ func verifyAddSubnetValidatorTx(

// Ensure that the period this validator validates the specified subnet
// is a subset of the time they validate the primary network.
if !tx.Validator.BoundedBy(primaryNetworkValidator.StartTime, primaryNetworkValidator.EndTime) {
return ErrValidatorSubset
if !txs.BoundedBy(
tx.Validator.StartTime(),
tx.Validator.EndTime(),
primaryNetworkValidator.StartTime,
primaryNetworkValidator.EndTime,
) {
return ErrPeriodMismatch
}

baseTxCreds, err := verifyPoASubnetAuthorization(backend, chainState, sTx, tx.SubnetValidator.Subnet, tx.SubnetAuth)
Expand Down Expand Up @@ -392,11 +397,19 @@ func verifyAddDelegatorTx(
return nil, err
}

canDelegate, err := canDelegate(chainState, primaryNetworkValidator, maximumWeight, newStaker)
if !txs.BoundedBy(
newStaker.StartTime,
newStaker.EndTime,
primaryNetworkValidator.StartTime,
primaryNetworkValidator.EndTime,
) {
return nil, ErrPeriodMismatch
}
overDelegated, err := overDelegated(chainState, primaryNetworkValidator, maximumWeight, newStaker)
if err != nil {
return nil, err
}
if !canDelegate {
if overDelegated {
return nil, ErrOverDelegated
}

Expand Down Expand Up @@ -522,8 +535,13 @@ func verifyAddPermissionlessValidatorTx(

// Ensure that the period this validator validates the specified subnet
// is a subset of the time they validate the primary network.
if !tx.Validator.BoundedBy(primaryNetworkValidator.StartTime, primaryNetworkValidator.EndTime) {
return ErrValidatorSubset
if !txs.BoundedBy(
tx.Validator.StartTime(),
tx.Validator.EndTime(),
primaryNetworkValidator.StartTime,
primaryNetworkValidator.EndTime,
) {
return ErrPeriodMismatch
}

txFee = backend.Config.AddSubnetValidatorFee
Expand Down Expand Up @@ -686,11 +704,19 @@ func verifyAddPermissionlessDelegatorTx(
return err
}

canDelegate, err := canDelegate(chainState, validator, maximumWeight, newStaker)
if !txs.BoundedBy(
newStaker.StartTime,
newStaker.EndTime,
validator.StartTime,
validator.EndTime,
) {
return ErrPeriodMismatch
}
overDelegated, err := overDelegated(chainState, validator, maximumWeight, newStaker)
if err != nil {
return err
}
if !canDelegate {
if overDelegated {
return ErrOverDelegated
}

Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/txs/executor/staker_tx_verification_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ func TestVerifyAddPermissionlessValidatorTx(t *testing.T) {
txF: func() *txs.AddPermissionlessValidatorTx {
return &verifiedTx
},
expectedErr: ErrValidatorSubset,
expectedErr: ErrPeriodMismatch,
},
{
name: "flow check fails",
Expand Down
18 changes: 9 additions & 9 deletions vms/platformvm/txs/executor/standard_tx_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: nil,
AP3Time: defaultGenesisTime,
expectedExecutionErr: ErrOverDelegated,
expectedMempoolErr: ErrOverDelegated,
expectedExecutionErr: ErrPeriodMismatch,
expectedMempoolErr: ErrPeriodMismatch,
},
{
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
Expand Down Expand Up @@ -228,8 +228,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: addMinStakeValidator,
AP3Time: defaultGenesisTime,
expectedExecutionErr: ErrOverDelegated,
expectedMempoolErr: ErrOverDelegated,
expectedExecutionErr: ErrPeriodMismatch,
expectedMempoolErr: ErrPeriodMismatch,
},
{
description: "delegator stops before validator",
Expand All @@ -241,8 +241,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: addMinStakeValidator,
AP3Time: defaultGenesisTime,
expectedExecutionErr: ErrOverDelegated,
expectedMempoolErr: ErrOverDelegated,
expectedExecutionErr: ErrPeriodMismatch,
expectedMempoolErr: ErrPeriodMismatch,
},
{
description: "valid",
Expand Down Expand Up @@ -408,7 +408,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand Down Expand Up @@ -523,7 +523,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand All @@ -549,7 +549,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrValidatorSubset)
require.ErrorIs(err, ErrPeriodMismatch)
}

{
Expand Down
9 changes: 4 additions & 5 deletions vms/platformvm/txs/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ func (v *Validator) Verify() error {
}
}

// BoundedBy returns true iff the period that [validator] validates is a
// (non-strict) subset of the time that [other] validates.
// Namely, startTime <= v.StartTime() <= v.EndTime() <= endTime
func (v *Validator) BoundedBy(startTime, endTime time.Time) bool {
return !v.StartTime().Before(startTime) && !v.EndTime().After(endTime)
// BoundedBy returns true iff staker start and end are a
// (non-strict) subset of the provided time bound
func BoundedBy(stakerStart, stakerEnd, lowerBound, upperBound time.Time) bool {
return !stakerStart.Before(lowerBound) && !stakerEnd.After(upperBound) && !stakerEnd.Before(stakerStart)
}
30 changes: 15 additions & 15 deletions vms/platformvm/txs/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const defaultWeight = 10000
// each key controls an address that has [defaultBalance] AVAX at genesis
var keys = secp256k1.TestKeys()

func TestValidatorBoundedBy(t *testing.T) {
func TestBoundedBy(t *testing.T) {
require := require.New(t)

// case 1: a starts, a finishes, b starts, b finishes
Expand All @@ -38,54 +38,54 @@ func TestValidatorBoundedBy(t *testing.T) {
End: bEndTime,
Wght: defaultWeight,
}
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
require.False(BoundedBy(a.StartTime(), b.EndTime(), b.StartTime(), b.EndTime()))
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 2: a starts, b starts, a finishes, b finishes
a.Start = 0
b.Start = 1
a.End = 2
b.End = 3
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 3: a starts, b starts, b finishes, a finishes
a.Start = 0
b.Start = 1
b.End = 2
a.End = 3
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
require.True(b.BoundedBy(a.StartTime(), a.EndTime()))
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.True(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 4: b starts, a starts, a finishes, b finishes
b.Start = 0
a.Start = 1
a.End = 2
b.End = 3
require.True(a.BoundedBy(b.StartTime(), b.EndTime()))
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
require.True(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 5: b starts, b finishes, a starts, a finishes
b.Start = 0
b.End = 1
a.Start = 2
a.End = 3
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 6: b starts, a starts, b finishes, a finishes
b.Start = 0
a.Start = 1
b.End = 2
a.End = 3
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))

// case 3: a starts, b starts, b finishes, a finishes
a.Start = 0
b.Start = 0
b.End = 1
a.End = 1
require.True(a.BoundedBy(b.StartTime(), b.EndTime()))
require.True(b.BoundedBy(a.StartTime(), a.EndTime()))
require.True(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
require.True(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
}

0 comments on commit c7e1c6a

Please sign in to comment.