-
Notifications
You must be signed in to change notification settings - Fork 741
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
Client Full Sync Support #1000
Client Full Sync Support #1000
Comments
Have made various updates to the initial comment, please read on site. |
Hi @holgerd77, we should indeed focus on full sync first. A loose definition here would be to "fetch blocks" and run these on the chain. This is how you get your MPT to the tip of the chain (as opposed to fast sync where you download the entire MPT, or beam sync which selectively downloads nodes of the MPT). For now I think we should not bother with things like PoA and focus on a PoW chain, although PoA can of course be implemented in parallel in the VM and should have semi-high priority. I think we should keep Block verification So just to be clear here: Serving the network |
Hi @jochem-brouwer, thanks for the answer on this, here are some additional notes from my side. FastSynchronizer/FullSynchronizerWe might be coming from some different angles here. I was looking at the code from FastSynchronizer and after skimming through the different functions like Following conclusions are possible: a) This is actually a full sync implementation and was unintentionally named "fast sync" by Vinay, this would speak for the "we just rename everything fastsync to fullsync" solution (wouldn't make sense to have laying a falsely implemented fastsync around) All I am saying is that we should have a closer look here to see what is actually the case to come to a structurally clean and well-grounded solution. My basis for my analysis is this ethereum/go-ethereum#1889 write-up on the fast sync mechanism. PoA / PoWThe
Will this reliably sync (since no-one is likely interfering) or will this get on the wrong track? Block VerificationJust a small note here: I would assume that if there is - e.g. on the Yolo testnet - really a consensus issue and the block is just deleted with the client not coming to a halt, this will just go into an eternal loop trying to process the same block over and over again (so we should rather catch here)? 🤔 |
Some notes here: Regarding syncing, I don't think in (a) that the naming was a mistake due to this comment. In full sync you don't fetch the state trie; you "calculate"/populate it by running the VM. The problem with (b) is that if you make FastSync depend upon FullSync this implies that FastSync also starts processing blocks from the root (which you don't want to do). You also can't just start running it at the head since you don't have the complete state trie at that point. The thing they have in common is that Fast/Full both need BlockFetcher, in Full you start running blocks from the genesis block, while in Fast you first download the entire state trie of a recent block and then start to run blocks from that recent block (not genesis). So it does not seem to me that FastSync inherits FullSync although it does have things in common. This corresponds to the analysis in geth. Note here that it implies fast sync at some point switches to "classical sync" (full sync). If we connect to If we process a block using |
In case we come to the conclusion that there is nothing fast sync specific yet in the code we nevertheless might want to consider just to rename, we don't want to do a fast sync implementation for quite some time, right? And it likely doesn't make that much sense to have a half-ready fast sync implementation exposed if we won't continue the work there. Have given the PoA spec some closer look over the weekend, I think I might just give this a try during the next 2-3 weeks, seems very much doable to me and I would be personally interested to take on this. For this catch case I am not talking about a bogus block but the case where the network distributes a valid one and we have got a consensus issue in our code. Wouldn't that lead to the situation where the same block is requested all over again and? Not an urgent question to solve though, we will just experience stuff like this in "life runs" and should be a somewhat easy fix then. |
I think this is a very good proposal; fast sync has all of the code which we need for full sync, and any specific fast-sync code does not appear to be present yet (have not found any). So just renaming this sounds like a good idea here. Yes, in case that there is a valid block where we have a consensus issue then the Note that if it was actually a bogus block (which has valid PoW otherwise it is not considered) then at some point the actual canonical chain would take over, since most of the hashrate is (should be) mining on valid blocks and not on bogus blocks. |
I would regard this as implemented, will close. 😄 |
As agreed upon within the team we want to start with implementing a full sync mechanism before going over to something more sophisticated (like e.g. beam sync, see #992), since full sync is the most simple and purist way of syncing and getting to a recent network state.
For a full sync overview see e.g. Ethhub, we will likely want to have some differentiated look at the several properties and will likely not just implement everything down on first round (respectively, also one possibility: ever) but concentrate on the properties we want to prioritize.
Note that this won't be
mainnet
ready implementation - minimally on first round - but it is rather targeted to be ready for some more lightweight testnets as well as serve simulation, test and development purposes.Here is some checklist for a first round on what might be the tasks (it's always really helpful to have a look at our diagram to see how things work together):
1. Syncing the blockchain
fast
sync related already in the code (needs some investigation or evidence) and if we should simply repurpose (respectively eventually just: "rename") e.g. FastSynchronizer ->FullSynchronizer
, alternative ways of integration respectively inheritance hierarchies, TBD by the sync method properties, needs a more close look into the implemented methods:2. Storing received blocks
3. Block validation
validateBLocks
(always? triggered by sync mode choice?) on chain creation?validateConsensus
(old:validatePow
) option, leave for PoW for now for performance reasons? Prioritization question on PoA, is a PoA implementation needed right now (what happens if left out on a relatively intimate network like the Yolo network?)4. Block verification
I'll take this as: verify the block hash by running the VM, correct me if this is the wrong semantics), on some thought this might be really relatively simple for now, my suggestion:
vm
option to Config, initialize with defaultv5
VM
if no VM passed in (I think it's attractive to do this exposure early on, not much extra cost and then people can directly start playing with their own customized/modfied VM instances)BlockFetcher
(e.g. store()) or - maybe better - along thefetched
event in the synchronizervm.ts
orBlockVerifier.ts
wrapper (I didn't get the suggestion:client.ts
TBH) or is directly executing on the VM enough for now?5. Serving the network
Serving already synced data to other peers to be a good network citizen 😄
Phew, also not un-complex, I would say on first round. But doable. 😄
That's a summary of my current view on this as some basis for further reflection, discussion and implementation. We might want to open dedicated issues on single items if things get too complex and keep this as a tracking issue. But let's see how things evolve.
The text was updated successfully, but these errors were encountered: