This is the simplest solution, not the best, but there's significant risk in try to remove the "we have a path" assumption in the code pay code.
Includes removing a `tal_steal` which was incorrect: the buffer has the same lifetime as the plugin, so if we steal it then things get messy when we free the struct payment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `pay` will now pay your own invoices if you try.
Previously, the "payment" and "invoice" paths were completely separate, but this now calls both. It bypasses htlc_sets (and thus, cannot do MPP), and bypasses the hook too: the former is tied closely to HTLCs, and the hook is also very htlc-centric.
Includes finishing unfinished sentence in sendpay man page, as a bonus.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `sendpay` now allows self-payment of invoices, by specifying an empty route.
Clean these up: they were debug logs, but we want to pass this information
back for self-payments.
Also fixes "Attept" typo which altered tests!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For non-v0 witness programs we weren't stripping the data push byte
before writing into the fallback address.
According to BIP14, all witness scripts will be data pushes (up to 40-bytes)
so trimming the datapush byte should be kosher.
From BIP141:
A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that
consists of a 1-byte push opcode (for 0 to 16) followed by a
data push between 2 and 40 bytes gets a new special meaning.
The value of the first push is called the "version byte". The
following byte vector pushed is called the "witness program".
Changelog-Fixed: Adding a >0 version witness program to a fallback address now is *just* the witness program, as per bolt11 spec
Pointed out by @ShahanaFarooqui, we leave a single unused entry in the datastore,
so we should clean that up too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If they have invalid runes, we bail, but if they have runes which used
a different master secret (old commando.py allowed you to override
secret), we just complain and delete them.
Note that this requires more mocks in wallet/test/run-db.c...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The wallet_datastore_first() SELECT statement only iterates from the
given key (if any), relying on the caller to notice when the key no
longer applies. (e.g. startkey = ["foo", "bar"] will return key
["foo", "bar"] then ["foo", "bar", "child" ], then ["foo", "baz"]).
The only caller (listdatastore) would notice the keychange and stop
looping, but reallly wallet_datastore_next() should do this. When I
tried to use it for migrations, I got very confused!
Also, several places want a simple "wallet_datastore_get()" function,
so provide that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
During migrations, wallet doesn't exist yet, so we use raw db. Split
functions into lower-level ones and make public API a simple wrapper.
Unfortunately, this means db_datastore_next needs to proceed db_datastore_first
since they're now static (and first calls next), plus, fix some weird indents,
so diff is bigger than expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to access this in db migrations, which happen very early, but
runes_init needs the db, creating a circular dependency which must be
split.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means (temporarily) that blacklisting won't work (fix later), and
means that old-style (commando.py) master-secret-override doesn't work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Plugins: `commando` no longer allows datastore ['commando', 'secret'] to override master secret (re-issue runes if you were using that!).
In preparation for going async:
1. Split try_command's tail into a new function called execute_command() after
the rune checks have succeeded.
2. Put all the info execute_command() needs into struct cond_info, to make it
a simple callback style.
So we create new_cond_info() which dynamically allocates `struct cond_info`
and sets the destructor.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would create a `struct commando` to marshal our incoming messages,
then try_command would create a *new* one. We can simply reuse, but
when I did I noticed a trick: the new one was not in the `incomings`
array, so didn't work towards the ratelimit. So we need to remove it
from `incomings` in `try_command`, but at least it's now explicit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to activate on the first rune creation, but we're no longer in charge
of runes, so we can't make that call.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And do them on the first run (where we check parameters), instead
of every time. Might as well do them in non-developer mode too,
since they're simply programmer correctness.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The uid isn't enough: it could be someone else's rune. This is tested
in the command rune list tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make this a bit less difficult to see what's going on; use common
variables for the paths etc.
Also make the final `rm` step recursive, same as the above find step
which patches the files.
Files are in `contrib/pyln-grpc-proto/pyln/grpc`, but the upper level
find command acts at a level above.
At cold start, if your node is behind the blocktip and you've sent your
peer a blockheight counter from the future, we shouldn't confuse ourselves
with our rollback/replay.
Should fix flakes in CI that were spotting BROKEN blockheight updates.
Logs below from a previuos CI fail (edited for relative clarity)
The one that sasy "{ SENT_ADD_ACK_REVOCATION:111 }, our current 108` is
the tell; the last line is the node finally catching up to the tip.
In the test we get into this state by stopping and restarting the node.
```
2023-07-22T11:24:28.2754533Z lightningd-1 2023-07-22T11:19:34.188Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#2: Already have funding locked in
2023-07-22T11:24:28.2755486Z lightningd-1 2023-07-22T11:19:34.188Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#2: attempting update blockheight a5b23dff5177badd6df725c
efeb83ceccbfc52dc64a16b38894a41f0ad8fa181
2023-07-22T11:24:28.2755778Z lightningd-1 2023-07-22T11:19:34.188Z DEBUG lightningd: update_blockheight: height = 108
2023-07-22T11:24:28.2766210Z lightningd-1 2023-07-22T11:19:34.210Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: init LOCAL: remote_per_commit = 029563e7c898
5d8b95bdfe19e47e494bb8ec8d53ff4edb93f156be57667bfee8c9, old_remote_per_commit = 02bf3117c149d324361f0b418db8984b1e29af70c773eb2865a41ff7f583c7c9ed next_idx_local = 3 next_idx_remote = 3 revocations_recei
ved = 2 feerates { SENT_ADD_ACK_REVOCATION:3750 } range 253-150000 blockheights { SENT_ADD_ACK_REVOCATION:111 }, our current 108
2023-07-22T11:24:28.2768866Z lightningd-1 2023-07-22T11:19:34.211Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: peer_out WIRE_CHANNEL_REESTABLISH
2023-07-22T11:24:28.2769416Z lightningd-1 2023-07-22T11:19:34.211Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: billboard: Sent reestablish, waiting for the
irs
2023-07-22T11:24:28.2771115Z lightningd-1 2023-07-22T11:19:34.212Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: peer_in WIRE_CHANNEL_REESTABLISH
2023-07-22T11:24:28.2774150Z lightningd-1 2023-07-22T11:19:34.212Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: Got reestablish commit=3 revoke=2
2023-07-22T11:24:28.2776056Z lightningd-1 2023-07-22T11:19:34.212Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: next_revocation_number = 2
2023-07-22T11:24:28.2805639Z lightningd-1 2023-07-22T11:19:34.239Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: current blockheight 108 less than last 111
2023-07-22T11:24:28.2823960Z lightningd-1 2023-07-22T11:19:34.240Z DEBUG lightningd: Adding block 109: 5f67b6e110eb3c3457bea4fcf0d04ce9be90efeee5df8e083ed4266074ca911f
2023-07-22T11:24:28.2833154Z lightningd-1 2023-07-22T11:19:34.251Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: current blockheight 108 less than last 111
2023-07-22T11:24:28.2833630Z lightningd-1 2023-07-22T11:19:34.252Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: Trying commit
2023-07-22T11:24:28.2834165Z lightningd-1 2023-07-22T11:19:34.252Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: current blockheight 108 less than last 111
2023-07-22T11:24:28.2835070Z lightningd-1 2023-07-22T11:19:34.252Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: Can't send commit: nothing to send, feechange not wanted ({ SENT_ADD_ACK_REVOCATION:3750 }) blockheight not wanted ({ SENT_ADD_ACK_REVOCATION:111 })
2023-07-22T11:24:28.2835516Z lightningd-1 2023-07-22T11:19:34.350Z DEBUG lightningd: Adding block 110: 5f43f3ac9d808e3a309720d1b0727a00d5a3d3ddca71d97401e233637e87639c
2023-07-22T11:24:28.2835962Z lightningd-1 2023-07-22T11:19:34.476Z DEBUG lightningd: Adding block 111: 55b0d1e0a08ff6233e186e6735cb1cbec33e2b0a6e7d08f2622e8c1db30b54b9
```
This looked like a test flake, but was real:
```
l1.daemon.wait_for_log("closing soon due to the funding outpoint being spent")
# We won't gossip the dead channel any more (but we still propagate node_announcement). But connectd is not explicitly synced, so wait for "a bit".
time.sleep(1)
> assert len(get_gossip(l1)) == 2
E assert 4 == 2
```
We can see that two channel_updates come in *after* we mark it dying:
```
gossipd: channel 103x1x0 closing soon due to the funding outpoint being spent
gossipd: REPLY WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY with 0 fds
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/0 now DISABLED
022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Received channel_update for channel 103x1x0/1 now DISABLED
```
We should keep marking channel_updates the same way.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We get intermittant reports of subd->conn being leaked, but I could never find it.
That's because it's actually subd which is not referenced any more: subd->conn
gets reported because it's subd's tal_parent (and, except for the reference in
subd, not referenced either).
The real issue is that the channel->owner is reassigned to the new subdaemon,
and the old one is still exiting. During that time, we can see a "leak".
```
- Node /tmp/ltests-hkr089bp/test_sql_1/lightning-3/ has memory leaks: [
{
"backtrace": [
"ccan/ccan/tal/tal.c:477 (tal_alloc_)",
"ccan/ccan/io/io.c:91 (io_new_conn_)",
"lightningd/subd.c:774 (new_subd)",
"lightningd/subd.c:828 (new_channel_subd_)",
"lightningd/dual_open_control.c:3662 (peer_restart_dualopend)",
"lightningd/peer_control.c:1161 (connect_activate_subd)",
"lightningd/peer_control.c:1273 (peer_connected_hook_final)",
"lightningd/plugin_hook.c:213 (plugin_hook_callback)",
"lightningd/plugin.c:591 (plugin_response_handle)",
"lightningd/plugin.c:702 (plugin_read_json_one)",
"lightningd/plugin.c:747 (plugin_read_json)",
"ccan/ccan/io/io.c:59 (next_plan)",
"ccan/ccan/io/io.c:407 (do_plan)",
"ccan/ccan/io/io.c:417 (io_ready)",
"ccan/ccan/io/poll.c:453 (io_loop)",
"lightningd/io_loop_with_timers.c:22 (io_loop_with_timers)",
"lightningd/lightningd.c:1249 (main)"
],
"label": "ccan/ccan/io/io.c:91:struct io_conn",
"parents": [
"lightningd/lightningd.c:107:struct lightningd"
],
"value": "0x556c63c859f8"
}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
channeld WARNING: Bad update_fail_malformed_htlc failure code 4103
```
Warren Togami reports this happening with Bitrefill on every reconnect,
so it's clearly something LND does.
(4103 is TEMPORARY_CHANNEL_FAILURE, which does not belong in update_fail_malformed_htlc).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: We allow update_fail_malformed_htlc with invalid error codes (LND?)
GH was complaining about Node v12 being used in v2.2.4. We used to
hold back from this upgrade because v3 was breaking quotas, but we
should try again.
The channel selection did not consider the amounts that we are trying
to transfer, which in a multiplexed channel world could end up always
selecting a channel that is too small for the payment. We also log
which channel was selected based on the selector that is passed in,
allowing us to better follow the decisions.
Changelog-Fixed: pay: `sendonion` and `sendpay` will now consider amounts involved when using picking one channel for a peer
Debugging a number of payments showed that we sometimes waste a number
of attempts routing through a channel via its alias, rather than its
scid. This is because while we annotate the scid when it has been set,
we do not do so for the alias. The alias then is picked for routing
despite not having enough capacity, failing the attempt locally.
It can also happen that we alternate between scid and alias, doubling
the number of failed attempts before we can make progress.
This patch sets the hint for the alias to a capacity of 0 and disables
it as if the peer were offline. This means when available we'll always
use the scid, which is also far easier to read in the logs.
Changelog-Fixed: pay: We now track spendable amounts when routing on both the local alias as well as the short channel ID
