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

Local bigint serialization #231

Merged
merged 4 commits into from
Feb 19, 2020
Merged

Local bigint serialization #231

merged 4 commits into from
Feb 19, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Implemented local serialization for big int types
  • Updated dependency references from using my bigint fork

So since the serde macros don't call the associated variant of serialize, you can use a module in place of the type to serialize (similar to how serde_bytes works). This is important in future to not have to deploy/use a forked bigint dependency, cleaner for prod and makes easier to update dependency in the future. I have all bigint dependencies routing through this crate I created.

Downside to this is having to define a tuple type to annotate the serialization method. Another option is to later add a type to be able to convert to/from which could act as a proxy for serialization similar to how it is done currently when types need to be converted for serialization (I think the usage added in this PR is cleaner though).

Reference issue to close (if applicable)

Closes

Other information and links

math/bigint/src/biguint_ser.rs Outdated Show resolved Hide resolved
@austinabell austinabell requested a review from ec2 February 18, 2020 21:36
@austinabell austinabell merged commit b598ffc into master Feb 19, 2020
@austinabell austinabell deleted the austin/bigintser branch February 19, 2020 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
encoding hashing and serialization Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants