We used to have "unsaved" payments: now we don't we can use
our normal "iterator" pattern rather than returning arrays.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It used to be used for both `sendpay` and `waitsendpay` but now it's
only for the latter, so the name is confusing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We didn't write to db immediately, but waited until it the actual HTLC got
added (or failed). That way we didn't have a separate transaction to
write the payment into the db, but the complexity is not worth it: it
makes the next refactors harder, since we can't use the normal
iterator patterns like we do with the rest of the db (as we have to add
the unstored ones).
We might as well also make sendpay return immediately: we used to return
once the HTLC had been confirmed sent, since we entered it in the db
at that point, but we can keep it simple now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Adding an index means:
1. Add the new subsystem, and new updated_index field to the db, and
create xxx_index_deleted/created/updated APIs.
2. Hook up these functions to the points they need to be called.
3. Add index, start and limit fields to the list command.
4. Add created_index and updated_index into the list command.
This does #1.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes `check` much more thorough, and useful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `check` now does much more checking on every command (not just basic parameter types).
This is the simplest solution, not the best, but there's significant risk in try to remove the "we have a path" assumption in the code pay code.
Includes removing a `tal_steal` which was incorrect: the buffer has the same lifetime as the plugin, so if we steal it then things get messy when we free the struct payment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `pay` will now pay your own invoices if you try.
Previously, the "payment" and "invoice" paths were completely separate, but this now calls both. It bypasses htlc_sets (and thus, cannot do MPP), and bypasses the hook too: the former is tied closely to HTLCs, and the hook is also very htlc-centric.
Includes finishing unfinished sentence in sendpay man page, as a bonus.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `sendpay` now allows self-payment of invoices, by specifying an empty route.
The channel selection did not consider the amounts that we are trying
to transfer, which in a multiplexed channel world could end up always
selecting a channel that is too small for the payment. We also log
which channel was selected based on the selector that is passed in,
allowing us to better follow the decisions.
Changelog-Fixed: pay: `sendonion` and `sendpay` will now consider amounts involved when using picking one channel for a peer
`struct log` becomes `struct logger`, and the member which points to the
`struct log_book` becomes `->log_book` not `->lr`.
Also, we don't need to keep the log_book in struct plugin, since it has
access to ld's log_book.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, our code checked for the presence of the `lightning:`
prefix while decoding a bolt11 string. Although this prefix is valid
and accepted by the core lightning pay command, it was causing issues
with how we managed invoices. Specifically, we were skipping the prefix
when creating a copy of the invoice string and storing the raw invoice
(including the prefix) in the database, which caused inconsistencies
in the user experience.
To address this issue, we need to strip the `lightning:` prefix before
calling each core lightning command. In addition, we should
modify the invstring inside the db with the canonical one.
This commit fixes the issue by stripping the `lightning:` prefix
from the `listsendpays` function, which will improve the
user experience and ensure consistency in our invoice management (see
next commit).
Reported-by: @johngribbin
Link: ElementsProject#6207
Fixes: debbdc0
Changelog-Fixes: trim the `lightning:` prefix from invoice everywhere.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We usually have access to `ld`, so avoid the global.
The only place generic code needs it is for the json command struct,
and that already has accessors: add one for libplugin and lightningd
to tell it if deprecated apis are OK.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"BOLT 4: Remove legacy format, make var_onion_optin compulsory."
This also renamed the redundant "tlv_payload" to "payload", so we
replace "tlv_tlv_payload" with "tlv_payload" everyhere!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are cases (difficult to reproduce with a test) where
a payment will fail one time and succeed later.
As far I understand in this case the groupid field of the payment
is the same, and the only thing that change is the status, so
our logic inside the delpay is ambiguous where it is not
possible to delete a payment as described in https://github.com/ElementsProject/lightning/issues/6114
A sequence of commands that explain the problem is
```
$ lc -k listpays payment_hash=H
{
"pays": [
{
"bolt11": "I",
"destination": "redacted",
"payment_hash": "H",
"status": "complete",
"created_at": redacted,
"completed_at": redacted,
"preimage": "P",
"amount_msat": "redacted",
"amount_sent_msat": "redacted"
}
]
}
$ lc delpay H complete
{
"code": 211,
"message": "Payment with hash H has failed status but it should be complete"
}
```
In this case, the delpay is not able to delete a payment because the
listpays is returning only the succeeded one, so by running the
listsendpays we may see the following result where our delpay logic
will be stuck because it works to ensure that all the payments stored
in the database has the status specified by the user
```
➜ VincentSSD clightning --testnet listsendpays -k payment_hash=7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4
{
"payments": [
{
"id": 322,
"payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
"groupid": 1,
"partid": 1,
"destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
"amount_msat": 300,
"amount_sent_msat": 1664,
"created_at": 1679510203,
"completed_at": 1679510205,
"status": "failed",
"bolt11": "lntb1pjpkj4xsp52trda39rfpe7qtqahx8jjplhnj3tatxy8rh6sc6afgvmdz7n0llspp50lr5hmdm0re0xvcp2hv3nf2wwvx0r8q3h3e7jmqz0awdfg6w206qdp0w3jhxarfdenjqargv5sxgetvwpshjgrzw4njqun9wphhyaqxqyjw5qcqp2rzjqtp28uqy77te96ylt7ek703h4ayldljsf8rnlztgf3p8mg7pd0qzwf8a3yqqpdqqqyqqqqt2qqqqqqgqqc9qxpqysgqgeya2lguaj6sflc4hx2d89jvah8mw9uax4j77d8rzkut3rkm0554x37fc7gy92ws9l76yprdva2lalrs7fqjp9lcx40zuty8gca0g5spme3dup"
},
{
"id": 323,
"payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
"groupid": 1,
"partid": 2,
"destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
"amount_msat": 300,
"amount_sent_msat": 3663,
"created_at": 1679510205,
"completed_at": 1679510207,
"status": "failed"
},
{
"id": 324,
"payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
"groupid": 1,
"partid": 3,
"destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
"amount_msat": 300,
"amount_sent_msat": 3663,
"created_at": 1679510207,
"completed_at": 1679510209,
"status": "failed"
},
{
"id": 325,
"payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
"groupid": 1,
"partid": 4,
"destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
"amount_msat": 300,
"amount_sent_msat": 4663,
"created_at": 1679510209,
"completed_at": 1679510221,
"status": "complete",
"payment_preimage": "43f746f2d28d4902489cbde9b3b8f3d04db5db7e973f8a55b7229ce774bf33a7"
}
]
}
```
This commit solves the problem by forcing the delete query in the
database to specify status too, and work around this kind of
ambiguous case.
Fixes: f52ff07558 (lightningd: allow delpay to delete a specific payment.)
Reported-by: Antoine Poinsot <darosior@protonmail.com>
Link: https://github.com/ElementsProject/lightning/issues/6114
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Developed-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: delpay be more pedantic about delete logic by allowing
delete payments by status directly on the database.
It works for the trivial case, where groupid and partid are the same,
but silently deletes nothing in the other cases (or worse, deletes the
wrong entry!).
See: #5835
Changelog-Fixed: `delpay`: actually delete the specified payment (mainly found by `autoclean`).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When a channel is not yet confirmed onchain and only has an alias,
`getroute` will return the alias under the "channel" key. However, if
you supply that to `sendpay`, it will not be able to find the channel
and will fail since it calls `find_channel_for_htlc_add` and that only
looks for channels by their scid and doesn't consider their alias.
This patch makes `find_channel_for_htlc_add` also consider channel
aliases when looking for channels.
Changelog-Fixed: JSON-RPC: `sendpay` now can send to a short-channel-id alias for the first hop.
We no longer use offers for "I want to send you money", but we'll use
invoice_requests directly. Create a new table for them, and
associated functions.
The "localofferid" for "pay" and "sendpay" is now "localinvreqid".
This is an experimental-only option, so document the change under
experimental only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: JSON-RPC: `pay` and `sendpay` `localofferid` is now `localinvreqid`.
I know this is an unforgivably large diff, but the spec has changed so
much that most of this amounts to a rewrite.
Some points:
* We no longer have "offer_id" fields, we generate that locally, as all
offer fields are mirrored into invoice_request and then invoice.
* Because of that mirroring, field names all have explicit offer/invreq/invoice
prefixes.
* The `refund_for` fields have been removed from spec: will re-add locally later.
* quantity_min was removed, max == 0 now mean "must specify a quantity".
* I have put recurrence fields back in locally.
This brings us to 655df03d8729c0918bdacac99eb13fdb0ee93345 ("BOLT 12:
add explicit invoice_node_id.")
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Don't shoehorn it into onion_nonfinal_hop() and onion_final_hop(), but
provide an explicit routine "blinded_onion_hops" and an onion helper
"onion_blinded_hop()" for it to call.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"sphinx_add_hop" takes a literal hop to include,
"sphinx_add_modern_hop" prepends the length. Now we always prepend a
length, make it clear that the literal version is a shortcut:
* sphinx_add_hop -> sphinx_add_hop_has_length
* sphinx_add_modern_hop -> sphinx_add_hop
In addition, we check that length is actually correct! This means
`createonion` can no longer create legacy or otherwise-invalid onions:
fix tests and update man page to remove legacy usage.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `createonion` no longer allows non-TLV-style payloads.
This is actually what the autoclean plugin wants, especially since
you can't otherwise delete a payment which has failed then succeeded.
But insist on neither or both being specified, at least for now.
Changelog-Added: JSON-RPC: `delpay` takes optional `groupid` and `partid` parameters to specify exactly what payment to delete.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows GDB to print values, but also allows us to use them in
'case' statements. This wasn't allowed before because they're not
constant terms.
This also made it clear there's a clash between two error codes,
so move one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: Error code from bcli plugin changed from 400 to 500.
Reported-by: Warren Togami
Changelog-Changed: `sendpay` and `sendonion` now obey the first hop "channel" short_channel_id, if specified.
Changelog-Deprecated: `sendpay` and `sendonion` could take a bogus first hop "channel" short_channel_id
There are hardly any lightningd-specific JSON functions: all that's left
are the feerate ones, and there's already a comment that we should have
a lightningd/feerate.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have them split over common/param.c, common/json.c,
common/json_helpers.c, common/json_tok.c and common/json_stream.c.
Change that to:
* common/json_parse (all the json_to_xxx routines)
* common/json_parse_simple (simplest the json parsing routines, for cli too)
* common/json_stream (all the json_add_xxx routines)
* common/json_param (all the param and param_xxx routines)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since it's a deprecation, we simply ignore one, rather than properly
checking they match etc.
Fixes: #5386
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should be using amount_msat always. Many tests were not. Plus,
deprecating it simplifies the code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSONRPC: `sendpay` `route` elements `msatoshi` (use `amount_msat`)
This is consistent with our output changes, and increases consistency.
It also keeps future sanity checks happy, that we only use JSON msat
helpers with '_msat' fields.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice`: `msatoshi` argument is now called `amount_msat` to match other fields.
Changelog-Deprecated: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice` `msatoshi` (use `amount_msat`)
We keep the parameter around for future use (e.g. PTLC support?), but clboss
master still sets 'style' to "legacy". Accept, but ignore it.
Reported-by: grubman on #c-lightning (IRC)
Changelog-Deprecated: JSON-RPC: `sendpay` `route` argument `style` "legacy" (don't use it at all, we ignore it now and always use "tlv" anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is generally verboten now, since there can be multiple. There are a
few exceptions:
1. We sometimes want to know if there are *any* active channels.
2. Some dev commands still take peer id when they mean channel_id.
3. We still allow peer id when it's fully determined.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `close` by peer id will fail if there is more than one live channel (use `channel_id` or `short_channel_id` as id arg).
As per proposal in https://github.com/lightning/bolts/pull/962
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: protocol: support for legacy onion format removed, since everyone supports the new one.
