Skip to content

Commit c7e1c6a

Browse files
felipemaderofelipemadero
authored
Improve delegation error message to specify invalid times or over delegated (#1606)
1 parent b85b31c commit c7e1c6a

File tree

7 files changed

+77
-62
lines changed

7 files changed

+77
-62
lines changed

‎vms/platformvm/txs/executor/proposal_tx_executor.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -700,35 +700,25 @@ func GetValidator(state state.Chain, subnetID ids.ID, nodeID ids.NodeID) (*state
700700
return state.GetPendingValidator(subnetID, nodeID)
701701
}
702702

703-
// canDelegate returns true if [delegator] can be added as a delegator of
704-
// [validator].
703+
// overDelegated returns true if [validator] will be overdelegated when adding [delegator].
705704
//
706-
// A [delegator] can be added if:
707-
// - [delegator]'s start time is not before [validator]'s start time
708-
// - [delegator]'s end time is not after [validator]'s end time
709-
// - the maximum total weight on [validator] will not exceed [weightLimit]
710-
func canDelegate(
705+
// A [validator] would become overdelegated if:
706+
// - the maximum total weight on [validator] exceeds [weightLimit]
707+
func overDelegated(
711708
state state.Chain,
712709
validator *state.Staker,
713710
weightLimit uint64,
714711
delegator *state.Staker,
715712
) (bool, error) {
716-
if delegator.StartTime.Before(validator.StartTime) {
717-
return false, nil
718-
}
719-
if delegator.EndTime.After(validator.EndTime) {
720-
return false, nil
721-
}
722-
723713
maxWeight, err := GetMaxWeight(state, validator, delegator.StartTime, delegator.EndTime)
724714
if err != nil {
725-
return false, err
715+
return true, err
726716
}
727717
newMaxWeight, err := math.Add64(maxWeight, delegator.Weight)
728718
if err != nil {
729-
return false, err
719+
return true, err
730720
}
731-
return newMaxWeight <= weightLimit, nil
721+
return newMaxWeight > weightLimit, nil
732722
}
733723

734724
// GetMaxWeight returns the maximum total weight of the [validator], including

‎vms/platformvm/txs/executor/proposal_tx_executor_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
114114
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
115115
setup: nil,
116116
AP3Time: defaultGenesisTime,
117-
expectedErr: ErrOverDelegated,
117+
expectedErr: ErrPeriodMismatch,
118118
},
119119
{
120120
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
@@ -150,7 +150,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
150150
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
151151
setup: addMinStakeValidator,
152152
AP3Time: defaultGenesisTime,
153-
expectedErr: ErrOverDelegated,
153+
expectedErr: ErrPeriodMismatch,
154154
},
155155
{
156156
description: "delegator stops before validator",
@@ -162,7 +162,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
162162
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
163163
setup: addMinStakeValidator,
164164
AP3Time: defaultGenesisTime,
165-
expectedErr: ErrOverDelegated,
165+
expectedErr: ErrPeriodMismatch,
166166
},
167167
{
168168
description: "valid",
@@ -318,7 +318,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
318318
Tx: tx,
319319
}
320320
err = tx.Unsigned.Visit(&executor)
321-
require.ErrorIs(err, ErrValidatorSubset)
321+
require.ErrorIs(err, ErrPeriodMismatch)
322322
}
323323

324324
{
@@ -444,7 +444,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
444444
Tx: tx,
445445
}
446446
err = tx.Unsigned.Visit(&executor)
447-
require.ErrorIs(err, ErrValidatorSubset)
447+
require.ErrorIs(err, ErrPeriodMismatch)
448448
}
449449

450450
{
@@ -474,7 +474,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) {
474474
Tx: tx,
475475
}
476476
err = tx.Unsigned.Visit(&executor)
477-
require.ErrorIs(err, ErrValidatorSubset)
477+
require.ErrorIs(err, ErrPeriodMismatch)
478478
}
479479

480480
{

‎vms/platformvm/txs/executor/staker_tx_verification.go

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ var (
2727
ErrStakeTooLong = errors.New("staking period is too long")
2828
ErrFlowCheckFailed = errors.New("flow check failed")
2929
ErrFutureStakeTime = fmt.Errorf("staker is attempting to start staking more than %s ahead of the current chain time", MaxFutureStartTime)
30-
ErrValidatorSubset = errors.New("all subnets' staking period must be a subset of the primary network")
3130
ErrNotValidator = errors.New("isn't a current or pending validator")
3231
ErrRemovePermissionlessValidator = errors.New("attempting to remove permissionless validator")
3332
ErrStakeOverflow = errors.New("validator stake exceeds limit")
33+
ErrPeriodMismatch = errors.New("proposed staking period is not inside dependant staking period")
3434
ErrOverDelegated = errors.New("validator would be over delegated")
3535
ErrIsNotTransformSubnetTx = errors.New("is not a transform subnet tx")
3636
ErrTimestampNotBeforeStartTime = errors.New("chain timestamp not before start time")
@@ -216,8 +216,13 @@ func verifyAddSubnetValidatorTx(
216216

217217
// Ensure that the period this validator validates the specified subnet
218218
// is a subset of the time they validate the primary network.
219-
if !tx.Validator.BoundedBy(primaryNetworkValidator.StartTime, primaryNetworkValidator.EndTime) {
220-
return ErrValidatorSubset
219+
if !txs.BoundedBy(
220+
tx.Validator.StartTime(),
221+
tx.Validator.EndTime(),
222+
primaryNetworkValidator.StartTime,
223+
primaryNetworkValidator.EndTime,
224+
) {
225+
return ErrPeriodMismatch
221226
}
222227

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

395-
canDelegate, err := canDelegate(chainState, primaryNetworkValidator, maximumWeight, newStaker)
400+
if !txs.BoundedBy(
401+
newStaker.StartTime,
402+
newStaker.EndTime,
403+
primaryNetworkValidator.StartTime,
404+
primaryNetworkValidator.EndTime,
405+
) {
406+
return nil, ErrPeriodMismatch
407+
}
408+
overDelegated, err := overDelegated(chainState, primaryNetworkValidator, maximumWeight, newStaker)
396409
if err != nil {
397410
return nil, err
398411
}
399-
if !canDelegate {
412+
if overDelegated {
400413
return nil, ErrOverDelegated
401414
}
402415

@@ -522,8 +535,13 @@ func verifyAddPermissionlessValidatorTx(
522535

523536
// Ensure that the period this validator validates the specified subnet
524537
// is a subset of the time they validate the primary network.
525-
if !tx.Validator.BoundedBy(primaryNetworkValidator.StartTime, primaryNetworkValidator.EndTime) {
526-
return ErrValidatorSubset
538+
if !txs.BoundedBy(
539+
tx.Validator.StartTime(),
540+
tx.Validator.EndTime(),
541+
primaryNetworkValidator.StartTime,
542+
primaryNetworkValidator.EndTime,
543+
) {
544+
return ErrPeriodMismatch
527545
}
528546

529547
txFee = backend.Config.AddSubnetValidatorFee
@@ -686,11 +704,19 @@ func verifyAddPermissionlessDelegatorTx(
686704
return err
687705
}
688706

689-
canDelegate, err := canDelegate(chainState, validator, maximumWeight, newStaker)
707+
if !txs.BoundedBy(
708+
newStaker.StartTime,
709+
newStaker.EndTime,
710+
validator.StartTime,
711+
validator.EndTime,
712+
) {
713+
return ErrPeriodMismatch
714+
}
715+
overDelegated, err := overDelegated(chainState, validator, maximumWeight, newStaker)
690716
if err != nil {
691717
return err
692718
}
693-
if !canDelegate {
719+
if overDelegated {
694720
return ErrOverDelegated
695721
}
696722

‎vms/platformvm/txs/executor/staker_tx_verification_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ func TestVerifyAddPermissionlessValidatorTx(t *testing.T) {
381381
txF: func() *txs.AddPermissionlessValidatorTx {
382382
return &verifiedTx
383383
},
384-
expectedErr: ErrValidatorSubset,
384+
expectedErr: ErrPeriodMismatch,
385385
},
386386
{
387387
name: "flow check fails",

‎vms/platformvm/txs/executor/standard_tx_executor_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
189189
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
190190
setup: nil,
191191
AP3Time: defaultGenesisTime,
192-
expectedExecutionErr: ErrOverDelegated,
193-
expectedMempoolErr: ErrOverDelegated,
192+
expectedExecutionErr: ErrPeriodMismatch,
193+
expectedMempoolErr: ErrPeriodMismatch,
194194
},
195195
{
196196
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
@@ -228,8 +228,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
228228
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
229229
setup: addMinStakeValidator,
230230
AP3Time: defaultGenesisTime,
231-
expectedExecutionErr: ErrOverDelegated,
232-
expectedMempoolErr: ErrOverDelegated,
231+
expectedExecutionErr: ErrPeriodMismatch,
232+
expectedMempoolErr: ErrPeriodMismatch,
233233
},
234234
{
235235
description: "delegator stops before validator",
@@ -241,8 +241,8 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
241241
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
242242
setup: addMinStakeValidator,
243243
AP3Time: defaultGenesisTime,
244-
expectedExecutionErr: ErrOverDelegated,
245-
expectedMempoolErr: ErrOverDelegated,
244+
expectedExecutionErr: ErrPeriodMismatch,
245+
expectedMempoolErr: ErrPeriodMismatch,
246246
},
247247
{
248248
description: "valid",
@@ -408,7 +408,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
408408
Tx: tx,
409409
}
410410
err = tx.Unsigned.Visit(&executor)
411-
require.ErrorIs(err, ErrValidatorSubset)
411+
require.ErrorIs(err, ErrPeriodMismatch)
412412
}
413413

414414
{
@@ -523,7 +523,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
523523
Tx: tx,
524524
}
525525
err = tx.Unsigned.Visit(&executor)
526-
require.ErrorIs(err, ErrValidatorSubset)
526+
require.ErrorIs(err, ErrPeriodMismatch)
527527
}
528528

529529
{
@@ -549,7 +549,7 @@ func TestStandardTxExecutorAddSubnetValidator(t *testing.T) {
549549
Tx: tx,
550550
}
551551
err = tx.Unsigned.Visit(&executor)
552-
require.ErrorIs(err, ErrValidatorSubset)
552+
require.ErrorIs(err, ErrPeriodMismatch)
553553
}
554554

555555
{

‎vms/platformvm/txs/validator.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,8 @@ func (v *Validator) Verify() error {
6060
}
6161
}
6262

63-
// BoundedBy returns true iff the period that [validator] validates is a
64-
// (non-strict) subset of the time that [other] validates.
65-
// Namely, startTime <= v.StartTime() <= v.EndTime() <= endTime
66-
func (v *Validator) BoundedBy(startTime, endTime time.Time) bool {
67-
return !v.StartTime().Before(startTime) && !v.EndTime().After(endTime)
63+
// BoundedBy returns true iff staker start and end are a
64+
// (non-strict) subset of the provided time bound
65+
func BoundedBy(stakerStart, stakerEnd, lowerBound, upperBound time.Time) bool {
66+
return !stakerStart.Before(lowerBound) && !stakerEnd.After(upperBound) && !stakerEnd.Before(stakerStart)
6867
}

‎vms/platformvm/txs/validator_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const defaultWeight = 10000
1717
// each key controls an address that has [defaultBalance] AVAX at genesis
1818
var keys = secp256k1.TestKeys()
1919

20-
func TestValidatorBoundedBy(t *testing.T) {
20+
func TestBoundedBy(t *testing.T) {
2121
require := require.New(t)
2222

2323
// case 1: a starts, a finishes, b starts, b finishes
@@ -38,54 +38,54 @@ func TestValidatorBoundedBy(t *testing.T) {
3838
End: bEndTime,
3939
Wght: defaultWeight,
4040
}
41-
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
42-
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
41+
require.False(BoundedBy(a.StartTime(), b.EndTime(), b.StartTime(), b.EndTime()))
42+
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
4343

4444
// case 2: a starts, b starts, a finishes, b finishes
4545
a.Start = 0
4646
b.Start = 1
4747
a.End = 2
4848
b.End = 3
49-
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
50-
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
49+
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
50+
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
5151

5252
// case 3: a starts, b starts, b finishes, a finishes
5353
a.Start = 0
5454
b.Start = 1
5555
b.End = 2
5656
a.End = 3
57-
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
58-
require.True(b.BoundedBy(a.StartTime(), a.EndTime()))
57+
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
58+
require.True(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
5959

6060
// case 4: b starts, a starts, a finishes, b finishes
6161
b.Start = 0
6262
a.Start = 1
6363
a.End = 2
6464
b.End = 3
65-
require.True(a.BoundedBy(b.StartTime(), b.EndTime()))
66-
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
65+
require.True(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
66+
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
6767

6868
// case 5: b starts, b finishes, a starts, a finishes
6969
b.Start = 0
7070
b.End = 1
7171
a.Start = 2
7272
a.End = 3
73-
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
74-
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
73+
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
74+
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
7575

7676
// case 6: b starts, a starts, b finishes, a finishes
7777
b.Start = 0
7878
a.Start = 1
7979
b.End = 2
8080
a.End = 3
81-
require.False(a.BoundedBy(b.StartTime(), b.EndTime()))
82-
require.False(b.BoundedBy(a.StartTime(), a.EndTime()))
81+
require.False(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
82+
require.False(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
8383

8484
// case 3: a starts, b starts, b finishes, a finishes
8585
a.Start = 0
8686
b.Start = 0
8787
b.End = 1
8888
a.End = 1
89-
require.True(a.BoundedBy(b.StartTime(), b.EndTime()))
90-
require.True(b.BoundedBy(a.StartTime(), a.EndTime()))
89+
require.True(BoundedBy(a.StartTime(), a.EndTime(), b.StartTime(), b.EndTime()))
90+
require.True(BoundedBy(b.StartTime(), b.EndTime(), a.StartTime(), a.EndTime()))
9191
}

0 commit comments

Comments
 (0)