From 4d16cc3c70bfd215dfeb3bd781a2e0f48c64d017 Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Thu, 18 Jan 2024 17:27:41 -0500 Subject: [PATCH] `vms/platformvm`: Remove `MempoolTxVerifier` (#2362) Co-authored-by: Stephen Buttolph --- vms/platformvm/block/executor/manager.go | 32 +++- vms/platformvm/txs/executor/export_test.go | 16 +- vms/platformvm/txs/executor/import_test.go | 16 +- .../txs/executor/standard_tx_executor_test.go | 20 --- vms/platformvm/txs/executor/state_changes.go | 25 +++ .../txs/executor/tx_mempool_verifier.go | 143 ------------------ 6 files changed, 64 insertions(+), 188 deletions(-) delete mode 100644 vms/platformvm/txs/executor/tx_mempool_verifier.go diff --git a/vms/platformvm/block/executor/manager.go b/vms/platformvm/block/executor/manager.go index ed82197a3568..27d35a7641ad 100644 --- a/vms/platformvm/block/executor/manager.go +++ b/vms/platformvm/block/executor/manager.go @@ -127,12 +127,34 @@ func (m *manager) VerifyTx(tx *txs.Tx) error { return ErrChainNotSynced } - return tx.Unsigned.Visit(&executor.MempoolTxVerifier{ - Backend: m.txExecutorBackend, - ParentID: m.preferred, - StateVersions: m, - Tx: tx, + stateDiff, err := state.NewDiff(m.preferred, m) + if err != nil { + return err + } + + nextBlkTime, _, err := executor.NextBlockTime(stateDiff, m.txExecutorBackend.Clk) + if err != nil { + return err + } + + _, err = executor.AdvanceTimeTo(m.txExecutorBackend, stateDiff, nextBlkTime) + if err != nil { + return err + } + + err = 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 { diff --git a/vms/platformvm/txs/executor/export_test.go b/vms/platformvm/txs/executor/export_test.go index 1272d2815142..10085f08436f 100644 --- a/vms/platformvm/txs/executor/export_test.go +++ b/vms/platformvm/txs/executor/export_test.go @@ -59,19 +59,15 @@ func TestNewExportTx(t *testing.T) { ) require.NoError(err) - fakedState, err := state.NewDiff(lastAcceptedID, env) + stateDiff, err := state.NewDiff(lastAcceptedID, env) require.NoError(err) - fakedState.SetTimestamp(tt.timestamp) + stateDiff.SetTimestamp(tt.timestamp) - fakedParent := ids.GenerateTestID() - env.SetState(fakedParent, fakedState) - - verifier := MempoolTxVerifier{ - Backend: &env.backend, - ParentID: fakedParent, - StateVersions: env, - Tx: tx, + verifier := StandardTxExecutor{ + Backend: &env.backend, + State: stateDiff, + Tx: tx, } require.NoError(tx.Unsigned.Visit(&verifier)) }) diff --git a/vms/platformvm/txs/executor/import_test.go b/vms/platformvm/txs/executor/import_test.go index 5d281da57185..f9f270801b2b 100644 --- a/vms/platformvm/txs/executor/import_test.go +++ b/vms/platformvm/txs/executor/import_test.go @@ -182,19 +182,15 @@ func TestNewImportTx(t *testing.T) { require.Equal(env.config.TxFee, totalIn-totalOut) - fakedState, err := state.NewDiff(lastAcceptedID, env) + stateDiff, err := state.NewDiff(lastAcceptedID, env) require.NoError(err) - fakedState.SetTimestamp(tt.timestamp) + stateDiff.SetTimestamp(tt.timestamp) - fakedParent := ids.GenerateTestID() - env.SetState(fakedParent, fakedState) - - verifier := MempoolTxVerifier{ - Backend: &env.backend, - ParentID: fakedParent, - StateVersions: env, - Tx: tx, + verifier := StandardTxExecutor{ + Backend: &env.backend, + State: stateDiff, + Tx: tx, } require.NoError(tx.Unsigned.Visit(&verifier)) }) diff --git a/vms/platformvm/txs/executor/standard_tx_executor_test.go b/vms/platformvm/txs/executor/standard_tx_executor_test.go index a86c579fee79..2e99150e5a54 100644 --- a/vms/platformvm/txs/executor/standard_tx_executor_test.go +++ b/vms/platformvm/txs/executor/standard_tx_executor_test.go @@ -179,7 +179,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup func(*environment) AP3Time time.Time expectedExecutionErr error - expectedMempoolErr error } tests := []test{ @@ -194,7 +193,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: nil, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrPeriodMismatch, - expectedMempoolErr: ErrPeriodMismatch, }, { description: fmt.Sprintf("delegator should not be added more than (%s) in the future", MaxFutureStartTime), @@ -207,7 +205,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: nil, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrFutureStakeTime, - expectedMempoolErr: nil, }, { description: "validator not in the current or pending validator sets", @@ -220,7 +217,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: nil, AP3Time: defaultGenesisTime, expectedExecutionErr: database.ErrNotFound, - expectedMempoolErr: database.ErrNotFound, }, { description: "delegator starts before validator", @@ -233,7 +229,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: addMinStakeValidator, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrPeriodMismatch, - expectedMempoolErr: ErrPeriodMismatch, }, { description: "delegator stops before validator", @@ -246,7 +241,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: addMinStakeValidator, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrPeriodMismatch, - expectedMempoolErr: ErrPeriodMismatch, }, { description: "valid", @@ -259,7 +253,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: addMinStakeValidator, AP3Time: defaultGenesisTime, expectedExecutionErr: nil, - expectedMempoolErr: nil, }, { description: "starts delegating at current timestamp", @@ -272,7 +265,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: nil, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrTimestampNotBeforeStartTime, - expectedMempoolErr: ErrTimestampNotBeforeStartTime, }, { description: "tx fee paying key has no funds", @@ -297,7 +289,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { }, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrFlowCheckFailed, - expectedMempoolErr: ErrFlowCheckFailed, }, { description: "over delegation before AP3", @@ -310,7 +301,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: addMaxStakeValidator, AP3Time: defaultValidateEndTime, expectedExecutionErr: nil, - expectedMempoolErr: nil, }, { description: "over delegation after AP3", @@ -323,7 +313,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { setup: addMaxStakeValidator, AP3Time: defaultGenesisTime, expectedExecutionErr: ErrOverDelegated, - expectedMempoolErr: ErrOverDelegated, }, } @@ -363,15 +352,6 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { } err = tx.Unsigned.Visit(&executor) require.ErrorIs(err, tt.expectedExecutionErr) - - mempoolExecutor := MempoolTxVerifier{ - Backend: &freshTH.backend, - ParentID: lastAcceptedID, - StateVersions: freshTH, - Tx: tx, - } - err = tx.Unsigned.Visit(&mempoolExecutor) - require.ErrorIs(err, tt.expectedMempoolErr) }) } } diff --git a/vms/platformvm/txs/executor/state_changes.go b/vms/platformvm/txs/executor/state_changes.go index 3086358304a3..36981b095e8c 100644 --- a/vms/platformvm/txs/executor/state_changes.go +++ b/vms/platformvm/txs/executor/state_changes.go @@ -10,6 +10,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/vms/platformvm/reward" "github.com/ava-labs/avalanchego/vms/platformvm/state" "github.com/ava-labs/avalanchego/vms/platformvm/txs" @@ -57,6 +58,30 @@ func VerifyNewChainTime( return nil } +func NextBlockTime(state state.Chain, clk *mockable.Clock) (time.Time, bool, error) { + var ( + timestamp = clk.Time() + parentTime = state.GetTimestamp() + ) + if parentTime.After(timestamp) { + timestamp = parentTime + } + // [timestamp] = max(now, parentTime) + + nextStakerChangeTime, err := GetNextStakerChangeTime(state) + if err != nil { + return time.Time{}, false, fmt.Errorf("failed getting next staker change time: %w", err) + } + + // timeWasCapped means that [timestamp] was reduced to [nextStakerChangeTime] + timeWasCapped := !timestamp.Before(nextStakerChangeTime) + if timeWasCapped { + timestamp = nextStakerChangeTime + } + // [timestamp] = min(max(now, parentTime), nextStakerChangeTime) + return timestamp, timeWasCapped, nil +} + // AdvanceTimeTo applies all state changes to [parentState] resulting from // advancing the chain time to [newChainTime]. // Returns true iff the validator set changed. diff --git a/vms/platformvm/txs/executor/tx_mempool_verifier.go b/vms/platformvm/txs/executor/tx_mempool_verifier.go deleted file mode 100644 index 348e457c1c78..000000000000 --- a/vms/platformvm/txs/executor/tx_mempool_verifier.go +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package executor - -import ( - "errors" - "fmt" - "time" - - "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/utils/timer/mockable" - "github.com/ava-labs/avalanchego/vms/platformvm/state" - "github.com/ava-labs/avalanchego/vms/platformvm/txs" -) - -var _ txs.Visitor = (*MempoolTxVerifier)(nil) - -type MempoolTxVerifier struct { - *Backend - ParentID ids.ID - StateVersions state.Versions - Tx *txs.Tx -} - -func (*MempoolTxVerifier) AdvanceTimeTx(*txs.AdvanceTimeTx) error { - return ErrWrongTxType -} - -func (*MempoolTxVerifier) RewardValidatorTx(*txs.RewardValidatorTx) error { - return ErrWrongTxType -} - -func (v *MempoolTxVerifier) AddValidatorTx(tx *txs.AddValidatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) AddSubnetValidatorTx(tx *txs.AddSubnetValidatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) AddDelegatorTx(tx *txs.AddDelegatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) CreateChainTx(tx *txs.CreateChainTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) CreateSubnetTx(tx *txs.CreateSubnetTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) ImportTx(tx *txs.ImportTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) ExportTx(tx *txs.ExportTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) RemoveSubnetValidatorTx(tx *txs.RemoveSubnetValidatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) TransformSubnetTx(tx *txs.TransformSubnetTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) AddPermissionlessValidatorTx(tx *txs.AddPermissionlessValidatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) AddPermissionlessDelegatorTx(tx *txs.AddPermissionlessDelegatorTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) TransferSubnetOwnershipTx(tx *txs.TransferSubnetOwnershipTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) BaseTx(tx *txs.BaseTx) error { - return v.standardTx(tx) -} - -func (v *MempoolTxVerifier) standardTx(tx txs.UnsignedTx) error { - baseState, err := v.standardBaseState() - if err != nil { - return err - } - - executor := StandardTxExecutor{ - Backend: v.Backend, - State: baseState, - Tx: v.Tx, - } - err = tx.Visit(&executor) - // We ignore [errFutureStakeTime] here because the time will be advanced - // when this transaction is issued. - if errors.Is(err, ErrFutureStakeTime) { - return nil - } - return err -} - -func (v *MempoolTxVerifier) standardBaseState() (state.Diff, error) { - state, err := state.NewDiff(v.ParentID, v.StateVersions) - if err != nil { - return nil, err - } - - nextBlkTime, _, err := NextBlockTime(state, v.Clk) - if err != nil { - return nil, err - } - - _, err = AdvanceTimeTo(v.Backend, state, nextBlkTime) - return state, err -} - -func NextBlockTime(state state.Chain, clk *mockable.Clock) (time.Time, bool, error) { - var ( - timestamp = clk.Time() - parentTime = state.GetTimestamp() - ) - if parentTime.After(timestamp) { - timestamp = parentTime - } - // [timestamp] = max(now, parentTime) - - nextStakerChangeTime, err := GetNextStakerChangeTime(state) - if err != nil { - return time.Time{}, false, fmt.Errorf("failed getting next staker change time: %w", err) - } - - // timeWasCapped means that [timestamp] was reduced to [nextStakerChangeTime] - timeWasCapped := !timestamp.Before(nextStakerChangeTime) - if timeWasCapped { - timestamp = nextStakerChangeTime - } - // [timestamp] = min(max(now, parentTime), nextStakerChangeTime) - return timestamp, timeWasCapped, nil -}