Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce TxVerifier interface to network #2533

Merged
merged 10 commits into from
Dec 22, 2023
Merged

Introduce TxVerifier interface to network #2533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
eb863bb
Move locking into issueTx
StephenButtolph Dec 21, 2023
3e8b3fc
whoopsie
StephenButtolph Dec 21, 2023
dac0b83
nit
StephenButtolph Dec 21, 2023
e0aa0e0
yessir
StephenButtolph Dec 21, 2023
831720f
Introduce TxVerifier to network
StephenButtolph Dec 21, 2023
a5271af
move invariant
StephenButtolph Dec 21, 2023
705cfd1
cleanup tests
StephenButtolph Dec 21, 2023
25e7516
merged
StephenButtolph Dec 22, 2023
bf02a21
Merge branch 'dev' into tx-verifier
StephenButtolph Dec 22, 2023
4ae04a8
simplify table tests
StephenButtolph Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletions vms/avm/network/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow"
"github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/vms/avm/block/executor"
"github.com/ava-labs/avalanchego/vms/avm/txs"
"github.com/ava-labs/avalanchego/vms/avm/txs/mempool"
"github.com/ava-labs/avalanchego/vms/components/message"
Expand All @@ -36,8 +35,6 @@ type Network interface {
// If the tx is already in the mempool, mempool.ErrDuplicateTx will be
// returned.
// If the tx is not added to the mempool, an error will be returned.
//
// Invariant: The context lock is not held
IssueTx(context.Context, *txs.Tx) error

// IssueVerifiedTx attempts to add a tx to the mempool. If the tx is added
Expand All @@ -54,11 +51,11 @@ type network struct {
// We embed a noop handler for all unhandled messages
common.AppHandler

ctx *snow.Context
parser txs.Parser
manager executor.Manager
mempool mempool.Mempool
appSender common.AppSender
ctx *snow.Context
parser txs.Parser
txVerifier TxVerifier
mempool mempool.Mempool
appSender common.AppSender

// gossip related attributes
recentTxsLock sync.Mutex
Expand All @@ -68,18 +65,18 @@ type network struct {
func New(
ctx *snow.Context,
parser txs.Parser,
manager executor.Manager,
txVerifier TxVerifier,
mempool mempool.Mempool,
appSender common.AppSender,
) Network {
return &network{
AppHandler: common.NewNoOpAppHandler(ctx.Log),

ctx: ctx,
parser: parser,
manager: manager,
mempool: mempool,
appSender: appSender,
ctx: ctx,
parser: parser,
txVerifier: txVerifier,
mempool: mempool,
appSender: appSender,

recentTxs: &cache.LRU[ids.ID, struct{}]{
Size: recentTxsCacheSize,
Expand Down Expand Up @@ -154,11 +151,7 @@ func (n *network) issueTx(tx *txs.Tx) error {
return reason
}

// Verify the tx at the currently preferred state
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),
Expand Down
186 changes: 94 additions & 92 deletions vms/avm/network/network_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ func TestNetworkAppGossip(t *testing.T) {
require.NoError(t, testTx.Initialize(parser.Codec()))

type test struct {
name string
msgBytesFunc func() []byte
mempoolFunc func(*gomock.Controller) mempool.Mempool
managerFunc func(*gomock.Controller) executor.Manager
appSenderFunc func(*gomock.Controller) common.AppSender
name string
msgBytesFunc func() []byte
mempoolFunc func(*gomock.Controller) mempool.Mempool
txVerifierFunc func(*gomock.Controller) TxVerifier
appSenderFunc func(*gomock.Controller) common.AppSender
}

tests := []test{
Expand All @@ -61,15 +61,6 @@ func TestNetworkAppGossip(t *testing.T) {
msgBytesFunc: func() []byte {
return []byte{0x00}
},
mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool {
return mempool.NewMockMempool(ctrl)
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
},
{
name: "invalid tx bytes",
Expand All @@ -81,15 +72,6 @@ func TestNetworkAppGossip(t *testing.T) {
require.NoError(t, err)
return msgBytes
},
mempoolFunc: func(ctrl *gomock.Controller) mempool.Mempool {
return mempool.NewMockMempool(ctrl)
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
},
{
name: "tx already in mempool",
Expand All @@ -106,12 +88,6 @@ func TestNetworkAppGossip(t *testing.T) {
mempool.EXPECT().Get(gomock.Any()).Return(testTx, true)
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
},
{
name: "tx previously dropped",
Expand All @@ -129,12 +105,6 @@ func TestNetworkAppGossip(t *testing.T) {
mempool.EXPECT().GetDropReason(gomock.Any()).Return(errTest)
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
},
{
name: "transaction invalid",
Expand All @@ -153,13 +123,10 @@ func TestNetworkAppGossip(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
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier {
txVerifier := executor.NewMockManager(ctrl)
txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(errTest)
return txVerifier
},
},
{
Expand All @@ -180,10 +147,10 @@ func TestNetworkAppGossip(t *testing.T) {
mempool.EXPECT().RequestBuildBlock()
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
manager := executor.NewMockManager(ctrl)
manager.EXPECT().VerifyTx(gomock.Any()).Return(nil)
return manager
txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier {
txVerifier := executor.NewMockManager(ctrl)
txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(nil)
return txVerifier
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
appSender := common.NewMockSender(ctrl)
Expand All @@ -205,14 +172,35 @@ func TestNetworkAppGossip(t *testing.T) {
})
require.NoError(err)

mempoolFunc := func(ctrl *gomock.Controller) mempool.Mempool {
return mempool.NewMockMempool(ctrl)
}
if tt.mempoolFunc != nil {
mempoolFunc = tt.mempoolFunc
}

txVerifierFunc := func(ctrl *gomock.Controller) TxVerifier {
return executor.NewMockManager(ctrl)
}
if tt.txVerifierFunc != nil {
txVerifierFunc = tt.txVerifierFunc
}

appSenderFunc := func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
}
if tt.appSenderFunc != nil {
appSenderFunc = tt.appSenderFunc
}

n := New(
&snow.Context{
Log: logging.NoLog{},
},
parser,
tt.managerFunc(ctrl),
tt.mempoolFunc(ctrl),
tt.appSenderFunc(ctrl),
txVerifierFunc(ctrl),
mempoolFunc(ctrl),
appSenderFunc(ctrl),
)
require.NoError(n.AppGossip(context.Background(), ids.GenerateTestNodeID(), tt.msgBytesFunc()))
})
Expand All @@ -221,11 +209,11 @@ func TestNetworkAppGossip(t *testing.T) {

func TestNetworkIssueTx(t *testing.T) {
type test struct {
name string
mempoolFunc func(*gomock.Controller) mempool.Mempool
managerFunc func(*gomock.Controller) executor.Manager
appSenderFunc func(*gomock.Controller) common.AppSender
expectedErr error
name string
mempoolFunc func(*gomock.Controller) mempool.Mempool
txVerifierFunc func(*gomock.Controller) TxVerifier
appSenderFunc func(*gomock.Controller) common.AppSender
expectedErr error
}

tests := []test{
Expand All @@ -236,12 +224,6 @@ func TestNetworkIssueTx(t *testing.T) {
mempool.EXPECT().Get(gomock.Any()).Return(nil, true)
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
expectedErr: mempool.ErrDuplicateTx,
},
{
Expand All @@ -252,12 +234,6 @@ func TestNetworkIssueTx(t *testing.T) {
mempool.EXPECT().GetDropReason(gomock.Any()).Return(errTest)
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
return executor.NewMockManager(ctrl)
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
expectedErr: errTest,
},
{
Expand All @@ -269,13 +245,10 @@ 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
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier {
txVerifier := executor.NewMockManager(ctrl)
txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(errTest)
return txVerifier
},
expectedErr: errTest,
},
Expand All @@ -289,13 +262,10 @@ 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(nil)
return manager
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier {
txVerifier := executor.NewMockManager(ctrl)
txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(nil)
return txVerifier
},
expectedErr: errTest,
},
Expand All @@ -309,10 +279,10 @@ func TestNetworkIssueTx(t *testing.T) {
mempool.EXPECT().RequestBuildBlock()
return mempool
},
managerFunc: func(ctrl *gomock.Controller) executor.Manager {
manager := executor.NewMockManager(ctrl)
manager.EXPECT().VerifyTx(gomock.Any()).Return(nil)
return manager
txVerifierFunc: func(ctrl *gomock.Controller) TxVerifier {
txVerifier := executor.NewMockManager(ctrl)
txVerifier.EXPECT().VerifyTx(gomock.Any()).Return(nil)
return txVerifier
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
appSender := common.NewMockSender(ctrl)
Expand All @@ -335,14 +305,35 @@ func TestNetworkIssueTx(t *testing.T) {
})
require.NoError(err)

mempoolFunc := func(ctrl *gomock.Controller) mempool.Mempool {
return mempool.NewMockMempool(ctrl)
}
if tt.mempoolFunc != nil {
mempoolFunc = tt.mempoolFunc
}

txVerifierFunc := func(ctrl *gomock.Controller) TxVerifier {
return executor.NewMockManager(ctrl)
}
if tt.txVerifierFunc != nil {
txVerifierFunc = tt.txVerifierFunc
}

appSenderFunc := func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
}
if tt.appSenderFunc != nil {
appSenderFunc = tt.appSenderFunc
}

n := New(
&snow.Context{
Log: logging.NoLog{},
},
parser,
tt.managerFunc(ctrl),
tt.mempoolFunc(ctrl),
tt.appSenderFunc(ctrl),
txVerifierFunc(ctrl),
mempoolFunc(ctrl),
appSenderFunc(ctrl),
)
err = n.IssueTx(context.Background(), &txs.Tx{})
require.ErrorIs(err, tt.expectedErr)
Expand All @@ -367,9 +358,6 @@ func TestNetworkIssueVerifiedTx(t *testing.T) {
mempool.EXPECT().MarkDropped(gomock.Any(), gomock.Any())
return mempool
},
appSenderFunc: func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
},
expectedErr: errTest,
},
{
Expand Down Expand Up @@ -401,14 +389,28 @@ func TestNetworkIssueVerifiedTx(t *testing.T) {
})
require.NoError(err)

mempoolFunc := func(ctrl *gomock.Controller) mempool.Mempool {
return mempool.NewMockMempool(ctrl)
}
if tt.mempoolFunc != nil {
mempoolFunc = tt.mempoolFunc
}

appSenderFunc := func(ctrl *gomock.Controller) common.AppSender {
return common.NewMockSender(ctrl)
}
if tt.appSenderFunc != nil {
appSenderFunc = tt.appSenderFunc
}

n := New(
&snow.Context{
Log: logging.NoLog{},
},
parser,
executor.NewMockManager(ctrl), // Should never verify a tx
tt.mempoolFunc(ctrl),
tt.appSenderFunc(ctrl),
mempoolFunc(ctrl),
appSenderFunc(ctrl),
)
err = n.IssueVerifiedTx(context.Background(), &txs.Tx{})
require.ErrorIs(err, tt.expectedErr)
Expand Down
Loading
Loading