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: endianness option to scalar serde for Banderwagon #342

Merged
merged 5 commits into from
Jan 16, 2024

Conversation

agnxsh
Copy link
Collaborator

@agnxsh agnxsh commented Jan 16, 2024

Adding an endianness param, set to BigEndian by default in Banderwagon scalar serde, using this because in the Verkle Spec sometimes conversion to and from in littleEndian byte arrays are also prevalent. In those cases, this provides a decent layer of abstraction for someone not knowing about the internals of Constantine.

@advaita-saha
Copy link
Collaborator

Have you run the tests once after making these changes ?

@agnxsh
Copy link
Collaborator Author

agnxsh commented Jan 16, 2024

Have you run the tests once after making these changes ?

Yes, tests are fine. Since it is calling marshal and unmarshal internally, and they are tested, I hadn't added for a little endian scenario, but okay I will be adding.

@@ -130,19 +130,19 @@ func deserialize*(dst: var EC_Prj, src: array[32, byte]): CttCodecEccStatus =
## Banderwagon Scalar Serialization
##
## ############################################################
##
func serialize_scalar*(dst: var array[32, byte], scalar: matchingOrderBigInt(Banderwagon)): CttCodecScalarStatus =
## Adding an optional Endianness param default at BigEndian
Copy link
Owner

Choose a reason for hiding this comment

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

procedure comments are below the procedure in Nim

Copy link
Owner

Choose a reason for hiding this comment

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

image
image

very weird

@mratsim mratsim merged commit c4be82d into mratsim:master Jan 16, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants