Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(enr): added enr builder waku capabilities extension #1599

Merged
merged 1 commit into from
Mar 10, 2023
Merged

feat(enr): added enr builder waku capabilities extension #1599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/common/test_enr_builder.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{.used.}

import
std/options ,
std/options,
stew/results,
stew/shims/net,
testutils/unittests
Expand Down
33 changes: 30 additions & 3 deletions tests/v2/test_waku_enr.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,31 @@ suite "Waku ENR - Capabilities bitfield":
check:
caps == @[Capabilities.Relay, Capabilities.Filter, Capabilities.Lightpush]

test "encode and extract capabilities from record":
test "encode and extract capabilities from record (EnrBuilder ext)":
## Given
let
enrSeqNum = 1u64
enrPrivKey = generatesecp256k1key()

## When
var builder = EnrBuilder.init(enrPrivKey, seqNum = enrSeqNum)
builder.withWakuCapabilities(Capabilities.Relay, Capabilities.Store)

let recordRes = builder.build()

## Then
check recordRes.isOk()
let record = recordRes.tryGet()

let bitfieldRes = record.getCapabilitiesField()
check bitfieldRes.isOk()

let bitfield = bitfieldRes.tryGet()
check:
bitfield.toCapabilities() == @[Capabilities.Relay, Capabilities.Store]

test "encode and extract capabilities from record (deprecated)":
# TODO: Remove after removing the `Record.init()` proc
## Given
let enrkey = generatesecp256k1key()
let caps = CapabilitiesBitfield.init(Capabilities.Relay, Capabilities.Store)
Expand All @@ -56,8 +80,11 @@ suite "Waku ENR - Capabilities bitfield":

test "cannot extract capabilities from record":
## Given
let enrkey = generatesecp256k1key()
let record = Record.init(1, enrkey, wakuFlags=none(CapabilitiesBitfield))
let
enrSeqNum = 1u64
enrPrivKey = generatesecp256k1key()

let record = EnrBuilder.init(enrPrivKey, enrSeqNum).build().tryGet()

## When
let bitfieldRes = record.getCapabilitiesField()
Expand Down
26 changes: 19 additions & 7 deletions waku/v2/protocol/waku_enr.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import
stew/[endians2, results],
stew/shims/net,
eth/keys,
eth/p2p/discoveryv5/enr,
libp2p/[multiaddress, multicodec],
libp2p/crypto/crypto
import
../../common/enr

export enr, crypto, multiaddress, net

Expand All @@ -23,16 +24,17 @@ const
CapabilitiesEnrField* = "waku2"


## Capabilities
## Node capabilities

type
## 8-bit flag field to indicate Waku capabilities.
## 8-bit flag field to indicate Waku node capabilities.
## Only the 4 LSBs are currently defined according
## to RFC31 (https://rfc.vac.dev/spec/31/).
CapabilitiesBitfield* = distinct uint8

## See: https://rfc.vac.dev/spec/31/#waku2-enr-key
## each enum numbers maps to a bit (where 0 is the LSB)
# TODO: Make this enum {.pure.}
Capabilities* = enum
Relay = 0,
Store = 1,
Expand Down Expand Up @@ -66,9 +68,19 @@ func toCapabilities*(bitfield: CapabilitiesBitfield): seq[Capabilities] =
toSeq(Capabilities.low..Capabilities.high).filterIt(supportsCapability(bitfield, it))


## TODO: Turn into an EnrBuilder extension
func toFieldPair*(caps: CapabilitiesBitfield): FieldPair =
toFieldPair(CapabilitiesEnrField, @[caps.uint8])
# ENR builder extension

proc withWakuCapabilities*(builder: var EnrBuilder, caps: CapabilitiesBitfield) =
builder.addFieldPair(CapabilitiesEnrField, @[caps.uint8])

proc withWakuCapabilities*(builder: var EnrBuilder, caps: varargs[Capabilities]) =
withWakuCapabilities(builder, CapabilitiesBitfield.init(caps))

proc withWakuCapabilities*(builder: var EnrBuilder, caps: openArray[Capabilities]) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just an opinion. I generally prefer one function rather than multiple ones. less code to read, more simple and clearer. not sure I see the value of having all three.

Isn't this too low level? Cool thing about the builder is abstracting away all this
withWakuCapabilities(..., caps: CapabilitiesBitfield))

Both are very similar.
withWakuCapabilities(Relay, Store)
withWakuCapabilities(@[Relay, Store])

tldr: I would just leave withWakuCapabilities(@[Relay, Store])

Copy link
Contributor Author

@LNSD LNSD Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different proc signatures, cover different use cases.

At first sight, you can see it as too low level, for example. But IIRC, this is the exact API we need to create an ENR in the Waku discv5 module. A similar thing happens with the multiple capabilities methods: it is not the same to pass a seq[T] than a varargs[T]. You can convert easily a varargs instance into a sequence, but the opposite has some compilation-time implications. The varargs type can be understood as an array of known sizes at compile time, on the other hand, the sequence type is a dynamic size (pointer, length, and capacity backed) list of elements.

withWakuCapabilities(builder, CapabilitiesBitfield.init(@caps))


# ENR record accessors (e.g., Record, TypedRecord, etc.)

proc getCapabilitiesField*(r: Record): EnrResult[CapabilitiesBitfield] =
let field = ?r.get(CapabilitiesEnrField, seq[uint8])
Expand Down Expand Up @@ -201,7 +213,7 @@ func init*(T: type enr.Record,

# `waku2` field
if wakuFlags.isSome():
wakuEnrFields.add(toFieldPair(wakuFlags.get()))
wakuEnrFields.add(toFieldPair(CapabilitiesEnrField, @[wakuFlags.get().uint8]))

# `multiaddrs` field
if multiaddrs.len > 0:
Expand Down