Update to secp256k1 0.21.2 #755
Conversation
sanket1729
changed the title
sanket1729
force-pushed
the
secp_update
branch
from
b8cdd3e
to
f9dde94
Compare
sanket1729
force-pushed
the
secp_update
branch
2 times, most recently
from
84c48f7
to
10883eb
Compare
1671dfc Release 0.21.2 (sanket1729) 837be22 Basic derives for Parity (sanket1729) 7059192 Wildcard export from key module (sanket1729) Pull request description: Sorry for getting another point release. This time I have tested against this branch for rust-bitcoin rust-bitcoin/rust-bitcoin#755. Hopefully, this is the last release. Next release, we should have a Release Candidate for a couple of days before publishing a release. ACKs for top commit: apoelstra: ACK 1671dfc Tree-SHA512: 263ad027da3da764bd76f719200382c47ba21a976caefc23ebef45d1c4be35ddfc80ce619b57326310aaab22bbf75ca7f1db80b45e95ec076584805efb791f3f
sanket1729
force-pushed
the
secp_update
branch
from
10883eb
to
b5d4f03
Compare
sanket1729
changed the title
We can check tweak add priv key with latest secp
sanket1729
force-pushed
the
secp_update
branch
from
b5d4f03
to
91470f5
Compare
|
This is ready for review now and kind of a blocker for the rest of the PRs for the next release. @apoelstra, @dr-orlovsky , @Kixunil |
| @@ -700,8 +700,7 @@ impl ControlBlock { | |||
|
|
|||
| /// Serialize to a writer. Returns the number of bytes written | |||
| pub fn encode<Write: io::Write>(&self, mut writer: Write) -> io::Result<usize> { | |||
| let first_byte: u8 = | |||
| (if self.output_key_parity { 1 } else { 0 }) | self.leaf_version.as_u8(); | |||
| let first_byte: u8 = i32::from(self.output_key_parity) as u8 | self.leaf_version.as_u8(); | |||
There was a problem hiding this comment.
Here, we rely on Parity being only one bit. But converting to u8 should be enough.
There was a problem hiding this comment.
ACK 91470f5
However, I'm highly surprised Parity is not an enum (Even, Odd) and even more strangely, impl From<i32> for Parity exists. I'd expect it to be TryFrom. I understand this is something from secp256k1, that's why I'm ACKing this even though I find it suspicious, maybe even wrong.
If someone could explain why it's not an enum, it'd be appreciated.
This was a mistake in rust-secp and I the maintainers agree with it. I think we have to live with this now till the next release :(. For what it's worth, rust-bitcoin only has states Parity(1)/Parity(0) and there is no function that takes input Parity from the user. |
|
As far as I can see, the consumer of rust-bitcoin could construct invalid |
There was a problem hiding this comment.
utACK 91470f5 in order to unlock the rest of PRs required for Taproot, which depend on this.
However I feel that we need another major secp256k1 release fixing Parity (making it enum) before releasing new rust-bitcoin version.
|
One use of deprecated function: Since this is a non-critical warning I will merge this anyway, but it may be nice to fix it in some other PR |
No description provided.