Add TxVerifier interface to network (#2542)

Signed-off-by: Joshua Kim <[email protected]>
ava-labs · Dec 22, 2023 · 5888ac3 · 5888ac3
1 parent c950f0f
commit 5888ac3
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 48 deletions.
diff --git a/vms/platformvm/block/builder/helpers_test.go b/vms/platformvm/block/builder/helpers_test.go
@@ -181,9 +181,10 @@ func newEnvironment(t *testing.T) *environment {
 		pvalidators.TestManager,
 	)
 
+	txVerifier := network.NewLockedTxVerifier(&res.ctx.Lock, res.blkManager)
 	res.network = network.New(
 		res.backend.Ctx,
-		res.blkManager,
+		txVerifier,
 		res.mempool,
 		res.backend.Config.PartialSyncPrimaryNetwork,
 		res.sender,

diff --git a/vms/platformvm/network/network.go b/vms/platformvm/network/network.go
@@ -14,7 +14,6 @@ import (
 	"github.com/ava-labs/avalanchego/snow"
 	"github.com/ava-labs/avalanchego/snow/engine/common"
 	"github.com/ava-labs/avalanchego/vms/components/message"
-	"github.com/ava-labs/avalanchego/vms/platformvm/block/executor"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool"
 )
@@ -38,7 +37,7 @@ type network struct {
 	common.AppHandler
 
 	ctx                       *snow.Context
-	manager                   executor.Manager
+	txVerifier                TxVerifier
 	mempool                   mempool.Mempool
 	partialSyncPrimaryNetwork bool
 	appSender                 common.AppSender
@@ -50,7 +49,7 @@ type network struct {
 
 func New(
 	ctx *snow.Context,
-	manager executor.Manager,
+	txVerifier TxVerifier,
 	mempool mempool.Mempool,
 	partialSyncPrimaryNetwork bool,
 	appSender common.AppSender,
@@ -59,7 +58,7 @@ func New(
 		AppHandler: common.NewNoOpAppHandler(ctx.Log),
 
 		ctx:                       ctx,
-		manager:                   manager,
+		txVerifier:                txVerifier,
 		mempool:                   mempool,
 		partialSyncPrimaryNetwork: partialSyncPrimaryNetwork,
 		appSender:                 appSender,
@@ -145,13 +144,7 @@ func (n *network) issueTx(tx *txs.Tx) error {
 	}
 
 	// Verify the tx at the currently preferred state
-	//
-	// We need to grab the context lock here to avoid racy behavior with
-	// transaction verification + mempool modifications.
-	n.ctx.Lock.Lock()
-	err := n.manager.VerifyTx(tx)
-	n.ctx.Lock.Unlock()
-	if err != nil {
+	if err := n.txVerifier.VerifyTx(tx); err != nil {
 		n.ctx.Log.Debug("tx failed verification",
 			zap.Stringer("txID", txID),
 			zap.Error(err),

diff --git a/vms/platformvm/network/network_test.go b/vms/platformvm/network/network_test.go
@@ -18,12 +18,22 @@ import (
 	"github.com/ava-labs/avalanchego/utils/logging"
 	"github.com/ava-labs/avalanchego/vms/components/avax"
 	"github.com/ava-labs/avalanchego/vms/components/message"
-	"github.com/ava-labs/avalanchego/vms/platformvm/block/executor"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool"
 )
 
-var errTest = errors.New("test error")
+var (
+	errTest            = errors.New("test error")
+	_       TxVerifier = (*testTxVerifier)(nil)
+)
+
+type testTxVerifier struct {
+	err error
+}
+
+func (t testTxVerifier) VerifyTx(*txs.Tx) error {
+	return t.err
+}
 
 func TestNetworkAppGossip(t *testing.T) {
 	testTx := &txs.Tx{
@@ -159,7 +169,7 @@ func TestNetworkAppGossip(t *testing.T) {
 				&snow.Context{
 					Log: logging.NoLog{},
 				},
-				executor.NewMockManager(ctrl), // Manager is unused in this test
+				testTxVerifier{},
 				tt.mempoolFunc(ctrl),
 				tt.partialSyncPrimaryNetwork,
 				tt.appSenderFunc(ctrl),
@@ -175,7 +185,7 @@ func TestNetworkIssueTx(t *testing.T) {
 	type test struct {
 		name                      string
 		mempoolFunc               func(*gomock.Controller) mempool.Mempool
-		managerFunc               func(*gomock.Controller) executor.Manager
+		txVerifier                testTxVerifier
 		partialSyncPrimaryNetwork bool
 		appSenderFunc             func(*gomock.Controller) common.AppSender
 		expectedErr               error
@@ -189,10 +199,6 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().Get(gomock.Any()).Return(tx, true)
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				// Unused in this test
-				return executor.NewMockManager(ctrl)
-			},
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Should gossip the tx
 				appSender := common.NewMockSender(ctrl)
@@ -209,11 +215,7 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().MarkDropped(gomock.Any(), gomock.Any())
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				manager := executor.NewMockManager(ctrl)
-				manager.EXPECT().VerifyTx(gomock.Any()).Return(errTest)
-				return manager
-			},
+			txVerifier: testTxVerifier{err: errTest},
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Shouldn't gossip the tx
 				return common.NewMockSender(ctrl)
@@ -228,11 +230,7 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().MarkDropped(gomock.Any(), gomock.Any())
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				manager := executor.NewMockManager(ctrl)
-				manager.EXPECT().VerifyTx(gomock.Any()).Return(errTest)
-				return manager
-			},
+			txVerifier: testTxVerifier{err: errTest},
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Shouldn't gossip the tx
 				return common.NewMockSender(ctrl)
@@ -248,11 +246,6 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().MarkDropped(gomock.Any(), errTest)
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				manager := executor.NewMockManager(ctrl)
-				manager.EXPECT().VerifyTx(gomock.Any()).Return(nil)
-				return manager
-			},
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Shouldn't gossip the tx
 				return common.NewMockSender(ctrl)
@@ -266,11 +259,6 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().Get(gomock.Any()).Return(nil, false)
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				manager := executor.NewMockManager(ctrl)
-				manager.EXPECT().VerifyTx(gomock.Any()).Return(nil)
-				return manager
-			},
 			partialSyncPrimaryNetwork: true,
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Should gossip the tx
@@ -289,11 +277,6 @@ func TestNetworkIssueTx(t *testing.T) {
 				mempool.EXPECT().RequestBuildBlock(false)
 				return mempool
 			},
-			managerFunc: func(ctrl *gomock.Controller) executor.Manager {
-				manager := executor.NewMockManager(ctrl)
-				manager.EXPECT().VerifyTx(gomock.Any()).Return(nil)
-				return manager
-			},
 			appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
 				// Should gossip the tx
 				appSender := common.NewMockSender(ctrl)
@@ -313,7 +296,7 @@ func TestNetworkIssueTx(t *testing.T) {
 				&snow.Context{
 					Log: logging.NoLog{},
 				},
-				tt.managerFunc(ctrl),
+				tt.txVerifier,
 				tt.mempoolFunc(ctrl),
 				tt.partialSyncPrimaryNetwork,
 				tt.appSenderFunc(ctrl),
@@ -334,7 +317,7 @@ func TestNetworkGossipTx(t *testing.T) {
 		&snow.Context{
 			Log: logging.NoLog{},
 		},
-		executor.NewMockManager(ctrl),
+		testTxVerifier{},
 		mempool.NewMockMempool(ctrl),
 		false,
 		appSender,

diff --git a/vms/platformvm/network/tx_verifier.go b/vms/platformvm/network/tx_verifier.go
@@ -0,0 +1,36 @@
+// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+
+package network
+
+import (
+	"sync"
+
+	"github.com/ava-labs/avalanchego/vms/platformvm/txs"
+)
+
+var _ TxVerifier = (*LockedTxVerifier)(nil)
+
+type TxVerifier interface {
+	// VerifyTx verifies that the transaction should be issued into the mempool.
+	VerifyTx(tx *txs.Tx) error
+}
+
+type LockedTxVerifier struct {
+	lock       sync.Locker
+	txVerifier TxVerifier
+}
+
+func (l *LockedTxVerifier) VerifyTx(tx *txs.Tx) error {
+	l.lock.Lock()
+	defer l.lock.Unlock()
+
+	return l.txVerifier.VerifyTx(tx)
+}
+
+func NewLockedTxVerifier(lock sync.Locker, txVerifier TxVerifier) *LockedTxVerifier {
+	return &LockedTxVerifier{
+		lock:       lock,
+		txVerifier: txVerifier,
+	}
+}
diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go
@@ -190,9 +190,11 @@ func (vm *VM) Initialize(
 		txExecutorBackend,
 		validatorManager,
 	)
+
+	txVerifier := network.NewLockedTxVerifier(&txExecutorBackend.Ctx.Lock, vm.manager)
 	vm.Network = network.New(
 		txExecutorBackend.Ctx,
-		vm.manager,
+		txVerifier,
 		mempool,
 		txExecutorBackend.Config.PartialSyncPrimaryNetwork,
 		appSender,