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

network: add IsIdle argument to the PeerFetchStatusReady fingerprint #1705

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

nfrisby
Copy link
Contributor

@nfrisby nfrisby commented Feb 26, 2020

Fixes #1147.

I've been using this commit during all of my recent development on the tests -- without it, Issue #1147 failures have occasionally masked my other focus.

I still don't have a repro on master, but I think I'll be able to find one after a couple more PRs are merged -- the #1147 failure has arisen during my development even without the simulated network latency that originally revealed it.

Disclaimer: though this commit seems well-vetted by the use in the not-yet-on-master tests mentioned above, I do not understand the BlockFetch code very well, and moreover it's been a while since I actually wrote this code (in particular, I'm not exactly sure what it is that the Boolean adds beyond checking if the adjacent set is empty). It'd be wonderful to see a localized test case for this.

@dcoutts
Copy link
Contributor

dcoutts commented Feb 26, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 26, 2020

@iohk-bors iohk-bors bot merged commit 36861a9 into master Feb 26, 2020
@iohk-bors iohk-bors bot deleted the nfrisby/issue-1147 branch February 26, 2020 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some BlockFetch decline decisions are not revisited after completeFetchBatch
2 participants