Normally we don't care, but if we're withholding it, keep it around
so we can sign & broadcast later.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Interestingly, @niftynei added a funding_psbt column to the db in 2020,
but we don't use it (it was removed early 2021 with the "inflight"
architecture). So we don't need to add a new column, just plumb it
back in.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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 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>
Iterating through every peer and channel every time can be very slow
for large nodes, when calling wallet_coinmoves_extract for listcoinmoves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This replaces the old "iterate through each peer, then each peer's channel" suboptimality.
A bit of care required that we don't expose scids if we're forwarding,
but that was already carefully handled.
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>
Changelog-Fixed: Protocol: we now allow routing through old short-channel-ids once a splice is done (previously we would refuse, leading to a 6 block gap in service).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There can be any number of these, and it will be useful to allow
routing by older scids (when other nodes haven't seen our gossip, or
even before we *can* announce the new post-splice channel).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to use a check on the active psbt to see if our splice signature was in it — but now we need to generate the signature early.
So we have to add a field tracking if we’ve sent it and add it to the database, wire protocols, and inflight objects.
Prior to it being compulsory, these daemons would need a default value. Now it's
always required, it's clearer if it's always told.
There's no "default_channel_type" now everyone has to specify channel_type either,
so rename it to "desired_channel_type" and put it in lightningd specifically.
Note that the channel_type can have options added: either option_scid_alias or option_zeroconf.
This results in a slight behavior change: we will get type zeroconf even if we didn't ask for it, if they gave it to us.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: fundchannel / fundchannel_start returned `channel_type` will include option_zeroconf if it was implied by a 0 minimum_depth, even if we didn't explicitly ask for a zero conf channel.
The spec used to say you had to wait for channel to be ready, *and* 6
depth before exchanging signatures. Now the 6 depth requirement is only
on the actual announcing of the channel: you can send sigs any time.
This means our state machine goes from:
NOT_USABLE -> NOT_DEEP_ENOUGH -> NEED_PEER_SIGS -> ANNOUNCED
to:
NOT_USABLE -> NEED_PEER_SIGS -> NOT_DEEP_ENOUGH -> ANNOUNCED
However, this revealed that our state machine is insufficient, so
rework it to be more general and understandable. In particular,
check for unexpected state transitions, and thus document them.
Note that cg->sent_sigs replaces channel->replied_to_announcement_sigs,
too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: We now exchange `announcement_signatures` as soon as we're ready, rather than waiting for 6 blocks (as per recent BOLT update)
A splice where reestablish happens at the right moment causes an infinite loop of announcement signatures being sent back and forth.
Limit the announcement sigs we send in response to announcement sigs to once per channel session.
ChangelogNone
`bitcoin_tx_with_psbt` would somewhat opaquely steal the passed `psbt` value.
This caused a bug where code made a `bitcoin_tx` using a psbt without realizing the value was stolen. Because the resulting `bitcoin_tx` was placed in tmpctx it was not immediately clear that using `psbt` afterwards was an error until the tmpctx was cleared — creating a valgrind backtrace far from the actual issue.
Switching to the routine to using TAKES and adding documentation in the header, makes it explicitly clear which operation the user is doing — helping prevent future regressions of this kind.
Changelog-None
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>
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
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.
This is more useful than the last address, which may be it connecting
to us. And use it when we restore it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Enable storing the remote funding pubkey in DB if the channel peer decides to change it during splicing. It needs to be in DB incase of restarts mid-splice.
Changelog-None
Christian noted that if we don't do this we could flood onchaind with messages:
particularly in Greenlight where the HSM (remote) may delay indefinitely, so
onchaind doesn't process messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We start by telling onchaind about the funding spend, and anything
which spends it, and it tells us the txids it *doesn't* want to watch
any more. We're going to use a separate set of watches for the replay
case: this implements that code.
Once we're caught up, we convert any remaining watches to normal ones
to follow future blocks.
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>
And instead of loading them in listpeerchannels, use them. This means
listpeerchannels no longer touches the db.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSONRPC: `listpeerchannels` (and thus, pay) sped up on very large nodes.
We still support *existing* channels. Just not new ones (before they could,
in theory, explicitly ask for one).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a u64, we should pass by copy. This is a big sweeping change,
but mainly mechanical (change one, compile, fix breakage, repeat).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a nice reflection of channel stability: in particular, worse case
ping time is 45 seconds, so we should have had some traffic.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, we were sending `announcement_signatures` before
`channel_reestablish`; we allow this because LND used to do it, but
it's not spec compliant.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is a bit messy, but it tries to do the minimal switchover.
Some tests change, so those are included here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
