* Update CI test with latest bitcoin core (switch from autotools to cmake)
bitcoin core now uses cmake instead of autotools.
CI test is triggered by a cron job but can now also be triggered manually.
* Don't spawn anchor tx publisher if commit is confirmed
It is inefficient to spawn a tx publisher for anchor txs if we already
know that the commit tx is confirmed: we will make calls to our bitcoin
node that can easily be avoided. This can matter when force-closing a
large number of channels with frequent disconnections (e.g. wallets).
* Improve `TxTimeLocksMonitor` performance
When publishing a transaction that has CSV delays, we previously used
the watcher and set a `minDepth` on the parent transaction matching
the CSV delay of the child transaction. While this was very simple,
it was unnecessarily expensive for large CSV delays: the watcher would
check for tx confirmations at every block, even when the CSV delay is
very large. When we force-close a large number of channels, it results
in a very large number of RPC calls to our `bitcoind` node.
We don't use the watcher in the `TxTimeLocksMonitor` anymore: instead
we check the parent confirmations once, and then we check again after
the CSV delay.
* Add relative delay hints to `ZmqWatcher`
When we tell the `ZmqWatcher` to watch for confirmations on transactions
that have a relative delay, it is highly inefficient to call our bitcoin
node at every new block to check for confirmations (especially when the
parent transaction isn't even confirmed). We now tell the watcher about
the relative delay, which lets it check for confirmations only at block
heights where we expect the transaction to reach its minimum depth. This
is especially useful to improve performance for delayed transactions
that usually use a CSV of at least 720 blocks.
We refactor `NodeRelay.scala` to re-order some steps. The steps are:
1. Fully receive the incoming payment
2. Resolve the next node (unwrap blinded paths if needed)
3. Wake-up the next node if necessary (mobile wallet)
4. Relay outgoing payment
Note that we introduce a wake-up step, that can be extended to include
mobile notifications. We introduce that same wake-up step in channel
relay and message relay. We also allow relaying data to contain a wallet
`node_id` instead of an scid. When that's the case, we start by waking
up that wallet node before we try relaying onion messages or payments.
This wake-up step doesn't contain any logic right now apart from waiting
for the peer to connect, if it isn't connected already. But it can easily be
extended to send a mobile notification to prompt the wallet to connect.
* Reject new static_remote_key channels
We will still load and use existing static_remote_key channels, we can still open static_remote_key channels, but we will not accept new static_remote_key channels.
This behaviour can be overridden by setting `eclair.channel.accept-incoming-static-remote-key-channels` to `true`.
* Reject new obsolete incoming channels
We reject new incoming channels that don't even support `option_static_remotekey` (which is assumed to be on in the BOLTs).
Unit tests have been modified to use static_remote_key or anchor channels (default used to be the obsolete "standard" channel).
---------
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
There weren't any logs when relaying onion messages, which makes it
impossible to troubleshoot. While we're working on cross-compatibility,
we keep those logs at the `INFO` level, and will change some of them to
`DEBUG` once stabilized.
We move the `Upstream` trait closer to the `Origin`, and make it more
obvious than a hot `Origin` is:
- an `Upstream` referencing the upstream HTLCs
- an actor requesting the outgoing payment
We also improve the cold trampoline relay class to record the incoming
HTLC amount, which we previously didn't bother encoding but is useful to
compute the fees collected during relay. To ensure backwards-compat, it
is set to `0 msat` for pending HTLCs. It will only affect HTLCs that
were pending during the upgrade, which is acceptable.
When we require inputs to be confirmed, we can reliably check whether
they are unspent. We can't reliably check this for unconfirmed inputs,
because they could be valid but simply not in our mempool, in which
case bitcoind would incorrectly consider them unspendable.
We want to reject unspendable inputs early to immediately fail the
funding attempt, instead of waiting to detect the double-spend later.
A race exists because node C can publish the txs to spend the htlcs from the revoked commitment *before* F publishes their txs to spend two of the htlc outputs.
Instead we handle the rare case of a `txn-mempool-conflict` failure.
We will sometimes miss testing that C claims those two htlcs via 3rd stage txs.
The channel initiator traditionally pays the commit tx fees, but we may
want to override that when providing services to wallet users. We thus
split the current `isInitiator` flag into two flags:
- `isChannelOpener`
- `paysCommitTxFees`
We always set `paysCommitTxFees` to the same value as `isChannelOpener`.
Custom feature bits may override that behavior if necessary.
Note that backwards compatibility is preserved since our previous `bool8`
codec encodes `true` as `0xff` and `false` as `0x00`.
We do log all incoming/outgoing messages, including `TxAddInput`, but
the `toString()` prints the whole serialized tx, not the txid, making
grepping more difficult.
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
Sending a payment to a blinded route that starts at our node can be tricky, we remove this problem by unwrapping such routes before starting the payment flow.
---------
Co-authored-by: t-bast <bastien@acinq.fr>
https://github.com/lightning/bolts/pull/1163 makes the channel update in
onion failures optional. One reason for this change is that it can be a
privacy issue: by applying a `channel_update` received from an payment
attempt, you may reveal that you are the sender. Another reason is that
some nodes have been omitting that field for years (which was arguably
a bug), and it's better to be able to correctly handle such failures.
To save space in the offer, we can skip the node id for offers that use blinded paths.
The node id used to sign the invoice will be the last blinded node id of the path used to request the invoice.
We also make the description optional for offers without amount.
Bitcoin Core 26.1 contains ancestor-aware funding: it will automatically
fetch unconfirmed ancestors during funding and adapt the fee to apply
the target feerate to the whole unconfirmed package.
We had custom code to implement this entirely in eclair, which we can
now remove.
https://github.com/lightning/bolts/pull/1092 initially made the
`gossip_queries` feature mandatory and assumed: however it generally
doesn't make sense for mobile wallets. It was thus relaxed in the last
commits of this BOLTs PR, so we revert our change to `mandatory` to
make this optional and avoid sending gossip queries to nodes who don't
activate the feature.
If the peer had `option_shutdown_anysegwit` enabled when they initially sent their `remoteScriptPubkey`, but reconnected later with `option_shutdown_anysegwit` disabled, then `isValidFinalScriptPubkey` may not pass anymore.
Allow sending messages to self
Fixes corner cases caused by compact encoding of node ids. Every message to be relayed now follows the same path and `MessageRelay` can relay to self.
We started using dual-funding by default in some integration tests. One
of the changes is that we consider the channel created once we send our
commitment signatures. At that point, we are not yet in a state where
we have fully signed commitments: that will happen once we receive our
peer's signatures.
Some tests assumed that when receiving the `ChannelCreated` event, we
would have fully signed commitment. We update those tests to handle the
case where we are actually still exchanging signatures.
We support dual funding, but we cannot decide for the node operator when
to contribute to channels that are being opened to us. Node operators
may want very different policies depending on their goals. Instead of
trying to figure out a general set of policies, we let plugins decide
when to contribute to channels and how much. Node operators are thus
free to implement whatever logic makes sense for them in a plugin.
When there is a mismatch between the feerate of a channel and the
feerate we get from our estimator, we may want to force-close because
that could be exploited by our peer to steal HTLCs. But that's only the
case if the feerate is too low, not if it's too high. We previously
force-closed in both cases, whereas we only need to do it when the
feerate is too low.
This should avoid some unnecessary force-close that we've observed and
are due to buggy fee estimators (fee estimation is hard!), or to peers
who simply do some smoothing and slightly delay lowering the feerate of
our channels.
When we detect that the remote commit has been published, we spend our
anchor output from that commit if the fees are too low and we have funds
at risk. But if the remote commit is then evicted from our mempool, we
cannot publish our anchor tx and must instead skip it, since we don't
provide the fully signed remote commitment to the publisher.
We otherwise error when calling `fundrawtransaction`, where `bitcoind`
fails because it cannot find the external non-wallet utxo. This change
gets rid of those errors that can be quite confusing for node operators.
When funding a transaction that contains inputs that are external to our
bitcoin wallet, bitcoin core will add a lock to those external inputs,
thinking that they may be wallet inputs that it doesn't know about yet.
In our case, those are never wallet inputs, they are instead:
- the current channel output, when creating a splice transaction
- an output of the commitment transaction, when force-closing
We previously explicitly filtered those inputs before calling `unlock`,
which was wrong. We now also unlock those utxos.
To save space, blinded routes may use a compact node id (scid + direction instead of public key) as an introduction node.
When using such a compact route, the sender must use it's knowledge of the network to convert that to a public key, however trampoline users don't have that knowledge, they must transmit the compact route to the trempoline provider.
We extend the spec to allow compact node ids in the `next_node_id` field.
Co-authored-by: t-bast <bastien@acinq.fr>
When a channel is closed, we can forget the data from historical HTLCs
sent and received through that channel (which is otherwise required to
punish cheating attempts by our peer).
We previously synchronously removed that data from the DB when the closing
transaction confirmed. However, this could create performance issues as
the `htlc_infos` table can be very large for busy nodes and many concurrent
writes may be happening at the same time.
We don't need to get rid of this data immediately: we only want to remove
it to avoid degrading the performance of active channels that read and
write to the `htlc_infos` table. We now mark channels as closed in a
dedicated table, and run a background actor that deletes batches of
obsolete htlc data at regular intervals. This ensures that the table is
eventually cleaned up, without impacting the performance of active
channels.
When a splice transaction confirms, all the revoked commitment transactions
that only applied to the previous funding transaction cannot be published
anymore, because the previous funding output has already been spent.
We can thus forget all the historical HTLCs that were included in those
commitments, because we will never need to generate the corresponding
penalty transactions.
This ensures that the growth of our DB is bounded, and will shrink every
time a splice transaction is confirmed.
Fixes#2610, #2702 and #2740
* Test bitcoind wallet behavior during mempool eviction
When transactions are being evicted from the mempool, we want to make
sure that bitcoind keeps the corresponding wallet inputs locked to
avoid accidentally double-spending ourselves. We update our unit test
to be closer to common scenarios (chain of unconfirmed splice txs and
commit txs replaced by the remote version).
This test confirms that bitcoind keeps wallet inputs locked, and we
have to call `abandontransaction` to free up utxos used in txs that
have been permanently double-spent.
* Test bitcoind wallet behavior during double spend
When one of our wallet transactions is directly double-spent, the
bitcoin wallet detects that and automatically frees up wallet inputs
from the permanently double-spent wallet transaction. This is great,
that means we only need to manually call `abandontransaction` for
transactions that have been invalidated because one of their parents
has been double-spent.
We had some cases where channels were staying stuck in the CLOSING state
without going to the CLOSED state in previous versions of eclair where
we published different versions of our 3rd-stage transactions after a
restart and a feerate increase.
This happened in two cases:
- when we published a different version of our claim-main, since we only
store the latest (it's an `Option`, not a `Seq`) and put a watch on
this specific `txid` instead of watching the corresponding output
- when we re-published 3rd-stage txs after receiving `CMD_FULFILL_HTLC`
because we replaced the previous `claim-htlc-delayed` transactions
This was on my backlog for a while, and I wanted to take another look at
it after the splicing implementation. Fortunately, this was fixed as
part of the splicing changes, but it's worth having a test to ensure
that there's no regression from future changes.
