-
Notifications
You must be signed in to change notification settings - Fork 113
Use lightning::util::anchor_channel_reserves
#674
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,9 @@ use lightning::ln::channel_state::ChannelShutdownState; | |
| use lightning::ln::channelmanager::PaymentId; | ||
| use lightning::ln::msgs::SocketAddress; | ||
| use lightning::routing::gossip::NodeAlias; | ||
| use lightning::util::anchor_channel_reserves::{ | ||
| get_reserve_per_channel, AnchorChannelReserveContext, | ||
| }; | ||
| use lightning::util::persist::KVStoreSync; | ||
| use lightning_background_processor::process_events_async; | ||
| use liquidity::{LSPS1Liquidity, LiquiditySource}; | ||
|
|
@@ -1069,7 +1072,7 @@ impl Node { | |
| if init_features.requires_anchors_zero_fee_htlc_tx() | ||
| && !c.trusted_peers_no_reserve.contains(&node_id) | ||
| { | ||
| c.per_channel_reserve_sats | ||
| get_reserve_per_channel(&AnchorChannelReserveContext::default()).to_sat() | ||
| } else { | ||
| 0 | ||
| } | ||
|
|
@@ -1132,13 +1135,11 @@ impl Node { | |
| /// channel counterparty on channel open. This can be useful to start out with the balance not | ||
| /// entirely shifted to one side, therefore allowing to receive payments from the getgo. | ||
| /// | ||
| /// If Anchor channels are enabled, this will ensure the configured | ||
| /// [`AnchorChannelsConfig::per_channel_reserve_sats`] is available and will be retained before | ||
| /// opening the channel. | ||
| /// If Anchor channels are enabled, this will ensure the reserved amount per | ||
| /// channel is available and will be retained before opening the channel. | ||
| /// | ||
| /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. | ||
| /// | ||
| /// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats | ||
| pub fn open_channel( | ||
| &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, | ||
| push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>, | ||
|
|
@@ -1167,13 +1168,11 @@ impl Node { | |
| /// channel counterparty on channel open. This can be useful to start out with the balance not | ||
| /// entirely shifted to one side, therefore allowing to receive payments from the getgo. | ||
| /// | ||
| /// If Anchor channels are enabled, this will ensure the configured | ||
| /// [`AnchorChannelsConfig::per_channel_reserve_sats`] is available and will be retained before | ||
| /// opening the channel. | ||
| /// If Anchor channels are enabled, this will ensure the reserved amount per | ||
| /// channel is available and will be retained before opening the channel. | ||
| /// | ||
| /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. | ||
| /// | ||
| /// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats | ||
| pub fn open_announced_channel( | ||
| &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, | ||
| push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>, | ||
|
|
@@ -1580,6 +1579,8 @@ impl_writeable_tlv_based!(NodeMetrics, { | |
| pub(crate) fn total_anchor_channels_reserve_sats( | ||
| channel_manager: &ChannelManager, config: &Config, | ||
| ) -> u64 { | ||
| let reserve_sat_per_channel = get_anchor_reserve_per_channel(); | ||
|
|
||
| config.anchor_channels_config.as_ref().map_or(0, |anchor_channels_config| { | ||
| channel_manager | ||
| .list_channels() | ||
|
|
@@ -1593,6 +1594,12 @@ pub(crate) fn total_anchor_channels_reserve_sats( | |
| .map_or(false, |t| t.requires_anchors_zero_fee_htlc_tx()) | ||
| }) | ||
| .count() as u64 | ||
| * anchor_channels_config.per_channel_reserve_sats | ||
| * reserve_sat_per_channel | ||
| }) | ||
| } | ||
|
|
||
| /// Returns the configured anchor channel reserve per channel in satoshis. | ||
| pub fn get_anchor_reserve_per_channel() -> u64 { | ||
|
Collaborator
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. I don't think we should make this public, esp. given the only other call site is our tests. |
||
| let context = AnchorChannelReserveContext::default(); | ||
| get_reserve_per_channel(&context).to_sat() | ||
|
Collaborator
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. Hmm, it seems the current defaults would result in a channel reserve of 336513sats (~$300) per channel. This seems very conservative, and much too high for the typical use case we currently expect in LDK Node. This makes me wonder if we either need to use different defaults, or not opt for dynamic estimation right now. Will discuss this with a few other devs and get back to this. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ use electrsd::corepc_node::{Client as BitcoindClient, Node as BitcoinD}; | |
| use electrsd::{corepc_node, ElectrsD}; | ||
| use electrum_client::ElectrumApi; | ||
| use ldk_node::config::{AsyncPaymentsRole, Config, ElectrumSyncConfig, EsploraSyncConfig}; | ||
| use ldk_node::get_anchor_reserve_per_channel; | ||
| use ldk_node::io::sqlite_store::SqliteStore; | ||
| use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus}; | ||
| use ldk_node::{ | ||
|
|
@@ -640,8 +641,9 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>( | |
| let addr_a = node_a.onchain_payment().new_address().unwrap(); | ||
| let addr_b = node_b.onchain_payment().new_address().unwrap(); | ||
|
|
||
| let premine_amount_sat = if expect_anchor_channel { 2_125_000 } else { 2_100_000 }; | ||
| let anchor_reserve = if expect_anchor_channel { get_anchor_reserve_per_channel() } else { 0 }; | ||
|
Collaborator
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. Rather than calling this to get the dynamic value, can we still keep a fixated value here that would also allow us to check if the method works as expected and detect if/when something changes. |
||
|
|
||
| let premine_amount_sat = 2_100_000 + anchor_reserve; | ||
| premine_and_distribute_funds( | ||
| &bitcoind, | ||
| electrsd, | ||
|
|
@@ -725,7 +727,8 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>( | |
| ); | ||
|
|
||
| let onchain_fee_buffer_sat = 5000; | ||
| let node_a_anchor_reserve_sat = if expect_anchor_channel { 25_000 } else { 0 }; | ||
| let node_a_anchor_reserve_sat = | ||
| if expect_anchor_channel { get_anchor_reserve_per_channel() } else { 0 }; | ||
| let node_a_upper_bound_sat = | ||
| premine_amount_sat - node_a_anchor_reserve_sat - funding_amount_sat; | ||
| let node_a_lower_bound_sat = premine_amount_sat | ||
|
|
@@ -746,7 +749,7 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>( | |
| { | ||
| 0 | ||
| } else { | ||
| 25_000 | ||
| get_anchor_reserve_per_channel() | ||
| }; | ||
| assert_eq!( | ||
| node_b.list_balances().spendable_onchain_balance_sats, | ||
|
|
||
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 we might want to keep this as an override and make it an
Option<u64>so that user can still set a specific value if they want. But probably the default should beNone, i.e., dynamic estimation.