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.

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

Remove support for initial_routing_sync #1683

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Feb 4, 2021

We keep the GetRoutingState API available in the router as it's useful to query network information locally (or between actors), but we stop sending that data to remote nodes.

I've checked the last week on our node, and there were no queries using initial_routing_sync, so it feels ok to drop support.

@t-bast t-bast marked this pull request as draft February 4, 2021 08:37
@codecov-io
Copy link

codecov-io commented Feb 4, 2021

Codecov Report

Merging #1683 (02bfe3f) into master (ac054a2) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1683      +/-   ##
==========================================
+ Coverage   85.95%   85.99%   +0.04%     
==========================================
  Files         151      151              
  Lines       11457    11455       -2     
  Branches      484      500      +16     
==========================================
+ Hits         9848     9851       +3     
+ Misses       1609     1604       -5     
Impacted Files Coverage Δ
...main/scala/fr/acinq/eclair/io/PeerConnection.scala 82.97% <ø> (+1.04%) ⬆️
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala 92.04% <100.00%> (+0.04%) ⬆️
...nq/eclair/blockchain/electrum/ElectrumWallet.scala 80.25% <0.00%> (-0.26%) ⬇️
.../acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala 99.27% <0.00%> (+0.72%) ⬆️
...q/eclair/blockchain/electrum/ElectrumWatcher.scala 76.98% <0.00%> (+1.58%) ⬆️

We keep the GetRoutingState API available in the router as it's useful to
query network information locally (or between actors), but we stop sending
that data to remote nodes.
@t-bast t-bast force-pushed the remove-initial-routing-sync branch from 5d96056 to 02bfe3f Compare February 4, 2021 14:38
@t-bast t-bast marked this pull request as ready for review February 4, 2021 14:38
@t-bast t-bast requested a review from pm47 February 4, 2021 14:38
@pm47 pm47 changed the title Remove initial routing sync Remove support for initial_routing_sync Feb 4, 2021
@pm47
Copy link
Member

pm47 commented Feb 4, 2021

cc @Roasbeef @rustyrussell

@t-bast t-bast merged commit f241ef9 into master Feb 4, 2021
@t-bast t-bast deleted the remove-initial-routing-sync branch February 4, 2021 15:53
@Roasbeef
Copy link

Roasbeef commented Feb 5, 2021

Cool, we don't actively use it on the lnd end, and have a flag that lets users ignore the message all together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants