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

Add serialization and deserialization to Coins #1809

Closed
wants to merge 1 commit into from

Conversation

shanev
Copy link
Contributor

@shanev shanev commented Aug 7, 2023

This allows Coins to be used in storage without depending on Vec<Coin> which could sort in non-deterministic ways.

@shanev shanev marked this pull request as ready for review August 7, 2023 21:39
@webmaster128
Copy link
Member

webmaster128 commented Aug 15, 2023

depending on Vec which could sort in non-deterministic ways

Using to_vec and into_vec actually does guarantee sorting. @chipshort could you ensure this property is tested and documented?

I'm not sure if it is good to have two different ways of serializing coins. The proposed change will probably create a JSON dictionary and expose internal structure. However, this structure is subject to change. My gut feeling is that if we implement Serialize/Deserialize for Coins, we should implement it explicitly via to_vec.

@webmaster128
Copy link
Member

As of now, I consider .to_vec()/.into_vec() the right (tm) way to do this. See also #1821. This requires coins: Vec<Coin> in the state object and you need a Coins::try_from(state.coins)? which might be slightly inconvenient but should do the job. Feel free to re-open if there is anything left here.

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.

2 participants