diff --git a/p2p/discover/table.go b/p2p/discover/table.go index 21a256bb9f..7ece8cf2f5 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -513,16 +513,9 @@ func (tab *Table) handleAddNode(req addNodeOp) bool { } b := tab.bucket(req.node.ID()) - if !req.isInbound && req.node.Seq() == 0 && contains(b.entries, req.node.ID()) { - // Special case for discv4: because endpoint information is not - // authenticated, do not update until the node is successfully communicated - // with. Therefore, non-inbounds which are already in the bucket should be - // skipped. - return false - - } - if tab.bumpInBucket(b, req.node.Node) { - // Already in bucket, update record. + exists, _ := tab.bumpInBucket(b, req.node.Node, req.isInbound) + if exists { + // Already in bucket. return false } if len(b.entries) >= bucketSize { @@ -613,26 +606,37 @@ func (tab *Table) deleteInBucket(b *bucket, id enode.ID) *node { return rep } -// bumpInBucket updates the node record of n in the bucket. -func (tab *Table) bumpInBucket(b *bucket, newRecord *enode.Node) bool { +// bumpInBucket updates a node record if it exists in the bucket. +func (tab *Table) bumpInBucket(b *bucket, newRecord *enode.Node, isInbound bool) (exists, updated bool) { i := slices.IndexFunc(b.entries, func(elem *node) bool { return elem.ID() == newRecord.ID() }) if i == -1 { - return false + return false, false // node not in bucket } - if !newRecord.IP().Equal(b.entries[i].IP()) { - // Endpoint has changed, ensure that the new IP fits into table limits. - tab.removeIP(b, b.entries[i].IP()) + // Disallow updates unless the sequence number is increased. + // Note there is a special case for discv4: if the node contacts us (isInbound), + // it is allowed to update its own entry. + oldRecord := b.entries[i] + isUpdate := newRecord.Seq() > oldRecord.Seq() + isDiscv4Update := oldRecord.Seq() == 0 && newRecord.Seq() == 0 && isInbound + if !(isUpdate || isDiscv4Update) { + return true, false + } + + // Check if there is an endpoint update and validate against IP limits. + if newRecord.IPAddr() != b.entries[i].IPAddr() { + tab.removeIP(b, oldRecord.IP()) if !tab.addIP(b, newRecord.IP()) { - // It doesn't, put the previous one back. - tab.addIP(b, b.entries[i].IP()) - return false + // It doesn't fit with the limit, put the previous record back. + tab.addIP(b, oldRecord.IP()) + return true, false } } + b.entries[i].Node = newRecord - return true + return true, true } func (tab *Table) handleTrackRequest(op trackRequestOp) { diff --git a/p2p/discover/table_reval.go b/p2p/discover/table_reval.go index 01a5bdb6fa..c5160131f9 100644 --- a/p2p/discover/table_reval.go +++ b/p2p/discover/table_reval.go @@ -161,7 +161,7 @@ func (tr *tableRevalidation) handleResponse(tab *Table, resp revalidationRespons n.isValidatedLive = true var endpointChanged bool if resp.newRecord != nil { - endpointChanged = tab.bumpInBucket(b, resp.newRecord) + _, endpointChanged = tab.bumpInBucket(b, resp.newRecord, false) if endpointChanged { // If the node changed its advertised endpoint, the updated ENR is not served // until it has been revalidated. diff --git a/p2p/discover/table_test.go b/p2p/discover/table_test.go index 945b1a294e..0aaf7916de 100644 --- a/p2p/discover/table_test.go +++ b/p2p/discover/table_test.go @@ -275,7 +275,7 @@ func (*closeTest) Generate(rand *rand.Rand, size int) reflect.Value { return reflect.ValueOf(t) } -func TestTable_addVerifiedNode(t *testing.T) { +func TestTable_addInboundNode(t *testing.T) { tab, db := newTestTable(newPingRecorder(), Config{}) <-tab.initDone defer db.Close() @@ -308,7 +308,7 @@ func TestTable_addVerifiedNode(t *testing.T) { checkIPLimitInvariant(t, tab) } -func TestTable_addSeenNode(t *testing.T) { +func TestTable_addFoundNode(t *testing.T) { tab, db := newTestTable(newPingRecorder(), Config{}) <-tab.initDone defer db.Close()