Pay: use gossmap #4093
Pay: use gossmap #4093
Conversation
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
3fd2e6b
to
0e1a891
Compare
|
Trivial rebase... |
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
0e1a891
to
c2285b3
Compare
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
c2285b3
to
848ccbe
Compare
|
Ya trivial rebase. |
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
848ccbe
to
a0ff1ef
Compare
cdecker
force-pushed
the
guilt/pay-map-gossip-store
branch
from
a0ff1ef
to
0bb0e54
Compare
There was a problem hiding this comment.
Excellent change, I particularly like that since it is now synchronous we no longer end up trampling the channel_hints.
ACK 0bb0e54
Still seems a bit suboptimal to compute a route on a static map, and only afterwards considering things like route length, CLTV and fee constraints. We could add a view-layer on top of the gossmap, created for each dijkstra-call that keeps track of these parameters, and sets the distance to unreachable if any of these constraints is violated. If we add the struct payment to the scoring function we could even do this in the scoring function itself, not requiring us to change gossmap at all. But all that's for another time :-)
common/dijkstra.c
Outdated
| @@ -22,6 +22,9 @@ struct dijkstra { | |||
| /* NULL means it's been visited already. */ | |||
| const struct gossmap_node **heapptr; | |||
|
|
|||
| /* How we decide "best" */ | |||
There was a problem hiding this comment.
| /* How we decide "best" */ | |
| /* How we decide "best", lower is better */ |
Since I intuitively associate high scores with being better :-)
There was a problem hiding this comment.
Good idea, I'll do a followon cleanup commit.
| struct amount_msat new_risk, | ||
| void *unused) | ||
| /* Prioritize costs over distance */ | ||
| u64 route_score_cheaper(u32 distance, |
There was a problem hiding this comment.
Can we just use implementation of route_score_shorter and swap high and low bytes? Avoids duplicating the code.
common/gossmap.c
Outdated
| @@ -129,6 +130,18 @@ static void map_nodeid(const struct gossmap *map, size_t offset, | |||
| map_copy(map, offset, id, sizeof(*id)); | |||
| } | |||
|
|
|||
| static bool map_feature_set(const struct gossmap *map, int bit, | |||
There was a problem hiding this comment.
Could this be either is_set or get? Might be my Java past catching up with me, but set sounds like we're setting it :-)
common/gossmap.c
Outdated
| if (map_feature_set(map, OPTIONAL_FEATURE(fbit), | ||
| c->cann_off + feature_len_off + 2, feature_len)) | ||
| return OPTIONAL_FEATURE(fbit); | ||
| if (map_feature_set(map, COMPULSORY_FEATURE(fbit), |
There was a problem hiding this comment.
We could let map_feature_set return both optional and compulsory flags, in order to avoid calling it twice.
There was a problem hiding this comment.
Yes, it's fairly cheap, but since this is the only way we call it...
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
0bb0e54
to
764819a
Compare
|
Tests using the
Otherwise looking good 👍 |
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
2 times, most recently
from
296fab9
to
1f48e7e
Compare
|
Ah, tests without COMPAT enabled. OK, fixed. |
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
1f48e7e
to
f6fc1bb
Compare
We were always ordering heap by distance, not score (which are different if we are routing by cheapest, not shortest!). This simplifies our callbacks, too. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
So we don't have to handle them at load time, either. Signed-off-by: Rusty Russell <[email protected]>
We would return junk before. Signed-off-by: Rusty Russell <[email protected]>
Faster than pulling the announce msg and parsing. We need this to test if the node supports TLV. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Instead of a boutique message, use a "real" channel_announcement for private channels (with fake sigs and pubkeys). This makes it far easier for gossmap to handle local channels. Backwards compatible update, since we update old stores. We also fix devtools/dump-gossipstore to know about the tombstone markers. Since we increment our channel_announce count for local channels now, the stats in the tests changed too. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Start with attaching the payment to cmd (in case of failure), then steal onto the plugin itself. Signed-off-by: Rusty Russell <[email protected]>
So we can use this for routing determinations. Signed-off-by: Rusty Russell <[email protected]>
This is a fairly direct translation. Even so, it should be faster in most cases, and and we can do more sophisticated things if we want. This also handles disabled channels better. Signed-off-by: Rusty Russell <[email protected]> Changelog-Changed: plugins: `pay` will now try disabled channels as a last resort.
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/pay-map-gossip-store
branch
from
f6fc1bb
to
64673c4
Compare
This is a simple translation, but it involves a routing fix and some enhancements.
Building on this to get more route information is a job for @cdecker.
(Moving getroute et-al out to a plugin is still in my todo list, as is random routing!)