pay: Share channel_hints across payments and plugins #7487
Conversation
cdecker
force-pushed
the
202407-pay-channel-hint-notifications
branch
3 times, most recently
from
1fa5a3f
to
1601835
Compare
cdecker
force-pushed
the
202407-pay-channel-hint-notifications
branch
3 times, most recently
from
a22375e
to
33a5a0c
Compare
cdecker
force-pushed
the
202407-pay-channel-hint-notifications
branch
from
33a5a0c
to
f6c1cc3
Compare
| /* | ||
| static struct channel_hint *channel_hint_from_json(const tal_t *ctx, | ||
| const char *buffer, | ||
| const jsmntok_t *toks) | ||
| { | ||
| const char *ret; | ||
| struct channel_hint *hint = tal(ctx, struct channel_hint); | ||
| ret = json_scan(ctx, buffer, toks, | ||
| "{timestamp:%,scid:%,capacity_msat:%,enabled:%}", | ||
| JSON_SCAN(json_to_u32, &hint->timestamp), | ||
| JSON_SCAN(json_to_short_channel_id_dir, &hint->scid), | ||
| JSON_SCAN(json_to_msat, &hint->estimated_capacity), | ||
| JSON_SCAN(json_to_bool, &hint->enabled)); | ||
|
|
||
| if (ret != NULL) | ||
| hint = tal_free(hint); | ||
| return hint; | ||
| } |
There was a problem hiding this comment.
I could have split this into a later commit, but it seemed like a good idea to have both the writing and reading part of the notification in one commit.
The `pay` plugin, as well as other plugins making use of the tree-pay executor, will now emit their observations as they see them. The notifications are sent on the `channel_hint_updated` topic, and any subscriber will get them. We also added a `timestamp` to the `struct channel_hint`, as these observations now outlive the `pay` call, and have to be attenuated / relaxed as they age, until we can eliminate them completely (when the restriction is equal to the structural information gathered from the gossip). Changelog-Added: pay: Payments now emit `channel_hint_updated` notification to share inferred balances and observations across multiple payments.
We were missing the `short_channel_id_dir` helpers.
cdecker
force-pushed
the
202407-pay-channel-hint-notifications
branch
from
f6c1cc3
to
a8504b7
Compare
There was a problem hiding this comment.
Cosmetics only, given the proximity to release I'm tempted to apply this now and tweak later...
Ack a8504b7
| @@ -485,6 +484,16 @@ void json_add_short_channel_id(struct json_stream *response, | |||
| short_channel_id_outnum(scid)); | |||
| } | |||
|
|
|||
| void json_add_short_channel_id_dir(struct json_stream *response, | |||
| const char *fieldname, | |||
| struct short_channel_id_dir scidd) | |||
There was a problem hiding this comment.
scidd is a complex type, so unlike scids it should be passed as "const struct short_channel_id_dir *". This also means the definition doesn't need to be in the header...
| @@ -386,6 +386,16 @@ void payment_start(struct payment *p) | |||
| payment_start_at_blockheight(p, INVALID_BLOCKHEIGHT); | |||
| } | |||
|
|
|||
| static void channel_hint_to_json(const char *name, const struct channel_hint *hint, struct json_stream *dest) | |||
There was a problem hiding this comment.
This would normally be called "json_add_channel_hint(struct json_stream *, const char *, const struct channel_hint *)". It's internal, but consistency is nice.
Not as a solution, but as a step towards it yes. The channel_hints are being emitted for the benefit of other plugins at the moment, but I have one pending that'll also inject from other plugins into I'd rather mark #7494 as the solution, as it is what was being proposed in the issue. |
|
This is not a solution to fixing payment in CLN. I just built 24.08rc3 from source and issues persist. Payments still are no longer supported in CLN. The last funcitonal version is from before version 24. |
While performing a payment we attempt to get several HTLCs through
from the sender to the destination. Some of these attempts may fail,
due to temporary unavailability, insufficient balance, or many other
reasons. We use
channel_hints to track these failures and compilethe observations into a more detailed view of the network.
So far we were just forgetting all of the inferred information once
the payment is done, however this information is useful for later
payments too, as it allows us to skip attempts that are unlikely to
succeed, and essentially continue the exploration where the previous
payments have left off.
The main issue we face is how do we relax the hints over time, such
that we maximize their usefulness by allowing us to skip channels that
are unlikely to succeed, while at the same time minimizing the false
negative rate where we skip a channel that would have worked, based on
the prior observations. We implement it here as a linear decay, in
line with the leaky bucket rate limiting algorithm.
We model the sharing of the
channel_hints as notifications since wemight want to share observations across multiple plugins, and maybe
even across multiple nodes, if they are operated by the same
operator. DO NOT accept
channel_hintnotifications from untrustedsources, as they can be used directly to steer the route selection.
Changelog-Added: pay: Payments now emit
channel_hint_updatednotification to share inferred balances and observations across multiple payments.This is part of the following PR set:
graph TD; CLN7487 --> CLN7494 ; click CLN7487 "https://github.com/ElementsProject/lightning/pull/7487" click CLN7494 "https://github.com/ElementsProject/lightning/pull/7494" click CLN7543 "https://github.com/ElementsProject/lightning/pull/7543" style CLN7487 fill:#AAFFAA