remove bound on number of SignedPeerRecords

core/peerstore/peerstore.go

@@ -121,12 +121,10 @@ type AddrBook interface {
 	PeersWithAddrs() peer.IDSlice
 }
 
-// CertifiedAddrBook manages "self-certified" addresses for remote peers.
-// Self-certified addresses are contained in signed peer.PeerRecords.
-// Certified addresses are generally more secure than uncertified
-// addresses.
+// CertifiedAddrBook manages signed peer records and "self-certified" addresses
+// contained within them.
+// Use this interface with an `AddrBook`.
 //
-// This interface is most useful when combined with AddrBook.
 // To test whether a given AddrBook / Peerstore implementation supports
 // certified addresses, callers should use the GetCertifiedAddrBook helper or
 // type-assert on the CertifiedAddrBook interface:
@@ -135,28 +133,28 @@ type AddrBook interface {
 //	    cab.ConsumePeerRecord(signedPeerRecord, aTTL)
 //	}
 type CertifiedAddrBook interface {
-	// ConsumePeerRecord adds addresses from a signed peer.PeerRecord, which will expire when
-	// all addresses associated with the peer have expired. The addresses in provided signed
-	// peer.PeerRecord are expired after `ttl` duration.
+	// ConsumePeerRecord stores a signed peer record and the contained addresses for
+	// for ttl duration.
+	// The addresses contained in the signed peer record will expire after ttl. If any
+	// address is already present in the peer store, it'll expire at max of existing ttl and
+	// provided ttl.
+	// The signed peer record itself will be expired when all the addresses associated with the peer,
+	// self-certified or not, are removed from the AddrBook.
+	// To delete the signed peer record, use `AddrBook.UpdateAddrs`,`AddrBook.SetAddrs`, or
+	// `AddrBook.ClearAddrs` with ttl 0.
+	// Note: Future calls to ConsumePeerRecord will not expire self-certified addresses from the
+	// previous calls.
 	//
 	// The `accepted` return value indicates that the record was successfully processed. If
 	// `accepted` is false but no error is returned, it means that the record was ignored, most
-	// likely because a newer record exists for the same peer.
+	// likely because a newer record exists for the same peer with a greater seq value.
 	//
-	// If the signed peer.PeerRecord belongs to a peer that already has certified addresses in
-	// the CertifiedAddrBook, and if the new record has a higher sequence number than the
-	// existing record, the new addresses will be added and the older ones will be kept
-	// unchanged. Attempting to add a peer record with a sequence number that's lower than an
-	// existing record will not result in an error, but the record will be ignored, and the
-	// `accepted` return value will be false.
-	//
-	// The Envelopes containing the PeerRecords can be retrieved by calling
+	// The Envelopes containing the signed peer records can be retrieved by calling
 	// GetPeerRecord(peerID).
 	ConsumePeerRecord(s *record.Envelope, ttl time.Duration) (accepted bool, err error)
 
-	// GetPeerRecord returns an Envelope containing a PeerRecord for the
-	// given peer id, if one exists.
-	// Returns nil if no signed PeerRecord exists for the peer.
+	// GetPeerRecord returns an Envelope containing a peer record for the
+	// peer, or nil if no record exists.
 	GetPeerRecord(p peer.ID) *record.Envelope
 }
 

p2p/host/basic/basic_host.go

@@ -495,9 +495,12 @@ func (h *BasicHost) SignalAddressChange() {
 	}
 }
 
-func makeUpdatedAddrEvent(prev, current []ma.Multiaddr) *event.EvtLocalAddressesUpdated {
+func (h *BasicHost) makeUpdatedAddrEvent(prev, current []ma.Multiaddr) *event.EvtLocalAddressesUpdated {
+	if prev == nil && current == nil {
+		return nil
+	}
 	prevmap := make(map[string]ma.Multiaddr, len(prev))
-	evt := event.EvtLocalAddressesUpdated{Diffs: true}
+	evt := &event.EvtLocalAddressesUpdated{Diffs: true}
 	addrsAdded := false
 
 	for _, addr := range prev {
@@ -524,7 +527,19 @@ func makeUpdatedAddrEvent(prev, current []ma.Multiaddr) *event.EvtLocalAddresses
 		return nil
 	}
 
-	return &evt
+	// Our addresses have changed. Make a new signed peer record.
+	if !h.disableSignedPeerRecord {
+		// add signed peer record to the event
+		sr, err := h.makeSignedPeerRecord(current)
+		if err != nil {
+			log.Errorf("error creating a signed peer record from the set of current addresses, err=%s", err)
+			// drop this change
+			return nil
+		}
+		evt.SignedPeerRecord = sr
+	}
+
+	return evt
 }
 
 func (h *BasicHost) makeSignedPeerRecord(addrs []ma.Multiaddr) (*record.Envelope, error) {
@@ -548,34 +563,27 @@ func (h *BasicHost) background() {
 	var lastAddrs []ma.Multiaddr
 
 	emitAddrChange := func(currentAddrs []ma.Multiaddr, lastAddrs []ma.Multiaddr) {
-		// nothing to do if both are nil..defensive check
-		if currentAddrs == nil && lastAddrs == nil {
-			return
-		}
-
-		changeEvt := makeUpdatedAddrEvent(lastAddrs, currentAddrs)
-
+		changeEvt := h.makeUpdatedAddrEvent(lastAddrs, currentAddrs)
 		if changeEvt == nil {
 			return
 		}
-
+		// Our addresses have changed.
+		// store the signed peer record in the peer store.
 		if !h.disableSignedPeerRecord {
-			// add signed peer record to the event
-			sr, err := h.makeSignedPeerRecord(currentAddrs)
-			if err != nil {
-				log.Errorf("error creating a signed peer record from the set of current addresses, err=%s", err)
-				return
-			}
-			changeEvt.SignedPeerRecord = sr
-
-			// persist the signed record to the peerstore
-			if _, err := h.caBook.ConsumePeerRecord(sr, peerstore.PermanentAddrTTL); err != nil {
+			if _, err := h.caBook.ConsumePeerRecord(changeEvt.SignedPeerRecord, peerstore.PermanentAddrTTL); err != nil {
 				log.Errorf("failed to persist signed peer record in peer store, err=%s", err)
 				return
 			}
 		}
+		// update host addresses in the peer store
+		removedAddrs := make([]ma.Multiaddr, 0, len(changeEvt.Removed))
+		for _, ua := range changeEvt.Removed {
+			removedAddrs = append(removedAddrs, ua.Address)
+		}
+		h.Peerstore().SetAddrs(h.ID(), currentAddrs, peerstore.PermanentAddrTTL)
+		h.Peerstore().SetAddrs(h.ID(), removedAddrs, 0)
 
-		// emit addr change event on the bus
+		// emit addr change event
 		if err := h.emitters.evtLocalAddrsUpdated.Emit(*changeEvt); err != nil {
 			log.Warnf("error emitting event for updated addrs: %s", err)
 		}
@@ -587,11 +595,10 @@ func (h *BasicHost) background() {
 	defer ticker.Stop()
 
 	for {
+		// Update our local IP addresses before checking our current addresses.
 		if len(h.network.ListenAddresses()) > 0 {
 			h.updateLocalIpAddr()
 		}
-		// Request addresses anyways because, technically, address filters still apply.
-		// The underlying AllAddrs call is effectively a no-op.
 		curr := h.Addrs()
 		emitAddrChange(curr, lastAddrs)
 		lastAddrs = curr

p2p/host/peerstore/pstoremem/addr_book.go

@@ -16,8 +16,6 @@ import (
 	ma "github.com/multiformats/go-multiaddr"
 )
 
-var SignedPeerRecordBound = 100_000
-
 var log = logging.Logger("peerstore")
 
 type expiringAddr struct {
@@ -231,8 +229,6 @@ func (mab *memoryAddrBook) AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Du
 	mab.addAddrs(p, addrs, ttl)
 }
 
-var ErrTooManyRecords = fmt.Errorf("too many signed peer records. Dropping this one")
-
 // ConsumePeerRecord adds addresses from a signed peer.PeerRecord, which will expire after the given TTL.
 // See https://godoc.org/github.com/libp2p/go-libp2p/core/peerstore#CertifiedAddrBook for more details.
 func (mab *memoryAddrBook) ConsumePeerRecord(recordEnvelope *record.Envelope, ttl time.Duration) (bool, error) {
@@ -250,9 +246,6 @@ func (mab *memoryAddrBook) ConsumePeerRecord(recordEnvelope *record.Envelope, tt
 
 	mab.mu.Lock()
 	defer mab.mu.Unlock()
-	if (len(mab.signedPeerRecords)) >= SignedPeerRecordBound {
-		return false, ErrTooManyRecords
-	}
 
 	// ensure seq is greater than or equal to the last received
 	lastState, found := mab.signedPeerRecords[rec.PeerID]