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.

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

Move network implementation to separate package #2296

Merged
merged 35 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e910b5d
Move management of platformvm preferred block to `executor.Manager`
dhrubabasu Nov 10, 2023
5851eed
Add `VerifyTx` to `executor.Manager`
dhrubabasu Nov 10, 2023
a93a379
`vms/platformvm`: Remove `Network` interface from `Builder`
dhrubabasu Nov 10, 2023
149d211
move to own pkg
dhrubabasu Nov 10, 2023
691f9ec
remove todo
dhrubabasu Nov 10, 2023
87dbb0c
nit
dhrubabasu Nov 10, 2023
23c2ce7
nit
dhrubabasu Nov 10, 2023
85adc91
reduce diff
dhrubabasu Nov 12, 2023
c418f6c
Merge branch 'move-preferred-to-blk-manager' into move-verify-tx-to-b…
dhrubabasu Nov 12, 2023
bdd6da1
fix merge
dhrubabasu Nov 12, 2023
bbcc960
merged
dhrubabasu Nov 14, 2023
662bbdc
merged
dhrubabasu Nov 14, 2023
46cd75c
merged
dhrubabasu Nov 14, 2023
f739b51
Move `AddUnverifiedTx` logic to `Network` interface
dhrubabasu Nov 14, 2023
b3844a9
Remove `AddUnverifiedTx` from `Builder`
dhrubabasu Nov 14, 2023
fec3091
Remove `Network` interface from `Builder`
dhrubabasu Nov 14, 2023
ac427c5
merged
dhrubabasu Nov 14, 2023
c2515b7
nit
dhrubabasu Nov 14, 2023
3433403
maybe
dhrubabasu Nov 15, 2023
7772c04
fill in contextx
dhrubabasu Nov 15, 2023
068469a
fill in ctxs
dhrubabasu Nov 15, 2023
290e5f3
merged
dhrubabasu Nov 15, 2023
8131488
Merge branch 'remove-network-interface-from-builder' into remove-add-…
dhrubabasu Nov 15, 2023
cbc5cee
push
dhrubabasu Nov 15, 2023
9a4b955
nit
dhrubabasu Nov 15, 2023
ac930ea
revert
dhrubabasu Nov 15, 2023
8fb176c
merged
dhrubabasu Nov 15, 2023
d69f0e4
Merge branch 'delete-add-unverified-tx' into remove-network-interface…
dhrubabasu Nov 15, 2023
5c92cbf
Merge branch 'remove-network-interface-from-builder' into remove-add-…
dhrubabasu Nov 15, 2023
599248a
merged
dhrubabasu Nov 16, 2023
eef0f77
Merge branch 'remove-network-interface-from-builder' into remove-add-…
dhrubabasu Nov 16, 2023
f6a9140
merged
dhrubabasu Nov 16, 2023
1d1be7c
Merge branch 'dev' into remove-add-unverified-tx
dhrubabasu Nov 16, 2023
e0f1b94
nit
dhrubabasu Nov 17, 2023
d2cf336
Apply suggestions from code review
StephenButtolph Nov 17, 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
36 changes: 0 additions & 36 deletions vms/platformvm/block/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ var (
type Builder interface {
mempool.Mempool
mempool.BlockTimer
Network

// AddUnverifiedTx verifier the tx before adding it to mempool
AddUnverifiedTx(tx *txs.Tx) error

// BuildBlock is called on timer clock to attempt to create
// next block
Expand All @@ -57,7 +53,6 @@ type Builder interface {
// builder implements a simple builder to convert txs into valid blocks
type builder struct {
mempool.Mempool
Network

txBuilder txbuilder.Builder
txExecutorBackend *txexecutor.Backend
Expand All @@ -78,7 +73,6 @@ func New(
txExecutorBackend *txexecutor.Backend,
blkManager blockexecutor.Manager,
toEngine chan<- common.Message,
appSender common.AppSender,
) Builder {
builder := &builder{
Mempool: mempool,
Expand All @@ -90,40 +84,10 @@ func New(

builder.timer = timer.NewTimer(builder.setNextBuildBlockTime)

builder.Network = NewNetwork(
txExecutorBackend.Ctx,
builder,
appSender,
)

go txExecutorBackend.Ctx.Log.RecoverAndPanic(builder.timer.Dispatch)
return builder
}

// AddUnverifiedTx verifies a transaction and attempts to add it to the mempool
func (b *builder) AddUnverifiedTx(tx *txs.Tx) error {
txID := tx.ID()
if b.Mempool.Has(txID) {
// If the transaction is already in the mempool - then it looks the same
// as if it was successfully added
return nil
}

if err := b.blkManager.VerifyTx(tx); err != nil {
b.MarkDropped(txID, err)
return err
}

// If we are partially syncing the Primary Network, we should not be
// maintaining the transaction mempool locally.
if !b.txExecutorBackend.Config.PartialSyncPrimaryNetwork {
if err := b.Mempool.Add(tx); err != nil {
return err
}
}
return b.GossipTx(context.TODO(), tx)
}

// BuildBlock builds a block to be added to consensus.
// This method removes the transactions from the returned
// blocks from the mempool.
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/block/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestBlockBuilderAddLocalTx(t *testing.T) {
env.sender.SendAppGossipF = func(context.Context, []byte) error {
return nil
}
require.NoError(env.Builder.AddUnverifiedTx(tx))
require.NoError(env.network.IssueTx(context.Background(), tx))
require.True(env.mempool.Has(txID))

// show that build block include that tx and removes it from mempool
Expand Down
25 changes: 24 additions & 1 deletion vms/platformvm/block/builder/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/ava-labs/avalanchego/vms/platformvm/config"
"github.com/ava-labs/avalanchego/vms/platformvm/fx"
"github.com/ava-labs/avalanchego/vms/platformvm/metrics"
"github.com/ava-labs/avalanchego/vms/platformvm/network"
"github.com/ava-labs/avalanchego/vms/platformvm/reward"
"github.com/ava-labs/avalanchego/vms/platformvm/state"
"github.com/ava-labs/avalanchego/vms/platformvm/status"
Expand Down Expand Up @@ -87,6 +88,7 @@ type environment struct {
Builder
blkManager blockexecutor.Manager
mempool mempool.Mempool
network network.Network
sender *common.SenderTest

isBootstrapped *utils.Atomic[bool]
Expand Down Expand Up @@ -168,13 +170,20 @@ func newEnvironment(t *testing.T) *environment {
pvalidators.TestManager,
)

res.network = network.New(
res.backend.Ctx,
res.blkManager,
res.mempool,
res.backend.Config.PartialSyncPrimaryNetwork,
res.sender,
)

res.Builder = New(
res.mempool,
res.txBuilder,
&res.backend,
res.blkManager,
nil, // toEngine,
res.sender,
)

res.blkManager.SetPreference(genesisID)
Expand Down Expand Up @@ -420,3 +429,17 @@ func shutdownEnvironment(env *environment) error {
env.baseDB.Close(),
)
}

func getValidTx(txBuilder txbuilder.Builder, t *testing.T) *txs.Tx {
Copy link
Contributor Author

@dhrubabasu dhrubabasu Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in builder_test.go and was originally in network_test.go. Added here as we moved network_test.go to its own pkg

tx, err := txBuilder.NewCreateChainTx(
testSubnet1.ID(),
nil,
constants.AVMID,
nil,
"chain name",
[]*secp256k1.PrivateKey{testSubnet1ControlKeys[0], testSubnet1ControlKeys[1]},
ids.ShortEmpty,
)
require.NoError(t, err)
return tx
}
147 changes: 0 additions & 147 deletions vms/platformvm/block/builder/network_test.go

This file was deleted.

14 changes: 14 additions & 0 deletions vms/platformvm/network/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package network

import (
"testing"

"go.uber.org/goleak"
)

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
Loading
Loading