Update usize serialization and deserialization
#1266
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fumuran
commented
Mar 1, 2024
bobbinth
requested changes
Mar 3, 2024
There was a problem hiding this comment.
Thank you! Looks good. I left a few comments inline.
Also, I think we can roll back the AST-related changes as these will not be needed after the assembler refactoring that @bitwalker is working on.
Fumuran
force-pushed
the
andrew-update-usize-serialization
branch
from
7d27b42
to
da18d4d
Compare
bobbinth
reviewed
Mar 4, 2024
bobbinth
reviewed
Mar 4, 2024
Fumuran
force-pushed
the
andrew-update-usize-serialization
branch
from
852d1ee
to
82e71b8
Compare
bobbinth
requested changes
Mar 5, 2024
Fumuran
force-pushed
the
andrew-update-usize-serialization
branch
2 times, most recently
from
68a993f
to
024c631
Compare
bobbinth
approved these changes
Mar 8, 2024
core/src/stack/inputs.rs
Outdated
| // CONSTANTS | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| pub const MAX_STACK_INPUTS_SIZE: usize = u16::MAX as usize; |
There was a problem hiding this comment.
The naming is a bit redundant here - e.g., StackInputs::MAX_STACK_INPUTS_SIZE. I would probably change this to something like MAX_LEN and then it would be StackInputs::MAX_LEN.
core/src/stack/outputs.rs
Outdated
| // CONSTANTS | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| pub const MAX_STACK_OUTPUTS_SIZE: usize = u16::MAX as usize; |
There was a problem hiding this comment.
Same comment as above - probably MAX_LEN would be sufficient here.
core/src/stack/inputs.rs
Outdated
| /// # Errors | ||
| /// Returns an error if: | ||
| /// - The values do not represent a valid field element. | ||
| /// - Number of values in the iterator exceeds the allowed maximum number of input values. | ||
| pub fn try_from_values<I>(iter: I) -> Result<Self, InputError> |
There was a problem hiding this comment.
I just noticed that StackOutputs has try_from_ints() method. So, we should either name this one try_from_ints() or rename the other one into try_from_values(). I think the first option is probably a bit better.
Fumuran
force-pushed
the
andrew-update-usize-serialization
branch
from
024c631
to
bc11f61
Compare
Fumuran
force-pushed
the
andrew-update-usize-serialization
branch
from
bc11f61
to
b0df9b8
Compare
Al-Kindi-0
pushed a commit
that referenced
this pull request
Mar 28, 2024
Al-Kindi-0
pushed a commit
that referenced
this pull request
Mar 28, 2024
Al-Kindi-0
added a commit
that referenced
this pull request
Mar 28, 2024
refactor: serialize usize with write_usize (#1266)
bobbinth
pushed a commit
that referenced
this pull request
Apr 3, 2024
Al-Kindi-0
added a commit
that referenced
this pull request
May 22, 2024
refactor: serialize usize with write_usize (#1266)
Al-Kindi-0
added a commit
that referenced
this pull request
May 24, 2024
refactor: serialize usize with write_usize (#1266)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the serialization and deserialization method of
usizevalues towrite_usize()andread_usize()respectively.