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

Rlp decoding fails for byte strings that also happen to be hex. #292

Open
RogerPodacter opened this issue Oct 30, 2024 · 8 comments
Open
Labels
bug Something isn't working

Comments

@RogerPodacter
Copy link
Contributor

For example, Eth::Rlp.decode(Eth::Rlp.encode("a")) throws RLP string ends with -32 superfluous bytes (Eth::Rlp::DecodingError)

The issue is the hex conversion here: https://github.com/q9f/eth.rb/blob/main/lib/eth/rlp/decoder.rb#L34

@samuelarogbonlo
Copy link

How do i replicate this error ? @RogerPodacter

@RogerPodacter
Copy link
Contributor Author

If you run Eth::Rlp.decode(Eth::Rlp.encode("a")) you expect to get back ”a” but you get an exception instead.

@samuelarogbonlo
Copy link

Okay! I have a fix. I will send in a PR now, maybe you can review as well and test.

@samuelarogbonlo
Copy link

samuelarogbonlo commented Nov 28, 2024

#293

This is a temporary draft PR that you can test. @RogerPodacter

Also bringing in @q9f for visibility.

@RogerPodacter
Copy link
Contributor Author

That PR looks good! However there will still be a bug if any RLP-encoded byte string happens to consist of all hex chars due to the Util.hex_to_bin rlp if Util.hex? rlp logic. However I’m not sure whether this is possible outside of the single character case and I do not have a way to replicate it.

This is a problem elsewhere as well and though it stems from a noble desire to make things “just work” for the user and most of the time it’s fine, in production contexts consistent behavior is important.

Anyway, that’s a larger topic. For the moment your PR looks good and is a definite improvement!

@samuelarogbonlo
Copy link

Duly noted! I can check other pointers, however we can get this merged.

Thanks

@q9f q9f added the bug Something isn't working label Dec 17, 2024
@q9f
Copy link
Owner

q9f commented Dec 17, 2024

I think the problem is not RLP coder but the utility function that handles binary versus hexstrings.

I just discovered this:

Util.bin_to_hex Util.hex_to_bin "a"
=> "a0"

Which is a very obvious bug. I'll try to fix this first and then revisit the RLP issue above.

Ok this is just "0a" hex but big-endian which is a bit confusing. It should be little endian.

Ok this is just me confusing the nibble sin Ruby's pack library. I got a fix incoming for this.

Sorry for the slow response, I'm not spending so much time on this library anymore.

@q9f
Copy link
Owner

q9f commented Dec 17, 2024

@RogerPodacter can you check if #298 works for you?

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

3 participants