Fix multi-remote-HTLC-claim preimage learning

When our counterparty claims multiple HTLCs from offered outputs in
one transaction we should still be able to learn the preimages.
Sadly, due to two bugs we were not previously doing so.
This commit is contained in:
Matt Corallo 2019-01-06 15:14:43 -05:00
parent a5bcd5651d
commit c9df4bd011
2 changed files with 82 additions and 52 deletions

View file

@ -1747,10 +1747,13 @@ impl ChannelMonitor {
for tx in txn.iter() {
broadcaster.broadcast_transaction(tx);
}
let mut updated = self.is_resolving_htlc_output(tx);
if updated.len() > 0 {
htlc_updated.append(&mut updated);
}
}
// While all commitment/HTLC-Success/HTLC-Timeout transactions have one input, HTLCs
// can also be resolved in a few other ways which can have more than one output. Thus,
// we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check.
let mut updated = self.is_resolving_htlc_output(tx);
if updated.len() > 0 {
htlc_updated.append(&mut updated);
}
}
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
@ -1882,10 +1885,10 @@ impl ChannelMonitor {
|| (input.witness.len() == 3 && input.witness[2].len() == ACCEPTED_HTLC_SCRIPT_WEIGHT && input.witness[1].len() == 33) {
log_error!(self, "Remote used revocation sig to take a {} HTLC output at index {} from commitment_tx {}", if input.witness[2].len() == OFFERED_HTLC_SCRIPT_WEIGHT { "offered" } else { "accepted" }, input.previous_output.vout, input.previous_output.txid);
} else if input.witness.len() == 5 && input.witness[4].len() == ACCEPTED_HTLC_SCRIPT_WEIGHT {
payment_preimage.0.copy_from_slice(&tx.input[0].witness[3]);
payment_preimage.0.copy_from_slice(&input.witness[3]);
htlc_updated.push((source, Some(payment_preimage), payment_hash));
} else if input.witness.len() == 3 && input.witness[2].len() == OFFERED_HTLC_SCRIPT_WEIGHT {
payment_preimage.0.copy_from_slice(&tx.input[0].witness[1]);
payment_preimage.0.copy_from_slice(&input.witness[1]);
htlc_updated.push((source, Some(payment_preimage), payment_hash));
} else {
htlc_updated.push((source, None, payment_hash));

View file

@ -2648,14 +2648,15 @@ fn test_htlc_on_chain_success() {
// Test that in case of an unilateral close onchain, we detect the state of output thanks to
// ChainWatchInterface and pass the preimage backward accordingly. So here we test that ChannelManager is
// broadcasting the right event to other nodes in payment path.
// We test with two HTLCs simultaneously as that was not handled correctly in the past.
// A --------------------> B ----------------------> C (preimage)
// First, C should claim the HTLC output via HTLC-Success when its own latest local
// First, C should claim the HTLC outputs via HTLC-Success when its own latest local
// commitment transaction was broadcast.
// Then, B should learn the preimage from said transactions, attempting to claim backwards
// towards B.
// B should be able to claim via preimage if A then broadcasts its local tx.
// Finally, when A sees B's latest local commitment transaction it should be able to claim
// the HTLC output via the preimage it learned (which, once confirmed should generate a
// the HTLC outputs via the preimage it learned (which, once confirmed should generate a
// PaymentSent event).
let nodes = create_network(3);
@ -2669,6 +2670,7 @@ fn test_htlc_on_chain_success() {
send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 8000000);
let (our_payment_preimage, _payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
let (our_payment_preimage_2, _payment_hash_2) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3000000);
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
// Broadcast legit commitment tx from C on B's chain
@ -2677,7 +2679,8 @@ fn test_htlc_on_chain_success() {
assert_eq!(commitment_tx.len(), 1);
check_spends!(commitment_tx[0], chan_2.3.clone());
nodes[2].node.claim_funds(our_payment_preimage);
check_added_monitors!(nodes[2], 1);
nodes[2].node.claim_funds(our_payment_preimage_2);
check_added_monitors!(nodes[2], 2);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
assert!(updates.update_fail_htlcs.is_empty());
@ -2686,22 +2689,28 @@ fn test_htlc_on_chain_success() {
nodes[2].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 1);
check_closed_broadcast!(nodes[2]);
let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 1 (commitment tx), ChannelMonitor : 2 (2 * HTLC-Success tx)
assert_eq!(node_txn.len(), 3);
assert_eq!(node_txn[1], commitment_tx[0]);
assert_eq!(node_txn[0], node_txn[2]);
let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 1 (commitment tx), ChannelMonitor : 4 (2*2 * HTLC-Success tx)
assert_eq!(node_txn.len(), 5);
assert_eq!(node_txn[0], node_txn[3]);
assert_eq!(node_txn[1], node_txn[4]);
assert_eq!(node_txn[2], commitment_tx[0]);
check_spends!(node_txn[0], commitment_tx[0].clone());
check_spends!(node_txn[1], commitment_tx[0].clone());
assert_eq!(node_txn[0].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[1].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert_eq!(node_txn[0].lock_time, 0);
assert_eq!(node_txn[1].lock_time, 0);
// Verify that B's ChannelManager is able to extract preimage from HTLC Success tx and pass it backward
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: node_txn}, 1);
let events = nodes[1].node.get_and_clear_pending_msg_events();
{
let mut added_monitors = nodes[1].chan_monitor.added_monitors.lock().unwrap();
assert_eq!(added_monitors.len(), 1);
assert_eq!(added_monitors.len(), 2);
assert_eq!(added_monitors[0].0.txid, chan_1.3.txid());
assert_eq!(added_monitors[1].0.txid, chan_1.3.txid());
added_monitors.clear();
}
assert_eq!(events.len(), 2);
@ -2719,25 +2728,45 @@ fn test_htlc_on_chain_success() {
},
_ => panic!("Unexpected event"),
};
{
// nodes[1] now broadcasts its own local state as a fallback, suggesting an alternate
// commitment transaction with a corresponding HTLC-Timeout transaction, as well as a
// timeout-claim of the output that nodes[2] just claimed via success.
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 1 (timeout tx) * 2 (block-rescan)
assert_eq!(node_txn.len(), 4);
assert_eq!(node_txn[0], node_txn[3]);
check_spends!(node_txn[0], commitment_tx[0].clone());
assert_eq!(node_txn[0].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
assert_ne!(node_txn[0].lock_time, 0);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
check_spends!(node_txn[1], chan_2.3.clone());
check_spends!(node_txn[2], node_txn[1].clone());
assert_eq!(node_txn[1].input[0].witness.clone().last().unwrap().len(), 71);
assert_eq!(node_txn[2].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert_ne!(node_txn[2].lock_time, 0);
node_txn.clear();
macro_rules! check_tx_local_broadcast {
($node: expr, $htlc_offered: expr, $commitment_tx: expr, $chan_tx: expr) => { {
// ChannelManager : 3 (commitment tx, 2*HTLC-Timeout tx), ChannelMonitor : 2 (timeout tx) * 2 (block-rescan)
let mut node_txn = $node.tx_broadcaster.txn_broadcasted.lock().unwrap();
assert_eq!(node_txn.len(), 7);
assert_eq!(node_txn[0], node_txn[5]);
assert_eq!(node_txn[1], node_txn[6]);
check_spends!(node_txn[0], $commitment_tx.clone());
check_spends!(node_txn[1], $commitment_tx.clone());
assert_ne!(node_txn[0].lock_time, 0);
assert_ne!(node_txn[1].lock_time, 0);
if $htlc_offered {
assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
} else {
assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
}
check_spends!(node_txn[2], $chan_tx.clone());
check_spends!(node_txn[3], node_txn[2].clone());
check_spends!(node_txn[4], node_txn[2].clone());
assert_eq!(node_txn[2].input[0].witness.last().unwrap().len(), 71);
assert_eq!(node_txn[3].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[4].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[3].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert!(node_txn[4].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert_ne!(node_txn[3].lock_time, 0);
assert_ne!(node_txn[4].lock_time, 0);
node_txn.clear();
} }
}
// nodes[1] now broadcasts its own local state as a fallback, suggesting an alternate
// commitment transaction with a corresponding HTLC-Timeout transactions, as well as a
// timeout-claim of the output that nodes[2] just claimed via success.
check_tx_local_broadcast!(nodes[1], false, commitment_tx[0], chan_2.3);
// Broadcast legit commitment tx from A on B's chain
// Broadcast preimage tx by B on offered output from A commitment tx on A's chain
@ -2749,7 +2778,9 @@ fn test_htlc_on_chain_success() {
assert_eq!(node_txn.len(), 3);
assert_eq!(node_txn[0], node_txn[2]);
check_spends!(node_txn[0], commitment_tx[0].clone());
assert_eq!(node_txn[0].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[0].input.len(), 2);
assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[0].input[1].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_eq!(node_txn[0].lock_time, 0);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment
check_spends!(node_txn[1], chan_1.3.clone());
@ -2761,26 +2792,22 @@ fn test_htlc_on_chain_success() {
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![commitment_tx[0].clone(), node_txn[0].clone()] }, 1);
check_closed_broadcast!(nodes[0]);
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentSent { payment_preimage } => {
assert_eq!(payment_preimage, our_payment_preimage);
},
_ => panic!("Unexpected event"),
assert_eq!(events.len(), 2);
let mut first_claimed = false;
for event in events {
match event {
Event::PaymentSent { payment_preimage } => {
if payment_preimage == our_payment_preimage {
assert!(!first_claimed);
first_claimed = true;
} else {
assert_eq!(payment_preimage, our_payment_preimage_2);
}
},
_ => panic!("Unexpected event"),
}
}
let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 1 (HTLC-Timeout tx) * 2 (block-rescan)
assert_eq!(node_txn.len(), 4);
assert_eq!(node_txn[0], node_txn[3]);
check_spends!(node_txn[0], commitment_tx[0].clone());
assert_eq!(node_txn[0].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert_ne!(node_txn[0].lock_time, 0);
assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
check_spends!(node_txn[1], chan_1.3.clone());
check_spends!(node_txn[2], node_txn[1].clone());
assert_eq!(node_txn[1].input[0].witness.clone().last().unwrap().len(), 71);
assert_eq!(node_txn[2].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
assert!(node_txn[2].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output
assert_ne!(node_txn[2].lock_time, 0);
check_tx_local_broadcast!(nodes[0], true, commitment_tx[0], chan_1.3);
}
#[test]