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

Add serde for BoxedUInt #587

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

pinkforest
Copy link

@pinkforest pinkforest commented Apr 2, 2024

Missing impl Serialize and Deserialize

@pinkforest pinkforest changed the title Add serde for BoxedUInt WIP: Add serde for BoxedUInt Apr 2, 2024
src/uint/boxed.rs Outdated Show resolved Hide resolved
src/uint/boxed.rs Outdated Show resolved Hide resolved
src/uint/boxed.rs Outdated Show resolved Hide resolved
Comment on lines 401 to 402
#[cfg(feature = "serde")]
impl<'de> Deserialize<'de> for BoxedUint
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Thanks done

@pinkforest pinkforest changed the title WIP: Add serde for BoxedUInt Add serde for BoxedUInt Apr 6, 2024
D: Deserializer<'de>,
{
let mut buffer = Self::zero().to_le_bytes();
serdect::array::deserialize_hex_or_bin(buffer.as_mut(), deserializer)?;
Copy link
Author

@pinkforest pinkforest Apr 6, 2024

Choose a reason for hiding this comment

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

Doesn't work because it doesn't know the size and zero() doesn't size it for the incoming buf
Having some weird errors from the alloc variant

Copy link
Member

Choose a reason for hiding this comment

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

You need to use serdect::slice if the size isn't known a priori

Copy link
Author

Choose a reason for hiding this comment

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

hmm I thought I tried (the alloc variant) that and it gave some weird runtime behaviour

@dignifiedquire
Copy link
Member

@tarcieri @pinkforest what's the state here?

@tarcieri
Copy link
Member

The feedback I left hasn't been addressed and the tests aren't yet all passing

@pinkforest
Copy link
Author

pinkforest commented Jul 22, 2024

Please anyone feel free to cherry on my commit and fix - I got stuck with the alloc version playing up weird.
I am atm too busy with personal disasters sorry 😿 . I may be able to look this in a week or two apologies 😞

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.

3 participants