Skip to content

Fold sign_holder_commitment_htlc_transactions into sign_holder_commitment #770

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 18, 2021
Merged

Fold sign_holder_commitment_htlc_transactions into sign_holder_commitment #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb83cfe
Fold sign_holder_commitment_htlc_transactions into sign_holder_commit…
devrandom Jan 5, 2021
63c56a4
Panic if signing fails in OnChainTx
devrandom Jan 5, 2021
0b20cf6
Simplify handling of OnChainTx.holder_commitment
devrandom Jan 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 41 additions & 45 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {

let key_derivation_params = keys.key_derivation_params();
let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;
let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys, channel_parameters.clone());

let secp_ctx = Secp256k1::new();

Expand All @@ -991,7 +990,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
};
(holder_commitment_tx, trusted_tx.commitment_number())
};
onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);

let onchain_tx_handler =
OnchainTxHandler::new(destination_script.clone(), keys, channel_parameters.clone(), initial_holder_commitment_tx);

let mut outputs_to_watch = HashMap::new();
outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
Expand Down Expand Up @@ -1725,28 +1726,26 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
pub fn get_latest_holder_commitment_txn<L: Deref>(&mut self, logger: &L) -> Vec<Transaction> where L::Target: Logger {
log_trace!(logger, "Getting signed latest holder commitment transaction!");
self.holder_tx_signed = true;
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript) {
let txid = commitment_tx.txid();
let mut res = vec![commitment_tx];
for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
if let Some(vout) = htlc.0.transaction_output_index {
let preimage = if !htlc.0.offered {
if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
// We can't build an HTLC-Success transaction without the preimage
continue;
}
} else { None };
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage) {
res.push(htlc_tx);
let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
let txid = commitment_tx.txid();
let mut res = vec![commitment_tx];
for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
if let Some(vout) = htlc.0.transaction_output_index {
let preimage = if !htlc.0.offered {
if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
// We can't build an HTLC-Success transaction without the preimage
continue;
}
} else { None };
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage) {
res.push(htlc_tx);
}
}
// We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
// The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation.
return res
}
Vec::new()
// We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
// The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation.
return res;
}

/// Unsafe test-only version of get_latest_holder_commitment_txn used by our test framework
Expand All @@ -1755,26 +1754,24 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
pub fn unsafe_get_latest_holder_commitment_txn<L: Deref>(&mut self, logger: &L) -> Vec<Transaction> where L::Target: Logger {
log_trace!(logger, "Getting signed copy of latest holder commitment transaction!");
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_holder_tx(&self.funding_redeemscript) {
let txid = commitment_tx.txid();
let mut res = vec![commitment_tx];
for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
if let Some(vout) = htlc.0.transaction_output_index {
let preimage = if !htlc.0.offered {
if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
// We can't build an HTLC-Success transaction without the preimage
continue;
}
} else { None };
if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage) {
res.push(htlc_tx);
let commitment_tx = self.onchain_tx_handler.get_fully_signed_copy_holder_tx(&self.funding_redeemscript);
let txid = commitment_tx.txid();
let mut res = vec![commitment_tx];
for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() {
if let Some(vout) = htlc.0.transaction_output_index {
let preimage = if !htlc.0.offered {
if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(preimage.clone()) } else {
// We can't build an HTLC-Success transaction without the preimage
continue;
}
} else { None };
if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage) {
res.push(htlc_tx);
}
}
return res
}
Vec::new()
return res
}

/// Processes transactions in a newly connected block, which may result in any of the following:
Expand Down Expand Up @@ -1853,15 +1850,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
}
if should_broadcast {
self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript) {
self.holder_tx_signed = true;
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
if !new_outputs.is_empty() {
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
}
claimable_outpoints.append(&mut new_outpoints);
}
let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
self.holder_tx_signed = true;
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
if !new_outputs.is_empty() {
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
}
claimable_outpoints.append(&mut new_outpoints);
}
if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) {
for ev in events {
Expand Down
41 changes: 17 additions & 24 deletions lightning/src/chain/keysinterface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,21 @@ pub trait ChannelKeys : Send+Clone + Writeable {
// TODO: Document the things someone using this interface should enforce before signing.
fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;

/// Create a signature for a holder's commitment transaction. This will only ever be called with
/// the same commitment_tx (or a copy thereof), though there are currently no guarantees
/// that it will not be called multiple times.
/// Create a signatures for a holder's commitment transaction and its claiming HTLC transactions.
/// This will only ever be called with a non-revoked commitment_tx. This will be called with the
/// latest commitment_tx when we initiate a force-close.
/// This will be called with the previous latest, just to get claiming HTLC signatures, if we are
/// reacting to a ChannelMonitor replica that decided to broadcast before it had been updated to
/// the latest.
/// This may be called multiple times for the same transaction.
///
/// An external signer implementation should check that the commitment has not been revoked.
///
/// May return Err if key derivation fails. Callers, such as ChannelMonitor, will panic in such a case.
//
// TODO: Document the things someone using this interface should enforce before signing.
fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;
// TODO: Key derivation failure should panic rather than Err
fn sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;

/// Same as sign_holder_commitment, but exists only for tests to get access to holder commitment
/// transactions which will be broadcasted later, after the channel has moved on to a newer
Expand All @@ -248,18 +256,6 @@ pub trait ChannelKeys : Send+Clone + Writeable {
#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;

/// Create a signature for each HTLC transaction spending a holder's commitment transaction.
///
/// Unlike sign_holder_commitment, this may be called multiple times with *different*
/// commitment_tx values. While this will never be called with a revoked
/// commitment_tx, it is possible that it is called with the second-latest
/// commitment_tx (only if we haven't yet revoked it) if some watchtower/secondary
/// ChannelMonitor decided to broadcast before it had been updated to the latest.
///
/// Either an Err should be returned, or a Vec with one entry for each HTLC which exists in
/// commitment_tx.
fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()>;

/// Create a signature for the given input in a transaction spending an HTLC or commitment
/// transaction output when our counterparty broadcasts an old state.
///
Expand Down Expand Up @@ -500,11 +496,14 @@ impl ChannelKeys for InMemoryChannelKeys {
Ok((commitment_sig, htlc_sigs))
}

fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
fn sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
let sig = commitment_tx.trust().built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
Ok(sig)
let channel_parameters = self.get_channel_parameters();
let trusted_tx = commitment_tx.trust();
let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
Ok((sig, htlc_sigs))
}

#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
Expand All @@ -514,12 +513,6 @@ impl ChannelKeys for InMemoryChannelKeys {
Ok(commitment_tx.trust().built_transaction().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
}

fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
let channel_parameters = self.get_channel_parameters();
let trusted_tx = commitment_tx.trust();
trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)
}

fn sign_justice_transaction<T: secp256k1::Signing + secp256k1::Verification>(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
let revocation_key = match chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key) {
Ok(revocation_key) => revocation_key,
Expand Down
4 changes: 1 addition & 3 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4742,15 +4742,13 @@ mod tests {
&chan.holder_keys.pubkeys().funding_pubkey,
chan.counterparty_funding_pubkey()
);
let holder_sig = chan_keys.sign_holder_commitment(&holder_commitment_tx, &secp_ctx).unwrap();
let (holder_sig, htlc_sigs) = chan_keys.sign_holder_commitment_and_htlcs(&holder_commitment_tx, &secp_ctx).unwrap();
assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig");

let funding_redeemscript = chan.get_funding_redeemscript();
let tx = holder_commitment_tx.add_holder_sig(&funding_redeemscript, holder_sig);
assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx");

let htlc_sigs = chan_keys.sign_holder_commitment_htlc_transactions(&holder_commitment_tx, &secp_ctx).unwrap();

// ((htlc, counterparty_sig), (index, holder_sig))
let mut htlc_sig_iter = holder_commitment_tx.htlcs().iter().zip(&holder_commitment_tx.counterparty_htlc_sigs).zip(htlc_sigs.iter().enumerate());

Expand Down
Loading