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.

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

Jump to bottom

Jaden/chainsync/asyncverification #419

Merged
merged 16 commits into from
May 20, 2020
Merged

Jaden/chainsync/asyncverification #419

merged 16 commits into from
May 20, 2020

Conversation

flodesi
Copy link
Contributor

@flodesi flodesi commented May 16, 2020

Summary of changes
Changes introduced in this pull request:

  • Added async to compute heavy checks in ChainSyncer's validate method
  • Created a FuturesUnordered, as well as implemented spawn_blocking for the async checks
  • Modified error handling in ChainSyncer's validate function to create a more rich error

Reference issue to close (if applicable)

#410

timvermeulen
timvermeulen reviewed May 16, 2020
View reviewed changes
blockchain/state_manager/src/lib.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
@flodesi flodesi marked this pull request as ready for review May 16, 2020 22:16
@flodesi flodesi requested a review from timvermeulen May 16, 2020 22:16
austinabell
austinabell reviewed May 16, 2020
View reviewed changes
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/state_manager/src/lib.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
@flodesi flodesi requested a review from austinabell May 19, 2020 15:46
austinabell
austinabell reviewed May 19, 2020
View reviewed changes
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
@flodesi flodesi requested a review from austinabell May 19, 2020 17:21
austinabell
austinabell reviewed May 19, 2020
View reviewed changes
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
blockchain/chain_sync/src/sync.rs Outdated Show resolved Hide resolved
@flodesi flodesi requested a review from austinabell May 19, 2020 20:04
austinabell
austinabell approved these changes May 20, 2020
View reviewed changes
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

Would be nice to have tests for this in the future, but looks good!

@flodesi flodesi merged commit b80ab49 into master May 20, 2020
@flodesi flodesi deleted the jaden/chainsync/asyncverification branch May 20, 2020 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinabell austinabell austinabell approved these changes

@ec2 ec2 Awaiting requested review from ec2

@ansermino ansermino Awaiting requested review from ansermino

@RajarupanSampanthan RajarupanSampanthan Awaiting requested review from RajarupanSampanthan

@StaticallyTypedAnxiety StaticallyTypedAnxiety Awaiting requested review from StaticallyTypedAnxiety

@dutterbutter dutterbutter Awaiting requested review from dutterbutter

@timvermeulen timvermeulen Awaiting requested review from timvermeulen

Assignees
No one assigned
Labels
Type: Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@flodesi @timvermeulen @austinabell