Skip to content

Commit 15d1b03

Browse files
author
Antoine Riard
committed
Implement bumping engine in ChannelMonitor::block_connected
Add RBF-bumping of justice txn, given they are only signed by us we can RBF at wish. Aggregation of bump-candidates and more aggresive bumping heuristics are left open Fix tests broken by introduction of more txn broadcast. Some tests may have a relaxed check (claim_htlc_ouputs_single_tx) as broadcast bumped txn are now interwining in previous broadcast ones and breaking simple expectations Use bumping engine to rebuild claiming transaction in case of partial- claim of its outpoints set.
1 parent 0af58a6 commit 15d1b03

File tree

2 files changed

+167
-27
lines changed

2 files changed

+167
-27
lines changed

‎lightning/src/ln/channelmonitor.rs

Lines changed: 161 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use ln::chan_utils;
3434
use ln::chan_utils::HTLCOutputInCommitment;
3535
use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
3636
use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
37-
use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator, ConfirmationTarget};
37+
use chain::chaininterface::{ChainListener, ChainWatchInterface, BroadcasterInterface, FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
3838
use chain::transaction::OutPoint;
3939
use chain::keysinterface::SpendableOutputDescriptor;
4040
use util::logger::Logger;
@@ -506,7 +506,7 @@ pub struct ChannelMonitor {
506506
}
507507

508508
macro_rules! subtract_high_prio_fee {
509-
($self: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $spent_txid: expr, $used_feerate: expr) => {
509+
($self: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $used_feerate: expr) => {
510510
{
511511
$used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority);
512512
let mut fee = $used_feerate * ($predicted_weight as u64) / 1000;
@@ -517,18 +517,18 @@ macro_rules! subtract_high_prio_fee {
517517
$used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
518518
fee = $used_feerate * ($predicted_weight as u64) / 1000;
519519
if $value <= fee {
520-
log_error!($self, "Failed to generate an on-chain punishment tx spending {} as even low priority fee ({} sat) was more than the entire claim balance ({} sat)",
521-
$spent_txid, fee, $value);
520+
log_error!($self, "Failed to generate an on-chain punishment tx as even low priority fee ({} sat) was more than the entire claim balance ({} sat)",
521+
fee, $value);
522522
false
523523
} else {
524-
log_warn!($self, "Used low priority fee for on-chain punishment tx spending {} as high priority fee was more than the entire claim balance ({} sat)",
525-
$spent_txid, $value);
524+
log_warn!($self, "Used low priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
525+
$value);
526526
$value -= fee;
527527
true
528528
}
529529
} else {
530-
log_warn!($self, "Used medium priority fee for on-chain punishment tx spending {} as high priority fee was more than the entire claim balance ({} sat)",
531-
$spent_txid, $value);
530+
log_warn!($self, "Used medium priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
531+
$value);
532532
$value -= fee;
533533
true
534534
}
@@ -1439,7 +1439,7 @@ impl ChannelMonitor {
14391439
let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::RevokedOfferedHTLC } else { InputDescriptors::RevokedReceivedHTLC }]);
14401440
let height_timer = Self::get_height_timer(height, htlc.cltv_expiry);
14411441
let mut used_feerate;
1442-
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1442+
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, used_feerate) {
14431443
let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
14441444
let (redeemscript, revocation_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
14451445
assert!(predicted_weight >= single_htlc_tx.get_weight());
@@ -1517,7 +1517,7 @@ impl ChannelMonitor {
15171517
let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&inputs_desc[..]);
15181518

15191519
let mut used_feerate;
1520-
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1520+
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, used_feerate) {
15211521
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs);
15221522
}
15231523

@@ -1712,7 +1712,7 @@ impl ChannelMonitor {
17121712
let predicted_weight = single_htlc_tx.get_weight() + Self::get_witnesses_weight(&[if htlc.offered { InputDescriptors::OfferedHTLC } else { InputDescriptors::ReceivedHTLC }]);
17131713
let height_timer = Self::get_height_timer(height, htlc.cltv_expiry);
17141714
let mut used_feerate;
1715-
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1715+
if subtract_high_prio_fee!(self, fee_estimator, single_htlc_tx.output[0].value, predicted_weight, used_feerate) {
17161716
let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
17171717
let (redeemscript, htlc_key) = sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
17181718
assert!(predicted_weight >= single_htlc_tx.get_weight());
@@ -1756,7 +1756,7 @@ impl ChannelMonitor {
17561756
let predicted_weight = timeout_tx.get_weight() + Self::get_witnesses_weight(&[InputDescriptors::ReceivedHTLC]);
17571757
let height_timer = Self::get_height_timer(height, htlc.cltv_expiry);
17581758
let mut used_feerate;
1759-
if subtract_high_prio_fee!(self, fee_estimator, timeout_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1759+
if subtract_high_prio_fee!(self, fee_estimator, timeout_tx.output[0].value, predicted_weight, used_feerate) {
17601760
let sighash_parts = bip143::SighashComponents::new(&timeout_tx);
17611761
let (redeemscript, htlc_key) = sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]);
17621762
assert!(predicted_weight >= timeout_tx.get_weight());
@@ -1790,7 +1790,7 @@ impl ChannelMonitor {
17901790
let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&inputs_desc[..]);
17911791

17921792
let mut used_feerate;
1793-
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1793+
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, used_feerate) {
17941794
return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs);
17951795
}
17961796

@@ -1900,7 +1900,7 @@ impl ChannelMonitor {
19001900
};
19011901
let predicted_weight = spend_tx.get_weight() + Self::get_witnesses_weight(&[InputDescriptors::RevokedOutput]);
19021902
let mut used_feerate;
1903-
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, tx.txid(), used_feerate) {
1903+
if !subtract_high_prio_fee!(self, fee_estimator, spend_tx.output[0].value, predicted_weight, used_feerate) {
19041904
return (None, None);
19051905
}
19061906

@@ -2184,6 +2184,7 @@ impl ChannelMonitor {
21842184
let mut watch_outputs = Vec::new();
21852185
let mut spendable_outputs = Vec::new();
21862186
let mut htlc_updated = Vec::new();
2187+
let mut bump_candidates = Vec::new();
21872188
for tx in txn_matched {
21882189
if tx.input.len() == 1 {
21892190
// Assuming our keys were not leaked (in which case we're screwed no matter what),
@@ -2250,28 +2251,31 @@ impl ChannelMonitor {
22502251
for inp in &tx.input {
22512252
if let Some(ancestor_claimable_txid) = self.claimable_outpoints.get(&inp.previous_output) {
22522253
// If outpoint has claim request pending on it...
2253-
if let Some(claim_material) = self.pending_claim_requests.get(&ancestor_claimable_txid.0) {
2254+
if let Some(claim_material) = self.pending_claim_requests.get_mut(&ancestor_claimable_txid.0) {
22542255
//... we need to verify equality between transaction outpoints and claim request
22552256
// outpoints to know if transaction is the original claim or a bumped one issued
22562257
// by us.
2257-
let mut set_equality = true;
2258-
if claim_material.per_input_material.len() != tx.input.len() {
2259-
set_equality = false;
2260-
}
2258+
let mut claimed_outpoints = Vec::new();
22612259
for (claim_inp, tx_inp) in claim_material.per_input_material.keys().zip(tx.input.iter()) {
22622260
if *claim_inp != tx_inp.previous_output {
2263-
set_equality = false;
2261+
claimed_outpoints.push(tx_inp.previous_output.clone());
22642262
}
22652263
}
2266-
if set_equality { // If true, register claim request to be removed after reaching a block security height
2264+
if claimed_outpoints.len() == 0 && claim_material.per_input_material.len() == tx.input.len() { // If true, register claim request to be removed after reaching a block security height
22672265
match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) {
22682266
hash_map::Entry::Occupied(_) => {},
22692267
hash_map::Entry::Vacant(entry) => {
22702268
entry.insert(vec![OnchainEvent::Claim { claim_request: ancestor_claimable_txid.0.clone()}]);
22712269
}
22722270
}
22732271
} else { // If false, generate new claim request with update outpoint set
2274-
//TODO: use bump engine
2272+
for already_claimed in claimed_outpoints {
2273+
claim_material.per_input_material.remove(&already_claimed);
2274+
}
2275+
// Avoid bump engine using inaccurate feerate due to new transaction size
2276+
claim_material.feerate_previous = 0;
2277+
//TODO: recompute soonest_timelock to avoid wasting a bit on fees
2278+
bump_candidates.push((ancestor_claimable_txid.0.clone(), claim_material.clone()));
22752279
}
22762280
} else {
22772281
panic!("Inconsistencies between pending_claim_requests map and claimable_outpoints map");
@@ -2323,6 +2327,21 @@ impl ChannelMonitor {
23232327
}
23242328
}
23252329
}
2330+
for (ancestor_claim_txid, ref mut cached_claim_datas) in self.pending_claim_requests.iter_mut() {
2331+
if cached_claim_datas.height_timer == height {
2332+
bump_candidates.push((ancestor_claim_txid.clone(), cached_claim_datas.clone()));
2333+
}
2334+
}
2335+
for &mut (_, ref mut cached_claim_datas) in bump_candidates.iter_mut() {
2336+
if let Some((new_timer, new_feerate, bump_tx)) = self.bump_claim_tx(height, &cached_claim_datas, fee_estimator) {
2337+
cached_claim_datas.height_timer = new_timer;
2338+
cached_claim_datas.feerate_previous = new_feerate;
2339+
broadcaster.broadcast_transaction(&bump_tx);
2340+
}
2341+
}
2342+
for (ancestor_claim_txid, cached_claim_datas) in bump_candidates.drain(..) {
2343+
self.pending_claim_requests.insert(ancestor_claim_txid, cached_claim_datas);
2344+
}
23262345
self.last_block_hash = block_hash.clone();
23272346
(watch_outputs, spendable_outputs, htlc_updated)
23282347
}
@@ -2536,6 +2555,126 @@ impl ChannelMonitor {
25362555
}
25372556
htlc_updated
25382557
}
2558+
2559+
/// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration
2560+
/// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent.
2561+
fn bump_claim_tx(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: &FeeEstimator) -> Option<(u32, u64, Transaction)> {
2562+
let mut inputs = Vec::new();
2563+
for outp in cached_claim_datas.per_input_material.keys() {
2564+
inputs.push(TxIn {
2565+
previous_output: *outp,
2566+
script_sig: Script::new(),
2567+
sequence: 0xfffffffd,
2568+
witness: Vec::new(),
2569+
});
2570+
}
2571+
let mut bumped_tx = Transaction {
2572+
version: 2,
2573+
lock_time: 0,
2574+
input: inputs,
2575+
output: vec![TxOut {
2576+
script_pubkey: self.destination_script.clone(),
2577+
value: 0
2578+
}],
2579+
};
2580+
2581+
macro_rules! RBF_bump {
2582+
($amount: expr, $old_feerate: expr, $fee_estimator: expr, $predicted_weight: expr) => {
2583+
{
2584+
let mut used_feerate;
2585+
// If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee...
2586+
let new_fee = if $old_feerate < $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) {
2587+
let mut value = $amount;
2588+
if subtract_high_prio_fee!(self, $fee_estimator, value, $predicted_weight, used_feerate) {
2589+
// Overflow check is done in subtract_high_prio_fee
2590+
$amount - value
2591+
} else {
2592+
log_trace!(self, "Can't new-estimation bump new claiming tx, amount {} is too small", $amount);
2593+
return None;
2594+
}
2595+
// ...else just increase the previous feerate by 25% (because that's a nice number)
2596+
} else {
2597+
let fee = $old_feerate * $predicted_weight / 750;
2598+
if $amount <= fee {
2599+
log_trace!(self, "Can't 25% bump new claiming tx, amount {} is too small", $amount);
2600+
return None;
2601+
}
2602+
fee
2603+
};
2604+
2605+
let previous_fee = $old_feerate * $predicted_weight / 1000;
2606+
let min_relay_fee = MIN_RELAY_FEE_SAT_PER_1000_WEIGHT * $predicted_weight / 1000;
2607+
// BIP 125 Opt-in Full Replace-by-Fee Signaling
2608+
// * 3. The replacement transaction pays an absolute fee of at least the sum paid by the original transactions.
2609+
// * 4. The replacement transaction must also pay for its own bandwidth at or above the rate set by the node's minimum relay fee setting.
2610+
let new_fee = if new_fee < previous_fee + min_relay_fee {
2611+
new_fee + previous_fee + min_relay_fee - new_fee
2612+
} else {
2613+
new_fee
2614+
};
2615+
Some((new_fee, new_fee * 1000 / $predicted_weight))
2616+
}
2617+
}
2618+
}
2619+
2620+
let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
2621+
let mut inputs_witnesses_weight = 0;
2622+
let mut amt = 0;
2623+
for per_outp_material in cached_claim_datas.per_input_material.values() {
2624+
match per_outp_material {
2625+
&InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
2626+
inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if script.len() == OFFERED_HTLC_SCRIPT_WEIGHT { &[InputDescriptors::RevokedOfferedHTLC] } else if script.len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { &[InputDescriptors::RevokedReceivedHTLC] } else { &[] });
2627+
amt += *amount;
2628+
},
2629+
&InputMaterial::RemoteHTLC { .. } => { return None; },
2630+
&InputMaterial::LocalHTLC { .. } => { return None; }
2631+
}
2632+
}
2633+
assert!(amt != 0);
2634+
2635+
let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight;
2636+
let new_feerate;
2637+
if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
2638+
// If new computed fee is superior at the whole claimable amount burn all in fees
2639+
if new_fee > amt {
2640+
bumped_tx.output[0].value = 0;
2641+
} else {
2642+
bumped_tx.output[0].value = amt - new_fee;
2643+
}
2644+
new_feerate = feerate;
2645+
} else {
2646+
return None;
2647+
}
2648+
assert!(new_feerate != 0);
2649+
2650+
for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
2651+
match per_outp_material {
2652+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
2653+
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
2654+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
2655+
let sig = self.secp_ctx.sign(&sighash, &key);
2656+
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
2657+
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
2658+
if *is_htlc {
2659+
bumped_tx.input[i].witness.push(pubkey.unwrap().clone().serialize().to_vec());
2660+
} else {
2661+
bumped_tx.input[i].witness.push(vec!(1));
2662+
}
2663+
bumped_tx.input[i].witness.push(script.clone().into_bytes());
2664+
log_trace!(self, "Going to broadcast bumped Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}", bumped_tx.txid(), if !is_htlc { "to_local" } else if script.len() == OFFERED_HTLC_SCRIPT_WEIGHT { "offered" } else if script.len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
2665+
},
2666+
&InputMaterial::RemoteHTLC { .. } => {},
2667+
&InputMaterial::LocalHTLC { .. } => {
2668+
//TODO : Given that Local Commitment Transaction and HTLC-Timeout/HTLC-Success are counter-signed by peer, we can't
2669+
// RBF them. Need a Lightning specs change and package relay modification :
2670+
// https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
2671+
return None;
2672+
}
2673+
}
2674+
}
2675+
assert!(predicted_weight >= bumped_tx.get_weight());
2676+
Some((new_timer, new_feerate, bumped_tx))
2677+
}
25392678
}
25402679

25412680
const MAX_ALLOC_SIZE: usize = 64*1024;

‎lightning/src/ln/functional_tests.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,6 +1884,7 @@ fn test_justice_tx() {
18841884

18851885
check_spends!(node_txn[0], revoked_local_txn[0].clone());
18861886
node_txn.swap_remove(0);
1887+
node_txn.truncate(1);
18871888
}
18881889
test_txn_broadcast(&nodes[1], &chan_5, None, HTLCType::NONE);
18891890

@@ -1901,6 +1902,10 @@ fn test_justice_tx() {
19011902
// We test justice_tx build by A on B's revoked HTLC-Success tx
19021903
// Create some new channels:
19031904
let chan_6 = create_announced_chan_between_nodes(&nodes, 0, 1, LocalFeatures::new(), LocalFeatures::new());
1905+
{
1906+
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1907+
node_txn.clear();
1908+
}
19041909

19051910
// A pending HTLC which will be revoked:
19061911
let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
@@ -2078,7 +2083,7 @@ fn claim_htlc_outputs_single_tx() {
20782083
}
20792084

20802085
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
2081-
assert_eq!(node_txn.len(), 22); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + 5 * (1 local commitment tx + 1 htlc timeout tx)
2086+
assert_eq!(node_txn.len(), 24); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + 5 * (1 local commitment tx + 1 htlc timeout tx)
20822087

20832088
assert_eq!(node_txn[0], node_txn[7]);
20842089
assert_eq!(node_txn[1], node_txn[8]);
@@ -2088,10 +2093,6 @@ fn claim_htlc_outputs_single_tx() {
20882093
assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcasted by ChannelManger
20892094
assert_eq!(node_txn[4], node_txn[6]);
20902095

2091-
for i in 12..22 {
2092-
if i % 2 == 0 { assert_eq!(node_txn[3], node_txn[i]); } else { assert_eq!(node_txn[4], node_txn[i]); }
2093-
}
2094-
20952096
assert_eq!(node_txn[0].input.len(), 1);
20962097
assert_eq!(node_txn[1].input.len(), 1);
20972098
assert_eq!(node_txn[2].input.len(), 1);

0 commit comments

Comments
 (0)