Use Optional for passphrase #644
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We need to distinguish between passphrase not set, and empty passphrase. So use an Optional instead of checking for the empty string.
achow101
force-pushed
the
optional-passphrase
branch
from
9ff1c32
to
0c4df0c
Compare
|
Updated to unify Trezor's enumerate and other commands behavior. Both will require a passphrase to be provided if passphrase protection is on. |
If the passphrase is not provided and passphrase protection is enabled, default to the empty string.
|
#639 (comment) recommends that we default to the empty string passphrase if no passphrase is provided, so I've updated this to do that. |
|
Just fyi: Works fine with Specter now. |
|
@achow101 just checking in, when are you planning a new release with that PR included? The last release was in May, so might perhaps be about time anyway? |
|
@moneymanolis I plan on making a release at the beginning of the new year. |
|
Great, thanks! |
achow101
added a commit
to achow101/hardware-wallet-interface
that referenced
this pull request
Jan 11, 2023
Since bitcoin-core#644, not providing a passphrase to a trezor that has passphrases enabled will only result in a warning rather than setting `needs_passphrase_set` to false.
achow101
added a commit
that referenced
this pull request
Jan 12, 2023
… passphrase changes b8a379c test, trezor: Properly pass empty string passphrase (Andrew Chow) c3f490f test, trezor: Check for warnings rather than needs_passphrase_sent (Andrew Chow) 203c844 trezor: Add missing NoPasswordError import (Andrew Chow) Pull request description: Fixes a couple things were missed following #644 which are causing CI failures in Trezor tests. Top commit has no ACKs. Tree-SHA512: 508c6c9cdbc18833360a146296293500480938723843927e980e124ab24cb17f035418623e7fcb3e1a196e3e6bb7754bdd3bd2903d5d9064918ad875124efd1a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We should be distinguishing from the empty passphrase and no passphrase provided, so instead of defaulting to the empty passphrase, change passphrase (aka password) to an
Optional. This way, if it is not provided, it isNoneand empty passphrases can be handled correctly.Fixes #639