Skip to content
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

ChainSync framework #243

Merged
merged 21 commits into from
Feb 28, 2020
Merged

ChainSync framework #243

merged 21 commits into from
Feb 28, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Sets up chainsync thread and network polling
  • Implements Connect sync manager with Libp2p RPC service #223 by giving the ChainSyncer a channel from the networking thread to this thread
  • Implements Implement parsing of blocksync response into more usable types #227 By allowing the conversion from a BlockSyncResponse including the tipset bundle and status code into a result with ok type being a full tipset
    • Error type can be improved if needed to handle the network errors differently, but no need for that yet. Error type is currently String but should in future be an enum matching status codes
  • Created NetworkContext struct to be able to handle RPC requests to the network and manage the request IDs
  • Refactored existing types and functions to be more usable/compatible

In this PR I've setup the ChainSyncer so that the underlying database and channels can be replaced from using a live db and p2p network to an in memory db and local channels for testing or replaced by anything else generic in the future. I will wait to write the actual integrated tests until the full structure is done, but I setup a test to make it easier to set that up if someone else does this.

There is some stuff still TODO, but as discussed I would open this PR as interim to hopefully allow others to build ontop of

Reference issue to close (if applicable)

Closes #223
Closes #227

Other information and links

blockchain/chain/src/store/chain_store.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Show resolved Hide resolved

#[derive(Default)]
pub struct SyncManager {
sync_queue: SyncBucketSet,
}

impl SyncManager {
pub fn schedule_tipset(&mut self, tipset: Rc<Tipset>) {
pub fn schedule_tipset(&mut self, tipset: Arc<Tipset>) {
Copy link
Member

Choose a reason for hiding this comment

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

comments

Copy link
Member

Choose a reason for hiding this comment

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

same for the 2 other funcs below

use libp2p::core::PeerId;
use log::trace;

/// Context used in chain sync to handle sequential
Copy link
Member

Choose a reason for hiding this comment

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

sequential what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it fixed on my local wip changes lol

@austinabell austinabell requested a review from ec2 February 28, 2020 17:24
/// Returns FullTipset from store if TipSetKeys exist in key-value store otherwise requests FullTipset
/// from block sync
pub fn fetch_tipset(&self, _peer_id: PeerId, tsk: &TipSetKeys) -> Result<FullTipset, Error> {
let fts = match self.load_fts(tsk) {
Copy link
Member

Choose a reason for hiding this comment

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

cant you just use ? instead of match? Just a nit dun matter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not changed in this PR but the idea is leaving like this until the TODO is done (I commented the same thing on Dustin's PR)

@austinabell austinabell merged commit 8c92449 into master Feb 28, 2020
@austinabell austinabell deleted the austin/chain_sync branch February 28, 2020 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement parsing of blocksync response into more usable types Connect sync manager with Libp2p RPC service
3 participants