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