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.

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

Bump hwi to 0.4.0 #825

Merged
merged 1 commit into from
Dec 26, 2022
Merged

Conversation

danielabrozzoni
Copy link
Member

@danielabrozzoni danielabrozzoni commented Dec 19, 2022

Description

Notes to the reviewers

Changelog notice

  • Bump hwi to 0.4.0

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Copy link
Contributor

@rajarshimaitra rajarshimaitra left a comment

Choose a reason for hiding this comment

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

Review ACK..

I am thinking how about adding a link to https://github.com/bitcoin-core/HWI in the doc above for people who don't know where to get the hwi python package from..

println!(
"The hardware wallet's descriptor is: {}",
descriptors.receive[0]
);

// Creating a custom signer from the device
let custom_signer = HWISigner::from_device(first_device, HWIChain::Test)?;
let custom_signer = HWISigner::from_device(&first_device, HWIChain::Test)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am hitting an OSError at this line with my coldcard.. Any guess?

Hold tight, I'm connecting to your hardware wallet...
No module named 'hwilib.devices.jadepy.jade_ble'
BLE scanning/connectivity will not be available
Look what I found, a coldcard!
The hardware wallet's descriptor is: pkh([ef2dfe99/44'/1'/0']tpubDDEc4pGkD8MPWq2D1DGb3DWH2RcXTe9cZYGqgFBGwYRA7rgf1uZop65ptiiKCKB12vS4UrysKucmdjFwjhVzpwNmcVNBXd84uqtj8TYLuVx/0/*)#ktdw6k4n
Error: PyErr("OSError: open failed")

@notmandatory
Copy link
Member

Should we try to get this into the 0.26.0 release? The code freeze was supposed to be today..but will probably delayed a few days..

@danielabrozzoni
Copy link
Member Author

Should we try to get this into the 0.26.0 release? The code freeze was supposed to be today..but will probably delayed a few days..

I think it'd be cool if we can make it :)

@rajarshimaitra can you try with bdk from master? It's possible that you'll hit the same error, in which case it's definitely to fix, but at least we know that it wasn't introduced in this PR :)

@rajarshimaitra
Copy link
Contributor

@rajarshimaitra can you try with bdk from master? It's possible that you'll hit the same error, in which case it's definitely to fix, but at least we know that it wasn't introduced in this PR :)

Yes I am hitting the same one in master too..

@notmandatory
Copy link
Member

@rajarshimaitra since you're getting the same error on master branch this should be ready to go right?

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 4cad18b

@danielabrozzoni
Copy link
Member Author

@rajarshimaitra since you're getting the same error on master branch this should be ready to go right?

I think it is! :)

@notmandatory notmandatory merged commit 5a48347 into bitcoindevkit:master Dec 26, 2022
@notmandatory notmandatory added the new feature New feature or request label Dec 26, 2022
@notmandatory notmandatory mentioned this pull request Dec 26, 2022
24 tasks
@danielabrozzoni danielabrozzoni deleted the bump_rust_hwi branch October 11, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants