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

keep track of first block in blockchain #1937

Merged
merged 4 commits into from
Aug 18, 2016
Merged

keep track of first block in blockchain #1937

merged 4 commits into from
Aug 18, 2016

Conversation

rphmeier
Copy link
Contributor

Closes #1936

All blocks in the inclusive range between the first and best block are guaranteed to be available for now.

@rphmeier rphmeier added the A0-pleasereview 🤓 Pull request needs code review. label Aug 16, 2016
@coveralls
Copy link

coveralls commented Aug 16, 2016

Coverage Status

Coverage decreased (-0.07%) to 85.827% when pulling 31a6798 on first_block into b45020f on master.

@rphmeier
Copy link
Contributor Author

travis failure seems spurious (temp directory name collision?)


// binary search for the first block.
if raw_first.is_empty() {
let (mut f, mut hash) = (best_block_number, best_block_hash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it better than migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seemed like it was a little small of a change for a migration, but maybe it's worthwhile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, as this doesn't actually change the format of the database it would be strange to build a migration around it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 86.945% when pulling a22813e on first_block into d279794 on master.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 18, 2016
@gavofyork gavofyork added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Aug 18, 2016
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Aug 18, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 86.968% when pulling 1462f4b on first_block into 9adf775 on master.

@arkpar arkpar merged commit 0e0cc20 into master Aug 18, 2016
@arkpar arkpar deleted the first_block branch October 3, 2016 15:36
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.

5 participants