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.
The string cut & paste hack was nasty; make ->failure a json_out
object so we can splice it in properly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now hand around struct json_out members, rather than using formatted
strings, so plugins need to construct them properly.
There's no automatic conversion between ' and " any more, so those
are eliminated too. pay still uses some manual construction of elements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since this handles escaping for us, this automatically fixes our previous
escaping issued.
Fixes: #2612
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are generalized from our internal implementations.
The main difference is that 'struct json_escaped' is now 'struct
json_escape', so we replace that immediately.
The difference between lightningd's json-writing ringbuffer and the
more generic ccan/json_out is that the latter has a better API and
handles escaping transparently if something slips through (though
it does offer direct accessors so you can mess things up yourself!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Allow timers to be freed in their callback.
2. Clarify in header that we have to terminate our timer with timer_finished()
eventually.
3. We don't currently have plugins with more than one outstanding timer, but
it certainly would be possible, so fix in_timer to be a counter.
Suggested-by: @ZmnSCPxj
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I tried to just do gossipd, but it was uncontainable, so this ended up being
a complete sweep.
We didn't get much space saving in gossipd, even though we should save
24 bytes per node.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Node ids are pubkeys, but we only use them as pubkeys for routing and checking
gossip messages. So we're packing and unpacking them constantly, and wasting
some space and time.
This introduces a new type, explicitly the SEC1 compressed encoding
(33 bytes). We ensure its validity when we load from the db, or get it
from JSON. We still use 'struct pubkey' for peer messages, which checks
validity.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
39475-39572(39518+/-36),2880732,41.150000-41.390000(41.298+/-0.085),2.260000-2.550000(2.336+/-0.11),44.390000-65.150000(58.648+/-7.5),32.740000-33.020000(32.89+/-0.093),44.130000-45.090000(44.566+/-0.32)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
*best is checked for null before the comparison against the uninitialized
variable ever happens, so this isn't a real issue.
Initialize it to zero so that we don't fail to compile on certain gcc versions.
plugins/pay.c: In function ‘add_shadow_route’:
plugins/pay.c:644:18: error: ‘sample’ may be used uninitialized in this function
if (!best || v > sample) {
~~^~~~~~~~
Signed-off-by: William Casarin <jb55@jb55.com>
New name is less confusing, and most people should be transitioning to
listpays rather than this anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is to future-proof against multi-part-payments: the low-level commands
will start returning multiple results once we have that, so prepare
transition plan now.
Closes: #2372
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the same deprecation, but one level up. For the moment, we
still support invoices with a `h` field (where description will be
necessary) but that will be removed once this option is removed.
Note that I just changed pylightning without backwards compatibility,
since the field was unlikely to be used, but we could do something
more complex here?
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to do it in various places, but we shouldn't do it lightly:
the primitives are there to help us get overflow handling correct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is particularly interesting because we handle overflow during route
calculation now; this could happen in theory once we wumbo.
It fixes a thinko when we print out routehints, too: we want to print
them out literally, not print out the effect they have on fees (which
is in the route, which we also print).
This ABI change doesn't need a CHANGELOG, since paystatus is new since
release.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current param_sat accepts "any": rename and move that to invoice.c
where it's called. We rename it to param_msat_or_any and invoice.c
is our first (trivial) amount_msat user.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to still accept it when parsing the database, but this flag
should allow upgrade testing for devs building on top
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Up until now, riskfactor was useless due to implementation bugs, and
also the default setting is wrong (too low to have an effect on
reasonable payment scenarios).
Let's simplify the definition (by assuming that P(failure) of a node
is 1), to make it a simple percentage. I examined the current network
fees to see what would work, and under this definition, a default of
10 seems reasonable (equivalent to 1000 under the old definition).
It is *this* change which finally fixes our test case! The riskfactor
is now 40msat (1500000 * 14 * 10 / 5259600 = 39.9), comparable with
worst-case fuzz is 50msat (1001 * 0.05 = 50).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the direct cause of the failure of the original
test_pay_direct test and it makes sense: invoice routehints may not be
necessary, so try without them *first* rather than last.
We didn't mention the use of routehints in CHANGELOG at all yet, so
do that now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian points out that we can iterate by ->size rather than calling
json_next() to find the end (which traverses the entire object!).
Now ->size is reliable (since previous patch), this is OK.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Avoid the unnecessary extra var, and don't use "capacity" since
that usually refers to static capacity.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
So add a new 'strategy' field. This makes it clearer what is going
on, currently one of:
* "Initial attempt"
* "Excluded channel <scid>"
* "Removed route hint"
* "Excluded expensive channel <scid>"
* "Excluded delaying channel <scid>"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We sanitize the routes: firstly, we assume appending so eliminate the
first hop if the route points straight to us. Secondly, eliminate empty
hints. Thirdly, trim overlong hints.
Then we just use the first route hint.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use the 'exclude' option to getroute for successive attempts. This
is more robust than having gossipd disable for some limited time.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I wrote this sync first, then rewrote async, then developed libplugin.
But committing all that just wastes reviewer time, so I present it as
if it was always asnc and using the library helper.
Currently the command it registers is 'pay2', but when it's complete
we'll remove the internal 'pay' and rename it. This does a single
'getroute/sendpay' call. No retries, no options.
Shockingly, this by itself is almost sufficient to pass our current test
suite with `pay`->`pay2`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
