From a41b9ac80b5589a11ac9661b9fa099dff4177e8c Mon Sep 17 00:00:00 2001 From: Sanaz Taheri Boshrooyeh <35961250+staheri14@users.noreply.github.com> Date: Thu, 15 Jul 2021 11:25:52 -0700 Subject: [PATCH] Refactor waku protocols and utils (#667) * refactors waku_filter * refactors waku_lightpush * refactors waku_swap * refactor namespacing.nim * refactor peers * refactors requests * adds top level {.push raises: [Defect].} * log scope for rln relay * cleans up comments * removes comments * comments out raise Defect * defines temp var then pass to constructor * Update waku/v2/protocol/waku_rln_relay/rln.nim Co-authored-by: oskarth * Update waku/v2/protocol/waku_swap/waku_swap.nim Co-authored-by: oskarth * explains the potential exception in waku_swap * creates temp var before return * adjusts spaces * adds line breaks, temp vars and fixes format * removes type declaration * fixes and indentation issue * adjusts spacing * adjusts line <80ch * formating improvement Co-authored-by: oskarth --- waku/v2/protocol/waku_filter/waku_filter.nim | 51 +++++++++++-------- .../waku_lightpush/waku_lightpush.nim | 46 ++++++++++------- waku/v2/protocol/waku_message.nim | 2 + waku/v2/protocol/waku_rln_relay/rln.nim | 4 +- .../waku_rln_relay/waku_rln_relay_utils.nim | 5 ++ waku/v2/protocol/waku_store/waku_store.nim | 12 ++--- waku/v2/protocol/waku_swap/waku_swap.nim | 50 ++++++++++++------ waku/v2/utils/namespacing.nim | 6 ++- waku/v2/utils/peers.nim | 2 +- waku/v2/utils/requests.nim | 2 +- 10 files changed, 113 insertions(+), 67 deletions(-) diff --git a/waku/v2/protocol/waku_filter/waku_filter.nim b/waku/v2/protocol/waku_filter/waku_filter.nim index 6c2d5f910..8cfea3e0b 100644 --- a/waku/v2/protocol/waku_filter/waku_filter.nim +++ b/waku/v2/protocol/waku_filter/waku_filter.nim @@ -73,19 +73,22 @@ proc unsubscribeFilters(subscribers: var seq[Subscriber], request: FilterRequest # @TODO: metrics? proc encode*(filter: ContentFilter): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, filter.contentTopic) + output.write(1, filter.contentTopic) + return output proc encode*(rpc: FilterRequest): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, uint64(rpc.subscribe)) + output.write(1, uint64(rpc.subscribe)) - result.write(2, rpc.pubSubTopic) + output.write(2, rpc.pubSubTopic) for filter in rpc.contentFilters: - result.write(3, filter.encode()) + output.write(3, filter.encode()) + + return output proc init*(T: type ContentFilter, buffer: seq[byte]): ProtoResult[T] = let pb = initProtoBuffer(buffer) @@ -93,7 +96,7 @@ proc init*(T: type ContentFilter, buffer: seq[byte]): ProtoResult[T] = var contentTopic: ContentTopic discard ? pb.getField(1, contentTopic) - ok(ContentFilter(contentTopic: contentTopic)) + return ok(ContentFilter(contentTopic: contentTopic)) proc init*(T: type FilterRequest, buffer: seq[byte]): ProtoResult[T] = var rpc = FilterRequest(contentFilters: @[], pubSubTopic: "") @@ -111,13 +114,15 @@ proc init*(T: type FilterRequest, buffer: seq[byte]): ProtoResult[T] = for buf in buffs: rpc.contentFilters.add(? ContentFilter.init(buf)) - ok(rpc) + return ok(rpc) proc encode*(push: MessagePush): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() for push in push.messages: - result.write(1, push.encode()) + output.write(1, push.encode()) + + return output proc init*(T: type MessagePush, buffer: seq[byte]): ProtoResult[T] = var push = MessagePush() @@ -129,7 +134,7 @@ proc init*(T: type MessagePush, buffer: seq[byte]): ProtoResult[T] = for buf in messages: push.messages.add(? WakuMessage.init(buf)) - ok(push) + return ok(push) proc init*(T: type FilterRPC, buffer: seq[byte]): ProtoResult[T] = var rpc = FilterRPC() @@ -147,14 +152,16 @@ proc init*(T: type FilterRPC, buffer: seq[byte]): ProtoResult[T] = rpc.push = ? MessagePush.init(pushBuffer) - ok(rpc) + return ok(rpc) proc encode*(rpc: FilterRPC): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, rpc.requestId) - result.write(2, rpc.request.encode()) - result.write(3, rpc.push.encode()) + output.write(1, rpc.requestId) + output.write(2, rpc.request.encode()) + output.write(3, rpc.push.encode()) + + return output method init*(wf: WakuFilter) = proc handle(conn: Connection, proto: string) {.async, gcsafe, closure.} = @@ -182,11 +189,13 @@ method init*(wf: WakuFilter) = wf.codec = WakuFilterCodec proc init*(T: type WakuFilter, peerManager: PeerManager, rng: ref BrHmacDrbgContext, handler: MessagePushHandler): T = - new result - result.rng = crypto.newRng() - result.peerManager = peerManager - result.pushHandler = handler - result.init() + let rng = crypto.newRng() + var wf = WakuFilter(rng: rng, + peerManager: peerManager, + pushHandler: handler) + wf.init() + + return wf proc setPeer*(wf: WakuFilter, peer: PeerInfo) = wf.peerManager.addPeer(peer, WakuFilterCodec) diff --git a/waku/v2/protocol/waku_lightpush/waku_lightpush.nim b/waku/v2/protocol/waku_lightpush/waku_lightpush.nim index be5af45ed..f3afa991f 100644 --- a/waku/v2/protocol/waku_lightpush/waku_lightpush.nim +++ b/waku/v2/protocol/waku_lightpush/waku_lightpush.nim @@ -34,10 +34,12 @@ const # Encoding and decoding ------------------------------------------------------- proc encode*(rpc: PushRequest): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, rpc.pubSubTopic) - result.write(2, rpc.message.encode()) + output.write(1, rpc.pubSubTopic) + output.write(2, rpc.message.encode()) + + return output proc init*(T: type PushRequest, buffer: seq[byte]): ProtoResult[T] = #var rpc = PushRequest(pubSubTopic: "", message: WakuMessage()) @@ -52,13 +54,15 @@ proc init*(T: type PushRequest, buffer: seq[byte]): ProtoResult[T] = discard ? pb.getField(2, buf) rpc.message = ? WakuMessage.init(buf) - ok(rpc) + return ok(rpc) proc encode*(rpc: PushResponse): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, uint64(rpc.isSuccess)) - result.write(2, rpc.info) + output.write(1, uint64(rpc.isSuccess)) + output.write(2, rpc.info) + + return output proc init*(T: type PushResponse, buffer: seq[byte]): ProtoResult[T] = var rpc = PushResponse(isSuccess: false, info: "") @@ -72,14 +76,16 @@ proc init*(T: type PushResponse, buffer: seq[byte]): ProtoResult[T] = discard ? pb.getField(2, info) rpc.info = info - ok(rpc) + return ok(rpc) proc encode*(rpc: PushRPC): ProtoBuffer = - result = initProtoBuffer() + var output = initProtoBuffer() - result.write(1, rpc.requestId) - result.write(2, rpc.request.encode()) - result.write(3, rpc.response.encode()) + output.write(1, rpc.requestId) + output.write(2, rpc.request.encode()) + output.write(3, rpc.response.encode()) + + return output proc init*(T: type PushRPC, buffer: seq[byte]): ProtoResult[T] = var rpc = PushRPC() @@ -97,17 +103,19 @@ proc init*(T: type PushRPC, buffer: seq[byte]): ProtoResult[T] = rpc.response = ? PushResponse.init(pushBuffer) - ok(rpc) + return ok(rpc) # Protocol ------------------------------------------------------- proc init*(T: type WakuLightPush, peerManager: PeerManager, rng: ref BrHmacDrbgContext, handler: PushRequestHandler, relay: WakuRelay = nil): T = debug "init" - new result - result.rng = crypto.newRng() - result.peerManager = peerManager - result.requestHandler = handler - result.relayReference = relay - result.init() + let rng = crypto.newRng() + var wl = WakuLightPush(rng: rng, + peerManager: peerManager, + requestHandler: handler, + relayReference: relay) + wl.init() + + return wl proc setPeer*(wlp: WakuLightPush, peer: PeerInfo) = wlp.peerManager.addPeer(peer, WakuLightPushCodec) diff --git a/waku/v2/protocol/waku_message.nim b/waku/v2/protocol/waku_message.nim index 73ea690f8..060529115 100644 --- a/waku/v2/protocol/waku_message.nim +++ b/waku/v2/protocol/waku_message.nim @@ -6,6 +6,8 @@ ## For payload content and encryption, see waku/v2/node/waku_payload.nim +{.push raises: [Defect].} + import libp2p/protobuf/minprotobuf diff --git a/waku/v2/protocol/waku_rln_relay/rln.nim b/waku/v2/protocol/waku_rln_relay/rln.nim index 51fe2ba28..1d6fea62a 100644 --- a/waku/v2/protocol/waku_rln_relay/rln.nim +++ b/waku/v2/protocol/waku_rln_relay/rln.nim @@ -1,5 +1,7 @@ # this module contains the Nim wrappers for the rln library https://github.com/kilic/rln/blob/3bbec368a4adc68cd5f9bfae80b17e1bbb4ef373/src/ffi.rs +{.push raises: [Defect].} + import os, waku_rln_relay_types @@ -58,4 +60,4 @@ proc hash*(ctx: ptr RLN[Bn256], inputs_buffer: ptr Buffer, input_len: uint, output_buffer: ptr Buffer): bool {.importc: "hash".} -{.pop.} \ No newline at end of file +{.pop.} diff --git a/waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim b/waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim index e6b5b86ea..b54d6bae7 100644 --- a/waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim +++ b/waku/v2/protocol/waku_rln_relay/waku_rln_relay_utils.nim @@ -1,3 +1,5 @@ +{.push raises: [Defect].} + import chronicles, options, chronos, stint, web3, @@ -5,6 +7,9 @@ import rln, waku_rln_relay_types +logScope: + topics = "wakurlnrelayutils" + # membership contract interface contract(MembershipContract): # TODO define a return type of bool for register method to signify a successful registration diff --git a/waku/v2/protocol/waku_store/waku_store.nim b/waku/v2/protocol/waku_store/waku_store.nim index 49f5caf3e..32d5e1f1c 100644 --- a/waku/v2/protocol/waku_store/waku_store.nim +++ b/waku/v2/protocol/waku_store/waku_store.nim @@ -54,7 +54,7 @@ proc encode*(index: Index): ProtoBuffer = ## returns the resultant ProtoBuffer # intiate a ProtoBuffer - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() # encodes index output.write(1, index.digest.data) @@ -68,7 +68,7 @@ proc encode*(pinfo: PagingInfo): ProtoBuffer = ## returns the resultant ProtoBuffer # intiate a ProtoBuffer - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() # encodes pinfo output.write(1, pinfo.pageSize) @@ -193,12 +193,12 @@ proc init*(T: type HistoryRPC, buffer: seq[byte]): ProtoResult[T] = return ok(rpc) proc encode*(filter: HistoryContentFilter): ProtoBuffer = - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() output.write(1, filter.contentTopic) return output proc encode*(query: HistoryQuery): ProtoBuffer = - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() output.write(2, query.pubsubTopic) @@ -214,7 +214,7 @@ proc encode*(query: HistoryQuery): ProtoBuffer = proc encode*(response: HistoryResponse): ProtoBuffer = - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() for msg in response.messages: output.write(1, msg.encode()) @@ -226,7 +226,7 @@ proc encode*(response: HistoryResponse): ProtoBuffer = return output proc encode*(rpc: HistoryRPC): ProtoBuffer = - var output: ProtoBuffer = initProtoBuffer() + var output = initProtoBuffer() output.write(1, rpc.requestId) output.write(2, rpc.query.encode()) diff --git a/waku/v2/protocol/waku_swap/waku_swap.nim b/waku/v2/protocol/waku_swap/waku_swap.nim index 8f20222e7..70266a42e 100644 --- a/waku/v2/protocol/waku_swap/waku_swap.nim +++ b/waku/v2/protocol/waku_swap/waku_swap.nim @@ -21,6 +21,13 @@ ## Things like settlement is for future work. ## +# Accessing Table's items is prone to KeyError exception when the key does not belong to the table +# such exception can be avoided by calling hasKey() before accessing the key (which is the case in this module) +# but from the compiler point of view, the use of hasKey() does not make any difference in the potential exceptions +# @TODO thus any key access should be wrapped inside try-except +# @TODO or otherwise the exception should be thrown by the proc and handled by the higher level calls +# {.push raises: [Defect].} + import std/[tables, options, json], bearssl, @@ -54,15 +61,21 @@ const # Serialization # ------------------------------------------------------------------------------- proc encode*(handshake: Handshake): ProtoBuffer = - result = initProtoBuffer() - result.write(1, handshake.beneficiary) + var output = initProtoBuffer() + + output.write(1, handshake.beneficiary) + + return output proc encode*(cheque: Cheque): ProtoBuffer = - result = initProtoBuffer() - result.write(1, cheque.beneficiary) - result.write(2, cheque.date) - result.write(3, cheque.amount) - result.write(4, cheque.signature) + var output = initProtoBuffer() + + output.write(1, cheque.beneficiary) + output.write(2, cheque.date) + output.write(3, cheque.amount) + output.write(4, cheque.signature) + + return output proc init*(T: type Handshake, buffer: seq[byte]): ProtoResult[T] = var beneficiary: seq[byte] @@ -71,7 +84,7 @@ proc init*(T: type Handshake, buffer: seq[byte]): ProtoResult[T] = discard ? pb.getField(1, handshake.beneficiary) - ok(handshake) + return ok(handshake) proc init*(T: type Cheque, buffer: seq[byte]): ProtoResult[T] = var beneficiary: seq[byte] @@ -86,7 +99,7 @@ proc init*(T: type Cheque, buffer: seq[byte]): ProtoResult[T] = discard ? pb.getField(3, cheque.amount) discard ? pb.getField(4, cheque.signature) - ok(cheque) + return ok(cheque) # Accounting # ------------------------------------------------------------------------------- @@ -263,13 +276,18 @@ proc init*(wakuSwap: WakuSwap) = # TODO Expression return? proc init*(T: type WakuSwap, peerManager: PeerManager, rng: ref BrHmacDrbgContext, swapConfig: SwapConfig): T = info "wakuSwap init 2" - new result - result.rng = rng - result.peerManager = peerManager - result.accounting = initTable[PeerId, int]() - result.text = "test" - result.config = swapConfig - result.init() + let + accounting = initTable[PeerId, int]() + text = "test" + + var ws = WakuSwap(rng: rng, + peerManager: peerManager, + accounting: accounting, + text: text, + config: swapConfig) + ws.init() + + return ws proc setPeer*(ws: WakuSwap, peer: PeerInfo) = ws.peerManager.addPeer(peer, WakuSwapCodec) diff --git a/waku/v2/utils/namespacing.nim b/waku/v2/utils/namespacing.nim index fc183ea12..0dae3e6a2 100644 --- a/waku/v2/utils/namespacing.nim +++ b/waku/v2/utils/namespacing.nim @@ -31,10 +31,12 @@ proc fromString*(T: type NamespacedTopic, topic: string): NamespacingResult[Name # Ensures that topic starts with a "/" return err("invalid topic format") - ok(NamespacedTopic(application: parts[1], + let namespacedTopic= NamespacedTopic(application: parts[1], version: parts[2], topicName: parts[3], - encoding: parts[4])) + encoding: parts[4]) + + return ok(namespacedTopic) proc `$`*(namespacedTopic: NamespacedTopic): string = ## Returns a string representation of a namespaced topic diff --git a/waku/v2/utils/peers.nim b/waku/v2/utils/peers.nim index 56d480fc0..a5c478bae 100644 --- a/waku/v2/utils/peers.nim +++ b/waku/v2/utils/peers.nim @@ -10,7 +10,7 @@ proc initAddress(T: type MultiAddress, str: string): T {.raises: [Defect, ValueE # @TODO: Rather than raising exceptions, this should return a Result let address = MultiAddress.init(str).tryGet() if IPFS.match(address) and matchPartial(multiaddress.TCP, address): - result = address + return address else: raise newException(ValueError, "Invalid bootstrap node multi-address") diff --git a/waku/v2/utils/requests.nim b/waku/v2/utils/requests.nim index e49b1c2ea..f0da4962c 100644 --- a/waku/v2/utils/requests.nim +++ b/waku/v2/utils/requests.nim @@ -5,4 +5,4 @@ import bearssl, stew/byteutils proc generateRequestId*(rng: ref BrHmacDrbgContext): string = var bytes: array[10, byte] brHmacDrbgGenerate(rng[], bytes) - toHex(bytes) + return toHex(bytes)