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

Askrene fixes and enhancements #7640

Merged
merged 22 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
54e308d
lightningd: remove incorrect old-payment-htlc-delete logic.
rustyrussell Sep 18, 2024
c2a516d
common/amount: add routine to calculate fees backwards.
rustyrussell Sep 18, 2024
69a1487
patch general-node-id-assign.patch
rustyrussell Sep 18, 2024
ade18df
common/amount: add amount_msat_accumulate()
rustyrussell Sep 18, 2024
5d4b1ae
common/amount: rename amount_sat_zero/amount_msat_zerp -> amount_sat_…
rustyrussell Sep 18, 2024
267d122
pytest: ensure there are no duplicate paths in test_live_spendable, a…
rustyrussell Sep 18, 2024
0179a71
pytest: duplicate test_live_spendable as a canned gossmap test.
rustyrussell Sep 18, 2024
19a68d3
pyln-testing: add gossip_store_file arg to get_node()
rustyrussell Sep 18, 2024
4f35bba
pytest: separate out routine which checks only some fields of getroutes.
rustyrussell Sep 18, 2024
a305a61
askrene: apply "auto.sourcefree" to channels created in layers, too.
rustyrussell Sep 18, 2024
06ceaf2
common/bolt12: do more required checks in invoice_decode.
rustyrussell Sep 18, 2024
b6ef1e8
patch libplugin-batch.patch
rustyrussell Sep 18, 2024
4ba121d
askrene: remove unused flow routines.
rustyrussell Sep 18, 2024
e69fbcd
askrene: remove `struct flow` `probability` member.
rustyrussell Sep 18, 2024
628d7bc
askrene: rename `struct flow` `amount` to `delivers`.
rustyrussell Sep 18, 2024
d02fd55
askrene: add a "refining" step to add fees and handle corner cases.
rustyrussell Sep 18, 2024
7749abf
askrene: don't have get_flow_paths() handle htlc_max, htlc_min and ex…
rustyrussell Sep 18, 2024
2bb6d16
pytest: test askrene treats htlc_maximum_msat and htlc_minimum_msat r…
rustyrussell Sep 18, 2024
8e376ed
askrene: take into account the reduction in "spendable" with addition…
rustyrussell Sep 18, 2024
08ba250
askrene: move flow refining code to its own file.
rustyrussell Sep 18, 2024
4acd7a8
askrene: re-check min_htlc violations after correcting for MCF rounding.
rustyrussell Sep 19, 2024
c7cfb06
askrene: remove unused function
Lagrang3 Sep 19, 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
14 changes: 6 additions & 8 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -2571,7 +2571,7 @@ static void add_amount_to_side(struct peer *peer,
{
enum tx_role role;

if (amount_sat_zero(amount))
if (amount_sat_is_zero(amount))
peer_failed_warn(peer->pps, &peer->channel_id,
"Unable to add 0 sat fields to calculation");

Expand Down Expand Up @@ -2897,7 +2897,7 @@ static struct amount_sat check_balances(struct peer *peer,
else
itr = &pending_htlcs[TX_ACCEPTER];

if (!amount_msat_add(itr, *itr, htlc->amount))
if (!amount_msat_accumulate(itr, htlc->amount))
peer_failed_warn(peer->pps, &peer->channel_id,
"Unable to add HTLC balance");
}
Expand Down Expand Up @@ -2925,14 +2925,12 @@ static struct amount_sat check_balances(struct peer *peer,
peer->channel->view->owed[REMOTE]))
peer_failed_warn(peer->pps, &peer->channel_id,
"Unable to calculate starting channel amount");
if (!amount_msat_add(&funding_amount,
funding_amount,
pending_htlcs[TX_INITIATOR]))
if (!amount_msat_accumulate(&funding_amount,
pending_htlcs[TX_INITIATOR]))
peer_failed_warn(peer->pps, &peer->channel_id,
"Unable to calculate starting channel amount");
if (!amount_msat_add(&funding_amount,
funding_amount,
pending_htlcs[TX_ACCEPTER]))
if (!amount_msat_accumulate(&funding_amount,
pending_htlcs[TX_ACCEPTER]))
peer_failed_warn(peer->pps, &peer->channel_id,
"Unable to calculate starting channel amount");

Expand Down
2 changes: 1 addition & 1 deletion channeld/commit_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ bool commit_tx_amount_trimmed(const struct htlc **htlcs,
if (trim(htlcs[i], feerate_per_kw, dust_limit,
option_anchor_outputs, option_anchors_zero_fee_htlc_tx,
side)) {
if (!amount_msat_add(amt, *amt, htlcs[i]->amount))
if (!amount_msat_accumulate(amt, htlcs[i]->amount))
return false;
}
}
Expand Down
4 changes: 2 additions & 2 deletions channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ static bool sum_offered_msatoshis(struct amount_msat *total,
*total = AMOUNT_MSAT(0);
for (i = 0; i < tal_count(htlcs); i++) {
if (htlc_owner(htlcs[i]) == side) {
if (!amount_msat_add(total, *total, htlcs[i]->amount))
if (!amount_msat_accumulate(total, htlcs[i]->amount))
return false;
}
}
Expand Down Expand Up @@ -647,7 +647,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
* - SHOULD send a `warning` and close the connection, or send an
* `error` and fail the channel.
*/
if (amount_msat_eq(htlc->amount, AMOUNT_MSAT(0))) {
if (amount_msat_is_zero(htlc->amount)) {
return CHANNEL_ERR_HTLC_BELOW_MINIMUM;
}
if (amount_msat_less(htlc->amount, channel->config[recipient].htlc_minimum)) {
Expand Down
58 changes: 54 additions & 4 deletions common/amount.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,12 @@ WARN_UNUSED_RESULT bool amount_msat_add(struct amount_msat *val,
return true;
}

WARN_UNUSED_RESULT bool amount_msat_accumulate(struct amount_msat *a,
struct amount_msat b)
{
return amount_msat_add(a, *a, b);
}

WARN_UNUSED_RESULT bool amount_msat_sub(struct amount_msat *val,
struct amount_msat a,
struct amount_msat b)
Expand Down Expand Up @@ -380,12 +386,12 @@ bool amount_sat_eq(struct amount_sat a, struct amount_sat b)
return a.satoshis == b.satoshis;
}

bool amount_sat_zero(struct amount_sat a)
bool amount_sat_is_zero(struct amount_sat a)
{
return a.satoshis == 0;
}

bool amount_msat_zero(struct amount_msat a)
bool amount_msat_is_zero(struct amount_msat a)
{
return a.millisatoshis == 0;
}
Expand Down Expand Up @@ -524,15 +530,15 @@ struct amount_sat amount_sat_div(struct amount_sat sat, u64 div)

bool amount_sat_mul(struct amount_sat *res, struct amount_sat sat, u64 mul)
{
if ( mul_overflows_u64(sat.satoshis, mul))
if (mul_overflows_u64(sat.satoshis, mul))
return false;
res->satoshis = sat.satoshis * mul;
return true;
}

bool amount_msat_mul(struct amount_msat *res, struct amount_msat msat, u64 mul)
{
if ( mul_overflows_u64(msat.millisatoshis, mul))
if (mul_overflows_u64(msat.millisatoshis, mul))
return false;
res->millisatoshis = msat.millisatoshis * mul;
return true;
Expand Down Expand Up @@ -560,6 +566,50 @@ bool amount_msat_fee(struct amount_msat *fee,
return amount_msat_add(fee, fee_base, fee_prop);
}

/* Does this input give enough to provide fee for output? */
static bool within_fee(struct amount_msat in,
struct amount_msat out,
u32 fee_base_msat,
u32 fee_proportional_millionths)
{
struct amount_msat with_fee = out;
if (!amount_msat_add_fee(&with_fee,
fee_base_msat,
fee_proportional_millionths))
return false;
return amount_msat_less_eq(with_fee, in);
}

struct amount_msat amount_msat_sub_fee(struct amount_msat in,
u32 fee_base_msat,
u32 fee_proportional_millionths)
{
struct amount_msat out, out_plus_one;

/* out = in - base - (out * prop / 1000000)
* Thus: out * (1 + prop / 1000000) = in - base
* out = (in - base) / (1 + prop / 1000000)
* out = 1000000 * (in - base) / (1000000 + prop)
*
* Since we round the fee down, out can be a bit bigger than
* expected, so we iterate upwards.
*/
if (!amount_msat_sub(&out, in, amount_msat(fee_base_msat)))
return AMOUNT_MSAT(0);
if (!amount_msat_mul(&out, out, 1000000))
return AMOUNT_MSAT(0);
out = amount_msat_div(out, 1000000ULL + fee_proportional_millionths);

/* If we calc reverse, it must work! */
assert(within_fee(in, out, fee_base_msat, fee_proportional_millionths));

/* We can be out-by-one */
if (amount_msat_add(&out_plus_one, out, AMOUNT_MSAT(1))
&& within_fee(in, out_plus_one, fee_base_msat, fee_proportional_millionths))
return out_plus_one;
return out;
}

bool amount_msat_add_fee(struct amount_msat *amt,
u32 fee_base_msat,
u32 fee_proportional_millionths)
Expand Down
13 changes: 11 additions & 2 deletions common/amount.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ WARN_UNUSED_RESULT bool amount_sat_add_sat_s64(struct amount_sat *val,
struct amount_sat a,
s64 b);

/* a += b */
WARN_UNUSED_RESULT bool amount_msat_accumulate(struct amount_msat *a,
struct amount_msat b);

struct amount_msat amount_msat_div(struct amount_msat msat, u64 div);
struct amount_sat amount_sat_div(struct amount_sat sat, u64 div);

Expand All @@ -111,8 +115,8 @@ bool amount_sat_eq(struct amount_sat a, struct amount_sat b);
bool amount_msat_eq(struct amount_msat a, struct amount_msat b);

/* Is a zero? */
bool amount_sat_zero(struct amount_sat a);
bool amount_msat_zero(struct amount_msat a);
bool amount_sat_is_zero(struct amount_sat a);
bool amount_msat_is_zero(struct amount_msat a);

/* Is a > b? */
bool amount_sat_greater(struct amount_sat a, struct amount_sat b);
Expand Down Expand Up @@ -188,6 +192,11 @@ WARN_UNUSED_RESULT bool amount_msat_add_fee(struct amount_msat *amt,
u32 fee_base_msat,
u32 fee_proportional_millionths);

/* Reversed: what is the largest possible output for a given input and fee? */
struct amount_msat amount_msat_sub_fee(struct amount_msat input,
u32 fee_base_msat,
u32 fee_proportional_millionths);

/* What is the fee for this tx weight? */
struct amount_sat amount_tx_fee(u32 fee_per_kw, size_t weight);

Expand Down
121 changes: 107 additions & 14 deletions common/bolt12.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
#include <assert.h>
#include <bitcoin/chainparams.h>
#include <ccan/tal/str/str.h>
#include <ccan/time/time.h>
#include <common/bech32_util.h>
#include <common/bolt12.h>
#include <common/bolt12_merkle.h>
#include <common/configdir.h>
#include <common/features.h>
#include <common/overflows.h>
#include <inttypes.h>
#include <secp256k1_schnorrsig.h>
#include <time.h>
Expand Down Expand Up @@ -324,11 +326,11 @@ char *invoice_encode(const tal_t *ctx, const struct tlv_invoice *invoice_tlv)
return to_bech32_charset(ctx, "lni", wire);
}

struct tlv_invoice *invoice_decode_nosig(const tal_t *ctx,
const char *b12, size_t b12len,
const struct feature_set *our_features,
const struct chainparams *must_be_chain,
char **fail)
struct tlv_invoice *invoice_decode_minimal(const tal_t *ctx,
const char *b12, size_t b12len,
const struct feature_set *our_features,
const struct chainparams *must_be_chain,
char **fail)
{
struct tlv_invoice *invoice;
const u8 *data;
Expand All @@ -344,6 +346,17 @@ struct tlv_invoice *invoice_decode_nosig(const tal_t *ctx,
return NULL;
}

/* BOLT-offers #12:
* - if `invreq_chain` is not present:
* - MUST fail the request if bitcoin is not a supported chain.
* - otherwise:
* - MUST fail the request if `invreq_chain`.`chain` is not a
* supported chain.
* - if `invoice_features` contains unknown _odd_ bits that are non-zero:
* - MUST ignore the bit.
* - if `invoice_features` contains unknown _even_ bits that are non-zero:
* - MUST reject the invoice.
*/
*fail = check_features_and_chain(ctx,
our_features, must_be_chain,
invoice->invoice_features,
Expand Down Expand Up @@ -476,17 +489,97 @@ struct tlv_invoice *invoice_decode(const tal_t *ctx,
char **fail)
{
struct tlv_invoice *invoice;
u64 expiry, now;

invoice = invoice_decode_minimal(ctx, b12, b12len, our_features,
must_be_chain, fail);
if (!invoice)
return NULL;

*fail = check_signature(ctx, invoice->fields,
"invoice", "signature",
invoice->invoice_node_id,
invoice->signature);
if (*fail)
return tal_free(invoice);

invoice = invoice_decode_nosig(ctx, b12, b12len, our_features,
must_be_chain, fail);
if (invoice) {
*fail = check_signature(ctx, invoice->fields,
"invoice", "signature",
invoice->invoice_node_id,
invoice->signature);
if (*fail)
invoice = tal_free(invoice);
/* BOLT-offers #12:
* A reader of an invoice:
* - MUST reject the invoice if `invoice_amount` is not present.
* - MUST reject the invoice if `invoice_created_at` is not present.
* - MUST reject the invoice if `invoice_payment_hash` is not present.
* - MUST reject the invoice if `invoice_node_id` is not present.
*/
if (!invoice->invoice_amount) {
*fail = tal_strdup(ctx, "missing invoice_amount");
return tal_free(invoice);
}
if (!invoice->invoice_created_at) {
*fail = tal_strdup(ctx, "missing invoice_created_at");
return tal_free(invoice);
}
if (!invoice->invoice_payment_hash) {
*fail = tal_strdup(ctx, "missing invoice_payment_hash");
return tal_free(invoice);
}
if (!invoice->invoice_node_id) {
*fail = tal_strdup(ctx, "missing invoicenode_id");
return tal_free(invoice);
}

/* BOLT-offers #12:
* - if `invoice_relative_expiry` is present:
* - MUST reject the invoice if the current time since 1970-01-01 UTC
* is greater than `invoice_created_at` plus `seconds_from_creation`.
* - otherwise:
* - MUST reject the invoice if the current time since 1970-01-01 UTC
* is greater than `invoice_created_at` plus 7200.
*/
if (invoice->invoice_relative_expiry)
expiry = *invoice->invoice_relative_expiry;
else
expiry = 7200;
now = time_now().ts.tv_sec;
/* If it overflows, it's forever */
if (!add_overflows_u64(*invoice->invoice_created_at, expiry)
&& now > *invoice->invoice_created_at + expiry) {
*fail = tal_fmt(ctx, "expired %"PRIu64" seconds ago",
now - (*invoice->invoice_created_at + expiry));
return tal_free(invoice);
}

/* BOLT-offers #12:
* - MUST reject the invoice if `invoice_paths` is not present or is
* empty. */
if (tal_count(invoice->invoice_paths) == 0) {
*fail = tal_strdup(ctx, "missing/empty invoice_paths");
return tal_free(invoice);
}

/* BOLT-offers #12:
* - MUST reject the invoice if `num_hops` is 0 in any
* `blinded_path` in `invoice_paths`.
*/
for (size_t i = 0; i < tal_count(invoice->invoice_paths); i++) {
if (tal_count(invoice->invoice_paths[i]->path) != 0)
continue;
*fail = tal_fmt(ctx, "zero num_hops in path %zu/%zu",
i, tal_count(invoice->invoice_paths));
return tal_free(invoice);
}

/* BOLT-offers #12:
* - MUST reject the invoice if `invoice_blindedpay` is not present.
* - MUST reject the invoice if `invoice_blindedpay` does not contain exactly one `blinded_payinfo` per `invoice_paths`.`blinded_path`.
*/
if (tal_count(invoice->invoice_blindedpay)
!= tal_count(invoice->invoice_paths)) {
*fail = tal_fmt(ctx, "expected %zu payinfo but found %zu",
tal_count(invoice->invoice_paths),
tal_count(invoice->invoice_blindedpay));
return tal_free(invoice);
}

return invoice;
}

Expand Down
17 changes: 10 additions & 7 deletions common/bolt12.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,23 @@ char *invoice_encode(const tal_t *ctx, const struct tlv_invoice *bolt12_tlv);
* @must_be_chain: if non-NULL, chain to enforce.
* @fail: pointer to descriptive error string, set if this returns NULL.
*
* Note: checks signature!
* It checks it's well-formed (has amount, payment_hash, node_id, and
* is not expired). It also checks signature.
*
* Note: blinded path features need to be checked by the caller before use!
*/
struct tlv_invoice *invoice_decode(const tal_t *ctx,
const char *b12, size_t b12len,
const struct feature_set *our_features,
const struct chainparams *must_be_chain,
char **fail);

/* Variant which does not check signature */
struct tlv_invoice *invoice_decode_nosig(const tal_t *ctx,
const char *b12, size_t b12len,
const struct feature_set *our_features,
const struct chainparams *must_be_chain,
char **fail);
/* This one only checks it decides, and optionally is correct chain/features */
struct tlv_invoice *invoice_decode_minimal(const tal_t *ctx,
const char *b12, size_t b12len,
const struct feature_set *our_features,
const struct chainparams *must_be_chain,
char **fail);

/* Check a bolt12-style signature. */
bool bolt12_check_signature(const struct tlv_field *fields,
Expand Down
2 changes: 1 addition & 1 deletion common/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bool route_can_carry_even_disabled(const struct gossmap *map,
if (!gossmap_chan_set(c, dir))
return false;
/* Amount 0 is a special "ignore min" probe case */
if (!amount_msat_eq(amount, AMOUNT_MSAT(0))
if (!amount_msat_is_zero(amount)
&& !gossmap_chan_has_capacity(c, dir, amount))
return false;
return true;
Expand Down
Loading
Loading