Usually we won't see this, since private is deleted. But we could
have already read the private channel before that. Handle it properly.
(Tested by removing the gossip_store deletion code and making sure
this worked).
We have to fix up the test, which announces a channel twice!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thanks to @zerofeerouting for another report.
"desc" here is the sanitized message, eg:
"ERROR error channel 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef: internal error"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Prior to 0.11.0 we had cases where we would treat errors
as warnings: regretfully, this is still needed. This message
in particular has been widely reported, and it now causes
channel force closes.
Downgrade and log. I did insert some snarky log message earlier,
but hey, I'm sure CLN has done worse things to our peers!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: treat LND "internal error" as warnings, not force close events (as we did in v0.10).
@whitslack complained of large CPU usage by connectd at startup;
I ran perf record on connectd on my machine (which sees a little spike, only)
and I see the cost of reading and discarding the entries:
```
- 95.52% 5.24% lightning_conne lightning_connectd [.] gossip_store_next
- 90.28% gossip_store_next
+ 40.27% tal_alloc_arr_
+ 22.78% tal_free
+ 11.74% crc32c
+ 9.32% fromwire_peektype
+ 4.10% __libc_pread64 (inlined)
1.70% be32_to_cpu
```
Much of this is caused by the search for our own gossip: keeping this separately
would be even better, but this fix is minimal.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: connectd: reduce initial CPU load when connecting to peers.
This changes many fields: in non-deprecated mode, they're now raw integers.
This was always the intention, but the transition was never completed.
Suggested-By: @ShahanaFarooqui
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: "_msat" fields can be raw numbers, not "123msat" strings (please handle both!)
Changelog-Deprecated: JSON-RPC: "_msat" fields as "123msat" strings (will be only numbers)
We had json_add_amount_msat_only(), which was designed to be used to
print out msat fields, if we had sats.
However, we misused it, so split it into the three different cases:
1. json_add_amount_sat_msat: We are using it correctly, with a field called
xxx_msat.
2. json_add_amount_sats_deprecated: We were using it wrong, so deprecate
the old field and create a new one which does end in _msat.
3. json_add_sats: we were using it to hand sats as a JSON parameter to an
interface, where "XXXsat".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Plugins: `rbf_channel` and `openchannel2` hooks `their_funding` (use `their_funding_msat`)
Changelog-Deprecated: Plugins: `openchannel2` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `funding_satoshis` (use `funding_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `channel_reserve_satoshis` (use `channel_reserve_msat`)
Changelog-Deprecated: Plugins: `channel_opened` notification `amount` (use `funding_msat`)
Changelog-Deprecated: JSON-RPC: `listtransactions` `msat` (use `amount_msat`)
Changelog-Deprecated: Plugins: `htlc_accepted` `forward_amount` (use `forward_msat`)
Per BIP-0171, the signature map is of pubkey to "The signature as would
be pushed to the stack from a scriptSig or witness".
Fixes 5298
Changelog-Fixed: PSBT: Fix signature encoding to comply with BIP-0171.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
This likely lead to a number of false errors when attempting to
route. We deemed a channel to be unusable as soon as either direction
isn't usable. This is bad since it excludes not only zeroconf
channels (which have different scids for the two directions), but it
also excludes any channel that we haven't seen an update from
yet. This was likely introduced when attemting to exclude nodes that
haven't sent a disable, but their peer has, but this is not necessary
as the unresponsive node would be marked as isolated by all its peers,
so we don't need to artificially mark a channel direction as disabled
when really we can't even enter the node to traverse the channel in
that direction.
Changelog-Fixed: routing: Fixed an issue where we would exclude the entire channel if either direction was disabled, or we hadn't seen an update yet.
After this, we can exactly reproduce the vectors (in DEVELOPER mode).
1. Move payment_metadata position to match test vector.
2. Create flag to suppress `c` field production.
3. Some vectors put secret before payment_hash, hack that in.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this (send warnings) in almost all cases anyway, so mainly this
is a textual update, but there are some changes:
1. Send ERROR not WARNING if they send a malformed commitment secret.
2. Send WARNING not ERROR if they get the shutdown_scriptpubkey wrong (vs upfront)
3. Send WARNING not ERROR if they send a bad shutdown_scriptpubkey (e.g. p2pkh in future)
4. Rename some vars 'err' to 'warn' to make it clear we send a warning.
This means test_option_upfront_shutdown_script can be made reliable, too,
and it now warns and doesn't automatically close channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This doesn't have an effect now (except in experimental mode), but it
will when we support anchors. So we deprecate the use of those in the
close command too.
For experimental mode we have to avoid using p2pkh; adapt that test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `shutdown` no longer allows p2pkh or p2sh addresses.
The signatures on the new examples are sometimes different from what we produce though?
They're valid, however.
And one example has an unneeded feature 5-bit; it's not *wrong*, but
it's not optimal.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Partial revert of 43a833e405
"lightningd: remove support for legacy onion format."; we restore the
ability to decode legacy onions for forwarding, but not to generate them.
(We don't accept them properly since making payment_secret compulsory
anyway, so no real change there!)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Protocol: ... but we still forward legacy HTLC onions for now.
I removed these prematurely: we *haven't* had a release since
introducing them!
This consists of reverting d15d629b8b
"plugins/fetchinvoice: remove obsolete string-based API." and
plugins/fetchinvoice: remove obsolete string-based
API. "onion_messages: remove obs2 support."
Some minor changes due to updated fromwire_tlv API since they
were removed, but not much.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: REVERT: Removed backwards compat with onion messages from v0.10.1.
There's a race under CI, where a channel is deleted then we see the
channel_update in the gossip store. We assumed this wouldn't happen,
but it can!
```
[gw1] [ 95%] FAILED tests/test_connection.py::test_multichan
[gw1] [ 95%] ERROR tests/test_connection.py::test_multichan
...
> raise ValueError(str(errors))
E ValueError:
E Node errors:
E - lightningd-3: had BROKEN messages
E - lightningd-3: Node exited with return code 1
E Global errors:
...
lightningd-3: 2022-03-28T00:11:42.160Z DEBUG wallet: Owning output 0 100000sat (SEGWIT) txid 30616903feba1839a3834e2b3b6123759ce1fe0d76414ca77e2dbc17414772e0 CONFIRMED
lightningd-3: 2022-03-28T00:11:42.392Z DEBUG hsmd: Client: Received message 5 from client
lightningd-3: 2022-03-28T00:11:42.393Z DEBUG hsmd: new_client: 2
lightningd-3: 2022-03-28T00:11:42.398Z INFO plugin-topology: Killing plugin: exited during normal operation
lightningd-3: 2022-03-28T00:11:42.400Z **BROKEN** plugin-topology: Plugin marked as important, shutting down lightningd!
...
----------------------------- Captured stderr call -----------------------------
topology: update for channel 105x1x1 not found!
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Means that field is now optional in JSON output.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `delinvoice` has a new parameter `desconly` to remove description.
LNURL wants this so they can include images etc in descriptions.
Replaces: #4892
Changelog-Added: JSON-RPC: `invoice` has a new parameter `deschashonly` to put hash of description in bolt11.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was tlv_fields_valid that wanted a non-const: now that's gone, we
can make this correctly const.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Callers were supposed to call "tlv_fields_valid" after fromwire_tlv,
but few did. Make this the default, and call the underlying function
directly where we want to be more flexible (one place).
This loses the ability to allow misordered fields, or to pass through
*any* even fields. We restore that for special cases in the next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Requiring the caller to allocate them is ugly, and differs from
other types.
This means we need a context arg if we don't have one already.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No more "towire_offer", but "towire_tlv_offer".
This means we double-up on the unfortunately-named `tlv_payload` inside
the onion, but we should rename that in the spec when we remove
old payloads.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, this changes the name of a field in invoice_request:
`payer_signature` becomes simply `signature`. So we allow both for
now, and send the old one unless deprecated_apis is disabled.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we did fill them in, we filled them in wrong: the offset should be
the offset in the message, not the field number!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also has to fix up tests.
Changelog-Fixed: cli doesn't required anymore to confirm the password if the `hsm_secret` is already encrypted.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Things allocated by libwally all get the tal_name "wally_tal",
which cost me a few hours trying to find a leak.
In the case where we're making one of the allocations the parent
of the others (e.g. a wally_psbt), we can do better: supply a name
for the tal_wally_end().
So I add a new tal_wally_end_onto() which does the standard
tal_steal() trick, and also changes the (typechecked!) name.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As per proposal in https://github.com/lightning/bolts/pull/962
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: protocol: support for legacy onion format removed, since everyone supports the new one.
Christian Decker
Greg Sanders
Vincenzo Palazzo
Rusty Russell
Michael Schmoock
Jon Griffiths
Michael Dance
Ken Sedgwick