Skip to content

Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheBlueMatt merged 3 commits into from
Jan 30, 2025
Merged

Set correct counterparty_spendable_height on c.p. revoked HTLCs #3564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e93622b
Drop `counterparty_spendable_height` accessor
TheBlueMatt Jan 27, 2025
5eb0a34
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs
TheBlueMatt Jan 27, 2025
6c57a1f
Mark counterparty revoked offered HTLCs as `Unpinnable`
TheBlueMatt Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
return (claimable_outpoints, to_counterparty_output_info);
}
let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
let counterparty_spendable_height = if htlc.offered {
htlc.cltv_expiry
} else {
height
};
let justice_package = PackageTemplate::build_package(
commitment_txid,
transaction_output_index,
PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
);
claimable_outpoints.push(justice_package);
}
Expand Down
46 changes: 29 additions & 17 deletions lightning/src/chain/package.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
match self {
PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
PackageMalleability::Malleable(AggregationCluster::Unpinnable),
PackageSolvingData::RevokedHTLCOutput(..) =>
PackageMalleability::Malleable(AggregationCluster::Pinnable),
PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
if htlc.offered {
PackageMalleability::Malleable(AggregationCluster::Unpinnable)
} else {
PackageMalleability::Malleable(AggregationCluster::Pinnable)
}
},
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
PackageMalleability::Malleable(AggregationCluster::Unpinnable),
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
/// Block height at which our counterparty can potentially claim this output as well (assuming
/// they have the keys or information required to do so).
///
/// This is used primarily by external consumers to decide when an output becomes "pinnable"
/// because the counterparty can potentially spend it. It is also used internally by
/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
/// type of output.
/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
/// identify when an output must be claimed by, depending on the type of output.
///
/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
/// we upgraded from LDK 0.1 or prior.
counterparty_spendable_height: u32,
// Cache of package feerate committed at previous (re)broadcast. If bumping resources
// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
// Now check that we only merge packages if they are both unpinnable or both
// pinnable.
let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
if self_pinnable && other_pinnable {
return true;
}

let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
if self_unpinnable && other_unpinnable {
return true;
}
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
pub(crate) fn is_malleable(&self) -> bool {
matches!(self.malleability, PackageMalleability::Malleable(..))
}
/// The height at which our counterparty may be able to spend this output.
///
/// This is an important limit for aggregation as after this height our counterparty may be
/// able to pin transactions spending this output in the mempool.
pub(crate) fn counterparty_spendable_height(&self) -> u32 {
self.counterparty_spendable_height
}
pub(crate) fn previous_feerate(&self) -> u64 {
self.feerate_previous
}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
(4, _height_original, option), // Written with a dummy value since 0.1
(6, height_timer, option),
});
for (_, input) in &inputs {
if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
// LDK versions through 0.1 set the wrong counterparty_spendable_height for
// non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
// claim with a preimage immediately). Here we detect this and reset the value to
// zero, as the value is unused except for merging decisions which doesn't care
// about any values below the current height.
if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
counterparty_spendable_height = 0;
}
}
}
Ok(PackageTemplate {
inputs,
malleability,
Expand Down
79 changes: 46 additions & 33 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
mine_transaction(&nodes[1], &revoked_local_txn[0]);
{
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
// The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
// be claimed in separate transactions.
assert_eq!(node_txn.len(), 2);
for tx in node_txn.iter() {
assert_eq!(tx.input.len(), 1);
check_spends!(tx, revoked_local_txn[0]);
}
assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
// The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
// thus claimed in the same transaction with the revoked to_self output.
assert_eq!(node_txn.len(), 1);
assert_eq!(node_txn[0].input.len(), 2);
check_spends!(node_txn[0], revoked_local_txn[0]);
assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
node_txn.clear();
}
check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
assert_eq!(node_txn.len(), 2); // Two penalty transactions:
assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
check_spends!(node_txn[0], revoked_local_txn[0]);
check_spends!(node_txn[1], revoked_local_txn[0]);
assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

// The ChannelMonitor should claim the accepted HTLC output separately from the offered
// HTLC and to_self outputs.
let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
check_spends!(accepted_claim, revoked_local_txn[0]);
check_spends!(offered_to_self_claim, revoked_local_txn[0]);
assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

let mut witness_lens = BTreeSet::new();
witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
assert_eq!(witness_lens.len(), 3);
witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
assert_eq!(witness_lens.len(), 2);
assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

// Finally, mine the penalty transactions and check that we get an HTLC failure after
// Finally, mine the penalty transaction and check that we get an HTLC failure after
// ANTI_REORG_DELAY confirmations.
mine_transaction(&nodes[1], &node_txn[0]);
mine_transaction(&nodes[1], &node_txn[1]);
mine_transaction(&nodes[1], accepted_claim);
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
expect_payment_failed!(nodes[1], payment_hash_2, false);
}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

if split_tx {
connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
}

mine_transaction(&nodes[1], &revoked_local_txn[0]);
check_closed_broadcast!(nodes[1], true);
check_added_monitors!(nodes[1], 1);
check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
// in separate transactions.
// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
// separately.
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
assert_eq!(node_txn.len(), 2);
assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
for tx in node_txn.iter() {
assert_eq!(tx.input.len(), 1);
assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
check_spends!(tx, revoked_local_txn[0]);
}
assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
if split_tx {
assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
}

mine_transaction(&nodes[1], &node_txn[0]);
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
// locktime.
connect_blocks(&nodes[1], TEST_FINAL_CLTV);
// B will generate justice tx from A's revoked commitment/HTLC tx
connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
check_closed_broadcast!(nodes[1], true);
Expand Down
10 changes: 7 additions & 3 deletions lightning/src/ln/monitor_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

// First connect blocks until the HTLC expires with
// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
// pinnable claims, which the remainder of the test assumes.
connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
[HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
// A will generate justice tx from B's revoked commitment/HTLC tx
mine_transaction(&nodes[0], &revoked_local_txn[0]);
check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
[HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
// As time goes on A may split its revocation claim transaction into multiple.
let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
for tx in as_fewer_input_rbf.iter() {
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Set correct `counterparty_spendable_height` on c.p. revoked HTLCs by TheBlueMatt · Pull Request #3564 · lightningdevkit/rust-lightning · GitHub
Skip to content
Set correct counterparty_spendable_height on c.p. revoked HTLCs#3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 3 commits into

from

Jan 30, 2025
 Merged


Set correct counterparty_spendable_height on c.p. revoked HTLCs
#3564
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e93622b

Drop `counterparty_spendable_height` accessor


TheBlueMatt Jan 27, 2025
5eb0a34

Set correct `counterparty_spendable_height` on c.p. revoked HTLCs


TheBlueMatt Jan 27, 2025
6c57a1f

Mark counterparty revoked offered HTLCs as `Unpinnable`


TheBlueMatt Jan 27, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


7 changes: 6 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3556,11 +3556,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 return (claimable_outpoints, to_counterparty_output_info);
 }
 let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
 let counterparty_spendable_height = if htlc.offered {
 htlc.cltv_expiry
 } else {
 height
 };
 let justice_package = PackageTemplate::build_package(
 commitment_txid,
 transaction_output_index,
 PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
htlc.cltv_expiry,
counterparty_spendable_height,
 );
 claimable_outpoints.push(justice_package);
 }
Expand Down

46 changes: 29 additions & 17 deletions
lightning/src/chain/package.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -699,8 +699,13 @@ impl PackageSolvingData {
 match self {
 PackageSolvingData::RevokedOutput(RevokedOutput { .. }) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::RevokedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Pinnable),
 PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) => {
 if htlc.offered {
 PackageMalleability::Malleable(AggregationCluster::Unpinnable)
 } else {
 PackageMalleability::Malleable(AggregationCluster::Pinnable)
 }
 },
 PackageSolvingData::CounterpartyOfferedHTLCOutput(..) =>
 PackageMalleability::Malleable(AggregationCluster::Unpinnable),
 PackageSolvingData::CounterpartyReceivedHTLCOutput(..) =>
Expand DownExpand Up@@ -771,10 +776,12 @@ pub struct PackageTemplate {
	/// Block height at which our counterparty can potentially claim this output as well (assuming
	/// they have the keys or information required to do so).
	///
	/// This is used primarily by external consumers to decide when an output becomes "pinnable"
	/// because the counterparty can potentially spend it. It is also used internally by
	/// [`Self::get_height_timer`] to identify when an output must be claimed by, depending on the
	/// type of output.
	/// This is used primarily to decide when an output becomes "pinnable" because the counterparty
	/// can potentially spend it. It is also used internally by [`Self::get_height_timer`] to
	/// identify when an output must be claimed by, depending on the type of output.
	///
	/// Note that for revoked counterparty HTLC outputs the value may be zero in some cases where
	/// we upgraded from LDK 0.1 or prior.
	counterparty_spendable_height: u32,
	// Cache of package feerate committed at previous (re)broadcast. If bumping resources
	// (either claimed output value or external utxo), it will keep increasing until holder
Expand DownExpand Up@@ -834,17 +841,17 @@ impl PackageTemplate {
 // Now check that we only merge packages if they are both unpinnable or both
 // pinnable.
 let self_pinnable = self_cluster == AggregationCluster::Pinnable ||
 self.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_pinnable = other_cluster == AggregationCluster::Pinnable ||
 other.counterparty_spendable_height() <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height <= cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_pinnable && other_pinnable {
 return true;
 }

 let self_unpinnable = self_cluster == AggregationCluster::Unpinnable &&
 self.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 self.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 let other_unpinnable = other_cluster == AggregationCluster::Unpinnable &&
 other.counterparty_spendable_height() > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 other.counterparty_spendable_height > cur_height + COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE;
 if self_unpinnable && other_unpinnable {
 return true;
 }
Expand All@@ -855,13 +862,6 @@ impl PackageTemplate {
	pub(crate) fn is_malleable(&self) -> bool {
 matches!(self.malleability, PackageMalleability::Malleable(..))
	}
	/// The height at which our counterparty may be able to spend this output.
	///
	/// This is an important limit for aggregation as after this height our counterparty may be
	/// able to pin transactions spending this output in the mempool.
	pub(crate) fn counterparty_spendable_height(&self) -> u32 {
 self.counterparty_spendable_height
	}
	pub(crate) fn previous_feerate(&self) -> u64 {
 self.feerate_previous
	}
Expand DownExpand Up@@ -1225,6 +1225,18 @@ impl Readable for PackageTemplate {
 (4, _height_original, option), // Written with a dummy value since 0.1
 (6, height_timer, option),
 });
 for (_, input) in &inputs {
 if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput { htlc, .. }) = input {
 // LDK versions through 0.1 set the wrong counterparty_spendable_height for
 // non-offered revoked HTLCs (ie HTLCs we sent to our counterparty which they can
 // claim with a preimage immediately). Here we detect this and reset the value to
 // zero, as the value is unused except for merging decisions which doesn't care
 // about any values below the current height.
 if !htlc.offered && htlc.cltv_expiry == counterparty_spendable_height {
 counterparty_spendable_height = 0;
 }
 }
 }
 Ok(PackageTemplate {
 inputs,
 malleability,
Expand Down

79 changes: 46 additions & 33 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@ use crate::chain;
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
Expand DownExpand Up@@ -2509,14 +2509,12 @@ fn test_justice_tx_htlc_timeout() {
 mine_transaction(&nodes[1], &revoked_local_txn[0]);
 {
 let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 // The unpinnable, revoked to_self output, and the pinnable, revoked htlc output will
 // be claimed in separate transactions.
 assert_eq!(node_txn.len(), 2);
 for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 check_spends!(tx, revoked_local_txn[0]);
 }
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 // The revoked HTLC output is not pinnable for another `TEST_FINAL_CLTV` blocks, and is
 // thus claimed in the same transaction with the revoked to_self output.
 assert_eq!(node_txn.len(), 1);
 assert_eq!(node_txn[0].input.len(), 2);
 check_spends!(node_txn[0], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[0].input[1].previous_output);
 node_txn.clear();
 }
 check_added_monitors!(nodes[1], 1);
Expand DownExpand Up@@ -2736,28 +2734,26 @@ fn claim_htlc_outputs() {
 assert!(nodes[1].node.get_and_clear_pending_events().is_empty());

 let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 assert_eq!(node_txn.len(), 2); // Two penalty transactions:
 assert_eq!(node_txn[0].input.len(), 1); // Claims the unpinnable, revoked output.
 assert_eq!(node_txn[1].input.len(), 2); // Claims both pinnable, revoked HTLC outputs separately.
 check_spends!(node_txn[0], revoked_local_txn[0]);
 check_spends!(node_txn[1], revoked_local_txn[0]);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_ne!(node_txn[1].input[0].previous_output, node_txn[1].input[1].previous_output);
 assert_eq!(node_txn.len(), 2); // ChannelMonitor: penalty txn

 // The ChannelMonitor should claim the accepted HTLC output separately from the offered
 // HTLC and to_self outputs.
 let accepted_claim = node_txn.iter().filter(|tx| tx.input.len() == 1).next().unwrap();
 let offered_to_self_claim = node_txn.iter().filter(|tx| tx.input.len() == 2).next().unwrap();
 check_spends!(accepted_claim, revoked_local_txn[0]);
 check_spends!(offered_to_self_claim, revoked_local_txn[0]);
 assert_eq!(accepted_claim.input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);

 let mut witness_lens = BTreeSet::new();
 witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
 witness_lens.insert(node_txn[1].input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 3);
 witness_lens.insert(offered_to_self_claim.input[0].witness.last().unwrap().len());
 witness_lens.insert(offered_to_self_claim.input[1].witness.last().unwrap().len());
 assert_eq!(witness_lens.len(), 2);
 assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT); // revoked offered HTLC
 assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
 assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), OFFERED_HTLC_SCRIPT_WEIGHT);

 // Finally, mine the penalty transactions and check that we get an HTLC failure after
 // Finally, mine the penalty transaction and check that we get an HTLC failure after
 // ANTI_REORG_DELAY confirmations.
 mine_transaction(&nodes[1], &node_txn[0]);
 mine_transaction(&nodes[1], &node_txn[1]);
 mine_transaction(&nodes[1], accepted_claim);
 connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
 expect_payment_failed!(nodes[1], payment_hash_2, false);
	}
Expand DownExpand Up@@ -4920,8 +4916,7 @@ fn test_static_spendable_outputs_timeout_tx() {
	check_spends!(spend_txn[2], node_txn[0], commitment_tx[0]); // All outputs
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
fn do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(split_tx: bool) {
	let chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
Expand All@@ -4937,20 +4932,28 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {

	claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

	if split_tx {
 connect_blocks(&nodes[1], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE + 1);
	}

	mine_transaction(&nodes[1], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[1], true);
	check_added_monitors!(nodes[1], 1);
	check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);

	// The unpinnable, revoked to_self output and the pinnable, revoked HTLC output will be claimed
	// in separate transactions.
	// If the HTLC expires in more than COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE blocks, we'll
	// claim both the revoked and HTLC outputs in one transaction, otherwise we'll split them as we
	// consider the HTLC output as pinnable and want to claim pinnable and unpinnable outputs
	// separately.
	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
	assert_eq!(node_txn.len(), 2);
	assert_eq!(node_txn.len(), if split_tx { 2 } else { 1 });
	for tx in node_txn.iter() {
 assert_eq!(tx.input.len(), 1);
 assert_eq!(tx.input.len(), if split_tx { 1 } else { 2 });
 check_spends!(tx, revoked_local_txn[0]);
	}
	assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	if split_tx {
 assert_ne!(node_txn[0].input[0].previous_output, node_txn[1].input[0].previous_output);
	}

	mine_transaction(&nodes[1], &node_txn[0]);
	connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
Expand All@@ -4960,6 +4963,12 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	check_spends!(spend_txn[0], node_txn[0]);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(true);
	do_test_static_spendable_outputs_justice_tx_revoked_commitment_tx(false);
}

#[test]
fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
Expand DownExpand Up@@ -4992,6 +5001,10 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
	check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]);
	assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout

	// In order to connect `revoked_htlc_txn[0]` we must first advance the chain by
	// `TEST_FINAL_CLTV` blocks as otherwise the transaction is consensus-invalid due to its
	// locktime.
	connect_blocks(&nodes[1], TEST_FINAL_CLTV);
	// B will generate justice tx from A's revoked commitment/HTLC tx
	connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()]));
	check_closed_broadcast!(nodes[1], true);
Expand Down

10 changes: 7 additions & 3 deletions
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand DownExpand Up@@ -1734,6 +1734,12 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
	assert_eq!(revoked_htlc_success.lock_time, LockTime::ZERO);
	assert_ne!(revoked_htlc_timeout.lock_time, LockTime::ZERO);

	// First connect blocks until the HTLC expires with
	// `COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE` blocks, making us consider all the HTLCs
	// pinnable claims, which the remainder of the test assumes.
	connect_blocks(&nodes[0], TEST_FINAL_CLTV - COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// A will generate justice tx from B's revoked commitment/HTLC tx
	mine_transaction(&nodes[0], &revoked_local_txn[0]);
	check_closed_broadcast!(nodes[0], true);
Expand DownExpand Up@@ -1846,8 +1852,6 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) {
 sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()));

	connect_blocks(&nodes[0], revoked_htlc_timeout.lock_time.to_consensus_u32() - nodes[0].best_block_info().1);
	expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(&nodes[0],
 [HTLCDestination::FailedPayment { payment_hash: failed_payment_hash }]);
	// As time goes on A may split its revocation claim transaction into multiple.
	let as_fewer_input_rbf = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
	for tx in as_fewer_input_rbf.iter() {
Expand Down