Eclair

Fix a batch of low-severity issues (#3355)

* Remove explicit cast in channel codec

We used an explicit cast in our channel codec: this is safe for data
that we created ourselves, but would throw an exception if our data is
corrupted. It's trivial to be safe by removing the cast.

* Require `tls-tcp` when using cluster mode

We require that encryption is used between the front and backend nodes
in cluster mode, otherwise private data would be exposed. This protects
node from messing up their deployment configuration without realizing
it.

* Remove exception details from failed RPCs

We don't return the exception message anymore for failed RPC calls, to
avoid potentially leaking private information. This guarantees that only
the node operator, with access to the logs, can see why RPC calls fail.

* Don't decode unknown onion message TLVs

We don't support any TLV for onion messages yet. Since onion messages
can be spammy, we don't need to waste any ressources trying to decode
unknown TLVs that we'll throw away anyway.

* Limit the number of onion messages self-hops

We only include ourselves multiple times in an onion message path when
using dummy hops. If we're included too many times in a path, that's
most likely a remote node messing with us: we just drop the message.

Explicitly match on-the-fly HTLCs after a restart (#3357)

When we restart our node, we look at HTLCs that have been received but
not relayed yet: they must be failed back, otherwise we would let them
in the incoming channel until their timeout is reached, which would
force our peer to force-close.

There is an exception for HTLCs that are paying for an on-the-fly funded
channel: we will explicitly retry relaying those HTLCs, even after a
restart, because that's how we get paid for the funding fees. We were
only matching them by `payment_hash`, which wasn't precise enough: an
attacker could initiate an on-the-fly channel and then send unrelated
HTLCs that use the same `payment_hash`, just to mess up with us and get
some of our channels force-closed.

We already have the detailed information about on-the-fly HTLCs in our
DB, so we now use that to match HTLCs on the corresponding `channel_id`
and `htlc_id`, which guarantees uniqueness.

Better documentation for remote `bitcoind` (#3359)

Our documentation didn't make it clear enough that `bitcoind` should
ideally run on the same machine as `eclair`. If that's not possible,
since `bitcoind` doesn't provide support for TLS for its RPC and ZMQ
endpoints, the node operator MUST setup a secure tunnel (providing
encryption and authentication) between `eclair` and `bitcoind`. If
that isn't provided, a network attacker may drop or alter messages
between `eclair` and `bitcoind`, which can lead to a loss of funds.

Add more checks around funding amount and channel reserve (#3352)

* Add configurable `max_funding_satoshis`

We add a configuration parameter to control the maximum allowed size
of channels. This was previously limited by the wumbo feature bit, but
when we activated wumbo on the network we removed any upper limit. It
makes sense to still keep a reasonable bound on the channel size.

* Check channel reserve fundee requirements

We compare dust limits to channel reserves when we're fundee of a
single-funded channel to match the BOLTs. Note that this is less
clear for dual-funding, where the channel reserve is always set to
1% and those checks don't make as much sense and are replaced by a
check on the dust limits bounds. When using 0-fee commitments, we'll
want to allow high(ish) dust limits, which means that the reserve
may not be met: this is fine, as it only slightly changes incentives
but doesn't introduce a funds loss vector.

Fix several on-the-fly-funding bugs (#3351)

* Handle on-the-fly on-chain downstream failure post-restart

When restarting our node, we have a custom hook to avoid failing HTLCs
downstream when they're used with on-the-fly funding, because we want
to retry relaying those HTLCs. That doesn't apply to on-chain failures
though, because at that point the downstream channel has been closed.

* Don't relay on-the-fly HTLCs twice after restart

Before relaying on-the-fly HTLCs, we verify that they aren't already
being relayed. We only look at the pending changes, because once the
HTLCs are relayed we change their in-memory state in the Peer actor.

But if we restart after cross-signing on-the-fly HTLCs, we previously
would relay them again, thus paying the downstream user twice. We now
also look at the commitment transactions to avoid that.

* Force-close unpaid on-the-fly channels

When we fund an on-the-fly channel for which fees must be paid in the
follow-up HTLCs, our peer may act maliciously and ignore those HTLCs
when we relay them after funding the channel. We've already funded
the channel so it's too late to recover the on-chain fees we've paid,
so we previously just failed back the corresponding upstream HTLCs and
gave up on relaying them to the funded channel.

However, the HTLCs may already be cross-signed and pending on the funded
channel, which means that our peer could fulfill it and get paid while
we have failed the corresponding upstream HTLCs. In that case, we lose
funds. We can only fail the upstream HTLCs if we're sure that we haven't
relayed them yet,

Since obtaining this state is asynchronous and complex, we now opt-in to
a safer approach: we force-close the channel, check its commitments, and
only fail the upstream HTLCs that weren't relayed. If they have been
relayed, they will automatically be failed back when our HTLC-timeout
transaction confirms.

* Reject on-the-fly payment for batch of `payment_hashes`

While the protocol supports batching `payment_hashes`, we currently only
use it for one `payment_hash` at a time. It's thus safer to restrict it
to this to prevent potential abuse. We also explicitly reject the case
where an attacker tries to insert the same `payment_hash` twice to mess
up our state.

* Correctly relay preimages upstream after timeout

If we receive the preimage for an on-the-fly payment *after* the HTLC
has timed out, we still need to relay that preimage upstream, otherwise
we've paid downstream without being paid upstream.

Use `min_final_expiry_delta` in trampoline test handler (#3353)

We use the invoice's `min_final_expiry_delta` in the test trampoline
payment FSM (which isn't used for any real payment, it is only there
to allow cross-compat tests with other implementations).

When not using Bolt11, we increase the fallback value to 42, which
is what LDK uses by default.

We also add 1 block in case we're late in our view of the blockchain.

Harden permissions of seeds and datadir (#3340)

Several improvements suggested by Loupe (#3348)

* Remove transport keys from DEBUG logs

While `DEBUG` logs shouldn't be activated on mainnet, it is dangerous
that they expose key material. We remove those logs.

* Remove onion shared secrets from unparsable failure logs

When receiving a failure that we couldn't parse, we added the shared
secrets in the logs to help us troubleshoot (because we've seen that
error frequently on our node). It didn't let us figure out the issue,
which most likely comes from a buggy intermediate or final node. We
now remove those secrets from the logs for safety.

* Make incoming connection tracker idempotent

If we receive multiple times a request to track a given node, we ignore
the redundant ones.

* More robust network DB clean-up on restart

On restart, we remove `channel_update`s that we cannot decode from our
network DB. The way the code was written could be an issue depending
on the postgres driver, because it potentially interleaved SELECT and
DELETE clauses. It apparently doesn't matter for our current version
of the postgres driver, but it's more future-proof to explicitly split
the SELECTs from the DELETEs.

* Check wake-up config before trying message wake-up

We didn't check the wake-up config parameter when relaying onion
messages, which wasted resources by creating actors that would be
useless.

* Include wallet node_id encoding when resolving blinded paths

When resolving the introduction point of a blinded path and checking
whether one of the nodes is ourselves, we must check that with all
potential encodings, including the wallet-reserved one.

* Don't include extraneous HTLCs in trampolinefee calculation

When relaying a trampoline payment, we must ensure that we don't include
extraneous payments (that are rejected), otherwise we will use a very
large route fee budget.

* Correctly clean up MPP payment-initiator state

When directly crafting MPP payments using the `sendtoroute` API, we
use the child payment ID as tracking key in the `PaymentInitiator`
internal state. If the payment fails, the `PaymentFailed` event will
correctly use that ID, so the state will be cleaned up. But if the
payment succeeds, the `PaymentSent` event uses the *parent* payment ID
so the state wasn't correctly cleaned up. We now clean up the child
state as well.

* Explicitly handle empty payment types

We've recently improved our liquidity ads codec, but we weren't
properly handling the case where no payment types are provided
(even though this will likely be rejected by remote nodes).

Improve defenses against malicious bitcoin RPC endpoint (#3343)

* Verify `txid` returned by Bitcoin Core

When using Bitcoin Core RPC, we now validate that the returned
transaction matches the request. This is part of a defense-in-depth
strategy in case our batching client messes up the order of RPC
responses.

* Use keyed requests in bitcoin batch RPC client

We previously only relied on the order of requests to match responses
that we receive from bitcoind, but there is no guarantee that the order
will be preserved. In practice it seems to be the case, but we should
be resilient if that changes.

We now use unique request IDs and match the corresponding response using
this ID instead of relying on position only.

* Verify amount funded by Bitcoin Core wallet

We verify that the amount contributed by our Bitcoin Core wallet matches
what we requested. A malicious Bitcoin Core may underfund or overfund,
which in both cases is undesirable behavior.

Gossip queries fixes and improvements (#3345)

* Rate-limit gossip queries

A `query_channel_range` is a few dozen bytes to send, but answering one forces us to scan our whole routing table, compute a checksum for every channel_update we know and send back megabytes of data.
We introduce rate-limits for gossip queries, ack them only when they've been processed, and make processing them more efficient:

- Add per-connection rate-limits for `query_channel_range` per connection. Queries that we drop are still acked, otherwise the transport would stop reading from the connection.
The new`router.sync.max-queries-per-second` defaults to 10: peers only need to send a handful of those per connection, so this doesn't affect normal syncing.

- Only acks a query once we've sent our replies: acking first releases the transport-level back-pressure while we're still working, which lets queries pile up in the router's mailbox.

- Ignore gossip queries that are for another chain: we were answering them, which leaked our routing table to nodes that aren't even on our network. We already had that check for `gossip_timestamp_range`.

- cache the timestamps and checksums of our channel updates in `PublicChannel`, instead of re-serializing every update we know on every incoming query. The instance is replaced whenever an update changes, so the cache is transparently invalidated.

- remove the spent channels with `--` instead of rebuilding the whole channels map with `filterNot`.

- drop the `keep` filter in `handleQueryChannelRange`: it built a copy of our whole routing table, and `split` already restricts ids to the requested range as it iterates them in block height order.

* Bound the work a query_short_channel_ids can create

Ignore duplicate `short_channel_id`s: the spec doesn't forbid them, but a
query is capped at 65kB while our reply isn't capped at all, so repeating
the same scid was worth ~90x amplification to a peer. We already tracked the
node announcements we had sent for that reason, we now track channel ids the
same way.

Reject queries whose `encoded_query_flags` don't decode to exactly one flag
per `short_channel_id`, and queries that arrive before we've sent the
`reply_short_channel_ids_end` for the previous one. BOLT 7 explicitly allows
sending a `warning` for both, and forbids senders from doing either.

The dead `numca`/`numcu` accumulators in `processChannelQuery` are removed:
they were never incremented and the result was discarded. The counts that we
log and report to Kamon come from the callbacks and are unchanged.

* Bound the sync state a peer can make us keep

Our peer decides when a routing table sync ends, so we can't let it dictate
how much we buffer for it. Cap the number of `query_short_channel_ids` we
queue for a given peer during a sync with `router.sync.max-queries-per-sync`
(default 2000, i.e. 200 000 channels at the default chunk size, several
times the size of the network). Past that we ignore the extra channel ids
and keep draining the queries we already have: the sync is then incomplete
for that peer, but gossip and our other sync peers make up for it.

Ignore `reply_short_channel_ids_end` messages that don't answer a query we
have in flight. We were popping and sending the next query on any of them,
so a peer could send one right after answering our `query_channel_range`,
when we have nothing in flight: we would log "sync complete", drop our sync
state, and ignore the rest of its replies as unsolicited.

Note that `reply_short_channel_ids_end` carries nothing that identifies the
query it answers, so we can't do better than tracking whether we're waiting
for one: a peer that answers our queries can still send extra end messages
to drain our pending queries faster than we intended. That is now bounded by
the cap above, and the queries only go back to that peer.

`Syncing.started` is replaced by the explicit `queryInFlight` flag it was
being used as a proxy for.

* Don't drop queued queries when starting a sync

`addToSync` built a fresh `Syncing` in the branch that sends a query right away, which discarded `current.remainingQueries`.
That branch used to be reached only when we had no state at all for that peer, but since #1587 we pre-create a `Syncing(Nil, 0)` entry when we send our `query_channel_range`,
so it now overwrites real state instead of creating the first one.

This is not currently a bug as `remainingQueries` is always empty there today: it's an implicit invariant that nothing enforces. We pop from the queued list instead of overwriting it, so we can never silently drop queries.

Also narrow `Syncing.remainingQueries` to `List[QueryShortChannelIds]`: that's the only thing we ever put in it, and `addToSync` already returns that type.

* Rate-limit query_short_channel_ids

Same reasoning as for query_channel_range.

* Router: forget sync state for peers that are disconnected

Multiple bug fixes found by AI scanning (#3346)

* Fix 0-fee commitments fee bumping with remote-main

When using 0-fee commitments, if our peer publishes their commitment
and our main output is large enough, we don't need to use a wallet
input to pay RBF fees, we can directly do it using our main output.

However, we didn't take into account the whole package weight when
doing so: this means that there was a balance range where we would
not correctly bump the commit tx to the target feerate.

* Retry transaction publishing on unclassified failures

We classify bitcoind publish failures by substring-matching the RPC
error message. Any rejection that doesn't match any of the RPC error
messages that we know would previously not be retried.

This was dangerous, as it could cause us to abandon a time-sensitive
transaction, which could cause loss of funds. We now eagerly retry
when we cannot classify the RPC publish failure.

* Fix `channel_ready` using invalid musig2 nonce if RBF

We must use `commitments1` instead of `d.commitments` when creating our
`channel_ready` message to ensure that we use the correct musig2 nonce
for taproot channels. Otherwise, if we have multiple RBF attempts and
the ony confirming is *not* the latest one, we will use the wrong nonce.

* Fix out-of-bound array access in payment FSM

Remote failures can be manipulated to trigger an out-of-bound array
access which crashes the payment lifecycle actor.

Improve liquidity ads codec performance (#3344)

* Improve liquidity ads codec

We improve the performance of the liquidity ads codec by ignoring
unknown payment types, and simply checking whether the payment types
we support are activated or not.

* Reject node_announcement with invalid liquidity TLV

If the liquidity ads TLVs is present but with empty fields, we fail to
correctly re-encode it. We must reject it immediately instead.

Correctly handle unknown flags in `channel_update` (#3341)

We previously ignored unknown `channel_flags` and `message_flags` in
the channel updates we received at decoding time, which means we would
always replace them by `0` when re-encoding the channel update before
forwarding it. If a flag that we don't understand was set to `1`, the
signature becomes invalid.

In order to be future-proof, we must preserve the values of unknown
flags during a codec round-trip: this way old eclair nodes will still
correctly relay future channel updates that contain flags that haven't
yet been specified.

Back to dev (#3339)

After the v0.14.1 release.

Eclair v0.14.1 release (#3338)

Disable Bolt12 recipient path fee discount (#3332)

In https://github.com/ACINQ/eclair/pull/2993, we introduced a mechanism
in the default Bolt12 offer handler to let the recipient pay the fees of
the blinded paths they include in their Bolt12 invoices, instead of the
payer having to pay fees for privacy chosen by the recipient (when the
recipient decides to use a "real" blinded path with external nodes they
don't control).

This wasn't correctly taking MPP into account: payers could split the
payment in many tiny parts such that each part used the whole path fee
discount. As a result, the payer would have still paid the whole amount
but most of it would be collected by intermediate nodes inside the path
instead of the recipient, which isn't the goal.

We're thus disabling this feature: we'll need a cleaner protocol to
correctly account for MPP with base fees. Bolt12 isn't used yet by
merchants so it's fine: we'll wait for recipients to adopt it before
deciding how we introduce a blinded path fee discount feature (if this
is something that users request). Note that we also recommend users
to implement their own offer handler to customize their blinded paths,
if they care about privacy, instead of using the default handler which
is mostly there as a reference.

Prevent `channel_id` collisions (#3337)

* Reject channel if initial DB write fails

If the first DB write of a channel fails, we want to abort the channel
immediately, otherwise our state may be unusable. If for some reason
we already have a channel (for example because of a partial restore
from a DB backup), we want to avoid overwriting this state as well.

* Add global `channel_id`s collision resistance

We add a global, concurrent map that keeps track of every temporary
and final `channel_id` being used. We always add to this map before
creating channel actors or transitioning to a final `channel_id`
(and thus before storing channel data in the DB).

We only remove from that map when a channel actor dies: no other
channel can be associated with the corresponding temporary or
final `channel_id` since the channel was created after registering
the IDs.

Apply RBF limits to remote closing transactions (#3331)

* Apply RBF limits to remote closing transactions

When using `option_simple_close`, we didn't apply explicit rate-limits
to remote closing transactions, apart from the fact that the feerate
must be higher for each attempt.

There is no good reason to make a large number of RBF attempts when
closing a channel cooperatively, so we apply the same rate-limits we
use for funding.

* Bound the number of local closing transactions

We apply the BOLTs recommendation for the maximum number of closing
transactions we create when using the legacy closing protocol (not
`option_simple_close`). We previously restricted remote closing
transactions, but we now also apply it to our local transactions.

* Apply the maximum attempts limit to splice RBF

We previously only applied the rate-limit (X attempts every Y blocks)
but we didn't apply the maximum attempts limit. The risk is that we may
end up with a splice that is stuck unconfirmed, but will eventually
confirm, unless the peer is malicious and the feerate was very badly
estimated (in which case they would have found a way to prevent the
splice from confirming anyway).

Reject `start_batch` with size <= 1 (#3333)

Bolt2 specifies that a receiving node:

```
If batch_size is not strictly greater than 1:
- MUST ignore the start_batch message.
- SHOULD send a warning.
```

Don't store duplicate settlement messages (#3336)

If our peer is buggy, or if we have a bug in our message queue, we may
receive duplicate settlement messages for pending HTLCs. It is fine to
handle them and relay the settlement downstream (instead of immediately
force-closing), but we shouldn't store the duplicate settlement in the
remote changes, otherwise it will trigger a force-close when exchanging
`commit_sig`.

Accept Bolt12 invoices with reply path (#3325)

We previously rejected Bolt12 invoices when a reply path was provided
in the onion message. This wasn't in the spec, and it could make sense
to provide a specific reply path with an invoice in case the payer
considers the invoice invalid and wants to send back an invoice_error
to inform the recipient.

This created compatibility issues with LDK.

Fixes #3322

Ignore repeated invalid `tx_signatures` (#3328)

When our peer sends us an invalid `tx_signatures`, we processed it and
ignored the failure when we've already sent our own `tx_signatures`,
because the transaction may be broadcast and may confirm anyway.

We now stop processing those invalid `tx_signatures` repeatedly: if our
peer sends us more than 3 invalid `tx_signatures` for a given funding
transaction, they're definitely buggy so we'll just wait for the
transaction to confirm without trying to obtain the fully signed one
locally.

We also now avoid unnecessary DB writes on `shutdown`.
We don't need to write to the DB every time we receive `shutdown` from
our peer, since the BOLTs require `shutdown` to be retransmitted on
reconnection.

Update Bitcoin Core to v31.1 (#3327)

This release contains a new index which we need for #3279.

Reject `temporary_channel_id` duplicates early (#3324)

We previously rejected duplicate `temporary_channel_id` immediately, but
we didn't immediately reject `temporary_channel_id`s that conflict with
a *final* `channel_id` with the same peer: the rejection would happen
later in the flow when transitioning from temporary to final IDs, which
made debugging harder.

Since the channel interceptor step isn't atomic, we must check for
duplicate channel IDs *after* rate-limiting, to ensure that we don't
try inserting an entry in the channels map that is already there.

Reject incoming HTLCs with a high `cltv_expiry` (#3323)

We currently reject HTLCs with a `cltv_expiry` higher than 2 weeks in
the future when sending them (in the `sendAdd` method). That's because
they would lock up our funds for a long period of time if our peer
decided to force-close. Also, a large `cltv_expiry` makes slow jamming
easier.

We now apply the same limits to incoming HTLCs for consistency (since
they may differ by `cltv_expiry_delta`). Note that we don't do it in
the `receiveAdd` function, which would trigger a force-close. We just
accept those HTLCs and then fail them, since it is harmless to have
them only temporarily in our commitment as long as we don't relay.