-
Notifications
You must be signed in to change notification settings - Fork 20k
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
eth: dial nodes from discv5 #30302
eth: dial nodes from discv5 #30302
Conversation
Unlike discv4, the discv5 random nodes iterator will always provide full ENRs. This means we can apply filtering to the results and will only try dialing nodes which explictly opt into the eth protocol with a matching chain. I have also removed the dial iterator from snap. We don't have an official DNS list for snap anymore, and I doubt anyone else is running one. While we could potentially filter for snap on discv5, there will be very few nodes announcing it, and the extra iterator would just stall the dialer.
Generally looks good to me, but could explain the circular dependency you described in #29533? Were you referring to way the PR used |
About the circular dependency: Ultimately, the nodes iterator has to end up in Previously, we only used the DNS list iterator as a per-protocol discovery source. We can create this iterator before starting the node because it is only dependent on the list of URLs which are provided in However, to call I have resolved the dependency here by putting an empty |
Okay thanks I see, now the |
A full integration test of drawing devp2p peers from discv5 would be good, maybe that's a job for hive? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran locally and am at least able to find peers with DNS / discv4. Probably best to start rolling this out and go from there.
You can test like this: modified eth/protocols/eth/discovery.go
@@ -17,6 +17,8 @@
package eth
import (
+ "fmt"
+
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/forkid"
"github.com/ethereum/go-ethereum/p2p/enode"
@@ -72,9 +74,15 @@ func NewNodeFilter(chain *core.BlockChain) func(*enode.Node) bool {
return func(n *enode.Node) bool {
var entry enrEntry
if err := n.Load(entry); err != nil {
+ fmt.Println("no eth", n.ID())
return false
}
err := filter(entry.ForkID)
+ if err != nil {
+ fmt.Println("wrong chain", n.ID())
+ } else {
+ fmt.Println("match!", n.ID())
+ }
return err == nil
}
}
If it ever prints |
Here I am adding a discv5 nodes source into the p2p dial iterator. It's an improved version of #29533.
Unlike discv4, the discv5 random nodes iterator will always provide full ENRs. This means we can apply filtering to the results and will only try dialing nodes which explictly opt into the eth protocol with a matching chain.
I have also removed the dial iterator from snap. We don't have an official DNS list for snap anymore, and I doubt anyone else is running one. While we could potentially filter for snap on discv5, there will be very few nodes announcing it, and the extra iterator would just stall the dialer.