Skip to content
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

Added temporary_channel_id to create_channel. #2699

Merged

Conversation

mhrheaume
Copy link
Contributor

By default, LDK will generate the initial temporary channel ID for you. However, in certain cases, it's desirable to have a temporary channel ID specified by the caller in case of any pre-negotiation that needs to happen between peers prior to the channel open message. For example, LND has a FundingShim API that allows for advanced funding flows based on the temporary channel ID of the channel.

This patch adds support for optionally specifying the temporary channel ID of the channel through the create_channel API.

@mhrheaume mhrheaume force-pushed the mhr/temporary_channel_id branch 2 times, most recently from f383319 to 6d0ab1c Compare October 31, 2023 23:29
@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (281a0ae) 88.78% compared to head (bf39507) 88.79%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2699      +/-   ##
==========================================
+ Coverage   88.78%   88.79%   +0.01%     
==========================================
  Files         113      113              
  Lines       89116    89126      +10     
  Branches    89116    89126      +10     
==========================================
+ Hits        79123    79143      +20     
+ Misses       7752     7735      -17     
- Partials     2241     2248       +7     
Files Coverage Δ
lightning-background-processor/src/lib.rs 86.26% <ø> (ø)
lightning-invoice/src/utils.rs 94.74% <100.00%> (-0.07%) ⬇️
lightning/src/ln/async_signer_tests.rs 95.04% <100.00%> (ø)
lightning/src/ln/chanmon_update_fail_tests.rs 97.71% <100.00%> (ø)
lightning/src/ln/channel.rs 88.66% <100.00%> (+<0.01%) ⬆️
lightning/src/ln/functional_test_utils.rs 90.92% <100.00%> (-0.14%) ⬇️
lightning/src/ln/functional_tests.rs 97.28% <100.00%> (-0.03%) ⬇️
lightning/src/ln/payment_tests.rs 98.24% <100.00%> (ø)
lightning/src/ln/priv_short_conf_tests.rs 97.46% <100.00%> (ø)
lightning/src/ln/reload_tests.rs 96.34% <100.00%> (ø)
... and 2 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wpaulino
Copy link
Contributor

wpaulino commented Nov 3, 2023

LGTM after CI is fixed

wpaulino
wpaulino previously approved these changes Nov 3, 2023
@@ -5876,7 +5876,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
pub fn new<ES: Deref, F: Deref>(
fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP, counterparty_node_id: PublicKey, their_features: &InitFeatures,
channel_value_satoshis: u64, push_msat: u64, user_id: u128, config: &UserConfig, current_chain_height: u32,
outbound_scid_alias: u64
outbound_scid_alias: u64, temporary_channel_id_opt: Option<ChannelId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in channel_manager public api we named it just "temporary_channel_id", lets rename to that for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured I wouldn't be able to do that because I do let temporary_channel_id = ... below, but it looks like Rust allows it!

By default, LDK will generate the initial temporary channel ID for you.
However, in certain cases, it's desirable to have a temporary channel ID
specified by the caller in case of any pre-negotiation that needs to
happen between peers prior to the channel open message. For example, LND
has a `FundingShim` API that allows for advanced funding flows based on
the temporary channel ID of the channel.

This patch adds support for optionally specifying the temporary channel
ID of the channel through the `create_channel` API.
@TheBlueMatt TheBlueMatt merged commit 50eba26 into lightningdevkit:main Nov 5, 2023
15 checks passed
@mhrheaume mhrheaume deleted the mhr/temporary_channel_id branch November 6, 2023 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants