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.

Sign up for GitHub

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

add max_total_htlc_out to listpeerchannels #7193

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add max_total_htlc_out to listpeerchannels #7193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bb2462
add max_htlc_value_in_flight_msat to listpeerchan
Lagrang3 Apr 2, 2024
ddb994f
fetch max total htlc value from listpeerchannel
Lagrang3 Oct 31, 2024
ed35ad8
add autogenerated files
Lagrang3 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .msggen.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2657,6 +2657,7 @@
"ListPeerChannels.channels[].next_fee_step": 15,
"ListPeerChannels.channels[].next_feerate": 14,
"ListPeerChannels.channels[].opener": 19,
"ListPeerChannels.channels[].our_max_htlc_value_in_flight_msat": 62,
"ListPeerChannels.channels[].our_reserve_msat": 32,
"ListPeerChannels.channels[].our_to_self_delay": 39,
"ListPeerChannels.channels[].out_fulfilled_msat": 51,
Expand All @@ -2675,6 +2676,7 @@
"ListPeerChannels.channels[].state": 3,
"ListPeerChannels.channels[].state_changes[]": 42,
"ListPeerChannels.channels[].status[]": 43,
"ListPeerChannels.channels[].their_max_htlc_value_in_flight_msat": 61,
"ListPeerChannels.channels[].their_reserve_msat": 31,
"ListPeerChannels.channels[].their_to_self_delay": 38,
"ListPeerChannels.channels[].to_us_msat": 23,
Expand Down Expand Up @@ -10053,7 +10055,7 @@
},
"ListPeerChannels.channels[].max_total_htlc_in_msat": {
"added": "v23.02",
"deprecated": null
"deprecated": "v24.11"
},
"ListPeerChannels.channels[].maximum_htlc_out_msat": {
"added": "v23.02",
Expand Down Expand Up @@ -10083,6 +10085,10 @@
"added": "v23.02",
"deprecated": null
},
"ListPeerChannels.channels[].our_max_htlc_value_in_flight_msat": {
"added": "v24.11",
"deprecated": null
},
"ListPeerChannels.channels[].our_reserve_msat": {
"added": "v23.02",
"deprecated": null
Expand Down Expand Up @@ -10175,6 +10181,10 @@
"added": "v23.02",
"deprecated": null
},
"ListPeerChannels.channels[].their_max_htlc_value_in_flight_msat": {
"added": "v24.11",
"deprecated": null
},
"ListPeerChannels.channels[].their_reserve_msat": {
"added": "v23.02",
"deprecated": null
Expand Down
2 changes: 2 additions & 0 deletions cln-grpc/proto/node.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion cln-grpc/src/convert.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions cln-rpc/src/model.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions common/gossmods_listpeerchannels.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ void gossmod_add_localchan(struct gossmap_localmods *mods,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat spendable,
struct amount_msat total_htlcmax,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand Down Expand Up @@ -52,6 +53,7 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat sr_able,
struct amount_msat max_total_htlc,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand All @@ -72,6 +74,7 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,
bool enabled;
struct node_id dst;
struct amount_msat capacity_msat, spendable, receivable, fee_base[NUM_SIDES], htlc_min[NUM_SIDES], htlc_max[NUM_SIDES];
struct amount_msat max_total_in_htlc, max_total_out_htlc;
u32 fee_proportional[NUM_SIDES], cltv_delta[NUM_SIDES];
const char *state, *err;

Expand All @@ -87,6 +90,8 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,
"direction?:%,"
"spendable_msat?:%,"
"receivable_msat?:%,"
"our_max_htlc_value_in_flight_msat?:%,"
"their_max_htlc_value_in_flight_msat?:%,"
"peer_connected:%,"
"state:%,"
"peer_id:%,"
Expand All @@ -109,6 +114,8 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,
JSON_SCAN(json_to_int, &scidd.dir),
JSON_SCAN(json_to_msat, &spendable),
JSON_SCAN(json_to_msat, &receivable),
JSON_SCAN(json_to_msat, &max_total_in_htlc),
JSON_SCAN(json_to_msat, &max_total_out_htlc),
Copy link
Contributor

Choose a reason for hiding this comment

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

how often does this get updated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The max_htlc_value_in_flight pair is fixed for the entire lifetime of the channel.
The gossmod_from_listpeerchannels is called to parse the response from listpeerchannels.
In askrene or renepay this is expected to happen once per rpc call.

JSON_SCAN(json_to_bool, &enabled),
JSON_SCAN_TAL(tmpctx, json_strdup, &state),
JSON_SCAN(json_to_node_id, &dst),
Expand Down Expand Up @@ -155,7 +162,7 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,
/* We add both directions */
cb(mods, self, &dst, &scidd, capacity_msat,
htlc_min[LOCAL], htlc_max[LOCAL],
spendable, fee_base[LOCAL], fee_proportional[LOCAL],
spendable, max_total_out_htlc, fee_base[LOCAL], fee_proportional[LOCAL],
Copy link
Contributor

Choose a reason for hiding this comment

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

just to confirm my understanding: max_total_out_htlc is the total amount that we can receive, so it gets added as the total max on the local receiving channel; max_total_in_htlc is the total amount we can send, so it gets added to the REMOTE set.

maybe renaming these would make this clearer? max_total_out seems to imply amount we can receive and vice versa for total_in lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rustyrussell: help!

From the spec BOLT02:

A sending node:
    - if the total value of offered HTLCs would exceed the remote's `max_htlc_value_in_flight_msat`:
        - MUST NOT add an HTLC.

A receiving node:
    - if a sending node adds more than receiver `max_value_in_flight_msat` worth of offered HTLCs to its local commitment transaction:
        - SHOULD send a `warning` and close the connection, or send an `error` and fail the channel.

My interpretation is that our_max_value_in_flight_msat limits how much can we receive, and their_max_value_in_flight_max limits how much can we send.

cltv_delta[LOCAL], enabled, buf, channel, cbarg);

/* If we didn't have a remote update, it's not usable yet */
Expand All @@ -166,7 +173,7 @@ gossmods_from_listpeerchannels_(const tal_t *ctx,

cb(mods, self, &dst, &scidd, capacity_msat,
htlc_min[REMOTE], htlc_max[REMOTE],
receivable, fee_base[REMOTE], fee_proportional[REMOTE],
receivable, max_total_in_htlc, fee_base[REMOTE], fee_proportional[REMOTE],
cltv_delta[REMOTE], enabled, buf, channel, cbarg);
}

Expand Down
3 changes: 3 additions & 0 deletions common/gossmods_listpeerchannels.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct gossmap_localmods *gossmods_from_listpeerchannels_(const tal_t *ctx,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat spendable,
struct amount_msat total_htlcmax,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand All @@ -55,6 +56,7 @@ struct gossmap_localmods *gossmods_from_listpeerchannels_(const tal_t *ctx,
struct amount_msat, \
struct amount_msat, \
struct amount_msat, \
struct amount_msat, \
u32, \
u16, \
bool, \
Expand All @@ -71,6 +73,7 @@ void gossmod_add_localchan(struct gossmap_localmods *mods,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat spendable,
struct amount_msat total_htlcmax,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand Down
20 changes: 19 additions & 1 deletion contrib/msggen/msggen/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23999,7 +23999,25 @@
"max_total_htlc_in_msat": {
"type": "msat",
"description": [
"Max amount accept in a single payment."
"Max amount accept in a single payment. This field is deprecated, use instead our_max_htlc_value_in_flight_msat"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this commit should be squashed into the commit where these fields are originally introduced/added to the RPC (the first one in the series, in this case)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's complicated. These are autogenerated, if you ask me I wouldn't have these files into the source code cause no human wrote them. I add them as a separate commit to make it easier to rebase on the master branch in case someone else has also modified them.

My usual rebase when changing schemas is

  • soft reset the last commit (with the autogenerated files, very annoying),
  • rebase
  • then generate those files again and commit

],
"deprecated": [
"v24.11",
"v25.05"
]
},
"their_max_htlc_value_in_flight_msat": {
"type": "msat",
"added": "v24.11",
"description": [
"Cap on total value of outstanding HTLCs offered to the remote node. This limits the total amount in flight we can send through this channel."
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to make this more readable: cap on total value of outstanding HTLCs offered to the remote node (from our balance).

Copy link
Contributor

Choose a reason for hiding this comment

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

though now that i've written these out, i feel like the their part of the max htlc value in flight name is confusing, since it's our balance that's in flight? something about this seems wrong. Either the modifier "this limits the total amount in flight we can send" is wrong or their max in flight is incorrect?

we can't add more HTLCs to be sent to the peer, because they don't want to let X% of their balance be committed as HTLCs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is weird. It is like our_max_value... is telling the peer "please don't try to send me more than this
or I will not accept it". The prefix their_ and our_ refers to who set the configuration value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The description is a sentence taken from BOLT02.

]
},
"our_max_htlc_value_in_flight_msat": {
"type": "msat",
"added": "v24.11",
"description": [
"Cap on total value of outstanding HTLCs we accept from the remote node. This limits the total amount in flight we can receive through this channel."
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to make this more readable: cap on total value of outstanding HTLCs offered to the local node (from peer's balance).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could this also work?
our_max_htlc_value_in_flight_msat: Cap on total value of outstanding HTLCs we accept from the remote node.

]
},
"their_reserve_msat": {
Expand Down
1,524 changes: 762 additions & 762 deletions contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions contrib/pyln-testing/pyln/testing/grpc2py.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ def listpeerchannels_channels2py(m):
"minimum_htlc_out_msat": amount2msat(m.minimum_htlc_out_msat), # PrimitiveField in generate_composite
"next_fee_step": m.next_fee_step, # PrimitiveField in generate_composite
"next_feerate": m.next_feerate, # PrimitiveField in generate_composite
"our_max_htlc_value_in_flight_msat": amount2msat(m.our_max_htlc_value_in_flight_msat), # PrimitiveField in generate_composite
"our_reserve_msat": amount2msat(m.our_reserve_msat), # PrimitiveField in generate_composite
"our_to_self_delay": m.our_to_self_delay, # PrimitiveField in generate_composite
"out_fulfilled_msat": amount2msat(m.out_fulfilled_msat), # PrimitiveField in generate_composite
Expand All @@ -1033,6 +1034,7 @@ def listpeerchannels_channels2py(m):
"scratch_txid": hexlify(m.scratch_txid), # PrimitiveField in generate_composite
"short_channel_id": m.short_channel_id, # PrimitiveField in generate_composite
"spendable_msat": amount2msat(m.spendable_msat), # PrimitiveField in generate_composite
"their_max_htlc_value_in_flight_msat": amount2msat(m.their_max_htlc_value_in_flight_msat), # PrimitiveField in generate_composite
"their_reserve_msat": amount2msat(m.their_reserve_msat), # PrimitiveField in generate_composite
"their_to_self_delay": m.their_to_self_delay, # PrimitiveField in generate_composite
"to_us_msat": amount2msat(m.to_us_msat), # PrimitiveField in generate_composite
Expand Down
20 changes: 19 additions & 1 deletion doc/schemas/lightning-listpeerchannels.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,25 @@
"max_total_htlc_in_msat": {
"type": "msat",
"description": [
"Max amount accept in a single payment."
"Max amount accept in a single payment. This field is deprecated, use instead our_max_htlc_value_in_flight_msat"
],
"deprecated": [
"v24.11",
"v25.05"
]
},
"their_max_htlc_value_in_flight_msat": {
"type": "msat",
"added": "v24.11",
"description": [
"Cap on total value of outstanding HTLCs offered to the remote node. This limits the total amount in flight we can send through this channel."
]
},
"our_max_htlc_value_in_flight_msat": {
"type": "msat",
"added": "v24.11",
"description": [
"Cap on total value of outstanding HTLCs we accept from the remote node. This limits the total amount in flight we can receive through this channel."
]
},
"their_reserve_msat": {
Expand Down
6 changes: 6 additions & 0 deletions lightningd/peer_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,12 @@ static void NON_NULL_ARGS(1, 2, 4, 5) json_add_channel(struct command *cmd,
channel->our_config.dust_limit);
json_add_amount_msat(response, "max_total_htlc_in_msat",
channel->our_config.max_htlc_value_in_flight);
json_add_amount_msat(
response, "their_max_htlc_value_in_flight_msat",
channel->channel_info.their_config.max_htlc_value_in_flight);
json_add_amount_msat(
response, "our_max_htlc_value_in_flight_msat",
channel->our_config.max_htlc_value_in_flight);

/* The `channel_reserve_satoshis` is imposed on
* the *other* side (see `channel_reserve_msat`
Expand Down
5 changes: 4 additions & 1 deletion plugins/askrene/askrene.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ static void add_localchan(struct gossmap_localmods *mods,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat spendable,
struct amount_msat max_total_htlc,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand Down Expand Up @@ -722,8 +723,10 @@ static void add_localchan(struct gossmap_localmods *mods,
additional_cost_htable_add(info->additional_costs, phc);
}

/* can't send more than expendable and no more than max_total_htlc */
struct amount_msat max_msat = amount_msat_min(spendable, max_total_htlc);
/* Known capacity on local channels (ts = max) */
layer_add_constraint(info->local_layer, scidd, UINT64_MAX, &spendable, &spendable);
layer_add_constraint(info->local_layer, scidd, UINT64_MAX, &max_msat, &max_msat);
}

static struct command_result *
Expand Down
6 changes: 4 additions & 2 deletions plugins/renepay/mods.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ static void gossmod_cb(struct gossmap_localmods *mods,
struct amount_msat htlcmin,
struct amount_msat htlcmax,
struct amount_msat spendable,
struct amount_msat max_total_htlc,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand All @@ -403,9 +404,10 @@ static void gossmod_cb(struct gossmap_localmods *mods,
struct amount_msat min, max;

if (scidd->dir == node_id_idx(self, peer)) {
/* local channels can send up to what's spendable */
/* local channels can send up to what's spendable but there is a
* limit also the total amount in-flight */
min = AMOUNT_MSAT(0);
max = spendable;
max = amount_msat_min(spendable, max_total_htlc);
} else {
/* remote channels can send up no more than spendable */
min = htlcmin;
Expand Down
2 changes: 2 additions & 0 deletions plugins/test/run-route-calc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ void gossmod_add_localchan(struct gossmap_localmods *mods UNNEEDED,
struct amount_msat htlcmin UNNEEDED,
struct amount_msat htlcmax UNNEEDED,
struct amount_msat spendable UNNEEDED,
struct amount_msat total_htlcmax UNNEEDED,
struct amount_msat fee_base UNNEEDED,
u32 fee_proportional UNNEEDED,
u16 cltv_delta UNNEEDED,
Expand All @@ -75,6 +76,7 @@ struct gossmap_localmods *gossmods_from_listpeerchannels_(const tal_t *ctx UNNEE
struct amount_msat htlcmin UNNEEDED,
struct amount_msat htlcmax UNNEEDED,
struct amount_msat spendable UNNEEDED,
struct amount_msat total_htlcmax UNNEEDED,
struct amount_msat fee_base UNNEEDED,
u32 fee_proportional UNNEEDED,
u16 cltv_delta UNNEEDED,
Expand Down
2 changes: 2 additions & 0 deletions plugins/test/run-route-overlong.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ void gossmod_add_localchan(struct gossmap_localmods *mods UNNEEDED,
struct amount_msat htlcmin UNNEEDED,
struct amount_msat htlcmax UNNEEDED,
struct amount_msat spendable UNNEEDED,
struct amount_msat total_htlcmax UNNEEDED,
struct amount_msat fee_base UNNEEDED,
u32 fee_proportional UNNEEDED,
u16 cltv_delta UNNEEDED,
Expand All @@ -72,6 +73,7 @@ struct gossmap_localmods *gossmods_from_listpeerchannels_(const tal_t *ctx UNNEE
struct amount_msat htlcmin UNNEEDED,
struct amount_msat htlcmax UNNEEDED,
struct amount_msat spendable UNNEEDED,
struct amount_msat total_htlcmax UNNEEDED,
struct amount_msat fee_base UNNEEDED,
u32 fee_proportional UNNEEDED,
u16 cltv_delta UNNEEDED,
Expand Down
3 changes: 2 additions & 1 deletion plugins/topology.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ static void gossmod_add_unknown_localchan(struct gossmap_localmods *mods,
struct amount_msat min,
struct amount_msat max,
struct amount_msat spendable,
struct amount_msat max_total_htlc,
struct amount_msat fee_base,
u32 fee_proportional,
u16 cltv_delta,
Expand All @@ -367,7 +368,7 @@ static void gossmod_add_unknown_localchan(struct gossmap_localmods *mods,
return;

gossmod_add_localchan(mods, self, peer, scidd, capacity_msat,
min, max, spendable,
min, max, spendable, max_total_htlc,
fee_base, fee_proportional, cltv_delta, enabled,
buf, chantok, gossmap);
}
Expand Down
Loading