Before:
Ten builds, laptop -j5, no ccache:
```
real 0m36.686000-38.956000(38.608+/-0.65)s
user 2m32.864000-42.253000(40.7545+/-2.7)s
sys 0m16.618000-18.316000(17.8531+/-0.48)s
```
Ten builds, laptop -j5, ccache (warm):
```
real 0m8.212000-8.577000(8.39989+/-0.13)s
user 0m12.731000-13.212000(12.9751+/-0.17)s
sys 0m3.697000-3.902000(3.83722+/-0.064)s
```
After:
Ten builds, laptop -j5, no ccache: 8% faster
```
real 0m33.802000-35.773000(35.468+/-0.54)s
user 2m19.073000-27.754000(26.2542+/-2.3)s
sys 0m15.784000-17.173000(16.7165+/-0.37)s
```
Ten builds, laptop -j5, ccache (warm): 1% faster
```
real 0m8.200000-8.485000(8.30138+/-0.097)s
user 0m12.485000-13.100000(12.7344+/-0.19)s
sys 0m3.702000-3.889000(3.78787+/-0.056)s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We make it a first-class citizen internally, even though we won't use
it over the wire (at least, non-experimental builds). This scheme
follows the latest draft, in which features are flagged compulsory.
We also add several helper functions.
Since uses the *even* bits (as per latest spec), not the *odd* bits,
we have some other fixups.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This touches a lot of text, mainly to change "if `option_anchor_outputs`"
to "if `option_anchors`"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This check is going away anyway (only Electrum enforced it), but we
know that all wumbo peers expect large HTLCs to work today.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: Allow sending large HTLCs if peer offers option_support_large_channel (> 4294967295msat)
Pointed out by @fiatjaf, and indeed it happened to me as well; a peer with
a high feerate reconnects and sends a similar (but now ludicrous) feerate,
and we get upset:
```
$ lightning-cli listpeers 039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36
{
"peers": [
{
"id": "039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36",
"connected": false,
"channels": [
{
"state": "CHANNELD_NORMAL",
"scratch_txid": "d796aa9c44920cc7169cdb61e36437bf180cedaec44103a69591ce2baac9b1d9",
"last_tx_fee": "14329000msat",
"last_tx_fee_msat": "14329000msat",
"feerate": {
"perkw": 19791,
"perkb": 79164
},
```
Then in the logs:
```
2021-07-23T19:34:56.227Z DEBUG 039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36-channeld-chan#39381: billboard perm: update_fee 17055 outside range 253-7210
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
By default, we won't close a channel that we leased to a peer.
You can override this with the `force_lease_closed` flag.
Changelog-Added: JSON-RPC: close now has parameter to force close a leased channel (option_will_fund)
Enable non-dev builds to send custom messages.
Preserves 'dev-' for compat-enabled builds.
Changelog-Changed: JSON-RPC: moved dev-sendcustommsg to sendcustommsg
This has been reported several times on regtest, most recently by Gijs
van Dam. It turns out approx_max_feerate() was returning 0 in some
corner cases: we should *not* be using that value (as shown, it's
overly conservative) except as a ceiling on fee *increases*.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: don't ever send 0 fee_updates (regtest bug).
This lets us transition (with a few supporting changes) to closingd,
which will happily let them mutual close with us.
We already handle the case where this mutual close is redundant (for
packet loss), so this is easy.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We will now reestablish and negotiate mutual close on channels we've already closed (great if peer has lost their database).
This supports reestablish on a closed channel: we tell channeld to
respond to the reestablish message appropriately, then close the
channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Let the callers do that (only channeld needs to do this).
We temporarily send an error on unknown reestablish in openingd, as
this mimic previous behavior and avoids breaking tests (it does leave
a BROKEN message in the logs though, so
test_funding_external_wallet_corners needs to ignore that for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't actually set desired_type yet, but this handles it.
Changelog-EXPERIMENTAL: Protocol: we can now upgrade old channels to `option_static_remotekey` from https://github.com/lightningnetwork/lightning-rfc/pull/868
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For quiescence, we can't have sent any updates at all.
But for upgrades on reconnection, we may have already added
uncommitted HTLCs for retransmission, but they don't count towards
"are we quiesced" since they're not sent yet.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This includes anysegwit and the updated HTLC tiebreak test vector. It
also adds explicit wording for invalid per_commitment_secret (which
nicely matches our code already!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This may have made our feestates fully resolved, so we can send
update_fee again. Without this fix our tests sometimes timeout.
Also add debugging so we can see when we suppressed a feechange.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are several reports of desynchronization with LND here; a simple
approach is to only have one feerate change in flight at any time.
Even if this turns out to be our fault, it's been a historic area of
confusion, so this restriction seems reasonable.
Changelog-Fixed: Protocol: Don't create more than one feerate change at a time, as this seems to desync with LND.
Fixes: #4152
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You can now activate dual-funded channels using the
`--experimental-dual-fund` flag
Changelog-Changed: Config: `--experimental-dual-fund` runtime flag will enable dual-funded protocol on this node
Looks like #4394 treated a symptom but not the root cause. We were
actually sending the message framed with the WIRE_CUSTOMMSG_OUT and
the length prefix over the encrypted connection to the peer. It just
happened to be a valid custommsg...
This fixes the issue, and this time I made sure we actually send the
raw message over the wire. However for backward compatibility we
needed to imitate the faulty behavior which is 90% of this patch :-)
Changelog-Fixed: plugin: `dev-sendcustommsg` included the type and length prefix when sending a message.
We move over to the new "warning" paradigm, instead of using
an "rbf_fail" message.
Every failure is either a warning or an error; on warnings we
hang up and reconnect later, effectively resetting the state.
And make all the callers choose which one. In general, I prefer warn,
which lets them reconnect and try again, however some places are either
stated that they must be errors in the spec itself, or in openingd
where we abandon the channel when we close the connection anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: we now send warning messages and close the connection, except on unrecoverable errors.
This takes from the draft spec at https://github.com/lightningnetwork/lightning-rfc/pull/834
Note that if this draft does not get included, the peer will simply
ignore the warning message (we always close the connection afterwards
anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now report the new (draft) warning message.
Don't include exp directly, use an ifdef in common/bolt12
(like we do for peer and onion wiregen files).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's a case where a dropped funding_locked will result in the peer
moving onto channeld, while we stay in dualopend. As we haven't
received their funding_locked, we retransmit tx_sigs, which channeld
will need to handle.
With the patch the peer drops it on the floor; the peer will resend
funding_locked on reconnect, which will correctly advance us to
channeld and CHANNELD_NORMAL
We used this for dual funded opens, to track the receipt of signatures.
We're moving all of this over to dualopend now, however, so we no longer
need the PSBT in channeld.
The previous onion_message code required a confirmed, not-shutting-down
channel, not just a connection. That's overkill; plus before widespread
adoption we will want to connect directly as a last resort.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Still asserts that it's the standard size, but makes it a dynamic
member. For simpliciy, changes the parse_onionpacket API (it must be
a tal object now, so we might as well allocate it here to catch all
the callers).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
While debugging a hanging channel with a user I noticed that they
called `close` on a channel, resulting in the channel showing
`CHANNELD_SHUTTING_DOWN`, but the billboard seemed to show the
information the wrong way around:
```json
{
"peers": [
{
"connected": true,
// ...
"channels": [
{
"state": "CHANNELD_SHUTTING_DOWN",
// ...
"status": [
"CHANNELD_SHUTTING_DOWN:Reconnected, and reestablished.",
"CHANNELD_SHUTTING_DOWN:Funding transaction locked. They need our announcement signatures. They've sent shutdown, waiting for ours"
],
// ...
}
]
}
]
}
```
Aside from the hung channel, the switch in direction of the status
seemed weird. Checking the billboard code seems to have the status
switched as well:
ff8830876d/channeld/channeld.c (L223-L226)
We set `shutdown_sent[LOCAL]` when we send the shutdown:
ff8830876d/channeld/channeld.c (L823-L839)
And we set `shutdown_sent[REMOTE]` when we receive the shutdown:
ff8830876d/channeld/channeld.c (L1730-L1781)
So I think the billboard code just needs to be switched around.
Changelog-Fixed: JSON-RPC: The status of the shutdown meesages being exchanged is now displayed correctly.
Required to determine if this msg used expected reply path.
Also remove FIXME (om->enctlv is handled above).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
channeld/channeld.c:237:2: error: ‘shutdown_status’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
```
Reported-by: az0re on IRC
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
libwally has a quirk where the finalize method will fail to 'completely'
finalize an input's parts if either the final_scriptsig or
final_redeemscript fields are set
since we manually set the final_witness stack here, we also need to
fully finalize the redeemscript -> final_scriptsig here as well.
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>
We used to send our tx_sigs before we got to channeld existing. We
changed how this worked so that multifundchannel could live, but failed
to clean up the logic of what "having a psbt around" means wrt channeld
and messaging our peer.
The general idea is that we want to send `tx_signatures` to our peer on
reconnect until they've sent us `funding_locked`.
Note that it's an error to
- send `funding_locked` without having sent `tx_signatures`
- send `tx_signatures` after sending `funding_locked`
We use the 'finalized' state of the peer's inputs/outputs to help signal
where we are in receiving their sigs -- but this doesn't work at all for
opens where the peer doesn't contribute inputs at all.
This isn't really a huge deal, but it does mean that if we receive a
peer's `tx_sigs` more than once (will happen for a reconnect before
`funding_locked`), then we'll issue a notification about receiving their
sigs multiple times. /shrug
Prior to this patch update, we expected a client to call
`openchannel_signed` before checking for peer's tx-sigs messages on the
wire.
When moving to a 'multifundchannel' approach, we'll need to be able to
collect sigs from our peers before sending our tx_sigs message. There's
no strict ordering on when tx-sigs messages are sent/received, so this
is fine.
To do this, we go ahead and start up channeld as soon as
commitment_sigs are secured, so that we process incoming tx-sigs from
our peers as soon as we get them.
We're about to totally upset the order that sigs are set on our PSBTs
for new channel opens, making it such that our peer's sigs may arrive
before ours do.
We can no longer rely on the 'set witness means this is our input' since
there's no guarantee that our input sigs have been added yet, so we
check the serial_id and only set the stack on their (odd) inputs.
Instead of a boutique message, use a "real" channel_announcement for
private channels (with fake sigs and pubkeys). This makes it far
easier for gossmap to handle local channels.
Backwards compatible update, since we update old stores.
We also fix devtools/dump-gossipstore to know about the tombstone markers.
Since we increment our channel_announce count for local channels now,
the stats in the tests changed too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's a few structs/wire calls that only exist under experimental features.
These were in a common file that was shared/used a bunch of places but
this causes problems. Here we move one of the problematic methods back
into `openingd`, as it's only used locally and then isolate the
references to the `witness_stack` in a new `common/psbt_internal` file.
This lets us remove the iff EXP_FEATURES inclusion switches in most of
the Makefiles.
...right time.
We re-send the tx_sigs on start/init/reconnect until we've gotten a
funding_locked from our peer. We also build it in channeld now, instead
of in dualopend, and don't pass in a message for them anymore
We need the PSBT to create the finalized tx from once the peer's
tx_signatures are received. Since we're passing the PSBT, we no longer
need the secondary message to be passed, as it was derived from the
PSBT.
Also removes now unused witness serialization code
We had one report of this, and then Eugene and Roasbeef of Lightning
Labs confirmed it; they saw misordered HTLCs on reconnection too.
Since we didn't enforce this when we receive HTLCs, we never noticed :(
Fixes: #3920
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: fixed retransmission order of multiple new HTLCs (causing channel close with LND)
1. Rename memleak_enter_allocations to memleak_find_allocations.
2. Unify scanning for pointers into memleak_remove_region / memleak_remove_pointer.
3. Document the functions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
v2 of channel establishment, in the accpeter case, now sends 2 messages
to our peer after saving the information to disk (our commitment
signatures and our funding transaction signatures)
v2 channel open uses a different method to derive the channel_id, so now
we save it to the database so that we dont have to remember how to
derive it for each.
includes a migration for existing channels
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>
Fixes: #3608
Changelog-Changed: protocol: Ignore (and log as "unusual") repeated `WIRE_CHANNEL_REESTABLISH` messages, to be compatible with buggy peer software that sometimes does this.
Note that other directories were explicitly depending on the generated
file, instead of relying on their (already existing) dependency on
$(LIGHTNINGD_HSM_CLIENT_OBJS), so we remove that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means some files get renamed, and I took the opportunity to clarify
our naming (the *d* is important!)
1. channeld/channel_wire.csv -> channeld/channeld_wire.csv
2. channeld/gen_channel_wire.h -> channeld/channeld_wiregen.h
3. enum channel_wire_type -> enum channeld_wire
4. WIRE_CHANNEL_FUNDING_DEPTH -> WIRE_CHANNELD_FUNDING_DEPTH.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is best done by passing `struct bitcoin_signature` around instead
of raw signatures. We still save raw sigs to the db, and of course the
wire protocol uses them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Includes:
psbt: Use renamed functions for new wally version
psbt: Set the transaction directly to avoid script workarounds
psbt: Use low-S grinding when computing signatures
tx: Use wally_tx_clone from libwally now that its exported
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
The main change here is that the previously-optional open/accept
fields and reestablish fields are now compulsory (everyone was
including them anyway). In fact, the open/accept is a TLV
because it was actually the same format.
For more details, see lightning-rfc/f068dd0d8dfa5ae75feedd99f269e23be4777381
Changelog-Removed: protocol: support for optioned form of reestablish messages now compulsory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
now that witness script data is saved into the tx/psbt which is
serialized across the wire, there's no reason to use witscript to do
this. good bye witscript!
