Skip to content

Commit

Permalink
Remove Network interface from Builder (#2312)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhrubabasu committed Nov 16, 2023
1 parent 6484de4 commit 348f842
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 88 deletions.
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

0 comments on commit 348f842

Please sign in to comment.