Skip to content

Comments

serde: add stringify serializer over generic T:Display#340

Open
trevarj wants to merge 1 commit intoBlockstreamResearch:masterfrom
trevarj:serde-stringify
Open

serde: add stringify serializer over generic T:Display#340
trevarj wants to merge 1 commit intoBlockstreamResearch:masterfrom
trevarj:serde-stringify

Conversation

@trevarj
Copy link

@trevarj trevarj commented Feb 6, 2026

  • Added a crate serde module and re-exported external serde crate from within it
  • Added a simplicity::serde::stringify module that contains a generic serializer
    over T: Display, which can be used with
    #[serde(with = "simplicity::serde::stringify")] over a struct field
  • Added a unit test that uses string values from Value's value_display test

Resolves #330

src/serde/mod.rs Outdated
@@ -0,0 +1,3 @@
// re-export of the serde crate
pub use ::serde::*;
pub mod stringify;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In 467111c:

I really don't like this. It's a weird trick to pretend that there are more things in the serde crate than there actually are.

Copy link
Author

@trevarj trevarj Feb 18, 2026

Choose a reason for hiding this comment

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

That is true. Any reason for re-exporting serde?

Alternative module ideas:

  1. rust_simplicity::serializers::stringify
  2. rust_simplicity::value::serde::stringify (the serializer is generic so this really doesn't make sense)

Other ideas? What's your preference?

@apoelstra
Copy link
Collaborator

CI failures are because the lockfile is not updated.

- Added a crate `serializers` module
- Added a simplicity::serializers::stringify module that contains a generic serializer
  over T: Display, which can be used with
  #[serde(with = "simplicity::serializers::stringify")] over a struct field
- Added a unit test that uses string values from Value's `value_display` test
- Updated Cargo-recent.lock to add serde_json

Resolves BlockstreamResearch#330
@trevarj
Copy link
Author

trevarj commented Feb 20, 2026

Forced push with changing the module to simplicity::serializers::stringify and addressed the other feedback comments

encode_natural(n, &mut w).expect("encoding to vector");
w.flush_all().expect("flushing");
let m = BitIter::from(sink.into_iter())
let m: usize = BitIter::from(sink.into_iter())
Copy link
Author

@trevarj trevarj Feb 20, 2026

Choose a reason for hiding this comment

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

Once I imported serde_json this type couldn't be inferred because of conflicting implementations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should have serde module for encoding values as strings

2 participants