Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/peerstore impl #505

Merged
merged 6 commits into from
Jan 29, 2021
Merged

Feat/peerstore impl #505

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c506272
Added missing books
jm-clius Jan 15, 2021
48be0a7
Extract handlers from the encapsulating listener object.
jm-clius Jan 15, 2021
ff7d081
Improvement - move handlers to books
jm-clius Jan 19, 2021
926d340
Cleanup unnecessary newSeq initialisations
jm-clius Jan 19, 2021
12e0716
Genericise PeerStore
jm-clius Jan 26, 2021
218ca64
Removed metadataBook (for now)
jm-clius Jan 28, 2021
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
256 changes: 226 additions & 30 deletions libp2p/peerstore.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,19 @@

import
std/[tables, sets, sequtils],
./crypto/crypto,
./peerid,
./multiaddress

type
##############################
# Listener and handler types #
##############################
#################
# Handler types #
#################

AddrChangeHandler* = proc(peerId: PeerID, multiaddrs: HashSet[MultiAddress])

EventListener* = object
# Listener object with client-defined handlers for peer store events
addrChange*: AddrChangeHandler
# @TODO add handlers for other event types
ProtoChangeHandler* = proc(peerId: PeerID, protos: HashSet[string])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is FooChangeHandler the common name here? Nitpick, but something like FooUpdateHandler might be a tiny bit more descriptive, "Change" to me suggests something could be breaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. change was selected to stay as close as possible to similar events and naming in the js-implementation, but I am happy to update naming to update :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the naming convention makes sense here.

KeyChangeHandler* = proc(peerId: PeerID, publicKey: PublicKey)
MetadataChangeHandler* = proc(peerId: PeerID, metadata: Table[string, seq[byte]])

#########
# Books #
Expand All @@ -31,35 +30,59 @@ type
# Each book contains a book (map) and event handler(s)
AddressBook* = object
book*: Table[PeerID, HashSet[MultiAddress]]
addrChange: AddrChangeHandler
changeHandlers: seq[AddrChangeHandler]

ProtoBook* = object
book*: Table[PeerID, HashSet[string]]
changeHandlers: seq[ProtoChangeHandler]

KeyBook* = object
book*: Table[PeerID, PublicKey]
changeHandlers: seq[KeyChangeHandler]

MetadataBook* = object
book*: Table[PeerID, Table[string, seq[byte]]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this completely arbitrary? What do we imagine storing here? (Generally seems useful, mostly curious)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's supposed to be a general purpose key-value metadata store per peer. In my mind we can use it to keep performance related metrics, such as peer age, throughput, bandwidth, etc. In the go- and js implementations it's also used to store peer location, libp2p agent version and other peer-related metadata determined during the identify procedure (see e.g. libp2p/js-libp2p#800).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure about seq[byte] as it can just hold pure data, no references etc.. but for now it's just fine, it's something that can be fixed when we figure out better.

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed no raises which is good and so we can probably add on top {.push raises: [Defect].}

Ah, great catch, lets get that added right at the top of the file @jm-clius!

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, to clarify - there rule of thumb is that we use this style everywhere possible, the only place/reason it can't be used is in the presence of async, non-async apis should default to this style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, great suggestions and I especially like the consistency of the second suggestion. To be clear, do you then suggest that specialized entry objects be defined for AddressEntry, ProtoEntry and KeyEntry, but that the MetadataBook maintains the generic PeerBookEntry[T] so peer book clients can define their own entry types?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the generic entry yes!, maybe codegen will be bloaty but that's not our fault (nim things)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, great suggestions and I especially like the consistency of the second suggestion. To be clear, do you then suggest that specialized entry objects be defined for AddressEntry, ProtoEntry and KeyEntry, but that the MetadataBook maintains the generic PeerBookEntry[T] so peer book clients can define their own entry types?

Yep, precisely. In fact, we can make MetadataBook be the base for the more specialised book types as well. The only thing I'm not content with is the implicit entry*: T in PeerBookEntry, I wish Nim had a more concise syntax for that sort of thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, this is a very reasonable suggestion, since addresses, keys and protos can be seen as specialised types of metadata in any case.

My only concern would be that the API should still make explicit the subtle differences between the books. For example, an AddressBook maps a peerId to a collection of addrs (and therefore should allow clients to add individual addrs to each collection), whereas a KeyBook only allows a single PublicKey entry per peerId (and therefore only allows setting a singular entry).

I've attempted to do the above in 12e0716. This significantly changes (and simplifies) the PeerStore by making extensive use of generics. Each PeerStore still consists of a specialised addressBook, keyBook and protoBook, but now also contains one "any-purpose" metadataBook of generic type PeerBook[T] (this can easily be changed to a seq of such any-purpose books if there is a need for extensibility). Of course, this does not really allow for run-time object variance in the metadataBook (which I don't think generics are suitable for, in any case?), but does allow clients to specify their own metadata type when instantiating a new PeerStore. I'm not exactly happy with the PeerStore itself being genericised, but since it requires a concrete type when constructing the metadataBook I don't really see a way around this. I'll be glad for any suggestions to make this even more flexible. cc @sinkingsugar @dryajov

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, the concept of a free form metadata store doesn't make much sense in this context and I would just leave it out, allowing anyone who needs additional information to just extend the PeerBook and PeerStores accordingly.

changeHandlers: seq[MetadataChangeHandler]

####################
# Peer store types #
####################

PeerStore* = ref object of RootObj
addressBook*: AddressBook
listeners: seq[EventListener]
protoBook*: ProtoBook
keyBook*: KeyBook
metadataBook*: MetadataBook

StoredInfo* = object
# Collates stored info about a peer
## @TODO include data from other stores once added
peerId*: PeerID
addrs*: HashSet[MultiAddress]
protos*: HashSet[string]
publicKey*: PublicKey
metadata*: Table[string, seq[byte]]

proc init(T: type AddressBook, addrChange: AddrChangeHandler): AddressBook =
proc init(T: type AddressBook): AddressBook =
T(book: initTable[PeerId, HashSet[MultiAddress]](),
addrChange: addrChange)
changeHandlers: newSeq[AddrChangeHandler]())
dryajov marked this conversation as resolved.
Show resolved Hide resolved

proc init*(p: PeerStore) =
p.listeners = newSeq[EventListener]()
proc init(T: type ProtoBook): ProtoBook =
T(book: initTable[PeerId, HashSet[string]](),
changeHandlers: newSeq[ProtoChangeHandler]())

proc addrChange(peerId: PeerID, multiaddrs: HashSet[MultiAddress]) =
# Notify all listeners of change in multiaddr
for listener in p.listeners:
listener.addrChange(peerId, multiaddrs)

p.addressBook = AddressBook.init(addrChange)
proc init(T: type KeyBook): KeyBook =
T(book: initTable[PeerId, PublicKey](),
changeHandlers: newSeq[KeyChangeHandler]())

proc init(T: type MetadataBook): MetadataBook =
T(book: initTable[PeerId, Table[string, seq[byte]]](),
changeHandlers: newSeq[MetadataChangeHandler]())

proc init*(p: PeerStore) =
p.addressBook = AddressBook.init()
p.protoBook = ProtoBook.init()
p.keyBook = KeyBook.init()
p.metadataBook = MetadataBook.init()

proc init*(T: type PeerStore): PeerStore =
var p: PeerStore
Expand All @@ -86,7 +109,10 @@ proc add*(addressBook: var AddressBook,

addressBook.book.mgetOrPut(peerId,
initHashSet[MultiAddress]()).incl(multiaddr)
addressBook.addrChange(peerId, addressBook.get(peerId)) # Notify clients

# Notify clients
for handler in addressBook.changeHandlers:
handler(peerId, addressBook.get(peerId))

proc delete*(addressBook: var AddressBook,
peerId: PeerID): bool =
Expand All @@ -107,36 +133,206 @@ proc set*(addressBook: var AddressBook,
## Consider using addressBook.add() as alternative.

addressBook.book[peerId] = addrs
addressBook.addrChange(peerId, addressBook.get(peerId)) # Notify clients

# Notify clients
for handler in addressBook.changeHandlers:
handler(peerId, addressBook.get(peerId))

#####################
# Protocol Book API #
#####################

proc get*(protoBook: ProtoBook,
peerId: PeerID): HashSet[string] =
## Get the known protocols of a provided peer.

protoBook.book.getOrDefault(peerId,
initHashSet[string]())

proc add*(protoBook: var ProtoBook,
Copy link
Contributor

Choose a reason for hiding this comment

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

At what point will this be called? Who is responsible for calling it and at what point in the lifecycle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since a Peer Store provides a "standalone" API it can theoretically be instantiated and added to (at any time) by any module seeking to provide some level of peer management.
Eventually I envision it to be integrated in such a way that there is a single libp2p instance, with the following integration points (based on js-implementation):

  • a Dialer (TBD) adds "bootstrap" or statically defined peers when these are dialed. This is most likely the first point of integration for WakuRelay as well. The Dialer would also be responsible for maintaining connections and redialing peers when dropped.
  • the identify protocol adds information about identified peers.
  • Discovery/DHT mechanisms adds information about discovered peers.

peerId: PeerID,
protocol: string) =
## Adds known protocol codec for a given peer. If the peer is not known,
## it will be set with the provided protocol.

protoBook.book.mgetOrPut(peerId,
initHashSet[string]()).incl(protocol)

# Notify clients
for handler in protoBook.changeHandlers:
handler(peerId, protoBook.get(peerId))

proc delete*(protoBook: var ProtoBook,
peerId: PeerID): bool =
## Delete the provided peer from the book.

if not protoBook.book.hasKey(peerId):
return false
else:
protoBook.book.del(peerId)
return true

proc set*(protoBook: var ProtoBook,
peerId: PeerID,
protocols: HashSet[string]) =
## Set known protocol codecs for a given peer. This will replace previously
## stored protocols.

protoBook.book[peerId] = protocols

# Notify clients
for handler in protoBook.changeHandlers:
handler(peerId, protoBook.get(peerId))

################
# Key Book API #
################

proc get*(keyBook: KeyBook,
peerId: PeerID): PublicKey =
## Get the known public key of a provided peer.

keyBook.book.getOrDefault(peerId,
PublicKey())

proc delete*(keyBook: var KeyBook,
peerId: PeerID): bool =
## Delete the provided peer from the book.

if not keyBook.book.hasKey(peerId):
return false
else:
keyBook.book.del(peerId)
return true

proc set*(keyBook: var KeyBook,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto above, where is this called and who is responsible for it? Upon initial connection in nim-libp2p or somewhere else?

peerId: PeerID,
publicKey: PublicKey) =
## Set known public key for a given peer. This will replace any
## previously stored keys.

keyBook.book[peerId] = publicKey

# Notify clients
for handler in keyBook.changeHandlers:
handler(peerId, keyBook.get(peerId))

#####################
# Metadata Book API #
#####################

proc get*(metadataBook: MetadataBook,
peerId: PeerID): Table[string, seq[byte]] =
## Get all the known metadata of a provided peer.

metadataBook.book.getOrDefault(peerId,
initTable[string, seq[byte]]())

proc getValue*(metadataBook: MetadataBook,
peerId: PeerID,
key: string): seq[byte] =
## Get metadata for a provided peer corresponding to a specific key.

metadataBook.book.getOrDefault(peerId,
initTable[string, seq[byte]]())
.getOrDefault(key,
newSeq[byte]())

proc set*(metadataBook: var MetadataBook,
peerId: PeerID,
metadata: Table[string, seq[byte]]) =
## Set metadata for a given peerId. This will replace any
## previously stored metadata.

metadataBook.book[peerId] = metadata

# Notify clients
for handler in metadataBook.changeHandlers:
handler(peerId, metadataBook.get(peerId))

proc setValue*(metadataBook: var MetadataBook,
peerId: PeerID,
key: string,
value: seq[byte]) =
## Set a metadata key-value pair for a given peerId. This will replace
## any metadata previously stored against this key.

metadataBook.book.mgetOrPut(peerId,
initTable[string, seq[byte]]())[key] = value

# Notify clients
for handler in metadataBook.changeHandlers:
handler(peerId, metadataBook.get(peerId))

proc delete*(metadataBook: var MetadataBook,
peerId: PeerID): bool =
## Delete the provided peer from the book.

if not metadataBook.book.hasKey(peerId):
return false
else:
metadataBook.book.del(peerId)
return true

proc deleteValue*(metadataBook: var MetadataBook,
peerId: PeerID,
key: string): bool =
## Delete the metadata for a provided peer corresponding to a specific key.

if not metadataBook.book.hasKey(peerId) or not metadataBook.book[peerId].hasKey(key):
return false
else:
metadataBook.book[peerId].del(key)

# Notify clients
for handler in metadataBook.changeHandlers:
handler(peerId, metadataBook.get(peerId))

return true

##################
# Peer Store API #
##################

proc addListener*(peerStore: PeerStore,
listener: EventListener) =
## Register event listener to notify clients of changes in the peer store
proc addHandlers*(peerStore: PeerStore,
addrChangeHandler: AddrChangeHandler,
protoChangeHandler: ProtoChangeHandler,
keyChangeHandler: KeyChangeHandler,
metadataChangeHandler: MetadataChangeHandler) =
## Register event handlers to notify clients of changes in the peer store

peerStore.listeners.add(listener)
peerStore.addressBook.changeHandlers.add(addrChangeHandler)
peerStore.protoBook.changeHandlers.add(protoChangeHandler)
peerStore.keyBook.changeHandlers.add(keyChangeHandler)
peerStore.metadataBook.changeHandlers.add(metadataChangeHandler)

proc delete*(peerStore: PeerStore,
peerId: PeerID): bool =
## Delete the provided peer from every book.

peerStore.addressBook.delete(peerId)
peerStore.addressBook.delete(peerId) and
peerStore.protoBook.delete(peerId) and
peerStore.keyBook.delete(peerId) and
peerStore.metadataBook.delete(peerId)

proc get*(peerStore: PeerStore,
peerId: PeerID): StoredInfo =
## Get the stored information of a given peer.

StoredInfo(
peerId: peerId,
addrs: peerStore.addressBook.get(peerId)
addrs: peerStore.addressBook.get(peerId),
protos: peerStore.protoBook.get(peerId),
publicKey: peerStore.keyBook.get(peerId),
metadata: peerStore.metadataBook.get(peerId)
)

proc peers*(peerStore: PeerStore): seq[StoredInfo] =
## Get all the stored information of every peer.

let allKeys = toSeq(keys(peerStore.addressBook.book)) # @TODO concat keys from other books
let allKeys = concat(toSeq(keys(peerStore.addressBook.book)),
toSeq(keys(peerStore.protoBook.book)),
toSeq(keys(peerStore.keyBook.book)),
toSeq(keys(peerStore.metadataBook.book))).toHashSet()

return allKeys.mapIt(peerStore.get(it))
Loading