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

Upgrade CosmWasm to beta5 #215

Merged
merged 5 commits into from
Apr 27, 2021
Merged

Upgrade CosmWasm to beta5 #215

merged 5 commits into from
Apr 27, 2021

Conversation

webmaster128
Copy link
Member

@webmaster128 webmaster128 commented Apr 27, 2021

  • Upgrade CosmWasm to 0.14.0-beta5
  • Add missing Admin to WasmMsg::Instantiate
  • Add WasmMsg::UpdateAdmin and WasmMsg::ClearAdmin
  • Adapt to IBC timeout change
  • Expose GetMetrics from VM

@webmaster128 webmaster128 requested a review from ethanfrey April 27, 2021 12:58
@@ -41,18 +41,30 @@ func (t IBCTimeoutBlock) IsZero() bool {
return t.Revision == 0 && t.Height == 0
}

type IBCTimeoutBoth struct {
Block IBCTimeoutBlock `json:"block"`
Timestamp uint64 `json:"timestamp_nanos"` // TODO: simplify to just "timestamp" in Rust
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simpler field name coming with CosmWasm/cosmwasm#902. Timestamps are always nanoseconds in IBC and we have it documented right in the enum case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timestamps are always nanoseconds in IBC and we have it documented right in the enum case.

Let's discuss this in CosmWasm/cosmwasm#902
I wrote much of this code and twice returned seconds there, as there were no helpers, I just took env.block.time + 2000 or such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy for simpler name in Go

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, we can stick to it if helps. No rush here. I just felt it was overly long when writing the glue code.

Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff. Let's discuss field renames in cosmwasm.
But this is good to merge

api/mocks.go Show resolved Hide resolved
@@ -101,6 +101,11 @@ func (vm *VM) AnalyzeCode(checksum Checksum) (*types.AnalysisReport, error) {
return api.AnalyzeCode(vm.cache, checksum)
}

// GetMetrics some internal metrics for monitoring purposes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this for @alpe

@@ -41,18 +41,30 @@ func (t IBCTimeoutBlock) IsZero() bool {
return t.Revision == 0 && t.Height == 0
}

type IBCTimeoutBoth struct {
Block IBCTimeoutBlock `json:"block"`
Timestamp uint64 `json:"timestamp_nanos"` // TODO: simplify to just "timestamp" in Rust
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timestamps are always nanoseconds in IBC and we have it documented right in the enum case.

Let's discuss this in CosmWasm/cosmwasm#902
I wrote much of this code and twice returned seconds there, as there were no helpers, I just took env.block.time + 2000 or such.

@@ -41,18 +41,30 @@ func (t IBCTimeoutBlock) IsZero() bool {
return t.Revision == 0 && t.Height == 0
}

type IBCTimeoutBoth struct {
Block IBCTimeoutBlock `json:"block"`
Timestamp uint64 `json:"timestamp_nanos"` // TODO: simplify to just "timestamp" in Rust
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy for simpler name in Go

types/ibc.go Show resolved Hide resolved
types/msg.go Show resolved Hide resolved
types/msg.go Show resolved Hide resolved
@webmaster128 webmaster128 merged commit d4d83ab into main Apr 27, 2021
@webmaster128 webmaster128 deleted the upgrade-to-beta5 branch April 27, 2021 14:02
@webmaster128 webmaster128 mentioned this pull request Apr 27, 2021
1 task
// See https://golang.org/pkg/time/#Time.UnixNano
// at least one of timeout_block, timeout_timestamp is required
TimeoutTimestamp *uint64 `json:"timeout_timestamp,omitempty"`
Timeout IBCTimeout `json:"timeout"`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line introduces a bug because in cosmwasm-std the IbcPacket still uses the old format with fields timeout_block and timeout_timestamp.

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