Skip to content

Fix simple to_local revoked output claim and rebase #184 #199

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 4 commits into from
Sep 30, 2018
Merged

Fix simple to_local revoked output claim and rebase #184 #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae8bd1b
Fix and test simple remote-revoked-commitment-output claiming
TheBlueMatt Sep 29, 2018
a805567
Test htlc outputs shared tx claim case
Sep 20, 2018
0983193
Test htlc outputs single tx claim due to timeout case
Sep 29, 2018
af29adc
Macro-out checking a tx validly spends another (and add one more)
TheBlueMatt Sep 30, 2018
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
219 changes: 195 additions & 24 deletions src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2284,7 +2284,7 @@ mod tests {
use rand::{thread_rng,Rng};

use std::cell::RefCell;
use std::collections::HashMap;
use std::collections::{BTreeSet, HashMap};
use std::default::Default;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -2606,6 +2606,17 @@ mod tests {
(chan_announcement.1, chan_announcement.2, chan_announcement.3, chan_announcement.4)
}

macro_rules! check_spends {
($tx: expr, $spends_tx: expr) => {
{
let mut funding_tx_map = HashMap::new();
let spends_tx = $spends_tx;
funding_tx_map.insert(spends_tx.txid(), spends_tx);
$tx.verify(&funding_tx_map).unwrap();
}
}
}

fn close_channel(outbound_node: &Node, inbound_node: &Node, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate) {
let (node_a, broadcaster_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster) } else { (&outbound_node.node, &outbound_node.tx_broadcaster) };
let (node_b, broadcaster_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster) } else { (&inbound_node.node, &inbound_node.tx_broadcaster) };
Expand Down Expand Up @@ -2649,9 +2660,7 @@ mod tests {
tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
}
assert_eq!(tx_a, tx_b);
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(funding_tx.txid(), funding_tx);
tx_a.verify(&funding_tx_map).unwrap();
check_spends!(tx_a, funding_tx);

let events_2 = node_a.get_and_clear_pending_events();
assert_eq!(events_2.len(), 1);
Expand Down Expand Up @@ -3189,9 +3198,7 @@ mod tests {
let mut res = Vec::with_capacity(2);
node_txn.retain(|tx| {
if tx.input.len() == 1 && tx.input[0].previous_output.txid == chan.3.txid() {
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(chan.3.txid(), chan.3.clone());
tx.verify(&funding_tx_map).unwrap();
check_spends!(tx, chan.3.clone());
if commitment_tx.is_none() {
res.push(tx.clone());
}
Expand All @@ -3207,9 +3214,7 @@ mod tests {
if has_htlc_tx != HTLCType::NONE {
node_txn.retain(|tx| {
if tx.input.len() == 1 && tx.input[0].previous_output.txid == res[0].txid() {
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(res[0].txid(), res[0].clone());
tx.verify(&funding_tx_map).unwrap();
check_spends!(tx, res[0].clone());
if has_htlc_tx == HTLCType::TIMEOUT {
assert!(tx.lock_time != 0);
} else {
Expand All @@ -3233,9 +3238,7 @@ mod tests {
assert_eq!(node_txn.len(), 1);
node_txn.retain(|tx| {
if tx.input.len() == 1 && tx.input[0].previous_output.txid == revoked_tx.txid() {
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(revoked_tx.txid(), revoked_tx.clone());
tx.verify(&funding_tx_map).unwrap();
check_spends!(tx, revoked_tx.clone());
false
} else { true }
});
Expand All @@ -3251,10 +3254,7 @@ mod tests {

for tx in prev_txn {
if node_txn[0].input[0].previous_output.txid == tx.txid() {
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(tx.txid(), tx.clone());
node_txn[0].verify(&funding_tx_map).unwrap();

check_spends!(node_txn[0], tx.clone());
assert!(node_txn[0].input[0].witness[2].len() > 106); // must spend an htlc output
assert_eq!(tx.input.len(), 1); // must spend a commitment tx

Expand Down Expand Up @@ -3422,6 +3422,13 @@ mod tests {
let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
// Get the will-be-revoked local txn from nodes[0]
let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.iter().next().unwrap().1.last_local_commitment_txn.clone();
assert_eq!(revoked_local_txn.len(), 2); // First commitment tx, then HTLC tx
assert_eq!(revoked_local_txn[0].input.len(), 1);
assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_5.3.txid());
assert_eq!(revoked_local_txn[0].output.len(), 2); // Only HTLC and output back to 0 are present
assert_eq!(revoked_local_txn[1].input.len(), 1);
assert_eq!(revoked_local_txn[1].input[0].previous_output.txid, revoked_local_txn[0].txid());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check that it's a HTLC-Timeout with assert_eq!(..witness.len(), 5) and (..witness[5].len(), 133) ?

assert_eq!(revoked_local_txn[1].input[0].witness.last().unwrap().len(), 133); // HTLC-Timeout
// Revoke the old state
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_3);

Expand All @@ -3432,11 +3439,9 @@ mod tests {
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
assert_eq!(node_txn.len(), 3);
assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
assert_eq!(node_txn[0].input.len(), 1);
assert_eq!(node_txn[0].input.len(), 2); // We should claim the revoked output and the HTLC output

let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(revoked_local_txn[0].txid(), revoked_local_txn[0].clone());
node_txn[0].verify(&funding_tx_map).unwrap();
check_spends!(node_txn[0], revoked_local_txn[0].clone());
node_txn.swap_remove(0);
}
test_txn_broadcast(&nodes[1], &chan_5, None, HTLCType::NONE);
Expand All @@ -3452,6 +3457,173 @@ mod tests {
assert_eq!(nodes[1].node.list_channels().len(), 0);
}

#[test]
fn revoked_output_claim() {
// Simple test to ensure a node will claim a revoked output when a stale remote commitment
// transaction is broadcast by its counterparty
let nodes = create_network(2);
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
// node[0] is gonna to revoke an old state thus node[1] should be able to claim the revoked output
let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
assert_eq!(revoked_local_txn.len(), 1);
// Only output is the full channel value back to nodes[0]:
assert_eq!(revoked_local_txn[0].output.len(), 1);
// Send a payment through, updating everyone's latest commitment txn
send_payment(&nodes[0], &vec!(&nodes[1])[..], 5000000);

// Inform nodes[1] that nodes[0] broadcast a stale tx
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
assert_eq!(node_txn.len(), 3); // nodes[1] will broadcast justice tx twice, and its own local state once
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side-thought: Aren't duplicate (or more) tx generation due to block re-scan not gonna be a bottleneck for watchtowers ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I suppose it might, though hopefully watchtowers aren't broadcasting that many transactions to begin with as people would have to be misbehaving for that.


assert_eq!(node_txn[0], node_txn[2]);

check_spends!(node_txn[0], revoked_local_txn[0].clone());
check_spends!(node_txn[1], chan_1.3.clone());

// Inform nodes[0] that a watchtower cheated on its behalf, so it will force-close the chan
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
get_announce_close_broadcast_events(&nodes, 0, 1);
}

#[test]
fn claim_htlc_outputs_shared_tx() {
// Node revoked old state, htlcs haven't time out yet, claim them in shared justice tx
let nodes = create_network(2);

// Create some new channel:
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);

// Rebalance the network to generate htlc in the two directions
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
// node[0] is gonna to revoke an old state thus node[1] should be able to claim both offered/received HTLC outputs on top of commitment tx
let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
let _payment_preimage_2 = route_payment(&nodes[1], &vec!(&nodes[0])[..], 3000000).0;

// Get the will-be-revoked local txn from node[0]
let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
assert_eq!(revoked_local_txn.len(), 2); // commitment tx + 1 HTLC-Timeout tx
assert_eq!(revoked_local_txn[0].input.len(), 1);
assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_1.3.txid());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, maybe ensure that the witnessScript is an offered htlc one?

assert_eq!(revoked_local_txn[1].input.len(), 1);
assert_eq!(revoked_local_txn[1].input[0].previous_output.txid, revoked_local_txn[0].txid());
assert_eq!(revoked_local_txn[1].input[0].witness.last().unwrap().len(), 133); // HTLC-Timeout
check_spends!(revoked_local_txn[1], revoked_local_txn[0].clone());

//Revoke the old state
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_1);

{
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };

nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);

nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
assert_eq!(node_txn.len(), 4);

assert_eq!(node_txn[0].input.len(), 3); // Claim the revoked output + both revoked HTLC outputs
check_spends!(node_txn[0], revoked_local_txn[0].clone());

assert_eq!(node_txn[0], node_txn[3]); // justice tx is duplicated due to block re-scanning

let mut witness_lens = BTreeSet::new();
witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
witness_lens.insert(node_txn[0].input[1].witness.last().unwrap().len());
witness_lens.insert(node_txn[0].input[2].witness.last().unwrap().len());
assert_eq!(witness_lens.len(), 3);
assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), 133); // revoked offered HTLC
assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), 138); // revoked received HTLC

// Next nodes[1] broadcasts its current local tx state:
assert_eq!(node_txn[1].input.len(), 1);
assert_eq!(node_txn[1].input[0].previous_output.txid, chan_1.3.txid()); //Spending funding tx unique txouput, tx broadcasted by ChannelManager

assert_eq!(node_txn[2].input.len(), 1);
let witness_script = node_txn[2].clone().input[0].witness.pop().unwrap();
assert_eq!(witness_script.len(), 133); //Spending an offered htlc output
assert_eq!(node_txn[2].input[0].previous_output.txid, node_txn[1].txid());
assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[1].previous_output.txid);
}
get_announce_close_broadcast_events(&nodes, 0, 1);
assert_eq!(nodes[0].node.list_channels().len(), 0);
assert_eq!(nodes[1].node.list_channels().len(), 0);
}

#[test]
fn claim_htlc_outputs_single_tx() {
// Node revoked old state, htlcs have timed out, claim each of them in separated justice tx
let nodes = create_network(2);

let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);

// Rebalance the network to generate htlc in the two directions
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
// node[0] is gonna to revoke an old state thus node[1] should be able to claim both offered/received HTLC outputs on top of commitment tx, but this
// time as two different claim transactions as we're gonna to timeout htlc with given a high current height
let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
let _payment_preimage_2 = route_payment(&nodes[1], &vec!(&nodes[0])[..], 3000000).0;

// Get the will-be-revoked local txn from node[0]
let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();

//Revoke the old state
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_1);

{
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };

nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);

nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
assert_eq!(node_txn.len(), 12); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan)

assert_eq!(node_txn[0], node_txn[7]);
assert_eq!(node_txn[1], node_txn[8]);
assert_eq!(node_txn[2], node_txn[9]);
assert_eq!(node_txn[3], node_txn[10]);
assert_eq!(node_txn[4], node_txn[11]);
assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcated by ChannelManger
assert_eq!(node_txn[4], node_txn[6]);

assert_eq!(node_txn[0].input.len(), 1);
assert_eq!(node_txn[1].input.len(), 1);
assert_eq!(node_txn[2].input.len(), 1);

let mut revoked_tx_map = HashMap::new();
revoked_tx_map.insert(revoked_local_txn[0].txid(), revoked_local_txn[0].clone());
node_txn[0].verify(&revoked_tx_map).unwrap();
node_txn[1].verify(&revoked_tx_map).unwrap();
node_txn[2].verify(&revoked_tx_map).unwrap();

let mut witness_lens = BTreeSet::new();
witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
witness_lens.insert(node_txn[2].input[0].witness.last().unwrap().len());
assert_eq!(witness_lens.len(), 3);
assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), 133); // revoked offered HTLC
assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), 138); // revoked received HTLC

assert_eq!(node_txn[3].input.len(), 1);
check_spends!(node_txn[3], chan_1.3.clone());

assert_eq!(node_txn[4].input.len(), 1);
let witness_script = node_txn[4].input[0].witness.last().unwrap();
assert_eq!(witness_script.len(), 133); //Spending an offered htlc output
assert_eq!(node_txn[4].input[0].previous_output.txid, node_txn[3].txid());
assert_ne!(node_txn[4].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
assert_ne!(node_txn[4].input[0].previous_output.txid, node_txn[1].input[0].previous_output.txid);
}
get_announce_close_broadcast_events(&nodes, 0, 1);
assert_eq!(nodes[0].node.list_channels().len(), 0);
assert_eq!(nodes[1].node.list_channels().len(), 0);
}

#[test]
fn test_htlc_ignore_latest_remote_commitment() {
// Test that HTLC transactions spending the latest remote commitment transaction are simply
Expand Down Expand Up @@ -3608,9 +3780,8 @@ mod tests {
assert_eq!(node_txn[0].input[0].previous_output.txid, tx.txid());
assert_eq!(node_txn[0].lock_time, 0); // Must be an HTLC-Success
assert_eq!(node_txn[0].input[0].witness.len(), 5); // Must be an HTLC-Success
let mut funding_tx_map = HashMap::new();
funding_tx_map.insert(tx.txid(), tx);
node_txn[0].verify(&funding_tx_map).unwrap();

check_spends!(node_txn[0], tx);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/ln/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ impl ChannelMonitor {
ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key)))
},
};
let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.delayed_payment_base_key));
let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap()));
let a_htlc_key = match self.their_htlc_base_key {
None => return (txn_to_broadcast, (commitment_txid, watch_outputs)),
Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)),
Expand Down Expand Up @@ -856,7 +856,7 @@ impl ChannelMonitor {
};
let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
txn_to_broadcast.push(single_htlc_tx); // TODO: This is not yet tested in ChannelManager!
txn_to_broadcast.push(single_htlc_tx);
}
}
}
Expand Down