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 hardware signer into its own crate #872

Closed
LLFourn opened this issue Feb 21, 2023 · 1 comment · Fixed by #1161
Closed

Move hardware signer into its own crate #872

LLFourn opened this issue Feb 21, 2023 · 1 comment · Fixed by #1161
Assignees
@danielabrozzoni
Labels
api A breaking API change
Milestone
1.0.0-alpha

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Feb 21, 2023

It has its own very bespoke python runtime dependency and needs hardware to test (if I understand correctly). It doesn't need to be in the main bdk crate and I think it's desirable not to have non cargo dependencies for tests in the main bdk crate.
@LLFourn LLFourn added this to the v1 milestone Feb 21, 2023
@LLFourn LLFourn mentioned this issue Feb 22, 2023
Merged
9 tasks
@danielabrozzoni
Copy link
Member

Just to clarify, it would be moved to the same bdk workspace but in a different crate under crates, right? And I suppose there wouldn't be any feature inside of bdk for enabling it, but the people interested can just import bdk_hardware_signer themselves

@notmandatory notmandatory moved this to Todo in BDK Maintenance Feb 27, 2023
@notmandatory notmandatory removed this from the 1.0.0-alpha.0 milestone Mar 3, 2023
@notmandatory notmandatory added this to BDK May 4, 2023
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Oct 9, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
9623964 
Fixes bitcoindevkit#872
@danielabrozzoni danielabrozzoni mentioned this issue Oct 9, 2023
Merged
4 tasks
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Oct 9, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
4241c8b 
Fixes bitcoindevkit#872
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Oct 9, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
8cfee3e 
Fixes bitcoindevkit#872
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Oct 9, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
6d9920d 
Fixes bitcoindevkit#872
@realeinherjar realeinherjar mentioned this issue Oct 20, 2023
Closed
45 tasks
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Nov 9, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
4049c70 
Fixes bitcoindevkit#872
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Nov 24, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
e291408 
Fixes bitcoindevkit#872
@notmandatory notmandatory moved this to In Progress in BDK Dec 5, 2023
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Dec 13, 2023
@danielabrozzoni
ref(hwi): Move hwi out of bdk
f1124d4 
Fixes bitcoindevkit#872
@storopoli storopoli mentioned this issue Jan 6, 2024
Closed
45 tasks
notmandatory added a commit that referenced this issue Jan 8, 2024
@notmandatory
Merge #1161: ref(hwi): Move hwi out of bdk
6e6bad9 
105d70e ref(hwi): Move hwi out of bdk (Daniela Brozzoni)

Pull request description:

  Fixes #872

  ### Description

  This commit moves the `hardwaresigner` outside of bdk and inside `bdk_hwi`

  ### Notes to the reviewers

  There are currently two issues with the code:
  - `TransactionSigner` dictates that `sign_transaction` must return a `SignerError` - but being `SignerError` defined inside of bdk, we can't modify it to include an hwi specific error! I don't know how we could fix this (other than getting rid of the trait altogether :)); for now I just added a `SignerError::Generic` variant, lmk if you have better ideas!
  - The hwi tests used the bdk utils to get a funded wallet for testing, which aren't available in `bdk_hwi`, which made me realize - maybe we should expose them so that we can use them across our crates, and also our users can use them to test their code?
  For now, I just left the test commented.

  ### Changelog notice

  - The old `hardwaresigner` module has been moved out of `bdk` and inside `bdk_hwi`.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  ~~* [ ] I've added tests for the new feature~~
  * [x] I've added docs for the new feature

ACKs for top commit:
  notmandatory:
    reACK 105d70e

Tree-SHA512: 9ae3cd035cc27bd7b2831e89c104f40771c6f165cb3bfe1a9052f820050fca7c19f4dd68f171c630a71a7d18ccdee5f94adbc497c6475bd257c2d01cc08109a1
@github-project-automation github-project-automation bot moved this from In Progress to Done in BDK Jan 8, 2024
@notmandatory notmandatory added this to the 1.0.0 milestone Jan 8, 2024
casey-bowman pushed a commit to casey-bowman/bdk that referenced this issue Jan 9, 2024
@danielabrozzoni @casey-bowman
ref(hwi): Move hwi out of bdk
cde8956 
Fixes bitcoindevkit#872
@notmandatory notmandatory added the api A breaking API change label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danielabrozzoni danielabrozzoni

Labels
api A breaking API change
Projects
BDK
Archived in project
Milestone
1.0.0-alpha
Development

Successfully merging a pull request may close this issue.

ref(hwi): Move hwi out of bdk danielabrozzoni/bdk
3 participants
@LLFourn @notmandatory @danielabrozzoni