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 <ot@oskarthoren.com>

* Update waku/v2/protocol/waku_swap/waku_swap.nim

Co-authored-by: oskarth <ot@oskarthoren.com>

* 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 <ot@oskarthoren.com>

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)

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)

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
 

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.}
+{.pop.}

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

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())

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)

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

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")

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)