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: broken decoding #75

Closed
wants to merge 4 commits into from
Closed

fix: broken decoding #75

wants to merge 4 commits into from

Conversation

emhane
Copy link
Contributor

@emhane emhane commented Apr 15, 2024

Thanks to very important find by @mattsse. All enrs with for example "eth2" kv-pair have failed decoding.

  • Fixes decoding of enr with an custom kv pair

@AgeManning
Copy link
Member

A few things to note here.

The bug identified here relates solely to applications that are attempting to RLP-encode lists as ENR values and decode them. ENRs currently cannot decode values that are stored as RLP lists or recursive RLP lists.

The specific key you are referring to, "eth2" is used in Ethereum's consensus protocol is unaffected by this, because that protocol doesn't store ENR values as RLP lists. The protocol always uses straight bytes encoded as ssz.

This PR as it stands actually breaks the tests and prevents decoding of other fields that are not RLP lists. In its current form we cannot merge this PR.

I've tried pushing fixes to this branch, but appear not to have permissions. As this appears urgent for your use case, i'm going to open other PR with the changes.

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