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: decode rlp header on Enr::build #72

Merged
merged 4 commits into from
Apr 9, 2024

Conversation

mattsse
Copy link
Contributor

@mattsse mattsse commented Apr 3, 2024

closes #71

#71 (comment)

Unless the .build() fails on normal types, it should be the case that all rlp-bytes can be decoded as bytes right?

This changes the sanitize check from Bytes::decode to Header::decode,
this is required for list types that are added via Builder::add_value because Bytes::decode
fails on lists:
https://github.com/alloy-rs/rlp/blob/2ccf23f7b6a363c844443a13f9496391ee6892ac/crates/rlp/src/decode.rs#L83

added new test that fails on main:

test tests::test_add_content_value ... FAILED

failures:

---- tests::test_add_content_value stdout ----
thread 'tests::test_add_content_value' panicked at src/lib.rs:1519:14:
called `Result::unwrap()` on an `Err` value: InvalidRlpData(UnexpectedList)

also update Enr::get to restore the previous behaviour of rlp::data which returns the payload data

https://docs.rs/rlp/latest/src/rlp/rlpin.rs.html#159

we could add a Bytes::decode_unchecked to alloy-rlp that does not care about list

cc @DaniPopes

Copy link
Contributor

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

LGTM

@AgeManning AgeManning merged commit c76c9e5 into sigp:master Apr 9, 2024
3 checks passed
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.

Clarify enr content value format
3 participants