Unleash the power of Bitcoin Core into bdk-cli by rajarshimaitra · Pull Request #92 · bitcoindevkit/bdk-cli

rajarshimaitra · 2022-05-13T16:06:27Z

Description

fixes#62
fixes#76

This PR does the following

Bump bdk version to 0.19.
Opens up some basic bitcoin-core commands via a new sub-command bdk-cli node <command> [<args>]. The API of the node commands are kept very similar to bitcoin-cli api. This allows us to control the auto deployed backend node via regtest-* features from bdk-cli itself.
The Integration tests are written with 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.
These apis are also exposed in 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 this

$ ./target/debug/bdk-cli node --help
bdk-cli-node 0.5.0
Regtest Node mode
USAGE:
bdk-cli node <SUBCOMMAND>
FLAGS:
-h, --help Prints help information
-V, --version Prints version information
SUBCOMMANDS:
generate Generate blocks
getbalance Get Wallet balance
getinfo Get info
getnewaddress Get new address from node's test wallet
help Prints this message or the help of the given subcommand(s)
sendtoaddress Send to an external wallet address

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
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

notmandatory · 2022-05-16T00:55:45Z

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.

rajarshimaitra · 2022-05-17T10:54:46Z

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.

rajarshimaitra · 2022-06-14T13:00:34Z

Done some major refactoring.
Updated all the pending dependencies.
Updated to bdk v0.19.0
Broken down the commits into smaller chunks for easier review
Refactored the existing Backend struct
Backend now can be both bitcoind and electrum
Some minor update in the integration testing..

@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..

rajarshimaitra · 2022-06-15T08:36:31Z

Updated the PR doc..

The current test failure at cargo build --features reserves,electrum --locked fixes with https://github.com/weareseba/bdk-reserves/pull/5

rajarshimaitra · 2022-06-20T18:38:54Z

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:  ### 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  ### 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:  ### 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:  ### 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

rajarshimaitra · 2022-07-08T08:57:15Z

Closing this as we are moving ahead with #102

rajarshimaitra · 2022-05-13T16:06:27Z