Fixed regression caused by 2c1811d: we now don't force sending a
channel_update at the same time with channel_announcement.
This greatly simplifies the rebroadcast logic, and is what caused the
integration test to fail.
Added proper test on Peer, testing the actor, not only static methods.
Currently we don't remember channels that we have pruned, so we will happily revalidate the same channels again if a node re-sends them to us, and prune them again, a.k.a. the "zombie churn".
Before channel queries, rejecting a stale channel without validating it wasn't trivial, because nodes sent us the `channel_announcement` before `channel_update`s, and only after receiving the `channel_update` could we know if the channel was still stale. Since we had no way of requesting the `channel_announcement` for one particular channel, we would have to buffer it, which would lead to potential DOS issues.
But now that we have channel queries, we can now be much more efficient. Process goes like this:
(1) channel x is detected as stale gets pruned, and its id is added to the pruned db
(2) later on we receive a `channel_announcement` from Eve, we ignore it because the channel is in the pruned db
(3) we also receive old `channel_update`s from Eve nodes, just ignore them because we don't know the channel
(4) then one day some other node George sends us the `channel_announcement`, we still ignore it because the channel is still in the pruned db
(5) but then George sends us recent `channel_update`s, and we know that the channel is back from the dead. We ignore those `channel_update`s, but we aldo remove the channel id from the pruned db, and we request announcements for that node from George
(6) George sends us the `channel_announcement` again, we validate it
(7) George then sends us the `channel_update`s again, we process them
(8) done!
This also allows removing the pruning code that we were doing on-the-fly when answering to routing table sync requests.
Needless to say that this leads to a huge reduction in CPU/bandwidth usage on well-connected nodes.
Fixes#623, #624.
Nodes currently receive tons of bogus channel_announcements, mainly with unexisting or long-spent funding transactions. Of course those don't pass the validation and are rejected, but that takes a significant amount of resources: bandwidth, multiple calls to bitcoind, etc.
On top of that, we forget those announcements as soon as we have rejected them, and will happily revalidate them next time we receive them. As a result, a typical node on mainnet will validate 10s of thousands of useless announcements every day.
As far as we know, this is apparently due to bug in another implementation, but that may very well be used as a DOS attack vector in the future.
This PR adds a simple mechanism to react to misbehaving peer and handle three types of misbehaviors:
(a) bad announcement sigs: that is a serious offense, for now we just close the connection, but in the future we will ban the peer for that kind of things (the same way bitcoin core does)
(b) funding tx already spent: peer send us channel_announcement, but the channel has been closed (funding tx already spent); if we receive too many of those, we will ignore future announcements from this peer for a given time
(c) same as (b), but the channel doesn't even exist (funding tx not found). That may be due to reorgs on testnet.
Needless to say that this leads to a huge reduction in CPU/bandwidth usage on well-connected nodes.
* Improve startup error handling
* minor changes to ZMQACtor
Scheduler now sends messages instead of directly calling our checkXX methods.
It will work the same but should fix the NPE we sometimes get when we stop the app.
* Correctly handle multiple channel_range_replies
The scheme we use to keep tracks of channel queries with each peer would forget about
missing data when several channel_range_replies are sent back for a single channel_range_queries.
* RoutingSync: remove peer entry properly
* Remove peer entry on our sync map only when we've received a `reply_short_channel_ids_end` message.
* Make routing sync test more explicit
* Routing Sync: rename Sync.count to Sync.totalMissingCount
When we just signed an outgoing htlc, it is only present in the next
remote commit (which will become the remote commit once the current one
is revoked).
If we unilaterally close the channel, and our commitment is confirmed,
then the htlc will never reach the chain, it has been "overriden" and
should be failed ASAP. This is correctly handled since
6d5ec8c4fa.
But if remote unilaterally close the channel with its *current*
commitment (that doesn't contain the htlc), then the same thing happens:
the htlc is also "overriden", and we should fail it.
This fixes#691.
* permissive codec for failure messages
Some implementations were including/ommitting the message type when
including a `channel_update` in failure messages.
We now use a codec that supports both versions for decoding, and
will encode *with* the message type.
* make router handle updates from failure messages
This is a regression caused by 9f708acf04,
which made the `Peer` class encapsulates network announcements inside
`PeerRoutingMessage`, in order to preserve the origin of the messages.
But when we get channel updates as part of failure messages when making
payments, they weren't encapsulated and were ignored by the router.
Re-enabled a non-regression test in `IntegrationSpec` which will prevent
this from happening in the future.
* improve handling of `Update` payment failures
Exclude nodes that send us multiple `Update` failures for the same
payment (they may be bad actors)
There are two different expiry checks:
(a) relative checks: when relaying an htlc, we need to be sure that
the difference of expiries between the outgoing htlc and the incoming
htlc is equal to the `cltv_expiry_delta` that we advertise in the
`channel_update` for the outgoing channel;
(b) absolute checks: we need to make sure that those values are not too
early or too far compared to the current "blockchain time".
The check for (a) needs to be done in the `Relayer`, which is the case
currently. This means that we will check the expiry delta *after* having
signed the incoming htlc, and we will fail the htlc (not the channel) if
the delta is incorrect.
The check for (b) was done in the `Commitments.receiveAdd` method. This
seems to make sense, because we would want to make sure as early as
possible that an incoming htlc has correct expiries, but it is actually
incorrect: the spec tells us to accept (=cross-sign) the htlc, and only
then to fail it before relaying it to the next node.
Indeed there is no risk in accepting an htlc that has an expiry in the
past, or an expiry very far in the future, because this only affects the
counterparty's balance. We just don't want to sign that kind of outgoing
htlcs.
Moving the check to the `sendAdd` will result in an error being return
to the relayer, which will then fail the corresponding incoming htlc.
* removed max body size in http client
This is required because since f3676c6497
we retrieve multiple full blocks in parallel.
* trivial: removed unused code
* trivial: added log
* trivial: more unused code removal
* Add a "smooth" fee provider
It will return the moving average of fee estimates provided by
its internal fee provider, within a user defined window that can
be set with eclair.smooth-feerate-window.
* Use a default fee rate smoothing window of 3
This should smooth our fee rate when there is a sudden change in
onchain fees and prevent channels with c-lightning node from getting
closed because they disagree with our fee rate.
* ignore answers to CMD_UPDATE_FEE
* ignore BITCOIN_FUNDING_DEPTHOK in OFFLINE/SYNCING
* ignore BITCOIN_FUNDING_DEEPLYBURIED in OFFLINE/SYNCING
* improvements in eclair-cli:
- Made default `channel`/`channels` outputs more useful.
- Added support for timestamp filtering to `audit` and `networkfees`.
* added `ChannelPersisted` event
Our recovery logic didn't handle the case were our local commit is
up-to-date, but we don't know their local commit (probably because we
just lost the last state were we sent them a new `commit_sig`).
Also, process all cases in the same `channel_reestablish` handler, like we do
everywhere else.
Moved the sync tests in `Helpers` so that they are more understandable.
* get full blocks when looking for spending tx
With a verbosity of `0`, `getblock` returns the raw serialized
block. It saves us from calling `getrawtransaction` for each transaction
in the block.
Fixes#664.
This can happen in an unilateral close scenario, when local commit
"wins" the race to the blockchain, and some outgoing htlcs weren't yet
signed by remote.
This fixes#649.
Added a new `AuditDb` which keeps tracks of:
- every single payment (received/sent/relayed)
- every single network fee paid to the miners (funding, closing, and all commit/htlc transactions)
Note that network fees are considered paid when the corresponding tx has reached `min_depth`, it makes sense and allows us to compute the fee in one single place in the `CLOSING` handler. There is an exception for the funding tx, for which we consider the fee paid when the tx has successfully been published to the network. It simplifies the implementation and the tradeoff seems acceptable.
Three new functions have been added to the json-rpc api:
- `audit`: returns all individual payments, with optional filtering on timestamp.
- `networkfees`: returns every single fee paid to the miners, by type (`funding`, `mutual`, `revoked-commit`, etc.) and by channel, with optional filtering on timestamp.
- `channelstats`: maybe the most useful method; it returns a number of information per channel, including the `relayFee` (earned) and the `networkFee` (paid).
The `channels` method now returns details information about channels. It makes it far easier to compute aggregate information about channels using the command line.
Also added a new `ChannelFailed` event that allows e.g. the mobile app to know why a channel got closed.
This allows e.g. the mobile app to know why a channel got closed.
Depending on whether the error is local or remote, a
`Throwable`/`wire.Error` will be attached to the event.
This allows for a user of the library to implicitly pass the `ActorSystem` to the eclair node. Although if you are running multiple eclair instances on the same machine you need to make sure the `ActorSystems` that are passed implicitly are unique.
* Implement new 'routing sync' messages
* add a new feature bit for channel queries
when we receive their init message and check their features:
- if they set `initial_routing_sync` and `channel_range_queries` we do nothing, we should receive a
range query shorly
- if they support channel range queries we send a range query
* Modify query_short_channel_id to ask for a range of ids
And not just a single id
* use `SortedMap` to store channel announcements
* don't send prune channels with channel range queries
* update range queries type to match BOLT PR
* add timestamp-based filters for gossip messages
each peer can speficy a `timestamp range` to filter gossip messages against.
* don't preserve order in `decodeShortChannelIds`
It is not needed and allows us to return a `Set`, which is better suited
to how we use the result.
* channel range queries: handle multi-message responses
Handle case where there are too many short ids to fit in a single message.
* channel range queries: use zlib instead of gzip
but detect when a message was encoded with gzip and reply with gzip in that case.
* router: add more channel range queries logs
* Channel range queries: correctly set firstBlockNum and numberOfBlocks fields
* channel range queries: properly handle case where there is no data
we will just receive on byte (compression format)
* channel range queries: use their compression format to query channels
when we query channels with `query_short_channel_ids`, we now use the same compression
format as in their `repy_channel_range` message. So we should be able to communicate
with peers that have not implemented all compression formats.
* router: make sure that channel ids are sorted
For channel range queries to work properly, channel ids need to be sorted.
It is then much more efficient to use a sorted map in our router state.
* always use `keySet` instead of `keys`
`SortedMap`.`keySet` returns a `SortedSet`, whereas `SortedMap`.`keys`
returns an `Iterable`. This is a critical difference because channel
range queries has requirements on ordering of channel ids.
Using the former allows us to rely on type guarantees instead of on
assumptions that the `Iterable` is sorted in `ChannelRangeQueries`.
There is no cost difference as internally the `Iterator` is actually a
`SortedSet`.
Also, explicitely specified the type instead of relying on comments in
`Router`.
* publish channel update event on router startup
* channel range queries: use uint32 for 4-byte integers (and not int32)
* channel range queries: make sure we send at least one reply to `query_channel_range`
reply to `query_channel_range` messages for which we have no matching channel ids
with a single `reply_channel_range` that contains no channel ids.
* channel range queries: handle `query_channel_range` cleanly
add an explicit test when we have no matching channel ids and send back a reply with an
empty (uncompressed) channel ids payload
* channel range queries: rename GossipTimeRange to GossipTimestampFilter
* channel range queries: add gossip filtering test
* peer: forward all routing messages to the router
and not just channel range queries. this should not break anything and if
it does it would reveal a problem
* peer: add remote node id to messages sent to the router
this will improve logging and debugging and will help if we implement
banning strategies for mis-behaving peers
* router: filter messages with a wrong chainHash more cleanly
* channel range queries: set a "pass-all" timestamp filter
* router: remove useless pattern match
ChannelUpdates are wapped in a PeerRoutingMessage now
* Peer: fit typo in comment
* Peer: optimize our timestamp filter
* Router: use mdc to log remote node id when possible
* fix typos and improve log message formatting
* Peer: rephrase scala doc comment that breaks travis
* Peer: improve timestamp filtering + minor fixes
* Electrum tests: properly stop actor system at the end of the test
* Peer: filter out node announcements against our peer's timestamp
But we don't prune node annoucements for which we don't have a matching
channel in the same "rebroadcast" message
* relay htlcs to channels with the highest balance
In order to reduce unnecessary back-and-forth in case an outgoing
channel doesn't have enough capacity but another one has, the relayer
can now forward a payment to a different channel that the one specified
in the onion (to the same node of course).
If this preferred channel returns an error, then we will retry to the original
requested channel, this way if it fails again, the sender will always receive
an error for the channel she requested.
* improved logs on sig sent/received
* put 'sent announcements' log in debug
* added logging of IN/OUT wire messages
* added mdc support to IO classes
* reduced package length to 24 chars in logs
* add basic electrum wallet test
our wallet connects to a dockerized electrumx server
* electrum: clean up tests, and add watcher docker tests
* electrum wallet: fix balance computation issue
when different keys produced the exact same confirmed + unconfirmed balances, we
would compute an invalid balance because these duplicates would be pruned.
* electrum: rename wallet test
* electrum: add a specific test with identical outputs
* electrum: change scripthash balance logging level to debug
* electrum: make docker tests run on windows/mac
Our electrumx docker container needs to contains to bitcoind that is running on the host.
On linux we use the host network mode, which is not available on windows/osx
On windows/osx we use host.docker.internal, which is not available on linux. This
requires docker 18.03 or higher.
Our electrumx docker container needs to contains to bitcoind that
is running on the host.
On linux we use the host network mode, which is not available on windows/osx
On windows/osx we use host.docker.internal, which is not available on linux. This
requires docker 18.03 or higher.
electrum: change scripthash balance logging level to debug
electrum: add a specific test with identical outputs
electrum: rename wallet test
electrum wallet: fix balance computation issue
when different keys produced the exact same confirmed + unconfirmed balances, we
would compute an invalid balance because these duplicates would be pruned.
electrum: clean up tests, and add watcher docker tests
add basic electrum wallet test
our wallet connects to a dockerized electrumx server
* `ReceivePayment` now accepts additional routing info, which is useful for nodes that are not announced on the network but still want to receive funds.
* Fix scala.NotImplementedError when public-ips config parameter contains invalid values
* more comprehensive validations
* fix unit tests
This fixes#630
We should return a `FeeInsufficient` error when an incoming htlc doesn't
pay us what we require in our latest `channel_update`.
Note that the spec encourages us to being a bit more lax than that (BOLT
7):
> SHOULD accept HTLCs that pay an older fee, for some reasonable time
after sending channel_update.
> Note: this allows for any propagation delay.
* add api call to update channel relay fees
* fixed bug in GUI, as channel can had different fees in each direction!
* fire transitions on `TickRefreshChannelUpdate` (fixes#621)
* make router publish `channel_update`s on startup
* (gui) Channel info fees are now options and case where channels have no known fees data is now properly handled.
* rename InvalidDustLimit to DustLimitTooSmall
* make sure that our reserve is above our dust limit
* check that their accept message is valid
see BOLT 2:
- their channel reserve must be above their dust limit
- their channel reserve must be above our dust limit
- their dust limit must be below our reserve
* channel: check to_local and to_remote amounts againt channel_reserve_satoshis
see BOLT 2: The receiving node MUST fail the channel if both to_local and to_remote amounts for
the initial commitment transaction are less than or equal to channel_reserve_satoshis (see BOLT 3).
* channel: check that their open.max_accepted_htlcs is valid
* Added server address in ElectrumReady object
* Assigned remote address to variable to improve readability
* Checking that the master address exists in the addresses map
