-
Notifications
You must be signed in to change notification settings - Fork 83
feat: add redb as an alternative to sqlite #206
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not tested but I left a few comments.
I think it is a good idea to leave sqlite as the default db. Users can compile with no default features if they want to use redb. But when you open the PR, I will test it properly to have a better opinion.
In the meantime, a good job so far.
Pull Request Test Coverage Report for Build 17216612833Details
💛 - Coveralls |
Made sqlite a default feature again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, @110CodingP!
Your implementation looks great. I've left a few comments. Once you make the updates, I will test it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 26d7e8b
Weldone @110CodingP, I have tested and everything seems to work fine.
Kindly squash your commits into two or three at most, then I will approve.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK a5ecee6
Since the user does not need to know about how commands are being handled.
Rebased! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 83e6dd6
Will be handy having this in bdk-cli
for testing and as an example for how to use redb with bdk_wallet
.
Description
We currently only support
sqlite
persistence. This PR adds an alternativeredb
persistence by leveraging Summer Of Bitcoin work onbdk_redb
.Notes to the reviewers
The CI does not pass since
bdk_redb
has a MSRV of 1.85.0 which is in turn due toredb
. Alsobdk_redb
is yet to published as a crate so currently we use the GitHub version of the same.The following script tests a simple scenario of creating a wallet with bitcoind-rpc as chain source , sending funds to an address controlled by the wallet and creating a transaction using the wallet:
the conf is as follows:
Also removedsqlite
from default features since we now have an alternative.Changelog notice
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committingNew Features:
CHANGELOG.md