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 wasm gov proposals to cli #183

Merged
merged 7 commits into from
Jul 14, 2020
Merged

Add wasm gov proposals to cli #183

merged 7 commits into from
Jul 14, 2020

Conversation

alpe
Copy link
Member

@alpe alpe commented Jul 10, 2020

Resolves #178
Resolves #184

New commands

Usage:
  wasmcli tx gov submit-proposal [flags]
  wasmcli tx gov submit-proposal [command]

Available Commands:
  wasm-store           Submit a wasm binary proposal
  instantiate-contract Submit an instantiate wasm contract proposal
  migrate-contract     Submit a migrate wasm contract to a new code version proposal
  set-contract-admin   Submit a new admin for a contract proposal
  clear-contract-admin Submit a clear admin for a contract to prevent further migrations proposal

Disclaimer

I have manually tested all commands and submitted gov proposals via CLI.

Still open to test

  • The JSON parsers that convert json to proposals
  • Proposal execution

Store

  wasmcli tx gov submit-proposal wasm-store [wasm file] --source [source] --builder [builder] --title [text] --description [text] --creator [address] [flags]

Example:

wasmcli tx gov submit-proposal wasm-store contract.wasm --title=foo --description=bar --creator=cosmos100dejzacpanrldpjjwksjm62shqhyss44jf5xz --from validator --gas 10000000  -y --chain-id=testing --node=http://localhost:26657 -b block

⚠️ The wasm byte code creates UI issues when listing proposals later. See #184 ⚠️

Instantiate

wasmcli tx gov submit-proposal instantiate-contract [code_id_int64] [json_encoded_init_args] --label [text] --admin [address] --title [text] --description [text] --creator [address] [flags]

Example

wasmcli tx gov submit-proposal instantiate-contract 1 '{"verifier":"cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8","beneficiary":"cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8"}' --label my-test --admin cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8 --title instantiate --description test --creator cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8 --from validator --gas 10000000   --chain-id=testing --node=http://localhost:26657 -b block

Migrate

wasmcli tx gov submit-proposal migrate-contract [contract_addr_bech32] [new_code_id_int64] [json_encoded_migration_args] [flags]

Example

wasmcli tx gov submit-proposal migrate-contract cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5 2 '{"verifier": "cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8"}' --title instantiate --description test --sender cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8 --from validator --gas 10000000   --chain-id=testing --node=http://localhost:26657 -b block

Update Admin

wasmcli tx gov submit-proposal set-contract-admin [contract_addr_bech32] [new_admin_addr_bech32] [flags]

Example

wasmcli tx gov submit-proposal set-contract-admin cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5 cosmos1rsj2m3qas3jy50s8psns95qka09lhwhx2wvdq8 --title set-admin --description test  --from validator --gas 10000000   --chain-id=testing --node=http://localhost:26657 -b block

Clear Admin

wasmcli tx gov submit-proposal clear-contract-admin [contract_addr_bech32] [flags]

Example

wasmcli tx gov submit-proposal clear-contract-admin cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5  --title clear-admin --description test  --from validator --gas 10000000   --chain-id=testing --node=http://localhost:26657 -b block

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alpe alpe requested a review from ethanfrey as a code owner July 10, 2020 14:58
@ethanfrey
Copy link
Member

Thank you for the examples. I will look at the code, but the cli commands look good. The only question I have is the

--sender flag on migrate and --creator on store and instantiate

Maybe they can be the same name? Maybe a different name?

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.

Looks good to me. Glad you tested this manually. (I don't think there is any realistic way to test this as a n integration test)

cmd/wasmcli/main.go Outdated Show resolved Hide resolved
x/wasm/client/cli/gov_tx.go Show resolved Hide resolved
x/wasm/client/cli/tx.go Show resolved Hide resolved
@alpe
Copy link
Member Author

alpe commented Jul 13, 2020

--sender flag on migrate and --creator on store and instantiate

🤔 I don't have a good idea for better naming but this is some context for the different roles:

  • Store.Creator is the address that "owns" the code object.
  • Instantiate.Creator is the address that pays the init funds. It is the creator of the contract and passed to the contract as sender
  • Migrate.Sender is the address that is passed to the contract's environment. Should be admin but for gov can be any address.

@codecov
Copy link

codecov bot commented Jul 13, 2020

Codecov Report

Merging #183 into master will increase coverage by 0.65%.
The diff coverage is 97.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #183      +/-   ##
==========================================
+ Coverage   70.34%   71.00%   +0.65%     
==========================================
  Files          27       27              
  Lines        2509     2559      +50     
==========================================
+ Hits         1765     1817      +52     
+ Misses        628      626       -2     
  Partials      116      116              
Impacted Files Coverage Δ
app/app.go 89.34% <ø> (ø)
x/wasm/internal/keeper/proposal_handler.go 68.67% <60.00%> (ø)
x/wasm/internal/keeper/keeper.go 91.17% <100.00%> (ø)
x/wasm/internal/types/codec.go 100.00% <100.00%> (ø)
x/wasm/internal/types/proposal.go 94.94% <100.00%> (+2.14%) ⬆️
x/wasm/internal/types/test_fixtures.go 91.51% <100.00%> (-0.16%) ⬇️
lcd_test/helpers.go 75.71% <0.00%> (+0.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 041d8c6...3bd01fe. Read the comment docs.

@ethanfrey
Copy link
Member

I don't have a good idea for better naming but this is some context for the different roles:

Yeah, I saw that later. I guess my approach reading the samples was to seek to make them all common, but you are right, they have different meanings and you follow the name of the fields.

My issue is that the word --creator is a bit confusing in the context of a proposal. The one creating the proposal?

Maybe the word --run-as to say the account the passed proposal will execute as. Or maybe you have a better idea.

This is a minor point, we can leave it as is. But I doubt I will look at this for quite some time, so good to clarify the naming now if possible. If you like --run-as or have another idea, go for it. Otherwise, we can leave it as you have it.

@alpe
Copy link
Member Author

alpe commented Jul 14, 2020

--run-as sounds better. I will apply the changes

@ethanfrey
Copy link
Member

Looks good. And nice to add the --amount flag to the docs as well, I missed that one.

From my side, feel free to merge

@alpe alpe merged commit 9ab18fc into master Jul 14, 2020
@alpe alpe deleted the gov_cli branch July 14, 2020 11:30
zemyblue pushed a commit to Finschia/wasmd that referenced this pull request Jan 2, 2023
* l

* start cleanup

* cleanup gaia

* remove alpha mention

* crop

* update version

* link fix
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.

CLI: gov store proposal wasm code display issue Gov: cli support
2 participants