From ce54f4b8f5d9a762c69a997319fb40a85054a669 Mon Sep 17 00:00:00 2001 From: sukun Date: Mon, 16 Sep 2024 15:46:14 +0530 Subject: [PATCH] remove bound on number of SignedPeerRecords --- core/peerstore/peerstore.go | 38 ++++++++-------- p2p/host/basic/basic_host.go | 55 +++++++++++++---------- p2p/host/peerstore/pstoremem/addr_book.go | 7 --- 3 files changed, 49 insertions(+), 51 deletions(-) diff --git a/core/peerstore/peerstore.go b/core/peerstore/peerstore.go index 7bc28b9d74..6833565076 100644 --- a/core/peerstore/peerstore.go +++ b/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 } diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index 7b7f8855fb..b5d252e9d2 100644 --- a/p2p/host/basic/basic_host.go +++ b/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 diff --git a/p2p/host/peerstore/pstoremem/addr_book.go b/p2p/host/peerstore/pstoremem/addr_book.go index b37aee83c5..89b87bdb47 100644 --- a/p2p/host/peerstore/pstoremem/addr_book.go +++ b/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]