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

fix: remove enr padding #16

Merged
merged 50 commits into from
Nov 1, 2023
Merged

fix: remove enr padding #16

merged 50 commits into from
Nov 1, 2023

Conversation

4rgon4ut
Copy link
Contributor

@4rgon4ut 4rgon4ut commented Sep 3, 2023

This PR address prysm bootnode ENR fix. This ENR string contains padding containes padding (==), which doesn’t correspond encoding scheme that declared in EIP-778.

Jobs:

  • Check for padding
  • Check that ENR string is valid and can be decoded with enr-cli, also checks that all non-empty and non-comment strings are ENR strings

Job triggers on PR to the main branch.

@4rgon4ut 4rgon4ut marked this pull request as ready for review September 3, 2023 23:31
@filoozom
Copy link
Contributor

filoozom commented Sep 4, 2023

According to sigp/lighthouse#4530 (comment), the following ENRs are also invalid:

  • chiado-lighthouse-0
  • chiado-lighthouse-1
  • chiado-lodestar-0

Do we know why?

@4rgon4ut
Copy link
Contributor Author

4rgon4ut commented Sep 4, 2023

According to sigp/lighthouse#4530 (comment), the following ENRs are also invalid:

  • chiado-lighthouse-0
  • chiado-lighthouse-1
  • chiado-lodestar-0

Do we know why?

They seems to work on my side, I was able to get mentioned error only for chiado-prysm-0 (used same rust crate to check).
Node also starting to correctly with such ENRs.

@filoozom filoozom merged commit f3a9d77 into main Nov 1, 2023
1 check passed
@filoozom filoozom deleted the fix/prysm-bootnode-enr branch November 1, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants