diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index ae423ca9760e..eddd3921e5e0 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -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 diff --git a/vms/platformvm/txs/executor/proposal_tx_executor_test.go b/vms/platformvm/txs/executor/proposal_tx_executor_test.go index c74be119785f..4bebbb4105d9 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor_test.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor_test.go @@ -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), @@ -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", @@ -162,7 +162,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) { feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]}, setup: addMinStakeValidator, AP3Time: defaultGenesisTime, - expectedErr: ErrOverDelegated, + expectedErr: ErrPeriodMismatch, }, { description: "valid", @@ -318,7 +318,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { @@ -444,7 +444,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { @@ -474,7 +474,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { diff --git a/vms/platformvm/txs/executor/staker_tx_verification.go b/vms/platformvm/txs/executor/staker_tx_verification.go index dd13c8834c65..304c2d73edf8 100644 --- a/vms/platformvm/txs/executor/staker_tx_verification.go +++ b/vms/platformvm/txs/executor/staker_tx_verification.go @@ -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") @@ -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) @@ -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 } @@ -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 @@ -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 } diff --git a/vms/platformvm/txs/executor/staker_tx_verification_test.go b/vms/platformvm/txs/executor/staker_tx_verification_test.go index 300b56f027f7..56c0b4dca87e 100644 --- a/vms/platformvm/txs/executor/staker_tx_verification_test.go +++ b/vms/platformvm/txs/executor/staker_tx_verification_test.go @@ -381,7 +381,7 @@ func TestVerifyAddPermissionlessValidatorTx(t *testing.T) { txF: func() *txs.AddPermissionlessValidatorTx { return &verifiedTx }, - expectedErr: ErrValidatorSubset, + expectedErr: ErrPeriodMismatch, }, { name: "flow check fails", diff --git a/vms/platformvm/txs/executor/standard_tx_executor_test.go b/vms/platformvm/txs/executor/standard_tx_executor_test.go index 51453c7b89c7..fb5000e97f6e 100644 --- a/vms/platformvm/txs/executor/standard_tx_executor_test.go +++ b/vms/platformvm/txs/executor/standard_tx_executor_test.go @@ -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), @@ -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", @@ -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", @@ -408,7 +408,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { @@ -523,7 +523,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { @@ -549,7 +549,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) { Tx: tx, } err = tx.Unsigned.Visit(&executor) - require.ErrorIs(err, ErrValidatorSubset) + require.ErrorIs(err, ErrPeriodMismatch) } { diff --git a/vms/platformvm/txs/validator.go b/vms/platformvm/txs/validator.go index 79163392fee7..ea7d048f5074 100644 --- a/vms/platformvm/txs/validator.go +++ b/vms/platformvm/txs/validator.go @@ -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) } diff --git a/vms/platformvm/txs/validator_test.go b/vms/platformvm/txs/validator_test.go index 047c7180193b..3361d11939b4 100644 --- a/vms/platformvm/txs/validator_test.go +++ b/vms/platformvm/txs/validator_test.go @@ -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 @@ -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())) }