Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

Fix memory store signed peer record bug #133

Merged
merged 2 commits into from
Mar 30, 2020
Merged

Fix memory store signed peer record bug #133

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9868efd
fix memory store signed peer record bug
aarshkshah1992 Mar 27, 2020
cdec762
fix add addrs bug
aarshkshah1992 Mar 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pstoremem/addr_book.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (mab *memoryAddrBook) addAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Du
// }

// if we've expired all the signed addresses for a peer, remove their signed routing state record
if len(addrs) == 0 {
if len(amap) == 0 {
delete(s.signedPeerRecords, p)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the original code was trying to do anyway.

I think for the code to really do what the comment claims to do, it would have to:

  1. Go through all addresses in the peer record.
  2. Check if we still have them.
  3. If we don't, remove the signed record.

This just clears the record if we have no more addresses. But the method that actually adds addresses to a peer shouldn't be the one doing this; it should be done by the GC routine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yusefnapora Do we need to clear this record in addAddrs for some reason ? If not, I can get rid of this code path completely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you guys are right; this isn't needed and was likely leftover from when we only had one type of addr. thanks for catching @aarshkshah1992

}
Expand Down