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

fix: try traverse all unknown parent hash #2906

Merged
merged 2 commits into from
Aug 9, 2021
Merged

fix: try traverse all unknown parent hash #2906

merged 2 commits into from
Aug 9, 2021

Conversation

driftluo
Copy link
Collaborator

@driftluo driftluo commented Aug 4, 2021 •
edited
Loading

Try traverse all unknown parent hash

@driftluo driftluo requested a review from a team as a code owner August 4, 2021 05:18
@keroro520 keroro520 mentioned this pull request Aug 4, 2021
Closed
@driftluo driftluo changed the title fix: Fix fetch on received fix: try traverse all known unknow parent hash Aug 4, 2021
@driftluo driftluo changed the title fix: try traverse all known unknow parent hash fix: try traverse all known unknown parent hash Aug 4, 2021
@driftluo driftluo changed the title fix: try traverse all known unknown parent hash fix: try traverse all unknown parent hash Aug 4, 2021
quake
quake reviewed Aug 4, 2021
View reviewed changes
sync/src/relayer/mod.rs Outdated Show resolved Hide resolved
@driftluo driftluo force-pushed the fix-fetch-on-received branch 2 times, most recently from c38b40d to 03b5690 Compare August 4, 2021 09:08
@driftluo driftluo added the s:waiting-on-reviewers Status: Waiting for Review label Aug 4, 2021
@driftluo driftluo force-pushed the fix-fetch-on-received branch 4 times, most recently from deafee3 to 41b575a Compare August 5, 2021 03:34
@driftluo driftluo force-pushed the fix-fetch-on-received branch 2 times, most recently from a66a9a0 to 29bcfc1 Compare August 6, 2021 08:54
quake
quake previously approved these changes Aug 6, 2021
View reviewed changes
@doitian doitian removed the request for review from keroro520 August 6, 2021 12:34
doitian
doitian requested changes Aug 6, 2021
View reviewed changes
Copy link
Member

@doitian doitian left a comment

Choose a reason for hiding this comment

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

I'll review it tomorrow.

@driftluo
Copy link
Collaborator Author

driftluo commented Aug 7, 2021

I change self.leaders.retain(|key| !self.parents.contain(key)) to self.leaders.remove(&hash), it will reduce the consumption of traversal and hash，please review again @quake

@driftluo driftluo requested a review from quake August 7, 2021 00:04
doitian
doitian previously approved these changes Aug 7, 2021
View reviewed changes
Copy link
Member

@doitian doitian left a comment

Choose a reason for hiding this comment

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

Some minor suggestions which should not block the merge.

sync/src/orphan_block_pool.rs Show resolved Hide resolved
sync/src/orphan_block_pool.rs Show resolved Hide resolved
sync/src/orphan_block_pool.rs Show resolved Hide resolved
sync/src/orphan_block_pool.rs Show resolved Hide resolved
sync/src/orphan_block_pool.rs Outdated Show resolved Hide resolved
sync/src/orphan_block_pool.rs Outdated Show resolved Hide resolved
sync/src/orphan_block_pool.rs Outdated Show resolved Hide resolved
sync/src/orphan_block_pool.rs Outdated Show resolved Hide resolved
sync/src/orphan_block_pool.rs Outdated Show resolved Hide resolved
@driftluo driftluo requested a review from doitian August 7, 2021 09:26
@doitian
Copy link
Member

doitian commented Aug 7, 2021

@zhangsoledad @quake please review

@driftluo
Copy link
Collaborator Author

driftluo commented Aug 9, 2021

bors r=doitian,quake

bors bot added a commit that referenced this pull request Aug 9, 2021
@bors @keroro520 @driftluo
Merge #2906
fec8684 
2906: fix: try traverse all unknown parent hash r=doitian,quake a=driftluo

Try traverse all unknown parent hash

Co-authored-by: keroro <[email protected]>
Co-authored-by: driftluo <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 9, 2021

Build failed:

zhangsoledad
zhangsoledad reviewed Aug 9, 2021
View reviewed changes
sync/src/orphan_block_pool.rs
self.len() == 0
}

pub fn clone_leaders(&self) -> Vec<ParentHash> {
Copy link
Member

Choose a reason for hiding this comment

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

Could use impl trait return an Iterator here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but #2906 (comment), so I deleted it

@doitian
Copy link
Member

doitian commented Aug 9, 2021

Blocked by #2913

@driftluo
Copy link
Collaborator Author

driftluo commented Aug 9, 2021

bors r=doitian,quake

@bors
Copy link
Contributor

bors bot commented Aug 9, 2021

Build succeeded:

@bors bors bot merged commit 02274fe into nervosnetwork:develop Aug 9, 2021
@driftluo driftluo deleted the fix-fetch-on-received branch August 9, 2021 05:34
@driftluo driftluo removed the s:waiting-on-reviewers Status: Waiting for Review label Aug 9, 2021
@doitian doitian mentioned this pull request Aug 10, 2021
Closed
3 tasks
@doitian doitian mentioned this pull request Sep 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhangsoledad zhangsoledad zhangsoledad left review comments

@doitian doitian doitian approved these changes

@quake quake quake approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@driftluo @doitian @quake @zhangsoledad @keroro520