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

EXPERIMENTAL: MPP send and receive support (lowlevel) #3309

Merged
merged 22 commits into from Dec 12, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
178892b
db: add partid, total_msat fields to payment entries.
rustyrussell Dec 11, 2019
ad4ed97
db: add partid field to htlc_out.
rustyrussell Dec 11, 2019
2a03434
htlcs: remove origin_htlc_id from htlc_out.
rustyrussell Dec 11, 2019
c654478
lightningd: share more code between sendpay and sendonion.
rustyrussell Dec 11, 2019
04a46f0
lightningd: change amount-in-flight check to be more nuanced.
rustyrussell Dec 11, 2019
c61227b
sendpay/sendonion: add optional partid arg, finesse msatoshi argument.
rustyrussell Dec 11, 2019
15fa972
configure: make partid payments only available with EXPERIMENTAL_FEAT…
rustyrussell Dec 11, 2019
3bc4636
waitsendpay: add partid arg.
rustyrussell Dec 11, 2019
94d3897
pytest: Add tests to make sure received onion is as expected.
rustyrussell Dec 11, 2019
2e4416e
doc: update experimental bolt version quotes.
rustyrussell Dec 11, 2019
d94ae31
lightningd: cleanup redundant args from handle_localpay
rustyrussell Dec 11, 2019
3c6e33a
lightningd: split invoice check into separate function.
rustyrussell Dec 11, 2019
555b217
lightningd: implement htlc sets.
rustyrussell Dec 11, 2019
73bf9e0
lightningd: wrap htlc replay in a database transaction.
rustyrussell Dec 11, 2019
1839483
lightningd: sew in htlc set.
rustyrussell Dec 11, 2019
8cee375
plugins: listpays ignores pre-0.7.0 or manual sendpay payments w/ no …
rustyrussell Dec 11, 2019
84a2753
plugins: listpays will now consolidate multi-part payments.
rustyrussell Dec 11, 2019
c6bbb41
common: offer option_basic_mpp for EXPERIMENTAL_FEATURES.
rustyrussell Dec 11, 2019
cbfc84f
pytest: add more multi-part-payment tests.
rustyrussell Dec 11, 2019
2b4ca09
lightningd: require payment_secret for MPP.
rustyrussell Dec 11, 2019
207ae69
lightningd: fix spurious "more than twice final" error.
rustyrussell Dec 12, 2019
e6edb76
lightningd: fix failure message in waitsendpay with multi-part payments.
rustyrussell Dec 12, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 38 additions & 19 deletions lightningd/pay.c
Expand Up @@ -81,6 +81,8 @@ void json_add_payment_fields(struct json_stream *response,
{
json_add_u64(response, "id", t->id);
json_add_sha256(response, "payment_hash", &t->payment_hash);
if (t->partid)
json_add_u64(response, "partid", t->partid);
if (t->destination != NULL)
json_add_node_id(response, "destination", t->destination);

Expand Down Expand Up @@ -278,9 +280,11 @@ void payment_succeeded(struct lightningd *ld, struct htlc_out *hout,
struct wallet_payment *payment;

wallet_payment_set_status(ld->wallet, &hout->payment_hash,
/* FIXME: Set partid! */ 0,
PAYMENT_COMPLETE, rval);
payment = wallet_payment_by_hash(tmpctx, ld->wallet,
&hout->payment_hash);
&hout->payment_hash,
/* FIXME: Set partid! */ 0);
assert(payment);

tell_waiters_success(ld, &hout->payment_hash, payment);
Expand Down Expand Up @@ -467,14 +471,16 @@ remote_routing_failure(const tal_t *ctx,
return routing_failure;
}

void payment_store(struct lightningd *ld, const struct sha256 *payment_hash)
void payment_store(struct lightningd *ld,
const struct sha256 *payment_hash, u64 partid)
{
struct sendpay_command *pc;
struct sendpay_command *next;
const struct wallet_payment *payment;

wallet_payment_store(ld->wallet, payment_hash);
payment = wallet_payment_by_hash(tmpctx, ld->wallet, payment_hash);
wallet_payment_store(ld->wallet, payment_hash, partid);
payment = wallet_payment_by_hash(tmpctx, ld->wallet,
payment_hash, partid);
assert(payment);

/* Trigger any sendpay commands waiting for the store to occur. */
Expand All @@ -496,7 +502,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
int pay_errcode;

payment = wallet_payment_by_hash(tmpctx, ld->wallet,
&hout->payment_hash);
&hout->payment_hash,
/* FIXME: Set partid! */0);

#ifdef COMPAT_V052
/* Prior to "pay: delete HTLC when we delete payment." we would
Expand Down Expand Up @@ -566,11 +573,11 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
}

/* Save to DB */
payment_store(ld, &hout->payment_hash);
wallet_payment_set_status(ld->wallet, &hout->payment_hash,
payment_store(ld, &hout->payment_hash, /* FIXME: Set partid! */ 0);
wallet_payment_set_status(ld->wallet, &hout->payment_hash, /* FIXME: Set partid! */ 0,
PAYMENT_FAILED, NULL);
wallet_payment_set_failinfo(ld->wallet,
&hout->payment_hash,
&hout->payment_hash, /* FIXME: Set partid! */ 0,
fail ? NULL : hout->failuremsg,
pay_errcode == PAY_DESTINATION_PERM_FAIL,
fail ? fail->erring_index : -1,
Expand All @@ -590,7 +597,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
* Return callback if we called already, otherwise NULL. */
static struct command_result *wait_payment(struct lightningd *ld,
struct command *cmd,
const struct sha256 *payment_hash)
const struct sha256 *payment_hash,
u64 partid)
{
struct wallet_payment *payment;
u8 *failonionreply;
Expand All @@ -604,7 +612,8 @@ static struct command_result *wait_payment(struct lightningd *ld,
struct routing_failure *fail;
int faildirection;

payment = wallet_payment_by_hash(tmpctx, ld->wallet, payment_hash);
payment = wallet_payment_by_hash(tmpctx, ld->wallet,
payment_hash, partid);
if (!payment) {
return command_fail(cmd, PAY_NO_SUCH_PAYMENT,
"Never attempted payment for '%s'",
Expand All @@ -622,7 +631,9 @@ static struct command_result *wait_payment(struct lightningd *ld,

case PAYMENT_FAILED:
/* Get error from DB */
wallet_payment_get_failinfo(tmpctx, ld->wallet, payment_hash,
wallet_payment_get_failinfo(tmpctx, ld->wallet,
payment_hash,
partid,
&failonionreply,
&faildestperm,
&failindex,
Expand Down Expand Up @@ -706,8 +717,10 @@ static struct command_result *
send_payment(struct lightningd *ld,
struct command *cmd,
const struct sha256 *rhash,
u64 partid,
const struct route_hop *route,
struct amount_msat msat,
struct amount_msat total_msat,
const char *label TAKES,
const char *b11str TAKES,
const struct secret *payment_secret)
Expand Down Expand Up @@ -779,7 +792,7 @@ send_payment(struct lightningd *ld,
sphinx_add_hop(path, &pubkey, onion);

/* Now, do we already have a payment? */
payment = wallet_payment_by_hash(tmpctx, ld->wallet, rhash);
payment = wallet_payment_by_hash(tmpctx, ld->wallet, rhash, partid);
if (payment) {
/* FIXME: We should really do something smarter here! */
if (payment->status == PAYMENT_PENDING) {
Expand Down Expand Up @@ -853,18 +866,21 @@ send_payment(struct lightningd *ld,
* onchain_failed_our_htlc->payment_failed with no payment.
*/
if (payment) {
wallet_payment_delete(ld->wallet, rhash);
wallet_local_htlc_out_delete(ld->wallet, channel, rhash);
wallet_payment_delete(ld->wallet, rhash, payment->partid);
wallet_local_htlc_out_delete(ld->wallet, channel, rhash,
payment->partid);
}

/* If hout fails, payment should be freed too. */
payment = tal(hout, struct wallet_payment);
payment->id = 0;
payment->payment_hash = *rhash;
payment->partid = partid;
payment->destination = tal_dup(payment, struct node_id, &ids[n_hops - 1]);
payment->status = PAYMENT_PENDING;
payment->msatoshi = msat;
payment->msatoshi_sent = route[0].amount;
payment->total_msat = total_msat;
payment->timestamp = time_now().ts.tv_sec;
payment->payment_preimage = NULL;
payment->path_secrets = tal_steal(payment, path_secrets);
Expand Down Expand Up @@ -998,7 +1014,7 @@ static struct command_result *json_sendonion(struct command *cmd,
failcode);

/* Now, do we already have a payment? */
payment = wallet_payment_by_hash(tmpctx, ld->wallet, payment_hash);
payment = wallet_payment_by_hash(tmpctx, ld->wallet, payment_hash, /* FIXME: Set partid! */0);
if (payment) {
if (payment->status == PAYMENT_PENDING) {
log_debug(ld->log, "send_payment: previous still in progress");
Expand Down Expand Up @@ -1028,8 +1044,8 @@ static struct command_result *json_sendonion(struct command *cmd,

/* Cleanup any prior payment. We're about to retry. */
if (payment) {
wallet_payment_delete(ld->wallet, payment_hash);
wallet_local_htlc_out_delete(ld->wallet, channel, payment_hash);
wallet_payment_delete(ld->wallet, payment_hash, /* FIXME: Set partid! */0);
wallet_local_htlc_out_delete(ld->wallet, channel, payment_hash, /* FIXME: Set partid! */0);
}

failcode = send_onion(cmd->ld, &packet, first_hop, payment_hash, channel,
Expand Down Expand Up @@ -1247,7 +1263,10 @@ static struct command_result *json_sendpay(struct command *cmd,
}
#endif

res = send_payment(cmd->ld, cmd, rhash, route,
res = send_payment(cmd->ld, cmd, rhash, /* FIXME: Set partid! */ 0,
route,
msat ? *msat : route[routetok->size-1].amount,
/* FIXME: Set total_msat! */
msat ? *msat : route[routetok->size-1].amount,
label, b11str, payment_secret);
if (res)
Expand Down Expand Up @@ -1284,7 +1303,7 @@ static struct command_result *json_waitsendpay(struct command *cmd,
NULL))
return command_param_failed();

res = wait_payment(cmd->ld, cmd, rhash);
res = wait_payment(cmd->ld, cmd, rhash, /* FIXME: Set partid! */0);
if (res)
return res;

Expand Down
3 changes: 2 additions & 1 deletion lightningd/pay.h
Expand Up @@ -18,7 +18,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
const char *localfail);

/* Inform payment system to save the payment. */
void payment_store(struct lightningd *ld, const struct sha256 *payment_hash);
void payment_store(struct lightningd *ld,
const struct sha256 *payment_hash, u64 partid);

/* This json will be also used in 'sendpay_success' notifictaion. */
void json_add_payment_fields(struct json_stream *response,
Expand Down
3 changes: 2 additions & 1 deletion lightningd/peer_htlcs.c
Expand Up @@ -1274,7 +1274,8 @@ static bool update_out_htlc(struct channel *channel,
/* For our own HTLCs, we commit payment to db lazily */
if (hout->origin_htlc_id == 0)
payment_store(ld,
&hout->payment_hash);
&hout->payment_hash,
/* FIXME: Set partid! */ 0);
}

if (!htlc_out_update_state(channel, hout, newstate))
Expand Down
78 changes: 78 additions & 0 deletions wallet/db.c
Expand Up @@ -481,6 +481,84 @@ static struct migration dbmigrations[] = {
{SQL("UPDATE forwarded_payments SET received_time=0 WHERE received_time IS NULL;"),
NULL},
{SQL("ALTER TABLE invoices ADD COLUMN features BLOB DEFAULT '';"), NULL},
/* We can now have multiple payments in progress for a single hash, so
* add two fields; combination of payment_hash & partid is unique. */
{SQL("ALTER TABLE payments RENAME TO temp_payments;"), NULL},
{SQL("CREATE TABLE payments ("
Copy link
Member

Choose a reason for hiding this comment

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

Should we maybe go the extra mile and call this payment_parts instead? We could then pull the common fields into a separate payments table. Would be a nice cleanups and fix that naming nit about pays being the combination of multiple payments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that should be done at the db level; it may be premature optimization?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you're right, that'd be a bigger refactor that we should probably be done during a bit of downtime. It'd just be nice to have an actual payment concept that collects all the attempts that are performed trying to complete it. It may be my OOP/ORM past that is catching up with me 😉

" id BIGSERIAL"
", timestamp INTEGER"
", status INTEGER"
", payment_hash BLOB"
", destination BLOB"
", msatoshi BIGINT"
", payment_preimage BLOB"
", path_secrets BLOB"
", route_nodes BLOB"
", route_channels BLOB"
", failonionreply BLOB"
", faildestperm INTEGER"
", failindex INTEGER"
", failcode INTEGER"
", failnode BLOB"
", failchannel TEXT"
", failupdate BLOB"
", msatoshi_sent BIGINT"
", faildetail TEXT"
", description TEXT"
", faildirection INTEGER"
", bolt11 TEXT"
", total_msat BIGINT"
", partid BIGINT"
", PRIMARY KEY (id)"
", UNIQUE (payment_hash, partid))"), NULL},
{SQL("INSERT INTO payments ("
"id"
", timestamp"
", status"
", payment_hash"
", destination"
", msatoshi"
", payment_preimage"
", path_secrets"
", route_nodes"
", route_channels"
", failonionreply"
", faildestperm"
", failindex"
", failcode"
", failnode"
", failchannel"
", failupdate"
", msatoshi_sent"
", faildetail"
", description"
", faildirection"
", bolt11)"
"SELECT id"
", timestamp"
", status"
", payment_hash"
", destination"
", msatoshi"
", payment_preimage"
", path_secrets"
", route_nodes"
", route_channels"
", failonionreply"
", faildestperm"
", failindex"
", failcode"
", failnode"
", failchannel"
", failupdate"
", msatoshi_sent"
", faildetail"
", description"
", faildirection"
", bolt11 FROM temp_payments;"), NULL},
{SQL("UPDATE payments SET total_msat = msatoshi;"), NULL},
{SQL("UPDATE payments SET partid = 0;"), NULL},
{SQL("DROP TABLE temp_payments;"), NULL},
};

/* Leak tracking. */
Expand Down
18 changes: 13 additions & 5 deletions wallet/test/run-wallet.c
Expand Up @@ -486,7 +486,8 @@ void payment_failed(struct lightningd *ld UNNEEDED, const struct htlc_out *hout
const char *localfail UNNEEDED)
{ fprintf(stderr, "payment_failed called!\n"); abort(); }
/* Generated stub for payment_store */
void payment_store(struct lightningd *ld UNNEEDED, const struct sha256 *payment_hash UNNEEDED)
void payment_store(struct lightningd *ld UNNEEDED,
const struct sha256 *payment_hash UNNEEDED, u64 partid UNNEEDED)
{ fprintf(stderr, "payment_store called!\n"); abort(); }
/* Generated stub for payment_succeeded */
void payment_succeeded(struct lightningd *ld UNNEEDED, struct htlc_out *hout UNNEEDED,
Expand Down Expand Up @@ -1254,29 +1255,36 @@ static bool test_payment_crud(struct lightningd *ld, const tal_t *ctx)
t->id = 0;
t->msatoshi = AMOUNT_MSAT(100);
t->msatoshi_sent = AMOUNT_MSAT(101);
t->total_msat = t->msatoshi;
t->status = PAYMENT_PENDING;
t->payment_preimage = NULL;
memset(&t->payment_hash, 1, sizeof(t->payment_hash));
t->partid = 0;

db_begin_transaction(w->db);
wallet_payment_setup(w, tal_dup(NULL, struct wallet_payment, t));
wallet_payment_store(w, &t->payment_hash);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash);
wallet_payment_store(w, &t->payment_hash, 0);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash, 0);
CHECK(t2 != NULL);
CHECK(t2->status == t->status);
CHECK(sha256_eq(&t2->payment_hash, &t->payment_hash));
CHECK(t2->partid == t->partid);
CHECK(node_id_cmp(t2->destination, t->destination) == 0);
CHECK(amount_msat_eq(t2->msatoshi, t->msatoshi));
CHECK(amount_msat_eq(t2->msatoshi_sent, t->msatoshi_sent));
CHECK(amount_msat_eq(t2->total_msat, t->total_msat));
CHECK(!t2->payment_preimage);

t->status = PAYMENT_COMPLETE;
t->payment_preimage = tal(w, struct preimage);
memset(t->payment_preimage, 2, sizeof(*t->payment_preimage));
wallet_payment_set_status(w, &t->payment_hash, t->status,
wallet_payment_set_status(w, &t->payment_hash, t->partid, t->status,
t->payment_preimage);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash, t->partid);
CHECK(t2 != NULL);
CHECK(t2->status == t->status);
CHECK(sha256_eq(&t2->payment_hash, &t->payment_hash));
CHECK(t2->partid == t->partid);
CHECK(node_id_eq(t2->destination, t->destination));
CHECK(amount_msat_eq(t2->msatoshi, t->msatoshi));
CHECK(amount_msat_eq(t2->msatoshi_sent, t->msatoshi_sent));
Expand Down