We were using `tal_fmt` to truncate off the last byte of the
response (newline), which required an allocation, a call to `vsnprintf` and a
copy of the block contents. This being >2MB in size on mainnet was rather
expensive.
We now just signal the end of the string by overwriting the trailing newline,
and stealing directly onto the result.
We create ALL_PROGRAMS, ALL_TEST_PROGRAMS, ALL_C_SOURCES and
ALL_C_HEADERS. Then the toplevel Makefile knows which are
autogenerated (by wildcard), so it can have all the rules to clean
them or check the source as necessary.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Using `waitblockheight 0` is a very slightly faster query than `getinfo`.
Also, avoid querying blockheight for child payments (allow `waitblockheight`
paymod to provide the blockheight returned from the `waitblockheight`, and
just resample the starting blockheight from the parent).
Changelog-None: pointless micro-optimization
This was checked with `gcc -S -O2` to see how an optimized build
would compile the function.
The original code completed calls into each child (and the `.s`
file showed that GCC 9.x was not smart enough to do early-out).
This modification explicitly does early-out, and avoids call-return
stack overhead for the common case where a payment is an ancestor
of a long line of single-child payments due to retrying.
Changelog-None: pointless micro-optimization
This is the simplest possible fix: increase the target amount until we get
the desired number of parts, while still bucketizing payments together that
are in approximately the same size.
The current logic puts all payments that are in the range x < amount <= 16*x
in the same bucket, making them harder to distinguish.
Changelog-Fixed: pay: The `presplit` modifier now supports large payments without exhausting the available HTLCs.
We have sanity checks in there that it's a valid point. Simply store
the JSON token like we do with others.
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null
Before:
real 0m2.054s
user 0m0.114s
sys 0m0.024s
After:
real 0m1.781s
user 0m0.127s
sys 0m0.013s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null
Before:
real 0m12.447s
user 0m0.143s
sys 0m0.008s
After:
real 0m2.054s
user 0m0.114s
sys 0m0.024s
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).
It's currently always 0, but it won't be once we replace txprepare.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `fundchannel` has new `outnum` field indicating which output of the transaction funds the channel.
Reported-by: ZmnSCPxj
Signed-off-by: Christian Decker <@cdecker>
Changelog-Fixed: pay: Correct a case where we put the sub-payment value instead of the *total* value in the `total_msat` field of a multi-part payment.
Only advance through routehints if no route was found at all, or if the
estimated capacity at the routehint is lower than the amount that we
have to send through the routehint.
Changelog-Fixed: pay: Be less aggressive with forgetting routehints.
listpays: make doc-all missed
Changelog-Added: JSON-RPC: `listpays` can be used to query payments using the `payment_hash`
Changelog-Added: JSON-RPC: `listpays` now includes the `payment_hash`
Changelog-None: cleanup only.
Before this change:
```
$ ls -l plugins/bcli
-rwxrwxr-x 1 zmnscpxj zmnscpxj 1914976 Aug 5 21:54 plugins/bcli
```
After this change:
```
$ ls -l plugins/bcli
-rwxrwxr-x 1 zmnscpxj zmnscpxj 1830552 Aug 5 22:00 plugins/bcli
```
We already duplicate a lot of code between `lightningd` and eeach
builtin plugin because we do not use .so for `common/`, but including
an object file with code that is not referenced in the executable is
fairly low-hanging size optimization fruit.
Changelog-Fixed: pay: Fixed a bug where routehints would be ignored if the payment exceeded 10,000 satoshi. This is particularly bad if the payee is only reachable via routehints in an invoice.
It's not all that rare to do these operations, and requiring annotations
for it is a little painful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Arguably a low-priority bug since no current node ever generates routehints longer
than one hop.
However, it is possible as an edge case, if the destination is directly accessible
*and* supports multiple channels, that we route through the destination, one of the
*other* channels it has not in the routehint, to the entry point, and then through
the routehint.
This change removes the risk of the above edge case.
Changelog-None: arguably a low-priority bug.
We've had problems with blocksonly in the past and bitcoind still allows
to use outdated (thus potentially dangerous estimates) while running
bitcoin with -blocksonly.
ZmnSCPxj mentionned that we still don't document this, but i figured
that in this specific case an explicit check and error seems preferable.
Changelog-Added: We now explicitly check at startup that our default Bitcoin backend (bitcoind) does relay transactions.
Proposed-by: ZmnSCPxj <zmnscpxj@protonmail.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This allows us to kill two birds with one stone: once connected, we use
the output of the successful call to do some sanity checks (only
checking the version for now, but more are yet to come!).
Changelog-Added: We now explicitly check at startup the version of our default Bitcoin backend (bitcoind).
Co-Authored-by: ZmnSCPxj <zmnscpxj@protonmail.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
We'd previously take the failed attempt and estimate the failing channel's
capacity at 3/4 of the attempted amount, which is rather aggressive. This
reduces this aggressiveness to use the exact amount tried, but excluding on
equality. This still skips attempting the same route with the same amount, but
also permits attempts that are in the range [3/4, 1] of the failed attempt
amount to still be attempted.
We sum up the amounts just like we do with `amount_sent`, however we may have
parts that don't have that annotation, so make it optional.
Suggested-by: Rusty Russell <@rustyrussell>
While not directly necessary, it still feeds the `listpays` result, and so we
should pass it along if we can, so we don't have to rely solely on the
`amount_sent` field, which includes the fees.
Reported-by: Rusty Russell <@rustyrussell>
The shortcut in the retry_mod that we can skip retrying if getroute fails or
we have no result is only valid if the parameters don't change. As we iterate
through the routehints the parameters change, and so we must signal to the
retry_mod that it can retry even in those cases.
The child payments will sometimes depend on the step of the parent, and making
sure that the parent state is correct before we create the children is
therefore important.
This uses @cdecker's idea of excluding the routehinted channel from the route,
and also consumes the route hints as it goes so that it makes progress.
I don't know if this is correct, but it reliably passes tests/test_pay.py::test_tlv_or_legacy
now.
Christian Decker
Rusty Russell
ZmnSCPxj jxPCSnmZ
Vincenzo Palazzo
Vincent
Antoine Poinsot