We're going to use this on the "rpc_command" hook, to allow xpay to specify that it
only wants to be called on "pay" commands.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we add a new hook, not at the end, while hooks are getting called,
then iteration could be messed up (e.g. calling a plugin twice, or
skipping one).
The simplest thing is to defer updates until nobody is calling the
hook. In theory this could livelock, in practice it won't.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: we now re-transmit unseen funding transactions on startup, for more robustness.
Simplify wallet address generation by using a unified approach where
the derivation method (BIP86 vs BIP32) is determined by the wallet's
HSM secret type rather than having separate address types.
Add TLV field to hsmd_init_reply_v4 to communicate the HSM secret type
(mnemonic vs legacy) from HSM to lightningd. This allows lightningd to
automatically determine whether to use BIP86 or BIP32 derivation without
needing separate address types.
This commit fixes an issue where BIP86 addresses were not being
discovered during wallet recovery/rescan operations.
The root cause was that init_txfilter() only populated the transaction
filter with BIP32-derived keys, preventing lightningd from recognizing
BIP86 UTXOs during blockchain scans. Now both BIP32 and BIP86 derived
scripts are included in the filter when BIP86 derivation is enabled.
This ensures that wallets restored from BIP39 mnemonics can properly
discover and display previously funded BIP86 addresses without requiring
manual address generation first.
[ We also move the slightly-lost comment about libbacktrace so it is
where we actually include <backtrace.h> --RR ]
This happens if the channel is *not* announcable yet. Then we hit the assertion
in funding_depth_cb that the txid is the same as the current funding.txid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: fixed crash when we splice a channel which hasn't been announced yet.
Basically, `devtools/reduce-includes.sh */*.c`.
Build time from make clean (RUST=0) (includes building external libs):
Before:
real 0m38.944000-40.416000(40.1131+/-0.4)s
user 3m6.790000-17.159000(15.0571+/-2.8)s
sys 0m35.304000-37.336000(36.8942+/-0.57)s
After:
real 0m37.872000-39.974000(39.5466+/-0.59)s
user 3m1.211000-14.968000(12.4556+/-3.9)s
sys 0m35.008000-36.830000(36.4143+/-0.5)s
Build time after touch config.vars (RUST=0):
Before:
real 0m19.831000-21.862000(21.5528+/-0.58)s
user 2m15.361000-30.731000(28.4798+/-4.4)s
sys 0m21.056000-22.339000(22.0346+/-0.35)s
After:
real 0m18.384000-21.307000(20.8605+/-0.92)s
user 2m5.585000-26.843000(23.6017+/-6.7)s
sys 0m19.650000-22.003000(21.4943+/-0.69)s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we don't have to manually choose what to link against,
which is much of the complexity of our Makefiles: the compiler will
automatically use any object files it needs to link.
We already do this for ccan as libccan.a, now we have libcommon.a.
We don't link against it for *everything*, as some tests require their own
versions.
Notes:
1. I get rid of the weird plugins/test/Makefile2 (accidental commit?)
2. Many tests change due to update-mocks.
3. In some places I added the missing dependency on the Makefile itself, though most are in the next
patch.
Before:
Total program size: 221366528
Total tests size: 364243856
After:
Total program size: 190733656
Total tests size: 337880888
Build time from make clean (RUST=0) (includes building external libs):
Before:
real 0m38.227000-44.245000(41.8222+/-1.6)s
user 3m2.105000-33.696000(23.1442+/-8.4)s
sys 0m35.054000-42.269000(39.7231+/-2)s
After:
real 0m38.944000-40.416000(40.1131+/-0.4)s
user 3m6.790000-17.159000(15.0571+/-2.8)s
sys 0m35.304000-37.336000(36.8942+/-0.57)s
Build time after touch config.vars (RUST=0):
Before:
real 0m18.928000-22.776000(21.5084+/-1.1)s
user 2m8.613000-36.567000(27.7281+/-7.7)s
sys 0m20.458000-23.436000(22.3963+/-0.77)s
After:
real 0m19.831000-21.862000(21.5528+/-0.58)s
user 2m15.361000-30.731000(28.4798+/-4.4)s
sys 0m21.056000-22.339000(22.0346+/-0.35)s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rusty@rusty-Framework:~/devel/cvs/lightni
This was changing all the time when I tried to make
autogenerate-rpc-examples.py reproducible. Turns out it was being
corrupted (it does suspicious things with pointers); rather than try
to diagnose it, I simply rewrote the code to create it only when we
need it.
```
Valgrind error file: valgrind-errors.34506
==34506== Uninitialised byte(s) found during client check request
==34506== at 0x241732: memcheck_ (mem.h:247)
==34506== by 0x2417BC: towire (towire.c:17)
==34506== by 0x24185C: towire_u16 (towire.c:28)
==34506== by 0x20C8E4: towire_tlv_scb_tlvs_remote_to_self_delay (scb_wiregen.c:213)
==34506== by 0x240E78: towire_tlv (tlvstream.c:342)
==34506== by 0x20C99F: towire_tlv_scb_tlvs (scb_wiregen.c:234)
==34506== by 0x20C298: towire_modern_scb_chan (scb_wiregen.c:89)
==34506== by 0x1A6CF3: json_add_scb (peer_control.c:2488)
==34506== by 0x1A6E0C: json_staticbackup (peer_control.c:2519)
==34506== by 0x177E3F: command_exec (jsonrpc.c:799)
==34506== by 0x1785AE: rpc_command_hook_final (jsonrpc.c:945)
==34506== by 0x1BEC2D: plugin_hook_call_next (plugin_hook.c:199)
==34506== Address 0x1ffeffe736 is on thread 1's stack
==34506== in frame #2, created by towire_u16 (towire.c:26)
==34506==
{
<insert_a_suppression_name_here>
Memcheck:User
fun:memcheck_
fun:towire
fun:towire_u16
fun:towire_tlv_scb_tlvs_remote_to_self_delay
fun:towire_tlv
fun:towire_tlv_scb_tlvs
fun:towire_modern_scb_chan
fun:json_add_scb
fun:json_staticbackup
fun:command_exec
fun:rpc_command_hook_final
fun:plugin_hook_call_next
}
==34506== Uninitialised byte(s) found during client check request
==34506== at 0x241732: memcheck_ (mem.h:247)
==34506== by 0x2417BC: towire (towire.c:17)
==34506== by 0x240EF0: towire_tlv (tlvstream.c:354)
==34506== by 0x20C99F: towire_tlv_scb_tlvs (scb_wiregen.c:234)
==34506== by 0x20C298: towire_modern_scb_chan (scb_wiregen.c:89)
==34506== by 0x1A6CF3: json_add_scb (peer_control.c:2488)
==34506== by 0x1A6E0C: json_staticbackup (peer_control.c:2519)
==34506== by 0x177E3F: command_exec (jsonrpc.c:799)
==34506== by 0x1785AE: rpc_command_hook_final (jsonrpc.c:945)
==34506== by 0x1BEC2D: plugin_hook_call_next (plugin_hook.c:199)
==34506== by 0x1BEBA8: plugin_hook_callback (plugin_hook.c:186)
==34506== by 0x1B771E: plugin_response_handle (plugin.c:705)
==34506== Address 0x7bd1d08 is 40 bytes inside a block of size 42 alloc'd
==34506== at 0x484DCD3: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==34506== by 0x3C8614: tal_resize_ (tal.c:755)
==34506== by 0x2417A2: towire (towire.c:14)
==34506== by 0x24185C: towire_u16 (towire.c:28)
==34506== by 0x20C8E4: towire_tlv_scb_tlvs_remote_to_self_delay (scb_wiregen.c:213)
==34506== by 0x240E78: towire_tlv (tlvstream.c:342)
==34506== by 0x20C99F: towire_tlv_scb_tlvs (scb_wiregen.c:234)
==34506== by 0x20C298: towire_modern_scb_chan (scb_wiregen.c:89)
==34506== by 0x1A6CF3: json_add_scb (peer_control.c:2488)
==34506== by 0x1A6E0C: json_staticbackup (peer_control.c:2519)
==34506== by 0x177E3F: command_exec (jsonrpc.c:799)
==34506== by 0x1785AE: rpc_command_hook_final (jsonrpc.c:945)
==34506==
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For old channels, this can take a while, and it stops everything. But
we are only doing this to save space; it's not a *functional* necessity.
A quick and dirty test with 50,000 htlcs shows the htlc deletion took
450msec. I tried adding an index, and changing it to set hstate to
HTLC_STATE_INVALID instead of deleting entries, but it still took about 350ms.
Whereas the "COUNT(*)" only took 1.7msec, so it's worth keeping.
Reported-by: @michael1011
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: lightningd: we defer deletion of old htlcs on channel close, to avoid pausing for a long time (we clean them on startup)
Fixes: https://github.com/ElementsProject/lightning/issues/7962
One issue we have in CI is reconnection races: if an incoming
connection arrives while an outgoing one is negotiated, we close the
outgoing one and issue a disconnect, which fails any connect attempts.
By sending a "reconnected" message instead of disconnect/connect we
can avoid disturbing in-progress connection attempts which happens in CI
quite a bit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We allowed NULL for stub channels, but just don't put the stub scid
into the hash tables. This cleans up all the callers to make it
clear this is a non-optional parameter.
We opencode channel_set_random_local_alias, since there's only one caller now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They can all call get_block_height(); the extra argument confused me and
I thought they were called before the block height was actually updated.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This may be useful for their recovery, though they should see the spend onchain.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We now reply to `channel_reestablish` even on long-closed channels.
This was always false. peer_start_channeld was called in various places
with the argument "NULL" instead of "false", which unfortunately compilers
didn't complain about :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to start loading them into memory for nicer responses if
people try to reestablish closed channels, but we don't care about ones
which were never actually opened. We could add a new state, but easier
to simply remove them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. trace_span_start() is always called with a string literal, so
no copy needed (and we can use a macro to enforce this).
2. trace_span_tag() name and value are always longer-lived than
the span, so no need to copy these either.
Before:
real 0m18.524000-19.100000(18.7674+/-0.21)s
user 0m16.171000-16.833000(16.424+/-0.26)s
sys 0m2.259000-2.400000(2.337+/-0.059)s
After:
real 0m16.421000-18.407000(17.8128+/-0.72)s
user 0m14.242000-16.041000(15.5382+/-0.67)s
sys 0m2.179000-2.363000(2.273+/-0.061)s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This can happen with 24.11 and later. We scan back to exposed channel
opens, or that release.
The BROKEN log messages cause some tests to fail, so we fix those.
Fixes: https://github.com/ElementsProject/lightning/issues/8169
Changelog-Fixed: wallet: rescan for missing close outputs (can happen if peer doesn't support option_shutdown_anysegwit)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: plugins which didn't accept string JSON RPC fields (deprecated v23.08, disabled by default in v24.11).
This corner case started triggering on my machine with latest Bitcoind.
This test sabotages the closing negotiation, and as a result l1
doesn't see l2's CLOSING_SIGNED. l2 is happy, however, and it is in
CLOSINGD_COMPLETE. When l1 reconnects, it gets an error, and this causes
it to drop the unilateral tx to chain.
This unilateral tx from l1 replaces or races the mutual close tx from
l2, causing a unilateral close, which breaks our test.
Though this is a corner case, it's much friendlier to allow the
closing negotiation again until we actually see the close onchain.
This fixes the tests here, too.
```
def test_closing_negotiation_reconnect(node_factory, bitcoind):
disconnects = ['-WIRE_CLOSING_SIGNED',
'+WIRE_CLOSING_SIGNED']
l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': disconnects,
'may_reconnect': True},
{'may_reconnect': True}])
l1.pay(l2, 200000000)
assert bitcoind.rpc.getmempoolinfo()['size'] == 0
l1.rpc.close(l2.info['id'])
l1.daemon.wait_for_log(r'State changed from CHANNELD_NORMAL to CHANNELD_SHUTTING_DOWN')
l2.daemon.wait_for_log(r'State changed from CHANNELD_NORMAL to CHANNELD_SHUTTING_DOWN')
# Now verify that the closing tx is in the mempool.
bitcoind.generate_block(6, wait_for_mempool=1)
sync_blockheight(bitcoind, [l1, l2])
for n in [l1, l2]:
# Ensure we actually got a mutual close.
> n.daemon.wait_for_log(r'Resolved FUNDING_TRANSACTION/FUNDING_OUTPUT by MUTUAL_CLOSE')
tests/test_closing.py:275:
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We now renegotiate an interrupted close, even if we don't need it, instead of sending an error.
We define a new subtype 'modern_scb_chan' and a new tlvtype 'scb_tlvs' which includes
all the relevant information to create a penalty transaction when the peer tries to cheat.
Key Changes:
- Rename the old format to 'legacy_scb_chan' and define a new type 'modern_scb_chan'
- Include TLVs to 'modern_scb_chan'
- Create a new msgtype 'static_chan_backup_with_tlvs'
- Modify 'struct channel' to include 'struct modern_scb_chan'
- Add these two types to 'varsize_types' in generate.py
We ratelimited DEBUG messages, but that can be annoying and cause us to miss things.
We demoted the worst offenders in the last release, to TRACE level.
Now, only log trace if it's wanted, and never suppress DEBUG.
Changelog-Changed: Logging: we no longer suppress DEBUG messages from subdaemons.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/7917
Cleans up the API: we have two functions now, one which is explicitly for
"I'm failing this because I saw this tx onchain".
Now we can correctly report the tx which closed the channel (previously
we would always report our own tx(s)!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `close` now correctly reports the txid of the remote onchain unilateral tx if it races with a peer close.
Changelog-Fixed: Protocol: we no longer try to spend anchors if a commitment tx is already mined (reported by @niftynei).
Fixes: #7526
Rather than have lightningd call us repeatedly to try to connect, have
it tell us what peers are transient and aren't, and connectd will
automatically try to maintain that connection.
There's a new "downgrade_peer" message to tell it a peer is now
transient: to make it non-transient we simply tell connectd to
connect as a non-transient.
The first time, I missed that dual_open_control does its own state
transitions :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: `connectd` now handles maintaining/reconnecting to important peers, and we remember the last successful address we connected to.
We're going to use this to ask if there are any channels which make it
important to reconnect to the peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was a bit harder to identify: during an `io_loop` run we suspend
the current span before handing over to `io_loop`, and later when a callback
is called we resume the span again. Depending on how we return from
the `io_loop` instance that is used to drive the startup, we either
have resumed the last span, or we don't. Since we start a span before
`io_loop` and want it to be emitted afterwards, we need to take care
of the case where we returned from a callback that did not resume, and
therefore the current context is empty.
Making `trace_span_resume` idempotent means we can just resume it
manually.
Ideally we'd push the suspend / resume logic down into `io_loop`
itself, and then we'd have just one place. Maybe suspend and resume
callbacks that can be configured in `io_loop`?
lightningd/test/run-find_my_abspath.c includes ../lightningd.c, which includes
header_versions_gen.h, a generated header file.
lightningd/Makefile correctly declares that lightningd/lightningd.o depends on
header_versions_gen.h, but lightningd/test/Makefile lacks any such declaration
regarding lightningd/test/run-find_my_abspath.c, which leads to build failure:
In file included from lightningd/test/run-find_my_abspath.c:5:
lightningd/test/../lightningd.c:64:10: fatal error: header_versions_gen.h: No such file or directory
64 | #include <header_versions_gen.h>
| ^~~~~~~~~~~~~~~~~~~~~~~
Declare the missing dependency in lightningd/test/Makefile so that Make will
ensure that header_versions_gen.h is generated before it attempts to build
lightningd/test/run-find_my_abspath.o.
Changelog-None
This means it always tells us explicitly whether to keep watching or not,
and we know it's processed it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This message is supposed to include the msat amount received. But this is
obviously per-HTLC, and we hacked it to use the value for the first one.
And we add logging whenever we fail an HTLC set, since we removed logging
by not calling failmsg_incorrect_or_unknown() (which, now, no longer needs
to log).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No code changes, just catching up with the BOLT changes which rework our
blinded path terminology (for the better!).
Another patch will sweep the rest of our internal names, this tries only to
make things compile and fix up the BOLT quotes.
1. Inside payload: current_blinding_point -> current_path_key
2. Inside update_add_htlc TLV: blinding_point -> blinded_path
3. Inside blinded_path: blinding -> first_path_key
4. Inside onion_message: blinding -> path_key.
5. Inside encrypted_data_tlv: next_blinding_override -> next_path_key_override
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This name is clearer than the old one.
And since the struct contains a string, it's more natural for the
struct to be the tal parent of the string so it's a real object. This means
we need an array of pointers, so each struct can be its own tal object.
wallet_state_change_get is hoisted higher in the code and made static.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids a db lookup on every iteration of listpeerchannels, which
can be slow on large nodes (Postgres, I assume).
We can now simply add the fields we want to channel load, and remove
wallet_channel_stats_load entirely.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than making the callers do this, make the invoice decoder perform
the various sanity checks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
invoice_path_id is actually a generic path_id thing, so rename it.
We're going to use the same scheme for path secrets and the tweak to
node_id when we create a fake pubkey for invoice_requests, so a new
header is appropriate.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
