Avoid commitment broadcast upon detected funding spend

There's no need to broadcast our local commitment transaction if we've
already seen a confirmed one as it'll be immediately rejected as a
duplicate/conflict.

This will also help prevent dispatching spurious events for bumping
commitment and HTLC transactions through anchor outputs (once
implemented in future work) and the dispatch for said events follows the
same flow as our usual commitment broadcast.
This commit is contained in:
Wilmer Paulino 2022-08-25 13:11:19 -07:00
parent 2f4a1f7f79
commit 62236c70d8
No known key found for this signature in database
GPG key ID: 6DF57B9F9514972F
3 changed files with 57 additions and 69 deletions

View file

@ -793,7 +793,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
// of block connection between ChannelMonitors and the ChannelManager.
funding_spend_seen: bool,
/// Set to `Some` of the confirmed transaction spending the funding input of the channel after
/// reaching `ANTI_REORG_DELAY` confirmations.
funding_spend_confirmed: Option<Txid>,
confirmed_commitment_tx_counterparty_output: CommitmentTxCounterpartyOutputInfo,
/// The set of HTLCs which have been either claimed or failed on chain and have reached
/// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the
@ -3068,6 +3071,16 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
}
fn should_broadcast_holder_commitment_txn<L: Deref>(&self, logger: &L) -> bool where L::Target: Logger {
// There's no need to broadcast our commitment transaction if we've seen one confirmed (even
// with 1 confirmation) as it'll be rejected as duplicate/conflicting.
if self.funding_spend_confirmed.is_some() ||
self.onchain_events_awaiting_threshold_conf.iter().find(|event| match event.event {
OnchainEvent::FundingSpendConfirmation { .. } => true,
_ => false,
}).is_some()
{
return false;
}
// We need to consider all HTLCs which are:
// * in any unrevoked counterparty commitment transaction, as they could broadcast said
// transactions and we'd end up in a race, or

View file

@ -1267,44 +1267,32 @@ fn test_duplicate_htlc_different_direction_onchain() {
connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
assert_eq!(claim_txn.len(), 8);
assert_eq!(claim_txn.len(), 5);
check_spends!(claim_txn[0], remote_txn[0]); // Immediate HTLC claim with preimage
check_spends!(claim_txn[1], chan_1.3); // Alternative commitment tx
check_spends!(claim_txn[2], claim_txn[1]); // HTLC spend in alternative commitment tx
let bump_tx = if claim_txn[1] == claim_txn[4] {
assert_eq!(claim_txn[1], claim_txn[4]);
assert_eq!(claim_txn[2], claim_txn[5]);
check_spends!(claim_txn[7], claim_txn[1]); // HTLC timeout on alternative commitment tx
check_spends!(claim_txn[3], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
&claim_txn[3]
check_spends!(claim_txn[3], remote_txn[0]);
check_spends!(claim_txn[4], remote_txn[0]);
let preimage_tx = &claim_txn[0];
let (preimage_bump_tx, timeout_tx) = if claim_txn[3].input[0].previous_output == preimage_tx.input[0].previous_output {
(&claim_txn[3], &claim_txn[4])
} else {
assert_eq!(claim_txn[1], claim_txn[3]);
assert_eq!(claim_txn[2], claim_txn[4]);
check_spends!(claim_txn[5], claim_txn[1]); // HTLC timeout on alternative commitment tx
check_spends!(claim_txn[7], remote_txn[0]); // HTLC timeout on broadcasted commitment tx
&claim_txn[7]
(&claim_txn[4], &claim_txn[3])
};
assert_eq!(claim_txn[0].input.len(), 1);
assert_eq!(bump_tx.input.len(), 1);
assert_eq!(claim_txn[0].input[0].previous_output, bump_tx.input[0].previous_output);
assert_eq!(preimage_tx.input.len(), 1);
assert_eq!(preimage_bump_tx.input.len(), 1);
assert_eq!(claim_txn[0].input.len(), 1);
assert_eq!(claim_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
assert_eq!(remote_txn[0].output[claim_txn[0].input[0].previous_output.vout as usize].value, 800);
assert_eq!(preimage_tx.input.len(), 1);
assert_eq!(preimage_tx.input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); // HTLC 1 <--> 0, preimage tx
assert_eq!(remote_txn[0].output[preimage_tx.input[0].previous_output.vout as usize].value, 800);
assert_eq!(claim_txn[6].input.len(), 1);
assert_eq!(claim_txn[6].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
check_spends!(claim_txn[6], remote_txn[0]);
assert_eq!(remote_txn[0].output[claim_txn[6].input[0].previous_output.vout as usize].value, 900);
assert_eq!(timeout_tx.input.len(), 1);
assert_eq!(timeout_tx.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // HTLC 0 <--> 1, timeout tx
check_spends!(timeout_tx, remote_txn[0]);
assert_eq!(remote_txn[0].output[timeout_tx.input[0].previous_output.vout as usize].value, 900);
let events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 3);
@ -8100,45 +8088,40 @@ fn test_bump_penalty_txn_on_remote_commitment() {
let feerate_preimage;
{
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
// 9 transactions including:
// 1*2 ChannelManager local broadcasts of commitment + HTLC-Success
// 1*3 ChannelManager local broadcasts of commitment + HTLC-Success + HTLC-Timeout
// 2 * HTLC-Success (one RBF bump we'll check later)
// 1 * HTLC-Timeout
assert_eq!(node_txn.len(), 8);
// 5 transactions including:
// local commitment + HTLC-Success
// preimage and timeout sweeps from remote commitment + preimage sweep bump
assert_eq!(node_txn.len(), 5);
assert_eq!(node_txn[0].input.len(), 1);
assert_eq!(node_txn[6].input.len(), 1);
assert_eq!(node_txn[3].input.len(), 1);
assert_eq!(node_txn[4].input.len(), 1);
check_spends!(node_txn[0], remote_txn[0]);
check_spends!(node_txn[6], remote_txn[0]);
check_spends!(node_txn[3], remote_txn[0]);
check_spends!(node_txn[4], remote_txn[0]);
check_spends!(node_txn[1], chan.3);
check_spends!(node_txn[2], node_txn[1]);
if node_txn[0].input[0].previous_output == node_txn[3].input[0].previous_output {
preimage_bump = node_txn[3].clone();
check_spends!(node_txn[3], remote_txn[0]);
assert_eq!(node_txn[1], node_txn[4]);
assert_eq!(node_txn[2], node_txn[5]);
} else {
preimage_bump = node_txn[7].clone();
check_spends!(node_txn[7], remote_txn[0]);
assert_eq!(node_txn[0].input[0].previous_output, node_txn[7].input[0].previous_output);
assert_eq!(node_txn[1], node_txn[3]);
assert_eq!(node_txn[2], node_txn[4]);
}
timeout = node_txn[6].txid();
let index = node_txn[6].input[0].previous_output.vout;
let fee = remote_txn[0].output[index as usize].value - node_txn[6].output[0].value;
feerate_timeout = fee * 1000 / node_txn[6].weight() as u64;
check_spends!(node_txn[1], chan.3); // local commitment
check_spends!(node_txn[2], node_txn[1]); // local HTLC-Success
preimage = node_txn[0].txid();
let index = node_txn[0].input[0].previous_output.vout;
let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value;
feerate_preimage = fee * 1000 / node_txn[0].weight() as u64;
let (preimage_bump_tx, timeout_tx) = if node_txn[3].input[0].previous_output == node_txn[0].input[0].previous_output {
(node_txn[3].clone(), node_txn[4].clone())
} else {
(node_txn[4].clone(), node_txn[3].clone())
};
preimage_bump = preimage_bump_tx;
check_spends!(preimage_bump, remote_txn[0]);
assert_eq!(node_txn[0].input[0].previous_output, preimage_bump.input[0].previous_output);
timeout = timeout_tx.txid();
let index = timeout_tx.input[0].previous_output.vout;
let fee = remote_txn[0].output[index as usize].value - timeout_tx.output[0].value;
feerate_timeout = fee * 1000 / timeout_tx.weight() as u64;
node_txn.clear();
};
assert_ne!(feerate_timeout, 0);
@ -9014,11 +8997,8 @@ fn test_concurrent_monitor_claim() {
watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![bob_state_y.clone()] }, CHAN_CONFIRM_DEPTH + 2 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
{
let htlc_txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
// We broadcast twice the transaction, once due to the HTLC-timeout, once due
// the onchain detection of the HTLC output
assert_eq!(htlc_txn.len(), 2);
assert_eq!(htlc_txn.len(), 1);
check_spends!(htlc_txn[0], bob_state_y);
check_spends!(htlc_txn[1], bob_state_y);
}
}

View file

@ -508,13 +508,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
expect_payment_sent!(nodes[0], payment_preimage_1);
connect_blocks(&nodes[0], TEST_FINAL_CLTV*4 + 20);
let as_htlc_timeout_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
assert_eq!(as_htlc_timeout_txn.len(), 3);
let (first_htlc_timeout_tx, second_htlc_timeout_tx) = if as_htlc_timeout_txn[0] == as_commitment_tx {
(&as_htlc_timeout_txn[1], &as_htlc_timeout_txn[2])
} else {
assert_eq!(as_htlc_timeout_txn[2], as_commitment_tx);
(&as_htlc_timeout_txn[0], &as_htlc_timeout_txn[1])
};
assert_eq!(as_htlc_timeout_txn.len(), 2);
let (first_htlc_timeout_tx, second_htlc_timeout_tx) = (&as_htlc_timeout_txn[0], &as_htlc_timeout_txn[1]);
check_spends!(first_htlc_timeout_tx, as_commitment_tx);
check_spends!(second_htlc_timeout_tx, as_commitment_tx);
if first_htlc_timeout_tx.input[0].previous_output == bs_htlc_claim_txn[0].input[0].previous_output {