Skip to content

Commit a30946b

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #391 from ariard/2019-11-fix-preimage-collision-onchain
Avoid claiming remote received HTLCs with side-learned preimage
2 parents 2afd531 + 945c399 commit a30946b

File tree

2 files changed

+107
-38
lines changed

2 files changed

+107
-38
lines changed

‎src/ln/channelmonitor.rs

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,46 +1643,48 @@ impl ChannelMonitor {
16431643
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
16441644
}
16451645
if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
1646-
let input = TxIn {
1647-
previous_output: BitcoinOutPoint {
1648-
txid: commitment_txid,
1649-
vout: transaction_output_index,
1650-
},
1651-
script_sig: Script::new(),
1652-
sequence: idx as u32, // reset to 0xfffffffd in sign_input
1653-
witness: Vec::new(),
1654-
};
1655-
if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
1656-
inputs.push(input);
1657-
inputs_desc.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC });
1658-
inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value, htlc.cltv_expiry));
1659-
total_value += tx.output[transaction_output_index as usize].value;
1660-
} else {
1661-
let mut single_htlc_tx = Transaction {
1662-
version: 2,
1663-
lock_time: 0,
1664-
input: vec![input],
1665-
output: vec!(TxOut {
1666-
script_pubkey: self.destination_script.clone(),
1667-
value: htlc.amount_msat / 1000,
1668-
}),
1646+
if htlc.offered {
1647+
let input = TxIn {
1648+
previous_output: BitcoinOutPoint {
1649+
txid: commitment_txid,
1650+
vout: transaction_output_index,
1651+
},
1652+
script_sig: Script::new(),
1653+
sequence: idx as u32, // reset to 0xfffffffd in sign_input
1654+
witness: Vec::new(),
16691655
};
1670-
let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }]);
1671-
let height_timer = Self::get_height_timer(height, htlc.cltv_expiry);
1672-
let mut used_feerate;
1673-
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1674-
let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
1675-
let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
1676-
assert!(predicted_weight >= single_htlc_tx.get_weight());
1677-
spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
1678-
outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 },
1679-
output: single_htlc_tx.output[0].clone(),
1680-
});
1681-
match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) {
1682-
hash_map::Entry::Occupied(_) => {},
1683-
hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000 }, used_feerate, htlc.cltv_expiry, height)); }
1656+
if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
1657+
inputs.push(input);
1658+
inputs_desc.push(if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC });
1659+
inputs_info.push((payment_preimage, tx.output[transaction_output_index as usize].value, htlc.cltv_expiry));
1660+
total_value += tx.output[transaction_output_index as usize].value;
1661+
} else {
1662+
let mut single_htlc_tx = Transaction {
1663+
version: 2,
1664+
lock_time: 0,
1665+
input: vec![input],
1666+
output: vec!(TxOut {
1667+
script_pubkey: self.destination_script.clone(),
1668+
value: htlc.amount_msat / 1000,
1669+
}),
1670+
};
1671+
let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }]);
1672+
let height_timer = Self::get_height_timer(height, htlc.cltv_expiry);
1673+
let mut used_feerate;
1674+
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1675+
let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
1676+
let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
1677+
assert!(predicted_weight >= single_htlc_tx.get_weight());
1678+
spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
1679+
outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 },
1680+
output: single_htlc_tx.output[0].clone(),
1681+
});
1682+
match self.our_claim_txn_waiting_first_conf.entry(single_htlc_tx.input[0].previous_output.clone()) {
1683+
hash_map::Entry::Occupied(_) => {},
1684+
hash_map::Entry::Vacant(entry) => { entry.insert((height_timer, TxMaterial::RemoteHTLC { script: redeemscript, key: htlc_key, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000 }, used_feerate, htlc.cltv_expiry, height)); }
1685+
}
1686+
txn_to_broadcast.push(single_htlc_tx);
16841687
}
1685-
txn_to_broadcast.push(single_htlc_tx);
16861688
}
16871689
}
16881690
}

‎src/ln/functional_tests.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,73 @@ fn duplicate_htlc_test() {
12451245
claim_payment(&nodes[1], &vec!(&nodes[3])[..], payment_preimage);
12461246
}
12471247

1248+
#[test]
1249+
fn test_duplicate_htlc_different_direction_onchain() {
1250+
// Test that ChannelMonitor doesn't generate 2 preimage txn
1251+
// when we have 2 HTLCs with same preimage that go across a node
1252+
// in opposite directions.
1253+
let nodes = create_network(2, &[None, None]);
1254+
1255+
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new());
1256+
1257+
// balancing
1258+
send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
1259+
1260+
let (payment_preimage, payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 900_000);
1261+
1262+
let route = nodes[1].router.get_route(&nodes[0].node.get_our_node_id(), None, &Vec::new(), 800_000, TEST_FINAL_CLTV).unwrap();
1263+
send_along_route_with_hash(&nodes[1], route, &vec!(&nodes[0])[..], 800_000, payment_hash);
1264+
1265+
// Provide preimage to node 0 by claiming payment
1266+
nodes[0].node.claim_funds(payment_preimage);
1267+
check_added_monitors!(nodes[0], 1);
1268+
1269+
// Broadcast node 1 commitment txn
1270+
let remote_txn = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
1271+
1272+
assert_eq!(remote_txn[0].output.len(), 4); // 1 local, 1 remote, 1 htlc inbound, 1 htlc outbound
1273+
let mut has_both_htlcs = 0; // check htlcs match ones committed
1274+
for outp in remote_txn[0].output.iter() {
1275+
if outp.value == 800_000 / 1000 {
1276+
has_both_htlcs += 1;
1277+
} else if outp.value == 900_000 / 1000 {
1278+
has_both_htlcs += 1;
1279+
}
1280+
}
1281+
assert_eq!(has_both_htlcs, 2);
1282+
1283+
let header = BlockHeader { version: 0x2000_0000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
1284+
1285+
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![remote_txn[0].clone()] }, 1);
1286+
1287+
// Check we only broadcast 1 timeout tx
1288+
let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
1289+
let htlc_pair = if claim_txn[0].output[0].value == 800_000 / 1000 { (claim_txn[0].clone(), claim_txn[1].clone()) } else { (claim_txn[1].clone(), claim_txn[0].clone()) };
1290+
assert_eq!(claim_txn.len(), 6);
1291+
assert_eq!(htlc_pair.0.input.len(), 1);
1292+
assert_eq!(htlc_pair.0.input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
1293+
check_spends!(htlc_pair.0, remote_txn[0].clone());
1294+
assert_eq!(htlc_pair.1.input.len(), 1);
1295+
assert_eq!(htlc_pair.1.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
1296+
check_spends!(htlc_pair.1, remote_txn[0].clone());
1297+
1298+
let events = nodes[0].node.get_and_clear_pending_msg_events();
1299+
assert_eq!(events.len(), 2);
1300+
for e in events {
1301+
match e {
1302+
MessageSendEvent::BroadcastChannelUpdate { .. } => {},
1303+
MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => {
1304+
assert!(update_add_htlcs.is_empty());
1305+
assert!(update_fail_htlcs.is_empty());
1306+
assert_eq!(update_fulfill_htlcs.len(), 1);
1307+
assert!(update_fail_malformed_htlcs.is_empty());
1308+
assert_eq!(nodes[1].node.get_our_node_id(), *node_id);
1309+
},
1310+
_ => panic!("Unexpected event"),
1311+
}
1312+
}
1313+
}
1314+
12481315
fn do_channel_reserve_test(test_recv: bool) {
12491316
use ln::msgs::HandleError;
12501317

0 commit comments

Comments
 (0)