refactor: use deserialize_str instead

[kariy]

Resolves #526

[xJonathanLEI]

Thanks! Just curious: for bincode though, wouldn't you want to use bytes instead of str for the repr? AFAIK serde doesn't play well with multiple formats.

On the serialization side, the only option is to use newtypes (e.g. struct SerializeAsBytes(FieldElement)).

On the deserialization side, either use the newtype option, or just go with deserialize_any and implement many visit_* (but then you run into the issue this PR tries to solve).

[xJonathanLEI]

Or, of couse, you can always mark all your FieldElement occurrences with #[serde(with = "...")], or even go as far as manual impls. I guess what the "official" Serialize/Deserialize impls here only serve as the default. Maybe starknet-ff itself can offer alternative modules to be used with #[serde(with = "...")], if that's helpful.

[kariy]

Thanks! Just curious: for bincode though, wouldn't you want to use bytes instead of str for the repr?

Ideally yes

Or, of couse, you can always mark all your FieldElement occurrences with #[serde(with = "...")], or even go as far as manual impls.

I could do this but i dont quite like having to tag #[serde(..)] on every types that use it or even a newtype struct.

I guess what the "official" Serialize/Deserialize impls here only serve as the default. Maybe starknet-ff itself can offer alternative modules to be used with #[serde(with = "...")], if that's helpful.

i just found out about Deserializer::is_human_readable() . Maybe we can conditionally call the preferred deserialize entry points like this ?

 if deserializer.is_human_readable() {
            deserializer.deserialize_any(FieldElementVisitor)
        } else {
            deserializer.deserialize_bytes(FieldElementVisitor)
        }
}        

deserializer like serde_json would return true while something like bincode would return false, so it would still maintain the same behaviour as before but should now work with non self-describing formats.

this is also what alloy is using for their primitive types

[xJonathanLEI]

Thanks! I didn't know that.

This is sorta a breaking change though. Non-human-readable self-describing formats used to be serialized as strings but now they use bytes. Files previously saved with such a format will become unreadable.

I guess we can do this with a breaking semver bump.

[kariy]

I'm happy to contribute on that if you're okay with the approach.

[xJonathanLEI]

Yeah that's certainly a good approach. We just need to bump a breaking version with this change.

[xJonathanLEI]

@kariy This from rust_decimal is an interesting approach too. One issue with this approach though: someone enabling this feature in the dependency tree would affect everyone else.