Eclair batches bitcoind request by default for efficiency.
However this can create latency and reliability issues when
running bitcoind on a remote machine over an unreliable
network. In those cases it's better to opt-out of batching.
We also make the threshold of missing blocks for blockchain
watchdog configurable instead of a hard-coded value of 6.
When a channel is offline and we want to relay an HTLC through it, we emit
a new `channel_update` with the disable bit set (unless it was already
disabled).
We previously didn't persist our internal state with this new
`channel_update`, which created the following issue: if eclair is restarted
before the channel comes back online, eclair would only try to rebroadcast
the previous `channel_update` which has a lower timestamp than the one
disabling the channel. That `channel_update` is then rejected by the
network, causing the channel to stay disabled until the next refresh,
which only happens after 10 days.
* use zero-args method to skip fixture
This is the proper way to skip building the fixture. Using `_` actually
creates a fixture and then ignores it.
* consistency nit
* use list/map instead for test cases
* add a custom reporter to highlight slowest tests
When finding a payment route (typically for routing a trampoline payment) we already log the recipient node id, however it can be a private node hidden behind a routing hint in which case it is more useful to log the public node from the routing hint.
This is added to the `audit.path_finding_metrics` and we also add the `payment_hash` so that it can be joined with the `audit.relayed` and `audit.relayed_trampoline` tables.
* Remove close() in db interfaces
It shouldn't be the responsibility of individual db classes to close
the underlying db connection because they typically share the same db
instance (postgres) or db files (sqlite). Closing should be handled in
the `Databases` level (which is already the case for postgres.
For sqlite, closing was only useful for mobile apps, which now use
lightning-kmp.
Also removed `DbFeeProvider`, which was only used by mobile apps.
* increase github ci build time 20min->30min
This PR does two main things:
- introduce a new `FixtureSpec` base class for tests that involve a fixture. See the scaladoc for more info.
- add new simple integration tests in package `integration.basic`. They are based on `MinimalNodeFixture`, which is a full setup for a node with real actors, except the bitcoin part (watcher/wallet) which is mocked. They are much lighter than our previous integration tests, which allow us to keep each test individual, as opposed to having all tests of the same suite depend on each other. We can define more complex fixtures with any number of nodes.
Other minor improvements:
- update scalatest version
- simplify `ChannelStateTestsHelperMethods`
- replace all === by ==
Triple equals allows customizing the equality method, but we never use
that.
Detailed errors (which is what we were looking for) are provided by
mixing in scalatest's `Assertions` and using the regular `assert(x==y)`
syntax. The `Assertions` trait is already mixed in by default in all
scalatest suites.
Even though we use our anchor to CPFP our commit tx, the commit tx could
confirm on its own. If that happens, we don't care about the anchor tx so
we shouldn't RBF it. Ideally we'd like to cancel it but that's impossible,
so we just wait for it to either confirm or drop out of mempools after a
while.
When running behind a load balancer that doesn't preserve the client source
IP address, it doesn't make sense to include the `remote-address` tlv in
our `init` message, it will contain an IP address that is completely
unrelated to our peer.
Fixes#2256
This gives more time to rollback the funding tx in case where we moved
from `WAIT_FOR_FUNDING_INTERNAL`->`CLOSED`.
We could do it differently, with an additional step, or by only sending
the "shutdown" symbol after we receive the wallet response for this
specific transition, but it would be more complicated.
This is the final preparatory step before we can introduce channel aliases, which are arbitrary values of type `ShortChannelId`. We had to move to a stable identifier for private channels. Public channels can keep using the `shortChannelId`, it is stable because it is a "real scid" (and only used after 6 confs) and is useful as an index for channel queries.
This is a preparation for https://github.com/lightning/bolts/pull/910.
The relaying logic is now based on the `channelId` instead of
`shortChannelId`.
We used to consider `shortChannelId` as a unique identifier for the
channel, but it's not true anymore: there will be multiple aliases per
channel.
This is easier to use than having to decide which params we should look
into (local or remote). It will also be easier to integrate with dual funding.
Rename initialFeeratePerKw: this name was very confusing.
This feerate only applies to the commit tx, so we make that explicit.
In this PR we add a basic integration test between `Channel` and `Router` that checks the proper handling of local announcements.
* disable router rebroadcast in tests
* use separate ActorSystem for alice and bob in tests
* add a simple channel-router integration test
* fix bug found in new channel-router test
The new test was failing, due to a bug. When a local
channel graduates from private to public, we do not copy existing
known `channel_update`s. Current implementation guarantees that we
will process our local `channel_update` immediately, but what about our
peer?
* fix rebroadcast for local announcements
We fix a second bug where gossip origin wasn't properly set for local
announcements
* increase ram for tests to 2G
* improve debuggability of integration tests
In tests we can sleep for at least X seconds but because of other tests running in parallel, we can't get an upper bound on the sleeping time. This makes some tests that depend on time unfeasible.
Fixes#2175#2276
From scalatest's `ParallelTestExecution` doc:
> ScalaTest's normal approach for running suites of tests in parallel is to run different suites in parallel, but the tests of any one suite sequentially.
> [...]
> To make it easier for users to write tests that run in parallel, this trait runs each test in its own instance of the class. Running each test in its own instance enables tests to use the same instance vars and mutable objects referenced from instance variables without needing to synchronize. Although ScalaTest provides functional approaches to factoring out common test code that can help avoid such issues, running each test in its own instance is an insurance policy that makes running tests in parallel easier and less error prone.
This means that for each single test of the `channel.states` package, we instantiate one actor system, which contains two thread pools. With default settings, that's a minimum of 2*8 threads per individual test.
That's already pretty bad, and with 65cf238 (#2270) we add a factor of 3 on top of that, which makes us go past the OS limits on github CI.
setup | peak thread count | run time
-------|---------------------|----
baseline | 5447 | 5m 44s
sequential | 1927 | 5m 9s (*)
(*) It's actually so bad, that tests run actually faster without parallelization!
Our mechanism to detect double-spending wasn't correctly taking into
account unconfirmed inputs. This was only used in single-funder scenarios
so it could only be an issue in rare edge cases, where it would not lead
to any loss of funds as we keep commit tx data in our DB even for closed
channels.
There was a race condition when testing fee-bumping: we could emit the new
block event before the underlying actor registered to this event type.
This is fixed by emitting the `TransactionPublished` event after registering
to the new block events, and waiting for that in tests before publishing a
new block event.
We have two sources for channel routing parameters:
- channel updates
- routing hints in invoices
Instead of generating fake `channel_update`s from routing hints, we
define a `ChannelRelayParams` that can be built from announcements
or routing hints.
This is cleaner but also is a first step to decorrelate the identifier
of a channel in our graph, from whatever identifies the source.
While zlib provides good compression results for gossip, it's a dependency
that had a few important CVEs in the past. Some implementations are
reluctant to import it, so we decided to remove it from the specification
in https://github.com/lightning/bolts/pull/981
We stop compressing with zlib and only send uncompressed results, while
still supporting receiving compressed data. We will remove support for
decompression once our monitoring indicates that we stopped receiving
compressed data.
We also reduce the maximum allowed chunk size.
The previous calculation was wrong and could lead to messages bigger than 65kB.
Dijkstra's algorithm assumes that each edge has a fixed weight that does not depend on the rest of the path. In our case this assumption is broken when computing a cost for failed payments as it depends on all the edges of the path. We fix this by adding the possibility to use log probabilities which can be added per edge and offer a good approximation as long as the probabilities stay close to 1.
Edges still don't have a perfectly fixed weight because the fee of an edge depends on the fees of the previous edges, however since fees are typically low, it shouldn't matter in practice.
It was creating a race condition in the test
`MultiPartHandlerSpec`.`PaymentHandler should reject incoming
multi-part payment with an invalid expiry` due the 0-expiry
invoice being immediately purged.
In path-finding, we give a better score to channels with higher capacity because they have a higher change to have enough balance to relay the payment. But for local channels we know the balance so we can be sure that the payment can be relayed.
Add dual funding feature bit, but keep it disabled for now.
Add dual funding protocol messages and codecs.
We don't actually handle these messages yet.
When we receive them, they will simply be ignored and log a warning.
* Add a "stop" API method
This API call was added for certain uses cases where killing the process was impractical but internally it just calls `sys.exit()`.
Eclair is designed to shutdown cleanly when its process is killed and this is still the recommended way of stopping it.
When Phoenix was based on a branch of eclair, we needed a workaround for
invoice backwards-compatibility. Now that Phoenix uses lightning-kmp
instead of eclair, we can remove that work-around.
* Check that channel seed has not been modified
On startup, we check that for each of our channels the public key derived from the channel seed matches the key that was
used when the channel was funded.
* Rename DBCompatChecker to DBChecker and move channel seed check there
* Rename HasCommitments
* Introduce a TransientChannelData trait, which lets us have complete pattern
matching on channel data
* Remove channel data from the ChannelErrorOccured event
* A few fixes in Channel.scala
* Refactor channel transition handlers
The `option_scid_alias` feature (https://github.com/lightning/bolts/pull/910)
is going to clash with the feature bit we use for trampoline payments.
Fortunately we've never advertized it, so we can simply update it to a much
higher value for now (until the final version of trampoline is specified).
We use a version of bitcoin-lib that uses bitcon-kmp under the hood.
Its API is the same as before, but its package is now fr.acinq.bitcoin.scalacompat
The `channelbalances` API call retrieves information about the balances of all local channels, not just those with usable outgoing balances that are enabled for sending.
This change also adds the `isEnabled` attribute to the json results for both the new `channelbalances` and old `usablebalances` API calls.
Refactor the channel state machine into several separate traits.
This lets us split its logic into multiple files while keeping a single actor.
It also lets us isolate some parts of the state machines as if they were
small state machines: we apply this to the channel opening flow which
will let us add a new flow for dual funding.
This can also be applied to the closing mechanism in the future.
When using multipart payments, the path-finding request sent to the router does not match the actual payment, it is only a request to route a fraction of the amount but the fee budget is still the one for the full payment. As a consequence the router will return routes that will not satisfy the fee budget.
The fee budget is checked one last time before relaying the payment but ignoring the fee of the first channel (which should not be ignored for trampoline relays). I fix this last check.
