From 1c39de7fbc29e12aa06c8eab3639e1836c8a6d9d Mon Sep 17 00:00:00 2001 From: Lorenzo Delgado Date: Fri, 3 Mar 2023 10:07:25 +0100 Subject: [PATCH] refactor(message): move namespacing utils to waku message module --- apps/wakubridge/message_compat.nim | 32 ++++---- tests/all_tests_v2.nim | 2 +- tests/v2/test_namespacing_utils.nim | 46 ----------- tests/v2/test_waku_message_topics.nim | 99 +++++++++++++++++++++++ tests/wakubridge/test_message_compat.nim | 3 +- waku/v2/protocol/waku_message.nim | 2 + waku/v2/protocol/waku_message/codec.nim | 1 + waku/v2/protocol/waku_message/digest.nim | 1 + waku/v2/protocol/waku_message/message.nim | 12 +-- waku/v2/protocol/waku_message/topics.nim | 94 +++++++++++++++++++++ waku/v2/utils/namespacing.nim | 56 ------------- 11 files changed, 218 insertions(+), 130 deletions(-) delete mode 100644 tests/v2/test_namespacing_utils.nim create mode 100644 tests/v2/test_waku_message_topics.nim create mode 100644 waku/v2/protocol/waku_message/topics.nim delete mode 100644 waku/v2/utils/namespacing.nim diff --git a/apps/wakubridge/message_compat.nim b/apps/wakubridge/message_compat.nim index 7a7e5e153..d952f636a 100644 --- a/apps/wakubridge/message_compat.nim +++ b/apps/wakubridge/message_compat.nim @@ -4,15 +4,11 @@ else: {.push raises: [].} import - stew/byteutils + stew/[byteutils, results], + libp2p/crypto/crypto import - # Waku v1 imports ../../waku/v1/protocol/waku_protocol, - # Waku v2 imports - libp2p/crypto/crypto, - ../../waku/v2/utils/namespacing, - ../../waku/v2/protocol/waku_message, - ../../waku/v2/node/peer_manager + ../../waku/v2/protocol/waku_message const @@ -20,11 +16,17 @@ const ContentTopicAppVersion = "1" -proc toV1Topic*(contentTopic: ContentTopic): waku_protocol.Topic {.raises: [LPError, ValueError]} = +proc toV1Topic*(contentTopic: ContentTopic): waku_protocol.Topic {.raises: [ValueError]} = ## Extracts the 4-byte array v1 topic from a content topic ## with format `/waku/1//rfc26` - hexToByteArray(hexStr = NamespacedTopic.fromString(contentTopic).tryGet().topicName, N = 4) # Byte array length + let ns = NamespacedTopic.parse(contentTopic) + if ns.isErr(): + let err = ns.tryError() + raise newException(ValueError, $err) + + let name = ns.value.name + hexToByteArray(hexStr=name, N=4) # Byte array length proc toV2ContentTopic*(v1Topic: waku_protocol.Topic): ContentTopic = ## Convert a 4-byte array v1 topic to a namespaced content topic @@ -35,7 +37,7 @@ proc toV2ContentTopic*(v1Topic: waku_protocol.Topic): ContentTopic = namespacedTopic.application = ContentTopicApplication namespacedTopic.version = ContentTopicAppVersion - namespacedTopic.topicName = "0x" & v1Topic.toHex() + namespacedTopic.name = "0x" & v1Topic.toHex() namespacedTopic.encoding = "rfc26" return ContentTopic($namespacedTopic) @@ -43,10 +45,8 @@ proc toV2ContentTopic*(v1Topic: waku_protocol.Topic): ContentTopic = proc isBridgeable*(msg: WakuMessage): bool = ## Determines if a Waku v2 msg is on a bridgeable content topic + let ns = NamespacedTopic.parse(msg.contentTopic) + if ns.isErr(): + return false - let ns = NamespacedTopic.fromString(msg.contentTopic) - if ns.isOk(): - if ns.get().application == ContentTopicApplication and ns.get().version == ContentTopicAppVersion: - return true - - return false + return ns.value.application == ContentTopicApplication and ns.value.version == ContentTopicAppVersion diff --git a/tests/all_tests_v2.nim b/tests/all_tests_v2.nim index 443d503d9..480916208 100644 --- a/tests/all_tests_v2.nim +++ b/tests/all_tests_v2.nim @@ -8,6 +8,7 @@ import ./all_tests_common # Waku message test suite import + ./v2/test_waku_message_topics, ./v2/test_waku_message_digest # Waku archive test suite @@ -54,7 +55,6 @@ import ./v2/test_peer_manager, ./v2/test_peer_storage, ./v2/test_waku_keepalive, - ./v2/test_namespacing_utils, ./v2/test_waku_dnsdisc, ./v2/test_waku_discv5, ./v2/test_enr_utils, diff --git a/tests/v2/test_namespacing_utils.nim b/tests/v2/test_namespacing_utils.nim deleted file mode 100644 index b5602a73a..000000000 --- a/tests/v2/test_namespacing_utils.nim +++ /dev/null @@ -1,46 +0,0 @@ -{.used.} - -import - stew/results, - testutils/unittests -import - ../../waku/v2/utils/namespacing - -suite "Namespacing utils": - - test "Create from string": - # Expected case - let nsRes = NamespacedTopic.fromString("/waku/2/default-waku/proto") - - require nsRes.isOk() - - let ns = nsRes.get() - - check: - ns.application == "waku" - ns.version == "2" - ns.topicName == "default-waku" - ns.encoding == "proto" - - test "Invalid string - Topic is not namespaced": - check NamespacedTopic.fromString("this-is-not-namespaced").isErr() - - test "Invalid string - Topic should start with slash": - check NamespacedTopic.fromString("waku/2/default-waku/proto").isErr() - - test "Invalid string - Topic has too few parts": - check NamespacedTopic.fromString("/waku/2/default-waku").isErr() - - test "Invalid string - Topic has too many parts": - check NamespacedTopic.fromString("/waku/2/default-waku/proto/2").isErr() - - test "Stringify namespaced topic": - var ns = NamespacedTopic() - - ns.application = "waku" - ns.version = "2" - ns.topicName = "default-waku" - ns.encoding = "proto" - - check: - $ns == "/waku/2/default-waku/proto" diff --git a/tests/v2/test_waku_message_topics.nim b/tests/v2/test_waku_message_topics.nim new file mode 100644 index 000000000..1e46ef922 --- /dev/null +++ b/tests/v2/test_waku_message_topics.nim @@ -0,0 +1,99 @@ +{.used.} + +import + stew/results, + testutils/unittests +import + ../../waku/v2/protocol/waku_message/topics + +suite "Waku Message - Topics namespacing": + + test "Stringify namespaced topic": + ## Given + var ns = NamespacedTopic() + ns.application = "waku" + ns.version = "2" + ns.name = "default-waku" + ns.encoding = "proto" + + ## When + let topic = $ns + + ## Then + check: + topic == "/waku/2/default-waku/proto" + + test "Parse topic string - Valid string": + ## Given + let topic = "/waku/2/default-waku/proto" + + ## When + let nsRes = NamespacedTopic.parse(topic) + + ## Then + check nsRes.isOk() + + let ns = nsRes.get() + check: + ns.application == "waku" + ns.version == "2" + ns.name == "default-waku" + ns.encoding == "proto" + + test "Parse topic string - Invalid string: doesn't start with slash": + ## Given + let topic = "waku/2/default-waku/proto" + + ## When + let ns = NamespacedTopic.parse(topic) + + ## Then + check ns.isErr() + let err = ns.tryError() + check: + err.kind == NamespacingErrorKind.InvalidFormat + err.cause == "topic must start with slash" + + test "Parse topic string - Invalid string: not namespaced": + ## Given + let topic = "/this-is-not-namespaced" + + ## When + let ns = NamespacedTopic.parse(topic) + + ## Then + check ns.isErr() + let err = ns.tryError() + check: + err.kind == NamespacingErrorKind.InvalidFormat + err.cause == "invalid topic structure" + + + test "Parse topic string - Invalid string: missing encoding part": + ## Given + let topic = "/waku/2/default-waku" + + ## When + let ns = NamespacedTopic.parse(topic) + + ## Then + check ns.isErr() + let err = ns.tryError() + check: + err.kind == NamespacingErrorKind.InvalidFormat + err.cause == "invalid topic structure" + + test "Parse topic string - Invalid string: too many parts": + ## Given + let topic = "/waku/2/default-waku/proto/33" + + ## When + let ns = NamespacedTopic.parse(topic) + + ## Then + check ns.isErr() + let err = ns.tryError() + check: + err.kind == NamespacingErrorKind.InvalidFormat + err.cause == "invalid topic structure" + diff --git a/tests/wakubridge/test_message_compat.nim b/tests/wakubridge/test_message_compat.nim index ee06a9f7d..6be19e524 100644 --- a/tests/wakubridge/test_message_compat.nim +++ b/tests/wakubridge/test_message_compat.nim @@ -30,7 +30,8 @@ suite "WakuBridge - Message compat": # Invalid cases - expect LPError: + test "Invalid topics conversion between Waku v1 and Waku v2 fails": + expect ValueError: # Content topic not namespaced discard toV1Topic(ContentTopic("this-is-my-content")) diff --git a/waku/v2/protocol/waku_message.nim b/waku/v2/protocol/waku_message.nim index 2a3d85a3e..642776715 100644 --- a/waku/v2/protocol/waku_message.nim +++ b/waku/v2/protocol/waku_message.nim @@ -1,9 +1,11 @@ import + ./waku_message/topics, ./waku_message/message, ./waku_message/codec, ./waku_message/digest export + topics, message, codec, digest diff --git a/waku/v2/protocol/waku_message/codec.nim b/waku/v2/protocol/waku_message/codec.nim index 7f47dae47..b8e18deba 100644 --- a/waku/v2/protocol/waku_message/codec.nim +++ b/waku/v2/protocol/waku_message/codec.nim @@ -11,6 +11,7 @@ else: import ../../../common/protobuf, ../../utils/time, + ./topics, ./message diff --git a/waku/v2/protocol/waku_message/digest.nim b/waku/v2/protocol/waku_message/digest.nim index 672c5e529..cd5beb0cb 100644 --- a/waku/v2/protocol/waku_message/digest.nim +++ b/waku/v2/protocol/waku_message/digest.nim @@ -9,6 +9,7 @@ import stew/byteutils, nimcrypto/sha2 import + ./topics, ./message diff --git a/waku/v2/protocol/waku_message/message.nim b/waku/v2/protocol/waku_message/message.nim index 56a62207a..14b2df754 100644 --- a/waku/v2/protocol/waku_message/message.nim +++ b/waku/v2/protocol/waku_message/message.nim @@ -10,7 +10,8 @@ else: import - ../../utils/time + ../../utils/time, + ./topics const MaxMetaAttrLength* = 32 # 32 bytes @@ -18,15 +19,6 @@ const MaxWakuMessageSize* = 1024 * 1024 # 1 Mibytes. Corresponds to PubSub default -type - PubsubTopic* = string - ContentTopic* = string - -const - DefaultPubsubTopic*: PubsubTopic = PubsubTopic("/waku/2/default-waku/proto") - DefaultContentTopic*: ContentTopic = ContentTopic("/waku/2/default-content/proto") - - type WakuMessage* = object # Data payload transmitted. payload*: seq[byte] diff --git a/waku/v2/protocol/waku_message/topics.nim b/waku/v2/protocol/waku_message/topics.nim new file mode 100644 index 000000000..072c40e2f --- /dev/null +++ b/waku/v2/protocol/waku_message/topics.nim @@ -0,0 +1,94 @@ +## Waku topics definition and namespacing utils +## +## See 14/WAKU2-MESSAGE RFC: https://rfc.vac.dev/spec/14/ +## See 23/WAKU2-TOPICS RFC: https://rfc.vac.dev/spec/23/ + +when (NimMajor, NimMinor) < (1, 4): + {.push raises: [Defect].} +else: + {.push raises: [].} + + +import + std/strutils, + stew/results + + +## Topics + +type + PubsubTopic* = string + ContentTopic* = string + +const + DefaultPubsubTopic* = PubsubTopic("/waku/2/default-waku/proto") + DefaultContentTopic* = ContentTopic("/waku/2/default-content/proto") + + +## Namespacing + +type + NamespacedTopic* = object + application*: string + version*: string + name*: string + encoding*: string + +type + NamespacingErrorKind* {.pure.} = enum + InvalidFormat + + NamespacingError* = object + case kind*: NamespacingErrorKind + of InvalidFormat: + cause*: string + + NamespacingResult*[T] = Result[T, NamespacingError] + + +proc invalidFormat(T: type NamespacingError, cause = "invalid format"): T = + NamespacingError(kind: NamespacingErrorKind.InvalidFormat, cause: cause) + +proc `$`*(err: NamespacingError): string = + case err.kind: + of NamespacingErrorKind.InvalidFormat: + return "invalid format: " & err.cause + + +proc parse*(T: type NamespacedTopic, topic: PubsubTopic|ContentTopic|string): NamespacingResult[NamespacedTopic] = + ## Splits a namespaced topic string into its constituent parts. + ## The topic string has to be in the format `////` + + if not topic.startsWith("/"): + return err(NamespacingError.invalidFormat("topic must start with slash")) + + let parts = topic.split('/') + + # Check that we have an expected number of substrings + if parts.len != 5: + return err(NamespacingError.invalidFormat("invalid topic structure")) + + let namespaced = NamespacedTopic( + application: parts[1], + version: parts[2], + name: parts[3], + encoding: parts[4] + ) + + return ok(namespaced) + +proc `$`*(namespacedTopic: NamespacedTopic): string = + ## Returns a string representation of a namespaced topic + ## in the format `////` + var str = newString(0) + + str.add("/") + str.add(namespacedTopic.application) + str.add("/") + str.add(namespacedTopic.version) + str.add("/") + str.add(namespacedTopic.name) + str.add("/") + str.add(namespacedTopic.encoding) + + return str diff --git a/waku/v2/utils/namespacing.nim b/waku/v2/utils/namespacing.nim deleted file mode 100644 index 0dae3e6a2..000000000 --- a/waku/v2/utils/namespacing.nim +++ /dev/null @@ -1,56 +0,0 @@ -## Collection of utilities related to namespaced topics -## Implemented according to the specified Waku v2 Topic Usage Recommendations -## More at https://rfc.vac.dev/spec/23/ - -{.push raises: [Defect]} - -import - std/strutils, - stew/results - -type - NamespacedTopic* = object - application*: string - version*: string - topicName*: string - encoding*: string - - NamespacingResult*[T] = Result[T, string] - -proc fromString*(T: type NamespacedTopic, topic: string): NamespacingResult[NamespacedTopic] = - ## Splits a namespaced topic string into its constituent parts. - ## The topic string has to be in the format `////` - - let parts = topic.split('/') - - if parts.len != 5: - # Check that we have an expected number of substrings - return err("invalid topic format") - - if parts[0] != "": - # Ensures that topic starts with a "/" - return err("invalid topic format") - - let namespacedTopic= NamespacedTopic(application: parts[1], - version: parts[2], - topicName: parts[3], - encoding: parts[4]) - - return ok(namespacedTopic) - -proc `$`*(namespacedTopic: NamespacedTopic): string = - ## Returns a string representation of a namespaced topic - ## in the format `////` - - var topicStr = newString(0) - - topicStr.add("/") - topicStr.add(namespacedTopic.application) - topicStr.add("/") - topicStr.add(namespacedTopic.version) - topicStr.add("/") - topicStr.add(namespacedTopic.topicName) - topicStr.add("/") - topicStr.add(namespacedTopic.encoding) - - return topicStr