simapp: use tmjson on InitChainer #7514

fedekunze · 2020-10-12T12:53:43Z

No description provided.

alessio · 2020-10-12T12:56:17Z

simapp/app.go

@@ -450,7 +451,9 @@ func (app *SimApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Re
 // InitChainer application update at chain initialization
 func (app *SimApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 	var genesisState GenesisState
-	app.cdc.MustUnmarshalJSON(req.AppStateBytes, &genesisState)
+	if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {


it's a pity that the library does not provide a MustUnmarshal() method :(

I see it as an advantage. Panics should be avoided.

what is the reason for using tmjson here?

I thought GenesisDoc was marshalled using tmjson on the TM side?

ah wait, i might have read too fast, this is our own GenesisState. So it should be decoded with appCodec.

Proper error handling:

define critical error type

pass it up with a stack

handle it in the main routine allowing to shutdown things (maybe some shutdown's are not relevant here, but this is the right approach anyway).

pass it up with a stack

I +1 this idea, and would even propose more aggressively to only allow Tendermint to panic. It's probably a larger discussion, but if the TM spec would expose some well-defined errors that cause TM to panic, then the SDK (or any state machine) should only return these errors, and contain 0 panic itself.

Two weeks ago I've created an issue to clean the panics wherever possible: #7409

It's probably a larger discussion

I agree very much indeed to that. But until we got stuff sorted in Tendermint, there is not much we can do other than panic()ing in the SDK.

We generally have been using stdlib encoding/json

codecov · 2020-10-12T13:02:23Z

Codecov Report

Merging #7514 into master will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #7514   +/-   ##
=======================================
  Coverage   56.03%   56.03%           
=======================================
  Files         592      592           
  Lines       37253    37254    +1     
=======================================
+ Hits        20875    20876    +1     
  Misses      14261    14261           
  Partials     2117     2117

robert-zaremba

I would prefer to not use panic

robert-zaremba · 2020-10-13T11:04:37Z

simapp/app.go

@@ -450,7 +451,9 @@ func (app *SimApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Re
 // InitChainer application update at chain initialization
 func (app *SimApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 	var genesisState GenesisState
-	app.cdc.MustUnmarshalJSON(req.AppStateBytes, &genesisState)
+	if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {
+		panic(err)


Ouch, can we return an error and handle it somewhere higher, please?

This must panic (it used to panic too). An error that happens in InitChain() is unrecoverable, as such it must cause the program to crash and make as much noise as possible.

robert-zaremba · 2020-10-13T11:04:56Z

simapp/app.go

@@ -450,7 +451,9 @@ func (app *SimApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Re
 // InitChainer application update at chain initialization
 func (app *SimApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 	var genesisState GenesisState
-	app.cdc.MustUnmarshalJSON(req.AppStateBytes, &genesisState)
+	if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil {


I see it as an advantage. Panics should be avoided.

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

@fedekunze

* Update Encoding Doc for 0.40 (#7430) * Remove deprecated docs and add first guidelines to protobuf migration * Add conventions * Reorder and add more FAQ doc * Update guidelines for pb msg definitions * Use commit hash in github links Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * docs: Update "Basics" section (#7416) * Prettier * docs: Update "Basics" section * appcli -> appd * Better wording * Fix to appCodec * Add gRPC mention * Add grpc * Reference simapp code * Update docs/basics/accounts.md Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * Add section about gRPC query services * Optional LegacyQuerierHandler * Clearer docs * Update docs/basics/app-anatomy.md Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> * Update docs/basics/app-anatomy.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Address comments * Address comments * Update docs/basics/accounts.md Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Fix misbehaviour handling for solo machine (#7515) * add timestamp to SignatureAndData Add timestamp field to signature and data. Add ValidateBasic check for timestamp. Add ValidateBasic test. Update misbehaviour handler to use supplied timestamp. * fix typo * add timestamp check * fix lint Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * update solo machine specs (#7512) * update solo machine specs * update concepts * self review fixes * Apply suggestions from code review * add note on upgarding solo machines Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Corrected 'unsafe-reset-all' help text (#7504) * Merge PR #7415: Return empty store tree on non-existing version * tendermint: update sdk to rc5 (#7527) * update sdk to rc5 * Update baseapp/params.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Bump github.com/spf13/cobra from 1.0.0 to 1.1.0 (#7553) Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.0.0 to 1.1.0. - [Release notes](https://github.com/spf13/cobra/releases) - [Changelog](https://github.com/spf13/cobra/blob/master/CHANGELOG.md) - [Commits](spf13/cobra@v1.0.0...v1.1.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * remove test_utils.go in tm client (#7522) * remove test_utils from tm client * fix build * fix lint? * fix lint? * apply @fedekunze review suggestion * add tests as per @alessio suggestion * fix typo * ibc: cleanup channel types test (#7521) * Update Building Modules documentation (#7473) * Update module-manager.md * Update modules #messages doc * Fix typo * Update message implementation example link * Update messages-and-queries.md#queries doc * Update querier.md * Update handler.md * Update links in beginblock-endblock.md * Update keeper.md * Update invariants.md * Update genesis.md * Update module-interfaces.md#transaction-commands * Update module-interfaces.md#query-commands * Update module-interfaces.md#flags * Update module-interfaces.md#rest * Update structure.md * Update module-interfaces.md#grpc * Update errors.md * Update module-interfaces.md#grpc-gateway-rest * Add more info on swagger * Address comments * Fix go.sum * Fix app-anatomy.md * Update docs/building-modules/module-interfaces.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Update docs/building-modules/querier.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Update docs/building-modules/querier.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Update docs/building-modules/module-interfaces.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Update docs/building-modules/module-interfaces.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * Address part of review comments * Update old ref of RegisterQueryService * Add example code for Manager * Update queriers.md to query-services.md and refs * Add new query-services.md * Revert "Update old ref of RegisterQueryService" This reverts commit 1ea1ea8. * Update keeper.md * Update handler.md * Update handler.md * Update messages-and-queries.md * Update docs/basics/app-anatomy.md Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> * Update docs/building-modules/intro.md Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> * Fix typo Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> * client: add GetAccount and GetAccountWithHeight to AccountRetriever (#7558) * client: add GetAccount and GetAccountWithHeight to AccountRetriever * update ADR * address comments from review * Add the option of emitting amino encoded json from the CLI (#7221) * Add the option of emitting amino encoded json * Update AMINO JSON serialization with ConvertTxToStdTx * Make the Amino flag more self documenting by serializing the BroadcastRequest type instead of StdTx * Handle amino encoding error * Update x/auth/client/cli/tx_multisign.go Co-authored-by: Alessio Treglia <alessio@tendermint.com> * Update x/auth/client/cli/tx_sign.go Co-authored-by: Alessio Treglia <alessio@tendermint.com> * Apply suggestions from code review Co-authored-by: Alessio Treglia <alessio@tendermint.com> * Fix go format Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * ibc: update solo machine client command (#7579) Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> * docs: Revert SPEC-SPEC and update x/{auth,bank,evidence,slashing} (#7407) * Revert some changes from #7404 * Update x/slashing * Address review comments * Small tweak Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * remove id in localhost (#7577) Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Christopher Goes <cwgoes@pluranimity.org> * update changelog * Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Fix solomachine cmds (#7581) * fix solo machine cli cmds * polish Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * simapp: use tmjson on InitChainer (#7514) Co-authored-by: Alessio Treglia <alessio@tendermint.com> * [IAVL]: Bump to v0.15.0-rc4 (#7549) * iavl 0.15.0-rc4 version bump * update comments on error modes for store.LoadStore * update CONFIO_URL in makefile Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Handle nil *Any in UnpackAny and add panic handler for tx decoding (#7594) * Handle nil any in UnpackAny * Add test * Add flag back * Update runTx signature * Update Simulate signature * Update calls to Simulate * Add txEncoder in baseapp * Fix TestTxWithoutPublicKey * Wrap errors * Use amino in baseapp tests * Add txEncoder arg to Check & Deliver * Fix gas in test * Fix remaining base app tests * Rename to amionTxEncoder * Update codec/types/interface_registry.go Co-authored-by: Aaron Craelius <aaron@regen.network> * golangci-lint fix Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: Cory Levinson <cjlevinson@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Neeraj Murarka <njmurarka@users.noreply.github.com> Co-authored-by: Riccardo Montagnin <riccardo.montagnin@gmail.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Zaki Manian <zaki@manian.org> Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Christopher Goes <cwgoes@pluranimity.org> Co-authored-by: Aaron Craelius <aaron@regen.network>

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

simapp: use tmjson on InitChainer

f5783f6

fedekunze requested review from aaronc, alessio and alexanderbez as code owners October 12, 2020 12:53

fedekunze requested review from amaury1093 and blushi October 12, 2020 12:54

amaury1093 approved these changes Oct 12, 2020

View reviewed changes

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 12, 2020

alessio reviewed Oct 12, 2020

View reviewed changes

alessio approved these changes Oct 12, 2020

View reviewed changes

Merge branch 'master' into fedekunze/tmjson

f5775a0

mergify bot merged commit 7474a19 into master Oct 12, 2020

mergify bot deleted the fedekunze/tmjson branch October 12, 2020 13:13

robert-zaremba reviewed Oct 13, 2020

View reviewed changes

amaury1093 mentioned this pull request Oct 13, 2020

simapp InitChainer: use json instead of tmjson #7523

Merged

10 tasks

clevinson pushed a commit that referenced this pull request Oct 19, 2020

simapp: use tmjson on InitChainer (#7514)

182b01c

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

clevinson added the backport/0.40.x (Stargate) label Oct 19, 2020

clevinson mentioned this pull request Oct 19, 2020

[backports] v0.40.0-rc1 #7590

Merged

31 tasks

larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023

simapp: use tmjson on InitChainer (cosmos#7514)

2b51bee

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simapp: use tmjson on InitChainer #7514

simapp: use tmjson on InitChainer #7514

fedekunze commented Oct 12, 2020

alessio Oct 12, 2020

robert-zaremba Oct 13, 2020

tac0turtle Oct 13, 2020

amaury1093 Oct 13, 2020

amaury1093 Oct 13, 2020

robert-zaremba Oct 13, 2020

amaury1093 Oct 13, 2020

robert-zaremba Oct 13, 2020

alessio Oct 13, 2020

aaronc Oct 13, 2020

codecov bot commented Oct 12, 2020 •

edited

Loading

robert-zaremba left a comment

robert-zaremba Oct 13, 2020

alessio Oct 13, 2020

robert-zaremba Oct 13, 2020

simapp: use tmjson on InitChainer #7514

simapp: use tmjson on InitChainer #7514

Conversation

fedekunze commented Oct 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 12, 2020 • edited Loading

Codecov Report

robert-zaremba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 12, 2020 •

edited

Loading