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

libplugin-pay: use map for channel hints (on top of #7494) #7648

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5349f00
pay: Add a function to update `channel_hint`s based on their age
cdecker Jul 25, 2024
6975f56
route: Add the total capacity to route_hops
cdecker Jul 25, 2024
6f65862
pay: Use the total_mast amount as the upper limit for channel_hints
cdecker Jul 25, 2024
5ee2b96
pay: Make the `channel_hint`s global
cdecker Jul 29, 2024
17aaf90
make: Weaken over aggressive check-amount-access test
cdecker Jul 29, 2024
b265c13
plugin: Split out the `struct channel_hint` handling
cdecker Aug 2, 2024
25d55a3
pay: Rename overall_capacity to just capacity
cdecker Aug 11, 2024
9b1610f
route: Simplify direction
cdecker Aug 16, 2024
9a2827e
pay: Subscribe to the `channel_hint_update` notifications
cdecker Aug 9, 2024
124e215
route: Use safe `amount_sat_to_msat` conversion
cdecker Aug 16, 2024
f4911a9
route: Change the type of the funding capacity to `amount_sat`
cdecker Aug 16, 2024
dbe9685
pytest: Test that we remember learnt channel hints across payments
cdecker Jul 31, 2024
248113b
pay: Use the global `channel_hint_set` and remember across payments
cdecker Aug 22, 2024
b102c04
pay: Add a hysteresis for channel_hint relaxation
cdecker Aug 23, 2024
2902283
pay: Add `channel_hint_set_count` primitive
cdecker Aug 23, 2024
8db5e8d
pay: Log when and why we exclude a channel from the route
cdecker Aug 23, 2024
95b459f
pay: Inject `channel_hint`s we receive via plugin notifications
cdecker Aug 23, 2024
3b173e2
pay: Remove use of temporary local `channel_hint`
cdecker Aug 28, 2024
485725f
pytest: Remove the `test_mutual_reconnect_race` test
cdecker Aug 29, 2024
9b876f5
test: Fix up the `test_pay_routeboost` test
cdecker Sep 2, 2024
772a505
pytest: Fix up the `test_sendpay_grouping` test
cdecker Sep 4, 2024
1e833ba
pay: Simplify the `channel_hint` update logic
cdecker Sep 5, 2024
aaff2f3
libplugin-pay: use map for channel hints
JssDWt Sep 6, 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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ check-discouraged-functions:
# since it risks overflow.
check-amount-access:
@! (git grep -nE "(->|\.)(milli)?satoshis" -- "*.c" "*.h" ":(exclude)common/amount.*" ":(exclude)*/test/*" | grep -v '/* Raw:')
@! git grep -nE "\\(struct amount_(m)?sat\\)" -- "*.c" "*.h" ":(exclude)common/amount.*" ":(exclude)*/test/*"
@! git grep -nE "\\(struct amount_(m)?sat\\)" -- "*.c" "*.h" ":(exclude)common/amount.*" ":(exclude)*/test/*" | grep -vE "sizeof.struct amount_(m)?sat."

# For those without working cppcheck
check-source-no-cppcheck: check-makefile check-source-bolt check-whitespace check-spelling check-python check-includes check-shellcheck check-setup_locale check-tmpctx check-discouraged-functions check-amount-access
Expand Down
8 changes: 8 additions & 0 deletions common/json_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,14 @@ void json_add_amount_msat(struct json_stream *result,
json_add_u64(result, msatfieldname, msat.millisatoshis); /* Raw: low-level helper */
}

void json_add_amount_sat(struct json_stream *result,
const char *satfieldname,
struct amount_sat sat)
{
assert(strends(satfieldname, "_sat") || streq(satfieldname, "sat"));
json_add_u64(result, satfieldname, sat.satoshis); /* Raw: low-level helper */
}

void json_add_amount_sat_msat(struct json_stream *result,
const char *msatfieldname,
struct amount_sat sat)
Expand Down
7 changes: 6 additions & 1 deletion common/json_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
#define LIGHTNING_COMMON_JSON_STREAM_H
#include "config.h"

#include <bitcoin/short_channel_id.h>
#define JSMN_STRICT 1
# include <external/jsmn/jsmn.h>

#include <bitcoin/short_channel_id.h>
#include <ccan/short_types/short_types.h>
#include <ccan/tal/tal.h>
#include <ccan/time/time.h>
Expand Down Expand Up @@ -334,6 +334,11 @@ void json_add_amount_msat(struct json_stream *result,
struct amount_msat msat)
NO_NULL_ARGS;

void json_add_amount_sat(struct json_stream *result,
const char *satfieldname,
struct amount_sat sat)
NO_NULL_ARGS;

/* Adds an 'msat' field */
void json_add_amount_sat_msat(struct json_stream *result,
const char *msatfieldname,
Expand Down
11 changes: 5 additions & 6 deletions common/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ static bool dijkstra_to_hops(struct route_hop **hops,
const struct gossmap_node *next;
size_t num_hops = tal_count(*hops);
const struct half_chan *h;
struct amount_sat total;

if (dist == 0)
return true;
Expand All @@ -92,12 +93,8 @@ static bool dijkstra_to_hops(struct route_hop **hops,

/* OK, populate other fields. */
c = dijkstra_best_chan(dij, curidx);
if (c->half[0].nodeidx == curidx) {
(*hops)[num_hops].direction = 0;
} else {
assert(c->half[1].nodeidx == curidx);
(*hops)[num_hops].direction = 1;
}

(*hops)[num_hops].direction = c->half[0].nodeidx == curidx ? 0 : 1;
(*hops)[num_hops].scid = gossmap_chan_scid(gossmap, c);

/* Find other end of channel. */
Expand All @@ -107,6 +104,8 @@ static bool dijkstra_to_hops(struct route_hop **hops,
if (!dijkstra_to_hops(hops, gossmap, dij, next, amount, cltv))
return false;

gossmap_chan_get_capacity(gossmap, c, &total);
(*hops)[num_hops].capacity = total;
(*hops)[num_hops].amount = *amount;
(*hops)[num_hops].delay = *cltv;

Expand Down
2 changes: 2 additions & 0 deletions common/route.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ struct gossmap_node;
* @direction: 0 (dest node_id < src node_id), 1 (dest node_id > src).
* @node_id: the node_id of the destination of this hop.
* @amount: amount to send through this hop.
* @capacity: The amount the channel was funded with.
* @delay: total cltv delay at this hop.
*/
struct route_hop {
struct short_channel_id scid;
int direction;
struct node_id node_id;
struct amount_msat amount;
struct amount_sat capacity;
u32 delay;
};

Expand Down
1 change: 1 addition & 0 deletions common/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ common/test/run-route common/test/run-route-specific common/test/run-route-inflo
common/node_id.o \
common/pseudorand.o \
common/route.o \
gossipd/gossip_store_wiregen.o \
wire/fromwire.o \
wire/peer_wiregen.o \
wire/towire.o
Expand Down
8 changes: 8 additions & 0 deletions common/test/run-route-specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <common/setup.h>
#include <common/utils.h>
#include <bitcoin/chainparams.h>
#include <gossipd/gossip_store_wiregen.h>
#include <stdio.h>
#include <wire/peer_wiregen.h>
#include <unistd.h>
Expand Down Expand Up @@ -139,6 +140,13 @@ static void add_connection(int store_fd,
ids[0], ids[1],
&dummy_key, &dummy_key);
write_to_store(store_fd, msg);
tal_free(msg);

/* Also needs a hint as to the funding size. */
struct amount_sat capacity = AMOUNT_SAT(100000000);
msg = towire_gossip_store_channel_amount(tmpctx, capacity);
write_to_store(store_fd, msg);
tal_free(msg);

update_connection(store_fd, from, to, shortid, min, max,
base_fee, proportional_fee,
Expand Down
10 changes: 8 additions & 2 deletions common/test/run-route.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <common/setup.h>
#include <common/utils.h>
#include <bitcoin/chainparams.h>
#include <gossipd/gossip_store_wiregen.h>
#include <stdio.h>
#include <wire/peer_wiregen.h>
#include <unistd.h>
Expand Down Expand Up @@ -129,8 +130,13 @@ static void add_connection(int store_fd,
&dummy_key, &dummy_key);
write_to_store(store_fd, msg);

update_connection(store_fd, from, to, base_fee, proportional_fee,
delay, false);
/* Also needs a hint as to the funding size. */
struct amount_sat capacity = AMOUNT_SAT(100000000);
msg = towire_gossip_store_channel_amount(tmpctx, capacity);
write_to_store(store_fd, msg);
tal_free(msg);
update_connection(store_fd, from, to, base_fee, proportional_fee, delay,
false);
}

static bool channel_is_between(const struct gossmap *gossmap,
Expand Down
10 changes: 8 additions & 2 deletions plugins/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ PLUGIN_LIB_SRC := plugins/libplugin.c
PLUGIN_LIB_HEADER := plugins/libplugin.h
PLUGIN_LIB_OBJS := $(PLUGIN_LIB_SRC:.c=.o)

PLUGIN_PAY_LIB_SRC := plugins/libplugin-pay.c
PLUGIN_PAY_LIB_HEADER := plugins/libplugin-pay.h
PLUGIN_PAY_LIB_SRC := \
plugins/channel_hint.c \
plugins/libplugin-pay.c

PLUGIN_PAY_LIB_HEADER := \
plugins/channel_hint.h \
plugins/libplugin-pay.h

PLUGIN_PAY_LIB_OBJS := $(PLUGIN_PAY_LIB_SRC:.c=.o)

PLUGIN_OFFERS_SRC := plugins/offers.c plugins/offers_offer.c plugins/offers_invreq_hook.c plugins/offers_inv_hook.c plugins/establish_onion_path.c plugins/fetchinvoice.c
Expand Down
230 changes: 230 additions & 0 deletions plugins/channel_hint.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
#include "config.h"
#include <common/memleak.h>
#include <plugins/channel_hint.h>

size_t channel_hint_hash(const struct short_channel_id_dir *out)
{
struct siphash24_ctx ctx;
siphash24_init(&ctx, siphash_seed());
siphash24_update(&ctx, &out->scid.u64, sizeof(u64));
siphash24_update(&ctx, &out->dir, sizeof(int));
return siphash24_done(&ctx);
}

const struct short_channel_id_dir *channel_hint_keyof(const struct channel_hint *out)
{
return &out->scid;
}

bool channel_hint_eq(const struct channel_hint *a,
const struct short_channel_id_dir *b)
{
return short_channel_id_eq(a->scid.scid, b->scid) &&
a->scid.dir == b->dir;
}

static void memleak_help_channel_hint_map(struct htable *memtable,
struct channel_hint_map *channel_hints)
{
memleak_scan_htable(memtable, &channel_hints->raw);
}

void channel_hint_to_json(const char *name, const struct channel_hint *hint,
struct json_stream *dest)
{
json_object_start(dest, name);
json_add_u32(dest, "timestamp", hint->timestamp);
json_add_short_channel_id_dir(dest, "scid", hint->scid);
json_add_amount_msat(dest, "estimated_capacity_msat",
hint->estimated_capacity);
json_add_amount_sat(dest, "capacity_sat", hint->capacity);
json_add_bool(dest, "enabled", hint->enabled);
json_object_end(dest);
}

/* How long until even a channel whose estimate is down at 0msat will
* be considered fully refilled. The refill rate is the inverse of
* this times the channel size. The refilling is a linear
* approximation, with a small hysteresis applied in order to prevent
* a single payment relaxing its own constraints thus causing it to
* prematurely retry an already attempted channel.
*/
#define PAY_REFILL_TIME 7200

/* Add an artificial delay before accepting updates. This ensures we
* don't actually end up relaxing a tight constraint inbetween the
* attempt that added it and the next retry. If we were to relax right
* away, then we could end up retrying the exact same path we just
* failed at. If the `time_between_attempts * refill > 1msat`, we'd
* end up not actually constraining at all, because we set the
* estimate to `attempt - 1msat`. This also results in the updates
* being limited to once every minute, and causes a stairway
* pattern. The hysteresis has to be >60s otherwise a single payment
* can already end up retrying a previously excluded channel.
*/
#define PAY_REFILL_HYSTERESIS 60
/**
* Update the `channel_hint` in place, return whether it should be kept.
*
* This computes the refill-rate based on the overall capacity, and
* the time elapsed since the last update and relaxes the upper bound
* on the capacity, and resets the enabled flag if appropriate. If the
* hint is no longer useful, i.e., it does not provide any additional
* information on top of the structural information we've learned from
* the gossip, then we return `false` to signal that the
* `channel_hint` may be removed.
*/
bool channel_hint_update(const struct timeabs now, struct channel_hint *hint)
{
/* Precision is not required here, so integer division is good
* enough. But keep the order such that we do not round down
* too much. We do so by first multiplying, before
* dividing. The formula is `current = last + delta_t *
* overall / refill_rate`.
*/
struct amount_msat refill;
struct amount_msat capacity;
if (!amount_sat_to_msat(&capacity, hint->capacity))
abort();

if (now.ts.tv_sec < hint->timestamp + PAY_REFILL_HYSTERESIS)
return true;

u64 seconds = now.ts.tv_sec - hint->timestamp;
if (!amount_msat_mul(&refill, capacity, seconds))
abort();

refill = amount_msat_div(refill, PAY_REFILL_TIME);
if (!amount_msat_add(&hint->estimated_capacity,
hint->estimated_capacity, refill))
abort();

/* Clamp the value to the `overall_capacity` */
if (amount_msat_greater(hint->estimated_capacity, capacity))
hint->estimated_capacity = capacity;

/* TODO This is rather coarse. We could map the disabled flag
to having 0msat capacity, and then relax from there. But it'd
likely be too slow of a relaxation.*/
if (seconds > 60)
hint->enabled = true;

/* Since we update in-place we should make sure that we can
* just call update again and the result is stable, if no time
* has passed. */
hint->timestamp = now.ts.tv_sec;

/* We report this hint as useless, if the hint does not
* restrict the channel, i.e., if it is enabled and the
* estimate is the same as the overall capacity. */
return !hint->enabled ||
amount_msat_greater(capacity, hint->estimated_capacity);
}

struct channel_hint *channel_hint_set_find(const struct channel_hint_set *self,
const struct short_channel_id_dir *scidd)
{
return channel_hint_map_get(self->hints, scidd);
}

/* See header */
struct channel_hint *
channel_hint_set_add(struct channel_hint_set *self, u32 timestamp,
const struct short_channel_id_dir *scidd, bool enabled,
const struct amount_msat *estimated_capacity,
const struct amount_sat capacity, u16 *htlc_budget)
{
struct channel_hint *copy, *old, *newhint;

/* If the channel is marked as enabled it must have an estimate. */
assert(!enabled || estimated_capacity != NULL);

/* If there was no hint, add the new one, if there was one,
* pick the one with the newer timestamp. */
old = channel_hint_set_find(self, scidd);
copy = tal_dup(tmpctx, struct channel_hint, old);
if (old == NULL) {
newhint = tal(self, struct channel_hint);
newhint->enabled = enabled;
newhint->scid = *scidd;
newhint->capacity = capacity;
if (estimated_capacity != NULL)
newhint->estimated_capacity = *estimated_capacity;
newhint->local = NULL;
newhint->timestamp = timestamp;
channel_hint_map_add(self->hints, newhint);
return newhint;
} else if (old->timestamp <= timestamp) {
/* `local` is kept, since we do not pass in those
* annotations here. */
old->enabled = enabled;
old->timestamp = timestamp;
if (estimated_capacity != NULL)
old->estimated_capacity = *estimated_capacity;

/* We always pick the larger of the capacities we are
* being told. This is because in some cases, such as
* routehints, we're not actually being told the total
* capacity, just lower values. */
if (amount_sat_greater(capacity, old->capacity))
old->capacity = capacity;

return copy;
} else {
return NULL;
}
}

/**
* Load a channel_hint from its JSON representation.
*
* @return The initialized `channel_hint` or `NULL` if we encountered a parsing
* error.
*/
struct channel_hint *channel_hint_from_json(const tal_t *ctx,
const char *buffer,
const jsmntok_t *toks)
{
const char *ret;
const jsmntok_t *payload = json_get_member(buffer, toks, "payload"),
*jhint =
json_get_member(buffer, payload, "channel_hint");
struct channel_hint *hint = tal(ctx, struct channel_hint);

ret = json_scan(ctx, buffer, jhint,
"{timestamp:%,scid:%,estimated_capacity_msat:%,capacity_sat:%,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_sat, &hint->capacity),
JSON_SCAN(json_to_bool, &hint->enabled));

if (ret != NULL)
hint = tal_free(hint);
return hint;
}

struct channel_hint_set *channel_hint_set_new(const tal_t *ctx)
{
struct channel_hint_set *set = tal(ctx, struct channel_hint_set);
set->hints = tal(set, struct channel_hint_map);
channel_hint_map_init(set->hints);
memleak_add_helper(set->hints, memleak_help_channel_hint_map);
return set;
}

void channel_hint_set_update(struct channel_hint_set *set,
const struct timeabs now)
{
struct channel_hint *hint;
struct channel_hint_map_iter iter;
for (hint = channel_hint_map_first(set->hints, &iter);
hint;
hint = channel_hint_map_next(set->hints, &iter))
channel_hint_update(now, hint);
}

size_t channel_hint_set_count(const struct channel_hint_set *set)
{
return channel_hint_map_count(set->hints);
}
Loading
Loading