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

Improve Contract initialization in genesis and tooling #346

Merged
merged 8 commits into from
Jan 7, 2021

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Dec 16, 2020

Resolves #326

  • Extend wasm genesis with gen_mgs field for messages
  • handle messages in genesis init process
> wasmd add-wasm-genesis-message -h

genesis_contract_msg_326 f014acb
Wasm genesis subcommands

Usage:
  wasmd add-wasm-genesis-message [flags]
  wasmd add-wasm-genesis-message [command]

Available Commands:
  execute              Execute a command on a wasm contract
  instantiate-contract Instantiate a wasm contract
  list-codes           Lists all codes from genesis contract dump and queued messages
  list-contracts       Lists all contracts from genesis contract dump and queued messages
  store                Upload a wasm binary

Flags:
  -h, --help   help for add-wasm-genesis-message

Global Flags:
      --home string         directory for config and data (default "/Users/alex/.wasmd")
      --log_format string   The logging format (json|plain) (default "plain")
      --log_level string    The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --trace               print out full stack trace on errors

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.

Nice start. I like the approach and implementation so far

app/app.go Show resolved Hide resolved
doc/proto.md Show resolved Hide resolved
x/wasm/internal/keeper/genesis.go Show resolved Hide resolved
x/wasm/internal/keeper/genesis_test.go Outdated Show resolved Hide resolved
x/wasm/internal/keeper/test_common.go Show resolved Hide resolved
x/wasm/internal/types/genesis.proto Show resolved Hide resolved
@CosmWasm CosmWasm deleted a comment from codecov bot Dec 21, 2020
@CosmWasm CosmWasm deleted a comment from codecov bot Dec 23, 2020
@alpe alpe marked this pull request as ready for review December 23, 2020 13:25
@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #346 (bac2339) into master (9c0b974) will decrease coverage by 12.56%.
The diff coverage is 58.12%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #346       +/-   ##
===========================================
- Coverage   69.64%   57.08%   -12.57%     
===========================================
  Files          28       33        +5     
  Lines        2425     3421      +996     
===========================================
+ Hits         1689     1953      +264     
- Misses        618     1318      +700     
- Partials      118      150       +32     
Impacted Files Coverage Δ
x/wasm/client/cli/gov_tx.go 0.00% <0.00%> (ø)
x/wasm/client/cli/query.go 0.00% <ø> (ø)
x/wasm/internal/keeper/keeper.go 89.97% <ø> (ø)
x/wasm/client/cli/tx.go 26.24% <42.59%> (ø)
x/wasm/module.go 23.07% <50.00%> (+1.07%) ⬆️
x/wasm/client/cli/genesis_msg.go 71.75% <71.75%> (ø)
x/wasm/internal/keeper/genesis.go 90.32% <80.00%> (-2.28%) ⬇️
x/wasm/internal/types/test_fixtures.go 95.78% <92.68%> (-1.02%) ⬇️
app/app.go 87.75% <100.00%> (ø)
x/wasm/internal/types/genesis.go 97.61% <100.00%> (+1.06%) ⬆️
... and 7 more

@alpe alpe changed the title Start with wasm msg in genesis Improve Contract initialization in genesis and tooling Jan 5, 2021
@alpe alpe requested a review from ethanfrey January 5, 2021 10:57
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.

Very nice. I think this is ready to merge. I was just a bit confused by some of the changes to cli/proposal parsing, but the genesis stuff looks solid.

One comment, which could be added later, is to add a file (REAMDE or other) that shows how to add a few items to the genesis file via cli and how to get the contract addresses that would be created. I guess contrib/local/00-genesis.sh is a good reference, but a bit of test in the README (linking to that example) would make onboarding easier

x/wasm/client/cli/gov_tx.go Show resolved Hide resolved
x/wasm/client/cli/tx.go Show resolved Hide resolved
x/wasm/client/cli/tx.go Show resolved Hide resolved
x/wasm/internal/keeper/genesis.go Show resolved Hide resolved
x/wasm/internal/keeper/genesis_test.go Show resolved Hide resolved
x/wasm/module.go Show resolved Hide resolved
@alpe
Copy link
Contributor Author

alpe commented Jan 7, 2021

After #358

@alpe
Copy link
Contributor Author

alpe commented Jan 7, 2021

Rebased

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.

I will merge, but please address my comment in a follow-up PR (or let me know it is better how it is). I can now at least do last integration tests

return all
}

func hasAccountBalance(cmd *cobra.Command, a map[string]json.RawMessage, sender sdk.AccAddress, coins sdk.Coins) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

variable a is a bit confusing.

But I would note this check is only required if ! coins.IsEmpty(). I guess it could make sense that funded only genesis accounts send messages, but if we just want to prevent obvious errors, we can short circuit this is we are not sending any balance, so we let accounts with no balances create contracts as long as they don't interact with funds.

@ethanfrey ethanfrey merged commit 321491e into master Jan 7, 2021
@ethanfrey ethanfrey deleted the genesis_contract_msg_326 branch January 7, 2021 14:02
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.

Make it easy to initialize contracts in genesis file
2 participants