discv5: add dump, opstack-chainid, chainid flags and fix peer persistence #45
Conversation
blockchaindevsh
requested review from
dajuguan,
ping-ke,
qizhou,
qzhodl and
syntrust
p2p/discover/v5_udp.go
Outdated
| return nil, errors.New("duplicate record") | ||
| } | ||
| seen[node.ID()] = struct{}{} | ||
| if t.nodeFilter != nil && !t.nodeFilter(node) { |
There was a problem hiding this comment.
Since we’ve added node filtering in handleAddNode, what additional cases does this change cover?
There was a problem hiding this comment.
handleAddNode is table-level validation, verifyResponseNode is UDP protocol-level
validation, this ensures only matching peers can respond toFindnode request.
There was a problem hiding this comment.
IIUC, this change filters out invalid peers from the FindNode response.
But we already validate peers when inserting them into the routing table, so those invalid peers shouldn’t be added to the table anyway.
So is the goal here just to drop invalid peers earlier (at response-processing time), even though they would be rejected later during table insertion?
There was a problem hiding this comment.
This filter affects the result returned from Findnode, which has many callers , e.g., it will also affect result returned from RandomNodes. The filter in handleAddNode is only applied on boot node's direct peers, not on indirect peers like what Findnode may return.
There was a problem hiding this comment.
Looks like it's not necessary for bootnode, I've removed this change.
…ence (#45) * discv5: add dump, opstack-chainid, chainid flags and fix peer persistence * fix log * remove unnecessary change
This PR squashes all devp2p changes we need for bootnode.