From 0dbf7dce0692bd5478053836b2301a8da8749000 Mon Sep 17 00:00:00 2001 From: Marie Gauthier Date: Thu, 14 Apr 2022 15:46:23 +0200 Subject: [PATCH] chore: Tx Tips API audit (#11641) ## Description ref: https://github.com/cosmos/cosmos-sdk/issues/11087 --- ### 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... - [x] 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 - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] 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) --- api/cosmos/tx/signing/v1beta1/signing.pulsar.go | 8 +++++++- api/cosmos/tx/v1beta1/tx.pulsar.go | 4 ++-- client/tx/aux_builder.go | 4 ++-- docs/core/tips.md | 6 +++--- proto/cosmos/tx/v1beta1/tx.proto | 4 ++-- types/tx/signing/signing.pb.go | 8 +++++++- types/tx/tx.pb.go | 4 ++-- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/api/cosmos/tx/signing/v1beta1/signing.pulsar.go b/api/cosmos/tx/signing/v1beta1/signing.pulsar.go index 2f4ee5d21..95eced91d 100644 --- a/api/cosmos/tx/signing/v1beta1/signing.pulsar.go +++ b/api/cosmos/tx/signing/v1beta1/signing.pulsar.go @@ -2738,7 +2738,13 @@ const ( // SIGN_MODE_EIP_191 specifies the sign mode for EIP 191 signing on the Cosmos // SDK. Ref: https://eips.ethereum.org/EIPS/eip-191 // - // Since: cosmos-sdk 0.45 + // Currently, SIGN_MODE_EIP_191 is registered as a SignMode enum variant, + // but is not implemented on the SDK by default. To enable EIP-191, you need + // to pass a custom `TxConfig` that has an implementation of + // `SignModeHandler` for EIP-191. The SDK may decide to fully support + // EIP-191 in the future. + // + // Since: cosmos-sdk 0.45.2 SignMode_SIGN_MODE_EIP_191 SignMode = 191 ) diff --git a/api/cosmos/tx/v1beta1/tx.pulsar.go b/api/cosmos/tx/v1beta1/tx.pulsar.go index d39c5e6ef..b1b49c4bf 100644 --- a/api/cosmos/tx/v1beta1/tx.pulsar.go +++ b/api/cosmos/tx/v1beta1/tx.pulsar.go @@ -8814,11 +8814,11 @@ type AuxSignerData struct { // AuxSignerData across different chains, the bech32 prefix of the target // chain (where the final transaction is broadcasted) should be used. Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"` - // sign_doc is the SIGN_MOD_DIRECT_AUX sign doc that the auxiliary signer + // sign_doc is the SIGN_MODE_DIRECT_AUX sign doc that the auxiliary signer // signs. Note: we use the same sign doc even if we're signing with // LEGACY_AMINO_JSON. SignDoc *SignDocDirectAux `protobuf:"bytes,2,opt,name=sign_doc,json=signDoc,proto3" json:"sign_doc,omitempty"` - // mode is the signing mode of the single signer + // mode is the signing mode of the single signer. Mode v1beta1.SignMode `protobuf:"varint,3,opt,name=mode,proto3,enum=cosmos.tx.signing.v1beta1.SignMode" json:"mode,omitempty"` // sig is the signature of the sign doc. Sig []byte `protobuf:"bytes,4,opt,name=sig,proto3" json:"sig,omitempty"` diff --git a/client/tx/aux_builder.go b/client/tx/aux_builder.go index 7b6086f6d..da8dd1fb9 100644 --- a/client/tx/aux_builder.go +++ b/client/tx/aux_builder.go @@ -121,14 +121,14 @@ func (b *AuxTxBuilder) SetSignMode(mode signing.SignMode) error { return nil } -// SetTip sets an optional tip. +// SetTip sets an optional tip in the AuxSignerData. func (b *AuxTxBuilder) SetTip(tip *tx.Tip) { b.checkEmptyFields() b.auxSignerData.SignDoc.Tip = tip } -// SetSignature sets the aux signer's signature. +// SetSignature sets the aux signer's signature in the AuxSignerData. func (b *AuxTxBuilder) SetSignature(sig []byte) { b.checkEmptyFields() diff --git a/docs/core/tips.md b/docs/core/tips.md index 3765cc9f4..e750b8d9c 100644 --- a/docs/core/tips.md +++ b/docs/core/tips.md @@ -66,7 +66,7 @@ As we mentioned in the flow above, the tipper signs over the `SignDocDirectAux`, - The tipper MUST use `SIGN_MODE_DIRECT_AUX` or `SIGN_MODE_LEGACY_AMINO_JSON`. That is because the tipper needs to sign over the body, the tip, but not the other signers' information and not over the fee (which is unknown to the tipper). - The fee payer MUST use `SIGN_MODE_DIRECT` or `SIGN_MODE_LEGACY_AMINO_JSON`. The fee payer signs over the whole transaction. -For example, if the fee payers signs the whole transaction with `SIGN_MODE_DIRECT_AUX`, it will be rejected by the node, as that would introduce malleability issues (`SIGN_MODE_DIRECT_AUX` doesn't sign over fees). +For example, if the fee payer signs the whole transaction with `SIGN_MODE_DIRECT_AUX`, it will be rejected by the node, as that would introduce malleability issues (`SIGN_MODE_DIRECT_AUX` doesn't sign over fees). In both cases, using `SIGN_MODE_LEGACY_AMINO_JSON` is recommended only if hardware wallet signing is needed. @@ -98,7 +98,7 @@ If you are using the Cosmos SDK's default middleware stack `NewDefaultTxHandler( The Cosmos SDK also provides some CLI tooling for the transaction tips flow, both for the tipper and for the feepayer. -For the tipper, the CLI `tx` subcommand has two new flags: `--aux` and `--tip`. The `--aux` flag is used to denote that we are creating a `AuxSignerData` instead of a , and the `--tip` is used to populate its `Tip` field. +For the tipper, the CLI `tx` subcommand has two new flags: `--aux` and `--tip`. The `--aux` flag is used to denote that we are creating an `AuxSignerData` instead of a `Tx`, and the `--tip` is used to populate its `Tip` field. ```bash $ simd tx gov vote 16 yes --from --aux --tip 50ibcdenom @@ -110,7 +110,7 @@ $ simd tx gov vote 16 yes --from --aux --tip 50ibcdenom It is useful to pipe the JSON output to a file, `> aux_signed_tx.json` -For the fee payer, the Cosmos SDK added a `tx aux-to-fee` subcommand to include a `AuxSignerData` into a transaction, add fees to it, and broadcast it. +For the fee payer, the Cosmos SDK added a `tx aux-to-fee` subcommand to include an `AuxSignerData` into a transaction, add fees to it, and broadcast it. ```bash $ simd tx aux-to-fee aux_signed_tx.json --from --fees 30atom diff --git a/proto/cosmos/tx/v1beta1/tx.proto b/proto/cosmos/tx/v1beta1/tx.proto index ac7b690f4..b015604e3 100644 --- a/proto/cosmos/tx/v1beta1/tx.proto +++ b/proto/cosmos/tx/v1beta1/tx.proto @@ -238,11 +238,11 @@ message AuxSignerData { // AuxSignerData across different chains, the bech32 prefix of the target // chain (where the final transaction is broadcasted) should be used. string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; - // sign_doc is the SIGN_MOD_DIRECT_AUX sign doc that the auxiliary signer + // sign_doc is the SIGN_MODE_DIRECT_AUX sign doc that the auxiliary signer // signs. Note: we use the same sign doc even if we're signing with // LEGACY_AMINO_JSON. SignDocDirectAux sign_doc = 2; - // mode is the signing mode of the single signer + // mode is the signing mode of the single signer. cosmos.tx.signing.v1beta1.SignMode mode = 3; // sig is the signature of the sign doc. bytes sig = 4; diff --git a/types/tx/signing/signing.pb.go b/types/tx/signing/signing.pb.go index 102264052..95989270b 100644 --- a/types/tx/signing/signing.pb.go +++ b/types/tx/signing/signing.pb.go @@ -58,7 +58,13 @@ const ( // SIGN_MODE_EIP_191 specifies the sign mode for EIP 191 signing on the Cosmos // SDK. Ref: https://eips.ethereum.org/EIPS/eip-191 // - // Since: cosmos-sdk 0.45 + // Currently, SIGN_MODE_EIP_191 is registered as a SignMode enum variant, + // but is not implemented on the SDK by default. To enable EIP-191, you need + // to pass a custom `TxConfig` that has an implementation of + // `SignModeHandler` for EIP-191. The SDK may decide to fully support + // EIP-191 in the future. + // + // Since: cosmos-sdk 0.45.2 SignMode_SIGN_MODE_EIP_191 SignMode = 191 ) diff --git a/types/tx/tx.pb.go b/types/tx/tx.pb.go index d16c78981..ca8ae7806 100644 --- a/types/tx/tx.pb.go +++ b/types/tx/tx.pb.go @@ -922,11 +922,11 @@ type AuxSignerData struct { // AuxSignerData across different chains, the bech32 prefix of the target // chain (where the final transaction is broadcasted) should be used. Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"` - // sign_doc is the SIGN_MOD_DIRECT_AUX sign doc that the auxiliary signer + // sign_doc is the SIGN_MODE_DIRECT_AUX sign doc that the auxiliary signer // signs. Note: we use the same sign doc even if we're signing with // LEGACY_AMINO_JSON. SignDoc *SignDocDirectAux `protobuf:"bytes,2,opt,name=sign_doc,json=signDoc,proto3" json:"sign_doc,omitempty"` - // mode is the signing mode of the single signer + // mode is the signing mode of the single signer. Mode signing.SignMode `protobuf:"varint,3,opt,name=mode,proto3,enum=cosmos.tx.signing.v1beta1.SignMode" json:"mode,omitempty"` // sig is the signature of the sign doc. Sig []byte `protobuf:"bytes,4,opt,name=sig,proto3" json:"sig,omitempty"`