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

Rework ENR code to solve several shortcomings #706

Open
kdeme opened this issue Jun 20, 2024 · 1 comment
Open

Rework ENR code to solve several shortcomings #706

kdeme opened this issue Jun 20, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@kdeme
Copy link
Contributor

kdeme commented Jun 20, 2024

There are several issues in the current ENR code, to name some:

  • Currently it is possible to have FieldPairs with duplicate keys or even full duplicate FieldPairs in the ENR due to the custom extraFields in the API
  • Currently we cannot remove FIeldPairs in some update ENR function (see also ENR update call does not remove fields that are no longer applicable #639)
  • Currently when using the API with Opt IpAddres or Port, providing an Opt.none will not actually remove the field
  • Some API that is more restrictive than the other: Record.init versus initRecord. The latter is not even used anywhere except for testing.
  • the extraFields parameters and initRecord API allows for reserved keys with invalid types to get into the ENR.

Additionally, there is currently no easy API to set the ip6, tcp6 and udp6 reserved fields. It is only possible through the custom extraFields, which doesn't limit these reserved keys to the correct value type.

@kdeme kdeme added the bug Something isn't working label Jun 20, 2024
@kdeme kdeme self-assigned this Jun 20, 2024
@kdeme
Copy link
Contributor Author

kdeme commented Jun 21, 2024

Part 1: #707

More parts to do:

  • Rework also the decoding of an ENR (need to move id key check and possibly others?) and then make toTypedRecord no longer allowed to fail.
  • Add dual stack support in ENR
  • Cleaner kv access
  • Adding an EnrBuilder is something that fits well for ENR's I think
  • Removal of fields + better API for update and/or just set individual parts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant