Skip to content

Remove Network interface from Builder #2312

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

Merged
merged 9 commits into from
Nov 16, 2023
Merged
9 changes: 0 additions & 9 deletions vms/platformvm/block/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ var (
type Builder interface {
mempool.Mempool
mempool.BlockTimer
Network

// BuildBlock is called on timer clock to attempt to create
// next block
Expand All @@ -54,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 @@ -75,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 @@ -87,12 +84,6 @@ 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
}
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.IssueTx(context.Background(), 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
10 changes: 9 additions & 1 deletion vms/platformvm/block/builder/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ type environment struct {
Builder
blkManager blockexecutor.Manager
mempool mempool.Mempool
network Network
sender *common.SenderTest

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

res.network = NewNetwork(
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
38 changes: 23 additions & 15 deletions vms/platformvm/block/builder/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ 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"
)

// We allow [recentCacheSize] to be fairly large because we only store hashes
Expand All @@ -39,9 +41,11 @@ type network struct {
// We embed a noop handler for all unhandled messages
common.AppHandler

ctx *snow.Context
blkBuilder *builder
appSender common.AppSender
ctx *snow.Context
manager executor.Manager
mempool mempool.Mempool
partialSyncPrimaryNetwork bool
appSender common.AppSender

// gossip related attributes
recentTxsLock sync.Mutex
Expand All @@ -50,16 +54,20 @@ type network struct {

func NewNetwork(
ctx *snow.Context,
blkBuilder *builder,
manager executor.Manager,
mempool mempool.Mempool,
partialSyncPrimaryNetwork bool,
appSender common.AppSender,
) Network {
return &network{
AppHandler: common.NewNoOpAppHandler(ctx.Log),

ctx: ctx,
blkBuilder: blkBuilder,
appSender: appSender,
recentTxs: &cache.LRU[ids.ID, struct{}]{Size: recentCacheSize},
ctx: ctx,
manager: manager,
mempool: mempool,
partialSyncPrimaryNetwork: partialSyncPrimaryNetwork,
appSender: appSender,
recentTxs: &cache.LRU[ids.ID, struct{}]{Size: recentCacheSize},
}
}

Expand All @@ -69,7 +77,7 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b
zap.Int("messageLen", len(msgBytes)),
)

if n.blkBuilder.txExecutorBackend.Config.PartialSyncPrimaryNetwork {
if n.partialSyncPrimaryNetwork {
n.ctx.Log.Debug("dropping AppGossip message",
zap.String("reason", "primary network is not being fully synced"),
)
Expand Down Expand Up @@ -109,7 +117,7 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b
n.ctx.Lock.Lock()
defer n.ctx.Lock.Unlock()

if reason := n.blkBuilder.GetDropReason(txID); reason != nil {
if reason := n.mempool.GetDropReason(txID); reason != nil {
// If the tx is being dropped - just ignore it
return nil
}
Expand All @@ -126,21 +134,21 @@ func (n *network) AppGossip(ctx context.Context, nodeID ids.NodeID, msgBytes []b

func (n *network) IssueTx(ctx context.Context, tx *txs.Tx) error {
txID := tx.ID()
if n.blkBuilder.Mempool.Has(txID) {
if n.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 := n.blkBuilder.blkManager.VerifyTx(tx); err != nil {
n.blkBuilder.Mempool.MarkDropped(txID, err)
if err := n.manager.VerifyTx(tx); err != nil {
n.mempool.MarkDropped(txID, err)
return err
}

// If we are partially syncing the Primary Network, we should not be
// maintaining the transaction mempool locally.
if !n.blkBuilder.txExecutorBackend.Config.PartialSyncPrimaryNetwork {
if err := n.blkBuilder.Mempool.Add(tx); err != nil {
if !n.partialSyncPrimaryNetwork {
if err := n.mempool.Add(tx); err != nil {
return err
}
}
Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/block/builder/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestMempoolValidGossipedTxIsAddedToMempool(t *testing.T) {
// Free lock because [AppGossip] waits for the context lock
env.ctx.Lock.Unlock()
// show that unknown tx is added to mempool
require.NoError(env.AppGossip(context.Background(), nodeID, msgBytes))
require.NoError(env.network.AppGossip(context.Background(), nodeID, msgBytes))
require.True(env.Builder.Has(txID))
// Grab lock back
env.ctx.Lock.Lock()
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestMempoolInvalidGossipedTxIsNotAddedToMempool(t *testing.T) {
msgBytes, err := message.Build(&msg)
require.NoError(err)
env.ctx.Lock.Unlock()
require.NoError(env.AppGossip(context.Background(), nodeID, msgBytes))
require.NoError(env.network.AppGossip(context.Background(), nodeID, msgBytes))
env.ctx.Lock.Lock()
require.False(env.Builder.Has(txID))
}
Expand All @@ -125,7 +125,7 @@ func TestMempoolNewLocaTxIsGossiped(t *testing.T) {
tx := getValidTx(env.txBuilder, t)
txID := tx.ID()

require.NoError(env.Builder.IssueTx(context.Background(), tx))
require.NoError(env.network.IssueTx(context.Background(), tx))
require.NotNil(gossipedBytes)

// show gossiped bytes can be decoded to the original tx
Expand Down
16 changes: 8 additions & 8 deletions vms/platformvm/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,7 @@ func (s *Service) AddValidator(req *http.Request, args *AddValidatorArgs, reply

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1394,7 +1394,7 @@ func (s *Service) AddDelegator(req *http.Request, args *AddDelegatorArgs, reply

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1500,7 +1500,7 @@ func (s *Service) AddSubnetValidator(req *http.Request, args *AddSubnetValidator

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1576,7 +1576,7 @@ func (s *Service) CreateSubnet(req *http.Request, args *CreateSubnetArgs, respon

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1672,7 +1672,7 @@ func (s *Service) ExportAVAX(req *http.Request, args *ExportAVAXArgs, response *

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1757,7 +1757,7 @@ func (s *Service) ImportAVAX(req *http.Request, args *ImportAVAXArgs, response *

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -1881,7 +1881,7 @@ func (s *Service) CreateBlockchain(req *http.Request, args *CreateBlockchainArgs

return utils.Err(
err,
s.vm.Builder.IssueTx(req.Context(), tx),
s.vm.Network.IssueTx(req.Context(), tx),
user.Close(),
)
}
Expand Down Expand Up @@ -2173,7 +2173,7 @@ func (s *Service) IssueTx(req *http.Request, args *api.FormattedTx, response *ap
s.vm.ctx.Lock.Lock()
defer s.vm.ctx.Lock.Unlock()

if err := s.vm.Builder.IssueTx(req.Context(), tx); err != nil {
if err := s.vm.Network.IssueTx(req.Context(), tx); err != nil {
return fmt.Errorf("couldn't issue tx: %w", err)
}

Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,12 @@ func TestGetTxStatus(t *testing.T) {
service.vm.ctx.Lock.Lock()

// put the chain in existing chain list
err = service.vm.Builder.IssueTx(context.Background(), tx)
err = service.vm.Network.IssueTx(context.Background(), tx)
require.ErrorIs(err, database.ErrNotFound) // Missing shared memory UTXO

mutableSharedMemory.SharedMemory = sm

require.NoError(service.vm.Builder.IssueTx(context.Background(), tx))
require.NoError(service.vm.Network.IssueTx(context.Background(), tx))

block, err := service.vm.BuildBlock(context.Background())
require.NoError(err)
Expand Down Expand Up @@ -339,7 +339,7 @@ func TestGetTx(t *testing.T) {

service.vm.ctx.Lock.Lock()

require.NoError(service.vm.Builder.IssueTx(context.Background(), tx))
require.NoError(service.vm.Network.IssueTx(context.Background(), tx))

blk, err := service.vm.BuildBlock(context.Background())
require.NoError(err)
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/validator_set_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func addPrimaryValidatorWithoutBLSKey(vm *VM, data *validatorInputData) (*state.

func internalAddValidator(vm *VM, signedTx *txs.Tx) (*state.Staker, error) {
stakerTx := signedTx.Unsigned.(txs.StakerTx)
if err := vm.Builder.IssueTx(context.Background(), signedTx); err != nil {
if err := vm.Network.IssueTx(context.Background(), signedTx); err != nil {
return nil, fmt.Errorf("could not add tx to mempool: %w", err)
}

Expand Down Expand Up @@ -802,7 +802,7 @@ func buildVM(t *testing.T) (*VM, ids.ID, error) {
if err != nil {
return nil, ids.Empty, err
}
if err := vm.Builder.IssueTx(context.Background(), testSubnet1); err != nil {
if err := vm.Network.IssueTx(context.Background(), testSubnet1); err != nil {
return nil, ids.Empty, err
}

Expand Down
9 changes: 8 additions & 1 deletion vms/platformvm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var (
type VM struct {
config.Config
blockbuilder.Builder
blockbuilder.Network
validators.State

metrics metrics.Metrics
Expand Down Expand Up @@ -189,13 +190,19 @@ func (vm *VM) Initialize(
txExecutorBackend,
validatorManager,
)
vm.Network = blockbuilder.NewNetwork(
txExecutorBackend.Ctx,
vm.manager,
mempool,
txExecutorBackend.Config.PartialSyncPrimaryNetwork,
appSender,
)
vm.Builder = blockbuilder.New(
mempool,
vm.txBuilder,
txExecutorBackend,
vm.manager,
toEngine,
appSender,
)

// Create all of the chains that the database says exist
Expand Down
Loading