I previously mistyped the rather lengthy conditions for failures, so
let's dissect it into its smaller components and add rationale behind
the individual parts of the decision.
This adds a new state `PAYMENT_STEP_RETRY_GETROUTE` which is used to
retry just that one step, without spawning a completely new
attempt. It's a new state so that modifiers do not act on it twice.
Changelog-Fixed: pay: Improved the performance of the `pay` command considerably by avoiding conflicting changes to our local network view.
We were delaying the channel_hint update till after the `createonion`
call which gave us the same situation we had with concurrent
`getroute` calls. Now we update the hints as soon as the plugins have
had their say in the route construction. If we still fail, either
because a modifier changed the route causing the failure, or because
we interleaved the route computation for multiple parts, we reset the
attempt and retry inline (i.e., without creating a new sub-payment).
Notice that interleaved route computations now only happen if the
modifier makes an async call to some RPC or similar.
1. One place returned false instead of -1.
2. The names implied it returned a bool, and it doesn't.
Fix both, and curse C's loose typing a little.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that check-whitespace and check-bolt already do this, so we
can eliminate redundant lines in common/Makefile and bitcoin/Makefile.
We also include the plugin headers in ALL_C_HEADERS so they get
checked.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Technically there *are* two feerates that we need to know:
- the feerate to use for the funding transaction, and
- the feerate to tell our peer to use for our commitment txs/htlc txs
As written, `multifundchannel` uses the same feerate for both. This
optional parameter will allow us to differentiate between the two, which
will be exceedingly handy for anchor output worlds. ;)
FIXME: test this
Changelog-Added: JSON API: `multifundchannel` has a new optional argument, 'commitment_feerate', which can be used to differentiate between the funding feerate and the channel's initial commitment feerate
This is a fairly direct translation. Even so, it should be faster in
most cases, and and we can do more sophisticated things if we want.
This also handles disabled channels better.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: plugins: `pay` will now try disabled channels as a last resort.
Finally, extends the 'close_to' functionality up to the flagship 'open a
channel' command.
Changelog-Added: JSON-API `fundchannel` now accepts an optional 'close_to' param, a bitcoin address that the channel funding should be sent to on close. Requires `opt_upfront_shutdownscript`
We're rarely in a hurry here, and bitcoind is aggressive with fees.
You can always spend this output if you really have to, using CPFP.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: mutual closing feerate reduced to "slow" to avoid overpaying.
I got a corrupt file, which looked like multiple concurrent attempts
to build it. So instead, build it in one command, but also use
VERBOSE so we print correctly with V=1 (and --quiet).
Also move into plugins/ where it logically belongs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Deprecated, but this can happen:
```
==1578== Conditional jump or move depends on uninitialised value(s)
==1578== at 0x12B30E: amount_msat_add (amount.c:224)
==1578== by 0x11270B: add_amount_sent (pay.c:1734)
==1578== by 0x112D89: listsendpays_done (pay.c:1831)
==1578== by 0x114F4B: handle_rpc_reply (libplugin.c:555)
==1578== by 0x115704: rpc_read_response_one (libplugin.c:685)
==1578== by 0x115821: rpc_conn_read_response (libplugin.c:705)
==1578== by 0x148E40: next_plan (io.c:59)
==1578== by 0x1499BD: do_plan (io.c:407)
==1578== by 0x1499FB: io_ready (io.c:417)
==1578== by 0x14BBC1: io_loop (poll.c:445)
==1578== by 0x117A82: plugin_main (libplugin.c:1322)
==1578== by 0x113ABC: main (pay.c:2096)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #3926
(probably)
Changelog-Fixed: pay: Also limit the number of splits if the payee seems to have a low number of channels that can enter it, given the max-concurrent-htlcs limit.
The routehints paymod shares the storage of the array d->routehints and
p->invoice->routes, but once it operates, it possibly leaves it as a stale
pointer to memory it used to have.
Since other paymods may be interested in the invoice details, including
the routehints in the invoice, we should ensure the p->invoice->routes
remains valid whenever we try mutating that array.
As revealed by the failure of tests in #3936, where we ended up trying
to send a partial payment using legacy style, we are not handling
style properly.
1. BOLT9 has features, so we can *know* that the destination supports
MPP. We may not have seen a node_announcement.
2. We can't assume that nodes inside routehints support TLV.
3. We can't assume direct peers support TLV.
The keysend code tried to fix this up, so I'm not sure that this caused
the issue in #3968, though.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: `pay` will now make reliable multi-part payments to nodes it doesn't have a node_announcement for.
This will update the fee output if it exists, rather than unilaterally
adding a new one.
Also, if the fee output already exists, we should make sure that it
doesn't interfere with the outnums of the other outputs
Fixes: #2679
Changelog-Added: JSON-RPC: New `multiwithdraw` command to batch multiple onchain sends in a single transaction. Note it shuffles inputs and outputs, does not use BIP69.
Changelog-Added: We now have `multifundchannel` as a builtin plugin command to fund multiple channels to different peers all in a single onchain transaction.
Header from folded patch 'fixup-use-json_add_psbt.patch':
fixup!
Header from folded patch 'use-goto-no-ok-chain.patch':
fixup!
Header from folded patch 'destinations-at-parse-time.patch':
fixup!
Header from folded patch 'multifundchannel__use_jsmntoks_to_pass_through_json_string,_not_strings.patch':
multifundchannel: use jsmntoks to pass through json string, not strings
Passing in "" for utxos would crash lightningd on the command-line
otherwise. Now returns an error.
Header from folded patch 'update_plugins-multifundchannel.c.patch':
Update plugins/multifundchannel.c
Co-authored-by: Darosior <darosior@protonmail.com>
Changelog-Deprecated: plugin: `bcli` replacements should note that `sendrawtransaction` now has a second required Boolean argument, `allowhighfees`, which if `true`, means ignore any fee limits and just broadcast the transaction. Use `--deprecated-apis` to use older `bcli` replacement plugins that only support a single argument.
This removes the reservation cleanup at startup, too, now they're all
using 'reserved_til'.
This changes test_withdraw, since it asserted that outputs were marked
spent as soon as we broadcast a transaction: now they're reserved until
it's mined. Similarly, test_addfunds_from_block assumed we'd see funds
as soon as we broadcast the tx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `withdraw` now randomizes input and output order, not BIP69.
This is a little lazy, but simpler than extracting the common parts
or making withdraw a plugin which calls txprepare (which should be
deprecated soon in favor of fundpsbt etc).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Some minor phrasing differences cause test changes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: txprepare reservations stay across restarts: use fundpsbt/reservepsbt/unreservepsbt
Changelog-Removed: txprepare `destination` `satoshi` argument form removed (deprecated v0.7.3)
This uses `fundpsbt` and similar to simulate the txprepare command.
It has one difference (when complete), in that it those reservations
are now timed and don't get reset on restart.
It also doesn't have the restriction that `all` can only be used with
no other output, as I didn't realize that when I implemented it!
Note that change is now inserted in a random position, not sorted
into BIP69 order.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids overwriting the ones in git, and generally makes things neater.
We have convenience headers wire/peer_wire.h and wire/onion_wire.h to
avoid most #ifdefs: simply include those.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
