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

feature: minimal human-readable serialization of uints #243

Merged
merged 8 commits into from
May 30, 2023
Merged

Conversation

prestwich
Copy link
Collaborator

@prestwich prestwich commented May 12, 2023

Alternative to #225
Closes #224

Motivation

  • Satisfy ethereum Quantity spec for uint serialization
    • strip leading 0 bytes
    • strip leading 0 nibbles
    • use 0x0 for the value 0
  • Retain full-width serialization for bits
  • Retain differentiation between human-readable and non-human-readable

Solution

  • Modify serde support to include serialize_human_full serialize_human_minimal and serialize_binary
  • use serialize_binary for non-human-readable serializers
  • use serialize_human_full for bits in a human-readable serializer
  • use serialize_human_minimal for uints in a human-readable serializer

This work is based on #225, but moves the functionality into differentiated methods, rather than a single method, and biases towards iterator usage

This PR is semver minor as the serialization output will change in a backwards-compatible way

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@prestwich prestwich added the enhancement New feature or request label May 12, 2023
@prestwich prestwich requested a review from recmo May 12, 2023 20:12
@prestwich prestwich self-assigned this May 12, 2023
Chore: changelog

fix: result(s)

lint: clippy::uninlined_format_args

fix: undo reformatting changelog
@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@29010ec). Click here to learn what that means.
Patch coverage: 94.36% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #243   +/-   ##
=======================================
  Coverage        ?   84.96%           
=======================================
  Files           ?       50           
  Lines           ?     5986           
  Branches        ?        0           
=======================================
  Hits            ?     5086           
  Misses          ?      900           
  Partials        ?        0           
Impacted Files Coverage Δ
src/support/serde.rs 94.49% <94.36%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@prestwich prestwich changed the title feature: minimal encoding of uints feature: minimal human-readable serialization of uints May 12, 2023
@prestwich prestwich merged commit 36546c8 into main May 30, 2023
@prestwich prestwich deleted the prestwich/ser branch May 30, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uint Serialize impl does not skip leading zeros
2 participants