-
Notifications
You must be signed in to change notification settings - Fork 23
feat: example using hwi signing with #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
base: master
Are you sure you want to change the base?
Conversation
Thanks for this! looks like a good simple example for using |
So I think after constructing the psbt we can just call that function and voila. |
|
||
let _ = bb.register_wallet("test-wallet", descriptor).await.unwrap(); | ||
|
||
let bitbox_signer = HwiSigner::new(bb); |
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.
So at this level we can just call bibox_signer.sign(psbt)
given that the psbt has been constructed with bdk_wallet
or something else
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.
Yes I think this is the right approach. Even though it makes the example much simpler it's still useful to show the end-to-end flow of using async-hwi
to sign a psbt created with bdk_wallet::TxBuilder
(and eventually the bdk_tx::TxBuilder
).
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.
Is it a good idea to add syncing wallet code in the example?
If not what's a good way of mocking the wallet with some utxos to sign?
@notmandatory I also added a feature Please let me know what you think |
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.
Thanks for working on this one, I left a few comments and also would recommend squashing the commits into a single one and follow the https://www.conventionalcommits.org/en/v1.0.0/ (e.g chore(example): add example using `async-hwi`
.
[dependencies] | ||
bdk_wallet = { path = "../../wallet", features = ["file_store"] } | ||
tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros"] } | ||
bitbox-api = {version = "0.6.0", features = ["tokio", "simulator"]} |
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 probably can have this as an optional dependency with the simulator
feature.
[patch.crates-io] | ||
bitbox-api = {git = "https://github.com/sdmg15/bitbox-api-rs", branch = "0.6.0-simulator-patch", features = ["tokio", "simulator"]} |
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.
It looks like they already have this patched in the official release v0.8.0, we should be using that instead.
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.
Yes indeed I was the one that requested the fix 😉 . But it's not needed anymore. The initial way I understood what we wanted was not right.
|
||
## Build and run | ||
|
||
`$ cargo run ` |
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.
`$ cargo run ` | |
`$ cargo run --bin example_wallet_hwi_signer` |
|
||
./bitbox02-multi-v9.19.0-simulator1.0.0-linux-amd64 | ||
|
||
cargo run --features=simulator |
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.
cargo run --features=simulator | |
cargo run --bin example_wallet_hwi_signer --features=simulator |
tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros"] } | ||
bitbox-api = {version = "0.6.0", features = ["tokio", "simulator"]} | ||
|
||
async-hwi = "0.0.27" |
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.
As the example is focused on a BitBox device, we probably should also explicitly use only the bitbox
feature.
Thanks for taking a look @oleonardolima. I've addressed your suggestions. |
Description
Integrates a new BDK signer using async-hwi.
The included example uses Bitbox02 but can be easily extended to use any other supported device.
Notes for reviewer
It's related to issue #20
Changelog notice
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committing