Skip to content

Store override counterparty handshake limits until we enforce them #1292

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

Conversation

TheBlueMatt
Copy link
Collaborator

We currently allow users to provide an override_config in
ChannelManager::create_channel which it seems should apply to the
channel. However, because we don't store any of it, the only parts
which we apply to the channel are those which are set in the
Channel object immediately in Channel::new_outbound and used
from there.

This is great in most cases, however the
UserConfig::peer_channel_config_limits ChannelHandshakeLimits
object is used in accept_channel to bound what is acceptable in
our peer's AcceptChannel message. Thus, for outbound channels, we
are given a full UserConfig object to "override" the default
config, but we don't use any of the handshake limits specified in
it.

Here, we move to storing the ChannelHandshakeLimits explicitly
and applying it when we receive our peer's AcceptChannel. Note
that we don't need to store it anywhere because if we haven't
received an AcceptChannel from our peer when we reload from disk
we will forget the channel entirely anyway.

We currently allow users to provide an `override_config` in
`ChannelManager::create_channel` which it seems should apply to the
channel. However, because we don't store any of it, the only parts
which we apply to the channel are those which are set in the
`Channel` object immediately in `Channel::new_outbound` and used
from there.

This is great in most cases, however the
`UserConfig::peer_channel_config_limits` `ChannelHandshakeLimits`
object is used in `accept_channel` to bound what is acceptable in
our peer's `AcceptChannel` message. Thus, for outbound channels, we
are given a full `UserConfig` object to "override" the default
config, but we don't use any of the handshake limits specified in
it.

Here, we move to storing the `ChannelHandshakeLimits` explicitly
and applying it when we receive our peer's `AcceptChannel`. Note
that we don't need to store it anywhere because if we haven't
received an `AcceptChannel` from our peer when we reload from disk
we will forget the channel entirely anyway.
@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2022

Codecov Report

Merging #1292 (649af07) into main (482a2b9) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1292      +/-   ##
==========================================
- Coverage   90.41%   90.41%   -0.01%     
==========================================
  Files          71       71              
  Lines       38359    38364       +5     
==========================================
+ Hits        34684    34686       +2     
- Misses       3675     3678       +3     
Impacted Files Coverage Δ
lightning/src/ln/channel.rs 89.30% <73.68%> (+0.01%) ⬆️
lightning/src/ln/channelmanager.rs 84.47% <100.00%> (ø)
lightning/src/ln/functional_tests.rs 97.20% <0.00%> (-0.07%) ⬇️
lightning/src/util/config.rs 47.82% <0.00%> (+2.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 482a2b9...649af07. Read the comment docs.

@valentinewallace valentinewallace merged commit b8e9e8b into lightningdevkit:main Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants