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

Symmetrical HTLC limits #1828

Merged
merged 1 commit into from
May 28, 2021
Merged

Symmetrical HTLC limits #1828

merged 1 commit into from
May 28, 2021

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented May 27, 2021

The spec defines max_accepted_htlcs and max_htlc_value_in_flight_msat to let nodes reduce their exposure to pending HTLCs. This only applies to received HTLCs, and we use the remote peer's values for outgoing HTLCs.

But when we're more restrictive than our peer, it makes sense to apply our limits to outgoing HTLCs as well.

The spec defines `max_accepted_htlcs` and `max_htlc_value_in_flight_msat`
to let nodes reduce their exposure to pending HTLCs. This only applies to
received HTLCs, and we use the remote peer's values for outgoing HTLCs.

But when we're more restrictive than our peer, it makes sense to apply our
limits to outgoing HTLCs as well.
@t-bast t-bast requested review from pm47 and sstone May 27, 2021 12:42
@codecov-commenter
Copy link

Codecov Report

Merging #1828 (4f554c4) into master (43a89f8) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1828      +/-   ##
==========================================
+ Coverage   89.32%   89.36%   +0.03%     
==========================================
  Files         145      145              
  Lines       10980    10980              
  Branches      432      442      +10     
==========================================
+ Hits         9808     9812       +4     
+ Misses       1172     1168       -4     
Impacted Files Coverage Δ
...in/scala/fr/acinq/eclair/channel/Commitments.scala 93.57% <100.00%> (ø)
.../acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala 96.09% <0.00%> (-0.79%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 86.27% <0.00%> (+0.08%) ⬆️
...main/scala/fr/acinq/eclair/router/Validation.scala 92.30% <0.00%> (+1.53%) ⬆️

if (outgoingHtlcs.size > commitments1.remoteParams.maxAcceptedHtlcs) {
return Left(TooManyAcceptedHtlcs(commitments.channelId, maximum = commitments1.remoteParams.maxAcceptedHtlcs))
if (Seq(commitments1.localParams.maxAcceptedHtlcs, commitments1.remoteParams.maxAcceptedHtlcs).min < outgoingHtlcs.size) {
return Left(TooManyAcceptedHtlcs(commitments.channelId, maximum = Seq(commitments1.localParams.maxAcceptedHtlcs, commitments1.remoteParams.maxAcceptedHtlcs).min))
Copy link
Member

Choose a reason for hiding this comment

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

We could also add a new TooManyOfferedHtlcs class here but I agree it's not needed.

@t-bast t-bast merged commit af618bc into master May 28, 2021
@t-bast t-bast deleted the symmetric-htlc-limits branch May 28, 2021 15:13
sstone pushed a commit that referenced this pull request Jun 3, 2021
The spec defines `max_accepted_htlcs` and `max_htlc_value_in_flight_msat`
to let nodes reduce their exposure to pending HTLCs. This only applies to
received HTLCs, and we use the remote peer's values for outgoing HTLCs.

But when we're more restrictive than our peer, it makes sense to apply our
limits to outgoing HTLCs as well.
sstone pushed a commit that referenced this pull request Jun 3, 2021
The spec defines `max_accepted_htlcs` and `max_htlc_value_in_flight_msat`
to let nodes reduce their exposure to pending HTLCs. This only applies to
received HTLCs, and we use the remote peer's values for outgoing HTLCs.

But when we're more restrictive than our peer, it makes sense to apply our
limits to outgoing HTLCs as well.
dpad85 pushed a commit that referenced this pull request Jun 3, 2021
The spec defines `max_accepted_htlcs` and `max_htlc_value_in_flight_msat`
to let nodes reduce their exposure to pending HTLCs. This only applies to
received HTLCs, and we use the remote peer's values for outgoing HTLCs.

But when we're more restrictive than our peer, it makes sense to apply our
limits to outgoing HTLCs as well.

Co-authored-by: Bastien Teinturier <[email protected]>
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.

3 participants