Commit Graph

161 Commits

Author SHA1 Message Date
ZmnSCPxj jxPCSnmZ 919d371fe8 common/json_stream.c: Implement a `json_add_jsonstr` to add already-JSON strings to `json_stream` objects.
Also incidentally fixes #3613

ChangeLog-none
2020-04-09 14:09:23 +09:30
lisa neigut b51772ff68 pay: fix #3613; string output must be quoted
`json_add_member` requires quotes for string types

Changelog-Fixed: `pay` would crash on expired waits with tried routes
2020-04-08 15:31:52 +02:00
lisa neigut 3124d43d9f pay: check for null route in maybe_exclude
Fixes (?) #3607

Changelog-Fixed: Fix crash in pay when attempting to find cheaper route with exemptfee
2020-04-08 15:31:52 +02:00
Rusty Russell 72327f5cc2 libplugin: fix compilation issue on recent gccs.
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)
2020-04-07 20:49:58 -05:00
darosior b8262c3de2 fundechannel: don't pass deprecated 'satoshis' to fundchannel_start
Changelog-None
2020-04-06 11:54:37 +02:00
Christian Decker 891adfca1e pay: Speed up `pay` and fix the shadow route construction
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
2020-04-04 16:24:57 +10:30
Rusty Russell 2f1502abf4 cleanup: make 'u8 *features' and 'struct feature_set *fset' more explicit.
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>
2020-04-03 13:13:21 +10:30
Rusty Russell 28e3ffc66b plugins/fundchannel: make 'all' do the right thing for wumbo.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell ca512e3cb2 plugins/pay: use feature set from lightningd to decode bolt11.
This means we correctly reject invoices with features we don't understand.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell 8cb364dd28 libplugin: demarshal and stash the feature set given by lightningd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell cf43e44378 common/features: don't use internal global.
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>
2020-04-03 13:13:21 +10:30
darosior 7c0af81c21 bcli: use a more urgent feerate for HTLCs and penalty transactions
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
2020-04-01 23:02:47 -05:00
Rusty Russell e940d953bd fundchannel_start: We can't deprecate 'satoshi' here, since 'amount' wasn't sent
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`).
2020-03-30 12:47:01 +02:00
Rusty Russell 75838341a7 fundchannel/fundchannel_start: remove deprecated `satoshi` parameter
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).
2020-03-30 12:47:01 +02:00
darosior 610aa0b8e3 bcli: register --dev-max-fee-multiplier on our side
That way we pass the real min_acceptable (SLOW/2) and max_acceptable
(URGENT * 10) feerates to lightningd.
2020-03-30 20:17:18 +10:30
darosior 902edf56e6 libplugin: allow u32 options 2020-03-30 20:17:18 +10:30
darosior 5e72b22e80 bcli: adapt interface to the new fees estimation interface
We keep the same behaviour as lightningd before.
2020-03-30 20:17:18 +10:30
darosior 8e055a4506 bcli: remove a superfluous variable 2020-03-05 15:06:38 -06:00
Michael Schmoock d3ece69a1b fix: adds bcli plugin to check-source targets
Changelog-None
2020-03-04 14:04:51 +10:30
Rusty Russell 2aad3ffcf8 common: tal_dup_talarr() helper.
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>
2020-02-27 14:16:16 +10:30
Vasil Dimov 89ceb273f5 wire: remove towire_double()
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
2020-02-27 09:07:04 +10:30
Vasil Dimov a754e4c6aa gitignore: extend with recently added build products
Changelog-None
2020-02-21 09:44:41 +01:00
darosior e7b0c24db2 libplugin: use a typesafe_cb for plugin_timer 2020-02-18 10:21:48 +10:30
Rusty Russell 5e2053feea pay: fix crash when we paystatus too soon.
If the attempt hasn't started yet, we can have an empty 'attempts' array
(while it's in listpeers, for example):

```
pay: plugins/pay.c:1457: json_paystatus: Assertion `tal_count(ps->attempts)' failed.
pay: FATAL SIGNAL 6 (version v0.8.1rc1-1-g946af3f)
0x55e895110543 send_backtrace
	common/daemon.c:39
0x55e8951105e9 crashdump
	common/daemon.c:52
0x7f92b0928f1f ???
	???:0
0x7f92b0928e97 ???
	???:0
0x7f92b092a800 ???
	???:0
0x7f92b091a399 ???
	???:0
0x7f92b091a411 ???
	???:0
0x55e895100012 json_paystatus
	plugins/pay.c:1457
0x55e895103aaa ld_command_handle
	plugins/libplugin.c:968
0x55e895103bd4 ld_read_json_one
	plugins/libplugin.c:1011
0x55e895103cd2 ld_read_json
	plugins/libplugin.c:1030
0x55e895124cbd next_plan
	ccan/ccan/io/io.c:59
0x55e89512583a do_plan
	ccan/ccan/io/io.c:407
0x55e895125878 io_ready
	ccan/ccan/io/io.c:417
0x55e895127a3e io_loop
	ccan/ccan/io/poll.c:445
0x55e895104593 plugin_main
	plugins/libplugin.c:1194
0x55e895100cbc main
	plugins/pay.c:1697
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-02-13 14:20:50 +10:30
Rusty Russell 7a178ebb7a plugins: libplugin don't insist on 'jsonrpc' field from lightningd.
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>
2020-02-13 12:34:31 +10:30
darosior 5840e90ceb plugins/libplugin: don't crash if 'lightning-rpc' doesnt exist (yet)
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.
2020-02-12 11:45:07 +10:30
darosior c1009635ed plugins/bcli: register Bitcoin-related options 2020-02-12 11:45:07 +10:30
darosior f81cc9f552 plugins/bcli: wait for bitcoind to be warmed up at 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).
2020-02-12 11:45:07 +10:30
darosior 70a79e3998 plugins/bcli: a new plugin for gathering Bitcoin data
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`.
2020-02-12 11:45:07 +10:30
darosior 3eb0f56f87 libplugin: generalize the plugin_timer callback type
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.
2020-02-12 11:45:07 +10:30
darosior b0b55d36ef libplugin: add a 'still_pending' helper 2020-02-12 11:45:07 +10:30
darosior ceeb5503cc libplugin: fix 'dynamic' field in getmanifest
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
2020-02-10 09:49:15 +10:30
darosior b91433cb42 libplugin: use json_stream for all plugins' commands 2020-02-10 09:49:15 +10:30
darosior 2bff80e3de libplugin: use json_stream helpers for RPC calls
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.
2020-02-10 09:49:15 +10:30
darosior 0546728819 libplugin: use json_stream helpers for handle_getmanifest 2020-02-10 09:49:15 +10:30
darosior 45e9f53c6b libplugin: expose helpers to start and end a jsonrpc response
The usual json_stream starters and command_result enders.
2020-02-10 09:49:15 +10:30
darosior 4772025b5d libplugin don't expose the plugin struct 2020-02-05 17:05:56 +10:30
darosior b6b2e6727e libplugin: make deprecated_apis a global 2020-02-05 17:05:56 +10:30
darosior 02fe34ed95 libplugin: don't expose the rpc_conn struct 2020-02-04 13:24:32 +10:30
darosior b31e3b1541 libplugin: pass a pointer to plugin to send_outreq
autoclean needs to send outreqs from a timer cb, hence with cmd == NULL.
2020-02-04 13:24:32 +10:30
darosior c765a223f1 libplugin: use ccan/io for async Rpc requests 2020-02-04 13:24:32 +10:30
darosior 765626875e libplugin: use ccan/io for notifications 2020-02-04 13:24:32 +10:30
darosior 9ebfdf0b8c libplugin: add remaining globals to the global state
But not the outreqs helpers, which will be moved when passing
send_outreq_ to using ccan/io.
2020-02-04 13:24:32 +10:30
darosior 499dce1c38 libplugin: include the rpc conn into the global state
And rename the struct name, as it's now only used for RPC.
2020-02-04 13:24:32 +10:30
darosior 98e8aac75f libplugin: use ccan/io for response to plugin commands
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.
2020-02-04 13:24:32 +10:30
darosior 42cd23092c libplugin: use ccan/io for lightningd connections
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.
2020-02-04 13:24:32 +10:30
darosior ddf14613d1 libplugin: introduce a top-level object
Having a global object is very handy to pass to `io_plan`s, which would
have otherwise required a whole bunch of new globals.
2020-02-04 13:24:32 +10:30
darosior 3510c29e5d common: move json_stream helpers to common/json
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).
2020-02-04 13:24:32 +10:30
Wladimir J. van der Laan e4c6fd89b7 Add missing `extern` qualifiers for gcc 10
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.
2020-02-02 12:59:17 +10:30
Vasil Dimov 55173a56b7 Use dedicated type for error codes
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
2020-01-31 06:02:47 +00:00