From 1befeb8c2e74425787d7bfd7b53d6c60302161c7 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Thu, 3 Dec 2020 20:53:16 +0100 Subject: [PATCH] clean up peerid (#470) * fix dangling cstring on error return * remove some useless inlines * less mallocs in shortlog * proc -> func * rename test --- libp2p/peerid.nim | 104 ++++++++++++------------- libp2p/protocols/pubsub/mcache.nim | 11 ++- tests/testnative.nim | 2 +- tests/{testpeer.nim => testpeerid.nim} | 1 + 4 files changed, 56 insertions(+), 62 deletions(-) rename tests/{testpeer.nim => testpeerid.nim} (99%) diff --git a/libp2p/peerid.nim b/libp2p/peerid.nim index cd01ec1..bffc4c4 100644 --- a/libp2p/peerid.nim +++ b/libp2p/peerid.nim @@ -11,20 +11,19 @@ {.push raises: [Defect].} -import std/hashes -import chronicles -import nimcrypto/utils, stew/base58 -import crypto/crypto, multicodec, multihash, vbuffer -import protobuf/minprotobuf -import stew/results +import + std/[hashes, strutils], + stew/[base58, results], + chronicles, + nimcrypto/utils, + ./crypto/crypto, ./multicodec, ./multihash, ./vbuffer, + ./protobuf/minprotobuf export results const maxInlineKeyLength* = 42 -# TODO: add proper on disc serialization -# using peer-id protobuf format type PeerID* = object data*: seq[byte] @@ -36,19 +35,15 @@ func `$`*(pid: PeerID): string = func shortLog*(pid: PeerId): string = ## Returns compact string representation of ``pid``. var spid = $pid - if len(spid) <= 10: - result = spid - else: - result = newStringOfCap(10) - for i in 0..<2: - result.add(spid[i]) - result.add("*") - for i in (len(spid) - 6)..spid.high: - result.add(spid[i]) + if len(spid) > 10: + spid[3] = '*' + spid.delete(4, spid.high - 6) + + spid chronicles.formatIt(PeerID): shortLog(it) -proc toBytes*(pid: PeerID, data: var openarray[byte]): int = +func toBytes*(pid: PeerID, data: var openarray[byte]): int = ## Store PeerID ``pid`` to array of bytes ``data``. ## ## Returns number of bytes needed to store ``pid``. @@ -56,20 +51,19 @@ proc toBytes*(pid: PeerID, data: var openarray[byte]): int = if len(data) >= result and result > 0: copyMem(addr data[0], unsafeAddr pid.data[0], result) -proc getBytes*(pid: PeerID): seq[byte] {.inline.} = +template getBytes*(pid: PeerID): seq[byte] = ## Return PeerID ``pid`` as array of bytes. - result = pid.data + pid.data -proc hex*(pid: PeerID): string {.inline.} = +func hex*(pid: PeerID): string = ## Returns hexadecimal string representation of ``pid``. - if len(pid.data) > 0: - result = toHex(pid.data) + toHex(pid.data) -proc len*(pid: PeerID): int {.inline.} = +template len*(pid: PeerID): int = ## Returns length of ``pid`` binary representation. - result = len(pid.data) + len(pid.data) -proc cmp*(a, b: PeerID): int = +func cmp*(a, b: PeerID): int = ## Compares two peer ids ``a`` and ``b``. ## Returns: ## @@ -84,30 +78,29 @@ proc cmp*(a, b: PeerID): int = inc(i) result = len(a.data) - len(b.data) -proc `<=`*(a, b: PeerID): bool {.inline.} = +template `<=`*(a, b: PeerID): bool = (cmp(a, b) <= 0) -proc `<`*(a, b: PeerID): bool {.inline.} = +template `<`*(a, b: PeerID): bool = (cmp(a, b) < 0) -proc `>=`*(a, b: PeerID): bool {.inline.} = +template `>=`*(a, b: PeerID): bool = (cmp(a, b) >= 0) -proc `>`*(a, b: PeerID): bool {.inline.} = +template `>`*(a, b: PeerID): bool = (cmp(a, b) > 0) -proc `==`*(a, b: PeerID): bool {.inline.} = +template `==`*(a, b: PeerID): bool = (cmp(a, b) == 0) -proc hash*(pid: PeerID): Hash {.inline.} = - result = hash(pid.data) +template hash*(pid: PeerID): Hash = + hash(pid.data) -proc validate*(pid: PeerID): bool = +func validate*(pid: PeerID): bool = ## Validate check if ``pid`` is empty or not. - if len(pid.data) > 0: - result = MultiHash.validate(pid.data) + len(pid.data) > 0 and MultiHash.validate(pid.data) -proc hasPublicKey*(pid: PeerID): bool = +func hasPublicKey*(pid: PeerID): bool = ## Returns ``true`` if ``pid`` is small enough to hold public key inside. if len(pid.data) > 0: var mh: MultiHash @@ -115,19 +108,19 @@ proc hasPublicKey*(pid: PeerID): bool = if mh.mcodec == multiCodec("identity"): result = true -proc extractPublicKey*(pid: PeerID, pubkey: var PublicKey): bool = +func extractPublicKey*(pid: PeerID, pubkey: var PublicKey): bool = ## Returns ``true`` if public key was successfully decoded from PeerID ## ``pid``and stored to ``pubkey``. ## ## Returns ``false`` otherwise. - var mh: MultiHash if len(pid.data) > 0: + var mh: MultiHash if MultiHash.decode(pid.data, mh).isOk: if mh.mcodec == multiCodec("identity"): let length = len(mh.data.buffer) result = pubkey.init(mh.data.buffer.toOpenArray(mh.dpos, length - 1)) -proc init*(pid: var PeerID, data: openarray[byte]): bool = +func init*(pid: var PeerID, data: openarray[byte]): bool = ## Initialize peer id from raw binary representation ``data``. ## ## Returns ``true`` if peer was successfully initialiazed. @@ -136,7 +129,7 @@ proc init*(pid: var PeerID, data: openarray[byte]): bool = pid = p result = true -proc init*(pid: var PeerID, data: string): bool = +func init*(pid: var PeerID, data: string): bool = ## Initialize peer id from base58 encoded string representation. ## ## Returns ``true`` if peer was successfully initialiazed. @@ -150,7 +143,7 @@ proc init*(pid: var PeerID, data: string): bool = pid = opid result = true -proc init*(t: typedesc[PeerID], data: openarray[byte]): Result[PeerID, cstring] {.inline.} = +func init*(t: typedesc[PeerID], data: openarray[byte]): Result[PeerID, cstring] = ## Create new peer id from raw binary representation ``data``. var res: PeerID if not init(res, data): @@ -158,7 +151,7 @@ proc init*(t: typedesc[PeerID], data: openarray[byte]): Result[PeerID, cstring] else: ok(res) -proc init*(t: typedesc[PeerID], data: string): Result[PeerID, cstring] {.inline.} = +func init*(t: typedesc[PeerID], data: string): Result[PeerID, cstring] = ## Create new peer id from base58 encoded string representation ``data``. var res: PeerID if not init(res, data): @@ -166,9 +159,10 @@ proc init*(t: typedesc[PeerID], data: string): Result[PeerID, cstring] {.inline. else: ok(res) -proc init*(t: typedesc[PeerID], pubkey: PublicKey): Result[PeerID, cstring] = +func init*(t: typedesc[PeerID], pubkey: PublicKey): Result[PeerID, cstring] = ## Create new peer id from public key ``pubkey``. - var pubraw = ? pubkey.getBytes().orError("peerid: failed to get bytes from given key") + var pubraw = ? pubkey.getBytes().orError( + cstring("peerid: failed to get bytes from given key")) var mh: MultiHash if len(pubraw) <= maxInlineKeyLength: mh = ? MultiHash.digest("identity", pubraw) @@ -176,11 +170,11 @@ proc init*(t: typedesc[PeerID], pubkey: PublicKey): Result[PeerID, cstring] = mh = ? MultiHash.digest("sha2-256", pubraw) ok(PeerID(data: mh.data.buffer)) -proc init*(t: typedesc[PeerID], seckey: PrivateKey): Result[PeerID, cstring] {.inline.} = +func init*(t: typedesc[PeerID], seckey: PrivateKey): Result[PeerID, cstring] = ## Create new peer id from private key ``seckey``. - PeerID.init(? seckey.getKey().orError("invalid private key")) + PeerID.init(? seckey.getKey().orError(cstring("invalid private key"))) -proc match*(pid: PeerID, pubkey: PublicKey): bool {.inline.} = +func match*(pid: PeerID, pubkey: PublicKey): bool = ## Returns ``true`` if ``pid`` matches public key ``pubkey``. let p = PeerID.init(pubkey) if p.isErr: @@ -188,7 +182,7 @@ proc match*(pid: PeerID, pubkey: PublicKey): bool {.inline.} = else: pid == p.get() -proc match*(pid: PeerID, seckey: PrivateKey): bool {.inline.} = +func match*(pid: PeerID, seckey: PrivateKey): bool = ## Returns ``true`` if ``pid`` matches private key ``seckey``. let p = PeerID.init(seckey) if p.isErr: @@ -198,15 +192,15 @@ proc match*(pid: PeerID, seckey: PrivateKey): bool {.inline.} = ## Serialization/Deserialization helpers -proc write*(vb: var VBuffer, pid: PeerID) {.inline.} = +func write*(vb: var VBuffer, pid: PeerID) = ## Write PeerID value ``peerid`` to buffer ``vb``. vb.writeSeq(pid.data) -proc initProtoField*(index: int, pid: PeerID): ProtoField {.deprecated.} = +func initProtoField*(index: int, pid: PeerID): ProtoField {.deprecated.} = ## Initialize ProtoField with PeerID ``value``. - result = initProtoField(index, pid.data) + initProtoField(index, pid.data) -proc getValue*(data: var ProtoBuffer, field: int, value: var PeerID): int {. +func getValue*(data: var ProtoBuffer, field: int, value: var PeerID): int {. deprecated.} = ## Read ``PeerID`` from ProtoBuf's message and validate it. var pid: PeerID @@ -217,11 +211,11 @@ proc getValue*(data: var ProtoBuffer, field: int, value: var PeerID): int {. else: value = pid -proc write*(pb: var ProtoBuffer, field: int, pid: PeerID) = +func write*(pb: var ProtoBuffer, field: int, pid: PeerID) = ## Write PeerID value ``peerid`` to object ``pb`` using ProtoBuf's encoding. write(pb, field, pid.data) -proc getField*(pb: ProtoBuffer, field: int, +func getField*(pb: ProtoBuffer, field: int, pid: var PeerID): ProtoResult[bool] {.inline.} = ## Read ``PeerID`` from ProtoBuf's message and validate it var buffer: seq[byte] diff --git a/libp2p/protocols/pubsub/mcache.nim b/libp2p/protocols/pubsub/mcache.nim index 4cd39ba..ff25c94 100644 --- a/libp2p/protocols/pubsub/mcache.nim +++ b/libp2p/protocols/pubsub/mcache.nim @@ -20,20 +20,20 @@ type MCache* = object of RootObj msgs*: Table[MessageID, Message] history*: seq[seq[CacheEntry]] - historySize*: Natural windowSize*: Natural func get*(c: MCache, mid: MessageID): Option[Message] = - result = none(Message) if mid in c.msgs: - result = some(c.msgs[mid]) + some(c.msgs[mid]) + else: + none(Message) func contains*(c: MCache, mid: MessageID): bool = c.get(mid).isSome func put*(c: var MCache, msgId: MessageID, msg: Message) = - if msgId notin c.msgs: - c.msgs[msgId] = msg + if not c.msgs.hasKeyOrPut(msgId, msg): + # Only add cache entry if the message was not already in the cache c.history[0].add(CacheEntry(mid: msgId, topicIDs: msg.topicIDs)) func window*(c: MCache, topic: string): HashSet[MessageID] = @@ -58,6 +58,5 @@ func shift*(c: var MCache) = func init*(T: type MCache, window, history: Natural): T = T( history: newSeq[seq[CacheEntry]](history), - historySize: history, windowSize: window ) diff --git a/tests/testnative.nim b/tests/testnative.nim index 509a520..06b3258 100644 --- a/tests/testnative.nim +++ b/tests/testnative.nim @@ -13,7 +13,7 @@ import testmultibase, testmultihash, testmultiaddress, testcid, - testpeer + testpeerid import testtransport, testmultistream, diff --git a/tests/testpeer.nim b/tests/testpeerid.nim similarity index 99% rename from tests/testpeer.nim rename to tests/testpeerid.nim index 3418e20..47b4a61 100644 --- a/tests/testpeer.nim +++ b/tests/testpeerid.nim @@ -220,6 +220,7 @@ suite "Peer testing suite": $p1 == $p2 $p1 == $p3 $p1 == $p4 + len(shortLog(p1)) <= 10 if i in {3, 4, 5}: var ekey1, ekey2, ekey3, ekey4: PublicKey check: