-
Notifications
You must be signed in to change notification settings - Fork 11
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
Derive secrets from commitment points #96
Conversation
@@ -285,6 +297,14 @@ impl Privkey { | |||
pub fn pubkey(&self) -> Pubkey { | |||
Pubkey::from(self.0.public_key(secp256k1_instance())) | |||
} | |||
|
|||
pub fn tweak<I: Into<[u8; 32]>>(&self, scalar: I) -> Self { |
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.
Since you only need ref of [u8; 32]
, AsRef
is better than Into
here.
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.
The problem I encountered is that https://doc.rust-lang.org/std/convert/trait.AsRef.html#tymethod.as_ref AsRef<[u8; 32]>
is not automatically implemented for [u8; 32]
. The automatic implementation of the rust compiler only works without the length 32. https://doc.rust-lang.org/std/convert/trait.AsRef.html#impl-AsRef%3C%5BT%5D%3E-for-%5BT%5D
// Normally commitment number will be incremented after received a RevokeAndAck message. | ||
// But here channel has not been etablished yet, so we will not receive RevokeAndAck message. | ||
// We increment the commitment number here instead. | ||
state.increment_local_commitment_number(); |
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.
Could you explain why this is no longer needed?
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.
This commit has relevant information about this change. 11f710b. TLDR is that I moved the code to on_channel_ready
function. It has to be delayed to there because incrementing these numbers too soon may cause problems.
// Normally commitment number will be incremented after sent a RevokeAndAck message. | ||
// But here channel has not been etablished yet, so we will not send RevokeAndAck message. | ||
// We increment the commitment number here instead. | ||
self.increment_remote_commitment_number(); |
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.
Could you explain why this is no longer needed?
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.
Ditto
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.
Skimmed
Before current local commitment number change and channel ready, we actually use the commitment number to derive secrets. Changing the commitment number too soon will make the secrets derived from different parties different.
Fixes #63