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

Fix leveldb corruption bug in asutoshpalai's get_ordered_blocks() fork #28

Merged
merged 14 commits into from
Feb 26, 2018

Conversation

brannondorsey
Copy link
Contributor

This PR is based on @asutoshpalai's #17. I've swapped the leveldb dependency with plyvel==1.0.4 and fixed the .bitcoin/blocks/index corruption issue discussed in his PR. It would seem that Bitcoin Core doesn't use Snappy compression, so it must be disabled explicitly when connecting to the database with plyvel. I've tested these fixes with bitcoind and these changes are compatible with their leveldb index (you don't have to re-index every time using this PR).

I've also added the ability to iterate through the blocks in reverse order by specifying a start parameter that is greater than the end parameter in blockchain.get_ordered_blocks(...).

I've tested all of the blockchain.get_ordered_blocks(...) PRs on this repo, and @asutoshpalai's is hands down my favorite. Using bitcoind's own leveldb database is an elegant solution to the ordering problem 🥇. This feature is very useful and I vote for merging it.

@brannondorsey
Copy link
Contributor Author

CI tests are failing because of the new dependency on LevelDB.

  plyvel/_plyvel.cpp:547:24: fatal error: leveldb/db.h: No such file or directory
   #include "leveldb/db.h"

I don't have any experience with Travis, but I'm pretty sure installing LevelDB would fix the problem. That does mean adding LevelDB as a dependency to this project, but IMHO that is a worthwhile dependency addition for this powerful new feature.

@alecalve
Copy link
Owner

Looking at plyvel's docs, it seems that the pip version assumes the correct version of leveldb is installed, which is not the case for most Linux :

Plyvel 1.x depends on LevelDB >= 1.20, which at the time of writing (early 2018) is more recent than the versions packaged by various Linux distributions. Using an older version will result in compile-time errors. The easiest solution is to use the pre-built binary packages.

We should update the README to mention this.

@alecalve
Copy link
Owner

@brannondorsey I've just made a PR (brangerbriz#1) to your repo that should make this one mergeable 🤞

@brannondorsey
Copy link
Contributor Author

Plyvel 1.x depends on LevelDB >= 1.20, which at the time of writing (early 2018) is more recent than the versions packaged by various Linux distributions.

Strange, I've got Leveldb 1.18-5 installed (via apt-get) and am not running into errors with Python 3.5. That could explain why the 3.4 and 3.5 Travis tests were passing though.

I saw that you added a travis.sh file which downloads the leveldb 1.20 binaries. Should we update the documentation in the README to reflect this is necessary if you are running with Python 3.3?

@alecalve
Copy link
Owner

The travis.sh file is only there for Travis, end users just have to install libleveldb-dev 1.2.0 whichever way they want (I'd recommend using apt since it's easier)

@brannondorsey
Copy link
Contributor Author

Oops, sorry, I meant document that it is necessary to install libleveldb-dev version 1.2.0, not run travis.sh. I agree that we should leave it up to them as to how to do that. Definitely finding it strange that Python 3.5 seems to work with leveldb-dev 1.18, but 🤷‍♂️. I'd say we are ready to merge. Thanks for making those changes 👍.

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.

None yet

3 participants