This modifier splits a payment that has been attempted a number of times (by a
modifier earlier in the mod chain) and has failed consistently. It splits the
amount roughly in half, with a but if random fuzz, and then starts a new round
of attempts for the two smaller amounts.
Changelog-Added: The MPP presplit modifier splits large payments into 10k satoshi parts to maximize chances of performing the payment and to obfuscate the overall amount being sent.
With the `presplit`-modifier we actually skip execution of the root altogether
which results in the root not having a result at all. Instead we should use
the result returned by `payment_collect_result`.
With MPP we require that the sum of parts is equal to the `total_msat` amount
declared in the onion. Since that can't be changed once the first part arrives
we need a way to disable amount fuzzing for MPP.
Technically an API break, but nobody relies on these I hope!
Note that the feerates warning was buried inside the style object:
it should be top-level.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were applying the fee exemption to all payments individually, which is ok
until we switch to MPP, where amounts change. Also the log entry was referring
to the total amount, and not the fee of the payment.
This was causing the state flapping test to fail, since we were yielding
control of the io_loop, waiting for the blockheight to be reached, and not
setting the status beforehand. An interim `paystatus` call would then find a
failed leaf and deduce the entire payment failed. Setting it back to the
previous state keeps the overall payment pending while we wait.
As suggested during the paymod-03 review it is better to activate the new code
right away, and give users an escape hatch to use the legacy code instead. The
way I implemented it allows using either `legacypay` or `pay` and then set
`legacy` to switch to the other implementation.
Changelog-Added: JSON-RPC: The `pay` command now uses the new payment flow, the new `legacypay` command can be used to issue payment with the legacy code if required.
Suggested-by: Rusty Russell <@rustyrussell>
Suggested-by: ZmnSCPxj <@zmnscpxj>
This makes use of the payment modifier structure to just add the preimage to
the TLV payload for the last hop.
Changelog-Added: JSON-RPC: The `keysend` command allows sending to a node without requiring an invoice first.
It turns out that the `failcodename` doesn't get populated if the `failcode`
isn't a known error from the enum (duh...) so don't fail parsing if it's
missing.
This commit collects the changes required to the tests caused by the changes
to the `pay` and `paystatus` commands. They are also rather good hints as to
what these changes entail.
We want to differentiate a wrong block-height from other failure reasons, such
as an unknown `payment_hash`, so we skip the `waitblockheight` if we're
already at the correct height.
Add/remove the HTLC amount from the channel hints so concurrent getroute calls
have the correct exclusions. This can sometimes underflow, if we're unlucky
and call getroute too closely, but that's not a big issue, it can only result
in a failed MPP attempt too many, nothing fatal, and it'll get corrected based
on the result returned by the failing node.
These are primarily the fee and cltv constraints that we need to keep up to
date in order to give modifiers a correct view of what is and what isn't
allowed.
We could end up with multiple channel hints, which is a bit wasteful. We now
look for existing ones before adding a new one, and if one exists we use the
more restrictive parameters.
Suggested-by: Lisa Neigut <@niftynei>
We're lucky that we can distinguish the severity of the failure based on the
failcode, so we bubble up the one with the maximum failcode, and let callers
inspect details if they need more information.
We can have quite detailed information about our local channels, so call
`listpeers` before the `getroute` call on the root payment, to seed that
information in the channel_hints.
Since we end up consolidating some of the return values for `pay` and
`paystatus` and change the public interface we need to add the compatibility
flag and guard the switchover behind it.
most likely unused since the switch to libwally for internal blockchain
things.
these method names were clashing with ones that are to be introduced
with some libwally cleanups, so getting rid of them pre-emptively keeps
us libwally compatible
We need to keep them around so we can inspect them later. We'll also need a
background cleanup every once in a while to free some memory. More on that in
a future commit.
We were just handwaving the partid generation, which broke some tests that
expected the first payment attempt to always have partid=0, so here we just
track the partids assigned in the payment tree, starting at 0.
The status of what started as a simple JSON-RPC call is now spread across an
entire tree of partial payments and payment attempts. So we collect the status
in a single struct in order to report back success of failure.
This is just for testing for now, TLV payload computation will come next. We
stage all the payloads in deserialized form so modifiers can modify them more
easily and serialize them only before actually calling `createonion`.
This is necessary so we can build the absolute locktimes in the next step. For
now just fetch the blockheight on each (sub-)payment, later we can reuse the
root blockheight if it ends up using too much traffic.
A payment is considered finished if it is in a final state (success or
failure) or all its sub-payments are finished. If that's the case we notify
`payment_finished` and bubble up through `payment_child_finished`, eventually
bubbling up to the root, which can then report success of failure back to the
RPC command that initiated the whole process.
This is likely a bit of overkill for this type of functionality, but it is a
nice first use-case of how functionality can be compartmentalized into
modifiers. If makes swapping retry mechanisms in and out really simple.
This should make it easy for JSON-RPC functions and modifiers to get the
associated data for a given modifier name. Useful if a modifier needs to
access its parent's modifier data, or in other functions that need to access
modifier data, e.g., when passing destination pointers into the `param()`
call.
This commit can be reverted/skipped once we have implemented all the logic and
have feature parity with the normal `pay`. It's main purpose is to expose the
unfinished functionality to test it, without completely breaking the existing
`pay` command.
```
plugins/keysend.c:136:47: error: format specifies type 'unsigned long' but the argument has type 'time_t' (aka 'int') [-Werror,-Wformat]
ki->label = tal_fmt(ki, "keysend-%lu.%09lu", now.ts.tv_sec, now.ts.tv_nsec);
~~~ ^~~~~~~~~~~~~
%d
```
Changelog-None
The autocleaning will only happen if the autocleaninvoice-cycle startup
option is passed, which cannot happen if the plugin is started
post-startup.
Thus, it's less misleading for users to restrict its usage to startup.
Changelog-Added: plugins: The `autoclean` plugin is now static (you cannot manage it with the `plugin` RPC command anymore).
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
We did not take the value of --commit-fee into account : this removes
the unused option from lightningd and instead registers it in bcli,
where we set the actual feerate of commitment transactions. This also
corrects the documentation.
Changelog-Fixed: config: we now take the --commit-fee parameter into account.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Since we now over-write the wally malloc/free functions, we need to do
so for tests as well. Here we pull up all of the common setup/teardown
logic into a separate place, and update the tests that use libwally to
use the new common_setup core
Changelog-None
And the percentage of the initial amount, not the constently increasing
one !
Changelog-Fixed: pay: we now respect maxfeepercent, even for tiny amounts.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
They are already logged both in bcli, and in lightningd.
This just adds a lot of noise to the logs. We keep successed attempts
though for the tests.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
The documentation was wrong, and I copied my mistake to `libplugin` where it
was then ignored instead of ORed into the node's featurebits. This fixes both.
As discussed with Christian, prepending the length to the payload returned
is awkward, but it's the only way to set a legacy payload. As this will
be soon deprecated, simplify the external API.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
So far we were relying on `lightningd` to create an ad-hoc invoice when
telling it to `resolve` with a given preimage. We now switch to having the
plugin create the invoice, remove the mandatory `keysend_preimage`
field (which would upset `lightningd` otherwise), and then return the modified
payload with the instructions to `continue` instead of resolving.
This ties back in with the existing payment/invoice handling code. Invoices
are created only if we don't have a label clash (unlikely since we have the
nano-time in the label), or the `payment_hash` was already used for another
invoice (at which point `lightningd` will automatically reject the payment and
we're a bit poorer for it, but meh :-)
The generated wrappers will ignore the raw fields and will only consider the
shortcut fields. This function takes the raw fields and serializes them
instead.
This still uses the experimental TLV-type, but once the type is standardized
we can add detection for the new type quite easily.
Changelog-Added: pay: The `keysend` plugin implements the ability to receive spontaneous payments (keysend)
While we removed the `satoshi` param in #3603 it appears that the
`fundchannel` plugin was still passing it to the `fundchannel_start`
call. This fixes up the help text. Notice that technically the help text
changes the param name, but since it was internally always called `amount`
this change doesn't break the API, the help was just wrong.
They now use -fno-common by default, so duplicated variables cause
a link error:
/usr/bin/ld: common/utils.o:(.bss+0x10): multiple definition of `chainparams'; plugins/libplugin.o:(.bss+0x0): first defined here
collect2: error: ld returned 1 exit status
make: *** [Makefile:408: plugins/autoclean] Error 1
This was introduced in 9ebfdf0b8c.
Fixes: #3597
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Multiple definition of chainparams on Fedora (or other really recent gcc)
The shadow route algorithm is extending the route randomly using channels
adjacent to the current destination, in the hope to create a plausible route
extension. However, instead of only retrieving the channels adjacent to the
destination it was retrieving all channels in the entire topology, and
selecting a random channel from there. This resulted in a very large request
for all channels being processed, and then mostly not being used, but also in
shadow extensions to the path which were not plausible (they didn't extend the
real path, just random edges). This is fixed by restricting the call to
`listchannels` to the channels with the current destination as source.
On my laptop retrieving all channels in the current mainnet takes
approximately 1.2 seconds, and given the geometric series expansion of the 50%
extension probability this indeed would result in an overhead of 1.2 seconds
to the `pay` command. In contrast specifying a source results in an overhead
of ~30ms.
So good news everyone, your pay commands just shaved 1.17 seconds off their
runtime.
Changelog-Changed: pay: Improved the performance of the `pay`-plugin by limiting the `listchannels` when computing the shadow route.
Changelog-Fixed: pay: The `pay`-plugin was generating non-contiguous shadow routes
It's almost always "their_features" and "our_features" respectively, so
make those names clear.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Turns out that unnecessary: all callers can access the feature_set,
so make it much more like a normal primitive.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
A CONSERVATIVE/3 target for them.
Some noisy changes to the tests as we had to update the estimatesmartfee
mock.
Changelog-Changed: We now use a higher feerate for resolving onchain HTLCs and for penalty transactions
So we can't tell people they should use amount, until v0.8.2 is
released. Another 6 months before we can deprecated the 'satoshi'
field here :(
Fixes: d149ba2f3a
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON: `fundchannel_start` returns `amount` even when deprecated APIs are enabled.
Changelog-Deprecated: JSON: `fundchannel_start` `satoshi` field really deprecated now (use `amount`).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON: `fundchannel` and `fundchannel_start` `satoshi` parameter removed (renamed to `amount` in 0.7.3).
This is a common thing to do, so create a macro.
Unfortunately, it still needs the type arg, because the paramter may
be const, and the return cannot be, and C doesn't have a general
"(-const)" cast.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Before this patch we used to send `double`s over the wire by just
copying them. This is not portable because the internal represenation
of a `double` is implementation specific.
Instead of this, multiply any floating-point numbers that come from
the outside (e.g. JSONs) by 1 million and round them to integers when
handling them.
* Introduce a new param_millionths() that expects a floating-point
number and returns it multipled by 1000000 as an integer.
* Replace param_double() and param_percent() with param_millionths()
* Previously the riskfactor would be allowed to be negative, which must
have been unintentional. This patch changes that to require a
non-negative number.
Changelog-None
Spark does this, for example:
{"method":"pay","params":["lnbc..."],"id":22}
Which doesn't have a jsonrpc field. The result is that the command
doesn't terminate, there is nothing in the logs, stderr contains
"pay: JSON-RPC message does not contain "jsonrpc" field", and
from then on "Unknown command 'pay'".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We are going to initialize a plugin before its creation, so log as
UNUSUAL instead.
Also, `pay` and `fundchannel` inits are using rpc_delve(), so we need to
io_new_conn() (which sets the socket as non blocking) after calling the
plugin's init.
This is also taken and adapted from lightningd/bitcoind.
The call to 'getblockchaininfo' is replaced by 'echo' as we don't
make use of the result and the former can sometimes be slow (e.g. on
IBD).
Most is taken from lightningd/bitcoind and adapted. This currently
exposes 5 commands:
- `getchaininfo`, currently called at startup to check the network and
whether we are on IBD.
- `getrawblockbyheight`, which basically does the `getblockhash` +
`getblock` trick.
- `getfeerate`
- `sendrawtransaction`
- `getutxout`, used to gather infos about an output and currently used by
`getfilteredblock` in `lightningd/bitcoind`.
We don't take the callback result into account, so it can better be void.
Having a general callback parameter is handy, because for bcli we want
to pass it the struct bcli.
As a separated commit because it was pre-existent (changelog + xfail test).
This also fix a logical problem in lightningd/plugin_control: we were
assuming a plugin started with 'plugin start' but which did not comport
a 'dynamic' entry in its manifest to be dynamic, though it should have
been treated as static.
Changelog-fixed: plugins: Dynamic C plugins can now be managed when lightningd is up
This adds helpers to start and send a jsonrpc request using json_stream
in order to benefit from the helpers.
This then simplifies existing plugins RPC requests by using json_stream
helpers.
This pass to json_stream helpers for commands outputs, but keeps
compatibility with existing plugins which use jout as of now, by not
starting/closing the "result"/"error" objects.
Now we have streams and a global object, we can use them for io_plans.
This makes the logic closer from lightningd/plugin (it has been mostly
stolen from here), and simplifies the main.
This also allows plugins to use io_plans (thanks to the io_loop) for
asynchronous connections.
This commit only handle incoming commands.
Now that we have json_stream in common/, we can move all the related
helpers from lightningd/json to common/json. This way everyone can
benefit of them (including libplugin, the plugins themselves,
potentially lightning-cli), not lightningd alone!
Note that the Makefile of the common/test/ had to be modified, because
the new helpers make use of common/wireaddr... Which turns out to
\#include <lightingd/lightningd.h> ! So we couldnt just include the .c
and add mocks if we redefined some structs (hello run-param).
GCC 10 defaults to `-fno-common`. no longer automatically sharing
global variable definitions, which makes it important to define
them in only one place (otherwise there will be duplicate definition
errors). Add `extern` qualifiers where (I think) is the best place for
them.
Before this patch we used `int` for error codes. The problem with
`int` is that we try to pass it to/from wire and the size of `int` is
not defined by the standard. So a sender with 4-byte `int` would write
4 bytes to the wire and a receiver with 2-byte `int` (for example) would
read just 2 bytes from the wire.
To resolve this:
* Introduce an error code type with a known size:
`typedef s32 errcode_t`.
* Change all error code macros to constants of type `errcode_t`.
Constants also play better with gdb - it would visualize the name of
the constant instead of the numeric value.
* Change all functions that take error codes to take the new type
`errcode_t` instead of `int`.
* Introduce towire / fromwire functions to send / receive the newly added
type `errcode_t` and use it instead of `towire_int()`.
In addition:
* Remove the now unneeded `towire_int()`.
* Replace a hardcoded error code `-2` with a new constant
`INVOICE_EXPIRED_DURING_WAIT` (903).
Changelog-Changed: The waitinvoice command would now return error code 903 to designate that the invoice expired during wait, instead of the previous -2
Changelog-Fixed: Detect a previously non-permanent error (`final_cltv_too_soon`) that has been merged into a permanent error (`incorrect_or_unknown_payment_details`), and retry that failure case in `pay`.
it's that time of year (merry xmas!)
enables the ability to push_msat on fundchannel
Changelog-Added: RPC: `fundchannel` and `fundchannel_start` can now accept an optional parameter, `push_msat`, which will gift that amount of satoshis to the peer at channel open.
This won't usually be visible to the end-user, since the pay plugin doesn't
do multi-part yet (and mpp requires EXPERIMENTAL_FEATURES), but we're ready
once it does.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The pay plugin has been supplying the bolt11 string since 0.7.0, so only
ancient "pay" commands would be omitted by this change.
You can create a no-bolt11 "sendpay" manually, but then you'll find it
in 'listsendpays'.
Changelog-Removed: JSON: `listpays` won't shown payments made via sendpay without a bolt11 string, or before 0.7.0.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: RPC: The 'fundchannel' command now tries to connect to the peer before funding the channel, no need to 'connect' before 'fundchannel' anymore !
When doing the random walk through the channel, we now add the fees
(both the base and the proportional one) for that channel in addition to
the cltv delta.
Changelog-Added: Payment amount fuzzing is restored, but through shadow routing.
I was wondering why TAGS was missing some functions, and finally
tracked it down: PRINTF_FMT() confuses etags if it's at the start
of a function, and it ignores the rest of the file.
So we put PRINTF_FMT at the end, but that doesn't work for
*definitions*, only *declarations*. So we remove it from definitions
and add gratuitous declarations in the few static places.1
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently the only source for amount_asset is the value getter on a tx output,
and we don't hand it too far around (mainly ignoring it if it isn't the
chain's main currency). Eventually we could bubble them up to the wallet, use
them to select outputs or actually support assets in the channels.
Since we don't hand them around too widely I thought it was ok for them to be
pass-by-value rather than having to allocate them and pass them around by
reference. They're just 41 bytes currently so the overhead should be ok.
Signed-off-by: Christian Decker <@cdecker>
The fundchannel plugin needs to know how to build a transaction, so we need to
tell it which chainparams to use. Also adds `chainparams` as a global, since
that seems to be the way to do things in plugins.
in order to preserve current behavior, we cap at max if specified 'all';
otherwise we fail since the amount requested is larger than the
channel max capacity
Previously, we'd fail on 'all'. `fundchannel_start` needs an amount
in order to start a funding transaction.
The way that we approach this is to first call `txprepare` with a
placeholder address and the 'all' amount; this will return the maximum
amount available. We then clamp this to the max_funding (currently
hardcoded, in the future we'd want to consult our/the peer's features) and
then use the amount on the output in the prepared transaction as the
funding amount. We then pass this amount to fundchannel_start,
after we've started it successfully we cancel the held placeholder
transaction and prepare a second transaction for the exact amount,
using the funding address that fundchannel_start passed back.
Christian Decker
Rusty Russell
ZmnSCPxj jxPCSnmZ
niftynei
Vasil Dimov
Antoine Poinsot
Michael Schmoock
Wladimir J. van der Laan
Jorge Timón
trueptolemy