-
Notifications
You must be signed in to change notification settings - Fork 374
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
Add code example for each supported backend #526
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.
Concept ACK on adding more example to the project. But I think its possible without adding a new cargo project in the workspace.
Can't we just have running examples like this #519?
Hi, please rebase to pickup changes in #596. Thanks! |
@notmandatory sorry for the delay. I force pushed a new version that adds examples just for Electrum and Esplora, as there are already examples for Compact Filter and RPC backends. I also updated the code to 0.20.1 and addressed @rajarshimaitra's suggestion not to add a new cargo project. |
The new push added the documentation suggested in #685, explaining how to use |
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.
Concept ACK.. Few more fixes needs to be done..
8f1ac3d
to
400cbad
Compare
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.
Code looks good.. It need few more fixes to work.. Sorry if I reviewed prematurely.
a9c426e
to
95e9b7f
Compare
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 95e9b7f
Code loos good.. Thanks for the quick update.. One last comment..
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.
ACK f13d5e6
One last fix to pass the tests..
07f6747
to
c7e1247
Compare
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 c7e1247
LGTM.. One suggestion maybe for a future PR.
examples/esplora_backend.rs
Outdated
/// This can be run with `cargo run --features="use-esplora-reqwest, reqwest-default-tls" --example esplora_backend` | ||
/// in the root folder. | ||
/// | ||
/// Note: The configuration above uses asynchronous HTTP calls. |
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.
Sorry I have missed this before. Nothing major, feel free to ignore for this PR.
Even though the calls are async in reqwest
they are still blocking in our impl, using the await_or_block
macro like this,
bdk/src/blockchain/esplora/reqwest.rs
Line 126 in 06310f1
Ok(await_or_block!(self.url_client._get_tx(txid))?) |
So to actually make the wallet calls async in the example you need to use the async-interface
feature too.. In that case you will have await
in your wallets calls in the example..
This PR is good to go on its own.. Maybe as a future PR an idea.
- Transform this to a blocking example using
ureq
. - Create another fully
async
example usingreqwest
.
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.
That would be great. But, as mentioned above, the command cargo run --features="use-esplora-reqwest, reqwest-default-tls, async-interface" --example esplora_backend
results in error.
I think the problem is here (src/lib:210
)
#[cfg(all(feature = "async-interface", feature = "electrum"))]
compile_error!(
"Features async-interface and electrum are mutually exclusive and cannot be enabled together"
);
Even there is no electrum
in the features, it seems this is enabled by default. Is there a way to disable 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.
You can disable the default features (which are "key-value-db", "electrum") by adding --no-default-features
, eg:
cargo run --no-default-features --features=use-esplora-reqwest,reqwest-default-tls,async-interface --example esplora_backend
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.
@notmandatory this worked. Thanks.
@rajarshimaitra I created two files examples/esplora_backend_synchronous.rs
and examples/esplora_backend_asynchronous.rs
in a2b6a40.
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 don't know why CI / Rust fmt (pull_request)
failed. I ran cargo fmt --all
before committing.
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.
ACK a2b6a40
Thanks for the split.. This now looks complete.. Few more nits..
It was some temporary problem with rust update server.. I restarted the tests and it passed.. |
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 e50fcb6
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.
Sorry took me some time to get back to this.. Thanks for pushing this through..
tACK f99a6b9
All looks good to me, and great work separating the blocking and async example separate. This is good to go, no more changes should be required..
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.
if [ "$PWD" = "$HOME" ]; then
This PR adds code example for connecting to Esplora, Electrum Server, Neutrino and Bitcoin Core.
Also shows how to retrieve balance, sign and broadcast transactions.
To test: