Skip to content

move syncers to atomic vm #1009

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

Merged
merged 2 commits into from
Jun 6, 2025
Merged

move syncers to atomic vm #1009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd9055b
move syncers to atomic vm
ceyonur Jun 6, 2025
635155f
reviews
ceyonur Jun 6, 2025
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
14 changes: 8 additions & 6 deletions plugin/evm/atomic/sync/extender.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ type extender struct {
}

// Initialize initializes the sync extender with the backend and trie.
func NewExtender(backend *state.AtomicBackend, trie *state.AtomicTrie, requestSize uint16) *extender {
return &extender{
backend: backend,
trie: trie,
requestSize: requestSize,
}
func NewExtender() *extender {
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

Why is there a constructor if it doesn't do anything?

Copy link
Collaborator Author

@ceyonur ceyonur Jun 16, 2025

Choose a reason for hiding this comment

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

bc it was using an unexported struct

return &extender{}
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

Returning non-exported types can cause problems at the call sites as they're really only useable as interfaces, but because they're not exported there's no way to check the documentation to see if they implement the interface of interest.

}

func (a *extender) Initialize(backend *state.AtomicBackend, trie *state.AtomicTrie, requestSize uint16) {
a.backend = backend
a.trie = trie
a.requestSize = requestSize
Comment on lines +33 to +35
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

This is just a constructor.

Copy link
Collaborator Author

@ceyonur ceyonur Jun 16, 2025

Choose a reason for hiding this comment

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

In fact yes, but we need to initialize this after calling the innerVM.Initialize() (see #998)

}

func (a *extender) Sync(ctx context.Context, client syncclient.LeafClient, verDB *versiondb.Database, summary message.Syncable) error {
Expand Down
32 changes: 27 additions & 5 deletions plugin/evm/atomic/sync/leaf_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,45 @@
package sync

import (
"context"
"errors"

"github.com/ava-labs/avalanchego/codec"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/coreth/plugin/evm/message"
"github.com/ava-labs/coreth/sync/handlers"
"github.com/ava-labs/coreth/sync/handlers/stats"

"github.com/ava-labs/libevm/metrics"
"github.com/ava-labs/libevm/triedb"
)

// leafHandler is a wrapper around handlers.LeafRequestHandler that allows for initialization after creation
var (
_ handlers.LeafRequestHandler = (*uninitializedHandler)(nil)

errUninitialized = errors.New("uninitialized handler")
)

type uninitializedHandler struct{}

func (h *uninitializedHandler) OnLeafsRequest(ctx context.Context, nodeID ids.NodeID, requestID uint32, leafsRequest message.LeafsRequest) ([]byte, error) {
return nil, errUninitialized
}

// atomicLeafHandler is a wrapper around handlers.LeafRequestHandler that allows for initialization after creation
type leafHandler struct {
handlers.LeafRequestHandler
}

// Initialize initializes the leafHandler with the provided atomicTrieDB, trieKeyLength, and networkCodec
func NewLeafHandler(atomicTrieDB *triedb.Database, trieKeyLength int, networkCodec codec.Manager) *leafHandler {
handlerStats := stats.GetOrRegisterHandlerStats(metrics.Enabled)
// NewAtomicLeafHandler returns a new uninitialzied atomicLeafHandler that can be later initialized
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

that can be later initialized

It can't be. It always returns errUninitialized.

EDIT: looking at the code below, I think you mean "can be later replaced".

func NewLeafHandler() *leafHandler {
return &leafHandler{
LeafRequestHandler: handlers.NewLeafsRequestHandler(atomicTrieDB, trieKeyLength, nil, networkCodec, handlerStats),
LeafRequestHandler: &uninitializedHandler{},
}
}

// Initialize initializes the atomicLeafHandler with the provided atomicTrieDB, trieKeyLength, and networkCodec
func (a *leafHandler) Initialize(atomicTrieDB *triedb.Database, trieKeyLength int, networkCodec codec.Manager) {
handlerStats := stats.GetOrRegisterHandlerStats(metrics.Enabled)
a.LeafRequestHandler = handlers.NewLeafsRequestHandler(atomicTrieDB, trieKeyLength, nil, networkCodec, handlerStats)
}
10 changes: 6 additions & 4 deletions plugin/evm/atomic/sync/summary_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ type summaryProvider struct {
trie *state.AtomicTrie
}

func NewSummaryProvider(atomicTrie *state.AtomicTrie) *summaryProvider {
return &summaryProvider{
trie: atomicTrie,
}
func NewSummaryProvider() *summaryProvider {
return &summaryProvider{}
}

func (a *summaryProvider) Initialize(trie *state.AtomicTrie) {
a.trie = trie
Comment on lines +23 to +28
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

All my comments on *extender apply here. Non-exported type, no-op constructor, and an initialiser that's just a constructor.

}

// StateSummaryAtBlock returns the block state summary at [blk] if valid.
Expand Down
28 changes: 27 additions & 1 deletion plugin/evm/atomic/vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import (
avalanchecommon "github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
"github.com/ava-labs/avalanchego/utils/constants"
"github.com/ava-labs/avalanchego/utils/timer/mockable"

"github.com/ava-labs/coreth/params"
"github.com/ava-labs/coreth/params/extras"
"github.com/ava-labs/coreth/plugin/evm/atomic/state"
"github.com/ava-labs/coreth/plugin/evm/atomic/sync"
"github.com/ava-labs/coreth/plugin/evm/atomic/txpool"
"github.com/ava-labs/coreth/plugin/evm/extension"
"github.com/ava-labs/coreth/plugin/evm/message"

"github.com/ava-labs/libevm/common"
)
Expand Down Expand Up @@ -46,6 +49,8 @@ type InnerVM interface {
type VM struct {
InnerVM
ctx *snow.Context

clock mockable.Clock
}

func WrapVM(vm InnerVM) *VM {
Expand Down Expand Up @@ -81,9 +86,24 @@ func (vm *VM) Initialize(
// Create the atomic extension structs
// some of them need to be initialized after the inner VM is initialized
blockExtender := newBlockExtender(extDataHashes, vm)
syncExtender := sync.NewExtender()
syncProvider := sync.NewSummaryProvider()
// Create and pass the leaf handler to the atomic extension
// it will be initialized after the inner VM is initialized
leafHandler := sync.NewLeafHandler()
atomicLeafTypeConfig := &extension.LeafRequestConfig{
LeafType: sync.TrieNode,
MetricName: "sync_atomic_trie_leaves",
Handler: leafHandler,
}

extensionConfig := &extension.Config{
BlockExtender: blockExtender,
BlockExtender: blockExtender,
SyncableParser: sync.NewSummaryParser(),
SyncExtender: syncExtender,
SyncSummaryProvider: syncProvider,
ExtraSyncLeafHandlerConfig: atomicLeafTypeConfig,
Clock: &vm.clock,
}
if err := vm.SetExtensionConfig(extensionConfig); err != nil {
return fmt.Errorf("failed to set extension config: %w", err)
Expand All @@ -103,6 +123,12 @@ func (vm *VM) Initialize(
); err != nil {
return fmt.Errorf("failed to initialize inner VM: %w", err)
}

// Atomic backend is available now, we can initialize structs that depend on it
syncProvider.Initialize(vm.AtomicBackend().AtomicTrie())
syncExtender.Initialize(vm.AtomicBackend(), vm.AtomicBackend().AtomicTrie(), vm.Config().StateSyncRequestSize)
leafHandler.Initialize(vm.AtomicBackend().AtomicTrie().TrieDB(), state.TrieKeyLength, message.Codec)

Comment on lines +128 to +131
Copy link
Collaborator

@ARR4N ARR4N Jun 9, 2025

Choose a reason for hiding this comment

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

This is confusing because of the embedded InnerVM and took me about 10 minutes to figure out, including opening the code locally. As it was merged it suggests to the reader that there was no need to already initialise the InnerVM; I suggest making explicit that you are using the InnerVM's method.

atomicBE := vm.InnerVM.AtomicBackend()
atomicTrie := atomicBE.AtomicTrie()
syncProvider.Initialize(atomicTrie)
syncExtender.Initialize(atomicBE, atomicTrie, vm.Config().StateSyncRequestSize)
leafHandler.Initialize(atomicTrie.TrieDB(), state.TrieKeyLength, message.Codec)

I appreciate that this is a means to an end, but I'm still really concerned about this pattern. The PR chain needs to eventually simplify construction and initialisation.

Copy link
Collaborator Author

@ceyonur ceyonur Jun 16, 2025

Choose a reason for hiding this comment

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

In one of the previous PRs it was suggested to call InnerVM methods implicitly to reduce the complexity, but I agree it's easier to figure them out if they were explicit.

Copy link
Collaborator Author

@ceyonur ceyonur Jun 16, 2025

Choose a reason for hiding this comment

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

On the other hand atomicBackend will be moved to atomic/VM in #1011

return nil
}

Expand Down
33 changes: 32 additions & 1 deletion plugin/evm/extension/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,25 @@ import (
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow/consensus/snowman"
avalanchecommon "github.com/ava-labs/avalanchego/snow/engine/common"
"github.com/ava-labs/avalanchego/utils/timer/mockable"

"github.com/ava-labs/avalanchego/snow/engine/snowman/block"

"github.com/ava-labs/coreth/eth"
"github.com/ava-labs/coreth/params/extras"
"github.com/ava-labs/coreth/plugin/evm/config"
"github.com/ava-labs/coreth/plugin/evm/message"
"github.com/ava-labs/coreth/plugin/evm/sync"
"github.com/ava-labs/coreth/sync/handlers"

"github.com/ava-labs/libevm/core/types"
)

var errNilConfig = errors.New("nil extension config")
var (
errNilConfig = errors.New("nil extension config")
errNilSyncSummaryProvider = errors.New("nil sync summary provider")
errNilSyncableParser = errors.New("nil syncable parser")
)

type ExtensibleVM interface {
// SetExtensionConfig sets the configuration for the VM extension
Expand All @@ -39,6 +46,8 @@ type ExtensibleVM interface {
IsBootstrapped() bool
// Ethereum returns the Ethereum client
Ethereum() *eth.Ethereum
// Config returns the configuration for the VM
Config() config.Config
}

// InnerVM is the interface that must be implemented by the VM
Expand Down Expand Up @@ -93,14 +102,36 @@ type LeafRequestConfig struct {

// Config is the configuration for the VM extension
type Config struct {
// SyncSummaryProvider is the sync summary provider to use
// for the VM to be used in syncer.
// It's required and should be non-nil
SyncSummaryProvider sync.SummaryProvider
// SyncExtender can extend the syncer to handle custom sync logic.
// It's optional and can be nil
SyncExtender sync.Extender
// SyncableParser is to parse summary messages from the network.
// It's required and should be non-nil
SyncableParser message.SyncableParser
// BlockExtender allows the VM extension to create an extension to handle block processing events.
// It's optional and can be nil
BlockExtender BlockExtender
// ExtraSyncLeafHandlerConfig is the extra configuration to handle leaf requests
// in the network and syncer. It's optional and can be nil
ExtraSyncLeafHandlerConfig *LeafRequestConfig
// Clock is the clock to use for time related operations.
// It's optional and can be nil
Clock *mockable.Clock
}

func (c *Config) Validate() error {
if c == nil {
return errNilConfig
}
if c.SyncSummaryProvider == nil {
return errNilSyncSummaryProvider
}
if c.SyncableParser == nil {
return errNilSyncableParser
}
return nil
}
39 changes: 19 additions & 20 deletions plugin/evm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"github.com/ava-labs/coreth/params/extras"
"github.com/ava-labs/coreth/plugin/evm/atomic"
atomicstate "github.com/ava-labs/coreth/plugin/evm/atomic/state"
atomicsync "github.com/ava-labs/coreth/plugin/evm/atomic/sync"
atomictxpool "github.com/ava-labs/coreth/plugin/evm/atomic/txpool"
atomicvm "github.com/ava-labs/coreth/plugin/evm/atomic/vm"
"github.com/ava-labs/coreth/plugin/evm/config"
Expand Down Expand Up @@ -274,7 +273,7 @@ type VM struct {
builder *blockBuilder

baseCodec codec.Registry
clock mockable.Clock
clock *mockable.Clock
mempool *atomictxpool.Mempool

shutdownChan chan struct{}
Expand Down Expand Up @@ -321,7 +320,7 @@ type VM struct {
func (vm *VM) CodecRegistry() codec.Registry { return vm.baseCodec }

// Clock implements the secp256k1fx interface
func (vm *VM) Clock() *mockable.Clock { return &vm.clock }
func (vm *VM) Clock() *mockable.Clock { return vm.clock }

// Logger implements the secp256k1fx interface
func (vm *VM) Logger() logging.Logger { return vm.ctx.Log }
Expand Down Expand Up @@ -355,6 +354,9 @@ func (vm *VM) Initialize(
if err := vm.extensionConfig.Validate(); err != nil {
return fmt.Errorf("failed to validate extension config: %w", err)
}
if vm.extensionConfig.Clock != nil {
vm.clock = vm.extensionConfig.Clock
}
vm.config.SetDefaults(defaultTxPoolConfig)
if len(configBytes) > 0 {
if err := json.Unmarshal(configBytes, &vm.config); err != nil {
Expand Down Expand Up @@ -651,10 +653,10 @@ func (vm *VM) initializeChain(lastAcceptedHash common.Hash) error {
dummy.NewDummyEngine(
callbacks,
dummy.Mode{},
&vm.clock,
vm.clock,
desiredTargetExcess,
),
&vm.clock,
vm.clock,
)
if err != nil {
return err
Expand All @@ -673,7 +675,7 @@ func (vm *VM) initializeChain(lastAcceptedHash common.Hash) error {
return vm.initChainState(vm.blockChain.LastAcceptedBlock())
}

// initializeStateSyncClient initializes the client for performing state sync.
// initializeStateSync initializes the vm for performing state sync and responding to peer requests.
// If state sync is disabled, this function will wipe any ongoing summary from
// disk to ensure that we do not continue syncing from an invalid snapshot.
func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
Expand All @@ -688,6 +690,8 @@ func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
}.BackendConstructor,
},
)
leafHandlers := make(LeafHandlers)
leafMetricsNames := make(map[message.NodeType]string)
// register default leaf request handler for state trie
syncStats := handlerstats.GetOrRegisterHandlerStats(metrics.Enabled)
stateLeafRequestConfig := &extension.LeafRequestConfig{
Expand All @@ -699,17 +703,15 @@ func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
syncStats,
),
}
leafHandlers[stateLeafRequestConfig.LeafType] = stateLeafRequestConfig.Handler
leafMetricsNames[stateLeafRequestConfig.LeafType] = stateLeafRequestConfig.MetricName

atomicLeafHandlerConfig := &extension.LeafRequestConfig{
LeafType: atomicsync.TrieNode,
MetricName: "sync_atomic_trie_leaves",
Handler: atomicsync.NewLeafHandler(vm.atomicBackend.AtomicTrie().TrieDB(), atomicstate.TrieKeyLength, vm.networkCodec),
extraLeafConfig := vm.extensionConfig.ExtraSyncLeafHandlerConfig
if extraLeafConfig != nil {
leafHandlers[extraLeafConfig.LeafType] = extraLeafConfig.Handler
leafMetricsNames[extraLeafConfig.LeafType] = extraLeafConfig.MetricName
}

leafHandlers := make(LeafHandlers)
leafHandlers[stateLeafRequestConfig.LeafType] = stateLeafRequestConfig.Handler
leafHandlers[atomicLeafHandlerConfig.LeafType] = atomicLeafHandlerConfig.Handler

networkHandler := newNetworkHandler(
vm.blockChain,
vm.chaindb,
Expand All @@ -720,7 +722,7 @@ func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
)
vm.Network.SetRequestHandler(networkHandler)

vm.Server = vmsync.NewServer(vm.blockChain, atomicsync.NewSummaryProvider(vm.atomicBackend.AtomicTrie()), vm.config.StateSyncCommitInterval)
vm.Server = vmsync.NewServer(vm.blockChain, vm.extensionConfig.SyncSummaryProvider, vm.config.StateSyncCommitInterval)
stateSyncEnabled := vm.stateSyncEnabled(lastAcceptedHeight)
// parse nodeIDs from state sync IDs in vm config
var stateSyncIDs []ids.NodeID
Expand All @@ -737,9 +739,6 @@ func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
}

// Initialize the state sync client
leafMetricsNames := make(map[message.NodeType]string)
leafMetricsNames[stateLeafRequestConfig.LeafType] = stateLeafRequestConfig.MetricName
leafMetricsNames[atomicLeafHandlerConfig.LeafType] = atomicLeafHandlerConfig.MetricName

vm.Client = vmsync.NewClient(&vmsync.ClientConfig{
Chain: vm.eth,
Expand All @@ -763,8 +762,8 @@ func (vm *VM) initializeStateSync(lastAcceptedHeight uint64) error {
MetadataDB: vm.metadataDB,
ToEngine: vm.toEngine,
Acceptor: vm,
Parser: atomicsync.NewSummaryParser(),
Extender: atomicsync.NewExtender(vm.atomicBackend, vm.atomicBackend.AtomicTrie(), vm.config.StateSyncRequestSize),
Parser: vm.extensionConfig.SyncableParser,
Extender: vm.extensionConfig.SyncExtender,
})

// If StateSync is disabled, clear any ongoing summary so that we will not attempt to resume
Expand Down
6 changes: 6 additions & 0 deletions plugin/evm/vm_extensible.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/ava-labs/coreth/eth"
"github.com/ava-labs/coreth/plugin/evm/atomic/state"
"github.com/ava-labs/coreth/plugin/evm/atomic/txpool"
"github.com/ava-labs/coreth/plugin/evm/config"
"github.com/ava-labs/coreth/plugin/evm/extension"
)

Expand Down Expand Up @@ -63,6 +64,11 @@ func (vm *VM) Ethereum() *eth.Ethereum {
return vm.eth
}

func (vm *VM) Config() config.Config {
return vm.config
}

// TODO: remove these
func (vm *VM) AtomicBackend() *state.AtomicBackend {
return vm.atomicBackend
}
Expand Down
Loading