Add the option of emitting amino encoded json from the CLI #7221

zmanian · 2020-09-02T01:25:05Z

This adds the option of sign/multisign emitting amino encoded JSON instead of the standard JSON encoder. This is intended to make the the Legacy Amino interface for Stargate more self documenting.

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

codecov · 2020-09-02T01:29:23Z

Codecov Report

Merging #7221 into master will decrease coverage by 0.01%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##           master    #7221      +/-   ##
==========================================
- Coverage   54.79%   54.78%   -0.02%     
==========================================
  Files         601      601              
  Lines       38117    38145      +28     
==========================================
+ Hits        20888    20896       +8     
- Misses      15108    15125      +17     
- Partials     2121     2124       +3

zmanian · 2020-09-02T05:24:06Z

Works now

alexanderbez

utACK

alexanderbez · 2020-09-02T16:26:30Z

x/auth/client/cli/tx_multisign.go

@@ -146,11 +147,32 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) error {

 		sigOnly, _ := cmd.Flags().GetBool(flagSigOnly)

-		json, err := marshalSignatureJSON(txCfg, txBuilder, sigOnly)
+		aminoJSON, err := cmd.Flags().GetBool(flagAmino)


we can ignore the error IMHO

alexanderbez · 2020-09-02T16:26:53Z

x/auth/client/cli/tx_sign.go

@@ -257,11 +259,32 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
 			return err
 		}

-		json, err := marshalSignatureJSON(txCfg, txBuilder, generateSignatureOnly)
+		aminoJSON, err := cmd.Flags().GetBool(flagAmino)


zmanian · 2020-09-02T16:28:44Z

Confirmation in progress from the Lunie team that the submitted JSON actually is validates.

…tRequest type instead of StdTx

lgtm-com · 2020-09-03T05:58:29Z

This pull request introduces 1 alert when merging 9469598 into 9e85e81 - view on LGTM.com

new alerts:

1 for Useless assignment to local variable

…enerates-signed-amino-encoded-txs

lgtm-com · 2020-09-08T23:55:11Z

This pull request introduces 1 alert when merging 2f82b06 into a9547b5 - view on LGTM.com

new alerts:

1 for Useless assignment to local variable

amaury1093 · 2020-09-14T09:34:11Z

Confirmation in progress from the Lunie team that the submitted JSON actually is validates.

Did you get the confirmation finally?

If not, we could also add a test here?

alessio

Left few suggestions

x/auth/client/cli/tx_multisign.go

x/auth/client/cli/tx_sign.go

alessio · 2020-09-14T09:42:02Z

x/auth/client/cli/tx_sign.go

 		if err != nil {
 			return err
 		}


Suggested change

if err != nil {

return err

}

x/auth/client/cli/tx_sign.go

Co-authored-by: Alessio Treglia <[email protected]>

* 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 <[email protected]> * Update x/auth/client/cli/tx_sign.go Co-authored-by: Alessio Treglia <[email protected]> * Apply suggestions from code review Co-authored-by: Alessio Treglia <[email protected]> * Fix go format Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.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 <[email protected]> * Add section about gRPC query services * Optional LegacyQuerierHandler * Clearer docs * Update docs/basics/app-anatomy.md Co-authored-by: Marie Gauthier <[email protected]> * Update docs/basics/app-anatomy.md Co-authored-by: Federico Kunze <[email protected]> * Address comments * Address comments * Update docs/basics/accounts.md Co-authored-by: Marie Gauthier <[email protected]> Co-authored-by: Federico Kunze <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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] <[email protected]> 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 <[email protected]> * Update docs/building-modules/querier.md Co-authored-by: Federico Kunze <[email protected]> * Update docs/building-modules/querier.md Co-authored-by: Federico Kunze <[email protected]> * Update docs/building-modules/module-interfaces.md Co-authored-by: Federico Kunze <[email protected]> * Update docs/building-modules/module-interfaces.md Co-authored-by: Federico Kunze <[email protected]> * 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 <[email protected]> * Update docs/building-modules/intro.md Co-authored-by: Amaury Martiny <[email protected]> * Fix typo Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Amaury Martiny <[email protected]> * 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 <[email protected]> * Update x/auth/client/cli/tx_sign.go Co-authored-by: Alessio Treglia <[email protected]> * Apply suggestions from code review Co-authored-by: Alessio Treglia <[email protected]> * Fix go format Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * ibc: update solo machine client command (#7579) Co-authored-by: Federico Kunze <[email protected]> * 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 <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * remove id in localhost (#7577) Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Christopher Goes <[email protected]> * update changelog * Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk <[email protected]> * Fix solomachine cmds (#7581) * fix solo machine cli cmds * polish Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * simapp: use tmjson on InitChainer (#7514) Co-authored-by: Alessio Treglia <[email protected]> * [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 <[email protected]> * golangci-lint fix Co-authored-by: Amaury Martiny <[email protected]> Co-authored-by: Aaron Craelius <[email protected]> Co-authored-by: Cory Levinson <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Marie Gauthier <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Amaury Martiny <[email protected]> Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: colin axnér <[email protected]> Co-authored-by: Neeraj Murarka <[email protected]> Co-authored-by: Riccardo Montagnin <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Zaki Manian <[email protected]> Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: Christopher Goes <[email protected]> Co-authored-by: Aaron Craelius <[email protected]>

* 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 <[email protected]> * Update x/auth/client/cli/tx_sign.go Co-authored-by: Alessio Treglia <[email protected]> * Apply suggestions from code review Co-authored-by: Alessio Treglia <[email protected]> * Fix go format Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Add the option of emitting amino encoded json

df7d601

zmanian requested review from aaronc, alessio and alexanderbez as code owners September 2, 2020 01:25

Update AMINO JSON serialization with ConvertTxToStdTx

1b1eb57

alexanderbez approved these changes Sep 2, 2020

View reviewed changes

alexanderbez requested review from sahith-narahari, amaury1093, anilcse and clevinson September 2, 2020 16:27

jackzampolin approved these changes Sep 2, 2020

View reviewed changes

Make the Amino flag more self documenting by serializing the Broadcas…

9469598

…tRequest type instead of StdTx

Merge branch 'master' of github.com:cosmos/cosmos-sdk into zaki-cli-g…

2f82b06

…enerates-signed-amino-encoded-txs

zmanian and others added 2 commits September 8, 2020 20:39

Handle amino encoding error

f344fdb

Merge branch 'master' into zaki-cli-generates-signed-amino-encoded-txs

a2a8247

amaury1093 approved these changes Sep 14, 2020

View reviewed changes

alessio reviewed Sep 14, 2020

View reviewed changes

Merge branch 'master' into zaki-cli-generates-signed-amino-encoded-txs

fd55fd0

alessio reviewed Sep 14, 2020

View reviewed changes

x/auth/client/cli/tx_sign.go Outdated Show resolved Hide resolved

zmanian and others added 2 commits September 15, 2020 23:47

Update x/auth/client/cli/tx_multisign.go

ca1d556

Co-authored-by: Alessio Treglia <[email protected]>

Update x/auth/client/cli/tx_sign.go

ebb14b4

Co-authored-by: Alessio Treglia <[email protected]>

sahith-narahari approved these changes Sep 16, 2020

View reviewed changes

zmanian and others added 2 commits September 30, 2020 12:47

Merge branch 'master' into zaki-cli-generates-signed-amino-encoded-txs

877e0fd

Apply suggestions from code review

fe9c271

Co-authored-by: Alessio Treglia <[email protected]>

zmanian added the backport/0.40.x (Stargate) label Oct 15, 2020

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

mergify bot and others added 2 commits October 15, 2020 14:50

Merge branch 'master' into zaki-cli-generates-signed-amino-encoded-txs

f4b15a9

Fix go format

c821343

mergify bot merged commit c735a8e into master Oct 15, 2020

mergify bot deleted the zaki-cli-generates-signed-amino-encoded-txs branch October 15, 2020 15:12

clevinson mentioned this pull request Oct 19, 2020

[backports] v0.40.0-rc1 #7590

Merged

31 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the option of emitting amino encoded json from the CLI #7221

Add the option of emitting amino encoded json from the CLI #7221

zmanian commented Sep 2, 2020 •

edited

Loading

codecov bot commented Sep 2, 2020 •

edited

Loading

zmanian commented Sep 2, 2020

alexanderbez left a comment

alexanderbez Sep 2, 2020

alexanderbez Sep 2, 2020

zmanian commented Sep 2, 2020

lgtm-com bot commented Sep 3, 2020

lgtm-com bot commented Sep 8, 2020

amaury1093 commented Sep 14, 2020

alessio left a comment

alessio Sep 14, 2020

Add the option of emitting amino encoded json from the CLI #7221

Add the option of emitting amino encoded json from the CLI #7221

Conversation

zmanian commented Sep 2, 2020 • edited Loading

codecov bot commented Sep 2, 2020 • edited Loading

Codecov Report

zmanian commented Sep 2, 2020

alexanderbez left a comment

Choose a reason for hiding this comment

alexanderbez Sep 2, 2020

Choose a reason for hiding this comment

alexanderbez Sep 2, 2020

Choose a reason for hiding this comment

zmanian commented Sep 2, 2020

lgtm-com bot commented Sep 3, 2020

lgtm-com bot commented Sep 8, 2020

amaury1093 commented Sep 14, 2020

alessio left a comment

Choose a reason for hiding this comment

alessio Sep 14, 2020

Choose a reason for hiding this comment

zmanian commented Sep 2, 2020 •

edited

Loading

codecov bot commented Sep 2, 2020 •

edited

Loading