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
Fix wrong newtype APIs #1503
Fix wrong newtype APIs #1503
Conversation
This fixes several API bugs: * Use `TryFrom` instead of `From` for fallible conversions * Move byte conversion methods from `impl_array_newtype` to `impl_bytes_newtype` * Add missing trait impls like `AsRef`, `Borrow`, their mutable versions and infallible conversions from arrays Closes rust-bitcoin#1336
@@ -149,7 +149,7 @@ impl PartiallySignedTransaction { | |||
} | |||
let derivation = DerivationPath::from(path); | |||
// Keys, according to BIP-174, must be unique | |||
if xpub_map.insert(xpub, (Fingerprint::from(&fingerprint[..]), derivation)).is_some() { | |||
if xpub_map.insert(xpub, (Fingerprint::from(fingerprint), derivation)).is_some() { | |||
return Err(encode::Error::ParseFailed("Repeated global xpub key")) |
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.
I think there are two missing tests that need to be checked
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.
Unrelated but which ones? Why do 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.
ACK 64f7d25
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.
code review ACK 64f7d25
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.
ACK 64f7d25
This fixes several API bugs:
TryFrom
instead ofFrom
for fallible conversionsimpl_array_newtype
toimpl_bytes_newtype
AsRef
,Borrow
, their mutable versions and infallible conversions from arraysCloses #1336
Notably this change is much less bad than even I expected and some places actually get cleaner than before.