-
Notifications
You must be signed in to change notification settings - Fork 134
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
feat: implement state replace contract #208
Conversation
Looks good! We'd have to update how we call it in cc: @MaximusHaximus |
Very quick and easy to just have the format match the previous one if you'd like, but felt uncomfortable adding the redundancy in the serialization format personally.
Yeah, so it's more optimized if you just deploy one contract to do both to avoid pushing duplicate code. All three combinations are provided with this (cleanup, replace, both) but they are 67k, 69k, and 70k bytes respectively so very negligible. |
|
Awesome! Can this be reviewed and merged? @MaximusHaximus @agileurbanite |
@austinabell after further discussion with the team, we think it would be better to leave the |
Done :) Sorry for being opinionated and trying to avoid tech debt for this. format is now {"keys":...} for clean and {"entries: [[.., ..], ...]} for replace, as readme shows. Also updated workspaces and the tests now that that's released so this is good timing :D |
Awesome! Would love to see this merged :) |
Is there anything blocking this? Who is reviewing/approving this change? |
closes #203
cc @esaminu does this fit the criteria for what wallet needs?
What has changed:
'{"keys":["...", "..."]}'
to'["...", "..."]'
to avoid redundant strings.no longer allocates or copies values in any wayEdit: it does now because the stream deserialization was not correct, I'll look into it laterVec
buffer of all strings, now none of that -- optimized for gas usageDid not change the serialization from serde because it was indicated JSON is preferred. Can switch to using https://github.com/dtolnay/miniserde for very small code size, but it will come at the cost of less efficient deserializations. If gas optimizations are important, can open an issue to swap serialization by a feature and compare the differences.