Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move bdk_hwi to hwi module #1516

Closed
notmandatory opened this issue Jul 18, 2024 · 2 comments · Fixed by #1561
Closed

Move bdk_hwi to hwi module #1516

notmandatory opened this issue Jul 18, 2024 · 2 comments · Fixed by #1561
Assignees
@oleonardolima
Labels
new feature New feature or request
Milestone
1.0.0-beta

Comments

@notmandatory
Copy link
Member

notmandatory commented Jul 18, 2024 •
edited
Loading

Describe the enhancement

We should move bdk_hwi from the bdk repo crates and include it with to the hwi crate.

Use case

Removing bdk_hwi will simplify the bdk workspace and ci testing because we won't need to use pyo3.

Additional context

If we determine no one is using the hwi module we can more easily deprecate both hwi and bdk_hwi when they're together in the same crate. But also they will be together in case someone does want to use and maintain them.
@notmandatory notmandatory added the new feature New feature or request label Jul 18, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Jul 18, 2024
@oleonardolima
Copy link
Contributor

I can take this cleanup if nobody has already.

@oleonardolima
Copy link
Contributor

For future reference: https://github.com/wizardsardine/async-hwi.

If it becomes the standard for the ecosystem, or maybe a new one from rust-bitcoin, we can get deprecate and get rid of rust-hwi without much problem, referring users towards async-hwi.

This was referenced Aug 16, 2024
Merged
Merged
notmandatory added a commit to bitcoindevkit/rust-hwi that referenced this issue Sep 11, 2024
@notmandatory
Merge #104: feat: add HWISigner, moved from bdk_hwi
4004b0a 
7747b3f refactor(signer)!: add `signer` behind a non-default feature (Leonardo Lima)
b25168a feat: add `HWISigner`, moved from `bdk_hwi` (Leonardo Lima)

Pull request description:

  partially addresses bitcoindevkit/bdk#1516

  ## Description

  - adds a new `signer.rs` that contains the previous implementation of `HWISigner`, which implements `bdk_wallet::signer::{SignerCommon, TransactionSigner}` traits.
  - expose the new `signer::HWISigner` as public.
  - updates the crate documentation.
  - TODO: re-add test that relies on `bdk_wallet::tests::common::get_funded_wallet` helper methods.

  ## Notes for Reviewers

  I'm unsure if the documentation covers everything needed, please let me know if I'm missing something.

ACKs for top commit:
  notmandatory:
    ACK 7747b3f

Tree-SHA512: 8be7d681454eba31b5495dcf6cb9b399e2d992760f92f4ffc1d553908872b1272bb2e792ca608b679caf95b66126846bbb4dff37eb6f067a8e28d14454554098
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oleonardolima oleonardolima

Labels
new feature New feature or request
Projects
BDK
Archived in project
Milestone
1.0.0-beta
Development

Successfully merging a pull request may close this issue.

refactor(bdk_hwi): remove bdk_hwi, as HWISigner's being moved to rust-hwi oleonardolima/bdk
2 participants
@notmandatory @oleonardolima