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(types): RowNamespaceData binary deserialization #493

Conversation

citizen-stig
Copy link
Contributor

@citizen-stig citizen-stig commented Dec 24, 2024

I've encountered following error when have been deserializing NamespaceData. After some investigation I've noticed that problem stems from RowNamespaceData

called `Result::unwrap()` on an `Err` value: InvalidBoolEncoding(2)

Reproducible test is added as part of this PR. You can comment out actual changes and see how it fails.

I would suggest check all types that have serde(into to be checked against binary serialization and have symmetrical from or try_from. Those types are:

  • ExtendedDataSquare
  • Row
  • Sample

@citizen-stig citizen-stig changed the title Fix row `RowNamespaceData binary deserialization Fix row RowNamespaceData binary deserialization Dec 24, 2024
Copy link
Member

@zvolin zvolin left a comment

Choose a reason for hiding this comment

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

thanks. Feel free to open an issue about other types that you identified. We mainly support the binary encoding of those through .encode() and .decode(), serde is there just because it's reused in json rpc

@zvolin zvolin changed the title Fix row RowNamespaceData binary deserialization fix(types): RowNamespaceData binary deserialization Dec 27, 2024
@zvolin zvolin merged commit 7fb26b8 into eigerco:main Dec 27, 2024
6 checks passed
@zvolin zvolin mentioned this pull request Dec 27, 2024
This was referenced Jan 14, 2025
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