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>
* Update code dependencies
I also wanted to update logback, but I'm hitting issues because of our
custom logger in `FixtureSpec` (`LoggingEventAware` not found).
* Update build dependencies
We're now using mvn 3.9.2 to build eclair, which reports warnings in some
of the build plugins we use. Updating plugins fixes most of the warnings,
and the remaining warnings have to be fixed by the plugins themselves
to support mvn 4.x.
After exchanging `tx_complete`, we validate the splice transaction before
sending our `commit_sig`. If we consider the transaction invalid, we send
`tx_abort`. But if our peer thinks the transaction is valid, they will send
their `commit_sig`, which we must ignore until they've acked our `tx_abort`.
Test assumes that payments inserted in the db will be returned in a deterministic order because they have different timestamps.
It may not always be the case if TimestampMilli.now() returns the save value twice.
So here we use explicitly different timestamps: now = TimestampMilli.now(), now + 1.milli, now + 2.milli ....
The previous repository we used removed the binaries for mvn 3.9.2, which
broke our docker build. It looks like `archive.apache.org` keeps artifacts
longer and hopefully won't break our docker build again soon.
When restarting, we weren't checking whether it was using blinded paths.
If we were an intermediate node in the blinded path, we were incorrectly
returning a normal failure: it should be ok, since the introduction node
is supposed to translate those failures, but it's safer to assume that
they don't.
For fighting jamming attempts, or even just to detect one, we need to know how fast relayed HTLCs are fulfilled. We now measure this and store it in the audit database. Previously the "IN" and "OUT" directions for the same HTLC were storing the same timestamp (corresponding to when the HTLC is fulfilled), we now use the timestamp at which we received the UpdateAddHtlc for the "IN" direction.
Move away from the "block target" approach.
Get rid of the `FeeEstimator` abstraction and use an `AtomicReference` to store and update the current feerates, similar to the block count.
When sending a message, the postman can now ask the router to find a route using channels only.
The same route is also used as a reply path when applicable.
The graph data structure has been updated to include both active and disabled edges.
The graph now contains features for vertices.
It turns out that #2688 broke the JSON serialization of the `channels`
API by introducing a new `ActorRef`. We never want to surface `ActorRef`
in JSON responses, so we're defining serializers to filter them out.
Co-authored-by: Fabrice Drouin <sstone@users.noreply.github.com>
The `Peer` actor can now directly be queried for the list of its channels.
This makes this feature more reusable than the previous actor that was
customized for the peer-ready scenario.
The spec says that we must return an error if:
- cltv_expiry < outgoing_cltv_value
- cltv_expiry < current_block_height + min_final_cltv_expiry_delta
For the second check, we actually tested if:
- cltv_expiry < max(outgoing_cltv_value, current_block_height) + min_final_cltv_expiry_delta
But that check should only verify that our `min_final_expiry_delta`
requirement is fulfilled, which is unrelated to the `outgoing_cltv_value`.
It was redundant with the first check, which already guarantees that
intermediate nodes inside the blinded path cannot cheat by using a higher
`cltv_expiry_delta` than what the recipient intended.
In cluster mode, outgoing connections are initiated by frontend nodes.
If there are no frontend node available, we fail the connection attempt
with a dedicated error.
The `initial-random-reconnect-delay` should be configured to allow
enough time for the front nodes to bootstrap at reconnection.
When initializing the DB, we potentially run some data migration that
cannot be reverted. We want to this last, after we've checked every other
start-up requirement, such as the bitcoind version.
Fixes#2609
LND and CLN already use 2016 blocks. The network is generally raising the
values of `cltv_expiry_delta` to account for high on-chain fees, so we'll
need to allow longer maximum deltas to avoid rejecting payments.
When we want to add new types of inputs/outputs that contain specific
tlvs, we will need to store them alongside standard inputs/outputs.
We will use traits and case classes inside `InteractiveTxBuilder`, and
need to thus add a discriminator when encoding them.
This RPC allows to access the historic channel data without
relying on third party services like LN explorers.
Note that when the `remoteNodeId` filter is not provided, this
query may be expensive on nodes with a lot of closed channels.
In theory we don't have to store their commit_sig here, as they
would re-send it if we disconnect, but it is more consistent with
the case where we send our tx_signatures first.
Previous implementation had the advantage of being all in one place, but it left holes:
- `last_connected_timestamp` was only set after the first disconnection
- in some corner cases the `closed_timestamp` was never set (nothing at stake, funding tx timeout, post-restart)
If we force-close with HTLCs that have just been signed by our peer but
for which we haven't received their revocation, we should ignore them.
We have not relayed those HTLCs so they can't be fulfilled. It is our
peer's responsibility to claim them on-chain (using their HTLC-timeout),
but if for some reason they don't claim it, we don't want the channel to
be stuck in the closing state.
Fixes#2669
