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.

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

Add ENR Record.init and update call for dual stack #748

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
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
116 changes: 115 additions & 1 deletion eth/p2p/discoveryv5/enr.nim
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,9 @@ func init*(
extraFields: openArray[FieldPair] = []):
EnrResult[T] =
## Initialize a `Record` with given sequence number, private key, optional
## ip address, tcp port, udp port, and optional custom k:v pairs.
## IP address, TCP port, UDP port, and optional custom k:v pairs.
##
## The IP address can be an IPv4 or IPv6 address.
##
## Can fail in case the record exceeds the `maxEnrSize`.
doAssert(not hasPredefinedKey(extraFields), "Predefined key in custom pairs")
Expand All @@ -254,6 +256,51 @@ func init*(
fields.insert extraFields
makeEnrAux(seqNum, "v4", pk, fields)

func init*(
T: type Record,
seqNum: uint64, pk: PrivateKey,
ip4: array[4, byte],
ip6: array[16, byte],
Comment on lines +262 to +263
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no Opt here, as in dual stack case I'm assuming both IPs or no IP is known (perhaps this is not correct?).

Also not using IpAddress as from that type I cannot be sure whether it is IPv4 or IPv6 (unless I check for it of course).

In the other use cases, the init call with only 1 address can be used.

tcp4Port: Opt[Port] = Opt.none(Port),
udp4Port: Opt[Port] = Opt.none(Port),
tcp6Port: Opt[Port] = Opt.none(Port),
udp6Port: Opt[Port] = Opt.none(Port),
extraFields: openArray[FieldPair] = []
): EnrResult[T] =
## Initialize a `Record` with given sequence number, private key, IPv4 and IPv6
## address, optional TCP port and UDP port for both IPv4 and IPV6, and
## optional custom k:v pairs.
##
## This function is to be used when running in IPv4 and IPv6 dual stack mode.
## The tcp6 and udp6 fields will only be set if they differ from the tcp and udp
## fields.
##
## Can fail in case the record exceeds the `maxEnrSize`.
doAssert(not hasPredefinedKey(extraFields), "Predefined key in custom pairs")

var fields = newSeq[FieldPair]()

fields.insert(("ip", ip4.toField))
fields.insert(("ip6", ip6.toField))

if tcp4Port.isSome():
fields.insert(("tcp", tcp4Port.value().uint16.toField))
if udp4Port.isSome():
fields.insert(("udp", udp4Port.value().uint16.toField))

# From the ENR specification:
# "Declaring the same port number in both tcp, tcp6 or udp, udp6 should be avoided
# but doesn't render the record invalid."
Comment on lines +291 to +293
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps I don't need to do this check and can let the user decide what they want to do in case of same ports.

if tcp6Port.isSome():
if tcp6Port != tcp4Port:
fields.insert(("tcp6", tcp6Port.value().uint16.toField))
if udp6Port.isSome():
if udp6Port != udp4Port:
fields.insert(("udp6", udp6Port.value().uint16.toField))

fields.add extraFields
makeEnrAux(seqNum, "v4", pk, extraFields)

func getField(r: Record, name: string, field: var Field): bool =
# It might be more correct to do binary search,
# as the fields are sorted, but it's unlikely to
Expand Down Expand Up @@ -357,6 +404,73 @@ func update*(

ok()

func update*(
record: var Record,
pk: PrivateKey,
ip4: array[4, byte],
ip6: array[16, byte],
tcp4Port: Opt[Port] = Opt.none(Port),
udp4Port: Opt[Port] = Opt.none(Port),
tcp6Port: Opt[Port] = Opt.none(Port),
udp6Port: Opt[Port] = Opt.none(Port),
extraFields: openArray[FieldPair] = []):
EnrResult[void] =
## Update a `Record` with given IPv4 and IPv6 address, optional TCP port and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fact that I have to write such a lengthy documentation for this call probably is not a sign of good API.

## UDP port for both IPv4 and IPV6, and optional custom k:v pairs.
##
## This function is to be used when running in IPv4 and IPv6 dual stack mode.
## The tcp6 and udp6 fields will only be set if they differ from the tcp and udp
## fields.
##
## If none of the k:v pairs are changed, the sequence number of the `Record`
## will still be incremented and a new signature will be applied.
##
## Providing an `Opt.none` for `tcp4Port`/`udp4Port`/`tcp6Port`/`udp6Port`/
## will leave the corresponding field untouched.
Comment on lines +428 to +429
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps it makes more sense to unset the field in that case

##
## Can fail in case of wrong `PrivateKey`, if the size of the resulting record
## exceeds `maxEnrSize` or if maximum sequence number is reached. The `Record`
## will not be altered in these cases.
# TODO: deprecate this call and have individual functions for updating?
doAssert(not hasPredefinedKey(extraFields), "Predefined key in custom pairs")

var r = record

let pubkey = r.get(PublicKey)
if pubkey.isNone() or pubkey.get() != pk.toPublicKey():
return err("Public key does not correspond with given private key")

var fields = newSeq[FieldPair]()

fields.insert(("ip", ip4.toField))
fields.insert(("ip6", ip6.toField))

if tcp4Port.isSome():
fields.insert(("tcp", tcp4Port.value().uint16.toField))
if udp4Port.isSome():
fields.insert(("udp", udp4Port.value().uint16.toField))

# From the ENR specification:
# "Declaring the same port number in both tcp, tcp6 or udp, udp6 should be avoided
# but doesn't render the record invalid."
if tcp6Port.isSome():
if tcp6Port != tcp4Port:
fields.insert(("tcp6", tcp6Port.value().uint16.toField))
if udp6Port.isSome():
if udp6Port != udp4Port:
fields.insert(("udp6", udp6Port.value().uint16.toField))

r.pairs.insert extraFields

if r.seqNum == high(type r.seqNum): # highly unlikely
return err("Maximum sequence number reached")
r.seqNum.inc()

r.raw = ? makeEnrRaw(r.seqNum, pk, r.pairs)
record = r

ok()

func tryGet*(r: Record, key: string, T: type): Opt[T] =
## Get the value from the provided key.
## Return `none` if the key does not exist or if the value is invalid
Expand Down
Loading