discovery: deterministic bootstrapping for local test networks#10003
discovery: deterministic bootstrapping for local test networks#10003ellemouton merged 4 commits intolightningnetwork:masterfrom
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
aa4ed86 to
096fc03
Compare
096fc03 to
3f94a37
Compare
| // seeds. | ||
| if !s.cfg.Bitcoin.IsLocalNetwork() { | ||
| //nolint:ll | ||
| dnsSeeds, ok := chainreg.ChainDNSSeeds[*s.cfg.ActiveNetParams.GenesisHash] |
There was a problem hiding this comment.
I wonder if we update the DNSSeeds. I checked and it seems like dig ln.signet.secp.tech is down ? Not part of this PR but still wonder if you should keep those DNS seeds monitored.
There was a problem hiding this comment.
probs something we should check occasionally yeah
There was a problem hiding this comment.
That was my domain at some point. But a very long time ago 😅
We now have a proper seed though: https://github.com/lightningnetwork/lnd/blob/master/chainreg/chainregistry.go#L903
It returns true for Signet and Regtest networks.
It should be ok to use IsLocalNetwork here since the chainreg.ChainDNSSeeds map does not have an entry for either simnet or regtest.
Create an abstract hashAccumulator interface for the Channel graph bootstrapper so that we can later introduce a deterministic accumulator to be used during testing.
If LND is running on a local network, then use deterministic sampling so that we can have deterministic peer bootstrapping.
3f94a37 to
37d6390
Compare
ellemouton
left a comment
There was a problem hiding this comment.
thanks for the reviews yall 🙏 updated
| // seeds. | ||
| if !s.cfg.Bitcoin.IsLocalNetwork() { | ||
| //nolint:ll | ||
| dnsSeeds, ok := chainreg.ChainDNSSeeds[*s.cfg.ActiveNetParams.GenesisHash] |
There was a problem hiding this comment.
probs something we should check occasionally yeah
| // seeds. | ||
| if !s.cfg.Bitcoin.IsLocalNetwork() { | ||
| //nolint:ll | ||
| dnsSeeds, ok := chainreg.ChainDNSSeeds[*s.cfg.ActiveNetParams.GenesisHash] |
There was a problem hiding this comment.
That was my domain at some point. But a very long time ago 😅
We now have a proper seed though: https://github.com/lightningnetwork/lnd/blob/master/chainreg/chainregistry.go#L903
Follow-up from #9967
Fixes #9998
In the issue above, an itest flake appeared in the
peer bootstrappingitest where a peer would not connect to 3 nodes within the itest timeout. This can happen if theChannelGraphBootstrapperis initialised with ahashAccumulatorvalue that would result in any of the nodes in the test node's DB being skipped on the initial or even second sampling attempt.To fix this, we create a deterministic hash accumulator that is used in local test environments so that we can properly test the behaviour of peer bootstrapping.