Skip to content

Adds bitcoin-rpc-client example #1232

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

Closed
wants to merge 5 commits into from

Conversation

ConorOkus
Copy link

@ConorOkus ConorOkus commented Jan 10, 2022

Small demo showing how to interact with bitcoin core as a backend to a wallet.

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2022

Codecov Report

Merging #1232 (8ce40d5) into main (482a2b9) will decrease coverage by 0.00%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1232      +/-   ##
==========================================
- Coverage   90.41%   90.40%   -0.01%     
==========================================
  Files          71       71              
  Lines       38359    38359              
==========================================
- Hits        34683    34680       -3     
- Misses       3676     3679       +3     
Impacted Files Coverage Δ
lightning-block-sync/src/rpc.rs 78.51% <33.33%> (ø)
lightning/src/ln/functional_tests.rs 97.20% <0.00%> (-0.07%) ⬇️
lightning-invoice/src/de.rs 81.27% <0.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 482a2b9...8ce40d5. Read the comment docs.

@ConorOkus ConorOkus force-pushed the examples branch 3 times, most recently from e35827d to 5b5e2a0 Compare January 11, 2022 23:14
@ConorOkus ConorOkus force-pushed the examples branch 12 times, most recently from b6d183c to 6990a67 Compare January 19, 2022 10:23
Copy link
Member

@devrandom devrandom left a comment

Choose a reason for hiding this comment

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

first pass...

.call_method::<BlockchainInfoResponse>("getblockchaininfo", &vec![])
.await
.map_err(|_| {
std::io::Error::new(std::io::ErrorKind::PermissionDenied,
Copy link
Member

Choose a reason for hiding this comment

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

is this really the only possible way the call can fail?

Copy link
Author

Choose a reason for hiding this comment

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

I'm only ever able to generate a error code: -1

impl TryInto<GetBalanceResponse> for JsonResponse {
type Error = std::io::Error;
fn try_into(self) -> std::io::Result<GetBalanceResponse> {
Ok(GetBalanceResponse(self.0.as_f64().unwrap() as usize))
Copy link
Member

Choose a reason for hiding this comment

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

doesn't getbalance return a fractional number of bitcoin? does this need to be multiplied by 100_000_000?

Copy link
Author

Choose a reason for hiding this comment

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

Updated to use the Amount type

@ConorOkus
Copy link
Author

@Ramshandilya @devrandom would appreciate a seconds pass, please.

@Ramshandilya
Copy link

Hi Conor, left a comment -> #1232 (comment)
Looks good to go otherwise.

Conor Okus and others added 4 commits January 27, 2022 16:47
Update edition value

Add missing dependency

Downgrade Tokio version

Trigger build

Create seperate workspace for examples

Updates Cargo.toml

Build examples in ci

Fix yml formatting

Remove test script

Update tokio version

Make dependencies same as sample

Add missing dependency

Update script for windows

Update bash script

Updates shell script

Adds comment

Fix linting errors

Trigger Build
Co-authored-by: Ram Shandilya <[email protected]>
@TheBlueMatt
Copy link
Collaborator

I'm still a bit unclear on what we're demonstrating here, and if we can really maintain this. A different direction we could go here is to try to tackle rust-bitcoin/rust-bitcoincore-rpc#166 and then we get a super maintainable RPC client with async support there. Sergi even started that work at rust-bitcoin/rust-bitcoincore-rpc#201 though it needs rebase and need more review.

I don't think we really ever wanted our RPC client to be a general one for users to use for anything, the fact that it is is really just because we wanted something async and didn't have a lot of time. If we can make it the backing HTTP for rust-bitcoincore-rpc everyone seems much better off.

@jkczyz
Copy link
Contributor

jkczyz commented Feb 2, 2022

I'm still a bit unclear on what we're demonstrating here, and if we can really maintain this. A different direction we could go here is to try to tackle rust-bitcoin/rust-bitcoincore-rpc#166 and then we get a super maintainable RPC client with async support there. Sergi even started that work at rust-bitcoin/rust-bitcoincore-rpc#201 though it needs rebase and need more review.

I don't think we really ever wanted our RPC client to be a general one for users to use for anything, the fact that it is is really just because we wanted something async and didn't have a lot of time. If we can make it the backing HTTP for rust-bitcoincore-rpc everyone seems much better off.

I think at a higher level, we were aiming for examples of using LDK that are a little more bite-sized than the sample. Potentially, the examples could be dependencies for the sample node if they were to be designed as a library along with a simple binary exercising the example.

That said, there really isn't any interaction with LDK here as you noted and as removing the BlockSource implementation indicates. I imagine this example could be expanded to include that and also have it used with an SpvClient hooked up to a trivial chain::Listener, which simply prints when blocks are connected / disconnected.

@TheBlueMatt
Copy link
Collaborator

I think at a higher level, we were aiming for examples of using LDK that are a little more bite-sized than the sample. Potentially, the examples could be dependencies for the sample node if they were to be designed as a library along with a simple binary exercising the example.

Yep, got that, big fan of that idea.

That said, there really isn't any interaction with LDK here as you noted and as removing the BlockSource implementation indicates. I imagine this example could be expanded to include that and also have it used with an SpvClient hooked up to a trivial chain::Listener, which simply prints when blocks are connected / disconnected.

That makes sense, I guess my point here is that this doesn't seem to demonstrate a part of LDK that we're "proud" of - it really just demonstrates that you could use it as an RPC client. We could instead focus on the part where you hook it up as a blocksource and demonstrate getting blocks.

@jkczyz
Copy link
Contributor

jkczyz commented Feb 2, 2022

That makes sense, I guess my point here is that this doesn't seem to demonstrate a part of LDK that we're "proud" of - it really just demonstrates that you could use it as an RPC client. We could instead focus on the part where you hook it up as a blocksource and demonstrate getting blocks.

Yup, agreed! And to be fair, @ConorOkus put this together before we prepped for the livestream, where we focused more on the interaction with SpvClient and chain::Listen. Probably spent a little too much time setting up the BlockSource then, but we were able to get it all connected with SpvClient by the end.

Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

Just a few nits I can see from a quick review.

@@ -0,0 +1,33 @@
# Wallet actions with an RPC client

This is example shows how you could create a client that directly communicates with bitcoind from LDK. The API is flexible and allows for different ways to implement the interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This is example shows how you could create a client that directly communicates with bitcoind from LDK. The API is flexible and allows for different ways to implement the interface.
This example shows how you could create a client that directly communicates with bitcoind from LDK. The API is flexible and allows for different ways to implement the interface.


This is example shows how you could create a client that directly communicates with bitcoind from LDK. The API is flexible and allows for different ways to implement the interface.

It implements some basic RPC methods that allow you to create a wallet and print it's balance to stdout.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It implements some basic RPC methods that allow you to create a wallet and print it's balance to stdout.
It implements some basic RPC methods that allow you to create a wallet and print its balance to stdout.


It implements some basic RPC methods that allow you to create a wallet and print it's balance to stdout.

To run with this example you need to have a bitcoin core node running in regtest mode. Get the bitcoin core binary either from the [bitcoin core repo](https://bitcoincore.org/bin/bitcoin-core-0.22.0/) or [build from source](https://github.com./bitcoin/bitcoin/blob/v0.21.1/doc/build-unix.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To run with this example you need to have a bitcoin core node running in regtest mode. Get the bitcoin core binary either from the [bitcoin core repo](https://bitcoincore.org/bin/bitcoin-core-0.22.0/) or [build from source](https://github.com./bitcoin/bitcoin/blob/v0.21.1/doc/build-unix.md).
To run this example you need to have a bitcoin core node running in regtest mode. Get the bitcoin core binary either from the [bitcoin core repo](https://bitcoincore.org/bin/bitcoin-core-0.22.0/) or [build from source](https://github.com./bitcoin/bitcoin/blob/v0.21.1/doc/build-unix.md).

Comment on lines +43 to +83
pub fn get_new_rpc_client(&self) -> io::Result<RpcClient> {
let http_endpoint = HttpEndpoint::for_host(self.host.clone()).with_port(self.port);
let rpc_credentials =
base64::encode(format!("{}:{}", self.rpc_user.clone(), self.rpc_password.clone()));
RpcClient::new(&rpc_credentials, http_endpoint)
}

pub async fn get_blockchain_info(&self) -> BlockchainInfoResponse {
let mut rpc = self.bitcoind_rpc_client.lock().await;
rpc.call_method::<BlockchainInfoResponse>("getblockchaininfo", &vec![]).await.unwrap()
}

pub async fn create_wallet(&self) -> CreateWalletResponse {
let mut rpc = self.bitcoind_rpc_client.lock().await;
let create_wallet_args = vec![json!("test-wallet")];

rpc.call_method::<CreateWalletResponse>("createwallet", &create_wallet_args).await.unwrap()
}

pub async fn get_new_address(&self) -> String {
let mut rpc = self.bitcoind_rpc_client.lock().await;

let addr_args = vec![json!("LDK output address")];
let addr = rpc.call_method::<NewAddressResponse>("getnewaddress", &addr_args).await.unwrap();
addr.0.to_string()
}

pub async fn get_balance(&self) -> GetBalanceResponse {
let mut rpc = self.bitcoind_rpc_client.lock().await;

rpc.call_method::<GetBalanceResponse>("getbalance", &vec![]).await.unwrap()
}

pub async fn generate_to_address(&self, block_num: u64, address: &str) -> GenerateToAddressResponse {
let mut rpc = self.bitcoind_rpc_client.lock().await;

let generate_to_address_args = vec![json!(block_num), json!(address)];


rpc.call_method::<GenerateToAddressResponse>("generatetoaddress", &generate_to_address_args).await.unwrap()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Indentation of function bodies is a bit off in some places here.

@ConorOkus
Copy link
Author

Thanks for the feedback on this, I think I'm going to close in favour of something that shows more specific LDK functionality. I might create an issue to create a small demo for using the interaction with SpvClient and chain::Listen. I'm also focusing on an Android sample wallet using the Kotlin bindings if anyone is interested.

@ConorOkus ConorOkus closed this Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants