Fix the deserialization from JSON for I256 and U256 types with custom endianness#28
Conversation
|
Sorry for taking so long to look at this - for some reason I must have missed the notification. |
|
|
||
| [dev-dependencies] | ||
| serde = { version = "1", features = ["derive"] } | ||
| serde_json = "1" | ||
| bincode = "1" |
There was a problem hiding this comment.
Would it be possible to keep these dependencies out of the test (my motivation is mostly to make building and cargo test-ing ethnum crate as lean as possible). Especially since the serde_json with pre-compiled binary fiasco, I would prefer to keep it out.
You should be able to use Vec<u8> as IntoDeserializer to check the visit_seq implementation.
There was a problem hiding this comment.
Feel free to take a stab at this (otherwise, I don't mind doing it myself on top of these changes).
| bytes[i] = seq | ||
| .next_element()? | ||
| .ok_or(de::Error::invalid_length(i, &self))?; | ||
| } |
There was a problem hiding this comment.
I think you need to check that the seq has no elements after the for loop - otherwise a 33 byte long sequence would deserialize as well.
|
In general changes look good, just a couple of comments. |
|
Merging as is, will do the test refactoring in a follow up. |
|
@nlordell Sorry, I wanted to apply your comments, but was distracted. Thank you for merging this! |
|
No need to apologize, I took a while to answer too. Should be released in 1.4.0 |
2 participants
Closes #27.