Skip to content

feat(all): dynamic state sync #870

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

Draft
wants to merge 94 commits into
base: master
Choose a base branch
from
Draft

Conversation

alarso16
Copy link
Contributor

@alarso16 alarso16 commented Mar 11, 2025

This PR denotes rebased commits from two other PRs (#771 and #830) to the new clean libevm history. The first PR denotes changes to geth's snap protocol to enable usage in our repo, since a there were a few differences (the comment describes it). The second PR is the final step in which there was a successful state sync on Fuji prior to Fortuna upgrade.

There's a few things that I should mention that are mostly related to structure.

  • The snap_manager.go logic is very similar to that of the downloader.go in geth. The SnapSync() method essentially processSnapSyncContent. I removed a lot of the (seemingly) excessive logic here, but it could still be simpler - specifically the stateSync channeling. There's also technically some race conditions involving the channelling in snapManager on interruption, but I'm pretty sure it will always exit gracefully with helpful error messages.
  • Prior to this PR, the state trie sync and atomic trie sync could be done parallelly or in either order. Now, it is changed to completely finish the atomic trie sync before beginning the snap sync, because we will perform atomic operations synchronously throughout the state sync. However, we could likely still do these parallelly. A channel was added to notify when atomic sync is completely, and we can thus start executing atomic operations as normal.
  • We cannot postpone verify/accept operations on atomic operations until the pivot, because this can cause a deadlock with the P-Chain in avalancego. Because of this, we perform atomic operations synchronously, and then after the sync is complete, we perform the non-atomic operations in order as they are buffered to catch up the state trie to the atomic trie.
  • Passing the lock from stateSyncerclient to DynamicSyncer is necessary because we need to block from queueing blocks once we finish the state sync, and then once the queue is cleared, we release the lock, allowing blocks to be verified as normal.
  • The DynamicSyncer was designed to use the minimal manager interface, which is exactly what is used by x/sync, but with a small type change for convenience right now (x/sync uses ids.ID for the root). The goal was to completely separate the eth-syncing logic from the DynamicSyncer, which focuses on the queuing and block operation logic.
  • Currently, this requires a slightly-altered libevm repo - the branch has the same name. This is the majority of seemingly unrelated changes with the StackTrie. Without PathDB, this should be unnecessary

@alarso16 alarso16 changed the title Alarso16/libevm upstream sync feat(all): dynamic state sync Apr 9, 2025
@alarso16 alarso16 added enhancement New feature or request after libevm To be done only after using libevm for good labels Apr 9, 2025
peer/network.go Outdated
var req message.Request
if _, err := n.codec.Unmarshal(request, &req); err != nil {
log.Debug("forwarding AppRequest to SDK network", "nodeID", nodeID, "requestID", requestID, "requestLen", len(request), "err", err)
Copy link

Choose a reason for hiding this comment

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

note once the new state sync algo is in place, the legacy handler can be removed and all peer requests can directly be forwarded to the "SDK" handler.

vdbBatch, err := b.vm.versiondb.CommitBatch()
func (b *Block) acceptAtomicOps() error {
vm := b.vm
_, lastHeight, err := vm.readLastAccepted()
Copy link

Choose a reason for hiding this comment

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

nit: I think you could get this like: vm.LastAcceptedBlock().Height()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, this is the one tracked by the engine?

@alarso16 alarso16 changed the base branch from libevm to master April 15, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
after libevm To be done only after using libevm for good Do not merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants