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

feat: merge wasmd v0.27.0 #570

Merged
merged 144 commits into from
Jul 8, 2022
Merged

feat: merge wasmd v0.27.0 #570

merged 144 commits into from
Jul 8, 2022

Conversation

shiki-tak
Copy link
Contributor

@shiki-tak shiki-tak commented Jun 21, 2022

Description

closes: #555

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

ethanfrey and others added 30 commits June 7, 2022 15:17
This will not affect functionality whatsoever, it just makes for a more consistent code, as discussed in issue #616.
* Implement PinCodes proposal cli

* Implement UnpinCodes proposal cli

* Fix descriptions

* Apply suggestions from code review

Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>

Co-authored-by: Alexander Peters <alpe@users.noreply.github.com>
@shiki-tak shiki-tak requested review from loloicci and tnasu June 21, 2022 08:01
@shiki-tak shiki-tak self-assigned this Jun 21, 2022
@codecov
Copy link

codecov bot commented Jun 24, 2022

Codecov Report

Merging #570 (dba039e) into main (1a15177) will decrease coverage by 0.04%.
The diff coverage is 45.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
- Coverage   57.59%   57.54%   -0.05%     
==========================================
  Files         793      796       +3     
  Lines       86589    87814    +1225     
==========================================
+ Hits        49871    50534     +663     
- Misses      33558    34107     +549     
- Partials     3160     3173      +13     
Impacted Files Coverage Δ
x/params/types/subspace.go 80.32% <ø> (ø)
x/wasm/client/cli/gov_tx.go 0.00% <0.00%> (ø)
x/wasm/client/cli/query.go 0.00% <0.00%> (ø)
x/wasm/ibc.go 0.00% <0.00%> (ø)
x/wasm/ioutils/utils.go 64.70% <ø> (ø)
x/wasm/keeper/api.go 77.77% <0.00%> (+16.66%) ⬆️
x/wasm/keeper/ibc.go 78.26% <ø> (ø)
x/wasm/keeper/metrics.go 10.71% <ø> (ø)
x/wasm/types/params.go 64.38% <0.00%> (-1.44%) ⬇️
x/wasm/types/tx.go 45.00% <ø> (+4.27%) ⬆️
... and 59 more

go.mod Outdated
@@ -62,5 +63,7 @@ require (
replace (
github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1

github.com/line/wasmvm => github.com/shiki-tak/wasmvm v0.16.3-testing.0.20220624032404-3e5e9ef10a08
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the following PRs are merged, the latest version is applied.
Finschia/wasmvm#66

@zemyblue zemyblue mentioned this pull request Jun 28, 2022
4 tasks
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Please add this PR chagnes to the changelogs.md

addr := sdk.AccAddress(args[0])
err := sdk.ValidateAccAddress(args[0])
if err != nil {
inBuf := bufio.NewReader(cmd.InOrStdin())
keyringBackend, _ := cmd.Flags().GetString(flags.FlagKeyringBackend)
keyringBackend, _ := cmd.Flags().GetString(flags.FlagKeyringBackend) //nolint:errcheck
Copy link
Member

Choose a reason for hiding this comment

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

There was an error checking feature on this line, but removed. any reason to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

//nolint:errcheck was removed because it did not exist. Also, error check was not added here because it was not done in the original simd.

x/wasm/keeper/snapshotter.go Show resolved Hide resolved
x/wasm/types/params.go Show resolved Hide resolved
x/wasm/types/types.go Outdated Show resolved Hide resolved
simapp/app.go Show resolved Hide resolved
x/wasm/keeper/snapshotter.go Show resolved Hide resolved
x/wasm/keeper/snapshotter_integration_test.go Show resolved Hide resolved
@zemyblue
Copy link
Member

zemyblue commented Jun 30, 2022

@shiki-tak san, please merge as Merge pull request type when you merge this PR in order to we recognize the code history.

| `run_as` | [string](#string) | | RunAs is the address that is passed to the contract's environment as sender |
| `contract` | [string](#string) | | Contract is the address of the smart contract |
| `msg` | [bytes](#bytes) | | Msg json encoded message to be passed to the contract as execute |
| `funds` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | Funds coins that are transferred to the contract on instantiation |
Copy link
Contributor

Choose a reason for hiding this comment

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

change the protobuf path (maybe it comes from from another file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is correct because it is the based cosmos proto file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I suggested changing cosmos -> lbm. Is it ok still to be "cosmos.base.v1beta1.Coin"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As @zemyblue nim commented below, this place should have been changed to cosmos(#564), right?


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `pagination` | [cosmos.base.query.v1beta1.PageRequest](#cosmos.base.query.v1beta1.PageRequest) | | pagination defines an optional pagination for the request. |
Copy link
Contributor

Choose a reason for hiding this comment

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

change the protobuf path (maybe it comes from from another file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is correct because it is the based cosmos proto file.

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `code_ids` | [uint64](#uint64) | repeated | |
| `pagination` | [cosmos.base.query.v1beta1.PageResponse](#cosmos.base.query.v1beta1.PageResponse) | | pagination defines the pagination in the response. |
Copy link
Contributor

Choose a reason for hiding this comment

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

change the protobuf path (maybe it comes from from another file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is correct because it is the based cosmos proto file.

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

proto/lbm/wasm/v1/query.proto Outdated Show resolved Hide resolved
// This is tracked in https://github.com/CosmWasm/wasmd/issues/566 and https://github.com/CosmWasm/wasmd/issues/631.
// Gas adjustments are consensus breaking but may happen in any release marked as consensus breaking.
// Do not make assumptions on how much gas an operation will consume in places that are hard to adjust,
// such as hardcoding them in contracts.
//
// Please not that all gas prices returned to the wasmer engine should have this multiplied
Copy link
Contributor

Choose a reason for hiding this comment

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

missed the original change in this line.
62e976a55157c77f6bc2320a827d8851628e10e4

@@ -804,40 +804,6 @@ func TestUnmarshalContentFromJson(t *testing.T) {
}
}

func TestProposalJsonSignBytes(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this test deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -216,7 +216,7 @@ func TestMigrateProposal(t *testing.T) {
}

func TestExecuteProposal(t *testing.T) {
ctx, keepers := CreateTestInput(t, false, "staking")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change included in this commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because our CreateTestInput has four arguments, unlike the fork source.

// now, try to build a protobuf query
protoRequest = wasmvmtypes.QueryRequest{
Stargate: &wasmvmtypes.StargateQuery{
Path: "/cosmos.tx.v1beta1.Service/GetTx",
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmos -> lbm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@loloicci, This proto path was changes in #564 .

snapshot.WriteExtensionItem(protoWriter, wasmBytes)

return false
})

return nil
return rerr
Copy link
Contributor

Choose a reason for hiding this comment

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

typo?

@shiki-tak
Copy link
Contributor Author

@zemyblue @tnasu @loloicci PTAL

Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

LGTM

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
x/ibc/core/24-host/validate.go Show resolved Hide resolved
@shiki-tak shiki-tak merged commit df0f33b into Finschia:main Jul 8, 2022
@zemyblue zemyblue mentioned this pull request Oct 27, 2022
5 tasks
@zemyblue zemyblue mentioned this pull request Nov 28, 2022
5 tasks
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.

feat: migrate wasmd 1.0.0 into x/wasm