From 99e2e0f060b6675a449cd07147ca0bc96c577e56 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Mon, 18 Oct 2021 12:32:43 +0200 Subject: [PATCH 1/3] revert: Remove SIGN_MODE_AMINO_AUX (#10322) ## Description Revert #10268 As part of the TX working group, we decided not to introduce a new sign mode for tipper signing. The tipper will use amino-json to sign via ledger (see #10346). Also, add `signing.SignerData#Address` (will be used in subsequent PR #10346) --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- client/tx/tx.go | 1 + docs/core/proto-docs.md | 3 +- proto/cosmos/tx/signing/v1beta1/signing.proto | 4 - server/rosetta/converter.go | 2 + simapp/helpers/test_helpers.go | 2 + types/tx/signing/signing.pb.go | 78 +++++----- x/auth/client/cli/tx_multisign.go | 16 +- x/auth/client/cli/validate_sigs.go | 2 + x/auth/middleware/feegrant_test.go | 2 + x/auth/middleware/sigverify.go | 3 + x/auth/middleware/testutil_test.go | 2 + .../migrations/legacytx/amino_signing_test.go | 2 + x/auth/migrations/legacytx/stdsign_aux.go | 47 ------ x/auth/signing/handler_map_test.go | 2 + x/auth/signing/sign_mode_handler.go | 5 +- x/auth/signing/verify_test.go | 2 + x/auth/testutil/suite.go | 4 + x/auth/tx/amino_aux.go | 57 ------- x/auth/tx/amino_aux_test.go | 141 ------------------ x/auth/tx/direct_aux.go | 7 +- x/auth/tx/direct_aux_test.go | 2 + x/auth/tx/direct_test.go | 2 + x/auth/tx/legacy_amino_json.go | 2 +- x/auth/tx/legacy_amino_json_test.go | 2 + x/auth/tx/mode_handler.go | 5 +- x/authz/query.pb.go | 2 +- 26 files changed, 91 insertions(+), 306 deletions(-) delete mode 100644 x/auth/migrations/legacytx/stdsign_aux.go delete mode 100644 x/auth/tx/amino_aux.go delete mode 100644 x/auth/tx/amino_aux_test.go diff --git a/client/tx/tx.go b/client/tx/tx.go index 23de49941594..737a528cff0e 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -225,6 +225,7 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo AccountNumber: txf.accountNumber, Sequence: txf.sequence, SignerIndex: signerIndex, + Address: sdk.AccAddress(pubKey.Address()).String(), } // For SIGN_MODE_DIRECT, calling SetSignatures calls setSignerInfos on diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md index aaaaf20f1655..dbc0ff6b21ce 100644 --- a/docs/core/proto-docs.md +++ b/docs/core/proto-docs.md @@ -1279,7 +1279,7 @@ GrantAuthorization defines the GenesisState/GrantAuthorization type. ### QueryGranterGrantsRequest -QueryGranterGrantsRequest is the request type for the Query/Grants RPC method. +QueryGranterGrantsRequest is the request type for the Query/GranterGrants RPC method. | Field | Type | Label | Description | @@ -9400,7 +9400,6 @@ SignMode represents a signing mode with its own security guarantees. | SIGN_MODE_DIRECT | 1 | SIGN_MODE_DIRECT specifies a signing mode which uses SignDoc and is verified with raw bytes from Tx. | | SIGN_MODE_TEXTUAL | 2 | SIGN_MODE_TEXTUAL is a future signing mode that will verify some human-readable textual representation on top of the binary representation from SIGN_MODE_DIRECT. It is currently not supported. | | SIGN_MODE_DIRECT_AUX | 3 | SIGN_MODE_DIRECT_AUX specifies a signing mode which uses SignDocDirectAux. As opposed to SIGN_MODE_DIRECT, this sign mode does not require signers signing over other signers' `signer_info`. It also allows for adding Tips in transactions. | -| SIGN_MODE_AMINO_AUX | 4 | SIGN_MODE_AMINO_AUX specifies a signing mode which uses SignDocAminoAux. | | SIGN_MODE_LEGACY_AMINO_JSON | 127 | SIGN_MODE_LEGACY_AMINO_JSON is a backwards compatibility mode which uses Amino JSON and will be removed in the future. | diff --git a/proto/cosmos/tx/signing/v1beta1/signing.proto b/proto/cosmos/tx/signing/v1beta1/signing.proto index 0a13282ac0b3..a87b405f93cf 100644 --- a/proto/cosmos/tx/signing/v1beta1/signing.proto +++ b/proto/cosmos/tx/signing/v1beta1/signing.proto @@ -27,10 +27,6 @@ enum SignMode { // for adding Tips in transactions. SIGN_MODE_DIRECT_AUX = 3; - // SIGN_MODE_AMINO_AUX specifies a signing mode which uses - // SignDocAminoAux. - SIGN_MODE_AMINO_AUX = 4; - // SIGN_MODE_LEGACY_AMINO_JSON is a backwards compatibility mode which uses // Amino JSON and will be removed in the future. SIGN_MODE_LEGACY_AMINO_JSON = 127; diff --git a/server/rosetta/converter.go b/server/rosetta/converter.go index 4fdd87c75b67..3591605eaadb 100644 --- a/server/rosetta/converter.go +++ b/server/rosetta/converter.go @@ -722,9 +722,11 @@ func (c converter) SigningComponents(tx authsigning.Tx, metadata *ConstructionMe // set the signer data signerData := authsigning.SignerData{ + Address: signer.String(), ChainID: metadata.ChainID, AccountNumber: metadata.SignersData[i].AccountNumber, Sequence: metadata.SignersData[i].Sequence, + SignerIndex: i, } // get signature bytes diff --git a/simapp/helpers/test_helpers.go b/simapp/helpers/test_helpers.go index 9ccecbd976c4..887bc8d3da27 100644 --- a/simapp/helpers/test_helpers.go +++ b/simapp/helpers/test_helpers.go @@ -57,9 +57,11 @@ func GenTx(gen client.TxConfig, msgs []sdk.Msg, feeAmt sdk.Coins, gas uint64, ch // 2nd round: once all signer infos are set, every signer can sign. for i, p := range priv { signerData := authsign.SignerData{ + Address: sdk.AccAddress(p.PubKey().Address()).String(), ChainID: chainID, AccountNumber: accNums[i], Sequence: accSeqs[i], + SignerIndex: i, } signBytes, err := gen.SignModeHandler().GetSignBytes(signMode, signerData, tx.GetTx()) if err != nil { diff --git a/types/tx/signing/signing.pb.go b/types/tx/signing/signing.pb.go index e5e1ec4770ea..41cdf01fcf20 100644 --- a/types/tx/signing/signing.pb.go +++ b/types/tx/signing/signing.pb.go @@ -43,9 +43,6 @@ const ( // require signers signing over other signers' `signer_info`. It also allows // for adding Tips in transactions. SignMode_SIGN_MODE_DIRECT_AUX SignMode = 3 - // SIGN_MODE_AMINO_AUX specifies a signing mode which uses - // SignDocAminoAux. - SignMode_SIGN_MODE_AMINO_AUX SignMode = 4 // SIGN_MODE_LEGACY_AMINO_JSON is a backwards compatibility mode which uses // Amino JSON and will be removed in the future. SignMode_SIGN_MODE_LEGACY_AMINO_JSON SignMode = 127 @@ -56,7 +53,6 @@ var SignMode_name = map[int32]string{ 1: "SIGN_MODE_DIRECT", 2: "SIGN_MODE_TEXTUAL", 3: "SIGN_MODE_DIRECT_AUX", - 4: "SIGN_MODE_AMINO_AUX", 127: "SIGN_MODE_LEGACY_AMINO_JSON", } @@ -65,7 +61,6 @@ var SignMode_value = map[string]int32{ "SIGN_MODE_DIRECT": 1, "SIGN_MODE_TEXTUAL": 2, "SIGN_MODE_DIRECT_AUX": 3, - "SIGN_MODE_AMINO_AUX": 4, "SIGN_MODE_LEGACY_AMINO_JSON": 127, } @@ -403,43 +398,42 @@ func init() { } var fileDescriptor_9a54958ff3d0b1b9 = []byte{ - // 566 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x94, 0x41, 0x4f, 0xd4, 0x4e, - 0x18, 0xc6, 0x5b, 0xb6, 0x10, 0x78, 0xf9, 0xe7, 0x9f, 0x3a, 0x2c, 0x71, 0xa9, 0xa6, 0x12, 0x3c, - 0x48, 0x4c, 0x98, 0x06, 0x38, 0x18, 0xbd, 0x95, 0xdd, 0xba, 0xac, 0xb0, 0x8b, 0xb6, 0x4b, 0x82, - 0x5e, 0x36, 0x6d, 0x77, 0xa8, 0x0d, 0xdb, 0x4e, 0xed, 0x4c, 0x0d, 0x3d, 0xf9, 0x15, 0xfc, 0x14, - 0x26, 0x7e, 0x0e, 0x2f, 0x1e, 0x39, 0x7a, 0x34, 0xf0, 0x19, 0xbc, 0x1b, 0xa6, 0xed, 0x16, 0x0d, - 0xc6, 0xb8, 0xa7, 0xcd, 0xbc, 0xcf, 0xd3, 0xdf, 0xfb, 0x4c, 0xde, 0x77, 0x16, 0x1e, 0xf9, 0x94, - 0x45, 0x94, 0x19, 0xfc, 0xdc, 0x60, 0x61, 0x10, 0x87, 0x71, 0x60, 0xbc, 0xdf, 0xf6, 0x08, 0x77, - 0xb7, 0xab, 0x33, 0x4e, 0x52, 0xca, 0x29, 0x5a, 0x2b, 0x8c, 0x98, 0x9f, 0xe3, 0x4a, 0x28, 0x8d, - 0xda, 0x56, 0xc9, 0xf0, 0xd3, 0x3c, 0xe1, 0xd4, 0x88, 0xb2, 0x09, 0x0f, 0x59, 0x58, 0x83, 0xaa, - 0x42, 0x41, 0xd2, 0xd6, 0x02, 0x4a, 0x83, 0x09, 0x31, 0xc4, 0xc9, 0xcb, 0x4e, 0x0d, 0x37, 0xce, - 0x0b, 0x69, 0xe3, 0x14, 0x9a, 0x4e, 0x18, 0xc4, 0x2e, 0xcf, 0x52, 0xd2, 0x21, 0xcc, 0x4f, 0xc3, - 0x84, 0xd3, 0x94, 0xa1, 0x01, 0x00, 0xab, 0xea, 0xac, 0x25, 0xaf, 0x37, 0x36, 0x97, 0x77, 0x30, - 0xfe, 0x63, 0x22, 0x7c, 0x0b, 0xc4, 0xbe, 0x41, 0xd8, 0xf8, 0xa1, 0xc0, 0xca, 0x2d, 0x1e, 0xb4, - 0x0b, 0x90, 0x64, 0xde, 0x24, 0xf4, 0x47, 0x67, 0x24, 0x6f, 0xc9, 0xeb, 0xf2, 0xe6, 0xf2, 0x4e, - 0x13, 0x17, 0x79, 0x71, 0x95, 0x17, 0x9b, 0x71, 0x6e, 0x2f, 0x15, 0xbe, 0x03, 0x92, 0xa3, 0x2e, - 0x28, 0x63, 0x97, 0xbb, 0xad, 0x39, 0x61, 0xdf, 0xfd, 0xb7, 0x58, 0xb8, 0xe3, 0x72, 0xd7, 0x16, - 0x00, 0xa4, 0xc1, 0x22, 0x23, 0xef, 0x32, 0x12, 0xfb, 0xa4, 0xd5, 0x58, 0x97, 0x37, 0x15, 0x7b, - 0x7a, 0xd6, 0xbe, 0x34, 0x40, 0xb9, 0xb6, 0xa2, 0x21, 0x2c, 0xb0, 0x30, 0x0e, 0x26, 0xa4, 0x8c, - 0xf7, 0x6c, 0x86, 0x7e, 0xd8, 0x11, 0x84, 0x7d, 0xc9, 0x2e, 0x59, 0xe8, 0x15, 0xcc, 0x8b, 0x29, - 0x95, 0x97, 0x78, 0x3a, 0x0b, 0xb4, 0x7f, 0x0d, 0xd8, 0x97, 0xec, 0x82, 0xa4, 0x8d, 0x60, 0xa1, - 0x68, 0x83, 0x9e, 0x80, 0x12, 0xd1, 0x71, 0x11, 0xf8, 0xff, 0x9d, 0x87, 0x7f, 0x61, 0xf7, 0xe9, - 0x98, 0xd8, 0xe2, 0x03, 0x74, 0x1f, 0x96, 0xa6, 0x43, 0x13, 0xc9, 0xfe, 0xb3, 0xeb, 0x82, 0xf6, - 0x59, 0x86, 0x79, 0xd1, 0x13, 0x1d, 0xc0, 0xa2, 0x17, 0x72, 0x37, 0x4d, 0xdd, 0x6a, 0x68, 0x46, - 0xd5, 0xa4, 0xd8, 0x49, 0x3c, 0x5d, 0xc1, 0xaa, 0x53, 0x9b, 0x46, 0x89, 0xeb, 0xf3, 0xbd, 0x90, - 0x9b, 0xd7, 0x9f, 0xd9, 0x53, 0x00, 0x72, 0x7e, 0xd9, 0xb5, 0x39, 0xb1, 0x6b, 0x33, 0x0d, 0xf5, - 0x06, 0x66, 0x6f, 0x1e, 0x1a, 0x2c, 0x8b, 0x1e, 0x7f, 0x92, 0x61, 0xb1, 0xba, 0x23, 0x5a, 0x83, - 0x55, 0xa7, 0xd7, 0x1d, 0x8c, 0xfa, 0x47, 0x1d, 0x6b, 0x74, 0x3c, 0x70, 0x5e, 0x5a, 0xed, 0xde, - 0xf3, 0x9e, 0xd5, 0x51, 0x25, 0xd4, 0x04, 0xb5, 0x96, 0x3a, 0x3d, 0xdb, 0x6a, 0x0f, 0x55, 0x19, - 0xad, 0xc2, 0x9d, 0xba, 0x3a, 0xb4, 0x4e, 0x86, 0xc7, 0xe6, 0xa1, 0x3a, 0x87, 0x5a, 0xd0, 0xfc, - 0xdd, 0x3c, 0x32, 0x8f, 0x4f, 0xd4, 0x06, 0xba, 0x0b, 0x2b, 0xb5, 0x62, 0xf6, 0x7b, 0x83, 0x23, - 0x21, 0x28, 0xe8, 0x01, 0xdc, 0xab, 0x85, 0x43, 0xab, 0x6b, 0xb6, 0x5f, 0x97, 0xfa, 0x0b, 0xe7, - 0x68, 0xa0, 0x7e, 0xd8, 0xeb, 0x7e, 0xbd, 0xd4, 0xe5, 0x8b, 0x4b, 0x5d, 0xfe, 0x7e, 0xa9, 0xcb, - 0x1f, 0xaf, 0x74, 0xe9, 0xe2, 0x4a, 0x97, 0xbe, 0x5d, 0xe9, 0xd2, 0x9b, 0xad, 0x20, 0xe4, 0x6f, - 0x33, 0x0f, 0xfb, 0x34, 0x32, 0xaa, 0x77, 0x2f, 0x7e, 0xb6, 0xd8, 0xf8, 0xcc, 0xe0, 0x79, 0x42, - 0x6e, 0xfe, 0x99, 0x78, 0x0b, 0xe2, 0xd5, 0xec, 0xfe, 0x0c, 0x00, 0x00, 0xff, 0xff, 0x54, 0x1d, - 0xa5, 0xb2, 0x68, 0x04, 0x00, 0x00, + // 558 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x9c, 0x94, 0xc1, 0x6e, 0xd3, 0x4c, + 0x10, 0xc7, 0xed, 0x26, 0xad, 0xda, 0xe9, 0xa7, 0x4f, 0x66, 0x49, 0xa5, 0xd4, 0x20, 0x53, 0x95, + 0x03, 0x15, 0x52, 0xd7, 0x6a, 0x72, 0x40, 0x70, 0x73, 0x12, 0x93, 0x86, 0x36, 0x09, 0xd8, 0x89, + 0x54, 0xb8, 0x58, 0xb6, 0xb3, 0x35, 0x56, 0x63, 0xaf, 0xf1, 0xae, 0x51, 0x7d, 0xe2, 0x09, 0x90, + 0x78, 0x0d, 0x9e, 0x83, 0x0b, 0xc7, 0x1e, 0x39, 0xa2, 0xe4, 0x19, 0xb8, 0xa3, 0xd8, 0x71, 0x12, + 0x50, 0x11, 0x22, 0x27, 0x6b, 0x66, 0xfe, 0xfb, 0x9b, 0xff, 0x6a, 0x66, 0x0d, 0x8f, 0x5c, 0xca, + 0x02, 0xca, 0x54, 0x7e, 0xad, 0x32, 0xdf, 0x0b, 0xfd, 0xd0, 0x53, 0xdf, 0x9f, 0x38, 0x84, 0xdb, + 0x27, 0x45, 0x8c, 0xa3, 0x98, 0x72, 0x8a, 0xf6, 0x73, 0x21, 0xe6, 0xd7, 0xb8, 0x28, 0xcc, 0x85, + 0xf2, 0xf1, 0x9c, 0xe1, 0xc6, 0x69, 0xc4, 0xa9, 0x1a, 0x24, 0x63, 0xee, 0x33, 0x7f, 0x09, 0x2a, + 0x12, 0x39, 0x49, 0xde, 0xf7, 0x28, 0xf5, 0xc6, 0x44, 0xcd, 0x22, 0x27, 0xb9, 0x54, 0xed, 0x30, + 0xcd, 0x4b, 0x87, 0x97, 0x50, 0x31, 0x7d, 0x2f, 0xb4, 0x79, 0x12, 0x93, 0x16, 0x61, 0x6e, 0xec, + 0x47, 0x9c, 0xc6, 0x0c, 0xf5, 0x00, 0x58, 0x91, 0x67, 0x55, 0xf1, 0xa0, 0x74, 0xb4, 0x5b, 0xc3, + 0xf8, 0x8f, 0x8e, 0xf0, 0x2d, 0x10, 0x63, 0x85, 0x70, 0xf8, 0xa3, 0x0c, 0x77, 0x6f, 0xd1, 0xa0, + 0x3a, 0x40, 0x94, 0x38, 0x63, 0xdf, 0xb5, 0xae, 0x48, 0x5a, 0x15, 0x0f, 0xc4, 0xa3, 0xdd, 0x5a, + 0x05, 0xe7, 0x7e, 0x71, 0xe1, 0x17, 0x6b, 0x61, 0x6a, 0xec, 0xe4, 0xba, 0x33, 0x92, 0xa2, 0x36, + 0x94, 0x47, 0x36, 0xb7, 0xab, 0x1b, 0x99, 0xbc, 0xfe, 0x6f, 0xb6, 0x70, 0xcb, 0xe6, 0xb6, 0x91, + 0x01, 0x90, 0x0c, 0xdb, 0x8c, 0xbc, 0x4b, 0x48, 0xe8, 0x92, 0x6a, 0xe9, 0x40, 0x3c, 0x2a, 0x1b, + 0x8b, 0x58, 0xfe, 0x52, 0x82, 0xf2, 0x4c, 0x8a, 0x06, 0xb0, 0xc5, 0xfc, 0xd0, 0x1b, 0x93, 0xb9, + 0xbd, 0x67, 0x6b, 0xf4, 0xc3, 0x66, 0x46, 0x38, 0x15, 0x8c, 0x39, 0x0b, 0xbd, 0x82, 0xcd, 0x6c, + 0x4a, 0xf3, 0x4b, 0x3c, 0x5d, 0x07, 0xda, 0x9d, 0x01, 0x4e, 0x05, 0x23, 0x27, 0xc9, 0x16, 0x6c, + 0xe5, 0x6d, 0xd0, 0x13, 0x28, 0x07, 0x74, 0x94, 0x1b, 0xfe, 0xbf, 0xf6, 0xf0, 0x2f, 0xec, 0x2e, + 0x1d, 0x11, 0x23, 0x3b, 0x80, 0xee, 0xc3, 0xce, 0x62, 0x68, 0x99, 0xb3, 0xff, 0x8c, 0x65, 0x42, + 0xfe, 0x2c, 0xc2, 0x66, 0xd6, 0x13, 0x9d, 0xc1, 0xb6, 0xe3, 0x73, 0x3b, 0x8e, 0xed, 0x62, 0x68, + 0x6a, 0xd1, 0x24, 0xdf, 0x49, 0xbc, 0x58, 0xc1, 0xa2, 0x53, 0x93, 0x06, 0x91, 0xed, 0xf2, 0x86, + 0xcf, 0xb5, 0xd9, 0x31, 0x63, 0x01, 0x40, 0xe6, 0x2f, 0xbb, 0xb6, 0x91, 0xed, 0xda, 0x5a, 0x43, + 0x5d, 0xc1, 0x34, 0x36, 0xa1, 0xc4, 0x92, 0xe0, 0xf1, 0x47, 0x11, 0xb6, 0x8b, 0x3b, 0xa2, 0x7d, + 0xd8, 0x33, 0x3b, 0xed, 0x9e, 0xd5, 0xed, 0xb7, 0x74, 0x6b, 0xd8, 0x33, 0x5f, 0xea, 0xcd, 0xce, + 0xf3, 0x8e, 0xde, 0x92, 0x04, 0x54, 0x01, 0x69, 0x59, 0x6a, 0x75, 0x0c, 0xbd, 0x39, 0x90, 0x44, + 0xb4, 0x07, 0x77, 0x96, 0xd9, 0x81, 0x7e, 0x31, 0x18, 0x6a, 0xe7, 0xd2, 0x06, 0xaa, 0x42, 0xe5, + 0x77, 0xb1, 0xa5, 0x0d, 0x2f, 0xa4, 0x12, 0x7a, 0x00, 0xf7, 0x96, 0x95, 0x73, 0xbd, 0xad, 0x35, + 0x5f, 0x5b, 0x5a, 0xb7, 0xd3, 0xeb, 0x5b, 0x2f, 0xcc, 0x7e, 0x4f, 0xfa, 0xd0, 0x68, 0x7f, 0x9d, + 0x28, 0xe2, 0xcd, 0x44, 0x11, 0xbf, 0x4f, 0x14, 0xf1, 0xd3, 0x54, 0x11, 0x6e, 0xa6, 0x8a, 0xf0, + 0x6d, 0xaa, 0x08, 0x6f, 0x8e, 0x3d, 0x9f, 0xbf, 0x4d, 0x1c, 0xec, 0xd2, 0x40, 0x2d, 0x9e, 0x77, + 0xf6, 0x39, 0x66, 0xa3, 0x2b, 0x95, 0xa7, 0x11, 0x59, 0xfd, 0x67, 0x38, 0x5b, 0xd9, 0xe3, 0xa8, + 0xff, 0x0c, 0x00, 0x00, 0xff, 0xff, 0xda, 0x51, 0x6b, 0x5b, 0x4f, 0x04, 0x00, 0x00, } func (m *SignatureDescriptors) Marshal() (dAtA []byte, err error) { diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index 4044f8b3de2e..5f8883412593 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -127,13 +127,15 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) { return fmt.Errorf("set the chain id with either the --chain-id flag or config file") } - signingData := signing.SignerData{ - ChainID: txFactory.ChainID(), - AccountNumber: txFactory.AccountNumber(), - Sequence: txFactory.Sequence(), - } + for j, sig := range sigs { + signingData := signing.SignerData{ + Address: sdk.AccAddress(sig.PubKey.Address()).String(), + ChainID: txFactory.ChainID(), + AccountNumber: txFactory.AccountNumber(), + Sequence: txFactory.Sequence(), + SignerIndex: j, + } - for _, sig := range sigs { err = signing.VerifySignature(sig.PubKey, signingData, sig.Data, txCfg.SignModeHandler(), txBuilder.GetTx()) if err != nil { addr, _ := sdk.AccAddressFromHex(sig.PubKey.Address().String()) @@ -322,9 +324,11 @@ func makeBatchMultisignCmd() func(cmd *cobra.Command, args []string) error { multisigPub := pubKey.(*kmultisig.LegacyAminoPubKey) multisigSig := multisig.NewMultisig(len(multisigPub.PubKeys)) signingData := signing.SignerData{ + Address: sdk.AccAddress(pubKey.Address()).String(), ChainID: txFactory.ChainID(), AccountNumber: txFactory.AccountNumber(), Sequence: txFactory.Sequence(), + SignerIndex: i, } for _, sig := range signatureBatch { diff --git a/x/auth/client/cli/validate_sigs.go b/x/auth/client/cli/validate_sigs.go index 03acf833f13d..9a0911d3a8a3 100644 --- a/x/auth/client/cli/validate_sigs.go +++ b/x/auth/client/cli/validate_sigs.go @@ -106,9 +106,11 @@ func printAndValidateSigs( } signingData := authsigning.SignerData{ + Address: sigAddr.String(), ChainID: chainID, AccountNumber: accNum, Sequence: accSeq, + SignerIndex: i, } err = authsigning.VerifySignature(pubKey, signingData, sig.Data, signModeHandler, sigTx) if err != nil { diff --git a/x/auth/middleware/feegrant_test.go b/x/auth/middleware/feegrant_test.go index d45b44160496..c76b3ec825e4 100644 --- a/x/auth/middleware/feegrant_test.go +++ b/x/auth/middleware/feegrant_test.go @@ -212,9 +212,11 @@ func genTxWithFeeGranter(gen client.TxConfig, msgs []sdk.Msg, feeAmt sdk.Coins, // 2nd round: once all signer infos are set, every signer can sign. for i, p := range priv { signerData := authsign.SignerData{ + Address: sdk.AccAddress(p.PubKey().Address()).String(), ChainID: chainID, AccountNumber: accNums[i], Sequence: accSeqs[i], + SignerIndex: i, } signBytes, err := gen.SignModeHandler().GetSignBytes(signMode, signerData, tx.GetTx()) if err != nil { diff --git a/x/auth/middleware/sigverify.go b/x/auth/middleware/sigverify.go index 9ae464932684..0613b835c98f 100644 --- a/x/auth/middleware/sigverify.go +++ b/x/auth/middleware/sigverify.go @@ -481,10 +481,13 @@ func (svm sigVerificationTxHandler) sigVerify(ctx context.Context, tx sdk.Tx, is if !genesis { accNum = acc.GetAccountNumber() } + signerData := authsigning.SignerData{ + Address: signerAddrs[i].String(), ChainID: chainID, AccountNumber: accNum, Sequence: acc.GetSequence(), + SignerIndex: i, } if !simulate { diff --git a/x/auth/middleware/testutil_test.go b/x/auth/middleware/testutil_test.go index 11cfaf72c37e..4f174f6e69f2 100644 --- a/x/auth/middleware/testutil_test.go +++ b/x/auth/middleware/testutil_test.go @@ -139,9 +139,11 @@ func (s *MWTestSuite) createTestTx(txBuilder client.TxBuilder, privs []cryptotyp sigsV2 = []signing.SignatureV2{} for i, priv := range privs { signerData := xauthsigning.SignerData{ + Address: sdk.AccAddress(priv.PubKey().Address()).String(), ChainID: chainID, AccountNumber: accNums[i], Sequence: accSeqs[i], + SignerIndex: i, } sigV2, err := tx.SignWithPrivKey( s.clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, diff --git a/x/auth/migrations/legacytx/amino_signing_test.go b/x/auth/migrations/legacytx/amino_signing_test.go index 7abccbfd8206..89410d6c8948 100644 --- a/x/auth/migrations/legacytx/amino_signing_test.go +++ b/x/auth/migrations/legacytx/amino_signing_test.go @@ -46,9 +46,11 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { handler := stdTxSignModeHandler{} signingData := signing.SignerData{ + Address: addr1.String(), ChainID: chainId, AccountNumber: accNum, Sequence: seqNum, + SignerIndex: 0, } signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) diff --git a/x/auth/migrations/legacytx/stdsign_aux.go b/x/auth/migrations/legacytx/stdsign_aux.go deleted file mode 100644 index 5ba8539e747d..000000000000 --- a/x/auth/migrations/legacytx/stdsign_aux.go +++ /dev/null @@ -1,47 +0,0 @@ -package legacytx - -import ( - "encoding/json" - "fmt" - - "github.com/cosmos/cosmos-sdk/codec/legacy" - sdk "github.com/cosmos/cosmos-sdk/types" -) - -type StdSignDocAux struct { - AccountNumber uint64 `json:"account_number" yaml:"account_number"` - Sequence uint64 `json:"sequence" yaml:"sequence"` - TimeoutHeight uint64 `json:"timeout_height,omitempty" yaml:"timeout_height"` - ChainID string `json:"chain_id" yaml:"chain_id"` - Memo string `json:"memo" yaml:"memo"` - Msgs []json.RawMessage `json:"msgs" yaml:"msgs"` - Tip sdk.Coins `json:"tip" yaml:"tip"` -} - -// StdSignBytes returns the bytes to sign for a transaction. -func StdSignAuxBytes(chainID string, accnum, sequence, timeout uint64, tip sdk.Coins, msgs []sdk.Msg, memo string) []byte { - msgsBytes := make([]json.RawMessage, 0, len(msgs)) - for _, msg := range msgs { - legacyMsg, ok := msg.(LegacyMsg) - if !ok { - panic(fmt.Errorf("expected %T when using AMINO_AUX", (*LegacyMsg)(nil))) - } - - msgsBytes = append(msgsBytes, json.RawMessage(legacyMsg.GetSignBytes())) - } - - bz, err := legacy.Cdc.MarshalJSON(StdSignDocAux{ - AccountNumber: accnum, - ChainID: chainID, - Memo: memo, - Msgs: msgsBytes, - Sequence: sequence, - TimeoutHeight: timeout, - Tip: tip, - }) - if err != nil { - panic(err) - } - - return sdk.MustSortJSON(bz) -} diff --git a/x/auth/signing/handler_map_test.go b/x/auth/signing/handler_map_test.go index 5da162d3271f..88559c8c32a2 100644 --- a/x/auth/signing/handler_map_test.go +++ b/x/auth/signing/handler_map_test.go @@ -60,9 +60,11 @@ func TestHandlerMap_GetSignBytes(t *testing.T) { aminoJSONHandler := legacytx.NewStdTxSignModeHandler() signingData := signing.SignerData{ + Address: addr1.String(), ChainID: chainId, AccountNumber: accNum, Sequence: seqNum, + SignerIndex: 0, } signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) diff --git a/x/auth/signing/sign_mode_handler.go b/x/auth/signing/sign_mode_handler.go index cf82b3f66d8e..d66dedebbdd4 100644 --- a/x/auth/signing/sign_mode_handler.go +++ b/x/auth/signing/sign_mode_handler.go @@ -23,6 +23,9 @@ type SignModeHandler interface { // SignerData is the specific information needed to sign a transaction that generally // isn't included in the transaction body itself type SignerData struct { + // The address of the signer. + Address string + // ChainID is the chain that this transaction is targeted ChainID string @@ -35,6 +38,6 @@ type SignerData struct { // info. Sequence uint64 - // SignerIndex index of signer in the signer_infos array + // SignerIndex index of signer in the signer_infos array. SignerIndex int } diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 8ae55a3891a5..99eeaf8a04e3 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -49,9 +49,11 @@ func TestVerifySignature(t *testing.T) { msgs := []sdk.Msg{testdata.NewTestMsg(addr)} fee := legacytx.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) signerData := signing.SignerData{ + Address: addr.String(), ChainID: chainId, AccountNumber: acc.GetAccountNumber(), Sequence: acc.GetSequence(), + SignerIndex: 0, } signBytes := legacytx.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo) signature, err := priv.Sign(signBytes) diff --git a/x/auth/testutil/suite.go b/x/auth/testutil/suite.go index 253ce3409c5b..b9bcc8a9c609 100644 --- a/x/auth/testutil/suite.go +++ b/x/auth/testutil/suite.go @@ -127,9 +127,11 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { // sign transaction signerData := signing.SignerData{ + Address: addr.String(), ChainID: "test", AccountNumber: 1, Sequence: seq1, + SignerIndex: 0, } signBytes, err := signModeHandler.GetSignBytes(signModeHandler.DefaultMode(), signerData, sigTx) s.Require().NoError(err) @@ -137,9 +139,11 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { s.Require().NoError(err) signerData = signing.SignerData{ + Address: msigAddr.String(), ChainID: "test", AccountNumber: 3, Sequence: mseq, + SignerIndex: 0, } mSignBytes, err := signModeHandler.GetSignBytes(signModeHandler.DefaultMode(), signerData, sigTx) s.Require().NoError(err) diff --git a/x/auth/tx/amino_aux.go b/x/auth/tx/amino_aux.go deleted file mode 100644 index aa388a42a556..000000000000 --- a/x/auth/tx/amino_aux.go +++ /dev/null @@ -1,57 +0,0 @@ -package tx - -import ( - "fmt" - - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" - - "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" - "github.com/cosmos/cosmos-sdk/x/auth/signing" -) - -var _ signing.SignModeHandler = signModeAminoAuxHandler{} - -// signModeAminoAuxHandler defines the SIGN_MODE_AMINO_AUX SignModeHandler -type signModeAminoAuxHandler struct{} - -// DefaultMode implements SignModeHandler.DefaultMode -func (signModeAminoAuxHandler) DefaultMode() signingtypes.SignMode { - return signingtypes.SignMode_SIGN_MODE_AMINO_AUX -} - -// Modes implements SignModeHandler.Modes -func (signModeAminoAuxHandler) Modes() []signingtypes.SignMode { - return []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_AMINO_AUX} -} - -// GetSignBytes implements SignModeHandler.GetSignBytes -func (signModeAminoAuxHandler) GetSignBytes( - mode signingtypes.SignMode, data signing.SignerData, tx sdk.Tx, -) ([]byte, error) { - - if mode != signingtypes.SignMode_SIGN_MODE_AMINO_AUX { - return nil, fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_AMINO_AUX, mode) - } - - protoTx, ok := tx.(*wrapper) - if !ok { - return nil, fmt.Errorf("can only handle a protobuf Tx, got %T", tx) - } - - if protoTx.txBodyHasUnknownNonCriticals { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, aminoNonCriticalFieldsError) - } - - body := protoTx.tx.Body - - if len(body.ExtensionOptions) != 0 || len(body.NonCriticalExtensionOptions) != 0 { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "SIGN_MODE_AMINO_AUX does not support protobuf extension options.") - } - - return legacytx.StdSignAuxBytes( - data.ChainID, data.AccountNumber, data.Sequence, protoTx.GetTimeoutHeight(), - protoTx.tx.AuthInfo.Tip.Amount, tx.GetMsgs(), protoTx.GetMemo(), - ), nil -} diff --git a/x/auth/tx/amino_aux_test.go b/x/auth/tx/amino_aux_test.go deleted file mode 100644 index 8f008ff84644..000000000000 --- a/x/auth/tx/amino_aux_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package tx - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/cosmos/cosmos-sdk/codec" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/cosmos/cosmos-sdk/testutil/testdata" - sdk "github.com/cosmos/cosmos-sdk/types" - txtypes "github.com/cosmos/cosmos-sdk/types/tx" - signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" - "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" - "github.com/cosmos/cosmos-sdk/x/auth/signing" -) - -func TestAminoAuxHandler(t *testing.T) { - privKey, pubkey, addr := testdata.KeyTestPubAddr() - interfaceRegistry := codectypes.NewInterfaceRegistry() - interfaceRegistry.RegisterImplementations((*sdk.Msg)(nil), &testdata.TestMsg{}) - marshaler := codec.NewProtoCodec(interfaceRegistry) - - txConfig := NewTxConfig(marshaler, []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_AMINO_AUX}) - txBuilder := txConfig.NewTxBuilder() - - accountNumber := uint64(1) - chainId := "test-chain" - memo := "sometestmemo" - msgs := []sdk.Msg{testdata.NewTestMsg(addr)} - accSeq := uint64(2) // Arbitrary account sequence - timeout := uint64(10) - fee := txtypes.Fee{Amount: sdk.NewCoins(sdk.NewInt64Coin("atom", 150)), GasLimit: 20000} - tip := sdk.NewCoins(sdk.NewCoin("regen", sdk.NewInt(1000))) - - err := txBuilder.SetMsgs(msgs...) - require.NoError(t, err) - txBuilder.SetMemo(memo) - txBuilder.SetFeeAmount(fee.Amount) - txBuilder.SetGasLimit(fee.GasLimit) - txBuilder.SetTimeoutHeight(timeout) - txBuilder.SetTip(&txtypes.Tip{ - Amount: tip, - Tipper: addr.String(), // Not needed when signing using AMINO_AUX, but putting here for clarity. - }) - - sigData := &signingtypes.SingleSignatureData{ - SignMode: signingtypes.SignMode_SIGN_MODE_AMINO_AUX, - } - - sig := signingtypes.SignatureV2{ - PubKey: pubkey, - Data: sigData, - Sequence: accSeq, - } - err = txBuilder.SetSignatures(sig) - require.NoError(t, err) - - signingData := signing.SignerData{ - ChainID: chainId, - AccountNumber: accountNumber, - Sequence: accSeq, - SignerIndex: 0, - } - - handler := signModeAminoAuxHandler{} - signBytes, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_AMINO_AUX, signingData, txBuilder.GetTx()) - require.NoError(t, err) - require.NotNil(t, signBytes) - - expectedSignBytes := legacytx.StdSignAuxBytes(chainId, accountNumber, accSeq, timeout, tip, msgs, memo) - require.NoError(t, err) - require.Equal(t, expectedSignBytes, signBytes) - - t.Log("verify that setting signature doesn't change sign bytes") - sigData.Signature, err = privKey.Sign(signBytes) - require.NoError(t, err) - err = txBuilder.SetSignatures(sig) - require.NoError(t, err) - signBytes, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_AMINO_AUX, signingData, txBuilder.GetTx()) - require.NoError(t, err) - require.Equal(t, expectedSignBytes, signBytes) - - // expect error with wrong sign mode - _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) - require.Error(t, err) - - // expect error with extension options - bldr := newBuilder() - buildTx(t, bldr) - any, err := codectypes.NewAnyWithValue(testdata.NewTestMsg()) - require.NoError(t, err) - bldr.tx.Body.ExtensionOptions = []*codectypes.Any{any} - tx := bldr.GetTx() - _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_AMINO_AUX, signingData, tx) - require.Error(t, err) - - // expect error with non-critical extension options - bldr = newBuilder() - buildTx(t, bldr) - bldr.tx.Body.NonCriticalExtensionOptions = []*codectypes.Any{any} - tx = bldr.GetTx() - _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_AMINO_AUX, signingData, tx) - require.Error(t, err) -} - -func TestAminoAuxHandler_DefaultMode(t *testing.T) { - handler := signModeAminoAuxHandler{} - require.Equal(t, signingtypes.SignMode_SIGN_MODE_AMINO_AUX, handler.DefaultMode()) -} - -func TestAminoAuxModeHandler_nonDIRECT_MODE(t *testing.T) { - invalidModes := []signingtypes.SignMode{ - signingtypes.SignMode_SIGN_MODE_DIRECT, - signingtypes.SignMode_SIGN_MODE_TEXTUAL, - signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - signingtypes.SignMode_SIGN_MODE_UNSPECIFIED, - signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, - } - for _, invalidMode := range invalidModes { - t.Run(invalidMode.String(), func(t *testing.T) { - var dh signModeAminoAuxHandler - var signingData signing.SignerData - _, err := dh.GetSignBytes(invalidMode, signingData, nil) - require.Error(t, err) - wantErr := fmt.Errorf("expected %s, got %s", signingtypes.SignMode_SIGN_MODE_AMINO_AUX, invalidMode) - require.Equal(t, err, wantErr) - }) - } -} - -func TestAminoAuxModeHandler_nonProtoTx(t *testing.T) { - var ah signModeAminoAuxHandler - var signingData signing.SignerData - tx := new(nonProtoTx) - _, err := ah.GetSignBytes(signingtypes.SignMode_SIGN_MODE_AMINO_AUX, signingData, tx) - require.Error(t, err) - wantErr := fmt.Errorf("can only handle a protobuf Tx, got %T", tx) - require.Equal(t, err, wantErr) -} diff --git a/x/auth/tx/direct_aux.go b/x/auth/tx/direct_aux.go index cf6685967901..c7324c18bb90 100644 --- a/x/auth/tx/direct_aux.go +++ b/x/auth/tx/direct_aux.go @@ -4,9 +4,9 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" types "github.com/cosmos/cosmos-sdk/types/tx" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" - "github.com/cosmos/cosmos-sdk/x/auth/signing" ) @@ -39,6 +39,11 @@ func (signModeDirectAuxHandler) GetSignBytes( return nil, fmt.Errorf("can only handle a protobuf Tx, got %T", tx) } + signerInfo := protoTx.tx.AuthInfo.SignerInfos[data.SignerIndex] + if signerInfo == nil || signerInfo.PublicKey == nil { + return nil, sdkerrors.ErrInvalidRequest.Wrapf("got empty pubkey for signer #%d in %s handler", data.SignerIndex, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX) + } + signDocDirectAux := types.SignDocDirectAux{ BodyBytes: protoTx.getBodyBytes(), ChainId: data.ChainID, diff --git a/x/auth/tx/direct_aux_test.go b/x/auth/tx/direct_aux_test.go index 5930dd520213..df7460c5ba78 100644 --- a/x/auth/tx/direct_aux_test.go +++ b/x/auth/tx/direct_aux_test.go @@ -53,8 +53,10 @@ func TestDirectAuxHandler(t *testing.T) { require.NoError(t, err) signingData := signing.SignerData{ + Address: addr.String(), ChainID: "test-chain", AccountNumber: 1, + SignerIndex: 0, } modeHandler := signModeDirectAuxHandler{} diff --git a/x/auth/tx/direct_test.go b/x/auth/tx/direct_test.go index bef1e81ed974..418ab56e11a7 100644 --- a/x/auth/tx/direct_test.go +++ b/x/auth/tx/direct_test.go @@ -69,8 +69,10 @@ func TestDirectModeHandler(t *testing.T) { require.Len(t, modeHandler.Modes(), 1) signingData := signing.SignerData{ + Address: addr.String(), ChainID: "test-chain", AccountNumber: 1, + SignerIndex: 0, } signBytes, err := modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, txBuilder.GetTx()) diff --git a/x/auth/tx/legacy_amino_json.go b/x/auth/tx/legacy_amino_json.go index ace31134e474..fea9d78a9bd3 100644 --- a/x/auth/tx/legacy_amino_json.go +++ b/x/auth/tx/legacy_amino_json.go @@ -43,7 +43,7 @@ func (s signModeLegacyAminoJSONHandler) GetSignBytes(mode signingtypes.SignMode, body := protoTx.tx.Body if len(body.ExtensionOptions) != 0 || len(body.NonCriticalExtensionOptions) != 0 { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "SIGN_MODE_LEGACY_AMINO_JSON does not support protobuf extension options.") + return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s does not support protobuf extension options", signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) } return legacytx.StdSignBytes( diff --git a/x/auth/tx/legacy_amino_json_test.go b/x/auth/tx/legacy_amino_json_test.go index 9a4f3ff70864..893d06f5ff2d 100644 --- a/x/auth/tx/legacy_amino_json_test.go +++ b/x/auth/tx/legacy_amino_json_test.go @@ -45,9 +45,11 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { handler := signModeLegacyAminoJSONHandler{} signingData := signing.SignerData{ + Address: addr1.String(), ChainID: chainId, AccountNumber: accNum, Sequence: seqNum, + SignerIndex: 0, } signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) diff --git a/x/auth/tx/mode_handler.go b/x/auth/tx/mode_handler.go index 5ebd0a6c547e..bb59fdc29a0e 100644 --- a/x/auth/tx/mode_handler.go +++ b/x/auth/tx/mode_handler.go @@ -11,10 +11,11 @@ import ( var DefaultSignModes = []signingtypes.SignMode{ signingtypes.SignMode_SIGN_MODE_DIRECT, signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, } // makeSignModeHandler returns the default protobuf SignModeHandler supporting -// SIGN_MODE_DIRECT and SIGN_MODE_LEGACY_AMINO_JSON. +// SIGN_MODE_DIRECT, SIGN_MODE_DIRECT_AUX and SIGN_MODE_LEGACY_AMINO_JSON. func makeSignModeHandler(modes []signingtypes.SignMode) signing.SignModeHandler { if len(modes) < 1 { panic(fmt.Errorf("no sign modes enabled")) @@ -30,8 +31,6 @@ func makeSignModeHandler(modes []signingtypes.SignMode) signing.SignModeHandler handlers[i] = signModeLegacyAminoJSONHandler{} case signingtypes.SignMode_SIGN_MODE_DIRECT_AUX: handlers[i] = signModeDirectAuxHandler{} - case signingtypes.SignMode_SIGN_MODE_AMINO_AUX: - handlers[i] = signModeAminoAuxHandler{} default: panic(fmt.Errorf("unsupported sign mode %+v", mode)) } diff --git a/x/authz/query.pb.go b/x/authz/query.pb.go index 681c9f40092c..92e7b4f8a3ba 100644 --- a/x/authz/query.pb.go +++ b/x/authz/query.pb.go @@ -156,7 +156,7 @@ func (m *QueryGrantsResponse) GetPagination() *query.PageResponse { return nil } -// QueryGranterGrantsRequest is the request type for the Query/Grants RPC method. +// QueryGranterGrantsRequest is the request type for the Query/GranterGrants RPC method. type QueryGranterGrantsRequest struct { Granter string `protobuf:"bytes,1,opt,name=granter,proto3" json:"granter,omitempty"` // pagination defines an pagination for the request. From 573d6b236e40dbc0c865cab32c9c643e1970cf56 Mon Sep 17 00:00:00 2001 From: Daniel Wedul <72031080+dwedul-figure@users.noreply.github.com> Date: Mon, 18 Oct 2021 09:32:24 -0600 Subject: [PATCH 2/3] feat: Add cosmovisor run command (#10316) * [10285]: Move the Run function into a new cmd/run.go and have main call the renamed/updated RunCosmovisorCommand function. * [10285]: Shorten the run warning to a single line and log it at the start and end of a run. * [10285]: Make the version.go funcs public. Allow for --version. Limit help request to just the first argument. * [10285]: Create a LogConfigOrError that logs the config or config errors in a standard way. * [10285]: Output config info/errors with the help command. * [10285]: Update the Run command to use the new config/error logging. * [10285]: Only provide the first arg to the cmd tester funcs, since that's all that really matters. * [10285]: Tweak a function comment. * [10285]: For the version command, run the binary with version too. * [10285]: Trim whitespace from the first argument before checking if it's any commands. * [10285]: Update and add some unit tests. * [10285]: In GetConfigFromEnv, make sure the cfg isn't null before trying to use it. * [10285]: Add some unit tests for LogConfigOrError. * [10285]: Allow for the poll interval to be defined as a duration. * [10285]: Add changelog line. * [10285]: Update the README to reflect the addition of the run action. * [10285]: slight tweak to changlog. * [10285]: Add a couple lines to the help text about getting help from the configured binary. * [10285]: Remove a println from a unit test. * [10285]: Put the help command first in the README. Co-authored-by: Robert Zaremba --- cosmovisor/CHANGELOG.md | 8 ++ cosmovisor/README.md | 38 +++---- cosmovisor/args.go | 39 ++++++-- cosmovisor/args_test.go | 98 +++++++++++++++++++ cosmovisor/cmd/cosmovisor/cmd/help.go | 34 ++++--- cosmovisor/cmd/cosmovisor/cmd/help_test.go | 72 +++++--------- cosmovisor/cmd/cosmovisor/cmd/root.go | 41 ++++++-- cosmovisor/cmd/cosmovisor/cmd/run.go | 40 ++++++++ cosmovisor/cmd/cosmovisor/cmd/run_test.go | 76 ++++++++++++++ cosmovisor/cmd/cosmovisor/cmd/version.go | 14 ++- cosmovisor/cmd/cosmovisor/cmd/version_test.go | 97 +++++++++++++----- cosmovisor/cmd/cosmovisor/main.go | 40 +------- 12 files changed, 430 insertions(+), 167 deletions(-) create mode 100644 cosmovisor/cmd/cosmovisor/cmd/run.go create mode 100644 cosmovisor/cmd/cosmovisor/cmd/run_test.go diff --git a/cosmovisor/CHANGELOG.md b/cosmovisor/CHANGELOG.md index a5b298b1b904..91d0e9ffda8f 100644 --- a/cosmovisor/CHANGELOG.md +++ b/cosmovisor/CHANGELOG.md @@ -36,6 +36,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Features + ++ [\#10285](https://github.com/cosmos/cosmos-sdk/pull/10316) Added `run` action. + +### Deprecated + ++ [\#10285](https://github.com/cosmos/cosmos-sdk/pull/10316) Running `cosmovisor` without the `run` argument. + ## v1.0.0 2021-09-30 ### Features diff --git a/cosmovisor/README.md b/cosmovisor/README.md index afb7f0fe2ef8..b6e3b3e8eff1 100644 --- a/cosmovisor/README.md +++ b/cosmovisor/README.md @@ -4,9 +4,9 @@ #### Design -Cosmovisor is designed to be used as a wrapper for an `Cosmos SDK` app: -* it will pass all arguments to the associated app (configured by `DAEMON_NAME` env variable). - Running `cosmovisor arg1 arg2 ....` will run `app arg1 arg2 ...`; +Cosmovisor is designed to be used as a wrapper for a `Cosmos SDK` app: +* it will pass arguments to the associated app (configured by `DAEMON_NAME` env variable). + Running `cosmovisor run arg1 arg2 ....` will run `app arg1 arg2 ...`; * it will manage an app by restarting and upgrading if needed; * it is configured using environment variables, not positional arguments. @@ -34,7 +34,14 @@ go install github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor@latest ### Command Line Arguments And Environment Variables -All arguments passed to `cosmovisor` will be passed to the application binary (as a subprocess). `cosmovisor` will return `/dev/stdout` and `/dev/stderr` of the subprocess as its own. For this reason, `cosmovisor` cannot accept any command-line arguments other than those available to the application binary, nor will it print anything to output other than what is printed by the application binary. +The first argument passed to `cosmovisor` is the action for `cosmovisor` to take. Options are: +* `help`, `--help`, or `-h` - Output `cosmovisor` help information and check your `cosmovisor` configuration. +* `run` - Run the configured binary using the rest of the provided arguments. +* `version`, or `--version` - Output the `cosmovisor` version and also run the binary with the `version` argument. + +All arguments passed to `cosmovisor run` will be passed to the application binary (as a subprocess). `cosmovisor` will return `/dev/stdout` and `/dev/stderr` of the subprocess as its own. For this reason, `cosmovisor run` cannot accept any command-line arguments other than those available to the application binary. + +*Note: Use of `cosmovisor` without one of the action arguments is deprecated. For backwards compatability, if the first argument is not an action argument, `run` is assumed. However, this fallback might be removed in future versions, so it is recommended that you always provide `run`. `cosmovisor` reads its configuration from environment variables: @@ -42,11 +49,10 @@ All arguments passed to `cosmovisor` will be passed to the application binary (a * `DAEMON_NAME` is the name of the binary itself (e.g. `gaiad`, `regend`, `simd`, etc.). * `DAEMON_ALLOW_DOWNLOAD_BINARIES` (*optional*), if set to `true`, will enable auto-downloading of new binaries (for security reasons, this is intended for full nodes rather than validators). By default, `cosmovisor` will not auto-download new binaries. * `DAEMON_RESTART_AFTER_UPGRADE` (*optional*, default = `true`), if `true`, restarts the subprocess with the same command-line arguments and flags (but with the new binary) after a successful upgrade. Otherwise (`false`), `cosmovisor` stops running after an upgrade and requires the system administrator to manually restart it. Note restart is only after the upgrade and does not auto-restart the subprocess after an error occurs. -* `DAEMON_POLL_INTERVAL` is the interval length in milliseconds for polling the upgrade plan file. Default: 300. -* `UNSAFE_SKIP_BACKUP` (defaults to `false`), if set to `false`, backs up the data before trying the upgrade. Otherwise (`true`), upgrades directly without performing a backup. The default value of false is useful and recommended in case of failures and when a backup needed to rollback. We recommend using the default backup option `UNSAFE_SKIP_BACKUP=false`. +* `DAEMON_POLL_INTERVAL` is the interval length for polling the upgrade plan file. The value can either be a number (in milliseconds) or a duration (e.g. `1s`). Default: 300 milliseconds. +* `UNSAFE_SKIP_BACKUP` (defaults to `false`), if set to `true`, upgrades directly without performing a backup. Otherwise (`false`, default) backs up the data before trying the upgrade. The default value of false is useful and recommended in case of failures and when a backup needed to rollback. We recommend using the default backup option `UNSAFE_SKIP_BACKUP=false`. * `DAEMON_PREUPGRADE_MAX_RETRIES` (defaults to `0`). The maximum number of times to call `pre-upgrade` in the application after exit status of `31`. After the maximum number of retries, cosmovisor fails the upgrade. - ### Folder Layout `$DAEMON_HOME/cosmovisor` is expected to belong completely to `cosmovisor` and the subprocesses that are controlled by it. The folder content is organized as follows: @@ -91,22 +97,6 @@ In order to support downloadable binaries, a tarball for each upgrade binary wil The `DAEMON` specific code and operations (e.g. tendermint config, the application db, syncing blocks, etc.) all work as expected. The application binaries' directives such as command-line flags and environment variables also work as expected. -### Commands - -Because Cosmovisor is meant to be used as a wrapper for a Cosmos SDK application, it does not require many commands. - -To determine the version of Cosmovisor, run the following command: -``` -cosmovisor version -``` -The output of the `cosmovisor version` command shows the version of the Cosmos SDK application and the version of Cosmovisor: - -``` -Cosmovisor Version: v0.1.0-85-g65baacac0 -0.43.0-beta1-319-ge3aec1840 -``` - - ### Detecting Upgrades `cosmovisor` is polling the `$DAEMON_HOME/data/upgrade-info.json` file for new upgrade instructions. The file is created by the x/upgrade module in `BeginBlocker` when an upgrade is detected and the blockchain reaches the upgrade height. @@ -278,7 +268,7 @@ cp ./build/simd $DAEMON_HOME/cosmovisor/upgrades/test1/bin Start `cosmosvisor`: ``` -cosmovisor start +cosmovisor run start ``` Open a new terminal window and submit an upgrade proposal along with a deposit and a vote (these commands must be run within 20 seconds of each other): diff --git a/cosmovisor/args.go b/cosmovisor/args.go index c0c75996c3f4..0fbd2bda9ae9 100644 --- a/cosmovisor/args.go +++ b/cosmovisor/args.go @@ -11,6 +11,8 @@ import ( "strings" "time" + "github.com/rs/zerolog" + cverrors "github.com/cosmos/cosmos-sdk/cosmovisor/errors" ) @@ -143,13 +145,18 @@ func GetConfigFromEnv() (*Config, error) { interval := os.Getenv(EnvInterval) if interval != "" { - switch i, e := strconv.ParseUint(interval, 10, 32); { - case e != nil: - errs = append(errs, fmt.Errorf("invalid %s: %w", EnvInterval, err)) - case i == 0: - errs = append(errs, fmt.Errorf("invalid %s: cannot be 0", EnvInterval)) - default: - cfg.PollInterval = time.Millisecond * time.Duration(i) + var intervalUInt uint64 + intervalUInt, err = strconv.ParseUint(interval, 10, 32) + if err == nil { + cfg.PollInterval = time.Millisecond * time.Duration(intervalUInt) + } else { + cfg.PollInterval, err = time.ParseDuration(interval) + } + switch { + case err != nil: + errs = append(errs, fmt.Errorf("invalid %s: could not parse \"%s\" into either a duration or uint (milliseconds)", EnvInterval, interval)) + case cfg.PollInterval <= 0: + errs = append(errs, fmt.Errorf("invalid %s: must be greater than 0", EnvInterval)) } } else { cfg.PollInterval = 300 * time.Millisecond @@ -168,6 +175,24 @@ func GetConfigFromEnv() (*Config, error) { return cfg, nil } +// LogConfigOrError logs either the config details or the error. +func LogConfigOrError(logger zerolog.Logger, cfg *Config, cerr error) { + switch { + case cerr != nil: + switch err := cerr.(type) { + case *cverrors.MultiError: + logger.Error().Msg("multiple configuration errors found:") + for i, e := range err.GetErrors() { + logger.Error().Err(e).Msg(fmt.Sprintf(" %d:", i+1)) + } + default: + logger.Error().Err(cerr).Msg("configuration error:") + } + case cfg != nil: + logger.Info().Msg("Configuration is valid:\n" + cfg.DetailString()) + } +} + // validate returns an error if this config is invalid. // it enforces Home/cosmovisor is a valid directory and exists, // and that Name is set diff --git a/cosmovisor/args_test.go b/cosmovisor/args_test.go index 564c7477db74..86c0d0df1c28 100644 --- a/cosmovisor/args_test.go +++ b/cosmovisor/args_test.go @@ -1,12 +1,15 @@ package cosmovisor import ( + "bytes" "fmt" + "io" "os" "path/filepath" "testing" "time" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -451,6 +454,18 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { expectedCfg: newConfig(absPath, "testname", false, false, false, 987, 1), expectedErrCount: 0, }, + { + name: "poll interval 1s", + envVals: cosmovisorEnv{absPath, "testname", "false", "false", "false", "1s", "1"}, + expectedCfg: newConfig(absPath, "testname", false, false, false, 1000, 1), + expectedErrCount: 0, + }, + { + name: "poll interval -3m", + envVals: cosmovisorEnv{absPath, "testname", "false", "false", "false", "-3m", "1"}, + expectedCfg: nil, + expectedErrCount: 1, + }, // EnvHome, EnvName, EnvDownloadBin, EnvRestartUpgrade, EnvSkipBackup, EnvInterval, EnvPreupgradeMaxRetries { name: "prepupgrade max retries bad", @@ -497,3 +512,86 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { }) } } + +func (s *argsTestSuite) TestLogConfigOrError() { + cfg := &Config{ + Home: "/no/place/like/it", + Name: "cosmotestvisor", + AllowDownloadBinaries: true, + RestartAfterUpgrade: true, + PollInterval: 999, + UnsafeSkipBackup: false, + PreupgradeMaxRetries: 20, + } + errNormal := fmt.Errorf("this is a single error") + errs := []error{ + fmt.Errorf("multi-error error 1"), + fmt.Errorf("multi-error error 2"), + fmt.Errorf("multi-error error 3"), + } + errMulti := errors.FlattenErrors(errs...) + + makeTestLogger := func(testName string, out io.Writer) zerolog.Logger { + output := zerolog.ConsoleWriter{Out: out, TimeFormat: time.Kitchen, NoColor: true} + return zerolog.New(output).With().Str("test", testName).Timestamp().Logger() + } + + tests := []struct { + name string + cfg *Config + err error + contains []string + notcontains []string + }{ + { + name: "normal error", + cfg: nil, + err: errNormal, + contains: []string{"configuration error", errNormal.Error()}, // TODO: Fix this. + notcontains: nil, + }, + { + name: "multi error", + cfg: nil, + err: errMulti, + contains: []string{"multiple configuration errors found", errs[0].Error(), errs[1].Error(), errs[2].Error()}, + notcontains: nil, + }, + { + name: "config", + cfg: cfg, + err: nil, + contains: []string{"Configuration is valid", cfg.DetailString()}, + notcontains: nil, + }, + { + name: "error and config - no config details", + cfg: cfg, + err: errNormal, + contains: []string{"error"}, + notcontains: []string{"Configuration is valid", EnvName, cfg.Home}, // Just some spot checks. + }, + { + name: "nil nil - no output", + cfg: nil, + err: nil, + contains: nil, + notcontains: []string{" "}, + }, + } + + for _, tc := range tests { + s.T().Run(tc.name, func(t *testing.T) { + var b bytes.Buffer + logger := makeTestLogger(tc.name, &b) + LogConfigOrError(logger, tc.cfg, tc.err) + output := b.String() + for _, expected := range tc.contains { + assert.Contains(t, output, expected) + } + for _, unexpected := range tc.notcontains { + assert.NotContains(t, output, unexpected) + } + }) + } +} diff --git a/cosmovisor/cmd/cosmovisor/cmd/help.go b/cosmovisor/cmd/cosmovisor/cmd/help.go index 460c21d7e245..abb757dd91cc 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/help.go +++ b/cosmovisor/cmd/cosmovisor/cmd/help.go @@ -3,33 +3,34 @@ package cmd import ( "fmt" "os" - "strings" + "time" + + "github.com/rs/zerolog" "github.com/cosmos/cosmos-sdk/cosmovisor" ) +// HelpArgs are the strings that indicate a cosmovisor help command. +var HelpArgs = []string{"help", "--help", "-h"} + // ShouldGiveHelp checks the env and provided args to see if help is needed or being requested. // Help is needed if either cosmovisor.EnvName and/or cosmovisor.EnvHome env vars aren't set. -// Help is requested if any args are "help", "--help", or "-h". -func ShouldGiveHelp(args []string) bool { - if len(os.Getenv(cosmovisor.EnvName)) == 0 || len(os.Getenv(cosmovisor.EnvHome)) == 0 { - return true - } - if len(args) == 0 { - return false - } - for _, arg := range args { - if strings.EqualFold(arg, "help") || strings.EqualFold(arg, "--help") || strings.EqualFold(arg, "-h") { - return true - } - } - return false +// Help is requested if the first arg is "help", "--help", or "-h". +func ShouldGiveHelp(arg string) bool { + return isOneOf(arg, HelpArgs) || len(os.Getenv(cosmovisor.EnvName)) == 0 || len(os.Getenv(cosmovisor.EnvHome)) == 0 } // DoHelp outputs help text func DoHelp() { // Not using the logger for this output because the header and footer look weird for help text. fmt.Println(GetHelpText()) + // Check the config and output details or any errors. + // Not using the cosmovisor.Logger in order to ignore any level it might have set, + // and also to not have any of the extra parameters in the output. + output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.Kitchen} + logger := zerolog.New(output).With().Timestamp().Logger() + cfg, err := cosmovisor.GetConfigFromEnv() + cosmovisor.LogConfigOrError(logger, cfg, err) } // GetHelpText creates the help text multi-line string. @@ -45,5 +46,8 @@ and restart the App. Configuration of Cosmovisor is done through environment variables, which are documented in: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor/README.md + +To get help for the configured binary: + cosmovisor run help `, cosmovisor.EnvName, cosmovisor.EnvHome) } diff --git a/cosmovisor/cmd/cosmovisor/cmd/help_test.go b/cosmovisor/cmd/cosmovisor/cmd/help_test.go index 91a152e4d9d8..636695701919 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/help_test.go +++ b/cosmovisor/cmd/cosmovisor/cmd/help_test.go @@ -170,13 +170,13 @@ func (s *HelpTestSuite) TestShouldGiveHelpEnvVars() { s.T().Run(tc.name, func(t *testing.T) { prepEnv(t, cosmovisor.EnvHome, tc.envHome) prepEnv(t, cosmovisor.EnvName, tc.envName) - actual := ShouldGiveHelp(nil) + actual := ShouldGiveHelp("not-a-help-arg") assert.Equal(t, tc.expected, actual) }) } } -func (s HelpTestSuite) TestShouldGiveHelpArgs() { +func (s HelpTestSuite) TestShouldGiveHelpArg() { initialEnv := s.clearEnv() defer s.setEnv(nil, initialEnv) @@ -184,79 +184,59 @@ func (s HelpTestSuite) TestShouldGiveHelpArgs() { tests := []struct { name string - args []string + arg string expected bool }{ { - name: "nil args", - args: nil, + name: "empty string", + arg: "", expected: false, }, { - name: "empty args", - args: []string{}, + name: "random", + arg: "random", expected: false, }, { - name: "one arg random", - args: []string{"random"}, - expected: false, - }, - { - name: "five args random", - args: []string{"random1", "--random2", "-r", "random4", "-random5"}, - expected: false, - }, - { - name: "one arg help", - args: []string{"help"}, + name: "help", + arg: "help", expected: true, }, { - name: " two args help first", - args: []string{"help", "arg2"}, + name: "-h", + arg: "-h", expected: true, }, { - name: "two args help second", - args: []string{"arg1", "help"}, + name: "--help", + arg: "--help", expected: true, }, { - name: "one arg -h", - args: []string{"-h"}, + name: "help weird casing", + arg: "hELP", expected: true, }, { - name: "two args -h first", - args: []string{"-h", "arg2"}, - expected: true, - }, - { - name: "two args -h second", - args: []string{"arg1", "-h"}, - expected: true, - }, - { - name: "one arg --help", - args: []string{"--help"}, - expected: true, + name: "version", + arg: "version", + expected: false, }, { - name: "two args --help first", - args: []string{"--help", "arg2"}, - expected: true, + name: "--version", + arg: "--version", + expected: false, }, { - name: "two args --help second", - args: []string{"arg1", "--help"}, - expected: true, + name: "run", + arg: "run", + expected: false, }, } for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - actual := ShouldGiveHelp(tc.args) + s.T().Run(fmt.Sprintf("%s - %t", tc.name, tc.expected), func(t *testing.T) { + actual := ShouldGiveHelp(tc.arg) assert.Equal(t, tc.expected, actual) }) } diff --git a/cosmovisor/cmd/cosmovisor/cmd/root.go b/cosmovisor/cmd/cosmovisor/cmd/root.go index 3bea433f2005..a4d3f2b2f150 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/root.go +++ b/cosmovisor/cmd/cosmovisor/cmd/root.go @@ -1,12 +1,41 @@ package cmd -// RunCosmovisorCommands executes cosmosvisor commands e.g `cosmovisor version` -// Returned boolean is whether or not execution should continue. -func RunCosmovisorCommands(args []string) { +import ( + "strings" + + "github.com/cosmos/cosmos-sdk/cosmovisor" +) + +// RunCosmovisorCommand executes the desired cosmovisor command. +func RunCosmovisorCommand(args []string) error { + arg0 := "" + if len(args) > 0 { + arg0 = strings.TrimSpace(args[0]) + } switch { - case ShouldGiveHelp(args): + case ShouldGiveHelp(arg0): DoHelp() - case isVersionCommand(args): - printVersion() + return nil + case IsVersionCommand(arg0): + PrintVersion() + return Run([]string{"version"}) + case IsRunCommand(arg0): + return Run(args[1:]) + } + warnRun := func() { + cosmovisor.Logger.Warn().Msg("Use of cosmovisor without the 'run' command is deprecated. Use: cosmovisor run [args]") + } + warnRun() + defer warnRun() + return Run(args) +} + +// isOneOf returns true if the given arg equals one of the provided options (ignoring case). +func isOneOf(arg string, options []string) bool { + for _, opt := range options { + if strings.EqualFold(arg, opt) { + return true + } } + return false } diff --git a/cosmovisor/cmd/cosmovisor/cmd/run.go b/cosmovisor/cmd/cosmovisor/cmd/run.go new file mode 100644 index 000000000000..428aa5391798 --- /dev/null +++ b/cosmovisor/cmd/cosmovisor/cmd/run.go @@ -0,0 +1,40 @@ +package cmd + +import ( + "os" + + "github.com/cosmos/cosmos-sdk/cosmovisor" +) + +// RunArgs are the strings that indicate a cosmovisor run command. +var RunArgs = []string{"run"} + +// IsRunCommand checks if the given args indicate that a run is desired. +func IsRunCommand(arg string) bool { + return isOneOf(arg, RunArgs) +} + +// Run runs the configured program with the given args and monitors it for upgrades. +func Run(args []string) error { + cfg, cerr := cosmovisor.GetConfigFromEnv() + cosmovisor.LogConfigOrError(cosmovisor.Logger, cfg, cerr) + if cerr != nil { + return cerr + } + launcher, err := cosmovisor.NewLauncher(cfg) + if err != nil { + return err + } + + doUpgrade, err := launcher.Run(args, os.Stdout, os.Stderr) + // if RestartAfterUpgrade, we launch after a successful upgrade (only condition LaunchProcess returns nil) + for cfg.RestartAfterUpgrade && err == nil && doUpgrade { + cosmovisor.Logger.Info().Str("app", cfg.Name).Msg("upgrade detected, relaunching") + doUpgrade, err = launcher.Run(args, os.Stdout, os.Stderr) + } + if doUpgrade && err == nil { + cosmovisor.Logger.Info().Msg("upgrade detected, DAEMON_RESTART_AFTER_UPGRADE is off. Verify new upgrade and start cosmovisor again.") + } + + return err +} diff --git a/cosmovisor/cmd/cosmovisor/cmd/run_test.go b/cosmovisor/cmd/cosmovisor/cmd/run_test.go new file mode 100644 index 000000000000..b54b38fd8fcd --- /dev/null +++ b/cosmovisor/cmd/cosmovisor/cmd/run_test.go @@ -0,0 +1,76 @@ +package cmd + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIsRunCommand(t *testing.T) { + cases := []struct { + name string + arg string + expected bool + }{ + { + name: "empty string", + arg: "", + expected: false, + }, + { + name: "random", + arg: "random", + expected: false, + }, + { + name: "run", + arg: "run", + expected: true, + }, + { + name: "run weird casing", + arg: "RUn", + expected: true, + }, + { + name: "--run", + arg: "--run", + expected: false, + }, + { + name: "help", + arg: "help", + expected: false, + }, + { + name: "-h", + arg: "-h", + expected: false, + }, + { + name: "--help", + arg: "--help", + expected: false, + }, + { + name: "version", + arg: "version", + expected: false, + }, + { + name: "--version", + arg: "--version", + expected: false, + }, + } + + for _, tc := range cases { + t.Run(fmt.Sprintf("%s - %t", tc.name, tc.expected), func(t *testing.T) { + actual := IsRunCommand(tc.arg) + require.Equal(t, tc.expected, actual) + }) + } +} + +// TODO: Write tests for func Run(args []string) error diff --git a/cosmovisor/cmd/cosmovisor/cmd/version.go b/cosmovisor/cmd/cosmovisor/cmd/version.go index ad60a006f161..11c6ee75e5cd 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/version.go +++ b/cosmovisor/cmd/cosmovisor/cmd/version.go @@ -2,14 +2,20 @@ package cmd import ( "fmt" - "strings" ) // Version represents Cosmovisor version value. Set during build var Version string -func isVersionCommand(args []string) bool { - return len(args) == 1 && strings.EqualFold(args[0], "version") +// VersionArgs is the strings that indicate a cosmovisor version command. +var VersionArgs = []string{"version", "--version"} + +// IsVersionCommand checks if the given args indicate that the version is being requested. +func IsVersionCommand(arg string) bool { + return isOneOf(arg, VersionArgs) } -func printVersion() { fmt.Println("Cosmovisor Version: ", Version) } +// PrintVersion prints the cosmovisor version. +func PrintVersion() { + fmt.Println("Cosmovisor Version: ", Version) +} diff --git a/cosmovisor/cmd/cosmovisor/cmd/version_test.go b/cosmovisor/cmd/cosmovisor/cmd/version_test.go index 2804b3a3ba42..6c842494e85f 100644 --- a/cosmovisor/cmd/cosmovisor/cmd/version_test.go +++ b/cosmovisor/cmd/cosmovisor/cmd/version_test.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -8,33 +9,77 @@ import ( func TestIsVersionCommand(t *testing.T) { cases := []struct { - name string - args []string - expectRes bool - }{{ - name: "valid args - lowercase", - args: []string{"version"}, - expectRes: true, - }, { - name: "typo", - args: []string{"vrsion"}, - expectRes: false, - }, { - name: "non version command", - args: []string{"start"}, - expectRes: false, - }, { - name: "incorrect format", - args: []string{"start", "version"}, - expectRes: false, - }} + name string + arg string + expected bool + }{ + { + name: "empty string", + arg: "", + expected: false, + }, + { + name: "random", + arg: "random", + expected: false, + }, + { + name: "version", + arg: "version", + expected: true, + }, + { + name: "--version", + arg: "--version", + expected: true, + }, + { + name: "version weird casing", + arg: "veRSiOn", + expected: true, + }, + { + // -v should be reserved for verbose, and should not be used for --version. + name: "-v", + arg: "-v", + expected: false, + }, + { + name: "typo", + arg: "vrsion", + expected: false, + }, + { + name: "non version command", + arg: "start", + expected: false, + }, + { + name: "help", + arg: "help", + expected: false, + }, + { + name: "-h", + arg: "-h", + expected: false, + }, + { + name: "--help", + arg: "--help", + expected: false, + }, + { + name: "run", + arg: "run", + expected: false, + }, + } - for i := range cases { - tc := cases[i] - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - res := isVersionCommand(tc.args) - require.Equal(tc.expectRes, res) + for _, tc := range cases { + t.Run(fmt.Sprintf("%s - %t", tc.name, tc.expected), func(t *testing.T) { + actual := IsVersionCommand(tc.arg) + require.Equal(t, tc.expected, actual) }) } } diff --git a/cosmovisor/cmd/cosmovisor/main.go b/cosmovisor/cmd/cosmovisor/main.go index 07de6d012336..4d82805be2a2 100644 --- a/cosmovisor/cmd/cosmovisor/main.go +++ b/cosmovisor/cmd/cosmovisor/main.go @@ -1,54 +1,16 @@ package main import ( - "fmt" "os" "github.com/cosmos/cosmos-sdk/cosmovisor" "github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor/cmd" - "github.com/cosmos/cosmos-sdk/cosmovisor/errors" ) func main() { cosmovisor.SetupLogging() - if err := Run(os.Args[1:]); err != nil { + if err := cmd.RunCosmovisorCommand(os.Args[1:]); err != nil { cosmovisor.Logger.Error().Err(err).Msg("") os.Exit(1) } } - -// Run is the main loop, but returns an error -func Run(args []string) error { - cmd.RunCosmovisorCommands(args) - - cfg, cerr := cosmovisor.GetConfigFromEnv() - if cerr != nil { - switch err := cerr.(type) { - case *errors.MultiError: - cosmovisor.Logger.Error().Msg("multiple configuration errors found:") - for i, e := range err.GetErrors() { - cosmovisor.Logger.Error().Err(e).Msg(fmt.Sprintf(" %d:", i+1)) - } - default: - cosmovisor.Logger.Error().Err(err).Msg("configuration error:") - } - return cerr - } - cosmovisor.Logger.Info().Msg("Configuration is valid:\n" + cfg.DetailString()) - launcher, err := cosmovisor.NewLauncher(cfg) - if err != nil { - return err - } - - doUpgrade, err := launcher.Run(args, os.Stdout, os.Stderr) - // if RestartAfterUpgrade, we launch after a successful upgrade (only condition LaunchProcess returns nil) - for cfg.RestartAfterUpgrade && err == nil && doUpgrade { - cosmovisor.Logger.Info().Str("app", cfg.Name).Msg("upgrade detected, relaunching") - doUpgrade, err = launcher.Run(args, os.Stdout, os.Stderr) - } - if doUpgrade && err == nil { - cosmovisor.Logger.Info().Msg("upgrade detected, DAEMON_RESTART_AFTER_UPGRADE is off. Verify new upgrade and start cosmovisor again.") - } - - return err -} From 678986251a4fd9f9f0360c917a10274e04f8c375 Mon Sep 17 00:00:00 2001 From: yys Date: Tue, 19 Oct 2021 18:30:51 +0900 Subject: [PATCH 3/3] fix: rosetta `getHeight` function to use tmRPC.GenesisChunked() instead tmRPC.Genesis() (#10340) ## Description When we enable rosetta from the chain with non-zero initial height & huge genesis, it always return ``` Error: rosetta: (502) bad gateway ``` This is due to huge genesis load rejection from Tendermint. In current implementation, rosetta server requests genesis to get initial height ``` c.tmRPC.Genesis(ctx) ``` but, this will be failed with below message when the genesis is huge ``` { "jsonrpc": "2.0", "id": -1, "error": { "code": -32603, "message": "Internal error", "data": "genesis response is large, please use the genesis_chunked API instead" } } ``` To fix this, we can use following lines ``` status, err := c.tmRPC. GenesisChunked(ctx) ``` --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 3 ++- server/rosetta/client_online.go | 33 ++++++++++++++++++++++++++-- server/rosetta/client_online_test.go | 15 +++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 server/rosetta/client_online_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f059ebd001a..473540dfce4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,7 +135,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -* (client) [#10226](https://github.com/cosmos/cosmos-sdk/pull/10226) Fix --home flag parsing. +* (rosetta) [\#10340](https://github.com/cosmos/cosmos-sdk/pull/10340) Use `GenesisChunked(ctx)` instead `Genesis(ctx)` to get genesis block height +* (client) [#10226](https://github.com/cosmos/cosmos-sdk/pull/10226) Fix --home flag parsing. * [#10180](https://github.com/cosmos/cosmos-sdk/issues/10180) Documentation: make references to Cosmos SDK consistent * (x/genutil) [#10104](https://github.com/cosmos/cosmos-sdk/pull/10104) Ensure the `init` command reads the `--home` flag value correctly. * [\#9651](https://github.com/cosmos/cosmos-sdk/pull/9651) Change inconsistent limit of `0` to `MaxUint64` on InfiniteGasMeter and add GasRemaining func to GasMeter. diff --git a/server/rosetta/client_online.go b/server/rosetta/client_online.go index 0a34b8b1a878..177c01131810 100644 --- a/server/rosetta/client_online.go +++ b/server/rosetta/client_online.go @@ -3,8 +3,11 @@ package rosetta import ( "bytes" "context" + "encoding/base64" "encoding/hex" + "errors" "fmt" + "regexp" "strconv" "time" @@ -481,13 +484,39 @@ func (c *Client) blockTxs(ctx context.Context, height *int64) (crgtypes.BlockTra func (c *Client) getHeight(ctx context.Context, height *int64) (realHeight *int64, err error) { if height != nil && *height == -1 { - genesis, err := c.tmRPC.Genesis(ctx) + genesisChunk, err := c.tmRPC.GenesisChunked(ctx, 0) if err != nil { return nil, err } - realHeight = &(genesis.Genesis.InitialHeight) + + heightNum, err := extractInitialHeightFromGenesisChunk(genesisChunk.Data) + if err != nil { + return nil, err + } + + realHeight = &heightNum } else { realHeight = height } return } + +func extractInitialHeightFromGenesisChunk(genesisChunk string) (int64, error) { + firstChunk, err := base64.StdEncoding.DecodeString(genesisChunk) + if err != nil { + return 0, err + } + + re, err := regexp.Compile("\"initial_height\":\"(\\d+)\"") + if err != nil { + return 0, err + } + + matches := re.FindStringSubmatch(string(firstChunk)) + if len(matches) != 2 { + return 0, errors.New("failed to fetch initial_height") + } + + heightStr := matches[1] + return strconv.ParseInt(heightStr, 10, 64) +} diff --git a/server/rosetta/client_online_test.go b/server/rosetta/client_online_test.go new file mode 100644 index 000000000000..9aa8965cf6e9 --- /dev/null +++ b/server/rosetta/client_online_test.go @@ -0,0 +1,15 @@ +package rosetta + +import ( + "encoding/base64" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRegex(t *testing.T) { + genesisChuck := base64.StdEncoding.EncodeToString([]byte(`"genesis_time":"2021-09-28T09:00:00Z","chain_id":"bombay-12","initial_height":"5900001","consensus_params":{"block":{"max_bytes":"5000000","max_gas":"1000000000","time_iota_ms":"1000"},"evidence":{"max_age_num_blocks":"100000","max_age_duration":"172800000000000","max_bytes":"50000"},"validator":{"pub_key_types":["ed25519"]},"version":{}},"validators":[{"address":"EEA4891F5F8D523A6B4B3EAC84B5C08655A00409","pub_key":{"type":"tendermint/PubKeyEd25519","value":"UX71gTBNumQq42qRd6j/K8XN/y3/HAcuAJxj97utawI="},"power":"60612","name":"BTC.Secure"},{"address":"973F589DE1CC8A54ABE2ABE0E0A4ABF13A9EBAE4","pub_key":{"type":"tendermint/PubKeyEd25519","value":"AmGQvQSAAXzSIscx/6o4rVdRMT9QvairQHaCXsWhY+c="},"power":"835","name":"MoonletWallet"},{"address":"831F402BDA0C9A3F260D4F221780BC22A4C3FB23","pub_key":{"type":"tendermint/PubKeyEd25519","value":"Tw8yKbPNEo113ZNbJJ8joeXokoMdBoazRTwb1NQ77WA="},"power":"102842","name":"BlockNgine"},{"address":"F2683F267D2B4C8714B44D68612DB37A8DD2EED7","pub_key":{"type":"tendermint/PubKeyEd25519","value":"PVE4IcWDE6QEqJSEkx55IDkg5zxBo8tVRzKFMJXYFSQ="},"power":"23200","name":"Luna Station 88"},{"address":"9D2428CBAC68C654BE11BE405344C560E6A0F626","pub_key":{"type":"tendermint/PubKeyEd25519","value":"93hzGmZjPRqOnQkb8BULjqanW3M2p1qIcLVTGkf1Zhk="},"power":"35420","name":"Terra-India"},{"address":"DC9897F22E74BF1B66E2640FA461F785F9BA7627","pub_key":{"type":"tendermint/PubKeyEd25519","value":"mlYb/Dzqwh0YJjfH59OZ4vtp+Zhdq5Oj5MNaGHq1X0E="},"power":"25163","name":"SolidStake"},{"address":"AA1A027E270A2BD7AF154999E6DE9D39C5711DE7","pub_key":{"type":"tendermint/PubKeyEd25519","value":"28z8FlpbC7sR0f1Q8OWFASDNi0FAmdldzetwQ07JJzg="},"power":"34529","name":"syncnode"},{"address":"E548735750DC5015ADDE3B0E7A1294C3B868680B","pub_key":{"type":"tendermint/PubKeyEd25519","value":"BTDtLSKp4wpQrWBwmGvp9isWC5jXaAtX1nrJtsCEWew="},"power":"36082","name":"OneStar"}`)) + height, err := extractInitialHeightFromGenesisChunk(genesisChuck) + require.NoError(t, err) + require.Equal(t, height, int64(5900001)) +}