Skip to content

Commit 6ac2599

Browse files
tankyleotankyleo
committed
fixup: allocate a USER_COINS_WEIGHT_BUDGET when selecting HTLCs in the batch
1 parent 4b0af69 commit 6ac2599

File tree

2 files changed

+62
-41
lines changed

2 files changed

+62
-41
lines changed

‎lightning/src/events/bump_transaction/mod.rs‎

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -849,8 +849,8 @@ where
849849
/// Handles a [`BumpTransactionEvent::HTLCResolution`] event variant by producing a
850850
/// fully-signed, fee-bumped HTLC transaction that is broadcast to the network.
851851
async fn handle_htlc_resolution(
852-
&self, target_feerate_sat_per_1000_weight: u32, htlc_descriptors: &[HTLCDescriptor],
853-
tx_lock_time: LockTime,
852+
&self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32,
853+
htlc_descriptors: &[HTLCDescriptor], tx_lock_time: LockTime,
854854
) -> Result<(), ()> {
855855
let channel_type = &htlc_descriptors[0]
856856
.channel_derivation_parameters
@@ -885,29 +885,14 @@ where
885885
// and 483 * 705 ~= 341_000, and 341_000 < 400_000.
886886
chan_utils::MAX_STANDARD_TX_WEIGHT
887887
};
888+
// A 1-input 1-output transaction, both p2wpkh is 438 WU.
889+
// If the coins go above this limit, it's ok we pop a few HTLCs and try again.
890+
const USER_COINS_WEIGHT_BUDGET: u64 = 1000;
891+
888892
let mut broadcasted_htlcs = 0;
889893
let mut batch_size = htlc_descriptors.len() - broadcasted_htlcs;
890894

891895
while broadcasted_htlcs < htlc_descriptors.len() {
892-
let htlcs = &htlc_descriptors[broadcasted_htlcs..broadcasted_htlcs + batch_size];
893-
// Generate a new claim_id to map a user-provided utxo to this
894-
// particular set of HTLCs via `select_confirmed_utxos`.
895-
//
896-
// This matches the scheme used in `onchaintx.rs`, so for non-0fc-channels,
897-
// this should match the `ClaimId` of the claim generated in `onchaintx.rs`.
898-
let mut engine = Sha256::engine();
899-
for htlc in htlcs {
900-
engine.input(&htlc.commitment_txid.to_byte_array());
901-
engine.input(&htlc.htlc.transaction_output_index.unwrap().to_be_bytes());
902-
}
903-
let utxo_id = ClaimId(Sha256::from_engine(engine).to_byte_array());
904-
log_info!(
905-
self.logger,
906-
"Batch transaction assigned to UTXO id {} contains {} HTLCs: {}",
907-
log_bytes!(utxo_id.0),
908-
htlcs.len(),
909-
log_iter!(htlcs.iter().map(|d| d.outpoint()))
910-
);
911896
let mut htlc_tx = Transaction {
912897
version: if channel_type.supports_anchor_zero_fee_commitments() {
913898
Version::non_standard(3)
@@ -918,8 +903,20 @@ where
918903
input: vec![],
919904
output: vec![],
920905
};
921-
let mut must_spend = Vec::with_capacity(htlcs.len());
922-
for htlc_descriptor in htlcs {
906+
let mut must_spend = Vec::with_capacity(htlc_descriptors.len() - broadcasted_htlcs);
907+
let mut htlc_weight_sum = 0;
908+
for htlc_descriptor in
909+
&htlc_descriptors[broadcasted_htlcs..broadcasted_htlcs + batch_size]
910+
{
911+
let input_output_weight = if htlc_descriptor.preimage.is_some() {
912+
chan_utils::htlc_success_input_output_pair_weight(channel_type)
913+
} else {
914+
chan_utils::htlc_timeout_input_output_pair_weight(channel_type)
915+
};
916+
if htlc_weight_sum + input_output_weight >= max_weight - USER_COINS_WEIGHT_BUDGET {
917+
break;
918+
}
919+
htlc_weight_sum += input_output_weight;
923920
let htlc_input = htlc_descriptor.unsigned_tx_input();
924921
must_spend.push(Input {
925922
outpoint: htlc_input.previous_output.clone(),
@@ -935,6 +932,30 @@ where
935932
let htlc_output = htlc_descriptor.tx_output(&self.secp);
936933
htlc_tx.output.push(htlc_output);
937934
}
935+
batch_size = htlc_tx.input.len();
936+
let selected_htlcs =
937+
&htlc_descriptors[broadcasted_htlcs..broadcasted_htlcs + batch_size];
938+
939+
let utxo_id = if broadcasted_htlcs == 0 {
940+
claim_id
941+
} else {
942+
// Generate a new claim_id to map a user-provided utxo to this
943+
// particular set of HTLCs via `select_confirmed_utxos`.
944+
let mut engine = Sha256::engine();
945+
for htlc in selected_htlcs {
946+
engine.input(&htlc.commitment_txid.to_byte_array());
947+
engine.input(&htlc.htlc.transaction_output_index.unwrap().to_be_bytes());
948+
}
949+
ClaimId(Sha256::from_engine(engine).to_byte_array())
950+
};
951+
952+
log_info!(
953+
self.logger,
954+
"Batch transaction assigned to UTXO id {} contains {} HTLCs: {}",
955+
log_bytes!(utxo_id.0),
956+
batch_size,
957+
log_iter!(selected_htlcs.iter().map(|d| d.outpoint()))
958+
);
938959

939960
log_debug!(
940961
self.logger,
@@ -988,7 +1009,7 @@ where
9881009
// construct psbt
9891010
let mut htlc_psbt = Psbt::from_unsigned_tx(htlc_tx).unwrap();
9901011
// add witness_utxo to htlc inputs
991-
for (i, htlc_descriptor) in htlcs.iter().enumerate() {
1012+
for (i, htlc_descriptor) in selected_htlcs.iter().enumerate() {
9921013
debug_assert_eq!(
9931014
htlc_psbt.unsigned_tx.input[i].previous_output,
9941015
htlc_descriptor.outpoint()
@@ -999,7 +1020,7 @@ where
9991020
// add witness_utxo to remaining inputs
10001021
for (idx, utxo) in coin_selection.confirmed_utxos.into_iter().enumerate() {
10011022
// offset to skip the htlc inputs
1002-
let index = idx + htlcs.len();
1023+
let index = idx + selected_htlcs.len();
10031024
debug_assert_eq!(htlc_psbt.unsigned_tx.input[index].previous_output, utxo.outpoint);
10041025
if utxo.output.script_pubkey.is_witness_program() {
10051026
htlc_psbt.inputs[index].witness_utxo = Some(utxo.output);
@@ -1014,7 +1035,7 @@ where
10141035
let mut htlc_tx = self.utxo_source.sign_psbt(htlc_psbt).await?;
10151036

10161037
let mut signers = BTreeMap::new();
1017-
for (idx, htlc_descriptor) in htlcs.iter().enumerate() {
1038+
for (idx, htlc_descriptor) in selected_htlcs.iter().enumerate() {
10181039
let keys_id = htlc_descriptor.channel_derivation_parameters.keys_id;
10191040
let signer = signers
10201041
.entry(keys_id)
@@ -1101,6 +1122,7 @@ where
11011122
log_iter!(htlc_descriptors.iter().map(|d| d.outpoint()))
11021123
);
11031124
self.handle_htlc_resolution(
1125+
*claim_id,
11041126
*target_feerate_sat_per_1000_weight,
11051127
htlc_descriptors,
11061128
*tx_lock_time,

‎lightning/src/ln/zero_fee_commitment_tests.rs‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::events::{ClosureReason, Event};
2-
use crate::ln::chan_utils::shared_anchor_script_pubkey;
2+
use crate::ln::chan_utils;
33
use crate::ln::functional_test_utils::*;
44
use crate::ln::msgs::BaseMessageHandler;
55

@@ -48,10 +48,10 @@ fn test_p2a_anchor_values_under_trims_and_rounds() {
4848
)*
4949
let txn = get_local_commitment_txn!(nodes[0], chan_id);
5050
assert_eq!(txn.len(), 1);
51-
assert_eq!(txn[0].output.iter().find(|output| output.script_pubkey == shared_anchor_script_pubkey()).unwrap().value.to_sat(), $expected_p2a_value_sat);
51+
assert_eq!(txn[0].output.iter().find(|output| output.script_pubkey == chan_utils::shared_anchor_script_pubkey()).unwrap().value.to_sat(), $expected_p2a_value_sat);
5252
let txn = get_local_commitment_txn!(nodes[1], chan_id);
5353
assert_eq!(txn.len(), 1);
54-
assert_eq!(txn[0].output.iter().find(|output| output.script_pubkey == shared_anchor_script_pubkey()).unwrap().value.to_sat(), $expected_p2a_value_sat);
54+
assert_eq!(txn[0].output.iter().find(|output| output.script_pubkey == chan_utils::shared_anchor_script_pubkey()).unwrap().value.to_sat(), $expected_p2a_value_sat);
5555
for hash in node_0_1_hashes {
5656
fail_payment(&nodes[0], &[&nodes[1]], hash);
5757
}
@@ -160,10 +160,10 @@ fn test_htlc_claim_chunking() {
160160
check_spends!(htlc_claims[0], node_1_commit_tx[0], coinbase_tx);
161161
check_spends!(htlc_claims[1], node_1_commit_tx[0], coinbase_tx);
162162

163-
assert_eq!(htlc_claims[0].input.len(), 59);
164-
assert_eq!(htlc_claims[0].output.len(), 59);
165-
assert_eq!(htlc_claims[1].input.len(), 18);
166-
assert_eq!(htlc_claims[1].output.len(), 18);
163+
assert_eq!(htlc_claims[0].input.len(), 60);
164+
assert_eq!(htlc_claims[0].output.len(), 60);
165+
assert_eq!(htlc_claims[1].input.len(), 17);
166+
assert_eq!(htlc_claims[1].output.len(), 17);
167167

168168
check_closed_broadcast!(nodes[0], true);
169169
check_added_monitors!(nodes[0], 1);
@@ -202,23 +202,22 @@ fn test_htlc_claim_chunking() {
202202
let fresh_htlc_claims = nodes[1].tx_broadcaster.txn_broadcast();
203203
assert_eq!(fresh_htlc_claims.len(), 1);
204204
check_spends!(fresh_htlc_claims[0], node_1_commit_tx[0], htlc_claims[0]);
205-
assert_eq!(fresh_htlc_claims[0].input.len(), 18);
206-
assert_eq!(fresh_htlc_claims[0].output.len(), 18);
205+
assert_eq!(fresh_htlc_claims[0].input.len(), 17);
206+
assert_eq!(fresh_htlc_claims[0].output.len(), 17);
207207

208-
// Assert when we handled the second `BumpTransaction::HTLCResolution` event, we
209-
// assigned the same UTXO id to that set of HTLCs
210208
let log_entries = &nodes[1].logger.lines.lock().unwrap();
211-
let keys: Vec<_> = log_entries
209+
let mut keys: Vec<_> = log_entries
212210
.keys()
213211
.filter(|key| key.1.contains("Batch transaction assigned to UTXO id"))
214212
.collect();
215213
assert_eq!(keys.len(), 3);
216-
let mut values = vec![
214+
keys.sort_unstable();
215+
assert_eq!(keys[0].1.split_whitespace().nth(6), keys[1].1.split_whitespace().nth(6));
216+
assert_ne!(keys[0].1.split_whitespace().nth(6), keys[2].1.split_whitespace().nth(6));
217+
let values = vec![
217218
*log_entries.get(keys[0]).unwrap(),
218219
*log_entries.get(keys[1]).unwrap(),
219220
*log_entries.get(keys[2]).unwrap(),
220221
];
221-
let pos = values.iter().position(|&value| value == 2).unwrap();
222-
values.remove(pos);
223222
assert!(values.iter().all(|&value| value == 1));
224223
}

0 commit comments

Comments
 (0)