When using MPP, if we can't find a route, we need to add an entry to the
DB. Otherwise when users query their payment status, nothing will be
returned which is a bad UX.
Fixes#1512
We were previously only counting the additional HTLC at twice
the current feerate.
An HTLC in isolation doesn't make much sense: the feerate
applies to the whole commit tx. Our fee buffer needs to account
for a x2 feerate increase on the commit tx + an additional htlc.
Note that this introduces another subtlety. The commit tx fee at twice the
current feerate may actually be lower than the commit tx fee at the current
feerate (if the commit tx contains many htlcs that are only slightly above
the trim threshold).
Don't swallow bitcoind exceptions: we wrap them but preserve the
original one.
Allow configuring bitcoin core wallet: it makes sense to allow users
to use a different wallet from the default one.
There's one important caveat: once set, users shouldn't change it while
they have open channels. We mention it clearly in the documentation.
Fixes#1538
This change makes `IntegrationSpec` an abstract class trait that defines
common utilities used in most integration tests, and splits the actual tests
in two separate files that extend that class. Channel tests have themselves
been split between standard and anchor commitments to gain extra
parallelism.
The tests themselves aren't changed at all, except for the TCP ports they
use, and to remove nodes that have become useless in each of the specs. `expectMsg`
have been increased to 60 seconds to account for the increased parallelization.
Some tests have been refactored, cleaned up and block expiries have been reduced.
Allow plugins to register unknown features and message types they're able to handle.
This allows plugins to add new features independently of what eclair-core understands.
Plugins are able to receive and send arbitrary lightning messages, and advertise support
for non-standard features freely.
Change the static_remotekey behavior to use a wallet basepoint only when
using "standard" commitments; with anchor outputs we need to claim this
output (it doesn't directly spend to our wallet) so we must use
key-derivation with our lightning keys.
Correctly claim all outputs in unilateral cases, and add corresponding
test cases.
Anchor output commitments should now work end-to-end (but there's no support
for fee bumping yet).
We were watching actors for ignored watches (WatchLost), and printing
useless logs when ignoring duplicate watches.
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
In some tests we are parsing logs to prevent race conditions. This
change adds more leeway to wait for logging events, because they may be
delayed when we run a lot of tests in parallel.
The replies are always ignored currently anyway. A new trait `NoReplyTo`
has been introduced. Those commands have a particular workflow because
they are persisted in the pending relay db.
* introduce a new relay identifier
In a channel relay, it will be unique for all retries.
In a trampoline relay, it is equal to what previously was the parent
payment id.
* moved and cleaned up remaining relayer tests
Now all relay-related tests are in the `relayer` package.
The only impactful change is that by default on regtest and testnet
fallback fee (used when there is not enough historical data to correctly
estimate the feerate) is now set to 0, whereas it was set to 0.0002 btc
in previous versions.
We set it manually in tests `bitcoin.conf` to preserve the previous behavior.
* typeify response to command messages
* Status.Failure(AddHtlcFailed)->RES_ADD_HTLC_FAILED
`AddHtlcFailed` is renamed `RES_ADD_FAILED` and it is just a particular
kind of `CommandError`
* remove Status.Failure from payment lifecycle
* use replyTo pattern in relayers
* return ForwardMessage in CommandResponse
Instead of having the channel send `Relayer.ForwardMessage` to the
relayer, we encapsulate the `Relayer.ForwardMessage` within a
`CommandResponse[CMD_ADD_HTLC]`.
It looks like a cosmetic change, but it's not: now when the relayer
sends a `CMD_ADD_HTLC` to the channel, it will receive one or more
`CommandResponse[CMD_ADD_HTLC]`, for example:
- success scenario:
- `RES_SUCCESS[CMD_ADD_HTLC]`
- `RES_ADD_COMPLETED[RelayBackward.RelayFulfill]`
- htlc failed by downstream
- `RES_SUCCESS[CMD_ADD_HTLC]`
- `RES_ADD_SETTLED[RelayBackward.RelayFail]`
- command rejected:
- `RES_ADD_FAILED[_]`
- peer disconnected before signing:
- `RES_SUCCESS[CMD_ADD_HTLC]`
- `RES_ADD_FAILED[ChannelUnavailable]`
In addition to that, `RelayMessage` have been slightly refactored, to
better separate between `RelayForward` and `RelayBackward`.
This paves the way for typing `CMD_ADD_HTLC.replyTo` to
`ActorRef[CommandResponse[CMD_ADD_HTLC]]` and have the channel send all
related messages to the `replyTo` actor. Note that the `RelayForward`
actor will always be sent to the relayer, which makes sense since there
was no related command (the htlc was sent by the peer).
NB: `CMD_ADD_HTLC` is a special case, for all other commands there is
exactly one `CommandResponse[CMD_*]`, either `RES_SUCCESS[CMD_*]` or
`RES_FAILURE[CMD_*]`.
* use replyTo pattern in payment lifecycle
We were already close to this pattern with constructs like `case class
WaitingForRoute(sender: ActorRef, ...`
* typeify Origin/Upstream classes
The relationship between `Origin` and `Upstream` was obscure and they were
defined in two different files.
`Upstream` is the source of the payment in the context of a chain
of htlcs. For example, in the case of a typical relayed payment, it
would be an incoming htlc.
`Origin` is the source of the payment in the application: it can be an
actor, or nothing if the reference was lost after a restart.
Instead of using an `Option[ActorRef]` to differentiate between
known/unknown origin, new `Hot/Cold` traits that extend `Origin` have
been introduced. This means that now the `PostRestartHtlcCleaner` only
deals with `Cold` origins, whereas the `NodeRelayer` only handles `Hot`
origins. The channel codec will encode from both `Hot/Cold` origins, but
will only decode to `Cold`.
* refactor response types
Generalize the `CommandResponse[Command]` pattern for all commands.
There seem to be something ambiguous about the way we deal with closing
commands during the initialization phase of the channel. We used to
conflate `CommandResponse[CMD_CLOSE]` and
`ChannelOpenResponse.ChannelClosed` but those are not sent to the same
actors.
It turns out our testing of the `EclairImpl` class is very weak. We
could use this class in `IntegrationSpec` instead of sending raw
messages to channels.
* handle channel-relay in the post-htlc-restart too
The downstream HTLC-timeout integration test was sometimes hanging waiting
for the local commit tx to appear inside the mempool.
The reason was that the remote peer was also trying to get its version of
the commit tx in the mempool, and when it won that race we weren't testing
the right thing.
Simply disconnecting the two nodes fixes the issue as it ensure only the
local node will be broadcasting his commit tx.
Currently, the fundee computes it after receiving the `open_channel`
message, by calling `ChannelVersion.pickChannelVersion`.
Instead, we call this method in the `Peer` and add the resulting channel
version to the initialization parameters.
In the end, the behavior is exactly the same, but:
- it is more consistent with how the funder works;
- it may make sense to compute the channel version a bit earlier in the
process, because we may initiate a different kind of channel actor in
the future for some particular versions (?)
* Correctly handle the case where a tx has both a sequence and locktime set.
* Add tx publish tests to ZmqWatcher and ElectrumWatcher.
* Add documentation on watcher types.
We were previously returning TemporaryNodeFailure for trampoline payments
to neighbour nodes with depleted liquidity. This prevented us from
finding alternative, indirect routes.
In such cases, we now roughly estimate whether the fee is "big enough" to
allow finding alternative routes; if not we ask the sender to raise the fee
before telling them we're lacking liquidity.
A more costly alternative that we may implement in the future would be to
run the path-finding to find a route without bounding the fee, and send
that information back to the sender.
* add replyTo to Register messages
In preparation of the migration to typed actors, we need to remove the
use of `sender`, which doesn't exist in typed actors (and even returns
dead letters when mixing typed/untyped actors).
This also means that, in the tests, we should stop using the
`TestProbe.send()` method, which also relies on the recipient replying
to `sender`. Instead, we should use `targetActor ! msg` which guarantees
that the sender is void.
Using the `replyTo` pattern doesn't mix well with the `ask` pattern,
because we don't know the reference to the temporary actor. To deal with
that, we set `replyTo = ActorRef.noSender` which is a bit hackish.
* don't use `Status.Failure` in register responses
Encapsulating error responses in a `Status.Failure` is convenient when
using the `ask` pattern because those messages are automatically
converted to a failed future.
It does however force us to use exceptions, and make things more
complicated, especially when moving to _typed_ actors.
* add replyTo to Register messages
In preparation of the migration to typed actors, we need to remove the
use of `sender`, which doesn't exist in typed actors (and even returns
dead letters when mixing typed/untyped actors).
This also means that, in the tests, we should stop using the
`TestProbe.send()` method, which also relies on the recipient replying
to `sender`. Instead, we should use `targetActor ! msg` which guarantees
that the sender is void.
Using the `replyTo` pattern doesn't mix well with the `ask` pattern,
because we don't know the reference to the temporary actor. To deal with
that, we set `replyTo = ActorRef.noSender` which is a bit hackish.
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
We don't need one node per force-close scenario, we can use different
channels to the same node which makes the spec simpler.
Force-close tests now have better isolation: they create the channel at
the beginning of the test, and the test ends with that channel closed.
Common parts have been refactored as well, which will make it easier to
add more tests for anchor outputs without duplicating too much code.
These tests have been heavily enriched and refactored; they previously
relied on many unwritten assumptions about event ordering that appeared
as soon as I tried updating them (e.g. to use push_msat to ensure both
sides had an output in the commit tx).
* Type fee rates info
Fixes#1188
* Fix vsize comment
This is an alternative to #1425.
This may not correctly represent what Bitcoin Core does, it's likely that
we can in fact use a value smaller than 253, but this shows how we choose
to err on the side of safety with that calculation.
* Add 1008 feerate block target
Fixes#1486
It can be useful to sign arbitrary messages with the key associated with our node_id (to prove ownership of a node).
Adds 2 new API commands:
eclair-cli signmessage --msg=${message}
eclair-cli verifymessage --msg=${message} --sig=${signature}
We previously refused to relay HTLCs that would result in a low expiry
delta for the next node. However it's not our decision to make, it's the
remote's. We should forward these HTLCs and let the remote decide whether
they fail them because the expiry is too close or fulfill them.
Allow activating anchor outputs and have fully operating channels
during normal operation (open, add/fulfill/fail htlcs, close).
Interop testing has been done with lnd, and there is only one pending
issue during mutual close, where they incorrectly compute the closing
amounts, which they should fix soon.
However, anchor outputs should NOT be activated yet as unilateral
close scenario are not fully handled yet.
We don't do any kind of automatic fee bumping either; this will be done
later, once we have PSBT support and once bitcoind offers the
`psbtbumpfee` RPC (see bitcoin/bitcoin#18654).
* allow to explicitly disable features from configuration
* improved Features.toString
* explicitely disable unlisted features
Co-authored-by: pm47 <pm.padiou@gmail.com>
Watching the blockchain is an asynchronous task, so it is "always late"
and it doesn't matter if we don't synchronously set the watch back when
the node is restarted.
It allows us to smoothen the load if needed at startup, because setting
tens of thousands of `watch-spent` all at once at startup is pretty
expensive.
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
* Activate wumbo by default
This is safe as `max-funding-satoshis` is set to 16777215 sats, which is
the non-wumbo limit.
If users want to increase the maximum channel size, they can update this
configuration value.
* Update default minFinalCltvExpiryDelta
See https://github.com/lightningnetwork/lightning-rfc/pull/785
* Set minFinalCltvExpiryDelta in invoices
Our default fulfill-safety-window is now greater than the spec's default
min-final-expiry-delta in invoices, so we need to explicitly tell payers
what value they must use.
Otherwise we may end up closing channels if a block is produced while we're
waiting for our peer to accept an UpdateFulfillHtlc.
* Introduce transaction commitment format trait
This lets us re-use most of our existing transaction utilities for
anchor outputs.
Channels will always use the default commitment format (current spec),
but we will be able to change that by setting the `ChannelVersion` to
something appropriate for anchor outputs (and later other commitment formats).
* Clean up transaction tests
Remove obsolete claim-htlc tests: they used a different format from what
lightning uses and are redundant with existing tests in TransactionsSpec.
Add missing test cases in TransactionsSpec.
* Implement anchor outputs commitment transaction
Add support for creating an anchor outputs commitment transaction,
without any HTLC.
Support for the new HTLC format will be added in a separate commit.
* Refactor TestVectorsSpec
For anchor outputs, some values will not match the hard-coded ones.
We will instead need to read from the test vector.
* TestVectorsSpec filter unimplemented anchor outputs
Anchor outputs isn't fully implemented yet, so we need to ignore the tests
that are currently not passing.
* Implement anchor outputs HTLC transactions
Add support for HTLC transactions with a 1-block relative delay.
Add missing anchor outputs spec tests.
Add missing tests for some revoked paths.
* Always subtract both anchors from funder amount
For simplicity's sake, we always subtract both anchors from the funder's
main output, even if only one anchor materializes.
Shuffle methods around between ExtendedBitcoinClient and
BitcoinCoreWallet to help readability and separate concerns.
Add some documentation and fix harmless warnings.
Add bitcoin client tests.
* Non blocking bounded mailbox for backup handler
Akka 2.4 introduced a non-blocking unbounded mailbox.
It makes sense to upgrade the backup handler to that mailbox type.
We also add some backup-related metrics.
* Wrap DB calls to record metrics
We record the number of times each operation is executed and its
duration.
When we receive an `UpdateFulfillHtlc` from a downstream channel, it is
critical that we don't lose it because that is what allows us to pull
funds from the corresponding upstream channel(s). But this(ese) upstream
channel(s) may very well be currently OFFLINE and unable to update the
commitment right away, so we need to remember it for later. Same applies
to an `UpdateFailHtlc` (although it is less critical), because we don't
want the upstream htlc to timeout and cause a channel to force-close.
We were previously relying on a `CommandBuffer` actor, that uses a
"pending relay" database to store commands. Once the command is processed
by the target channel, it sends back an acknowledgment to the
`CommandBuffer`, which then removes the command from the database.
Unacknowledged commands are replayed at each reconnection or app restart.
This works well, but the flow is a little cumbersome and not easy to
understand.
With this PR, the sender (channel, payment handler, ...) is responsible for
storing commands to the pending relay db, instead of the command buffer,
which is completely removed. The target channel will acknowledge the
message and remove it from the pending relay db.
In the end, the logic is the same as before, we have just dropped the
intermediate `CommandBuffer`.
* Split feerate mismatch configuration
We want to be much stricter with feerates that are below our estimation
than feerates that are above it.
This also makes this configuration parameter easier to understand
for end users.
* Tolerate feerate mismatch while channel is unused
We can relax the conditions where we close a channel because of a feerate
mismatch: when the channel has no pending HTLCs, it's ok to temporarily
disagree on the feerate.
While we disagree on feerates, we don't use this channel to offer outgoing
HTLCs. If we receive an incoming HTLC, we have to close the channel because
that HTLC would be at risk (incorrect feerate).
This mechanism gives us time to adapt to feerate changes, hopefully reducing
the amount of unnecessary channel closures.
The channelstats API only returns results for the *outgoing* channels
used when relaying. We must also include the *incoming* channels, otherwise
it looks like they're inactive which doesn't reflect their real usage.
Fixes#1465
- Test was not executed (because the "tests" variable was an iterator that was emptied by the call to .size())
- HTLC regex had to be updated to skip over the HTLC number that was added to the reference test vectors
* Add metric to track onion payload format
This will be useful to decide when we can safely phase out support for
the legacy format.
* Add metric to track htlcs in flight
We track both the number of HTLCs and their amounts.
We track this at the channel level and globally.
If all trampoline retries fail, we should convert the error to a route
not found. We tried multiple fee targets and none of those was enough to
allow the trampoline node to find a satisfying route.
