#2761 introduced the ability for the HTLC sender to let a remote initiator
dip into its reserve to unblock channels after a large splice. However, we
relaxed that condition for all channels, even those that don't use splice.
This creates compatibility issues with other implementations that are
stricter than what the specification requires, and will force-close in
those situations.
This version of logback fixes the following CVE:
"a potential denial of service attack on a centralized logback receiver
when a third party controlling a remote appender connects to said
receiver and could shut down or slow down logging of events."
Eclair isn't affected since we don't use logback receivers, but if there
are applications or plugins that depend on eclair and use logback
receivers, it's better to use the logback version containing the fix.
The format used by bitcoinheaders.net is changing to use whole bytes
instead of nibles, which is easier to parse. We start using the v2 format
exclusively, which will allow deprecating the previous format.
Fixes#2786
* Add a txOut field to our InteractiveTxBuilder.Input interface
This will help us add support for taproot inputs: to create taproot signatures we need all prevouts (and not just prevouts for tx inputs that spend taproot outputs).
Offers (https://github.com/lightning/bolts/pull/798) allow nodes to be identified using either the public key or a pair channel id and direction.
The goal is to save bytes as channel id and direction only use 9 bytes instead of 33 for a public key.
When we receive a `channel_update` in `update_fail_htlc`, we should take
it into account to exclude channels or correctly retry with updated fees,
but we shouldn't apply it to our routing table.
If we did, that could be exploited to deanonymize our payments.
It shouldn't be necessary anyway, as honest nodes should broadcast
those `channel_update`s publicly, so we would receive them through the
normal gossip mechanism.
Fixes#2767
As soon as a channel is transitioning to a closing state (mutual or
unilateral close), it cannot be used to relay HTLCs anymore. We should
notify the network by sending a disabled `channel_update`.
Fixes#2766
If remote has a positive contribution, we do not check their
post-splice reserve level, because they are improving their
situation, even if they stay below the requirement. Note that if
local splices-in some funds in the same operation, remote
post-splice reserve may actually be worse than before, but that's
not their fault.
Splicing (and dual funding as well) introduce a new scenario that could
not happen before, where the channel initiator (who pays the fees for the
commit tx) can end up below the channel reserve, or right above it.
In that case it means that most of the channels funds are on the non
initiator side, so we should allow HTLCs from the non-initiator to the
initiator to move funds towards the initiator. We allow slightly dipping
into the channel reserve in that case, for at most 5 pending HTLCs.
If we abort the interactive-tx protocol while we're waiting for bitcoind
to fund the transaction, we must avoid sending an obsolete `tx_add_input`
for the funded transaction. There is a check in `Channel.scala` to only
send an outgoing interactive-tx message if we have an active interactive-tx
session, but this isn't sufficient in the following scenario:
- we disconnect and thus abort the interactive-tx session while bitcoind
is funding
- we reconnect and restart another interactive-tx session
- bitcoind responds for the first interactive-tx session
In that scenario we would mix `tx_add_input` from the old and the new
interactive-tx session, which leads to a duplicate `serial_id`.
We were incorrectly splitting the shared input amount based on each peer's
balance, but for the signing order the peer that adds the shared input
takes credit for the whole amount of that input.
Whenever we use the local commit, we must use the local dust limit to
trim HTLCs. Similarly, whenever we use the remote commit, we must use
the remote dust limit.
Sometimes the notifier for node A has not been killed before node A connects. This causes an extra GetPeerInfo message to be sent to the switchboard and the `assert(request3.remoteNodeId == remoteNodeId2)` check to fail when the GetPeerInfo for node A comes first. Just waiting to receive the `AsyncPaymentTimeout` message is not enough.
If node A connects after node B then this race does not occur.
Other implementations and Eclair (#2703) disable channels only when the other peer is offline.
So disabled channels should no be used for routing messages.
This PR removes `ActiveEdge` that was introduced in #2656
Some channels have only a few sats available to send but other nodes don't know it so they try to use them and fail.
When the balance goes below configurable thresholds we now advertize a lower maxHtlcAmount.
This should reduce the number of failed attempts and benefit the network.
We assumed that once quiescent, the local and remote commitment index
would be the same, but that's not true. Those two indices may diverge
because of concurrent updates. We need to keep using the right index for
each commitment during and after a splice, otherwise it leads to "invalid
commit sig" errors.
When we start, bitcoind may not be ready to handle requests if we restarted
it around the same time. We thus poll it for 30 seconds before giving up.
This is especially useful for tests (e.g. on regtest).
The `ChannelFeatures`->`ChannelType` conversion is problematic for the following reasons:
- It creates an implicit bidirectional mapping between the two classes, which is illogical and error-prone because both directions must be kept in sync.
- Not only must the conversions be kept in sync, it must also be stable over time.
- From the spec p.o.v., the scope of `ChannelType` is really limited to channel opening negotiation, while we currently make it a permanent concept.
- It has weird semantic side effects where we would check channel types in places where we really meant to check commitment formats.
If a channel has negotiated quiescence with pending htlcs then `InteractiveTxBuilder` will create/verify commit txs which include htlc outputs and create/verify htlc signatures when exchanging `CommitSig`.
The new attribute `htlcsAmount` of `Output.Shared` and `Input.Shared` accounts for the value in htlcs when computing fees.
* Move some logs to debug level
This should reduce the pressure on the file system and RAM without impacting
our ability to troubleshoot common issues.
* Avoid herd effect watching external channels
When we restart, we put watches on every public channel in the network.
That creates a lot of RPC calls to bitcoind, which aren't time-sensitive.
It's ok if we don't see immediately that an external channel was closed,
the spec even recommends waiting for 12 blocks to distinguish a channel
close from a splice.
By default, we now smooth that over a 1 hour period. This means we should
also allow our peers to be late at discovering that a channel was closed.
We thus stop sending a `warning` in that case and increase our tolerance
to that kind of behavior.
* Avoid herd effect watching local channels
When we restart, we set watches on our funding transactions. But we don't
actually need to watch them immediately, we just need enough time to react
to our peer broadcasting their commitment. We use long `cltv_delta` delays
to guarantee funds safety, so we can spread out the watches across several
blocks to reduce the start-up load. It essentially is the same thing as
receiving mempool transactions or blocks after a delay, which is something
that our threat model already takes into account.
* Allow splicing on non dual-funded channels
We currently allow splicing on top of a channel that was created without
using dual funding, but it results in an incorrect channel reserve being
used post-splice.
* Rename requestedChannelReserve_opt
This field only applies to the first funding index, so we rename it to
make that explicit. Once a splice has happened, it will be ignored.
A minor fix for tests that rely on the minimum feerate being low.
---------
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
We want to be able to bump a force-close regardless of which commitment
was published. If the remote commitment is in our mempool, we will use
our anchor on that commit tx to bump the fees.
* Always retransmit tx_signatures if requested
If our peer asks us to retransmit our `tx_signatures`, we should do it even
if the transaction is already confirmed. That means we need to store our
`tx_signatures` because they're annoying to recompute.
* Ignore previously received commit_sig
When expecting a retransmission of `tx_signatures`, we should ignore the
`commit_sig` they send just before if we've already received it. The right
way to check that we've already received it is to compare its signature
to our latest commitment transaction.
Some tests related to bitcoind's funding behavior have been randomly
failing since we updated to bitcoind 24.1. They now have more sensibility
over the exact set of available utxos, so we needed to fine-tune them
accordingly.
I also imported some tests that were added to feature branches but never
backported to `master`.
If we're unable to find the spending tx in the mempool, we start looking
for it in the blockchain. Unfortunately, there are cases where we end up
in that code path even though the spending tx is not confirmed:
- a timeout on the `getTransaction` call
- the spending tx gets dropped from our mempool for some reason
- bitcoind is malicious or buggy
Fetching the whole blockchain isn't really useful: after enough time has
passed, we can be pretty sure that a potential attacker would have claimed
the transaction's outputs already and we can't punish them.
We make the distinction between _not_ defining a bitcoin wallet name, and using an bitcoin wallet with an _empty_ name.
This remains mostly backward compatible, as:
- by default `eclair.bitcoind.wallet` is undefined so we keep using the default loaded wallet as before
- if user already set `eclair.bitcoind.wallet`, then eclair will use that wallet.
- only in the case where the user left `eclair.bitcoind.wallet` undefined, and if bitcoin had only one wallet with a non-empty name, then eclair will fail right at startup with an explicit error
---------
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
* Make Eclair manage bitcoin core's wallet private keys
We create an empty watch-only wallet and import public descriptors generated by Eclair.
Bitcoin Core can fund transaction and manage utxos, but can no longer sign transactions.
* Check that spent amounts and utxos are consistent before we sign a PSBT
PSBT utxo fields include the amount that are being spent by the PSBT inputs, but there is a "fee attack"
where using amounts that are lower than what is actually spent may make us sign a tx that spends much more
in fees than we think.
* Check that non-segwit uxto has been provided and inputs are signed with SIGHASH_ALL
* Verify that Bitcoin Core's fee match what we specified
When we call Bitcoin Core's `fundrawtransaction` RPC method, we check that the fee that we pay match the fee rate that we requested.
The fee is computed using the utxo information that Bitcoin Core adds to our PSBT before we sign it.
We can safely used this information because if Bitcoin Core lies about the value of the inputs that we're spending then the signature we produce will also not be valid (it commits to the value being spent).
When we're adding wallet inputs to "bump" the fees of a parent transaction we need to take the whole package into account when we verify the actual fee rate, which is why some internal methods were modified to return the package weight that was used as reference when `fundrawtransaction` was called.
* Check that fundrawtransaction does not add more than 1 change output
* Validate addresses and keys generated by bitcoin core
When eclair manages private keys, make sure that we can re-compute addresses and keys
generated by bitcoin core.
* Add a separate configuration file for Eclair's onchain signer
Eclair's onchain signer now has its own `eclair-signer.conf` configuration file in HOCON format.
It includes BIP39 mnemonic codes and passphrase, a wallet name and a timestamp.
When an `eclair-signer.conf` file is found, Eclair's API will return descriptors that can be imported into an
empty watch-only Bitcoin Wallet.
When wallet name in `eclair-signer.conf` matches the name of the Bitcoin Core wallet defined in `eclair.conf`
(`eclair.bitcoind.wallet`), Eclair will bypass Bitcoin Core and sign onchain transactions directly.
* Skip validation of local non-change outputs:
Local non-change outputs send to an external address, for splicing out funds for example.
We assume that these inputs are trusted i.e have been created by a trusted API call and our local
onchain key manager will not validate them during the signing process.
* Document why we use a separate, specific file for the onchain key manager
Using a new signer section is eclair.conf would be simpler but "leaks" because it becomes available everywhere
in the code through the actor system's settings instead of being contained to where it is actually needed
and could potentially be exposed through a bug that "exports" the configuration (through logs, ....)
though this is highly unlikely.
* Additional changes to delegate bitcoin core keys to eclair (#2726)
Refactor the `BitcoinCoreClient` and `LocalOnChainKeyManager` to:
- rely less on exceptions
- use more idiomatic scala (reduce dependency on kotlin types)
- provide more detailed logs
We also simplify the `useEclairSigner` field in `BitcoinCoreClient`.
The complexity of handling the case where there was an on-chain key
manager but for a different wallet than the one configured isn't
something that should be used, so it wasn't worth supporting.
Some checks were inconsistent and are now unified:
- checking the exact `scriptPubKey` in our outputs in TODO and TODO
- we allow using `fundTransaction` with a tx that already includes a
change output (which may happen when RBF-ing a transaction)
- `getP2wpkhPubkeyHashForChange` didn't verify the returned key
We completely separate the two cases in `signPsbt`, because otherwise
in the non eclair-backed case, we were calling bitcoind's `processpsbt`
twice for no good reason, which is bad for performance.
We also decouple the `OnChainKeyManager` from the `BitcoinCoreClient`.
This lets users keep running their eclair node with a bitcoin client that
owns the private key while configuring the on-chain key manager for a
future bitcoin client that will leverage this on-chain key manager.
Users can use the eclair APIs to get the master xpub and descriptors to
properly configure their next bitcoin core node, and switch to it once it
has synchronized the descriptors.
* Simplify replaceable tx funding
We were previously signing twice (with makes a call to `bitcoind`),
just to get the final weights and adjust the change outputs. This was
unnecessary, as we can adjust the weights before adding inputs.
We were also duplicating the checks where we verify that `bitcoind` is
malicious. We only need to check that once, during the final signing step.
---------
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
Node operators may disable automatic fee-bumping on their local commit if
they don't have anything at stake (no pending HTLCs), which saves fees in
most cases. A drawback in that case is that if the commitment doesn't
confirm quickly enough, the remote's funds are also locked.
This can be an issue for LSPs, where the remote peer doesn't have a good
ability to fee-bump commit txs. We give more control to the node operator
by letting them fee-bump local commit txs explicitly through the RPC to
unblock wallet users funds.
When reconnecting in the middle of signing a splice, we must ensure that
splice_locked is sent *after* tx_signatures. Otherwise when using 0-conf
we may retransmit splice_locked before tx_signatures, which our peer will
ignore because they don't have a corresponding fully signed commitment.
We start watching funding outputs once the corresponding funding transaction
has been confirmed with 3 confirmations. We may have already spent that
transaction with a child splice transaction by that time: providing hints
with the corresponding txids to the `ZmqWatcher` improves the performance
when that happens.
This is a follow up to #2565, more precisely to the _caveat_ here: https://github.com/ACINQ/eclair/pull/2565#issuecomment-1397052582.
We now create a fresh shutdown script even if one was already generated at channel creation, if the channel doesn't have the mandatory `option_upfront_shutdown_script` negotiated.
The (reasonable) assumption is that other implementations will ignore our pre-generated script if they didn't support the `option_upfront_shutdown_script` feature.
This "on-the-fly" approach is simpler and safer than a db migration.
This lets us use the new `gettxspendingprevout` instead of fetching the
whole mempool when looking for txs spending one of our channels.
A new feature was added to bitcoind 24.1+ that tries to make the change
output indistinguishable from the payment output. This is a great for
privacy, but it adds randomness to coin selection and uses a non-minimal
set of inputs sometimes. We work around this in tests by updating the
amount of the output we want bitcoind to use to make sure it's sufficient to
pay for both the channel funding and the change output.
This shouldn't be too much of an issue for normal operation, where we'll
sometimes use two inputs instead of one, which costs more fees, but
increases privacy.
See https://github.com/bitcoin/bitcoin/pull/24494 for details.
When our peer fails HTLCs, we only propagate the failure upstream once
we've received their revocation for the previous commitment (because they
could otherwise publish the previous commitment and claim those HTLCs).
If they publish the new commitment without sending us their revocation,
we previously didn't propagate the failure upstream, which leads to an
unnecessary force-close. We now correctly handle this scenario.
This change adds support for the quiescence negotiation protocol via the new `stfu` message. When a channel is quiescent, both sides will have the same set of signed htlc commitments and a splice can be performed without requiring the channel to be idle.
An additional PR is still required to update our splice implementation to properly account for in-flight htlcs. Quiescence should currently only be enabled for compatibility testing.
We send a warning and disconnect when a forbidden messages is received during quiescence; a disconnect ends quiescence. If an htlc is fulfilled/failed while quiescent, any preimage will be relayed immediately and the update will be replayed when quiescence ends.
We also send a warning and disconnect if both quiescence and splice negotiation are not complete before the quiescence timeout.
---------
Co-authored-by: t-bast <bastien@acinq.fr>
