Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

fix issue with starting requested block number was not included itself #512

Merged
merged 2 commits into from Feb 24, 2016

Conversation

NikVolf
Copy link
Contributor

@NikVolf NikVolf commented Feb 24, 2016

@NikVolf NikVolf added the A0-pleasereview 🤓 Pull request needs code review. label Feb 24, 2016
}
let max_count = min(MAX_HEADERS_TO_SEND, max_headers);
let mut count = 0;
let mut data = Bytes::new();
let inc = (skip + 1) as BlockNumber;
while number <= last && number > 0 && count < max_count {
while number <= last && number >= 0 && count < max_count {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if number <= inc couple of lines below should be if number < inc if when reverse blocks are requested they should also include 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah writing a test thnx

@NikVolf NikVolf added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 24, 2016
@NikVolf NikVolf added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Feb 24, 2016
@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 24, 2016

@tomusdrw looks like test speaks that should be no such change
i think reverse request was buggy only for sequence ending with genesis

@arkpar
Copy link
Collaborator

arkpar commented Feb 24, 2016

Forward request was "buggy" only for genesis block as well. The original idea was that we should not return genesis block anyway. But if the spec demands it, so be it

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 24, 2016
@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 24, 2016

@arkpar right

NikVolf added a commit that referenced this pull request Feb 24, 2016
fix issue with starting requested block number was not included itself
@NikVolf NikVolf merged commit 53af2d7 into master Feb 24, 2016
@NikVolf NikVolf deleted the fix-block-inclusive branch February 25, 2016 00:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants