Skip to content

BOLT #7: Make channel_update always have htlc_maximum_msat.#996

Closed
rustyrussell wants to merge 2 commits into
masterfrom
remove-option_channel_htlc_max
Closed

BOLT #7: Make channel_update always have htlc_maximum_msat.#996
rustyrussell wants to merge 2 commits into
masterfrom
remove-option_channel_htlc_max

Conversation

@rustyrussell

@rustyrussell rustyrussell commented May 31, 2022

Copy link
Copy Markdown
Collaborator

I think everyone sets this. Vastly simplifies parsing.

Indeed, every single of the 149964 channel updates seen by my node has his set.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker

cdecker commented May 31, 2022

Copy link
Copy Markdown
Collaborator

ACK 466317a

@Roasbeef Roasbeef left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🏂

Would be good to run some basic analysis just to check what % of channels aren't setting it.

@vincenzopalazzo vincenzopalazzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Concept ack for the "metrics"

@t-bast t-bast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Concept ACK, I think we can remove even more stuff

Comment thread 07-routing-gossip.md
Suggested-by: @t-bast
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
t-bast
t-bast previously approved these changes Jun 1, 2022

@t-bast t-bast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACK f99b549

@t-bast

t-bast commented Jun 6, 2022

Copy link
Copy Markdown
Collaborator

As noted in lightningdevkit/rust-lightning#1515 and discussed during the dev summit, it would also make sense to verify that every implementation sets this value to max_htlc_value_in_flight_msat instead of using the channel capacity.

@t-bast

t-bast commented Jun 10, 2022

Copy link
Copy Markdown
Collaborator

Retracting my ACK, I made a proposal at #999 that would benefit from message flags (but I did make htlc_maximum_msat mandatory, which was the goal of this PR).

@arik-so

arik-so commented Jul 15, 2022

Copy link
Copy Markdown

I have just downloaded a fresh batch of 134k channel updates from an LND peer; of those channel updates, 364 don't have an HTLC max set, which is 0.27%.

I have separately downloaded 161k updates from a different peer, and got 399 instances of an unset HTLC max, or 0.25%.

t-bast added a commit to ACINQ/eclair that referenced this pull request Aug 3, 2022
The specification is removing support for old channel updates that didn't
include an `htlc_maximum_msat` (lightning/bolts#996).

Every implementation has been generating updates containing this field for
years, so we can safely reject updates that don't contain it.
t-bast added a commit to ACINQ/eclair that referenced this pull request Aug 3, 2022
The specification is removing support for old channel updates that didn't
include an `htlc_maximum_msat` (lightning/bolts#996).

Every implementation has been generating updates containing this field for
years, so we can safely reject updates that don't contain it.
t-bast added a commit to ACINQ/eclair that referenced this pull request Aug 3, 2022
The specification is removing support for old channel updates that didn't
include an `htlc_maximum_msat` (lightning/bolts#996).

Every implementation has been generating updates containing this field for
years, so we can safely reject updates that don't contain it.
t-bast added a commit to ACINQ/eclair that referenced this pull request Aug 8, 2022
The specification is removing support for old channel updates that didn't
include an `htlc_maximum_msat` (lightning/bolts#996).

Every implementation has been generating updates containing this field for
years, so we can safely reject updates that don't contain it.
@Roasbeef

Copy link
Copy Markdown
Collaborator

Replaced by #999

@Roasbeef Roasbeef closed this Aug 15, 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.

7 participants