Skip to content

Commit 4691298

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
1 parent e482e61 commit 4691298

File tree

2 files changed

+143
-21
lines changed

2 files changed

+143
-21
lines changed

src/ln/channelmonitor.rs

Lines changed: 116 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,9 +2110,6 @@ impl ChannelMonitor {
21102110
}
21112111
}
21122112
}
2113-
for claim in pending_claims {
2114-
self.our_claim_txn_waiting_first_conf.insert(claim.0, claim.1);
2115-
}
21162113
if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) {
21172114
for ev in events {
21182115
match ev {
@@ -2126,7 +2123,29 @@ impl ChannelMonitor {
21262123
}
21272124
}
21282125
}
2129-
//TODO: iter on buffered TxMaterial in our_claim_txn_waiting_first_conf, if block timer is expired generate a bumped claim tx (RBF or CPFP accordingly)
2126+
let mut bumped_txn = Vec::new();
2127+
let mut pending_claims = Vec::new();
2128+
{
2129+
let mut bump_candidates = Vec::new();
2130+
for (claimed_outpoint, claim_tx_data) in self.our_claim_txn_waiting_first_conf.iter() {
2131+
if claim_tx_data.0 == height {
2132+
bump_candidates.push((claimed_outpoint, claim_tx_data));
2133+
}
2134+
}
2135+
for candidate in bump_candidates {
2136+
if let Some((new_timer, mut bumped_tx, feerate)) = self.bump_claim_tx(candidate.0, (candidate.1).0, &(candidate.1).1, (candidate.1).2, fee_estimator) {
2137+
pending_claims.push((*candidate.0, (new_timer, (candidate.1).1.clone() , feerate)));
2138+
bumped_txn.append(&mut vec![bumped_tx]);
2139+
}
2140+
}
2141+
}
2142+
for tx in pending_claims {
2143+
log_trace!(self, "Outpoint {}:{} is under claiming process, if it doesn't succeed, a bumped claiming txn is going to be broadcast at height {}", (tx.0).vout, (tx.0).txid, (tx.1).0);
2144+
self.our_claim_txn_waiting_first_conf.insert(tx.0, tx.1);
2145+
}
2146+
for tx in bumped_txn {
2147+
broadcaster.broadcast_transaction(&tx)
2148+
}
21302149
self.last_block_hash = block_hash.clone();
21312150
(watch_outputs, spendable_outputs, htlc_updated)
21322151
}
@@ -2339,6 +2358,99 @@ impl ChannelMonitor {
23392358
}
23402359
htlc_updated
23412360
}
2361+
2362+
/// 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
2363+
/// (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.
2364+
// TODO: we may use smarter heuristics to aggregate-at-bumping if we add timelock in cached materials
2365+
fn bump_claim_tx(&self, claimed_outpoint: &BitcoinOutPoint, old_timer: u32, tx_material: &TxMaterial, old_feerate: u64, fee_estimator: &FeeEstimator) -> Option<(u32, Transaction, u64)> {
2366+
let mut bumped_tx = Transaction {
2367+
version: 2,
2368+
lock_time: 0,
2369+
input: vec![TxIn {
2370+
previous_output: claimed_outpoint.clone(),
2371+
script_sig: Script::new(),
2372+
sequence: 0xfffffffd,
2373+
witness: Vec::new(),
2374+
}],
2375+
output: vec![TxOut {
2376+
script_pubkey: self.destination_script.clone(),
2377+
value: 0
2378+
}],
2379+
};
2380+
2381+
macro_rules! RBF_bump {
2382+
($amount: expr, $old_feerate: expr, $fee_estimator: expr, $predicted_weight: expr, $outpoint: expr, $output: expr) => {
2383+
{
2384+
// If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee...
2385+
let new_fee = if $old_feerate < $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) {
2386+
let mut value = $amount;
2387+
if subtract_high_prio_fee!(self, $fee_estimator, value, $predicted_weight, $outpoint.txid) {
2388+
$amount - value
2389+
} else {
2390+
log_trace!(self, "Can't new-estimation bump claiming on {} output {} from {}, amount {} is too small", $output, $outpoint.vout, $outpoint.txid, $amount);
2391+
return None;
2392+
}
2393+
// ...else just increase the previous feerate by 25% (because that's a nice number)
2394+
} else {
2395+
let fee = $old_feerate * $predicted_weight / 750;
2396+
if $amount <= fee {
2397+
log_trace!(self, "Can't 25% bump claiming on {} output {} from {}, amount {} is too small", $output, $outpoint.vout, $outpoint.txid, $amount);
2398+
return None;
2399+
}
2400+
fee
2401+
};
2402+
2403+
let previous_fee = $old_feerate * $predicted_weight / 1000;
2404+
let min_relay_fee = $fee_estimator.get_min_relay_sat_per_1000_weight() * $predicted_weight / 1000;
2405+
// BIP 125 Opt-in Full Replace-by-Fee Signaling
2406+
// * 3. The replacement transaction pays an absolute fee of at least the sum paid by the original transactions.
2407+
// * 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.
2408+
let new_fee = if new_fee < previous_fee + min_relay_fee {
2409+
new_fee + previous_fee + min_relay_fee - new_fee
2410+
} else {
2411+
new_fee
2412+
};
2413+
Some((new_fee, new_fee * 1000 / $predicted_weight))
2414+
}
2415+
}
2416+
}
2417+
2418+
// We set the new timer to 3 blocks, which is nearly HighConfirmation target
2419+
let new_timer = old_timer + 3; //TODO: we may even be more aggressive there by decreasing timer delay at each bumping
2420+
2421+
match tx_material {
2422+
TxMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
2423+
let predicted_weight = bumped_tx.get_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 { &[] });
2424+
if let Some((new_fee, new_feerate)) = RBF_bump!(*amount, old_feerate, fee_estimator, predicted_weight, claimed_outpoint, "revoked") {
2425+
bumped_tx.output[0].value = amount - new_fee;
2426+
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
2427+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[0], &script, *amount)[..]);
2428+
let sig = self.secp_ctx.sign(&sighash, &key);
2429+
bumped_tx.input[0].witness.push(sig.serialize_der().to_vec());
2430+
bumped_tx.input[0].witness[0].push(SigHashType::All as u8);
2431+
if *is_htlc {
2432+
bumped_tx.input[0].witness.push(pubkey.unwrap().clone().serialize().to_vec());
2433+
} else {
2434+
bumped_tx.input[0].witness.push(vec!(1));
2435+
}
2436+
bumped_tx.input[0].witness.push(script.clone().into_bytes());
2437+
assert!(predicted_weight >= bumped_tx.get_weight());
2438+
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 { "" }, claimed_outpoint.vout, claimed_outpoint.txid, new_feerate);
2439+
return Some((new_timer, bumped_tx, new_feerate));
2440+
}
2441+
},
2442+
TxMaterial::RemoteHTLC { .. } => {
2443+
return None;
2444+
},
2445+
TxMaterial::LocalHTLC { .. } => {
2446+
//TODO : Given that Local Commitment Transaction and HTLC-Timeout/HTLC-Success are counter-signed by peer, we can't
2447+
// RBF them. Need a Lightning specs change and package relay modification :
2448+
// https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
2449+
return None;
2450+
},
2451+
}
2452+
None
2453+
}
23422454
}
23432455

23442456
const MAX_ALLOC_SIZE: usize = 64*1024;

src/ln/functional_tests.rs

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,12 +1755,13 @@ fn test_justice_tx() {
17551755
connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
17561756
{
17571757
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1758-
assert_eq!(node_txn.len(), 3);
1758+
assert_eq!(node_txn.len(), 5);
17591759
assert_eq!(node_txn.remove(0), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
17601760
assert_eq!(node_txn[0].input.len(), 2); // We should claim the revoked output and the HTLC output
17611761

17621762
check_spends!(node_txn[0], revoked_local_txn[0].clone());
17631763
node_txn.swap_remove(0);
1764+
node_txn.truncate(1);
17641765
}
17651766
test_txn_broadcast(&nodes[1], &chan_5, None, HTLCType::NONE);
17661767

@@ -1779,6 +1780,10 @@ fn test_justice_tx() {
17791780
// We test justice_tx build by A on B's revoked HTLC-Success tx
17801781
// Create some new channels:
17811782
let chan_6 = create_announced_chan_between_nodes(&nodes, 0, 1);
1783+
{
1784+
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1785+
node_txn.clear();
1786+
}
17821787

17831788
// A pending HTLC which will be revoked:
17841789
let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
@@ -1796,17 +1801,22 @@ fn test_justice_tx() {
17961801
connect_blocks(&nodes[0].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
17971802
{
17981803
let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
1799-
assert_eq!(node_txn.len(), 3);
1804+
assert_eq!(node_txn.len(), 4);
18001805
assert_eq!(node_txn.remove(0), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
18011806
assert_eq!(node_txn[0].input.len(), 1); // We claim the received HTLC output
18021807

18031808
check_spends!(node_txn[0], revoked_local_txn[0].clone());
18041809
node_txn.swap_remove(0);
1810+
node_txn.remove(1);
18051811
}
18061812
test_txn_broadcast(&nodes[0], &chan_6, None, HTLCType::NONE);
18071813

18081814
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
18091815
connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
1816+
{
1817+
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1818+
node_txn.truncate(1);
1819+
}
18101820
let node_txn = test_txn_broadcast(&nodes[1], &chan_6, Some(revoked_local_txn[0].clone()), HTLCType::SUCCESS);
18111821
header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
18121822
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![node_txn[1].clone()] }, 1);
@@ -1836,12 +1846,12 @@ fn revoked_output_claim() {
18361846
nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
18371847
connect_blocks(&nodes[1].chain_monitor, HTLC_FAIL_ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
18381848
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1839-
assert_eq!(node_txn.len(), 3); // nodes[1] will broadcast justice tx twice, and its own local state once
1849+
assert_eq!(node_txn.len(), 4); // nodes[1] will broadcast justice tx twice, and its own local state once
18401850

18411851
assert_eq!(node_txn[0], node_txn[1]);
18421852

18431853
check_spends!(node_txn[0], revoked_local_txn[0].clone());
1844-
check_spends!(node_txn[2], chan_1.3.clone());
1854+
check_spends!(node_txn[3], chan_1.3.clone());
18451855

18461856
// Inform nodes[0] that a watchtower cheated on its behalf, so it will force-close the chan
18471857
nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
@@ -1893,7 +1903,7 @@ fn claim_htlc_outputs_shared_tx() {
18931903
}
18941904

18951905
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1896-
assert_eq!(node_txn.len(), 4);
1906+
assert_eq!(node_txn.len(), 7);
18971907

18981908
assert_eq!(node_txn[0].input.len(), 3); // Claim the revoked output + both revoked HTLC outputs
18991909
check_spends!(node_txn[0], revoked_local_txn[0].clone());
@@ -1910,15 +1920,15 @@ fn claim_htlc_outputs_shared_tx() {
19101920
assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), ACCEPTED_HTLC_SCRIPT_WEIGHT); // revoked received HTLC
19111921

19121922
// Next nodes[1] broadcasts its current local tx state:
1913-
assert_eq!(node_txn[2].input.len(), 1);
1914-
assert_eq!(node_txn[2].input[0].previous_output.txid, chan_1.3.txid()); //Spending funding tx unique txouput, tx broadcasted by ChannelManager
1923+
assert_eq!(node_txn[5].input.len(), 1);
1924+
assert_eq!(node_txn[5].input[0].previous_output.txid, chan_1.3.txid()); //Spending funding tx unique txouput, tx broadcasted by ChannelManager
19151925

1916-
assert_eq!(node_txn[3].input.len(), 1);
1917-
let witness_script = node_txn[3].clone().input[0].witness.pop().unwrap();
1926+
assert_eq!(node_txn[6].input.len(), 1);
1927+
let witness_script = node_txn[6].clone().input[0].witness.pop().unwrap();
19181928
assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output
1919-
assert_eq!(node_txn[3].input[0].previous_output.txid, node_txn[2].txid());
1920-
assert_ne!(node_txn[3].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
1921-
assert_ne!(node_txn[3].input[0].previous_output.txid, node_txn[0].input[1].previous_output.txid);
1929+
assert_eq!(node_txn[6].input[0].previous_output.txid, node_txn[5].txid());
1930+
assert_ne!(node_txn[6].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
1931+
assert_ne!(node_txn[6].input[0].previous_output.txid, node_txn[0].input[1].previous_output.txid);
19221932
}
19231933
get_announce_close_broadcast_events(&nodes, 0, 1);
19241934
assert_eq!(nodes[0].node.list_channels().len(), 0);
@@ -1962,12 +1972,12 @@ fn claim_htlc_outputs_single_tx() {
19621972
}
19631973

19641974
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
1965-
assert_eq!(node_txn.len(), 20); // 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)
1975+
assert_eq!(node_txn.len(), 23); // 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)
19661976

19671977
assert_eq!(node_txn[0], node_txn[5]);
19681978
assert_eq!(node_txn[1], node_txn[6]);
19691979
assert_eq!(node_txn[2], node_txn[7]);
1970-
for i in 8..20 {
1980+
for i in 8..16 {
19711981
if i % 2 == 0 { assert_eq!(node_txn[3], node_txn[i]); } else { assert_eq!(node_txn[4], node_txn[i]); }
19721982
}
19731983

@@ -3660,7 +3670,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
36603670
check_closed_broadcast!(nodes[1]);
36613671

36623672
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
3663-
assert_eq!(node_txn.len(), 3);
3673+
assert_eq!(node_txn.len(), 5);
36643674
assert_eq!(node_txn[0], node_txn[1]);
36653675
assert_eq!(node_txn[0].input.len(), 2);
36663676
check_spends!(node_txn[0], revoked_local_txn[0].clone());
@@ -3705,7 +3715,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
37053715
check_closed_broadcast!(nodes[1]);
37063716

37073717
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
3708-
assert_eq!(node_txn.len(), 4);
3718+
assert_eq!(node_txn.len(), 7);
37093719
assert_eq!(node_txn[2].input.len(), 1);
37103720
check_spends!(node_txn[2], revoked_htlc_txn[0].clone());
37113721

@@ -3750,7 +3760,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
37503760
check_closed_broadcast!(nodes[0]);
37513761

37523762
let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
3753-
assert_eq!(node_txn.len(), 4);
3763+
assert_eq!(node_txn.len(), 6);
37543764
assert_eq!(node_txn[2].input.len(), 1);
37553765
check_spends!(node_txn[2], revoked_htlc_txn[0].clone());
37563766

0 commit comments

Comments
 (0)