Skip to content

Commit

Permalink
Use correct input sequence for HTLC claims from counterparty commitments
Browse files Browse the repository at this point in the history
HTLC outputs, like the `to_remote` output, in commitment transactions
with anchor outputs also have an additional `1 CSV` constraint on the
counterparty. When spending such outputs, their corresponding input
needs to have their sequence set to 1. This was done for HTLC claims
from holder commitments, but unfortunately not for counterparty
commitments as we were lacking test coverage.
  • Loading branch information
wpaulino committed Sep 27, 2023
1 parent 1ac53ed commit 3c83783
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 23 deletions.
35 changes: 28 additions & 7 deletions lightning/src/chain/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,32 @@ impl PackageSolvingData {
_ => { mem::discriminant(self) == mem::discriminant(&input) }
}
}
fn as_tx_input(&self, previous_output: BitcoinOutPoint) -> TxIn {
let sequence = match self {
PackageSolvingData::RevokedOutput(_) => Sequence::ENABLE_RBF_NO_LOCKTIME,
PackageSolvingData::RevokedHTLCOutput(_) => Sequence::ENABLE_RBF_NO_LOCKTIME,
PackageSolvingData::CounterpartyOfferedHTLCOutput(outp) => if outp.channel_type_features.supports_anchors_zero_fee_htlc_tx() {
Sequence::from_consensus(1)
} else {
Sequence::ENABLE_RBF_NO_LOCKTIME
},
PackageSolvingData::CounterpartyReceivedHTLCOutput(outp) => if outp.channel_type_features.supports_anchors_zero_fee_htlc_tx() {
Sequence::from_consensus(1)
} else {
Sequence::ENABLE_RBF_NO_LOCKTIME
},
_ => {
debug_assert!(false, "This should not be reachable by 'untractable' or 'malleable with external funding' packages");
Sequence::ENABLE_RBF_NO_LOCKTIME
},
};
TxIn {
previous_output,
script_sig: Script::new(),
sequence,
witness: Witness::new(),
}
}
fn finalize_input<Signer: WriteableEcdsaChannelSigner>(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler<Signer>) -> bool {
match self {
PackageSolvingData::RevokedOutput(ref outp) => {
Expand Down Expand Up @@ -895,13 +921,8 @@ impl PackageTemplate {
value,
}],
};
for (outpoint, _) in self.inputs.iter() {
bumped_tx.input.push(TxIn {
previous_output: *outpoint,
script_sig: Script::new(),
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
witness: Witness::new(),
});
for (outpoint, outp) in self.inputs.iter() {
bumped_tx.input.push(outp.as_tx_input(*outpoint));
}
for (i, (outpoint, out)) in self.inputs.iter().enumerate() {
log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
Expand Down
70 changes: 54 additions & 16 deletions lightning/src/ln/monitor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1867,23 +1867,35 @@ fn test_yield_anchors_events() {
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config), Some(anchors_config)]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let chan_id = create_announced_chan_between_nodes_with_value(
let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(
&nodes, 0, 1, 1_000_000, 500_000_000
).2;
route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000);
);
let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[1], &[&nodes[0]], 2_000_000);

assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

*nodes[0].fee_estimator.sat_per_kw.lock().unwrap() *= 2;

connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
check_closed_broadcast!(&nodes[0], true);
assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());

connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
{
let txn = nodes[1].tx_broadcaster.txn_broadcast();
assert_eq!(txn.len(), 1);
check_spends!(txn[0], funding_tx);
}

get_monitor!(nodes[0], chan_id).provide_payment_preimage(
&payment_hash, &payment_preimage, &node_cfgs[0].tx_broadcaster,
&payment_hash_2, &payment_preimage_2, &node_cfgs[0].tx_broadcaster,
&LowerBoundedFeeEstimator::new(node_cfgs[0].fee_estimator), &nodes[0].logger
);
get_monitor!(nodes[1], chan_id).provide_payment_preimage(
&payment_hash_1, &payment_preimage_1, &node_cfgs[0].tx_broadcaster,
&LowerBoundedFeeEstimator::new(node_cfgs[1].fee_estimator), &nodes[1].logger
);

let mut holder_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
assert_eq!(holder_events.len(), 1);
Expand All @@ -1904,27 +1916,50 @@ fn test_yield_anchors_events() {
assert_eq!(txn.len(), 2);
let anchor_tx = txn.pop().unwrap();
let commitment_tx = txn.pop().unwrap();
check_spends!(commitment_tx, funding_tx);
check_spends!(anchor_tx, coinbase_tx, commitment_tx);
(commitment_tx, anchor_tx)
},
_ => panic!("Unexpected event"),
};

assert_eq!(commitment_tx.output[2].value, 1_000); // HTLC A -> B
assert_eq!(commitment_tx.output[3].value, 2_000); // HTLC B -> A

mine_transactions(&nodes[0], &[&commitment_tx, &anchor_tx]);
check_added_monitors!(nodes[0], 1);
mine_transactions(&nodes[1], &[&commitment_tx, &anchor_tx]);
check_added_monitors!(nodes[1], 1);

{
let mut txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
assert_eq!(txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 3 } else { 2 });

let htlc_preimage_tx = txn.pop().unwrap();
assert_eq!(htlc_preimage_tx.input.len(), 1);
assert_eq!(htlc_preimage_tx.input[0].previous_output.vout, 3);
check_spends!(htlc_preimage_tx, commitment_tx);

let htlc_timeout_tx = txn.pop().unwrap();
assert_eq!(htlc_timeout_tx.input.len(), 1);
assert_eq!(htlc_timeout_tx.input[0].previous_output.vout, 2);
check_spends!(htlc_timeout_tx, commitment_tx);

if let Some(commitment_tx) = txn.pop() {
check_spends!(commitment_tx, funding_tx);
}
}

let mut holder_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
// Certain block `ConnectStyle`s cause an extra `ChannelClose` event to be emitted since the
// best block is updated before the confirmed transactions are notified.
match *nodes[0].connect_style.borrow() {
ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstReorgsOnlyTip|ConnectStyle::BestBlockFirstSkippingBlocks => {
assert_eq!(holder_events.len(), 3);
if let Event::BumpTransaction(BumpTransactionEvent::ChannelClose { .. }) = holder_events.remove(0) {}
else { panic!("unexpected event"); }

},
_ => assert_eq!(holder_events.len(), 2),
};
if nodes[0].connect_style.borrow().updates_best_block_first() {
assert_eq!(holder_events.len(), 3);
if let Event::BumpTransaction(BumpTransactionEvent::ChannelClose { .. }) = holder_events.remove(0) {}
else { panic!("unexpected event"); }
} else {
assert_eq!(holder_events.len(), 2);
}
let mut htlc_txs = Vec::with_capacity(2);
for event in holder_events {
match event {
Expand Down Expand Up @@ -1958,6 +1993,9 @@ fn test_yield_anchors_events() {

// Clear the remaining events as they're not relevant to what we're testing.
nodes[0].node.get_and_clear_pending_events();
nodes[1].node.get_and_clear_pending_events();
nodes[0].node.get_and_clear_pending_msg_events();
nodes[1].node.get_and_clear_pending_msg_events();
}

#[test]
Expand Down

0 comments on commit 3c83783

Please sign in to comment.