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: detaching StoreCodeAndInstantiateContract from wasm's tx.proto #625

Merged
merged 9 commits into from
Aug 17, 2022

Conversation

zemyblue
Copy link
Member

@zemyblue zemyblue commented Aug 3, 2022

Description

related: #623

Detach custom changes proto from original cosmwasm proto

Detach custom proto change in cosmwasm/wasm/v1/

  • StoreCodeAndInstantiateContract from cosmwasm/wasm/v1/tx.proto to lbm/wasm/v1/tx.proto
  • UpdateContractStatusProposal from cosmwasm/wasm/v1/proposal.proto to lbm/wasm/v1/proposal.proto
  • ContractStatus type from cosmwasm/wasm/v1/types.protp to lbm/wasm/v1/types.proto

Change to original type

  • remove gas_multiplier, instance_cost and compile_cost of Params in cosmwasm/wasm/v1/types.proto. And create new Params for lbm's genesis to lbm/wasm/v1/types.proto
  • remove status of ContractInfo in cosmwasm/wasm/v1/types.proto

Add new genesis proto in lbm/wasm/v1/

Remove custom contract blacklist function

This contract blacklist function will be added through another PR.

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

@zemyblue zemyblue marked this pull request as draft August 3, 2022 04:08
@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #625 (4f303d7) into feat/wasm_refactor (9ba6ea3) will increase coverage by 0.25%.
The diff coverage is 88.57%.

Impacted file tree graph

@@                  Coverage Diff                   @@
##           feat/wasm_refactor     #625      +/-   ##
======================================================
+ Coverage               60.19%   60.45%   +0.25%     
======================================================
  Files                     817      825       +8     
  Lines                   94971    95654     +683     
======================================================
+ Hits                    57169    57827     +658     
- Misses                  34329    34370      +41     
+ Partials                 3473     3457      -16     
Impacted Files Coverage Δ
x/wasm/client/cli/tx.go 16.10% <0.00%> (ø)
x/wasm/keeper/authz_policy.go 100.00% <ø> (ø)
x/wasm/keeper/contract_keeper.go 92.85% <ø> (-0.48%) ⬇️
x/wasm/keeper/msg_server.go 18.49% <0.00%> (ø)
x/wasm/keeper/proposal_handler.go 66.96% <ø> (+1.12%) ⬆️
x/wasm/lbm/types/genesis.go 0.00% <0.00%> (ø)
x/wasm/types/codec.go 100.00% <ø> (ø)
x/wasm/types/params.go 61.16% <ø> (-2.98%) ⬇️
x/wasm/types/proposal.go 65.01% <ø> (+0.29%) ⬆️
x/wasm/types/test_fixtures.go 95.40% <ø> (-0.22%) ⬇️
... and 22 more

@zemyblue zemyblue changed the base branch from main to feat/wasm_refactor August 4, 2022 10:53
@zemyblue zemyblue marked this pull request as ready for review August 4, 2022 11:24
@zemyblue zemyblue self-assigned this Aug 4, 2022
x/wasm/keeper/keeper.go Outdated Show resolved Hide resolved
x/wasm/keeper/genesis.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/suite.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/suite.go Outdated Show resolved Hide resolved
x/wasm/lbm/types/params.go Outdated Show resolved Hide resolved
x/wasm/lbm/types/tx_test.go Outdated Show resolved Hide resolved
proto/lbm/wasm/v1/genesis.proto Show resolved Hide resolved
@@ -135,18 +135,6 @@ message UnpinCodesProposal {
repeated uint64 code_ids = 3 [(gogoproto.customname) = "CodeIDs", (gogoproto.moretags) = "yaml:\"code_ids\""];
}

// UpdateStatusProposal gov proposal content type to update the contract status.
message UpdateContractStatusProposal {
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need any more?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this proposal's role is to modify the smart contract as inactive or active. This is the one of contract blacklist functions. I'll add the contract blacklist function through another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that the blacklist functions will replace the msg we previously added on our own?

Copy link
Member Author

@zemyblue zemyblue Aug 16, 2022

Choose a reason for hiding this comment

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

@shiki-tak , Yes, I'll have a plan to add the contract blacklist data in the genesisState. So I'll add it to sixth index in the genesisState like below.

message GenesisState {
   ….

  // GenMsgs has wasmd sdk type messages to execute in the genesis phase
  repeated cosmwasm.wasm.v1.GenesisState.GenMsgs  gen_msgs  = 5 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "gen_msgs,omitempty"];

  // InactiveContractAddresses is a list of contract address that set inactive
  repeated string inactive_contract_addresses = 6 [(gogoproto.jsontag) = "inactive_contract_address, omitempty"];
}

In this case, the contract blacklist are saved in the genesisState, and when every contract is executed, it is checked if it is blacklist contract or not. it is a little difference with current checking system.

zemyblue and others added 2 commits August 10, 2022 18:42
Signed-off-by: zemyblue <[email protected]>
x/wasm/client/testutil/query.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/query.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/query.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/suite.go Outdated Show resolved Hide resolved
x/wasm/client/testutil/suite.go Outdated Show resolved Hide resolved
x/wasm/lbm/types/tx.go Outdated Show resolved Hide resolved
x/wasm/lbm/types/validation.go Outdated Show resolved Hide resolved
@0Tech 0Tech self-requested a review August 12, 2022 05:35
Copy link
Collaborator

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

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

LGTM

@zemyblue zemyblue merged commit 0f5056f into Finschia:feat/wasm_refactor Aug 17, 2022
@zemyblue zemyblue deleted the lbm_wasm_protos branch October 19, 2022 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detaching the custom wasm proto part of lbm-sdk
6 participants