I started replacing all get_node() calls, but got bored, so then just did the
tests which call get_node() 3 times or more.
Ends up not making a measurable speed difference, but it does make some
things neater and more standard.
Times with SLOW_MACHINE=1 (given that's how Travis tests):
Time before (non-valgrind):
393 sec (had 3 failures?)
Time after (non-valgrind):
410 sec
Time before (valgrind):
890 seconds (had 2 failures)
Time after (valgrind):
892 sec
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I thought this was timing out because I made it slow with the
change to txprepare as a plugin. In fact, it was timing out
because sometimes gossip comes so fast it gets suppressed
and we never get the log messags.
Still, before this it took 98 seconds under valgrind and
24 under non-valgrind, so it's an improvement to time as
well as robustness.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
Changelog-Added: New option `--important-plugin` loads a plugin is so important that if it dies, `lightningd` will exit rather than continue. You can still `--disable-plugin` it, however, which trumps `--important-plugin` and it will not be started at all.
the way we use PSBTs to sign things requires that we have the
scriptpubkey available on the utxo so we can populate the witness-utxo
field with it.
this causes problems if we don't already have the scriptpubkey cached in
the database, as in *some* cases we require a round trip to the HSM to
populate them
to get over this hump, we backfill any and all missing scriptpubkey
information for the utxo's that we hold in our wallet.
this will allow us to clean up the NULL handling of missing
scriptpubkeys.
```
# Excludes channel, then ignores routehint which includes that, then
# it excludes other channel.
> assert len(status) == 2
E assert 1 == 2
E -1
E +2
```
The invoice we use at the end has a routehint: 50% of the time it's
to l2 (which fails), 50% to l5 (which succeeds).
Change it to create invoice before channel with l5 so it does the
retry like we expect here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Since we started using `sendonion` in the `pay` plugin we no longer
automatically have the `amount` annotation on (partial) payments. This
replicates the issue so we can fix it.
Reported-by: Rusty Russell <@rustyrussell>
The adaptive MPP test was showing an issue with always using a routehint, even
when it wasn't necessary: we would insist on routhing to the entrypoint of the
routehint, even through the actual destination. If a channel on that loop
would result being over capacity we'd slam below 0, and then increase again by
unapplying the route. The solution really is not to insist on routing through
a routehint, so we implement random skipping of routehints, and we rotate them
if we have multiples.
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.
This PR includes the fix discussed on PR #3855. This fix was tested with the use case described inside the issue and worked.
Fixes: #3855
Changelog-None
The test had part 1 and 2 backward, but still worked. When I copied that to
*after* the test had succeeded, it complained. It should always complain,
to catch bugs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This wasn't important before, but now we have MPP it's good to enforce.
Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
While we were unsetting the `payment->cmd` in case of a success to signal that
we should not return to the JSON-RPC command twice, we were not doing that in
the case of failures. This was causing multiple responses to a single incoming
command, and `lightningd` was correctly killing the plugin. This issue was
introduced through early returns (anything setting `payment->abort=true`) and
was caused in Rusty's case through an MPP timeout.
Fixes#3847
Reported-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <@cdecker>
fundpsbt forces the caller to manually add their weight * feerate
to the satoshis they ask for. That means no named feerates.
Instead, create a startweight parameter and do the calc for them
internally, and return the feerate we used (and, while we're at it,
the estimated final weight).
This API change is best done now, as it would otherwise have to
be appended as a parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This exercises something that is simply not possible without MPP, i.e., the
bundling of multiple paths to get sufficient capacity to perform the payment.
Changelog-Added: The MPP presplit modifier splits large payments into 10k satoshi parts to maximize chances of performing the payment and to obfuscate the overall amount being sent.
We're actually going to deprecate this, so don't add new features!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: ***REMOVE*** JSON-API: `txprepare` returns a psbt version of the created transaction
It uses reservations heavily, and assumed we generated change, etc.
It's now a simpler test, in many ways.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
reserveinputs marks UTXOs reserved for 12 hours, so we won't select them
for spending: unreserveinputs marks them available again.
Exposes param_psbt() for wider use.
Disabled the test_sign_and_send_psbt since we're altering the API;
the final patch restores it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Technically an API break, but nobody relies on these I hope!
Note that the feerates warning was buried inside the style object:
it should be top-level.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As suggested during the paymod-03 review it is better to activate the new code
right away, and give users an escape hatch to use the legacy code instead. The
way I implemented it allows using either `legacypay` or `pay` and then set
`legacy` to switch to the other implementation.
Changelog-Added: JSON-RPC: The `pay` command now uses the new payment flow, the new `legacypay` command can be used to issue payment with the legacy code if required.
Suggested-by: Rusty Russell <@rustyrussell>
Suggested-by: ZmnSCPxj <@zmnscpxj>
This makes use of the payment modifier structure to just add the preimage to
the TLV payload for the last hop.
Changelog-Added: JSON-RPC: The `keysend` command allows sending to a node without requiring an invoice first.
This commit collects the changes required to the tests caused by the changes
to the `pay` and `paystatus` commands. They are also rather good hints as to
what these changes entail.
Rusty Russell
ZmnSCPxj jxPCSnmZ
Antoine Poinsot
niftynei
Christian Decker
Vincent