Skip to content

Commit 8a27d8e

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #613 from valentinewallace/less-confusing-chan-reserve-names
Make channel reserve variable names less confusing.
2 parents 9098240 + 1b656f4 commit 8a27d8e

File tree

2 files changed

+42
-40
lines changed

2 files changed

+42
-40
lines changed

‎lightning/src/ln/channel.rs

+33-31
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,9 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
329329
#[cfg(not(test))]
330330
their_max_htlc_value_in_flight_msat: u64,
331331
//get_our_max_htlc_value_in_flight_msat(): u64,
332-
/// minimum channel reserve for **self** to maintain - set by them.
333-
their_channel_reserve_satoshis: u64,
334-
//get_our_channel_reserve_satoshis(): u64,
332+
/// minimum channel reserve for self to maintain - set by them.
333+
local_channel_reserve_satoshis: u64,
334+
// get_remote_channel_reserve_satoshis(channel_value_sats: u64): u64
335335
their_htlc_minimum_msat: u64,
336336
our_htlc_minimum_msat: u64,
337337
their_to_self_delay: u16,
@@ -420,10 +420,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
420420
channel_value_satoshis * 1000 / 10 //TODO
421421
}
422422

423-
/// Returns a minimum channel reserve value **they** need to maintain
423+
/// Returns a minimum channel reserve value the remote needs to maintain,
424+
/// required by us.
424425
///
425426
/// Guaranteed to return a value no larger than channel_value_satoshis
426-
pub(crate) fn get_our_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
427+
pub(crate) fn get_remote_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
427428
let (q, _) = channel_value_satoshis.overflowing_div(100);
428429
cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
429430
}
@@ -452,7 +453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
452453

453454

454455
let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
455-
if Channel::<ChanSigner>::get_our_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
456+
if Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(channel_value_satoshis) < Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate) {
456457
return Err(APIError::FeeRateTooHigh{err: format!("Not enough reserve above dust limit can be found at current fee rate({})", background_feerate), feerate: background_feerate});
457458
}
458459

@@ -512,7 +513,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
512513
their_dust_limit_satoshis: 0,
513514
our_dust_limit_satoshis: Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate),
514515
their_max_htlc_value_in_flight_msat: 0,
515-
their_channel_reserve_satoshis: 0,
516+
local_channel_reserve_satoshis: 0,
516517
their_htlc_minimum_msat: 0,
517518
our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
518519
their_to_self_delay: 0,
@@ -638,15 +639,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
638639
let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
639640

640641
let our_dust_limit_satoshis = Channel::<ChanSigner>::derive_our_dust_limit_satoshis(background_feerate);
641-
let our_channel_reserve_satoshis = Channel::<ChanSigner>::get_our_channel_reserve_satoshis(msg.funding_satoshis);
642-
if our_channel_reserve_satoshis < our_dust_limit_satoshis {
642+
let remote_channel_reserve_satoshis = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(msg.funding_satoshis);
643+
if remote_channel_reserve_satoshis < our_dust_limit_satoshis {
643644
return Err(ChannelError::Close("Suitable channel reserve not found. aborting"));
644645
}
645646
if msg.channel_reserve_satoshis < our_dust_limit_satoshis {
646647
return Err(ChannelError::Close("channel_reserve_satoshis too small"));
647648
}
648-
if our_channel_reserve_satoshis < msg.dust_limit_satoshis {
649-
return Err(ChannelError::Close("Dust limit too high for our channel reserve"));
649+
if remote_channel_reserve_satoshis < msg.dust_limit_satoshis {
650+
return Err(ChannelError::Close("Dust limit too high for the channel reserve we require the remote to keep"));
650651
}
651652

652653
// check if the funder's amount for the initial commitment tx is sufficient
@@ -658,7 +659,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
658659

659660
let to_local_msat = msg.push_msat;
660661
let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT;
661-
if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= our_channel_reserve_satoshis * 1000 {
662+
if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= remote_channel_reserve_satoshis * 1000 {
662663
return Err(ChannelError::Close("Insufficient funding amount for initial commitment"));
663664
}
664665

@@ -737,7 +738,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
737738
their_dust_limit_satoshis: msg.dust_limit_satoshis,
738739
our_dust_limit_satoshis: our_dust_limit_satoshis,
739740
their_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
740-
their_channel_reserve_satoshis: msg.channel_reserve_satoshis,
741+
local_channel_reserve_satoshis: msg.channel_reserve_satoshis,
741742
their_htlc_minimum_msat: msg.htlc_minimum_msat,
742743
our_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
743744
their_to_self_delay: msg.to_self_delay,
@@ -952,9 +953,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
952953
} else {
953954
self.max_commitment_tx_output_remote.lock().unwrap()
954955
};
955-
debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.their_channel_reserve_satoshis as i64);
956+
debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.local_channel_reserve_satoshis as i64);
956957
max_commitment_tx_output.0 = cmp::max(max_commitment_tx_output.0, value_to_self_msat as u64);
957-
debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
958+
debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
958959
max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64);
959960
}
960961

@@ -1362,7 +1363,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
13621363
if msg.channel_reserve_satoshis < self.our_dust_limit_satoshis {
13631364
return Err(ChannelError::Close("Peer never wants payout outputs?"));
13641365
}
1365-
if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis) {
1366+
if msg.dust_limit_satoshis > Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) {
13661367
return Err(ChannelError::Close("Dust limit is bigger than our channel reverse"));
13671368
}
13681369
if msg.htlc_minimum_msat >= (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000 {
@@ -1424,7 +1425,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
14241425

14251426
self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
14261427
self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
1427-
self.their_channel_reserve_satoshis = msg.channel_reserve_satoshis;
1428+
self.local_channel_reserve_satoshis = msg.channel_reserve_satoshis;
14281429
self.their_htlc_minimum_msat = msg.htlc_minimum_msat;
14291430
self.their_to_self_delay = msg.to_self_delay;
14301431
self.their_max_accepted_htlcs = msg.max_accepted_htlcs;
@@ -1696,7 +1697,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
16961697
if htlc_inbound_value_msat + msg.amount_msat > Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) {
16971698
return Err(ChannelError::Close("Remote HTLC add would put them over our max HTLC value"));
16981699
}
1699-
// Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
1700+
// Check remote_channel_reserve_satoshis (we're getting paid, so they have to at least meet
17001701
// the reserve_satoshis we told them to always have as direct payment so that they lose
17011702
// something if we punish them for broadcasting an old state).
17021703
// Note that we don't really care about having a small/no to_remote output in our local
@@ -1716,8 +1717,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
17161717
removed_outbound_total_msat += htlc.amount_msat;
17171718
}
17181719
}
1719-
if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
1720-
return Err(ChannelError::Close("Remote HTLC add would put them over their reserve value"));
1720+
if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat {
1721+
return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value"));
17211722
}
17221723
if self.next_remote_htlc_id != msg.htlc_id {
17231724
return Err(ChannelError::Close("Remote skipped HTLC ID"));
@@ -1847,7 +1848,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
18471848
let num_htlcs = local_commitment_tx.1;
18481849
let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
18491850

1850-
if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + self.their_channel_reserve_satoshis {
1851+
let remote_reserve_we_require = Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis);
1852+
if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + remote_reserve_we_require {
18511853
return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee")));
18521854
}
18531855
}
@@ -3033,7 +3035,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
30333035
ChannelValueStat {
30343036
value_to_self_msat: self.value_to_self_msat,
30353037
channel_value_msat: self.channel_value_satoshis * 1000,
3036-
channel_reserve_msat: self.their_channel_reserve_satoshis * 1000,
3038+
channel_reserve_msat: self.local_channel_reserve_satoshis * 1000,
30373039
pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
30383040
pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
30393041
holding_cell_outbound_amount_msat: {
@@ -3321,7 +3323,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
33213323
push_msat: self.channel_value_satoshis * 1000 - self.value_to_self_msat,
33223324
dust_limit_satoshis: self.our_dust_limit_satoshis,
33233325
max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
3324-
channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
3326+
channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
33253327
htlc_minimum_msat: self.our_htlc_minimum_msat,
33263328
feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
33273329
to_self_delay: self.our_to_self_delay,
@@ -3354,7 +3356,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
33543356
temporary_channel_id: self.channel_id,
33553357
dust_limit_satoshis: self.our_dust_limit_satoshis,
33563358
max_htlc_value_in_flight_msat: Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
3357-
channel_reserve_satoshis: Channel::<ChanSigner>::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
3359+
channel_reserve_satoshis: Channel::<ChanSigner>::get_remote_channel_reserve_satoshis(self.channel_value_satoshis),
33583360
htlc_minimum_msat: self.our_htlc_minimum_msat,
33593361
minimum_depth: self.minimum_depth,
33603362
to_self_delay: self.our_to_self_delay,
@@ -3550,10 +3552,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
35503552
return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept"));
35513553
}
35523554

3553-
// Check self.their_channel_reserve_satoshis (the amount we must keep as
3554-
// reserve for them to have something to claim if we misbehave)
3555-
if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
3556-
return Err(ChannelError::Ignore("Cannot send value that would put us over their reserve value"));
3555+
// Check self.local_channel_reserve_satoshis (the amount we must keep as
3556+
// reserve for the remote to have something to claim if we misbehave)
3557+
if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat {
3558+
return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value"));
35573559
}
35583560

35593561
// Now update local state:
@@ -4019,7 +4021,7 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
40194021
self.their_dust_limit_satoshis.write(writer)?;
40204022
self.our_dust_limit_satoshis.write(writer)?;
40214023
self.their_max_htlc_value_in_flight_msat.write(writer)?;
4022-
self.their_channel_reserve_satoshis.write(writer)?;
4024+
self.local_channel_reserve_satoshis.write(writer)?;
40234025
self.their_htlc_minimum_msat.write(writer)?;
40244026
self.our_htlc_minimum_msat.write(writer)?;
40254027
self.their_to_self_delay.write(writer)?;
@@ -4175,7 +4177,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
41754177
let their_dust_limit_satoshis = Readable::read(reader)?;
41764178
let our_dust_limit_satoshis = Readable::read(reader)?;
41774179
let their_max_htlc_value_in_flight_msat = Readable::read(reader)?;
4178-
let their_channel_reserve_satoshis = Readable::read(reader)?;
4180+
let local_channel_reserve_satoshis = Readable::read(reader)?;
41794181
let their_htlc_minimum_msat = Readable::read(reader)?;
41804182
let our_htlc_minimum_msat = Readable::read(reader)?;
41814183
let their_to_self_delay = Readable::read(reader)?;
@@ -4254,7 +4256,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
42544256
their_dust_limit_satoshis,
42554257
our_dust_limit_satoshis,
42564258
their_max_htlc_value_in_flight_msat,
4257-
their_channel_reserve_satoshis,
4259+
local_channel_reserve_satoshis,
42584260
their_htlc_minimum_msat,
42594261
our_htlc_minimum_msat,
42604262
their_to_self_delay,

‎lightning/src/ln/functional_tests.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ fn test_insane_channel_opens() {
6464
// Instantiate channel parameters where we push the maximum msats given our
6565
// funding satoshis
6666
let channel_value_sat = 31337; // same as funding satoshis
67-
let channel_reserve_satoshis = Channel::<EnforcingChannelKeys>::get_our_channel_reserve_satoshis(channel_value_sat);
67+
let channel_reserve_satoshis = Channel::<EnforcingChannelKeys>::get_remote_channel_reserve_satoshis(channel_value_sat);
6868
let push_msat = (channel_value_sat - channel_reserve_satoshis) * 1000;
6969

7070
// Have node0 initiate a channel to node1 with aforementioned parameters
@@ -1578,9 +1578,9 @@ fn do_channel_reserve_test(test_recv: bool) {
15781578
// attempt to get channel_reserve violation
15791579
let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value + 1);
15801580
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err },
1581-
assert_eq!(err, "Cannot send value that would put us over their reserve value"));
1581+
assert_eq!(err, "Cannot send value that would put us under local channel reserve value"));
15821582
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
1583-
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over their reserve value".to_string(), 1);
1583+
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 1);
15841584
}
15851585

15861586
// adding pending output
@@ -1603,9 +1603,9 @@ fn do_channel_reserve_test(test_recv: bool) {
16031603
{
16041604
let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1);
16051605
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err },
1606-
assert_eq!(err, "Cannot send value that would put us over their reserve value"));
1606+
assert_eq!(err, "Cannot send value that would put us under local channel reserve value"));
16071607
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
1608-
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over their reserve value".to_string(), 2);
1608+
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 2);
16091609
}
16101610

16111611
{
@@ -1640,7 +1640,7 @@ fn do_channel_reserve_test(test_recv: bool) {
16401640
assert_eq!(nodes[1].node.list_channels().len(), 1);
16411641
assert_eq!(nodes[1].node.list_channels().len(), 1);
16421642
let err_msg = check_closed_broadcast!(nodes[1], true).unwrap();
1643-
assert_eq!(err_msg.data, "Remote HTLC add would put them over their reserve value");
1643+
assert_eq!(err_msg.data, "Remote HTLC add would put them under their reserve value");
16441644
check_added_monitors!(nodes[1], 1);
16451645
return;
16461646
}
@@ -1666,9 +1666,9 @@ fn do_channel_reserve_test(test_recv: bool) {
16661666
{
16671667
let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_22+1);
16681668
unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &None), true, APIError::ChannelUnavailable { err },
1669-
assert_eq!(err, "Cannot send value that would put us over their reserve value"));
1669+
assert_eq!(err, "Cannot send value that would put us under local channel reserve value"));
16701670
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
1671-
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us over their reserve value".to_string(), 3);
1671+
nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot send value that would put us under local channel reserve value".to_string(), 3);
16721672
}
16731673

16741674
let (route_22, our_payment_hash_22, our_payment_preimage_22) = get_route_and_payment_hash!(recv_value_22);
@@ -5977,7 +5977,7 @@ fn test_update_add_htlc_bolt2_receiver_sender_can_afford_amount_sent() {
59775977

59785978
assert!(nodes[1].node.list_channels().is_empty());
59795979
let err_msg = check_closed_broadcast!(nodes[1], true).unwrap();
5980-
assert_eq!(err_msg.data, "Remote HTLC add would put them over their reserve value");
5980+
assert_eq!(err_msg.data, "Remote HTLC add would put them under their reserve value");
59815981
check_added_monitors!(nodes[1], 1);
59825982
}
59835983

0 commit comments

Comments
 (0)