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

Add StoreAndInstantiateContract gov proposal #1074

Merged
merged 21 commits into from
Nov 11, 2022

Conversation

pinosu
Copy link
Contributor

@pinosu pinosu commented Nov 4, 2022

Closes #785

@pinosu pinosu requested a review from alpe as a code owner November 4, 2022 10:01
@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Merging #1074 (dfdccd3) into main (a9ce273) will increase coverage by 1.06%.
The diff coverage is 59.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1074      +/-   ##
==========================================
+ Coverage   59.27%   60.33%   +1.06%     
==========================================
  Files          53       54       +1     
  Lines        6758     7211     +453     
==========================================
+ Hits         4006     4351     +345     
- Misses       2456     2549      +93     
- Partials      296      311      +15     
Impacted Files Coverage Δ
app/app.go 88.14% <ø> (ø)
app/test_helpers.go 0.80% <ø> (+0.01%) ⬆️
x/wasm/client/cli/gov_tx.go 6.96% <0.00%> (+0.43%) ⬆️
x/wasm/client/cli/tx.go 28.05% <0.00%> (-4.45%) ⬇️
x/wasm/keeper/legacy_querier.go 69.89% <ø> (ø)
x/wasm/keeper/querier.go 72.07% <ø> (-0.25%) ⬇️
x/wasm/module.go 37.90% <ø> (ø)
x/wasm/types/errors.go 0.00% <ø> (ø)
x/wasm/types/proposal.go 55.52% <28.94%> (-5.79%) ⬇️
x/wasm/types/genesis.go 87.87% <42.85%> (-5.57%) ⬇️
... and 12 more

@ethanfrey ethanfrey added this to the v0.30.0 milestone Nov 4, 2022
Copy link
Member

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Nice start! It looks very complete. I added some minor notes.

From a framework design perspective, we should not expose the CreateAndInstantiate method anywhere. This opens another path into the system. Let's stick with create and instantiate are 2 separate steps in keepers.

@@ -51,6 +51,16 @@ type ContractOpsKeeper interface {
fixMsg bool,
) (sdk.AccAddress, []byte, error)

CreateAndInstantiate(
Copy link
Member

Choose a reason for hiding this comment

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

No need to export this.

msg RawContractMessage,
funds sdk.Coins,
) *StoreAndInstantiateContractProposal {
return &StoreAndInstantiateContractProposal{title, description, runAs, wasmBz, permission, unpinCode, admin, label, msg, funds}
Copy link
Member

@alpe alpe Nov 8, 2022

Choose a reason for hiding this comment

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

Please use keys to init objects. Golang can generate the constructor for you.

@@ -128,3 +140,15 @@ func (p PermissionedKeeper) SetContractInfoExtension(ctx sdk.Context, contract s
func (p PermissionedKeeper) SetAccessConfig(ctx sdk.Context, codeID uint64, caller sdk.AccAddress, newConfig types.AccessConfig) error {
return p.nested.setAccessConfig(ctx, codeID, caller, newConfig, p.authZPolicy)
}

func (p PermissionedKeeper) CreateAndInstantiate(
Copy link
Member

Choose a reason for hiding this comment

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

Please remove. No need to expose this

@@ -23,6 +23,18 @@ type decoratedKeeper interface {
authZ AuthorizationPolicy,
) (sdk.AccAddress, []byte, error)

createAndInstantiate(
Copy link
Member

Choose a reason for hiding this comment

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

No need to optimize for this worflow. Let's stick with the existing create and instantiate methods when used as a library


// StoreAndInstantiateContractProposal gov proposal content type to store
// and instantiate the contract.
message StoreAndInstantiateContractProposal {
Copy link
Member

Choose a reason for hiding this comment

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

👍 This looks complete

x/wasm/types/proposal.go Show resolved Hide resolved
}

// ValidateBasic validates the proposal
func (p StoreAndInstantiateContractProposal) ValidateBasic() error {
Copy link
Member

Choose a reason for hiding this comment

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

Please add a test for regression

Label: %s
Msg: %q
Funds: %s
`, p.Title, p.Description, p.RunAs, p.WASMByteCode, p.Admin, p.Label, p.Msg, p.Funds)
Copy link
Member

Choose a reason for hiding this comment

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

InstantiatePermission and unpinCode are missing.

Admin: p.Admin,
Label: p.Label,
Msg: string(p.Msg),
Funds: p.Funds,
Copy link
Member

Choose a reason for hiding this comment

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

unpinCode is missing

@@ -226,6 +226,41 @@ func InstantiateContractProposalFixture(mutators ...func(p *InstantiateContractP
return p
}

func StoreAndInstantiateContractProposalFixture(mutators ...func(p *StoreAndInstantiateContractProposal)) *StoreAndInstantiateContractProposal {
Copy link
Member

Choose a reason for hiding this comment

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

👍 good fixture

dependabot bot and others added 2 commits November 9, 2022 08:03
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@pinosu pinosu force-pushed the 785-add_store_instantiate_gov_proposal branch from 7fb18b2 to f4ac252 Compare November 9, 2022 14:03
@pinosu pinosu changed the title [WIP] Add StoreAndInstantiateContract gov proposal Add StoreAndInstantiateContract gov proposal Nov 10, 2022
pinosu and others added 9 commits November 11, 2022 09:09
* Add Developer's guide and best practices

* Fix comments
* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…om/prometheus/client_golang-1.14.0

Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0
Return contract history updates on query
* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Copy link
Member

@alpe alpe left a comment

Choose a reason for hiding this comment

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

👍 Almost done :-)

After rebasing and fixing the 2 issues in ProposalStoreAndInstantiateContractCmd I was able to submit a proposal via CLI:

wasmd tx gov submit-proposal store-instantiate "x/wasm/keeper/testdata/reflect.wasm" \
  '{}' --label "alex" \
  --title "testing" --description "Testing" --deposit "100ustake" \
  --run-as $(wasmd keys show -a validator) \
  --admin $(wasmd keys show -a validator) \
  --amount 123ustake \
  --from validator --gas auto --gas-adjustment=1.5 -y  --chain-id=testing --node=http://localhost:26657 -b block -o json

and query it via wasmd q gov proposals --node=http://localhost:26657 .

@@ -525,3 +525,12 @@ func toStdTxResponse(cliCtx client.Context, w http.ResponseWriter, data wasmProp
}
tx.WriteGeneratedTxResponse(cliCtx, w, baseReq, msg)
}

func EmptyRestHandler(cliCtx client.Context) govrest.ProposalRESTHandler {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -23,7 +23,7 @@ func ProposalStoreCodeCmd() *cobra.Command {
Short: "Submit a wasm binary proposal",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
clientCtx, proposalTitle, proposalDescr, deposit, err := getProposalInfo(cmd)
Copy link
Member

Choose a reason for hiding this comment

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

good refactoring. There was lot's of DRY

Args: cobra.ExactArgs(3),
Use: "store-instantiate [wasm file] [json_encoded_init_args] --label [text] --title [text] --description [text] --run-as [address] --admin [address,optional] --amount [coins,optional]",
Short: "Submit and instantiate a wasm contract proposal",
Args: cobra.ExactArgs(1),
Copy link
Member

Choose a reason for hiding this comment

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

Need to be 2 or fails

Suggested change
Args: cobra.ExactArgs(1),
Args: cobra.ExactArgs(2),

cmd.Flags().String(flagRunAs, "", "The address that is stored as code creator. It is the creator of the contract and passed to the contract as sender on proposal execution")
cmd.Flags().String(flagInstantiateByEverybody, "", "Everybody can instantiate a contract from the code, optional")
cmd.Flags().String(flagInstantiateNobody, "", "Nobody except the governance process can instantiate a contract from the code, optional")
cmd.Flags().String(flagInstantiateByAddress, "", "Only this address can instantiate a contract instance from the code, optional")
Copy link
Member

Choose a reason for hiding this comment

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

Also required to define now:

cmd.Flags().StringSlice(flagInstantiateByAnyOfAddress, []string{}, "Any of the addresses can instantiate a contract from the code, optional")

@pinosu pinosu merged commit ef9a84d into main Nov 11, 2022
@pinosu pinosu deleted the 785-add_store_instantiate_gov_proposal branch November 11, 2022 15:04
NoahSaso pushed a commit to NoahSaso/wasmd that referenced this pull request Dec 2, 2022
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

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

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
Magicloud pushed a commit to fpco/wasmd that referenced this pull request Jan 13, 2023
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

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

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Bump github.com/prometheus/client_golang from 1.13.0 to 1.14.0

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Remove exposed functions

* Add tests

* Add Developer's guide and best practices (CosmWasm#1075)

* Add Developer's guide and best practices

* Fix comments

* Change genesis preserving contract history (CosmWasm#1076)

* preserve contract created date on genesis import and add query contract created date

* add validate created

* fix sims test app import export

* add preserve contract history

* Make proto-all only

* Remove ResetFromGenesis

* Add validation

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

* Return contract history updates

* Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)

Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.13.0 to 1.14.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.13.0...v1.14.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "Bump github.com/spf13/viper from 1.13.0 to 1.14.0 (CosmWasm#1082)"

This reverts commit cf81eb8.

* Add cli and refactor code

* Contract authz - redesign (CosmWasm#1077)

* Add contract authz proto

* Implement contract autorization

* Register contract authz

* Add contract-authz tests

* Consume gas for contract authz

* Add contract authz cli

* Update cli usage

* Model spike

* Add max funds limit

* Redesign authz model

* Start e2e test

* Full e2e test

* Test filter and limits

* Test accept

* Fix description

* No linter warning

Co-authored-by: Giancarlos Salas <me@giansalex.dev>

* Add StoreAndInstantiateContract gov proposal

* Add integration tests

* Remove exposed functions

* Add tests

* Add cli and refactor code

* Fix comments

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: GNaD13 <89174180+GNaD13@users.noreply.github.com>
Co-authored-by: Alex Peters <alpe@users.noreply.github.com>
Co-authored-by: Giancarlos Salas <me@giansalex.dev>
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.

StoreAndInstantiate gov proposal
4 participants