Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add blockchain watchdog #1545

Merged
merged 6 commits into from
Nov 3, 2020
Merged

Add blockchain watchdog #1545

merged 6 commits into from
Nov 3, 2020

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Sep 30, 2020 •
edited
Loading

Introduce a blockchain watchdog, that fetches headers from multiple sources in order to detect whether we're being eclipsed.

The current blockchain sources we use are:

  • blockchainheaders.net (headers over DNS)
  • blockstream.info
  • mempool.space
  • blockcypher.com

We can add more sources later if needed.

@t-bast t-bast force-pushed the secondary-blockchain-sources branch from 4a7034d to e848b1d Compare October 2, 2020 07:40
@t-bast
Copy link
Member Author

t-bast commented Oct 2, 2020

I'm not sure how I should instantiate my top-level actor (BlockchainWatchdog).
It looks like the simplest way is to do it as child of an existing actor (does it make sense as child of ZmqWatcher?) otherwise I think I need to create a full akka typed ActorSystem in Setup.scala.

@pm47
Copy link
Member

pm47 commented Oct 2, 2020

I'm not sure how I should instantiate my top-level actor (BlockchainWatchdog).
It looks like the simplest way is to do it as child of an existing actor (does it make sense as child of ZmqWatcher?) otherwise I think I need to create a full akka typed ActorSystem in Setup.scala.

I believe a child of ZmqWatcher would make sense. Or maybe could create a MultiSourceWatcher that would itself be a watcher, and would instantiate a BlockchainWatchdog and ZmqWatcher?

@t-bast
Add blockchain watchdog
e509818 
Introduce a blockchain watchdog that fetches headers from multiple sources
in order to detect whether we're being eclipsed.

Use blockchainheaders.net and blockstream.info as first sources of blockchain
data. More blockchain sources will be added in future commits.
@t-bast t-bast force-pushed the secondary-blockchain-sources branch from 3b3218d to e509818 Compare October 9, 2020 11:05
@codecov-io
Copy link

codecov-io commented Oct 9, 2020 •
edited
Loading

Codecov Report

Merging #1545 into master will decrease coverage by 0.02%.
The diff coverage is 87.85%.

@@            Coverage Diff             @@
##           master    #1545      +/-   ##
==========================================
- Coverage   87.28%   87.25%   -0.03%     
==========================================
  Files         139      144       +5     
  Lines       10916    11034     +118     
  Branches      474      451      -23     
==========================================
+ Hits         9528     9628     +100     
- Misses       1388     1406      +18
Impacted Files Coverage Δ
...q/eclair/blockchain/watchdogs/HeadersOverDns.scala 82.60% <82.60%> (ø)
...cinq/eclair/blockchain/watchdogs/ExplorerApi.scala 88.57% <88.57%> (ø)
...lair/blockchain/watchdogs/BlockchainWatchdog.scala 95.00% <95.00%> (ø)
...ir-core/src/main/scala/fr/acinq/eclair/Setup.scala 70.81% <100.00%> (ø)
.../acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala 99.12% <100.00%> (+<0.01%) ⬆️
...acinq/eclair/blockchain/watchdogs/Monitoring.scala 100.00% <100.00%> (ø)
...clair/blockchain/electrum/ElectrumClientPool.scala 78.49% <0.00%> (-4.31%) ⬇️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala 93.70% <0.00%> (-1.58%) ⬇️
.../fr/acinq/eclair/transactions/CommitmentSpec.scala 82.92% <0.00%> (-0.41%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 85.88% <0.00%> (-0.16%) ⬇️
... and 18 more

@t-bast
Add blockcypher API
35efb86 
This is another blockchain explorer that can be used as a secondary source.
@t-bast
Refactor explorer API watchdogs
01ae1d9 
Use the same actor for all explorers we'll want to support.
Each explorer watchdog simply implements the API calls.

Fix the `BlockchainWatchdogSpec`: there was a race condition where we may
send `CurrentBlockCount` to the `eventStream` before the actor under test was
registered to these events.
@t-bast t-bast marked this pull request as ready for review October 13, 2020 13:16
@t-bast t-bast requested a review from pm47 October 13, 2020 13:16
@pm47
Copy link
Member

pm47 commented Oct 22, 2020

You should probably add https://mempool.space/api.

pm47
pm47 reviewed Oct 22, 2020
View reviewed changes
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that instead of looking for a blockCountDelta in the future, we should simply compare tips. In the case of bitcoinheaders.net which doesn't have this feature, we can look for n + 10 blocks. Maybe we are even more late than that but it's enough to raise the alarm.

@t-bast
Rework watchdogs to start from blockchain tip
f44c46c
@t-bast
Add mempool.space
031a6b2 
This is an esplora-based API which makes it very easy.
@t-bast t-bast requested a review from pm47 October 23, 2020 12:02
pm47
pm47 previously approved these changes Nov 3, 2020
View reviewed changes
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just suggestions, but we can go with this version if you want.

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDns.scala Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDns.scala Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDns.scala Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDns.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/blockchain/watchdogs/ExplorerApi.scala Show resolved Hide resolved
@t-bast t-bast merged commit c556654 into master Nov 3, 2020
@t-bast t-bast deleted the secondary-blockchain-sources branch November 3, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pm47 pm47 pm47 approved these changes

Assignees
No one assigned
Labels
Optech Make Me Famous!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@t-bast @pm47 @codecov-io