-
Notifications
You must be signed in to change notification settings - Fork 376
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
Implement PSBT fields that were missing for a Signer #2761
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,19 +10,19 @@ | |||||
//! Keys used to generate commitment transactions. | ||||||
//! See: <https://github.com/lightning/bolts/blob/master/03-transactions.md#keys> | ||||||
|
||||||
use bitcoin::hashes::Hash; | ||||||
use bitcoin::hashes::HashEngine; | ||||||
use bitcoin::secp256k1::Scalar; | ||||||
use bitcoin::secp256k1::SecretKey; | ||||||
use bitcoin::secp256k1::Secp256k1; | ||||||
use bitcoin::secp256k1; | ||||||
use crate::io; | ||||||
use crate::ln::msgs::DecodeError; | ||||||
use crate::util::ser::Readable; | ||||||
use crate::io; | ||||||
use crate::util::ser::Writer; | ||||||
use crate::util::ser::Writeable; | ||||||
use bitcoin::secp256k1::PublicKey; | ||||||
use crate::util::ser::Writer; | ||||||
use bitcoin::hashes::sha256::Hash as Sha256; | ||||||
use bitcoin::hashes::Hash; | ||||||
use bitcoin::hashes::HashEngine; | ||||||
use bitcoin::secp256k1; | ||||||
use bitcoin::secp256k1::PublicKey; | ||||||
use bitcoin::secp256k1::Scalar; | ||||||
use bitcoin::secp256k1::Secp256k1; | ||||||
use bitcoin::secp256k1::SecretKey; | ||||||
|
||||||
macro_rules! doc_comment { | ||||||
($x:expr, $($tt:tt)*) => { | ||||||
|
@@ -37,15 +37,28 @@ macro_rules! basepoint_impl { | |||||
pub fn to_public_key(&self) -> PublicKey { | ||||||
self.0 | ||||||
} | ||||||
|
||||||
/// Derives a per-commitment-transaction (eg an htlc key or delayed_payment key) private key addition tweak | ||||||
/// from a basepoint and a per_commitment_point: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really need some paragraph breaks in here. |
||||||
/// `privkey = basepoint_secret + SHA256(per_commitment_point || basepoint)` | ||||||
/// This calculates the hash part in the tweak derivation process, which is used to ensure | ||||||
/// that each key is unique and cannot be guessed by an external party. It is equivalent | ||||||
/// to the `from_basepoint` method, but without the addition operation, providing just the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should work?
Suggested change
|
||||||
/// tweak from the hash of the per_commitment_point and the basepoint. | ||||||
pub fn derive_add_tweak(&self, per_commitment_point: &PublicKey) -> [u8; 32] { | ||||||
let mut sha = Sha256::engine(); | ||||||
sha.input(&per_commitment_point.serialize()); | ||||||
sha.input(&self.to_public_key().serialize()); | ||||||
Sha256::from_engine(sha).to_byte_array() | ||||||
} | ||||||
} | ||||||
|
||||||
impl From<PublicKey> for $BasepointT { | ||||||
fn from(value: PublicKey) -> Self { | ||||||
Self(value) | ||||||
} | ||||||
} | ||||||
|
||||||
} | ||||||
}; | ||||||
} | ||||||
macro_rules! key_impl { | ||||||
($BasepointT:ty, $KeyName:expr) => { | ||||||
|
@@ -87,11 +100,9 @@ macro_rules! key_read_write { | |||||
Ok(Self(key)) | ||||||
} | ||||||
} | ||||||
} | ||||||
}; | ||||||
} | ||||||
|
||||||
|
||||||
|
||||||
/// Base key used in conjunction with a `per_commitment_point` to generate a [`DelayedPaymentKey`]. | ||||||
/// | ||||||
/// The delayed payment key is used to pay the commitment state broadcaster their | ||||||
|
@@ -102,7 +113,6 @@ pub struct DelayedPaymentBasepoint(pub PublicKey); | |||||
basepoint_impl!(DelayedPaymentBasepoint); | ||||||
key_read_write!(DelayedPaymentBasepoint); | ||||||
|
||||||
|
||||||
/// A derived key built from a [`DelayedPaymentBasepoint`] and `per_commitment_point`. | ||||||
/// | ||||||
/// The delayed payment key is used to pay the commitment state broadcaster their | ||||||
|
@@ -150,14 +160,26 @@ key_read_write!(HtlcKey); | |||||
/// Derives a per-commitment-transaction public key (eg an htlc key or a delayed_payment key) | ||||||
/// from the base point and the per_commitment_key. This is the public equivalent of | ||||||
/// derive_private_key - using only public keys to derive a public key instead of private keys. | ||||||
fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, base_point: &PublicKey) -> PublicKey { | ||||||
fn derive_public_key<T: secp256k1::Signing>( | ||||||
secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, base_point: &PublicKey, | ||||||
) -> PublicKey { | ||||||
let mut sha = Sha256::engine(); | ||||||
sha.input(&per_commitment_point.serialize()); | ||||||
sha.input(&base_point.serialize()); | ||||||
let res = Sha256::from_engine(sha).to_byte_array(); | ||||||
|
||||||
let hashkey = PublicKey::from_secret_key(&secp_ctx, | ||||||
&SecretKey::from_slice(&res).expect("Hashes should always be valid keys unless SHA-256 is broken")); | ||||||
add_public_key_tweak(secp_ctx, base_point, &res) | ||||||
} | ||||||
|
||||||
/// Adds a tweak to a public key to derive a new public key. | ||||||
pub fn add_public_key_tweak<T: secp256k1::Signing>( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given this can panic if the tweak isn't a hash, I'd prefer if we either make this crate-private or at least document it. |
||||||
secp_ctx: &Secp256k1<T>, base_point: &PublicKey, tweak: &[u8; 32], | ||||||
) -> PublicKey { | ||||||
let hashkey = PublicKey::from_secret_key( | ||||||
&secp_ctx, | ||||||
&SecretKey::from_slice(tweak) | ||||||
.expect("Hashes should always be valid keys unless SHA-256 is broken"), | ||||||
); | ||||||
base_point.combine(&hashkey) | ||||||
.expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak contains the hash of the key.") | ||||||
} | ||||||
|
@@ -169,7 +191,6 @@ pub struct RevocationBasepoint(pub PublicKey); | |||||
basepoint_impl!(RevocationBasepoint); | ||||||
key_read_write!(RevocationBasepoint); | ||||||
|
||||||
|
||||||
/// The revocation key is used to allow a channel party to revoke their state - giving their | ||||||
/// counterparty the required material to claim all of their funds if they broadcast that state. | ||||||
/// | ||||||
|
@@ -192,8 +213,7 @@ impl RevocationKey { | |||||
/// | ||||||
/// [`chan_utils::derive_private_revocation_key`]: crate::ln::chan_utils::derive_private_revocation_key | ||||||
pub fn from_basepoint<T: secp256k1::Verification>( | ||||||
secp_ctx: &Secp256k1<T>, | ||||||
countersignatory_basepoint: &RevocationBasepoint, | ||||||
secp_ctx: &Secp256k1<T>, countersignatory_basepoint: &RevocationBasepoint, | ||||||
per_commitment_point: &PublicKey, | ||||||
) -> Self { | ||||||
let rev_append_commit_hash_key = { | ||||||
|
@@ -227,28 +247,56 @@ impl RevocationKey { | |||||
} | ||||||
key_read_write!(RevocationKey); | ||||||
|
||||||
|
||||||
#[cfg(test)] | ||||||
mod test { | ||||||
use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey}; | ||||||
use bitcoin::hashes::hex::FromHex; | ||||||
use super::derive_public_key; | ||||||
use bitcoin::hashes::hex::FromHex; | ||||||
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; | ||||||
|
||||||
#[test] | ||||||
fn test_key_derivation() { | ||||||
// Test vectors from BOLT 3 Appendix E: | ||||||
let secp_ctx = Secp256k1::new(); | ||||||
|
||||||
let base_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f").unwrap()[..]).unwrap(); | ||||||
let per_commitment_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); | ||||||
let base_secret = SecretKey::from_slice( | ||||||
&<Vec<u8>>::from_hex( | ||||||
"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", | ||||||
) | ||||||
.unwrap()[..], | ||||||
) | ||||||
.unwrap(); | ||||||
let per_commitment_secret = SecretKey::from_slice( | ||||||
&<Vec<u8>>::from_hex( | ||||||
"1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100", | ||||||
) | ||||||
.unwrap()[..], | ||||||
) | ||||||
.unwrap(); | ||||||
|
||||||
let base_point = PublicKey::from_secret_key(&secp_ctx, &base_secret); | ||||||
assert_eq!(base_point.serialize()[..], <Vec<u8>>::from_hex("036d6caac248af96f6afa7f904f550253a0f3ef3f5aa2fe6838a95b216691468e2").unwrap()[..]); | ||||||
assert_eq!( | ||||||
base_point.serialize()[..], | ||||||
<Vec<u8>>::from_hex( | ||||||
"036d6caac248af96f6afa7f904f550253a0f3ef3f5aa2fe6838a95b216691468e2" | ||||||
) | ||||||
.unwrap()[..] | ||||||
); | ||||||
|
||||||
let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); | ||||||
assert_eq!(per_commitment_point.serialize()[..], <Vec<u8>>::from_hex("025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486").unwrap()[..]); | ||||||
|
||||||
assert_eq!(derive_public_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..], | ||||||
<Vec<u8>>::from_hex("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]); | ||||||
assert_eq!( | ||||||
per_commitment_point.serialize()[..], | ||||||
<Vec<u8>>::from_hex( | ||||||
"025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486" | ||||||
) | ||||||
.unwrap()[..] | ||||||
); | ||||||
|
||||||
assert_eq!( | ||||||
derive_public_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..], | ||||||
<Vec<u8>>::from_hex( | ||||||
"0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5" | ||||||
) | ||||||
.unwrap()[..] | ||||||
); | ||||||
} | ||||||
} |
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 cloning of
channel_transaction_parameters
fromself.onchain_tx_handler
is noted. While cloning ensures data integrity and ownership rules compliance, it's essential to consider the performance implications for large data structures. Ifchannel_transaction_parameters
is sizable or complex, evaluating alternative strategies like borrowing (if lifecycle permits) or implementing a more efficient cloning method (e.g., usingArc
for shared ownership) could enhance performance.