Skip to content

[Merged by Bors] - Add serde_utils module with quoted u64 support#1588

Closed
michaelsproul wants to merge 2 commits intomasterfrom
quoted-ints
Closed

[Merged by Bors] - Add serde_utils module with quoted u64 support#1588
michaelsproul wants to merge 2 commits intomasterfrom
quoted-ints

Conversation

@michaelsproul
Copy link
Member

Proposed Changes

This is an extraction of the quoted int code from #1569, that I've come to rely on for #1544.

It allows us to parse integers from serde strings in YAML, JSON, etc. The main differences from the code in Paul's original PR are:

  • Added a submodule that makes quoting mandatory (require_quotes).
  • Decoding is generic over the type T being decoded. You can use #[serde(with = "serde_utils::quoted_u64::require_quotes")] on Epoch and Slot fields (this is what I do in my slashing protection PR).

I've turned on quoting for Epoch and Slot in this PR, but will leave the other types changes to you Paul.

I opted to put everything in the conseus/serde_utils module so that BLS can use it without a circular dependency. In future when we want to publish types I think we could publish serde_utils as lighthouse_serde_utils or something. Open to other ideas on this front too.

@michaelsproul michaelsproul added ready-for-review The code is ready for review consensus An issue/PR that touches consensus code, such as state_processing or block verification. labels Sep 5, 2020
michaelsproul added a commit that referenced this pull request Sep 6, 2020
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Perfect!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 7, 2020
@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Sep 7, 2020
## Proposed Changes

This is an extraction of the quoted int code from #1569, that I've come to rely on for #1544.

It allows us to parse integers from serde strings in YAML, JSON, etc. The main differences from the code in Paul's original PR are:

* Added a submodule that makes quoting mandatory (`require_quotes`).
* Decoding is generic over the type `T` being decoded. You can use `#[serde(with = "serde_utils::quoted_u64::require_quotes")]` on `Epoch` and `Slot` fields (this is what I do in my slashing protection PR).

I've turned on quoting for `Epoch` and `Slot` in this PR, but will leave the other `types` changes to you Paul.

I opted to put everything in the `conseus/serde_utils` module so that BLS can use it without a circular dependency. In future when we want to publish `types` I think we could publish `serde_utils` as `lighthouse_serde_utils` or something. Open to other ideas on this front too.
@bors
Copy link

bors bot commented Sep 7, 2020

@bors bors bot changed the title Add serde_utils module with quoted u64 support [Merged by Bors] - Add serde_utils module with quoted u64 support Sep 7, 2020
@bors bors bot closed this Sep 7, 2020
@michaelsproul michaelsproul deleted the quoted-ints branch September 7, 2020 01:55
michaelsproul added a commit that referenced this pull request Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consensus An issue/PR that touches consensus code, such as state_processing or block verification. ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants