Skip to content

Commit a369f9e

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #985 from TheBlueMatt/2021-06-auto-chan-fee-updates
Automatically Update fees on outbound channels
2 parents 9d8d24f + d3af49e commit a369f9e

File tree

9 files changed

+673
-218
lines changed

9 files changed

+673
-218
lines changed

‎ci/check-compiles.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ echo Testing $(git log -1 --oneline)
55
cargo check
66
cargo doc
77
cargo doc --document-private-items
8-
cd fuzz && cargo check --features=stdin_fuzz
8+
cd fuzz && RUSTFLAGS="--cfg=fuzzing" cargo check --features=stdin_fuzz
99
cd ../lightning && cargo check --no-default-features --features=no-std

‎fuzz/src/chanmon_consistency.rs

+84-23
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget,
3737
use lightning::chain::keysinterface::{KeysInterface, InMemorySigner};
3838
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
3939
use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs};
40+
use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
4041
use lightning::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
4142
use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init};
4243
use lightning::ln::script::ShutdownScript;
@@ -58,16 +59,27 @@ use bitcoin::secp256k1::recovery::RecoverableSignature;
5859
use bitcoin::secp256k1::Secp256k1;
5960

6061
use std::mem;
61-
use std::cmp::Ordering;
62+
use std::cmp::{self, Ordering};
6263
use std::collections::{HashSet, hash_map, HashMap};
6364
use std::sync::{Arc,Mutex};
6465
use std::sync::atomic;
6566
use std::io::Cursor;
6667

67-
struct FuzzEstimator {}
68+
const MAX_FEE: u32 = 10_000;
69+
struct FuzzEstimator {
70+
ret_val: atomic::AtomicU32,
71+
}
6872
impl FeeEstimator for FuzzEstimator {
69-
fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 {
70-
253
73+
fn get_est_sat_per_1000_weight(&self, conf_target: ConfirmationTarget) -> u32 {
74+
// We force-close channels if our counterparty sends us a feerate which is a small multiple
75+
// of our HighPriority fee estimate or smaller than our Background fee estimate. Thus, we
76+
// always return a HighPriority feerate here which is >= the maximum Normal feerate and a
77+
// Background feerate which is <= the minimum Normal feerate.
78+
match conf_target {
79+
ConfirmationTarget::HighPriority => MAX_FEE,
80+
ConfirmationTarget::Background => 253,
81+
ConfirmationTarget::Normal => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE),
82+
}
7183
}
7284
}
7385

@@ -132,7 +144,7 @@ impl chain::Watch<EnforcingSigner> for TestChainMonitor {
132144
};
133145
let deserialized_monitor = <(BlockHash, channelmonitor::ChannelMonitor<EnforcingSigner>)>::
134146
read(&mut Cursor::new(&map_entry.get().1), &*self.keys).unwrap().1;
135-
deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator{}, &self.logger).unwrap();
147+
deserialized_monitor.update_monitor(&update, &&TestBroadcaster{}, &&FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }, &self.logger).unwrap();
136148
let mut ser = VecWriter(Vec::new());
137149
deserialized_monitor.write(&mut ser).unwrap();
138150
map_entry.insert((update.update_id, ser.0));
@@ -334,14 +346,13 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
334346

335347
#[inline]
336348
pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
337-
let fee_est = Arc::new(FuzzEstimator{});
338349
let broadcast = Arc::new(TestBroadcaster{});
339350

340351
macro_rules! make_node {
341-
($node_id: expr) => { {
352+
($node_id: expr, $fee_estimator: expr) => { {
342353
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
343354
let keys_manager = Arc::new(KeyProvider { node_id: $node_id, rand_bytes_id: atomic::AtomicU32::new(0), revoked_commitments: Mutex::new(HashMap::new()) });
344-
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{}), Arc::clone(&keys_manager)));
355+
let monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(), Arc::new(TestPersister{}), Arc::clone(&keys_manager)));
345356

346357
let mut config = UserConfig::default();
347358
config.channel_options.forwarding_fee_proportional_millionths = 0;
@@ -351,16 +362,16 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
351362
network,
352363
best_block: BestBlock::from_genesis(network),
353364
};
354-
(ChannelManager::new(fee_est.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, params),
365+
(ChannelManager::new($fee_estimator.clone(), monitor.clone(), broadcast.clone(), Arc::clone(&logger), keys_manager.clone(), config, params),
355366
monitor, keys_manager)
356367
} }
357368
}
358369

359370
macro_rules! reload_node {
360-
($ser: expr, $node_id: expr, $old_monitors: expr, $keys_manager: expr) => { {
371+
($ser: expr, $node_id: expr, $old_monitors: expr, $keys_manager: expr, $fee_estimator: expr) => { {
361372
let keys_manager = Arc::clone(& $keys_manager);
362373
let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone()));
363-
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), fee_est.clone(), Arc::new(TestPersister{}), Arc::clone(& $keys_manager)));
374+
let chain_monitor = Arc::new(TestChainMonitor::new(broadcast.clone(), logger.clone(), $fee_estimator.clone(), Arc::new(TestPersister{}), Arc::clone(& $keys_manager)));
364375

365376
let mut config = UserConfig::default();
366377
config.channel_options.forwarding_fee_proportional_millionths = 0;
@@ -379,7 +390,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
379390

380391
let read_args = ChannelManagerReadArgs {
381392
keys_manager,
382-
fee_estimator: fee_est.clone(),
393+
fee_estimator: $fee_estimator.clone(),
383394
chain_monitor: chain_monitor.clone(),
384395
tx_broadcaster: broadcast.clone(),
385396
logger,
@@ -497,11 +508,18 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
497508
} }
498509
}
499510

511+
let fee_est_a = Arc::new(FuzzEstimator { ret_val: atomic::AtomicU32::new(253) });
512+
let mut last_htlc_clear_fee_a = 253;
513+
let fee_est_b = Arc::new(FuzzEstimator { ret_val: atomic::AtomicU32::new(253) });
514+
let mut last_htlc_clear_fee_b = 253;
515+
let fee_est_c = Arc::new(FuzzEstimator { ret_val: atomic::AtomicU32::new(253) });
516+
let mut last_htlc_clear_fee_c = 253;
517+
500518
// 3 nodes is enough to hit all the possible cases, notably unknown-source-unknown-dest
501519
// forwarding.
502-
let (node_a, mut monitor_a, keys_manager_a) = make_node!(0);
503-
let (node_b, mut monitor_b, keys_manager_b) = make_node!(1);
504-
let (node_c, mut monitor_c, keys_manager_c) = make_node!(2);
520+
let (node_a, mut monitor_a, keys_manager_a) = make_node!(0, fee_est_a);
521+
let (node_b, mut monitor_b, keys_manager_b) = make_node!(1, fee_est_b);
522+
let (node_c, mut monitor_c, keys_manager_c) = make_node!(2, fee_est_c);
505523

506524
let mut nodes = [node_a, node_b, node_c];
507525

@@ -637,10 +655,10 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
637655
had_events = true;
638656
match event {
639657
events::MessageSendEvent::UpdateHTLCs { node_id, updates: CommitmentUpdate { update_add_htlcs, update_fail_htlcs, update_fulfill_htlcs, update_fail_malformed_htlcs, update_fee, commitment_signed } } => {
640-
for dest in nodes.iter() {
658+
for (idx, dest) in nodes.iter().enumerate() {
641659
if dest.get_our_node_id() == node_id {
642-
assert!(update_fee.is_none());
643660
for update_add in update_add_htlcs.iter() {
661+
out.locked_write(format!("Delivering update_add_htlc to node {}.\n", idx).as_bytes());
644662
if !$corrupt_forward {
645663
dest.handle_update_add_htlc(&nodes[$node].get_our_node_id(), update_add);
646664
} else {
@@ -655,14 +673,21 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
655673
}
656674
}
657675
for update_fulfill in update_fulfill_htlcs.iter() {
676+
out.locked_write(format!("Delivering update_fulfill_htlc to node {}.\n", idx).as_bytes());
658677
dest.handle_update_fulfill_htlc(&nodes[$node].get_our_node_id(), update_fulfill);
659678
}
660679
for update_fail in update_fail_htlcs.iter() {
680+
out.locked_write(format!("Delivering update_fail_htlc to node {}.\n", idx).as_bytes());
661681
dest.handle_update_fail_htlc(&nodes[$node].get_our_node_id(), update_fail);
662682
}
663683
for update_fail_malformed in update_fail_malformed_htlcs.iter() {
684+
out.locked_write(format!("Delivering update_fail_malformed_htlc to node {}.\n", idx).as_bytes());
664685
dest.handle_update_fail_malformed_htlc(&nodes[$node].get_our_node_id(), update_fail_malformed);
665686
}
687+
if let Some(msg) = update_fee {
688+
out.locked_write(format!("Delivering update_fee to node {}.\n", idx).as_bytes());
689+
dest.handle_update_fee(&nodes[$node].get_our_node_id(), &msg);
690+
}
666691
let processed_change = !update_add_htlcs.is_empty() || !update_fulfill_htlcs.is_empty() ||
667692
!update_fail_htlcs.is_empty() || !update_fail_malformed_htlcs.is_empty();
668693
if $limit_events != ProcessMessages::AllMessages && processed_change {
@@ -677,21 +702,24 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
677702
} });
678703
break;
679704
}
705+
out.locked_write(format!("Delivering commitment_signed to node {}.\n", idx).as_bytes());
680706
dest.handle_commitment_signed(&nodes[$node].get_our_node_id(), &commitment_signed);
681707
break;
682708
}
683709
}
684710
},
685711
events::MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
686-
for dest in nodes.iter() {
712+
for (idx, dest) in nodes.iter().enumerate() {
687713
if dest.get_our_node_id() == *node_id {
714+
out.locked_write(format!("Delivering revoke_and_ack to node {}.\n", idx).as_bytes());
688715
dest.handle_revoke_and_ack(&nodes[$node].get_our_node_id(), msg);
689716
}
690717
}
691718
},
692719
events::MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } => {
693-
for dest in nodes.iter() {
720+
for (idx, dest) in nodes.iter().enumerate() {
694721
if dest.get_our_node_id() == *node_id {
722+
out.locked_write(format!("Delivering channel_reestablish to node {}.\n", idx).as_bytes());
695723
dest.handle_channel_reestablish(&nodes[$node].get_our_node_id(), msg);
696724
}
697725
}
@@ -824,7 +852,9 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
824852
} }
825853
}
826854

827-
match get_slice!(1)[0] {
855+
let v = get_slice!(1)[0];
856+
out.locked_write(format!("READ A BYTE! HANDLING INPUT {:x}...........\n", v).as_bytes());
857+
match v {
828858
// In general, we keep related message groups close together in binary form, allowing
829859
// bit-twiddling mutations to have similar effects. This is probably overkill, but no
830860
// harm in doing so.
@@ -928,7 +958,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
928958
node_a_ser.0.clear();
929959
nodes[0].write(&mut node_a_ser).unwrap();
930960
}
931-
let (new_node_a, new_monitor_a) = reload_node!(node_a_ser, 0, monitor_a, keys_manager_a);
961+
let (new_node_a, new_monitor_a) = reload_node!(node_a_ser, 0, monitor_a, keys_manager_a, fee_est_a);
932962
nodes[0] = new_node_a;
933963
monitor_a = new_monitor_a;
934964
},
@@ -947,7 +977,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
947977
bc_events.clear();
948978
cb_events.clear();
949979
}
950-
let (new_node_b, new_monitor_b) = reload_node!(node_b_ser, 1, monitor_b, keys_manager_b);
980+
let (new_node_b, new_monitor_b) = reload_node!(node_b_ser, 1, monitor_b, keys_manager_b, fee_est_b);
951981
nodes[1] = new_node_b;
952982
monitor_b = new_monitor_b;
953983
},
@@ -961,7 +991,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
961991
node_c_ser.0.clear();
962992
nodes[2].write(&mut node_c_ser).unwrap();
963993
}
964-
let (new_node_c, new_monitor_c) = reload_node!(node_c_ser, 2, monitor_c, keys_manager_c);
994+
let (new_node_c, new_monitor_c) = reload_node!(node_c_ser, 2, monitor_c, keys_manager_c, fee_est_c);
965995
nodes[2] = new_node_c;
966996
monitor_c = new_monitor_c;
967997
},
@@ -1023,6 +1053,33 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
10231053
0x6c => { send_hop_payment(&nodes[0], &nodes[1], chan_a, &nodes[2], chan_b, 1, &mut payment_id); },
10241054
0x6d => { send_hop_payment(&nodes[2], &nodes[1], chan_b, &nodes[0], chan_a, 1, &mut payment_id); },
10251055

1056+
0x80 => {
1057+
let max_feerate = last_htlc_clear_fee_a * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1058+
if fee_est_a.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
1059+
fee_est_a.ret_val.store(max_feerate, atomic::Ordering::Release);
1060+
}
1061+
nodes[0].maybe_update_chan_fees();
1062+
},
1063+
0x81 => { fee_est_a.ret_val.store(253, atomic::Ordering::Release); nodes[0].maybe_update_chan_fees(); },
1064+
1065+
0x84 => {
1066+
let max_feerate = last_htlc_clear_fee_b * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1067+
if fee_est_b.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
1068+
fee_est_b.ret_val.store(max_feerate, atomic::Ordering::Release);
1069+
}
1070+
nodes[1].maybe_update_chan_fees();
1071+
},
1072+
0x85 => { fee_est_b.ret_val.store(253, atomic::Ordering::Release); nodes[1].maybe_update_chan_fees(); },
1073+
1074+
0x88 => {
1075+
let max_feerate = last_htlc_clear_fee_c * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
1076+
if fee_est_c.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
1077+
fee_est_c.ret_val.store(max_feerate, atomic::Ordering::Release);
1078+
}
1079+
nodes[2].maybe_update_chan_fees();
1080+
},
1081+
0x89 => { fee_est_c.ret_val.store(253, atomic::Ordering::Release); nodes[2].maybe_update_chan_fees(); },
1082+
10261083
0xff => {
10271084
// Test that no channel is in a stuck state where neither party can send funds even
10281085
// after we resolve all pending events.
@@ -1078,6 +1135,10 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
10781135
assert!(
10791136
send_payment(&nodes[1], &nodes[2], chan_b, 10_000_000, &mut payment_id) ||
10801137
send_payment(&nodes[2], &nodes[1], chan_b, 10_000_000, &mut payment_id));
1138+
1139+
last_htlc_clear_fee_a = fee_est_a.ret_val.load(atomic::Ordering::Acquire);
1140+
last_htlc_clear_fee_b = fee_est_b.ret_val.load(atomic::Ordering::Acquire);
1141+
last_htlc_clear_fee_c = fee_est_c.ret_val.load(atomic::Ordering::Acquire);
10811142
},
10821143
_ => test_return!(),
10831144
}

‎lightning-background-processor/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ impl BackgroundProcessor {
140140
let stop_thread = Arc::new(AtomicBool::new(false));
141141
let stop_thread_clone = stop_thread.clone();
142142
let handle = thread::spawn(move || -> Result<(), std::io::Error> {
143+
log_trace!(logger, "Calling ChannelManager's timer_tick_occurred on startup");
144+
channel_manager.timer_tick_occurred();
145+
143146
let mut last_freshness_call = Instant::now();
144147
let mut last_ping_call = Instant::now();
145148
loop {

‎lightning/src/chain/chaininterface.rs

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub trait BroadcasterInterface {
2323

2424
/// An enum that represents the speed at which we want a transaction to confirm used for feerate
2525
/// estimation.
26+
#[derive(Clone, Copy, PartialEq, Eq)]
2627
pub enum ConfirmationTarget {
2728
/// We are happy with this transaction confirming slowly when feerate drops some.
2829
Background,

0 commit comments

Comments
 (0)