search

status-im / nim-eth

Common utilities for Ethereum
https://nimbus.status.im
Apache License 2.0
83 stars 31 forks source link

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

Open kdeme opened 1 month ago

kdeme commented 1 month ago

Attempt to add dual stack support in ENR in line with the current API. In draft however as I don't really like this API, it feels rather clumsy, both in implementation and in usage.

I'll probably implement dualstack in Fluffy to see how it would typically be used, and continue/adjust from there.

Might also go for some EnrBuilder approach instead to get to something in the trend of:

let enr = EnrBuilder.init().tcp4(Port(9000)).ip4(ip4).ip6(ip6).build(privatekey)

and

let enrUpdated = EnrBuilder.init(enr).tcp4(Port(9000)).ip4(ip4).build(privatekey)

Ideas and/or use cases are welcome.

NagyZoltanPeter commented 1 month ago

Attempt to add dual stack support in ENR in line with the current API. In draft however as I don't really like this API, it feels rather clumsy, both in implementation and in usage.

I'll probably implement dualstack in Fluffy to see how it would typically be used, and continue/adjust from there.

Might also go for some EnrBuilder approach instead to get to something in the trend of:

let enr = EnrBuilder.init().tcp4(Port(9000)).ip4(ip4).ip6(ip6).build(privatekey)

and

let enrUpdated = EnrBuilder.init(enr).tcp4(Port(9000)).ip4(ip4).build(privatekey)

Ideas and/or use cases are welcome.

@kdeme Thanks for the effort! From Waku point of view I have gone through it. From our POV I would really vote for the builder pattern than anything else. In Waku we have something similar implemented over enr.Record. But the approach you crafter here, above it might be even better. I think it has greater advantages controlling what can get into the ENR or filtering duplicates, fine graining updates (with Opt.none for clearing a field).

For the draft solution, I have the feeling that making IP4 and IP6 mandatory we will still end up in incompatibilities/regression. Do you see dual stack to be forced?

arnetheduck commented 1 month ago

Do you see dual stack to be forced?

re ipv6, the idea in general is to push towards a world where dual stack is enabled by default (if the platform allows it), ie if the caller or user does "nothing special", they should end up in a dual stack world that serves both ipv4 and ipv6.