From bde7a26f9da874d2e550da361856d566db4e0b0f Mon Sep 17 00:00:00 2001 From: Tanguy Date: Wed, 6 Apr 2022 14:18:14 +0200 Subject: [PATCH 1/5] Fixes for integration --- .../private/eth/p2p/discoveryv5/protocol.nim | 36 ++++++++++++++++--- libp2pdht/private/eth/p2p/discoveryv5/spr.nim | 33 ++++++++--------- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim index fbd0ef3..2684053 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim @@ -222,12 +222,12 @@ func getRecord*(d: Protocol): SignedPeerRecord = ## Get the SPR of the local node. d.localNode.record -proc updateRecord*(d: Protocol): DiscResult[void] = +proc updateRecord*(d: Protocol, newSpr: SignedPeerRecord): DiscResult[void] = ## Update the ENR of the local node with provided `enrFields` k:v pairs. - # TODO: Do we need this proc? This simply serves so that seqNo will be - # incremented to satisfy the tests... - d.localNode.record.incSeqNo(d.privateKey) + info "Updated discovery SPR", uri=toURI(newSpr) + d.localNode.record = newSpr + ok() # TODO: Would it make sense to actively ping ("broadcast") to all the peers # we stored a handshake with in order to get that ENR updated? @@ -889,6 +889,7 @@ proc refreshLoop(d: Protocol) {.async.} = trace "refreshLoop canceled" proc ipMajorityLoop(d: Protocol) {.async.} = + #TODO this should be handled by libp2p, not the DHT ## When `enrAutoUpdate` is enabled, the IP:port combination returned ## by the majority will be used to update the local SPR. ## This should be safe as long as the routing table is not overwhelmed by @@ -1008,6 +1009,33 @@ proc newProtocol*( result.transport = newTransport(result, privKey, node, bindPort, bindIp, rng) +proc newProtocol*( + privKey: PrivateKey, + bindPort: Port, + record: SignedPeerRecord, + bootstrapRecords: openArray[SignedPeerRecord] = [], + bindIp = IPv4_any(), + config = defaultDiscoveryConfig, + rng = newRng()): + Protocol = + info "Discovery SPR initialized", seqNum = record.seqNum, uri = toURI(record) + let node = newNode(record).expect("Properly initialized record") + + # TODO Consider whether this should be a Defect + doAssert rng != nil, "RNG initialization failed" + + result = Protocol( + privateKey: privKey, + localNode: node, + bootstrapRecords: @bootstrapRecords, + ipVote: IpVote.init(), + enrAutoUpdate: false, #TODO this should be removed from nim-libp2p-dht + routingTable: RoutingTable.init( + node, config.bitsPerHop, config.tableIpLimits, rng), + rng: rng) + + result.transport = newTransport(result, privKey, node, bindPort, bindIp, rng) + proc open*(d: Protocol) {.raises: [Defect, CatchableError].} = info "Starting discovery node", node = d.localNode diff --git a/libp2pdht/private/eth/p2p/discoveryv5/spr.nim b/libp2pdht/private/eth/p2p/discoveryv5/spr.nim index f95c324..b55bc19 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/spr.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/spr.nim @@ -166,14 +166,14 @@ proc update*(r: var SignedPeerRecord, pk: crypto.PrivateKey, proc toTypedRecord*(r: SignedPeerRecord) : RecordResult[SignedPeerRecord] = ok(r) proc ip*(r: SignedPeerRecord): Option[array[4, byte]] = - let ma = r.data.addresses[0].address - - let code = ma[0].get.protoCode() - if code.isOk and code.get == multiCodec("ip4"): - var ipbuf: array[4, byte] - let res = ma[0].get.protoArgument(ipbuf) - if res.isOk: - return some(ipbuf) + for address in r.data.addresses: + let ma = address.address + let code = ma[0].get.protoCode() + if code.isOk and code.get == multiCodec("ip4"): + var ipbuf: array[4, byte] + let res = ma[0].get.protoArgument(ipbuf) + if res.isOk: + return some(ipbuf) # err("Incorrect IPv4 address") # else: @@ -188,15 +188,16 @@ proc ip*(r: SignedPeerRecord): Option[array[4, byte]] = # err("MultiAddress must be wire address (tcp, udp or unix)") proc udp*(r: SignedPeerRecord): Option[int] = - let ma = r.data.addresses[0].address + for address in r.data.addresses: + let ma = address.address - let code = ma[1].get.protoCode() - if code.isOk and code.get == multiCodec("udp"): - var pbuf: array[2, byte] - let res = ma[1].get.protoArgument(pbuf) - if res.isOk: - let p = fromBytesBE(uint16, pbuf) - return some(p.int) + let code = ma[1].get.protoCode() + if code.isOk and code.get == multiCodec("udp"): + var pbuf: array[2, byte] + let res = ma[1].get.protoArgument(pbuf) + if res.isOk: + let p = fromBytesBE(uint16, pbuf) + return some(p.int) proc fromBase64*(r: var SignedPeerRecord, s: string): bool = ## Loads SPR from base64-encoded protobuf-encoded bytes, and validates the From d90f0a03b9b3bba68b9411d20d1e22ba7c3b4d7f Mon Sep 17 00:00:00 2001 From: Tanguy Date: Thu, 14 Apr 2022 11:58:39 +0200 Subject: [PATCH 2/5] Fixes for json logs --- libp2pdht/private/eth/p2p/discoveryv5/protocol.nim | 9 ++++----- libp2pdht/private/eth/p2p/discoveryv5/transport.nim | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim index 2684053..58fdd74 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim @@ -316,7 +316,7 @@ proc handleTalkReq(d: Protocol, fromId: NodeId, fromAddr: Address, d.sendResponse(fromId, fromAddr, talkresp, reqId) proc addProviderLocal(p: Protocol, cId: NodeId, prov: SignedPeerRecord) = - trace "adding provider to local db", n=p.localNode, cId, prov + trace "adding provider to local db", cid=cId, spr=prov.data p.providers.mgetOrPut(cId, @[]).add(prov) proc handleAddProvider(d: Protocol, fromId: NodeId, fromAddr: Address, @@ -328,7 +328,7 @@ proc handleGetProviders(d: Protocol, fromId: NodeId, fromAddr: Address, #TODO: add checks, add signed version let provs = d.providers.getOrDefault(getProviders.cId) - trace "providers:", provs + trace "providers:", prov=provs.mapIt(it.data) ##TODO: handle multiple messages let response = ProvidersMessage(total: 1, provs: provs) @@ -692,7 +692,7 @@ proc getProviders*( # What providers do we know about? var res = d.getProvidersLocal(cId, maxitems) - trace "local providers:", res + trace "local providers:", prov=res.mapIt(it.data) let nodesNearby = await d.lookup(cId) trace "nearby:", nodesNearby @@ -710,7 +710,6 @@ proc getProviders*( providersFut.del(index) let providersMsg2 = await providersMsg - trace "2", providersMsg2 let providersMsgRes = providersMsg.read if providersMsgRes.isOk: @@ -720,7 +719,7 @@ proc getProviders*( error "Sending of GetProviders message failed", error = providersMsgRes.error # TODO: should we consider this as an error result if all GetProviders # requests fail?? - trace "getProviders collected: ", res + trace "getProviders collected: ", res=res.mapIt(it.data) return ok res diff --git a/libp2pdht/private/eth/p2p/discoveryv5/transport.nim b/libp2pdht/private/eth/p2p/discoveryv5/transport.nim index 4faf0bb..3fd1d84 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/transport.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/transport.nim @@ -110,7 +110,7 @@ proc receive*(t: Transport, a: Address, packet: openArray[byte]) = if packet.messageOpt.isSome(): let message = packet.messageOpt.get() trace "Received message packet", srcId = packet.srcId, address = a, - kind = message.kind, packet + kind = message.kind, p = $packet t.client.handleMessage(packet.srcId, a, message) else: trace "Not decryptable message packet received", From 2d93fa9e698ccbe4074676f32e7e432a6161ce05 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Mon, 12 Sep 2022 16:22:56 -0600 Subject: [PATCH 3/5] fix `updateRecord` - support incrementing seqNo - support updating with new record --- .../private/eth/p2p/discoveryv5/protocol.nim | 21 +++++++-- libp2pdht/private/eth/p2p/discoveryv5/spr.nim | 43 ++++++++++--------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim index 58fdd74..c26dae6 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/protocol.nim @@ -222,16 +222,29 @@ func getRecord*(d: Protocol): SignedPeerRecord = ## Get the SPR of the local node. d.localNode.record -proc updateRecord*(d: Protocol, newSpr: SignedPeerRecord): DiscResult[void] = +proc updateRecord*( + d: Protocol, + spr: Option[SignedPeerRecord] = SignedPeerRecord.none): DiscResult[void] = ## Update the ENR of the local node with provided `enrFields` k:v pairs. + ## - info "Updated discovery SPR", uri=toURI(newSpr) - d.localNode.record = newSpr - ok() + if spr.isSome: + let + newSpr = spr.get() + seqNo = d.localNode.record.seqNum + + info "Updated discovery SPR", uri = toURI(newSpr) + + d.localNode.record = newSpr + d.localNode.record.data.seqNo = seqNo + + ? d.localNode.record.incSeqNo(d.privateKey) # TODO: Would it make sense to actively ping ("broadcast") to all the peers # we stored a handshake with in order to get that ENR updated? + ok() + proc sendResponse(d: Protocol, dstId: NodeId, dstAddr: Address, message: SomeMessage, reqId: RequestId) = ## send Response using the specifid reqId diff --git a/libp2pdht/private/eth/p2p/discoveryv5/spr.nim b/libp2pdht/private/eth/p2p/discoveryv5/spr.nim index b55bc19..1e90e22 100644 --- a/libp2pdht/private/eth/p2p/discoveryv5/spr.nim +++ b/libp2pdht/private/eth/p2p/discoveryv5/spr.nim @@ -31,7 +31,6 @@ proc seqNum*(r: SignedPeerRecord): uint64 = r.data.seqNo proc fromBytes(r: var SignedPeerRecord, s: openArray[byte]): bool = - let decoded = SignedPeerRecord.decode(@s) if decoded.isErr: error "Error decoding SignedPeerRecord", error = decoded.error @@ -47,22 +46,21 @@ proc get*(r: SignedPeerRecord, T: type PublicKey): Option[T] = r.envelope.publicKey.some proc incSeqNo*( - r: var SignedPeerRecord, - pk: PrivateKey): RecordResult[void] = + r: var SignedPeerRecord, + pk: PrivateKey): RecordResult[void] = r.data.seqNo.inc() r = ? SignedPeerRecord.init(pk, r.data).mapErr( (e: CryptoError) => - ("Error initialising SignedPeerRecord with incremented seqNo: " & - $e).cstring - ) + ("Error initializing SignedPeerRecord with incremented seqNo: " & $e).cstring) ok() - -proc update*(r: var SignedPeerRecord, pk: crypto.PrivateKey, - ip: Option[ValidIpAddress], - tcpPort, udpPort: Option[Port] = none[Port]()): - RecordResult[void] = +proc update*( + r: var SignedPeerRecord, + pk: crypto.PrivateKey, + ip: Option[ValidIpAddress], + tcpPort, udpPort: Option[Port] = none[Port]()): + RecordResult[void] = ## Update a `SignedPeerRecord` with given ip address, tcp port, udp port and optional ## custom k:v pairs. ## @@ -110,7 +108,6 @@ proc update*(r: var SignedPeerRecord, pk: crypto.PrivateKey, transProtoPort = udpPort.get updated = MultiAddress.init(ipAddr, transProto, transProtoPort) - else: let existing = r.data.addresses[0].address @@ -158,8 +155,7 @@ proc update*(r: var SignedPeerRecord, pk: crypto.PrivateKey, r = ? SignedPeerRecord.init(pk, r.data) .mapErr((e: CryptoError) => - ("Failed to update SignedPeerRecord: " & $e).cstring - ) + ("Failed to update SignedPeerRecord: " & $e).cstring) return ok() @@ -223,11 +219,13 @@ proc toBase64*(r: SignedPeerRecord): string = proc toURI*(r: SignedPeerRecord): string = "spr:" & r.toBase64 -proc init*(T: type SignedPeerRecord, seqNum: uint64, - pk: PrivateKey, - ip: Option[ValidIpAddress], - tcpPort, udpPort: Option[Port]): - RecordResult[T] = +proc init*( + T: type SignedPeerRecord, + seqNum: uint64, + pk: PrivateKey, + ip: Option[ValidIpAddress], + tcpPort, udpPort: Option[Port]): + RecordResult[T] = ## Initialize a `SignedPeerRecord` with given sequence number, private key, optional ## ip address, tcp port, udp port, and optional custom k:v pairs. ## @@ -268,7 +266,9 @@ proc init*(T: type SignedPeerRecord, seqNum: uint64, let ma = MultiAddress.init(ipAddr, proto, protoPort) let pr = PeerRecord.init(peerId, @[ma], seqNum) - SignedPeerRecord.init(pk, pr).mapErr((e: CryptoError) => ("Failed to init SignedPeerRecord: " & $e).cstring) + SignedPeerRecord.init(pk, pr) + .mapErr( + (e: CryptoError) => ("Failed to init SignedPeerRecord: " & $e).cstring) proc contains*(r: SignedPeerRecord, fp: (string, seq[byte])): bool = # TODO: use FieldPair for this, but that is a bit cumbersome. Perhaps the @@ -281,4 +281,5 @@ proc contains*(r: SignedPeerRecord, fp: (string, seq[byte])): bool = debugEcho "`contains` is not yet implemented for SignedPeerRecords" return false -proc `==`*(a, b: SignedPeerRecord): bool = a.data == b.data +proc `==`*(a, b: SignedPeerRecord): bool = + a.data == b.data From 1cbe8c12a61fb0daffb3f5440330eeacd8faf7c9 Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Mon, 12 Sep 2022 16:26:29 -0600 Subject: [PATCH 4/5] suppress logging noise --- tests/nim.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/nim.cfg b/tests/nim.cfg index cf48491..11b328d 100644 --- a/tests/nim.cfg +++ b/tests/nim.cfg @@ -1,3 +1,4 @@ --path:".." --threads:on --tlsEmulation:off +-d:chronicles_enabled=off From 3be2c8445c090cff784710fe9dfaeefe77b103cc Mon Sep 17 00:00:00 2001 From: Dmitriy Ryajov Date: Mon, 12 Sep 2022 16:27:48 -0600 Subject: [PATCH 5/5] suppress logging noise --- tests/discv5/test_discoveryv5.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/discv5/test_discoveryv5.nim b/tests/discv5/test_discoveryv5.nim index eb6d9ef..61177b1 100644 --- a/tests/discv5/test_discoveryv5.nim +++ b/tests/discv5/test_discoveryv5.nim @@ -150,7 +150,7 @@ suite "Discovery v5 Tests": privKey = PrivateKey.fromHex(key).expect("Valid private key hex") pubKey = privKey.getPublicKey.expect("Valid private key for public key") id = pubKey.toNodeId.expect("Public key valid for node id") - debugEcho ">>> key: ", key + # debugEcho ">>> key: ", key check logDistance(targetId, id) == d test "Distance to id check":