-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fix JailMsg inconsistencies #20
Comments
It's tempting to ditch the 'forever' and return to the '0 means forever' format. |
I agree. I believe multiple devs were arguing against the "impurity" of it. I think we need to remember with these exposed types, that we are designing a JSON API, not a Rust / Go / etc struct, so we need to work on cleanest interchange format. I am open to other approaches here, but the current maps very poorly to JSON (and Go) |
You've made some comments on review that were listened to. No one else, at least not in my reviews nor messages that I've read.
Since currently we keep pub const JAIL: Map<&Addr, Expiration> = Map::new("jail"); and if we stick to 0, then we'd have to compute actuall expiration every read. let expiration = duration.after(&env.block); so currently Summary:
|
@ueco-jb good point. |
The solution to that would be to use |
This arose from https://github.com/confio/tgrade/pull/231/files#diff-8db14e10b9ef925e19d3a5efe4e78d5557798674beff468817945ab6a31ec08aR93-R99
When querying JailingPeriod, we now get:
However, when we make a JailMsg we produce:
The first one is annoying to parse in other languages (easiest if everything is a struct or all strings, don't mix). The second one is a bit of stutter and inconsistent with the Jailing Period.
I propose using the same JSON format for both of them, so JailingDuration would look like:
The text was updated successfully, but these errors were encountered: