-
Notifications
You must be signed in to change notification settings - Fork 65
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
Unleash the power of Bitcoin Core into bdk-cli #92
Unleash the power of Bitcoin Core into bdk-cli #92
Conversation
Hey @rajarshimaitra concept ACK! but I need to focus on the next bdk release (taproot!) so won't be able to do a through review until that's out. |
31c78a0
to
a16d101
Compare
Ya thanks @notmandatory no issues.. I opened this early for discussion.. This can wait till other major things are done.. |
- Update BDK to v0.19 - update electrsd to v0.19 to get latest required upstream changes Note that `regtest-esplora-*` features aren't working as of now. Some issues in `elctrsd/esplora` feature. Thus removed from CI tests.
Add a separate subcommand list for for node operation related commands. Right now they only include basic operations, but can be extended later as per need. Add a Backend struct that will hold the running bitcoind or electrsd process in the background. The electrsd and bitcoind will be connected together. And the wallet will connect to the backend electrsd via electrum blockchain config. So in case of `regtest-electrum` too we will operate the backend via rpc to the underlying core node.
Update the lib tests to accommodate changes of the previous commit.
The database creation functions are broken up and chained together to create the right database directory for the right context and not reuse code.
Update the Backend handling logic for new_blockchain() function. The Backend doesn't contain connection data anymore but the full bitcoind and electrsd instance. The Backend struct definition is moved into lib.rs.
Update the Backend handling logic in main() to the new Backend struct.
Update the handle_command() function to handle node command that operates on the Backend. Add node commands in REPL mode too to get regtest-* features available in repl.
This is a simple test framework to using std::Cmd to test custom built bdk-cli with `regtest-*` feature to quickly simulate integration testing of any kind of situation involving one/many bdk wallets, and one bitcoind or electrsd process on the background. All the bdk-cli command line commands can be used in this framework to operate all kind of tests. The `bdk-cli wallet <cmd>` and `bdk-cli node <cmd>` makes bdk-cli the complete integration testing environment itself. Each tests can be manually played in the `bdk-cli repl` mode too.
a16d101
to
fda07ac
Compare
@notmandatory this PR is now ready for review.. Edit: The tests failure is due to version conflicts in bdk ocuring from bdk-reserves.. Working on a fix for that.. The PR can be code reviewed in the mean time.. |
Updated the PR doc.. The current test failure at |
Opened an alternate version of the same PR in #102. Not closing this one yet, in case we need to revert back to previous crate structure.. |
5b20283 update CI to remove some features (rajarshimaitra) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description Currently the way `ExternalReserves` functionality is written, it can only be used with `electrum` feature. The tests should fail if the implementation behavior is enforced.. Disabling the tests in CI. Also removing the `regtest-esplora` features from the tests, because they won't work when #92 lands. ### Notes to the reviewers <!-- In this section you can include notes directed to the reviewers, like explaining why some parts of the PR were done in a specific way --> ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing ACKs for top commit: notmandatory: ACK 5b20283. Tree-SHA512: 09e875376e2a8b1a43c318b66849abdeb518deee32ca39ba1e10c9b9230b4af61a3391e5a1dcad0586643a820eb66fabfea9d2af95f0d97fe8383524c5e050d9
292dd1e Fix repl mode command parsing (Steve Myers) 073f1c3 Update with review comments (rajarshimaitra) 4e8f830 revert author list change (rajarshimaitra) b09c405 Remove base64 dependency (rajarshimaitra) 1e70ff9 Refactor everything (rajarshimaitra) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description This is a massive refactoring PR that changes the whole structure of the crate. Previously it was written like a library to be used to create the bdk-cli app. But eventually the crate itself became the app. This PR attempts to remove the remaining lib like patterns in the code, and make it a pure binary crate. This makes the code more modular and makes it look like a typical binary rust crate. There was no real good way to structure the change into separate commits, so I made one single big one.. The best way to review is to look at the final structure of the code itself, not the change set. The crate has following modules now - `main` : The main app runtime - `commands`: Includes all the structopt commands used by bdk-cli. - `handlers`: Include all the command handlers used buy the app. - `utils`: Include all the utility and helper functions - `Backend` : Defines the backend node process, and its related methods. (This will be filled more with #92). Apart from the structure changes there are few other changes that took place - Almost all of the previous doc comments are removed. As they were written to use bdk-cli as a lib. Instead new structopts "comments" are added to describe the app functionality better. As a result the app `--help` commands are more elaborate and descriptive now. I have also removed few redundant description messages used before, that would mess up the help comments. And as a by product it solves #93. - bdk is updated to v0.19.0 - bdk-reserves is updated with current version pointing to bdk v0.19.0. - Default database is now sqlite. Overall I think I managed not to break anything. Currently this change will remove most of the previous documentation on the crate. But those aren't useful to context of bdk-cli after this change.. My proposal would be reproduce the README instructions itself in doc.rs landing page. We also need to update the README to reflect these changes.. I will open that up in a separate PR. I also haven't updated changelog yet.. Not sure yet how to describe the change in short.. Will do that once this is almost finalized.. ### Notes to the reviewers ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [ ] I've added tests for the new feature * [ ] I've added docs for the new feature * [ ] I've updated `CHANGELOG.md` ACKs for top commit: notmandatory: ACK 292dd1e Tree-SHA512: 895d8088bf93a481fd776e2ac5fe85926f13b7b4535f17b9edd3c0363a89dc3689e28c6e13dbcac3970bc00e3ff206f402e94406f3b3688c9e4a7f9d31b20e40
292dd1e Fix repl mode command parsing (Steve Myers) 073f1c3 Update with review comments (rajarshimaitra) 4e8f830 revert author list change (rajarshimaitra) b09c405 Remove base64 dependency (rajarshimaitra) 1e70ff9 Refactor everything (rajarshimaitra) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description This is a massive refactoring PR that changes the whole structure of the crate. Previously it was written like a library to be used to create the bdk-cli app. But eventually the crate itself became the app. This PR attempts to remove the remaining lib like patterns in the code, and make it a pure binary crate. This makes the code more modular and makes it look like a typical binary rust crate. There was no real good way to structure the change into separate commits, so I made one single big one.. The best way to review is to look at the final structure of the code itself, not the change set. The crate has following modules now - `main` : The main app runtime - `commands`: Includes all the structopt commands used by bdk-cli. - `handlers`: Include all the command handlers used buy the app. - `utils`: Include all the utility and helper functions - `Backend` : Defines the backend node process, and its related methods. (This will be filled more with bitcoindevkit#92). Apart from the structure changes there are few other changes that took place - Almost all of the previous doc comments are removed. As they were written to use bdk-cli as a lib. Instead new structopts "comments" are added to describe the app functionality better. As a result the app `--help` commands are more elaborate and descriptive now. I have also removed few redundant description messages used before, that would mess up the help comments. And as a by product it solves bitcoindevkit#93. - bdk is updated to v0.19.0 - bdk-reserves is updated with current version pointing to bdk v0.19.0. - Default database is now sqlite. Overall I think I managed not to break anything. Currently this change will remove most of the previous documentation on the crate. But those aren't useful to context of bdk-cli after this change.. My proposal would be reproduce the README instructions itself in doc.rs landing page. We also need to update the README to reflect these changes.. I will open that up in a separate PR. I also haven't updated changelog yet.. Not sure yet how to describe the change in short.. Will do that once this is almost finalized.. ### Notes to the reviewers ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [ ] I've added tests for the new feature * [ ] I've added docs for the new feature * [ ] I've updated `CHANGELOG.md` ACKs for top commit: notmandatory: ACK 292dd1e Tree-SHA512: 895d8088bf93a481fd776e2ac5fe85926f13b7b4535f17b9edd3c0363a89dc3689e28c6e13dbcac3970bc00e3ff206f402e94406f3b3688c9e4a7f9d31b20e40
Closing this as we are moving ahead with #102 |
Description
fixes #62
fixes #76
This PR does the following
bdk-cli node <command> [<args>]
. The API of thenode
commands are kept very similar tobitcoin-cli
api. This allows us to control the auto deployed backend node viaregtest-*
features from bdk-cli itself.std::Command
from rust. That can be used to simulate various wallet transaction situations with bdk-cli, like Test watch-only LND wallet and signing PSBT #87.repl
mode so now bdk-cli can have real time communication between a backend and a wallet in repl shell itself. Which can be very useful for quick runs of different testing conditions.Notes to the reviewers
@sandipndev @krtk6160. This is the PR you guys can start working on top of to simulate the intended test situations. At least with
bitcoind
it can be done with all existing toolings. For LND some other wrapper needes to be built.@notmandatory let me know what you think about the whole framework.
Also looking for more integration test ideas too add into.
basic
node
usage looks like thisChecklists
All Submissions:
cargo fmt
andcargo clippy
before committingNew Features:
CHANGELOG.md