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.
1. We don't keep a pointer to payments (unlike pay, which keeps a
linked list), so mark it notleak.
2. plugin->our_features is overloaded for "features we want to set" (used by keysend)
and then "features we have". Create a new field, which is cleaner.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And turn "" includes into full-path (which makes it easier to put
config.h first, and finds some cases check-includes.sh missed
previously).
config.h sets _GNU_SOURCE which really needs to be done before any
'#includes': we mainly got away with it with glibc, but other platforms
like Alpine may have stricter requirements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We always allocate a new `struct command` when we get a full JSON
object from stdin:
b2df01dc73/plugins/libplugin.c (L1229-L1233)
If it happens to be a notification, we pass the `struct command` to
the handler, and not free it ourselves:
b2df01dc73/plugins/libplugin.c (L1270-L1275)
There are only nine points in `plugins/libplugin.c` where we `tal_free`
anything, and only one of them frees a `struct command`:
b2df01dc73/plugins/libplugin.c (L224-L234)
The above function `command_complete` is not appropriate for
notification handlers; the above function sends out a response
to our stdout, which a notification handler should not do.
However, as-is, it does mean that notification handling leaks
`struct command` objects, which can be problematic if we ever
have future built-in plugins which are significantly more
dependent on notifications.
This commit changes notification handlers to return
`struct command_result *`, because possibly in the future
notification handlers may want to perform `send_outreq`, so we
might as well use our standard convention for callbacks, and
to encourage future developers to check how to properly
terminate notification handlers (and free up the
`struct command`).
We also now provide a `notification_handled` function which a
notification handler must eventually call, as well as a
`notification_handler_pending` which is just a snowclone of
`command_still_pending`.
Before:
Ten builds, laptop -j5, no ccache:
```
real 0m36.686000-38.956000(38.608+/-0.65)s
user 2m32.864000-42.253000(40.7545+/-2.7)s
sys 0m16.618000-18.316000(17.8531+/-0.48)s
```
Ten builds, laptop -j5, ccache (warm):
```
real 0m8.212000-8.577000(8.39989+/-0.13)s
user 0m12.731000-13.212000(12.9751+/-0.17)s
sys 0m3.697000-3.902000(3.83722+/-0.064)s
```
After:
Ten builds, laptop -j5, no ccache: 8% faster
```
real 0m33.802000-35.773000(35.468+/-0.54)s
user 2m19.073000-27.754000(26.2542+/-2.3)s
sys 0m15.784000-17.173000(16.7165+/-0.37)s
```
Ten builds, laptop -j5, ccache (warm): 1% faster
```
real 0m8.200000-8.485000(8.30138+/-0.097)s
user 0m12.485000-13.100000(12.7344+/-0.19)s
sys 0m3.702000-3.889000(3.78787+/-0.056)s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: C plugins would could leak memory on every command (esp. seen when hammering topology's listchannels).
We weren't actually getting the last log out; this does that.
We have to fix test_bitcoin_failure which now notices the BROKEN
log message.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: libplugin: Fatal error messages from plugin_exit() now logged in lightningd.
As mentioned in previous commits: "result" must be an object,
and anything else is an antipattern.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes for more useful errors. It prints where it was up to in
the guide, but doesn't print the entire JSON it's scanning.
Suggested-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
memmem is also O(n^2), though it's faster. Now we have infrastructure,
let's do incremental parsing.
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null
Before:
real 0m13.674s
user 0m0.131s
sys 0m0.024s
After:
real 0m12.447s
user 0m0.143s
sys 0m0.008s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This doesn't make any difference, since lightningd generally sends us
short commands (command responses are via the rpc loop, which is
already done), but it's harmless.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The jsmn parser is a beautiful piece of code. In particular, you can parse
part of a string, then continue where you left off.
We don't take advantage of this, however, meaning for large JSON objects
we parse them multiple times before finally having enough to complete.
Expose the parser state and tokens through the API, so the caller can pass
them in repeatedly. For the moment, every caller is allocates each time
(except the unit tests).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to change the API on the more complete JSON parser, so
make and use a simple API for the easy cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tested on a test node which had made 50,000 payment, with no optimization.
For comparison, time for 'listsendpays' was 0.983s.
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null
Before:
real 0m52.415s
user 0m0.127s
sys 0m0.044s
After:
real 0m42.741s
user 0m0.149s
sys 0m0.016s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: libplugin: significant speedups for reading large JSON replies (e.g. calling listsendpays on large nodes, or listchannels / listnodes).
Changelog-Fixed: plugin: `bcli` no longer logs a harmless warning about being unable to connect to the JSON-RPC interface.
Changelog-Added: plugin: Plugins can opt out of having an RPC connection automatically initialized on startup.
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.
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)
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>
