Skip to content

Commit

Permalink
[vms/platformvm] Remove ErrFutureStakeTime check in VerifyTx (#2797)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhrubabasu
dhrubabasu authored Mar 6, 2024
1 parent 639b9ca commit 67b1aa0
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 281 deletions.
106 changes: 0 additions & 106 deletions vms/platformvm/block/builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,112 +279,6 @@ func TestBuildBlockForceAdvanceTime(t *testing.T) {
require.Equal(nextTime.Unix(), standardBlk.Timestamp().Unix())
}

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

env := newEnvironment(t, latestFork)
env.ctx.Lock.Lock()
defer env.ctx.Lock.Unlock()

// The [StartTime] in a staker tx is only validated pre-Durango.
// TODO: Delete this test post-Durango activation.
env.config.DurangoTime = mockable.MaxTime

var (
now = env.backend.Clk.Time()
defaultValidatorStake = 100 * units.MilliAvax

// Add a validator with StartTime in the future within [MaxFutureStartTime]
validatorStartTime = now.Add(txexecutor.MaxFutureStartTime - 1*time.Second)
validatorEndTime = validatorStartTime.Add(360 * 24 * time.Hour)
)

tx1, err := env.txBuilder.NewAddValidatorTx(
defaultValidatorStake,
uint64(validatorStartTime.Unix()),
uint64(validatorEndTime.Unix()),
ids.GenerateTestNodeID(),
preFundedKeys[0].PublicKey().Address(),
reward.PercentDenominator,
[]*secp256k1.PrivateKey{preFundedKeys[0]},
preFundedKeys[0].PublicKey().Address(),
nil,
)
require.NoError(err)
require.NoError(env.mempool.Add(tx1))
tx1ID := tx1.ID()
_, ok := env.mempool.Get(tx1ID)
require.True(ok)

// Add a validator with StartTime before current chain time
validator2StartTime := now.Add(-5 * time.Second)
validator2EndTime := validator2StartTime.Add(360 * 24 * time.Hour)

tx2, err := env.txBuilder.NewAddValidatorTx(
defaultValidatorStake,
uint64(validator2StartTime.Unix()),
uint64(validator2EndTime.Unix()),
ids.GenerateTestNodeID(),
preFundedKeys[1].PublicKey().Address(),
reward.PercentDenominator,
[]*secp256k1.PrivateKey{preFundedKeys[1]},
preFundedKeys[1].PublicKey().Address(),
nil,
)
require.NoError(err)
require.NoError(env.mempool.Add(tx2))
tx2ID := tx2.ID()
_, ok = env.mempool.Get(tx2ID)
require.True(ok)

// Add a validator with StartTime in the future past [MaxFutureStartTime]
validator3StartTime := now.Add(txexecutor.MaxFutureStartTime + 5*time.Second)
validator3EndTime := validator2StartTime.Add(360 * 24 * time.Hour)

tx3, err := env.txBuilder.NewAddValidatorTx(
defaultValidatorStake,
uint64(validator3StartTime.Unix()),
uint64(validator3EndTime.Unix()),
ids.GenerateTestNodeID(),
preFundedKeys[2].PublicKey().Address(),
reward.PercentDenominator,
[]*secp256k1.PrivateKey{preFundedKeys[2]},
preFundedKeys[2].PublicKey().Address(),
nil,
)
require.NoError(err)
require.NoError(env.mempool.Add(tx3))
tx3ID := tx3.ID()
_, ok = env.mempool.Get(tx3ID)
require.True(ok)

// Only tx1 should be in a built block
blkIntf, err := env.Builder.BuildBlock(context.Background())
require.NoError(err)

require.IsType(&blockexecutor.Block{}, blkIntf)
blk := blkIntf.(*blockexecutor.Block)
require.Len(blk.Txs(), 1)
require.Equal(tx1ID, blk.Txs()[0].ID())

// Mempool should have none of the txs
_, ok = env.mempool.Get(tx1ID)
require.False(ok)
_, ok = env.mempool.Get(tx2ID)
require.False(ok)
_, ok = env.mempool.Get(tx3ID)
require.False(ok)

// Only tx2 and tx3 should be dropped
require.NoError(env.mempool.GetDropReason(tx1ID))

tx2DropReason := env.mempool.GetDropReason(tx2ID)
require.ErrorIs(tx2DropReason, txexecutor.ErrTimestampNotBeforeStartTime)

tx3DropReason := env.mempool.GetDropReason(tx3ID)
require.ErrorIs(tx3DropReason, txexecutor.ErrFutureStakeTime)
}

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

Expand Down
10 changes: 1 addition & 9 deletions vms/platformvm/block/executor/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,11 @@ func (m *manager) VerifyTx(tx *txs.Tx) error {
return err
}

err = tx.Unsigned.Visit(&executor.StandardTxExecutor{
return tx.Unsigned.Visit(&executor.StandardTxExecutor{
Backend: m.txExecutorBackend,
State: stateDiff,
Tx: tx,
})
// We ignore [errFutureStakeTime] here because the time will be advanced
// when this transaction is issued.
//
// TODO: Remove this check post-Durango.
if errors.Is(err, executor.ErrFutureStakeTime) {
return nil
}
return err
}

func (m *manager) VerifyUniqueInputs(blkID ids.ID, inputs set.Set[ids.ID]) error {
Expand Down
44 changes: 0 additions & 44 deletions vms/platformvm/txs/executor/proposal_tx_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package executor

import (
"fmt"
"math"
"testing"
"time"
Expand Down Expand Up @@ -122,18 +121,6 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) {
AP3Time: defaultGenesisTime,
expectedErr: ErrPeriodMismatch,
},
{
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
stakeAmount: dummyH.config.MinDelegatorStake,
startTime: uint64(currentTimestamp.Add(MaxFutureStartTime + time.Second).Unix()),
endTime: uint64(currentTimestamp.Add(MaxFutureStartTime + defaultMinStakingDuration + time.Second).Unix()),
nodeID: nodeID,
rewardAddress: rewardAddress,
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: nil,
AP3Time: defaultGenesisTime,
expectedErr: ErrFutureStakeTime,
},
{
description: "validator not in the current or pending validator sets",
stakeAmount: dummyH.config.MinDelegatorStake,
Expand Down Expand Up @@ -770,37 +757,6 @@ func TestProposalTxExecuteAddValidator(t *testing.T) {
require.ErrorIs(err, ErrTimestampNotBeforeStartTime)
}

{
// Case: Validator's start time too far in the future
tx, err := env.txBuilder.NewAddValidatorTx(
env.config.MinValidatorStake,
uint64(defaultValidateStartTime.Add(MaxFutureStartTime).Unix()+1),
uint64(defaultValidateStartTime.Add(MaxFutureStartTime).Add(defaultMinStakingDuration).Unix()+1),
nodeID,
ids.ShortEmpty,
reward.PercentDenominator,
[]*secp256k1.PrivateKey{preFundedKeys[0]},
ids.ShortEmpty, // change addr
nil,
)
require.NoError(err)

onCommitState, err := state.NewDiff(lastAcceptedID, env)
require.NoError(err)

onAbortState, err := state.NewDiff(lastAcceptedID, env)
require.NoError(err)

executor := ProposalTxExecutor{
OnCommitState: onCommitState,
OnAbortState: onAbortState,
Backend: &env.backend,
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrFutureStakeTime)
}

{
nodeID := genesisNodeIDs[0]

Expand Down
35 changes: 5 additions & 30 deletions vms/platformvm/txs/executor/staker_tx_verification.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ var (
ErrStakeTooShort = errors.New("staking period is too short")
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)
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")
Expand Down Expand Up @@ -177,9 +176,7 @@ func verifyAddValidatorTx(
return nil, fmt.Errorf("%w: %w", ErrFlowCheckFailed, err)
}

// verifyStakerStartsSoon is checked last to allow
// the verifier visitor to explicitly check for this error.
return outs, verifyStakerStartsSoon(false /*=isDurangoActive*/, currentTimestamp, startTime)
return outs, nil
}

// verifyAddSubnetValidatorTx carries out the validation for an
Expand Down Expand Up @@ -267,9 +264,7 @@ func verifyAddSubnetValidatorTx(
return fmt.Errorf("%w: %w", ErrFlowCheckFailed, err)
}

// verifyStakerStartsSoon is checked last to allow
// the verifier visitor to explicitly check for this error.
return verifyStakerStartsSoon(isDurangoActive, currentTimestamp, startTime)
return nil
}

// Returns the representation of [tx.NodeID] validating [tx.Subnet].
Expand Down Expand Up @@ -459,9 +454,7 @@ func verifyAddDelegatorTx(
return nil, fmt.Errorf("%w: %w", ErrFlowCheckFailed, err)
}

// verifyStakerStartsSoon is checked last to allow
// the verifier visitor to explicitly check for this error.
return outs, verifyStakerStartsSoon(false /*=isDurangoActive*/, currentTimestamp, startTime)
return outs, nil
}

// verifyAddPermissionlessValidatorTx carries out the validation for an
Expand Down Expand Up @@ -583,9 +576,7 @@ func verifyAddPermissionlessValidatorTx(
return fmt.Errorf("%w: %w", ErrFlowCheckFailed, err)
}

// verifyStakerStartsSoon is checked last to allow
// the verifier visitor to explicitly check for this error.
return verifyStakerStartsSoon(isDurangoActive, currentTimestamp, startTime)
return nil
}

// verifyAddPermissionlessDelegatorTx carries out the validation for an
Expand Down Expand Up @@ -732,9 +723,7 @@ func verifyAddPermissionlessDelegatorTx(
return fmt.Errorf("%w: %w", ErrFlowCheckFailed, err)
}

// verifyStakerStartsSoon is checked last to allow
// the verifier visitor to explicitly check for this error.
return verifyStakerStartsSoon(isDurangoActive, currentTimestamp, startTime)
return nil
}

// Returns an error if the given tx is invalid.
Expand Down Expand Up @@ -806,17 +795,3 @@ func verifyStakerStartTime(isDurangoActive bool, chainTime, stakerTime time.Time
}
return nil
}

func verifyStakerStartsSoon(isDurangoActive bool, chainTime, stakerStartTime time.Time) error {
if isDurangoActive {
return nil
}

// Make sure the tx doesn't start too far in the future. This is done last
// to allow the verifier visitor to explicitly check for this error.
maxStartTime := chainTime.Add(MaxFutureStartTime)
if stakerStartTime.After(maxStartTime) {
return ErrFutureStakeTime
}
return nil
}
52 changes: 0 additions & 52 deletions vms/platformvm/txs/executor/staker_tx_verification_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,58 +475,6 @@ func TestVerifyAddPermissionlessValidatorTx(t *testing.T) {
},
expectedErr: ErrFlowCheckFailed,
},
{
name: "starts too far in the future",
backendF: func(ctrl *gomock.Controller) *Backend {
bootstrapped := &utils.Atomic[bool]{}
bootstrapped.Set(true)

flowChecker := utxo.NewMockVerifier(ctrl)
flowChecker.EXPECT().VerifySpend(
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(nil)

return &Backend{
FlowChecker: flowChecker,
Config: &config.Config{
CortinaTime: activeForkTime,
DurangoTime: mockable.MaxTime,
AddSubnetValidatorFee: 1,
},
Ctx: ctx,
Bootstrapped: bootstrapped,
}
},
stateF: func(ctrl *gomock.Controller) state.Chain {
mockState := state.NewMockChain(ctrl)
mockState.EXPECT().GetTimestamp().Return(now).Times(2) // chain time is Cortina fork activation since now.After(activeForkTime)
mockState.EXPECT().GetSubnetTransformation(subnetID).Return(&transformTx, nil)
mockState.EXPECT().GetCurrentValidator(subnetID, verifiedTx.NodeID()).Return(nil, database.ErrNotFound)
mockState.EXPECT().GetPendingValidator(subnetID, verifiedTx.NodeID()).Return(nil, database.ErrNotFound)
primaryNetworkVdr := &state.Staker{
StartTime: time.Unix(0, 0),
EndTime: mockable.MaxTime,
}
mockState.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, verifiedTx.NodeID()).Return(primaryNetworkVdr, nil)
return mockState
},
sTxF: func() *txs.Tx {
return &verifiedSignedTx
},
txF: func() *txs.AddPermissionlessValidatorTx {
// Note this copies [verifiedTx]
tx := verifiedTx
tx.Validator.Start = uint64(now.Add(MaxFutureStartTime).Add(time.Second).Unix())
tx.Validator.End = tx.Validator.Start + uint64(unsignedTransformTx.MinStakeDuration)
return &tx
},
expectedErr: ErrFutureStakeTime,
},
{
name: "success",
backendF: func(ctrl *gomock.Controller) *Backend {
Expand Down
40 changes: 0 additions & 40 deletions vms/platformvm/txs/executor/standard_tx_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package executor

import (
"errors"
"fmt"
"math"
"testing"
"time"
Expand Down Expand Up @@ -197,18 +196,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) {
AP3Time: defaultGenesisTime,
expectedExecutionErr: ErrPeriodMismatch,
},
{
description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime),
stakeAmount: dummyH.config.MinDelegatorStake,
startTime: currentTimestamp.Add(MaxFutureStartTime + time.Second),
endTime: currentTimestamp.Add(MaxFutureStartTime + defaultMinStakingDuration + time.Second),
nodeID: nodeID,
rewardAddress: rewardAddress,
feeKeys: []*secp256k1.PrivateKey{preFundedKeys[0]},
setup: nil,
AP3Time: defaultGenesisTime,
expectedExecutionErr: ErrFutureStakeTime,
},
{
description: "validator not in the current or pending validator sets",
stakeAmount: dummyH.config.MinDelegatorStake,
Expand Down Expand Up @@ -833,33 +820,6 @@ func TestBanffStandardTxExecutorAddValidator(t *testing.T) {
require.ErrorIs(err, ErrTimestampNotBeforeStartTime)
}

{
// Case: Validator's start time too far in the future
tx, err := env.txBuilder.NewAddValidatorTx(
env.config.MinValidatorStake,
uint64(defaultValidateStartTime.Add(MaxFutureStartTime).Unix()+1),
uint64(defaultValidateStartTime.Add(MaxFutureStartTime).Add(defaultMinStakingDuration).Unix()+1),
nodeID,
ids.ShortEmpty,
reward.PercentDenominator,
[]*secp256k1.PrivateKey{preFundedKeys[0]},
ids.ShortEmpty, // change addr
nil,
)
require.NoError(err)

onAcceptState, err := state.NewDiff(lastAcceptedID, env)
require.NoError(err)

executor := StandardTxExecutor{
Backend: &env.backend,
State: onAcceptState,
Tx: tx,
}
err = tx.Unsigned.Visit(&executor)
require.ErrorIs(err, ErrFutureStakeTime)
}

{
// Case: Validator in current validator set of primary network
startTime := defaultValidateStartTime.Add(1 * time.Second)
Expand Down

0 comments on commit 67b1aa0

Please sign in to comment.