-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new config setting max_balance_dust_htlc_msat
#1009
Merged
TheBlueMatt
merged 5 commits into
lightningdevkit:main
from
ariard:2021-07-add-forward-dust-limit
Aug 10, 2021
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
01bdc15
Add additional TLV serialization type of (default_value, N)
TheBlueMatt 53b6823
Add new config setting `max_balance_dust_htlc_msat`
ariard 29e755b
Modify pending inbound/outbound getters to access dust balances
ariard 1cf2b53
Enforce `max_balance_dust_htlc_msat` at HTLC reception/forward
ariard 730f6f3
Add test_max_balance_dust_htlc
ariard File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -274,6 +274,14 @@ enum HTLCInitiator { | |
RemoteOffered, | ||
} | ||
|
||
/// An enum gathering stats on pending HTLCs, either inbound or outbound side. | ||
struct HTLCStats { | ||
pending_htlcs: u32, | ||
pending_htlcs_value_msat: u64, | ||
on_counterparty_tx_dust_exposure_msat: u64, | ||
on_holder_tx_dust_exposure_msat: u64, | ||
} | ||
|
||
/// Used when calculating whether we or the remote can afford an additional HTLC. | ||
struct HTLCCandidate { | ||
amount_msat: u64, | ||
|
@@ -1816,32 +1824,63 @@ impl<Signer: Sign> Channel<Signer> { | |
Ok(()) | ||
} | ||
|
||
/// Returns (inbound_htlc_count, htlc_inbound_value_msat) | ||
fn get_inbound_pending_htlc_stats(&self) -> (u32, u64) { | ||
let mut htlc_inbound_value_msat = 0; | ||
/// Returns a HTLCStats about inbound pending htlcs | ||
fn get_inbound_pending_htlc_stats(&self) -> HTLCStats { | ||
let mut stats = HTLCStats { | ||
pending_htlcs: self.pending_inbound_htlcs.len() as u32, | ||
pending_htlcs_value_msat: 0, | ||
on_counterparty_tx_dust_exposure_msat: 0, | ||
on_holder_tx_dust_exposure_msat: 0, | ||
}; | ||
|
||
let counterparty_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; | ||
let holder_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; | ||
for ref htlc in self.pending_inbound_htlcs.iter() { | ||
htlc_inbound_value_msat += htlc.amount_msat; | ||
stats.pending_htlcs_value_msat += htlc.amount_msat; | ||
if htlc.amount_msat / 1000 < counterparty_dust_limit_timeout_sat { | ||
stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; | ||
} | ||
if htlc.amount_msat / 1000 < holder_dust_limit_success_sat { | ||
stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; | ||
} | ||
} | ||
(self.pending_inbound_htlcs.len() as u32, htlc_inbound_value_msat) | ||
stats | ||
} | ||
|
||
/// Returns (outbound_htlc_count, htlc_outbound_value_msat) *including* pending adds in our | ||
/// holding cell. | ||
fn get_outbound_pending_htlc_stats(&self) -> (u32, u64) { | ||
let mut htlc_outbound_value_msat = 0; | ||
/// Returns a HTLCStats about pending outbound htlcs, *including* pending adds in our holding cell. | ||
fn get_outbound_pending_htlc_stats(&self) -> HTLCStats { | ||
let mut stats = HTLCStats { | ||
pending_htlcs: self.pending_outbound_htlcs.len() as u32, | ||
pending_htlcs_value_msat: 0, | ||
on_counterparty_tx_dust_exposure_msat: 0, | ||
on_holder_tx_dust_exposure_msat: 0, | ||
}; | ||
|
||
let counterparty_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let holder_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; | ||
for ref htlc in self.pending_outbound_htlcs.iter() { | ||
htlc_outbound_value_msat += htlc.amount_msat; | ||
stats.pending_htlcs_value_msat += htlc.amount_msat; | ||
if htlc.amount_msat / 1000 < counterparty_dust_limit_success_sat { | ||
stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; | ||
} | ||
if htlc.amount_msat / 1000 < holder_dust_limit_timeout_sat { | ||
stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; | ||
} | ||
} | ||
|
||
let mut htlc_outbound_count = self.pending_outbound_htlcs.len(); | ||
for update in self.holding_cell_htlc_updates.iter() { | ||
if let &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } = update { | ||
htlc_outbound_count += 1; | ||
htlc_outbound_value_msat += amount_msat; | ||
stats.pending_htlcs += 1; | ||
stats.pending_htlcs_value_msat += amount_msat; | ||
if *amount_msat / 1000 < counterparty_dust_limit_success_sat { | ||
stats.on_counterparty_tx_dust_exposure_msat += amount_msat; | ||
} | ||
if *amount_msat / 1000 < holder_dust_limit_timeout_sat { | ||
stats.on_holder_tx_dust_exposure_msat += amount_msat; | ||
} | ||
} | ||
} | ||
|
||
(htlc_outbound_count as u32, htlc_outbound_value_msat) | ||
stats | ||
} | ||
|
||
/// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat. | ||
|
@@ -1853,11 +1892,11 @@ impl<Signer: Sign> Channel<Signer> { | |
( | ||
cmp::max(self.channel_value_satoshis as i64 * 1000 | ||
- self.value_to_self_msat as i64 | ||
- self.get_inbound_pending_htlc_stats().1 as i64 | ||
- self.get_inbound_pending_htlc_stats().pending_htlcs_value_msat as i64 | ||
- Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000, | ||
0) as u64, | ||
cmp::max(self.value_to_self_msat as i64 | ||
- self.get_outbound_pending_htlc_stats().1 as i64 | ||
- self.get_outbound_pending_htlc_stats().pending_htlcs_value_msat as i64 | ||
- self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, | ||
0) as u64 | ||
) | ||
|
@@ -2069,12 +2108,13 @@ impl<Signer: Sign> Channel<Signer> { | |
return Err(ChannelError::Close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.holder_htlc_minimum_msat, msg.amount_msat))); | ||
} | ||
|
||
let (inbound_htlc_count, htlc_inbound_value_msat) = self.get_inbound_pending_htlc_stats(); | ||
if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 { | ||
let inbound_stats = self.get_inbound_pending_htlc_stats(); | ||
let outbound_stats = self.get_outbound_pending_htlc_stats(); | ||
if inbound_stats.pending_htlcs + 1 > OUR_MAX_HTLCS as u32 { | ||
return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", OUR_MAX_HTLCS))); | ||
} | ||
let holder_max_htlc_value_in_flight_msat = Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis); | ||
if htlc_inbound_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { | ||
if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { | ||
return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", holder_max_htlc_value_in_flight_msat))); | ||
} | ||
// Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet | ||
|
@@ -2098,8 +2138,28 @@ impl<Signer: Sign> Channel<Signer> { | |
} | ||
} | ||
|
||
let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; | ||
if msg.amount_msat / 1000 < exposure_dust_limit_timeout_sats { | ||
let on_counterparty_tx_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + msg.amount_msat; | ||
if on_counterparty_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { | ||
log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", | ||
on_counterparty_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); | ||
pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); | ||
} | ||
} | ||
|
||
let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; | ||
if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { | ||
let on_holder_tx_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + msg.amount_msat; | ||
if on_holder_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { | ||
log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", | ||
on_holder_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); | ||
pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); | ||
} | ||
} | ||
|
||
let pending_value_to_self_msat = | ||
self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat; | ||
self.value_to_self_msat + inbound_stats.pending_htlcs_value_msat - removed_outbound_total_msat; | ||
let pending_remote_value_msat = | ||
self.channel_value_satoshis * 1000 - pending_value_to_self_msat; | ||
if pending_remote_value_msat < msg.amount_msat { | ||
|
@@ -3502,11 +3562,24 @@ impl<Signer: Sign> Channel<Signer> { | |
cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) | ||
} | ||
|
||
pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 { | ||
self.config.max_dust_htlc_exposure_msat | ||
} | ||
|
||
#[cfg(test)] | ||
pub fn get_feerate(&self) -> u32 { | ||
self.feerate_per_kw | ||
} | ||
|
||
pub fn get_dust_buffer_feerate(&self) -> u32 { | ||
ariard marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: s/feerate/feerate_per_kw There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but we express sat per kw in the whole codebase so kind of implicit I think. |
||
// When calculating our exposure to dust HTLCs, we assume that the channel feerate | ||
// may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, | ||
// whichever is higher. This ensures that we aren't suddenly exposed to significantly | ||
// more dust balance if the feerate increases when we have several HTLCs pending | ||
// which are near the dust limit. | ||
cmp::max(2530, self.feerate_per_kw * 1250 / 1000) | ||
ariard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 { | ||
self.cur_holder_commitment_transaction_number + 1 | ||
} | ||
|
@@ -4145,12 +4218,13 @@ impl<Signer: Sign> Channel<Signer> { | |
return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned())); | ||
} | ||
|
||
let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats(); | ||
if outbound_htlc_count + 1 > self.counterparty_max_accepted_htlcs as u32 { | ||
let inbound_stats = self.get_inbound_pending_htlc_stats(); | ||
let outbound_stats = self.get_outbound_pending_htlc_stats(); | ||
if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 { | ||
return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.counterparty_max_accepted_htlcs))); | ||
} | ||
// Check their_max_htlc_value_in_flight_msat | ||
if htlc_outbound_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { | ||
if outbound_stats.pending_htlcs_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { | ||
return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat))); | ||
} | ||
|
||
|
@@ -4165,7 +4239,25 @@ impl<Signer: Sign> Channel<Signer> { | |
} | ||
} | ||
|
||
let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat; | ||
let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; | ||
if amount_msat / 1000 < exposure_dust_limit_success_sats { | ||
let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + amount_msat; | ||
if on_counterparty_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { | ||
return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", | ||
on_counterparty_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); | ||
} | ||
} | ||
|
||
let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; | ||
if amount_msat / 1000 < exposure_dust_limit_timeout_sats { | ||
let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + amount_msat; | ||
if on_holder_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { | ||
return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", | ||
on_holder_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); | ||
} | ||
} | ||
|
||
let pending_value_to_self_msat = self.value_to_self_msat - outbound_stats.pending_htlcs_value_msat; | ||
if pending_value_to_self_msat < amount_msat { | ||
return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, pending_value_to_self_msat))); | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggested nit: s/value/balance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think balance includes either the
to_local
orto_remote
output so would like to avoid blurring meanings here.