Fixups for taproot improvements#778
Fixups for taproot improvements#778sanket1729 merged 4 commits intorust-bitcoin:masterfrom BP-WG:taproot/improvements-2
Conversation
tcharding
left a comment
There was a problem hiding this comment.
Just a question to help me learn more.
| let mut output_key = self.clone(); | ||
| output_key.tweak_add_assign(&secp, &tweak_value).expect("Tap tweak failed"); | ||
| TweakedKeyPair(output_key) | ||
| self.tweak_add_assign(&secp, &tweak_value).expect("Tap tweak failed"); |
There was a problem hiding this comment.
I don't mean to de-reail your PR but why is it ok to panic here please? The docs of this function say that this can error if the resulting key would be invalid, is that not possible here?
There was a problem hiding this comment.
It will fail with negligible probability - multiple orders if magnitude less then probability of meteor hitting the server directly.
It can only fail if the tweak * G is the inverse of the original public key - which could not happen with hash computed out of key itself - or if the tweak is > than group order, which has that "several orders of magnitude of meteor hitting server" probability.
There was a problem hiding this comment.
Perhaps the panic message should've been "Tap tweak broken" since that's a more likely cause for the error. (We had similar discussion not long ago.)
I wouldn't mind if the method itself just panicked without returning Result but that's another discussion.
This addresses @Kixunil review comments in #696 post-merge
Update: also closes nits from issues #764 and #770