Skip to content

Commit b562842

Browse files
TheBlueMattTheBlueMatt
committed
Clean up and clarify tx broadcast checks in channelmonitor tests
This effecitlvey reverts the refactors in 383bd90, however keeps the actully new test code. It also writes documentation for the super confusing tx test func and makes it a bit less permissive.
1 parent 3d54ae8 commit b562842

File tree

1 file changed

+58
-45
lines changed

1 file changed

+58
-45
lines changed

‎src/ln/channelmanager.rs

+58-45
Original file line numberDiff line numberDiff line change
@@ -3089,32 +3089,41 @@ mod tests {
30893089

30903090
#[derive(PartialEq)]
30913091
enum HTLCType { NONE, TIMEOUT, SUCCESS }
3092-
#[derive(PartialEq)]
3093-
enum PenaltyType { NONE, HTLC }
3094-
fn test_txn_broadcast(node: &Node, chan: &(msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction), commitment_tx: Option<Transaction>, revoked_tx: Option<Transaction>, has_htlc_tx: HTLCType, has_penalty_tx: PenaltyType) -> Vec<Transaction> {
3092+
/// Tests that the given node has broadcast transactions for the given Channel
3093+
///
3094+
/// First checks that the latest local commitment tx has been broadcast, unless an explicit
3095+
/// commitment_tx is provided, which may be used to test that a remote commitment tx was
3096+
/// broadcast and the revoked outputs were claimed.
3097+
///
3098+
/// Next tests that there is (or is not) a transaction that spends the commitment transaction
3099+
/// that appears to be the type of HTLC transaction specified in has_htlc_tx.
3100+
///
3101+
/// All broadcast transactions must be accounted for in one of the above three types of we'll
3102+
/// also fail.
3103+
fn test_txn_broadcast(node: &Node, chan: &(msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction), commitment_tx: Option<Transaction>, has_htlc_tx: HTLCType) -> Vec<Transaction> {
30953104
let mut node_txn = node.tx_broadcaster.txn_broadcasted.lock().unwrap();
3096-
assert!(node_txn.len() >= if has_htlc_tx == HTLCType::NONE { 0 } else { 1 } + if has_penalty_tx == PenaltyType::NONE { 0 } else { 1 });
3105+
assert!(node_txn.len() >= if commitment_tx.is_some() { 0 } else { 1 } + if has_htlc_tx == HTLCType::NONE { 0 } else { 1 });
30973106

30983107
let mut res = Vec::with_capacity(2);
3099-
3100-
if let Some(explicit_tx) = commitment_tx {
3101-
res.push(explicit_tx.clone());
3102-
} else {
3103-
for tx in node_txn.iter() {
3104-
if tx.input.len() == 1 && tx.input[0].previous_output.txid == chan.3.txid() {
3105-
let mut funding_tx_map = HashMap::new();
3106-
funding_tx_map.insert(chan.3.txid(), chan.3.clone());
3107-
tx.verify(&funding_tx_map).unwrap();
3108+
node_txn.retain(|tx| {
3109+
if tx.input.len() == 1 && tx.input[0].previous_output.txid == chan.3.txid() {
3110+
let mut funding_tx_map = HashMap::new();
3111+
funding_tx_map.insert(chan.3.txid(), chan.3.clone());
3112+
tx.verify(&funding_tx_map).unwrap();
3113+
if commitment_tx.is_none() {
31083114
res.push(tx.clone());
31093115
}
3110-
}
3111-
}
3112-
if !revoked_tx.is_some() && !(has_penalty_tx == PenaltyType::HTLC) {
3113-
assert_eq!(res.len(), 1);
3116+
false
3117+
} else { true }
3118+
});
3119+
if let Some(explicit_tx) = commitment_tx {
3120+
res.push(explicit_tx.clone());
31143121
}
31153122

3123+
assert_eq!(res.len(), 1);
3124+
31163125
if has_htlc_tx != HTLCType::NONE {
3117-
for tx in node_txn.iter() {
3126+
node_txn.retain(|tx| {
31183127
if tx.input.len() == 1 && tx.input[0].previous_output.txid == res[0].txid() {
31193128
let mut funding_tx_map = HashMap::new();
31203129
funding_tx_map.insert(res[0].txid(), res[0].clone());
@@ -3125,29 +3134,33 @@ mod tests {
31253134
assert!(tx.lock_time == 0);
31263135
}
31273136
res.push(tx.clone());
3128-
break;
3129-
}
3130-
}
3137+
println!("11");
3138+
false
3139+
} else { true }
3140+
});
31313141
assert_eq!(res.len(), 2);
31323142
}
31333143

3134-
if has_penalty_tx == PenaltyType::HTLC {
3135-
let revoked_tx = revoked_tx.unwrap();
3136-
for tx in node_txn.iter() {
3137-
if tx.input.len() == 1 && tx.input[0].previous_output.txid == revoked_tx.txid() {
3138-
let mut funding_tx_map = HashMap::new();
3139-
funding_tx_map.insert(revoked_tx.txid(), revoked_tx.clone());
3140-
tx.verify(&funding_tx_map).unwrap();
3141-
res.push(tx.clone());
3142-
break;
3143-
}
3144-
}
3145-
assert_eq!(res.len(), 1);
3146-
}
3147-
node_txn.clear();
3144+
assert!(node_txn.is_empty());
31483145
res
31493146
}
31503147

3148+
/// Tests that the given node has broadcast a claim transaction against the provided revoked
3149+
/// HTLC transaction.
3150+
fn test_revoked_htlc_claim_txn_broadcast(node: &Node, revoked_tx: Transaction) {
3151+
let mut node_txn = node.tx_broadcaster.txn_broadcasted.lock().unwrap();
3152+
assert_eq!(node_txn.len(), 1);
3153+
node_txn.retain(|tx| {
3154+
if tx.input.len() == 1 && tx.input[0].previous_output.txid == revoked_tx.txid() {
3155+
let mut funding_tx_map = HashMap::new();
3156+
funding_tx_map.insert(revoked_tx.txid(), revoked_tx.clone());
3157+
tx.verify(&funding_tx_map).unwrap();
3158+
false
3159+
} else { true }
3160+
});
3161+
assert!(node_txn.is_empty());
3162+
}
3163+
31513164
fn check_preimage_claim(node: &Node, prev_txn: &Vec<Transaction>) -> Vec<Transaction> {
31523165
let mut node_txn = node.tx_broadcaster.txn_broadcasted.lock().unwrap();
31533166

@@ -3221,10 +3234,10 @@ mod tests {
32213234
// Simple case with no pending HTLCs:
32223235
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), true);
32233236
{
3224-
let mut node_txn = test_txn_broadcast(&nodes[1], &chan_1, None, None, HTLCType::NONE, PenaltyType::NONE);
3237+
let mut node_txn = test_txn_broadcast(&nodes[1], &chan_1, None, HTLCType::NONE);
32253238
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
32263239
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn.drain(..).next().unwrap()] }, 1);
3227-
test_txn_broadcast(&nodes[0], &chan_1, None, None, HTLCType::NONE, PenaltyType::NONE);
3240+
test_txn_broadcast(&nodes[0], &chan_1, None, HTLCType::NONE);
32283241
}
32293242
get_announce_close_broadcast_events(&nodes, 0, 1);
32303243
assert_eq!(nodes[0].node.list_channels().len(), 0);
@@ -3236,10 +3249,10 @@ mod tests {
32363249
// Simple case of one pending HTLC to HTLC-Timeout
32373250
nodes[1].node.peer_disconnected(&nodes[2].node.get_our_node_id(), true);
32383251
{
3239-
let mut node_txn = test_txn_broadcast(&nodes[1], &chan_2, None, None, HTLCType::TIMEOUT, PenaltyType::NONE);
3252+
let mut node_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
32403253
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
32413254
nodes[2].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn.drain(..).next().unwrap()] }, 1);
3242-
test_txn_broadcast(&nodes[2], &chan_2, None, None, HTLCType::NONE, PenaltyType::NONE);
3255+
test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::NONE);
32433256
}
32443257
get_announce_close_broadcast_events(&nodes, 1, 2);
32453258
assert_eq!(nodes[1].node.list_channels().len(), 0);
@@ -3273,7 +3286,7 @@ mod tests {
32733286
// HTLC-Timeout and a nodes[3] claim against it (+ its own announces)
32743287
nodes[2].node.peer_disconnected(&nodes[3].node.get_our_node_id(), true);
32753288
{
3276-
let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, None, HTLCType::TIMEOUT, PenaltyType::NONE);
3289+
let node_txn = test_txn_broadcast(&nodes[2], &chan_3, None, HTLCType::TIMEOUT);
32773290

32783291
// Claim the payment on nodes[3], giving it knowledge of the preimage
32793292
claim_funds!(nodes[3], nodes[2], payment_preimage_1);
@@ -3298,7 +3311,7 @@ mod tests {
32983311
nodes[3].chain_monitor.block_connected_checked(&header, i, &Vec::new()[..], &[0; 0]);
32993312
}
33003313

3301-
let node_txn = test_txn_broadcast(&nodes[3], &chan_4, None, None, HTLCType::TIMEOUT, PenaltyType::NONE);
3314+
let node_txn = test_txn_broadcast(&nodes[3], &chan_4, None, HTLCType::TIMEOUT);
33023315

33033316
// Claim the payment on nodes[4], giving it knowledge of the preimage
33043317
claim_funds!(nodes[4], nodes[3], payment_preimage_2);
@@ -3310,7 +3323,7 @@ mod tests {
33103323
nodes[4].chain_monitor.block_connected_checked(&header, i, &Vec::new()[..], &[0; 0]);
33113324
}
33123325

3313-
test_txn_broadcast(&nodes[4], &chan_4, None, None, HTLCType::SUCCESS, PenaltyType::NONE);
3326+
test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
33143327

33153328
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
33163329
nodes[4].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn[0].clone()] }, TEST_FINAL_CLTV - 5);
@@ -3345,13 +3358,13 @@ mod tests {
33453358
node_txn[0].verify(&funding_tx_map).unwrap();
33463359
node_txn.swap_remove(0);
33473360
}
3348-
test_txn_broadcast(&nodes[1], &chan_5, None, None, HTLCType::NONE, PenaltyType::NONE);
3361+
test_txn_broadcast(&nodes[1], &chan_5, None, HTLCType::NONE);
33493362

33503363
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
3351-
let node_txn = test_txn_broadcast(&nodes[0], &chan_5, Some(revoked_local_txn[0].clone()), None, HTLCType::TIMEOUT, PenaltyType::NONE);
3364+
let node_txn = test_txn_broadcast(&nodes[0], &chan_5, Some(revoked_local_txn[0].clone()), HTLCType::TIMEOUT);
33523365
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
33533366
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn[1].clone()] }, 1);
3354-
test_txn_broadcast(&nodes[1], &chan_5, None, Some(node_txn[1].clone()), HTLCType::NONE, PenaltyType::HTLC);
3367+
test_revoked_htlc_claim_txn_broadcast(&nodes[1], node_txn[1].clone());
33553368
}
33563369
get_announce_close_broadcast_events(&nodes, 0, 1);
33573370
assert_eq!(nodes[0].node.list_channels().len(), 0);

0 commit comments

Comments
 (0)