Skip to content

Allow configuring different max fee rate from peer #2643

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2144,19 +2144,20 @@ impl<SP: Deref> Channel<SP> where
<SP::Target as SignerProvider>::Signer: WriteableEcdsaChannelSigner
{
fn check_remote_fee<F: Deref, L: Deref>(
channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
feerate_per_kw: u32, cur_feerate_per_kw: Option<u32>, logger: &L
channel_type: &ChannelTypeFeatures, config: &ChannelConfig,
fee_estimator: &LowerBoundedFeeEstimator<F>, feerate_per_kw: u32,
cur_feerate_per_kw: Option<u32>, logger: &L
) -> Result<(), ChannelError> where F::Target: FeeEstimator, L::Target: Logger,
{
// We only bound the fee updates on the upper side to prevent completely absurd feerates,
// always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
// by default accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
// We generally don't care too much if they set the feerate to something very high, but it
// could result in the channel being useless due to everything being dust. This doesn't
// apply to channels supporting anchor outputs since HTLC transactions are pre-signed with a
// zero fee, so their fee is no longer considered to determine dust limits.
if !channel_type.supports_anchors_zero_fee_htlc_tx() {
let upper_limit = cmp::max(250 * 25,
fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
let upper_limit = cmp::max(config.base_max_accepted_fee_rate as u64,
fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * config.max_accepted_fee_rate_multiplier as u64);
if feerate_per_kw as u64 > upper_limit {
return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
}
Expand Down Expand Up @@ -3854,7 +3855,7 @@ impl<SP: Deref> Channel<SP> where
if self.context.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned()));
}
Channel::<SP>::check_remote_fee(&self.context.channel_type, fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?;
Channel::<SP>::check_remote_fee(&self.context.channel_type, &self.context.config(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?;
let feerate_over_dust_buffer = msg.feerate_per_kw > self.context.get_dust_buffer_feerate(None);

self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
Expand Down Expand Up @@ -6289,7 +6290,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
if msg.htlc_minimum_msat >= full_channel_value_msat {
return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat)));
}
Channel::<SP>::check_remote_fee(&channel_type, fee_estimator, msg.feerate_per_kw, None, logger)?;
Channel::<SP>::check_remote_fee(&channel_type, &config.channel_config, fee_estimator, msg.feerate_per_kw, None, logger)?;

let max_counterparty_selected_contest_delay = u16::min(config.channel_handshake_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
if msg.to_self_delay > max_counterparty_selected_contest_delay {
Expand Down Expand Up @@ -7642,7 +7643,7 @@ mod tests {
use crate::ln::PaymentHash;
use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
use crate::ln::channel::InitFeatures;
use crate::ln::channel::{Channel, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, commit_tx_fee_msat};
use crate::ln::channel::{Channel, ChannelConfig, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, commit_tx_fee_msat};
use crate::ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS, MIN_THEIR_CHAN_RESERVE_SATOSHIS};
use crate::ln::features::ChannelTypeFeatures;
use crate::ln::msgs::{ChannelUpdate, DecodeError, UnsignedChannelUpdate, MAX_VALUE_MSAT};
Expand Down Expand Up @@ -7691,7 +7692,7 @@ mod tests {
let fee_est = TestFeeEstimator { fee_est: 42 };
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_est);
assert!(Channel::<&TestKeysInterface>::check_remote_fee(
&ChannelTypeFeatures::only_static_remote_key(), &bounded_fee_estimator,
&ChannelTypeFeatures::only_static_remote_key(), &ChannelConfig::default(), &bounded_fee_estimator,
u32::max_value(), None, &&test_utils::TestLogger::new()).is_err());
}

Expand Down
34 changes: 34 additions & 0 deletions lightning/src/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,24 @@ pub struct ChannelConfig {
/// [`PaymentClaimable::counterparty_skimmed_fee_msat`]: crate::events::Event::PaymentClaimable::counterparty_skimmed_fee_msat
// TODO: link to bLIP when it's merged
pub accept_underpaying_htlcs: bool,

/// When checking our channel counterparty's fee rate, we will allow any value that is up to the greater
/// between [`ChannelConfig::base_max_accepted_fee_rate`] and [`ChannelConfig::max_accepted_fee_rate_multiplier`]
/// times our [`HighPriority`] estimated feerate. This is only used for non-anchor channels.
///
/// Default value: 25 sat/vbyte.
///
/// [`HighPriority`]: crate::chain::chaininterface::ConfirmationTarget::HighPriority
pub base_max_accepted_fee_rate: u32,

/// When checking our channel counterparty's fee rate, we will allow any value that is up to the greater
/// between [`ChannelConfig::base_max_accepted_fee_rate`] and [`ChannelConfig::max_accepted_fee_rate_multiplier`]
/// times our [`HighPriority`] estimated feerate. This is only used for non-anchor channels.
///
/// Default value: 10.
///
/// [`HighPriority`]: crate::chain::chaininterface::ConfirmationTarget::HighPriority
pub max_accepted_fee_rate_multiplier: u32,
}

impl ChannelConfig {
Expand Down Expand Up @@ -518,6 +536,8 @@ impl Default for ChannelConfig {
max_dust_htlc_exposure: MaxDustHTLCExposure::FeeRateMultiplier(5000),
force_close_avoidance_max_fee_satoshis: 1000,
accept_underpaying_htlcs: false,
base_max_accepted_fee_rate: 25 * 250, // 25 sat/vbyte
max_accepted_fee_rate_multiplier: 10,
}
}
}
Expand All @@ -539,6 +559,8 @@ impl crate::util::ser::Writeable for ChannelConfig {
// LegacyChannelConfig. To make sure that serialization is not compatible with this one, we use
// the next required type of 10, which if seen by the old serialization will always fail.
(10, self.force_close_avoidance_max_fee_satoshis, required),
(11, self.base_max_accepted_fee_rate, required),
(13, self.max_accepted_fee_rate_multiplier, required),
});
Ok(())
}
Expand All @@ -553,6 +575,8 @@ impl crate::util::ser::Readable for ChannelConfig {
let mut max_dust_htlc_exposure_msat = None;
let mut max_dust_htlc_exposure_enum = None;
let mut force_close_avoidance_max_fee_satoshis = 1000;
let mut base_max_accepted_fee_rate = 25 * 250;
let mut max_accepted_fee_rate_multiplier = 10;
read_tlv_fields!(reader, {
(0, forwarding_fee_proportional_millionths, required),
(1, accept_underpaying_htlcs, (default_value, false)),
Expand All @@ -562,6 +586,8 @@ impl crate::util::ser::Readable for ChannelConfig {
// Has always been written, but became optionally read in 0.0.116
(6, max_dust_htlc_exposure_msat, option),
(10, force_close_avoidance_max_fee_satoshis, required),
(11, base_max_accepted_fee_rate, (default_value, 25 * 250 as u32)),
(13, max_accepted_fee_rate_multiplier, (default_value, 10_u32)),
});
let max_dust_htlc_fixed_limit = max_dust_htlc_exposure_msat.unwrap_or(5_000_000);
let max_dust_htlc_exposure_msat = max_dust_htlc_exposure_enum
Expand All @@ -573,6 +599,8 @@ impl crate::util::ser::Readable for ChannelConfig {
cltv_expiry_delta,
max_dust_htlc_exposure: max_dust_htlc_exposure_msat,
force_close_avoidance_max_fee_satoshis,
base_max_accepted_fee_rate,
max_accepted_fee_rate_multiplier,
})
}
}
Expand Down Expand Up @@ -666,6 +694,8 @@ impl crate::util::ser::Readable for LegacyChannelConfig {
let mut commit_upfront_shutdown_pubkey = false;
let mut forwarding_fee_base_msat = 0;
let mut max_dust_htlc_exposure_enum = None;
let mut base_max_accepted_fee_rate = 25 * 250;
let mut max_accepted_fee_rate_multiplier = 10;
read_tlv_fields!(reader, {
(0, forwarding_fee_proportional_millionths, required),
// Has always been written, but became optionally read in 0.0.116
Expand All @@ -676,6 +706,8 @@ impl crate::util::ser::Readable for LegacyChannelConfig {
(5, max_dust_htlc_exposure_enum, option),
(6, commit_upfront_shutdown_pubkey, required),
(8, forwarding_fee_base_msat, required),
(11, base_max_accepted_fee_rate, (default_value, 25 * 250 as u32)),
(13, max_accepted_fee_rate_multiplier, (default_value, 10_u32)),
});
let max_dust_htlc_exposure_msat_fixed_limit =
max_dust_htlc_exposure_msat_fixed_limit.unwrap_or(5_000_000);
Expand All @@ -689,6 +721,8 @@ impl crate::util::ser::Readable for LegacyChannelConfig {
force_close_avoidance_max_fee_satoshis,
forwarding_fee_base_msat,
accept_underpaying_htlcs: false,
base_max_accepted_fee_rate,
max_accepted_fee_rate_multiplier,
},
announced_channel,
commit_upfront_shutdown_pubkey,
Expand Down