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

Require inbound channels with anchor outputs to be accepted manually #2368

Merged

Conversation

wpaulino
Copy link
Contributor

Since the use of channels with anchor outputs requires a reserve of onchain funds to handle channel force closures, it would be irresponsible to allow a node to accept inbound channel without first consulting such reserves. To allow users to do so, we require such channels be manually accepted.

Depends on #2361 and #2367.

@wpaulino wpaulino added this to the 0.0.116 milestone Jun 20, 2023
@wpaulino wpaulino mentioned this pull request Jun 20, 2023
10 tasks
@wpaulino wpaulino force-pushed the inbound-anchors-manual-acceptance branch from 2329307 to ed14d0e Compare June 23, 2023 19:40
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino wpaulino force-pushed the inbound-anchors-manual-acceptance branch from ed14d0e to dece81e Compare June 23, 2023 21:27
TheBlueMatt
TheBlueMatt previously approved these changes Jun 23, 2023
Since the use of channels with anchor outputs requires a reserve of
onchain funds to handle channel force closures, it would be
irresponsible to allow a node to accept inbound channel without first
consulting such reserves. To allow users to do so, we require such
channels be manually accepted.
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 92.85% and project coverage change: +0.01 🎉

Comparison is base (3973865) 90.30% compared to head (e6348b8) 90.31%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2368      +/-   ##
==========================================
+ Coverage   90.30%   90.31%   +0.01%     
==========================================
  Files         106      106              
  Lines       54900    54948      +48     
  Branches    54900    54948      +48     
==========================================
+ Hits        49576    49628      +52     
+ Misses       5324     5320       -4     
Impacted Files Coverage Δ
lightning/src/chain/chainmonitor.rs 94.50% <ø> (-0.10%) ⬇️
lightning/src/chain/channelmonitor.rs 94.72% <ø> (ø)
lightning/src/chain/package.rs 91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs 41.55% <ø> (ø)
lightning/src/events/mod.rs 41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs 94.51% <ø> (ø)
lightning/src/sign/mod.rs 84.38% <ø> (+0.02%) ⬆️
lightning/src/util/config.rs 58.13% <ø> (+0.66%) ⬆️
lightning/src/util/enforcing_trait_impls.rs 88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs 93.06% <83.33%> (+0.34%) ⬆️
... and 4 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

TheBlueMatt
TheBlueMatt previously approved these changes Jun 24, 2023
@wpaulino wpaulino dismissed TheBlueMatt’s stale review June 24, 2023 15:26

The merge-base changed after approval.

dunxen
dunxen previously approved these changes Jun 24, 2023
@@ -1100,6 +1100,15 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, '
assert_eq!(open_channel_msg.temporary_channel_id, create_chan_id);
assert_eq!(node_a.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 42);
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &open_channel_msg);
if node_b.node.get_current_default_configuration().manually_accept_inbound_channels {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I was going to need this anyway for V2 establishment :)

#[test]
fn test_inbound_anchors_manual_acceptance() {
// Tests that we properly limit inbound channels when we have the manual-channel-acceptance
// flag set and (sometimes) accept channels as 0conf.
Copy link
Contributor

Choose a reason for hiding this comment

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

What conditions correspond with “sometimes” here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I copied another test as a template but forgot to update the comment here. Will fix in a future PR.

@wpaulino wpaulino dismissed dunxen’s stale review June 24, 2023 15:36

The merge-base changed after approval.

dunxen
dunxen previously approved these changes Jun 24, 2023
TheBlueMatt
TheBlueMatt previously approved these changes Jun 24, 2023
@wpaulino wpaulino dismissed stale reviews from TheBlueMatt and dunxen June 24, 2023 15:42

The merge-base changed after approval.

@TheBlueMatt
Copy link
Collaborator

Oh joy github is broken.

@TheBlueMatt TheBlueMatt merged commit d327c23 into lightningdevkit:main Jun 24, 2023
13 of 14 checks passed
@wpaulino wpaulino deleted the inbound-anchors-manual-acceptance branch June 24, 2023 17:24
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

4 participants