If we didn't plan on using zero-conf, but our peer sends us an early
channel_ready, we can opportunistically switch to zero-conf.
But we can only do that if we're sure that our peer cannot double-spend
the funding transaction. We previously checked their contribution to the
funding output, but that's not enough: they may add inputs to the funding
transaction even if they don't contribute to the funding output.
We were also setting duplicate `WatchPublished` in case we were already
using zero-conf, which is now fixed.
When our peer sends us channel_ready while we're still waiting for
confirmations, we may opportunistically switch to zero-conf, in which
case we have both a WatchPublished and a WatchConfirmed pending.
But it may not actually be a real switch to zero-conf: maybe the
transaction is confirmed, and they simply received the block slightly
before us. In that case, the WatchConfirmed may trigger first, and it
would be inefficient to let the WatchPublished override our funding
status: it will make us set a new WatchConfirmed that will instantly
trigger and rewrite the funding status again.
After calling this method, we perform actions at several places that
only make sense if the correct behavior happened. Instead of assuming
things went ok, we use proper typing and make the result explicit.
Each RBF attempt adds more data that we need to store and process,
so we want to limit our peers to a reasonable use of RBF.
We send a warning to let them know that they are close to reaching the
limits.
There are two possible strategies for MetaCommitments:
1. Move the duplication between every Commitment in a shared field
2. Access duplicated fields through the first Commitment
I initially wanted to go with solution 1., but it makes commitments really
hard to work with. The main reason is that the `CommitmentSpec` abstraction
is a very good abstraction to work with when updating commitments,
and it requires keeping the htlcs inside every commitment.
As long as we update commitments in a loop, there is no risk of common
values being desynchronized. Since they contain mostly pointers to shared
data, the memory overhead is negligible, as long as we make sure we don't
duplicate the data when serializing to disk. We're thus choosing solution 2.
We serialize htlcs separately and rebuild `CommitmentSpec` objects when
deserializing. We also only keep the main htlc fields during json serialization
to avoid performance issues when writing json fields to the DB. There was
no good reason to serialize everything as we previously did.
This requirement was always met for single-funded channels, but for dual
funded it may not be met if the non-initiator contributes a very large
amount (more than 100x what the initiator contributes). That scenario
could happen, and the funding attempt shouldn't fail as long as the
initiator's balance contains enough funds to pay the commit tx fees.
Adding splicing to the InteractiveTxBuilder requires:
- adding a shared input (the current funding output)
- adding outputs (when splicing out)
The `localAmount` and `remoteAmount` provided are the amounts each peer
contributes to the new funding output. Those amounts should be computed
by the caller depending on what they intend to do (splice funds in/out).
This model allows batching operations: if the caller wants to do several
splices in and several splices out, this is easy to do by iterating over
these operations and updating the targeted `localAmount` accordingly,
the InteractiveTxFunder will take care of the rest.
Note that when splicing, we currently truncate previous balances to sats.
This results in 1 sat being given away to miners as fees, and a passive
participant losing up to 999 msat of their balance. This can be changed
in the future depending on the final spec choice, and shouldn't need a
codec update since the previous balances come from the commitment field
provided in the purpose.
When splicing, the tx_add_input message we send for the shared input
doesn't contain the previous transaction. There are two reasons for
that:
- it potentially doesn't fit into a lightning message (if > 65kB)
- we don't need it, we know it correctly uses segwit
The ReplaceableTxPublisher only looked at the remote commit and ignored
the case where our peer publishes their next commit. This created two
issues:
- eclair would keep trying to publish htlc transactions that had no chance
of confirming, which is a waste of resources
- eclair would fail to RBF claim-htlc transactions: this had no impact today
because we currently target the next 2 blocks so RBF isn't necessary,
but it will become useful if we allow setting a more economical block
target in the future
We used to consider zero-conf funding txs as _confirmed_ as soon as they were _published_, but it was hacky. Before splices, it prevented RBF-ing zero-conf txs, which in theory make sense (we start using the channel without confirmations, but may still want to bump the fees later). After splices, it would prevent using a channel while a splice tx is pending confirmation (even if the channel isn't zero-conf).
What we are really doing is separating the state of the channel, from the state of the blockchain. As a consequence the management of the funding tx is more independent from the channel FSM, which is why we end up with catch-all handlers for `WatchPublishedTriggered`/`WatchFundingConfirmedTriggered`.
As a side effect, we also simplify how we put watchers: instead of putting them back at every connection, we do this once and for all (either when creating the channel, or at restart).
---------
Co-authored-by: t-bast <bastuc@hotmail.fr>
That additional event lets subcribers know when a channel was closed
without ever being used.
The possible flows for a channel lifecycle are now:
- ChannelCreated -> ChannelOpened -> ChannelClosed
- ChannelCreated -> ChannelOpened -> ChannelAborted
- ChannelCreated -> ChannelAborted
- ChannelCreated -> ChannelClosed
- ChannelAborted
Co-authored-by: Richard Myers <remyers@yakshaver.org>
We previously inflated the funding amount to always exceed the dust limit,
otherwise bitcoind would reject the funding attempt. It complicated the
code for a scenario that doesn't make sense in practice, so it's better
to just let the funding attempt fail in that case.
We also randomize the order of inputs and outputs, which is a common
privacy best practice.
It makes sense to store the params with each `rbf` attempt (as
opposed to the last one currently), and allows us to remove
`DATA_WAIT_FOR_DUAL_FUNDING_CONFIRMED.fundingParams`.
In case we are the introduction node of a message blinded route, we would not be able to send a message to that route. We now unwrap the first hop in case the route starts with us.
The order of the elements in a TLV stream is an implementation detail that will disappear with serialization. Equality between TlvStream shouldn't depend on this order.
For that we use `Set`s instead of `Iterable`s.
This reduces our dependency on `Commitments`, and makes
it more obvious where we really need a specific commitment
(and thus need to be careful when we have multiple).
This doesn't contain any logic changes, it's purely a refactoring.
* Add a finalScriptPubKey field to DATA_CLOSING (no functional changes)
* Add an explicit finalScriptPubKey parameter to helper methods (no functional changes)
* Rename LocalParams field defaultFinalScriptPubKey to upfrontShutdownScript_opt
`localParams.upfrontShutdownScript_opt` is now optional, and a new field has been added to DATA_CLOSING: finalScriptPubKey, which is the actual pubkey script onchain funds are sent to.
When reading DATA_CLOSING data serialized with older codecs, we rely on the fact that `localParams.upfrontShutdownScript_opt` was always present and was actually used as the final onchain
destination: it is safe to use `localParams.upfrontShutdownScript_opt.get` to populate DATA_CLOSING's finalScriptPubKey field.
* Update closing address when possible
When a channel gets closed, and when possible, we send all onchain funds to a recent onchain address and not
to an address that was generated when the channel was created.
This is possible for new channels in all cases except when mutual-closing a channel that sets `upfront-shutdown-script`.
Existing channels, which will keep sending onchain funds to addresses generated when they were created. Migration for existing channels will be handled in another PR.
Co-authored-by: Pierre-Marie Padiou <pm47@users.noreply.github.com>
Move all the logic that was previously inside `Commitments` to `MetaCommitments`.
The code changes are quite minimal, with a few subtleties: we need to update the
local/remote commit indices separately of the commitments themselves, because
they're not in the same data structure (yet).
Some of these changes show where the current model doesn't work well:
- `remotePerCommitmentPoint` is duplicated across all remote commits:
it's not obvious, but it has to be the same value
- htlcs should be shared between commitments
- the coupling between `remoteNextCommitInfo` and `nextRemoteCommit_opt`
is really weird to work with (especially in `receiveRevocation`): this can be fixed
by slightly re-working the model
The data model will be updated in future commits to fix these issues.
We introduce a `Purpose` trait, which indicates what the operation is about. Currently there are two possibilities:
- `FundingTx`
- `FundingTxRbf`
This allows for some nice factoring, but more importantly it will be complemented by a third possibility for splices.
Note that this change preserves the original separation between two main concerns:
- creating the funding tx (in `InteractiveTxParams`)
- creating the commitment (in `InteractiveTxBuilder`).
Fixes#2478.
I tried to keep the changes minimal, but had to move the _read_ from `Setup` to `Setup.bootstrap`, otherwise we would have kept a reference of the initial channels list forever.
Checking the db now happens a bit further in the startup procedure, but it is still blocking, and before the actors are initiated.
Apparently lnd rejects keysend payments that contain a payment secret,
instead of simply ignoring that field.
The blip doesn't mention this requirement:
https://github.com/lightning/blips/blob/master/blip-0003.md
But we don't have a strong reason to send it either, so let's remove that.
* Update `require_confirmed_inputs` tlv
It has been assigned tlv type 2 in the specification.
* Add second per-commitment point to dual funding
We can send the second per-commitment point right at the beginning of the
channel opening flow, to avoid the awkwardness of having to store an
invalid value in `remoteNextCommitInfo` until we receive `channel_ready`.
* Echo incoming tx_abort
When we receive tx_abort, we echo it back to let our peer know we've seen
their tx_abort and have reset our internal state accordingly.
Spec commit:
07cc0edc79
This is a collection of independent changes in preparation for splices. We group them inside a single PR so we don't have to support multiple intermediate codecs versions:
- store the full funding tx inside `Commitments`
- store the local/remote status for the funding tx inside `Commitments`
- define a `MetaCommitments`, which is a list of `Commitments`
- split `Commitments` between `Params`/`Common`/`Commitment`.
⚠️ This PR includes a new db codec version, and restricts Eclair to `regtest` in order to be able to squash further codec changes before the next release, without having to support intermediate versions.
Co-authored-by: t-bast <bastuc@hotmail.fr>
The postman now has a higher level API to send messages: the user provides the content and the paths to use and the postman takes care of building the message packet and sending it to the next node.
Apparently lnd and cln don't send a payment secret when using keysend,
even when we advertize the feature as mandatory. This is ok though, we
don't actually need a payment secret to protect keysend payments as long
as the sender doesn't use MPP (which is their responsibility, not ours).
Since our codebase requires payment secrets to index payments, we simply
use the preimage as secret when receiving keysend payments.
* Add ChannelOpened event
Emit an event when a channel is opened and ready to process payments.
This event is distinct from the `ChannelCreated` event which is sent
earlier, once we think a funding transaction has been successfully created
(but cannot guarantee when we're not the initiator).
* Add event to websocket
This is a breaking change, but it should be ok since the previous event
wasn't reliable: it was emitted at a time where we couldn't guarantee
that the channel would really confirm.
In preparation for splices, we make `makeFirstCommitTxs` more generic:
- expose the `commitmentIndex`, which was hardcoded to `0`
- take the funding/push amounts arithmetic out
We rename the existing method to `makeCommitTxsWithoutHtlcs` and define a new `makeFirstCommitTxs` with the exact same signature as before, making the change transparent from the outside.
Since we only sign one-input txs, the input index was hardcoded to `0`,
but with splices we will spend previous funding txs, which may have a
change. The change is trivial since we do have all the info to figure
out the output index.
When our peer doesn't contribute to an interactive-tx session, we don't
need to wait for their `tx_signatures`: they will be empty anyway, so we
can create them locally and publish the transaction immediately.
We must *always* lock inputs selected for funding, otherwise locking
wouldn't work at all, as the following scenario highlights:
- we fund a transaction for which we don't lock utxos
- we fund another unrelated transaction for which we lock utxos
- the second transaction ends up using the same utxos as the first one
- but the first transaction confirms, invalidating the second one
This would break the assumptions of the second transaction: its inputs
are locked, so it doesn't expect to potentially be double-spent.
It's particularly dangerous if the node supports zero-conf, as this could
happen to a zero-conf transaction and completely break the channel.
* homogenize WatchFundingConfirmedTriggered handlers
The handling is mostly the same in connected/disconnected states. This
does some refactoring to make them as similar as possible.
NB: we don't make a difference between previous/latest commitment being
confirmed.
* properly factor the pruning of commitments
This is a first glimpse of what will happen when we prune commitments
that have been overriden by a splice.
* prevent duplicate logs when funding tx confirms
At restart, we always put a `WatchFundingConfirmed` on all funding txs,
because this is how we detect force close in state disconnected (we only
put a `WatchSpent` on the funding tx output _after_ we detect that the
funding tx itself is confirmed).
With current code, this would result in a log indicating that the
funding tx has been confirmed at restart, for each channel in
`WAIT_FOR_DUAL_FUNDING_CONFIRMED` state. It can be quite spammy.
Instead, we now only log two cases:
- when we are disconnected, and there were alternative funding txs that
we discard;
- when we are connected (this can only happen once because we will
then transition to the next state).
* factor code for accepting the funding tx
There are 4 code paths:
- single/dual funding
- zero/non-zero conf
Besides the factorization, note that in the dual funding + non-zero conf case, we reverse the calling order between `pruneCommitments` and `acceptDualFundingTx`. This prepares a future change where we store the funding tx inside `Commitments`: it makes more sense then to first update the status of the funding tx in the corresponding commitment, and then prune the remaining commitments.
When using zero-conf, we wait for the transaction to be published before
using a channel. This guarantees that the bitcoind wallet has successfully
recorded the transaction and won't double-spend it.
Otherwise if the transaction fails to publish and the utxos are unlocked,
or if bitcoind is restarted, we may accidentally double-spend it which
would break the channel and be very painful to recover.
Factor and group all `WatchFundingSpentTriggered` handlers in `whenUnhandled`
Special cases (`NEGOTIATING`, `WAIT_FOR_FUTURE_`) are kept outside for readability.
We remove utxos locks that were previously set, otherwise we may have
utxos that stay locked indefinitely. This may happen if we started funding
a channel and restarted before completing the funding flow: the channel
will be safely forgotten but the corresponding utxos stay locked.
The only drawback that this may have is if we have funded and signed a
funding transaction but didn't publish it and we accidentally double-spend
it after a restart. This shouldn't be an issue because:
- locks are automatically removed when the transaction is published anyway
- funding transactions are republished at startup if they aren't in the
blockchain or in the mempool
- funding transactions detect when they are double-spent and abort
the channel creation
- the only case where double-spending a funding transaction causes a loss
of funds is when we accept a 0-conf channel and our peer double-spends it,
but we should never accept 0-conf channels from peers we don't trust
We provide three strategies when locked utxos are detected: stop eclair,
unlock the utxos automatically, or ignore them.
* Set htlc-max-in-flight to 100% by default: it makes it easier to reason
about liquidity in tests, and can be overridden for tests that need a
different value
* Wait for Carol to receive Bob's scid-alias, otherwise blinded routes
will fail to relay
This is just refactoring, there is no feature change. The goal is to later be able to add an upper layer that manages several commitments and acts as a "meta commitments".
This actor spawns only a single `PeerReadyNotifier` actor for each peer that is watched to prevent multiple actors from redundantly polling for when the same peer reconnects.
This actor can be extended to watch for onion messages from async payment receivers, instead of only watching for local peer re-connections.
