itest: test automatic peer bootstrapping#9967
itest: test automatic peer bootstrapping#9967guggero 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 (
|
ellemouton
force-pushed
the
peerBootstrapItest
branch
from
2bda297 to
fd1e84f
Compare
|
|
||
| // Dave still has bootstrapping disabled and so it should not connect | ||
| // to any peers. | ||
| err := wait.Invariant(func() bool { |
There was a problem hiding this comment.
nit: can use
ht.AssertNotConnected(dave, alice)
ht.AssertNotConnected(dave, bob)
ht.AssertNotConnected(dave, carol)so we don't need to wait for 5s
There was a problem hiding this comment.
the bootstrapping happens after start up though, so i think it is good to use an invariant here for this specific case cause there will be a time between restart and potential bootstrapping where no nodes will be connected. So we want to test here that the bootstrapping doesnt happen after restart
We already set `nobootstrap` in the default node flags for itest nodes, so we can remove this check now. This will allow us to later test bootstrapping in an itest. NOTE that with this change, any signet/simnet/regtest network users will now need to explicitly add the `--nobootstrap` flag if they want to prevent automatic bootstrapping. This warning is added to the release notes later on.
This will allow us to test bootstrapping in an itest.
Test the automatic peer bootstrapping logic via an itest.
Warn users of simnet/signet/regtest scripts that they will now need to explicitly specify the `--nobootstrap` flag to turn bootstrapping off.
ellemouton
force-pushed
the
peerBootstrapItest
branch
from
fd1e84f to
75a3531
Compare
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
Thanks for the swift review yall! updated 🫡
|
|
||
| // Dave still has bootstrapping disabled and so it should not connect | ||
| // to any peers. | ||
| err := wait.Invariant(func() bool { |
There was a problem hiding this comment.
the bootstrapping happens after start up though, so i think it is good to use an invariant here for this specific case cause there will be a time between restart and potential bootstrapping where no nodes will be connected. So we want to test here that the bootstrapping doesnt happen after restart
|
cc @guggero for override merge 🙏 |
4 participants
Description
In this PR, we add an itest to test the automatic peer bootstrapping logic. In other words, we test
that a node is able to use the graph data (specifically, node_announcements) to create some peer
connections.
Possible breaking change for regtest/simnet/signet users
We used to explicitly disallow bootstrapping for any simnet/signet/regtest nodes but this has now
been removed so that we can properly test this in an itest.
This does, however, mean that any users who have a simnet/signet/regtest network that depends on
this no-bootstrapping functionality will need to now explicitly add the
--nobootstrapconfig option.The release notes have been updated to inform users of this.
Why?