Skip to content

Check for timing-out HTLCs in remote unrevoked commitments #284

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 5 commits into from
Jan 13, 2019
Merged

Check for timing-out HTLCs in remote unrevoked commitments #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
09919d2
Make HTLCOutputInCommitment::transaction_output_index an Option
TheBlueMatt Jan 6, 2019
92424eb
Merge tracking of HTLCs-in-commitment with outbound-HTLCs
TheBlueMatt Jan 4, 2019
78939a7
Test for failing channel after local commitment dust HTLC timeout
TheBlueMatt Jan 7, 2019
d8707c2
Check for timing-out HTLCs in remote unrevoked commitments
TheBlueMatt Jan 4, 2019
f065a62
Fix comment spelling and clarify algebra a bit.
TheBlueMatt Jan 11, 2019
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
5 changes: 3 additions & 2 deletions src/ln/chan_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ pub struct HTLCOutputInCommitment {
pub amount_msat: u64,
pub cltv_expiry: u32,
pub payment_hash: PaymentHash,
pub transaction_output_index: u32,
pub transaction_output_index: Option<u32>,
}

#[inline]
Expand Down Expand Up @@ -222,12 +222,13 @@ pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKey
get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key)
}

/// panics if htlc.transaction_output_index.is_none()!
pub fn build_htlc_transaction(prev_hash: &Sha256dHash, feerate_per_kw: u64, to_self_delay: u16, htlc: &HTLCOutputInCommitment, a_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction {
let mut txins: Vec<TxIn> = Vec::new();
txins.push(TxIn {
previous_output: OutPoint {
txid: prev_hash.clone(),
vout: htlc.transaction_output_index,
vout: htlc.transaction_output_index.expect("Can't build an HTLC transaction for a dust output"),
},
script_sig: Script::new(),
sequence: 0,
Expand Down
149 changes: 77 additions & 72 deletions src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,6 @@ struct OutboundHTLCOutput {
fail_reason: Option<HTLCFailReason>,
}

macro_rules! get_htlc_in_commitment {
($htlc: expr, $offered: expr) => {
HTLCOutputInCommitment {
offered: $offered,
amount_msat: $htlc.amount_msat,
cltv_expiry: $htlc.cltv_expiry,
payment_hash: $htlc.payment_hash,
transaction_output_index: 0
}
}
}

/// See AwaitingRemoteRevoke ChannelState for more info
enum HTLCUpdateAwaitingACK {
AddHTLC {
Expand Down Expand Up @@ -759,8 +747,12 @@ impl Channel {
/// have not yet committed it. Such HTLCs will only be included in transactions which are being
/// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
/// which peer generated this transaction and "to whom" this transaction flows.
/// Returns (the transaction built, the number of HTLC outputs which were present in the
/// transaction, the list of HTLCs which were not ignored when building the transaction).
/// Note that below-dust HTLCs are included in the third return value, but not the second, and
/// sources are provided only for outbound HTLCs in the third return value.
#[inline]
fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64) -> (Transaction, Vec<HTLCOutputInCommitment>, Vec<(PaymentHash, &HTLCSource, Option<u32>)>) {
fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) {
let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);

let txins = {
Expand All @@ -775,38 +767,46 @@ impl Channel {
};

let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
let mut unincluded_htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::new();
let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();

let dust_limit_satoshis = if local { self.our_dust_limit_satoshis } else { self.their_dust_limit_satoshis };
let mut remote_htlc_total_msat = 0;
let mut local_htlc_total_msat = 0;
let mut value_to_self_msat_offset = 0;

macro_rules! get_htlc_in_commitment {
($htlc: expr, $offered: expr) => {
HTLCOutputInCommitment {
offered: $offered,
amount_msat: $htlc.amount_msat,
cltv_expiry: $htlc.cltv_expiry,
payment_hash: $htlc.payment_hash,
transaction_output_index: None
}
}
}

macro_rules! add_htlc_output {
($htlc: expr, $outbound: expr, $source: expr) => {
if $outbound == local { // "offered HTLC output"
let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
txouts.push((TxOut {
script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
value: $htlc.amount_msat / 1000
}, Some((htlc_in_tx, $source))));
} else {
if let Some(source) = $source {
unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
}
included_dust_htlcs.push((htlc_in_tx, $source));
}
} else {
let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
txouts.push((TxOut { // "received HTLC output"
script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
value: $htlc.amount_msat / 1000
}, Some((htlc_in_tx, $source))));
} else {
if let Some(source) = $source {
unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
}
included_dust_htlcs.push((htlc_in_tx, $source));
}
}
}
Expand Down Expand Up @@ -919,26 +919,23 @@ impl Channel {
transaction_utils::sort_outputs(&mut txouts);

let mut outputs: Vec<TxOut> = Vec::with_capacity(txouts.len());
let mut htlcs_included: Vec<HTLCOutputInCommitment> = Vec::with_capacity(txouts.len());
let mut htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::with_capacity(txouts.len() + unincluded_htlc_sources.len());
for (idx, out) in txouts.drain(..).enumerate() {
let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(txouts.len() + included_dust_htlcs.len());
for (idx, mut out) in txouts.drain(..).enumerate() {
outputs.push(out.0);
if let Some((mut htlc, source_option)) = out.1 {
htlc.transaction_output_index = idx as u32;
if let Some(source) = source_option {
htlc_sources.push((htlc.payment_hash, source, Some(idx as u32)));
}
htlcs_included.push(htlc);
if let Some((mut htlc, source_option)) = out.1.take() {
htlc.transaction_output_index = Some(idx as u32);
htlcs_included.push((htlc, source_option));
}
}
htlc_sources.append(&mut unincluded_htlc_sources);
let non_dust_htlc_count = htlcs_included.len();
htlcs_included.append(&mut included_dust_htlcs);

(Transaction {
version: 2,
lock_time: ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32),
input: txins,
output: outputs,
}, htlcs_included, htlc_sources)
}, non_dust_htlc_count, htlcs_included)
}

#[inline]
Expand Down Expand Up @@ -1451,9 +1448,9 @@ impl Channel {

// Now that we're past error-generating stuff, update our local state:

self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.last_local_commitment_txn = vec![local_initial_commitment_tx.clone()];
self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx, local_keys, self.feerate_per_kw, Vec::new(), Vec::new());
self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx, local_keys, self.feerate_per_kw, Vec::new());
self.channel_state = ChannelState::FundingSent as u32;
self.channel_id = funding_txo.to_channel_id();
self.cur_remote_commitment_transaction_number -= 1;
Expand Down Expand Up @@ -1490,7 +1487,7 @@ impl Channel {
secp_check!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey.unwrap()), "Invalid funding_signed signature from peer");

self.sign_commitment_transaction(&mut local_initial_commitment_tx, &msg.signature);
self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), local_keys, self.feerate_per_kw, Vec::new(), Vec::new());
self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), local_keys, self.feerate_per_kw, Vec::new());
self.last_local_commitment_txn = vec![local_initial_commitment_tx];
self.channel_state = ChannelState::FundingSent as u32;
self.cur_local_commitment_transaction_number -= 1;
Expand Down Expand Up @@ -1693,7 +1690,7 @@ impl Channel {

let mut local_commitment_tx = {
let mut commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, feerate_per_kw);
let htlcs_cloned: Vec<_> = commitment_tx.2.drain(..).map(|htlc_source| (htlc_source.0, htlc_source.1.clone(), htlc_source.2)).collect();
let htlcs_cloned: Vec<_> = commitment_tx.2.drain(..).map(|htlc| (htlc.0, htlc.1.map(|h| h.clone()))).collect();
(commitment_tx.0, commitment_tx.1, htlcs_cloned)
};
let local_commitment_txid = local_commitment_tx.0.txid();
Expand All @@ -1702,36 +1699,40 @@ impl Channel {

//If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
if update_fee {
let num_htlcs = local_commitment_tx.1.len();
let num_htlcs = local_commitment_tx.1;
let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;

if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
return Err(ChannelError::Close("Funding remote cannot afford proposed new fee"));
}
}

if msg.htlc_signatures.len() != local_commitment_tx.1.len() {
if msg.htlc_signatures.len() != local_commitment_tx.1 {
return Err(ChannelError::Close("Got wrong number of HTLC signatures from remote"));
}

let mut new_local_commitment_txn = Vec::with_capacity(local_commitment_tx.1.len() + 1);
let mut new_local_commitment_txn = Vec::with_capacity(local_commitment_tx.1 + 1);
self.sign_commitment_transaction(&mut local_commitment_tx.0, &msg.signature);
new_local_commitment_txn.push(local_commitment_tx.0.clone());

let mut htlcs_and_sigs = Vec::with_capacity(local_commitment_tx.1.len());
for (idx, htlc) in local_commitment_tx.1.drain(..).enumerate() {
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
let htlc_sig = if htlc.offered {
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
new_local_commitment_txn.push(htlc_tx);
htlc_sig
let mut htlcs_and_sigs = Vec::with_capacity(local_commitment_tx.2.len());
for (idx, (htlc, source)) in local_commitment_tx.2.drain(..).enumerate() {
if let Some(_) = htlc.transaction_output_index {
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
let htlc_sig = if htlc.offered {
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
new_local_commitment_txn.push(htlc_tx);
htlc_sig
} else {
self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
};
htlcs_and_sigs.push((htlc, Some((msg.htlc_signatures[idx], htlc_sig)), source));
} else {
self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
};
htlcs_and_sigs.push((htlc, msg.htlc_signatures[idx], htlc_sig));
htlcs_and_sigs.push((htlc, None, source));
}
}

let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
Expand All @@ -1758,7 +1759,7 @@ impl Channel {
self.monitor_pending_order = None;
}

self.channel_monitor.provide_latest_local_commitment_tx_info(local_commitment_tx.0, local_keys, self.feerate_per_kw, htlcs_and_sigs, local_commitment_tx.2);
self.channel_monitor.provide_latest_local_commitment_tx_info(local_commitment_tx.0, local_keys, self.feerate_per_kw, htlcs_and_sigs);

for htlc in self.pending_inbound_htlcs.iter_mut() {
let new_forward = if let &InboundHTLCState::RemoteAnnounced(ref forward_info) = &htlc.state {
Expand Down Expand Up @@ -3028,7 +3029,7 @@ impl Channel {
let temporary_channel_id = self.channel_id;

// Now that we're past error-generating stuff, update our local state:
self.channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.channel_monitor.provide_latest_remote_commitment_tx_info(&commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.channel_state = ChannelState::FundingCreated as u32;
self.channel_id = funding_txo.to_channel_id();
self.cur_remote_commitment_transaction_number -= 1;
Expand Down Expand Up @@ -3260,23 +3261,23 @@ impl Channel {
}
}

let (res, remote_commitment_tx, htlcs, htlc_sources) = match self.send_commitment_no_state_update() {
Ok((res, (remote_commitment_tx, htlcs, mut htlc_sources))) => {
let (res, remote_commitment_tx, htlcs) = match self.send_commitment_no_state_update() {
Ok((res, (remote_commitment_tx, mut htlcs))) => {
// Update state now that we've passed all the can-fail calls...
let htlc_sources_no_ref = htlc_sources.drain(..).map(|htlc_source| (htlc_source.0, htlc_source.1.clone(), htlc_source.2)).collect();
(res, remote_commitment_tx, htlcs, htlc_sources_no_ref)
let htlcs_no_ref = htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
(res, remote_commitment_tx, htlcs_no_ref)
},
Err(e) => return Err(e),
};

self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_commitment_tx, htlcs, htlc_sources, self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_commitment_tx, htlcs, self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
self.channel_state |= ChannelState::AwaitingRemoteRevoke as u32;
Ok((res, self.channel_monitor.clone()))
}

/// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation
/// when we shouldn't change HTLC/channel state.
fn send_commitment_no_state_update(&self) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<HTLCOutputInCommitment>, Vec<(PaymentHash, &HTLCSource, Option<u32>)>)), ChannelError> {
fn send_commitment_no_state_update(&self) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> {
let funding_script = self.get_funding_redeemscript();

let mut feerate_per_kw = self.feerate_per_kw;
Expand All @@ -3292,21 +3293,22 @@ impl Channel {
let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();
let our_sig = self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key);

let mut htlc_sigs = Vec::new();

for ref htlc in remote_commitment_tx.1.iter() {
let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
let mut htlc_sigs = Vec::with_capacity(remote_commitment_tx.1);
for &(ref htlc, _) in remote_commitment_tx.2.iter() {
if let Some(_) = htlc.transaction_output_index {
let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
}
}

Ok((msgs::CommitmentSigned {
channel_id: self.channel_id,
signature: our_sig,
htlc_signatures: htlc_sigs,
}, remote_commitment_tx))
}, (remote_commitment_tx.0, remote_commitment_tx.2)))
}

/// Adds a pending outbound HTLC to this channel, and creates a signed commitment transaction
Expand Down Expand Up @@ -4020,8 +4022,11 @@ mod tests {
macro_rules! test_commitment {
( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => {
unsigned_tx = {
let res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw);
(res.0, res.1)
let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw);
let htlcs = res.2.drain(..)
.filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
.collect();
(res.0, htlcs)
};
let their_signature = Signature::from_der(&secp_ctx, &hex::decode($their_sig_hex).unwrap()[..]).unwrap();
let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap();
Expand Down
Loading