-
Notifications
You must be signed in to change notification settings - Fork 355
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
Update to CosmWasm v0.14.0 #289
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Some potential improvements
@@ -77,7 +77,7 @@ impl Escrow { | |||
} | |||
|
|||
if let Some(end_time) = self.end_time { | |||
if env.block.time > end_time { | |||
if env.block.time > Timestamp::from_seconds(end_time) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could turn the state into pub end_time: Option<Timestamp>
now if you want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rather keep the user-facing API in seconds.
I guess this is state, but then we expose this in query.
Thanks for the review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
.circleci/config.yml
Outdated
#command: cargo install --debug --features iterator --example check_contract -- cosmwasm-vm | ||
command: cargo install --debug --features iterator --git https://github.com/CosmWasm/cosmwasm --tag=v0.14.0-beta5 --example check_contract -- cosmwasm-vm | ||
# Uses --debug for compilation speed (assumes 0.14.0 version of check_contract) | ||
command: cargo install --debug --features iterator --example check_contract -- cosmwasm-vm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
@@ -77,7 +77,7 @@ impl Escrow { | |||
} | |||
|
|||
if let Some(end_time) = self.end_time { | |||
if env.block.time > end_time { | |||
if env.block.time > Timestamp::from_seconds(end_time) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can migrate end_time
to Option<Timestamp>
in the future, and improve on this.
@@ -413,8 +413,7 @@ mod test { | |||
channel_id: "channel-1234".to_string(), | |||
}, | |||
sequence: 2, | |||
timeout_block: None, | |||
timeout_timestamp: Some(1665321069000000000u64), | |||
timeout: IbcTimeout::with_timestamp(Timestamp::from_seconds(1665321069)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicer.
"Uint128": { | ||
"description": "A thin wrapper around u128 that is using strings for JSON encoding/decoding, such that the full u128 range can be used for clients that convert JSON numbers to floats, like JavaScript and jq.\n\n# Examples\n\nUse `from` to create instances of this and `u128` to get the value out:\n\n``` # use cosmwasm_std::Uint128; let a = Uint128::from(123u128); assert_eq!(a.u128(), 123);\n\nlet b = Uint128::from(42u64); assert_eq!(b.u128(), 42);\n\nlet c = Uint128::from(70u32); assert_eq!(c.u128(), 70); ```", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These descriptions are a tad long / unreadable. Maybe we can improve on this in another iteration.
Duration::Time(duration) => { | ||
let (secs, nanos) = ( | ||
block.time.nanos() / 1_000_000_000, | ||
block.time.nanos() % 1_000_000_000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is test code, and therefore not important, but better to assign nanos()
to a variable and do the splitting, to avoid errors / inconsistencies.
block.time.nanos() % 1_000_000_000, | ||
); | ||
let earlier = Timestamp::from_seconds(secs - duration); | ||
block.time = earlier.plus_nanos(nanos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, not important, but I think these should be subtracted, not added.
No description provided.