From 956d8b4915d0344a8656e400fb4ffa7e466dfb2f Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Fri, 22 Oct 2021 15:13:37 +0200 Subject: [PATCH] feat: Add `fee.{payer,granter}` and `tip` fields to StdSignDoc (#10348) * Add IsTipper * Use addr in signer data * Always populate addr in signer data * fi error messages * make proto gen * fix build * Add fields to sign docs and sign mode handler * Remove getSequence * Update x/auth/migrations/legacytx/stdtx.go Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com> * Update x/auth/migrations/legacytx/stdsign.go Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com> * Use addressCodec * NewTxConfig with addrCdc * REmove simapp.NewBech32 * Move bech32 stuff to x/auth/address * Add changelog * Move address.Codec to x/auth * Fix test * Add tests for tipper and feepayer * Rename tests * Add more tests * Empty tip test * Revert unwanted files * Less line diff * fix test * fix another test * Fix test * Update x/auth/migrations/legacytx/stdtx_test.go Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com> * Address reviews Co-authored-by: Simon Warta <2603011+webmaster128@users.noreply.github.com> Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com> --- CHANGELOG.md | 4 +- types/tx/tips.go | 11 +++ x/auth/migrations/legacytx/amino_signing.go | 2 +- .../migrations/legacytx/amino_signing_test.go | 2 +- x/auth/migrations/legacytx/stdsign.go | 15 ++- x/auth/migrations/legacytx/stdsignmsg.go | 2 +- x/auth/migrations/legacytx/stdtx.go | 12 ++- x/auth/migrations/legacytx/stdtx_test.go | 63 +++++++++++-- x/auth/signing/verify_test.go | 4 +- x/auth/tx/builder.go | 5 + x/auth/tx/direct_aux.go | 2 +- x/auth/tx/legacy_amino_json.go | 28 +++++- x/auth/tx/legacy_amino_json_test.go | 92 +++++++++++++++---- 13 files changed, 203 insertions(+), 39 deletions(-) create mode 100644 types/tx/tips.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 911984bb0cf5..ff7a44201e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#10045](https://github.com/cosmos/cosmos-sdk/pull/10045) Revert [#8549](https://github.com/cosmos/cosmos-sdk/pull/8549). Do not route grpc queries through Tendermint. * [\#10326](https://github.com/cosmos/cosmos-sdk/pull/10326) `x/authz` add query all grants by granter query. * [\#10024](https://github.com/cosmos/cosmos-sdk/pull/10024) `store/cachekv` performance improvement by reduced growth factor for iterator ranging by using binary searches to find dirty items when unsorted key count >= 1024 +* [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) Add `fee.{payer,granter}` and `tip` fields to StdSignDoc for signing tipped transactions. ### API Breaking Changes @@ -98,8 +99,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Replace `baseapp.SetAnteHandler` with `baseapp.SetTxHandler`. * Move Msg routers from BaseApp to middlewares. * Move Baseapp panic recovery into a middleware. - * Rename simulation helper methods `baseapp.{Check,Deliver}` to `baseapp.Sim{Check,Deliver}**. + * Rename simulation helper methods `baseapp.{Check,Deliver}` to `baseapp.Sim{Check,Deliver}**`. * (x/gov) [\#10373](https://github.com/cosmos/cosmos-sdk/pull/10373) Removed gov `keeper.{MustMarshal, MustUnmarshal}`. +* [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) StdSignBytes takes a new argument of type `*tx.Tip` for signing over tips using LEGACY_AMINO_JSON. ### Client Breaking Changes diff --git a/types/tx/tips.go b/types/tx/tips.go new file mode 100644 index 000000000000..ca8afb36d5ae --- /dev/null +++ b/types/tx/tips.go @@ -0,0 +1,11 @@ +package tx + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// TipTx defines the interface to be implemented by Txs that handle Tips. +type TipTx interface { + sdk.FeeTx + GetTip() *Tip +} diff --git a/x/auth/migrations/legacytx/amino_signing.go b/x/auth/migrations/legacytx/amino_signing.go index 2f5b1d4a4218..4115efa70493 100644 --- a/x/auth/migrations/legacytx/amino_signing.go +++ b/x/auth/migrations/legacytx/amino_signing.go @@ -43,7 +43,7 @@ func (stdTxSignModeHandler) GetSignBytes(mode signingtypes.SignMode, data signin } return StdSignBytes( - data.ChainID, data.AccountNumber, data.Sequence, stdTx.GetTimeoutHeight(), StdFee{Amount: stdTx.GetFee(), Gas: stdTx.GetGas()}, tx.GetMsgs(), stdTx.GetMemo(), + data.ChainID, data.AccountNumber, data.Sequence, stdTx.GetTimeoutHeight(), StdFee{Amount: stdTx.GetFee(), Gas: stdTx.GetGas()}, tx.GetMsgs(), stdTx.GetMemo(), nil, ), nil } diff --git a/x/auth/migrations/legacytx/amino_signing_test.go b/x/auth/migrations/legacytx/amino_signing_test.go index 89410d6c8948..3342c2dba01b 100644 --- a/x/auth/migrations/legacytx/amino_signing_test.go +++ b/x/auth/migrations/legacytx/amino_signing_test.go @@ -55,7 +55,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.NoError(t, err) - expectedSignBz := StdSignBytes(chainId, accNum, seqNum, timeoutHeight, fee, msgs, memo) + expectedSignBz := StdSignBytes(chainId, accNum, seqNum, timeoutHeight, fee, msgs, memo, nil) require.Equal(t, expectedSignBz, signBz) diff --git a/x/auth/migrations/legacytx/stdsign.go b/x/auth/migrations/legacytx/stdsign.go index d22e3786d9f0..9467ee8027c8 100644 --- a/x/auth/migrations/legacytx/stdsign.go +++ b/x/auth/migrations/legacytx/stdsign.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -47,10 +48,11 @@ type StdSignDoc struct { Memo string `json:"memo" yaml:"memo"` Fee json.RawMessage `json:"fee" yaml:"fee"` Msgs []json.RawMessage `json:"msgs" yaml:"msgs"` + Tip *StdTip `json:"tip,omitempty" yaml:"tip"` } // StdSignBytes returns the bytes to sign for a transaction. -func StdSignBytes(chainID string, accnum, sequence, timeout uint64, fee StdFee, msgs []sdk.Msg, memo string) []byte { +func StdSignBytes(chainID string, accnum, sequence, timeout uint64, fee StdFee, msgs []sdk.Msg, memo string, tip *tx.Tip) []byte { msgsBytes := make([]json.RawMessage, 0, len(msgs)) for _, msg := range msgs { legacyMsg, ok := msg.(LegacyMsg) @@ -61,6 +63,15 @@ func StdSignBytes(chainID string, accnum, sequence, timeout uint64, fee StdFee, msgsBytes = append(msgsBytes, json.RawMessage(legacyMsg.GetSignBytes())) } + var stdTip *StdTip + if tip != nil { + if tip.Tipper == "" { + panic(fmt.Errorf("tipper cannot be empty")) + } + + stdTip = &StdTip{Amount: tip.Amount, Tipper: tip.Tipper} + } + bz, err := legacy.Cdc.MarshalJSON(StdSignDoc{ AccountNumber: accnum, ChainID: chainID, @@ -69,6 +80,7 @@ func StdSignBytes(chainID string, accnum, sequence, timeout uint64, fee StdFee, Msgs: msgsBytes, Sequence: sequence, TimeoutHeight: timeout, + Tip: stdTip, }) if err != nil { panic(err) @@ -106,7 +118,6 @@ func (ss StdSignature) MarshalYAML() (interface{}, error) { pk = ss.PubKey.String() } - bz, err := yaml.Marshal(struct { PubKey string `json:"pub_key"` Signature string `json:"signature"` diff --git a/x/auth/migrations/legacytx/stdsignmsg.go b/x/auth/migrations/legacytx/stdsignmsg.go index 07ee29a06324..56e68b0f5b6f 100644 --- a/x/auth/migrations/legacytx/stdsignmsg.go +++ b/x/auth/migrations/legacytx/stdsignmsg.go @@ -21,7 +21,7 @@ type StdSignMsg struct { // get message bytes func (msg StdSignMsg) Bytes() []byte { - return StdSignBytes(msg.ChainID, msg.AccountNumber, msg.Sequence, msg.TimeoutHeight, msg.Fee, msg.Msgs, msg.Memo) + return StdSignBytes(msg.ChainID, msg.AccountNumber, msg.Sequence, msg.TimeoutHeight, msg.Fee, msg.Msgs, msg.Memo, nil) } func (msg StdSignMsg) UnpackInterfaces(unpacker types.AnyUnpacker) error { diff --git a/x/auth/migrations/legacytx/stdtx.go b/x/auth/migrations/legacytx/stdtx.go index 55ec38e603cc..68f3b330eae8 100644 --- a/x/auth/migrations/legacytx/stdtx.go +++ b/x/auth/migrations/legacytx/stdtx.go @@ -25,8 +25,10 @@ var ( // which must be above some miminum to be accepted into the mempool. // [Deprecated] type StdFee struct { - Amount sdk.Coins `json:"amount" yaml:"amount"` - Gas uint64 `json:"gas" yaml:"gas"` + Amount sdk.Coins `json:"amount" yaml:"amount"` + Gas uint64 `json:"gas" yaml:"gas"` + Payer string `json:"payer,omitempty" yaml:"payer"` + Granter string `json:"granter,omitempty" yaml:"granter"` } // Deprecated: NewStdFee returns a new instance of StdFee @@ -70,6 +72,12 @@ func (fee StdFee) GasPrices() sdk.DecCoins { return sdk.NewDecCoinsFromCoins(fee.Amount...).QuoDec(sdk.NewDec(int64(fee.Gas))) } +// StdTip is the tips used in a tipped transaction. +type StdTip struct { + Amount sdk.Coins `json:"amount" yaml:"amount"` + Tipper string `json:"tipper" yaml:"tipper"` +} + // StdTx is the legacy transaction format for wrapping a Msg with Fee and Signatures. // It only works with Amino, please prefer the new protobuf Tx in types/tx. // NOTE: the first signature is the fee payer (Signatures must not be nil). diff --git a/x/auth/migrations/legacytx/stdtx_test.go b/x/auth/migrations/legacytx/stdtx_test.go index ec70281f6828..650f6f4b155c 100644 --- a/x/auth/migrations/legacytx/stdtx_test.go +++ b/x/auth/migrations/legacytx/stdtx_test.go @@ -17,6 +17,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -41,7 +42,7 @@ func NewTestStdFee() StdFee { func NewTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []cryptotypes.PrivKey, accNums []uint64, seqs []uint64, timeout uint64, fee StdFee) sdk.Tx { sigs := make([]StdSignature, len(privs)) for i, priv := range privs { - signBytes := StdSignBytes(ctx.ChainID(), accNums[i], seqs[i], timeout, fee, msgs, "") + signBytes := StdSignBytes(ctx.ChainID(), accNums[i], seqs[i], timeout, fee, msgs, "", nil) sig, err := priv.Sign(signBytes) if err != nil { @@ -80,24 +81,72 @@ func TestStdSignBytes(t *testing.T) { fee StdFee msgs []sdk.Msg memo string + tip *tx.Tip } defaultFee := NewTestStdFee() + defaultTip := &tx.Tip{Tipper: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin("tiptoken", 150))} tests := []struct { + name string args args want string }{ { - args{"1234", 3, 6, 10, defaultFee, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo"}, - fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"100000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\",\"timeout_height\":\"10\"}", addr), + "with timeout height", + args{"1234", 3, 6, 10, defaultFee, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000"},"memo":"memo","msgs":[["%s"]],"sequence":"6","timeout_height":"10"}`, addr), }, { - args{"1234", 3, 6, 0, defaultFee, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo"}, - fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"100000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr), + "no timeout height (omitempty)", + args{"1234", 3, 6, 0, defaultFee, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr), + }, + { + "empty fee", + args{"1234", 3, 6, 0, StdFee{}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr), + }, + { + "no fee payer and fee granter (both omitempty)", + args{"1234", 3, 6, 0, StdFee{Amount: defaultFee.Amount, Gas: defaultFee.Gas}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr), + }, + { + "with fee granter, no fee payer (omitempty)", + args{"1234", 3, 6, 0, StdFee{Amount: defaultFee.Amount, Gas: defaultFee.Gas, Granter: addr.String()}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000","granter":"%s"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr, addr), + }, + { + "with fee payer, no fee granter (omitempty)", + args{"1234", 3, 6, 0, StdFee{Amount: defaultFee.Amount, Gas: defaultFee.Gas, Payer: addr.String()}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000","payer":"%s"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr, addr), + }, + { + "with fee payer and fee granter", + args{"1234", 3, 6, 0, StdFee{Amount: defaultFee.Amount, Gas: defaultFee.Gas, Payer: addr.String(), Granter: addr.String()}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", nil}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000","granter":"%s","payer":"%s"},"memo":"memo","msgs":[["%s"]],"sequence":"6"}`, addr, addr, addr), + }, + { + "no fee, with tip", + args{"1234", 3, 6, 0, StdFee{}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", defaultTip}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[],"gas":"0"},"memo":"memo","msgs":[["%s"]],"sequence":"6","tip":{"amount":[{"amount":"150","denom":"tiptoken"}],"tipper":"%s"}}`, addr, addr), + }, + { + "with fee and with tip", + args{"1234", 3, 6, 0, StdFee{Amount: defaultFee.Amount, Gas: defaultFee.Gas, Payer: addr.String()}, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", defaultTip}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000","payer":"%s"},"memo":"memo","msgs":[["%s"]],"sequence":"6","tip":{"amount":[{"amount":"150","denom":"tiptoken"}],"tipper":"%s"}}`, addr, addr, addr), + }, + { + "with empty tip (but not nil), tipper cannot be empty", + args{"1234", 3, 6, 0, defaultFee, []sdk.Msg{testdata.NewTestMsg(addr)}, "memo", &tx.Tip{Tipper: addr.String()}}, + fmt.Sprintf(`{"account_number":"3","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"100000"},"memo":"memo","msgs":[["%s"]],"sequence":"6","tip":{"amount":[],"tipper":"%s"}}`, addr, addr), }, } for i, tc := range tests { - got := string(StdSignBytes(tc.args.chainID, tc.args.accnum, tc.args.sequence, tc.args.timeoutHeight, tc.args.fee, tc.args.msgs, tc.args.memo)) - require.Equal(t, tc.want, got, "Got unexpected result on test case i: %d", i) + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := string(StdSignBytes(tc.args.chainID, tc.args.accnum, tc.args.sequence, tc.args.timeoutHeight, tc.args.fee, tc.args.msgs, tc.args.memo, tc.args.tip)) + require.Equal(t, tc.want, got, "Got unexpected result on test case i: %d", i) + }) } } diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 99eeaf8a04e3..f67590491d61 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -55,7 +55,7 @@ func TestVerifySignature(t *testing.T) { Sequence: acc.GetSequence(), SignerIndex: 0, } - signBytes := legacytx.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo) + signBytes := legacytx.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo, nil) signature, err := priv.Sign(signBytes) require.NoError(t, err) @@ -73,7 +73,7 @@ func TestVerifySignature(t *testing.T) { multisigKey := kmultisig.NewLegacyAminoPubKey(2, pkSet) multisignature := multisig.NewMultisig(2) msgs = []sdk.Msg{testdata.NewTestMsg(addr, addr1)} - multiSignBytes := legacytx.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo) + multiSignBytes := legacytx.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo, nil) sig1, err := priv.Sign(multiSignBytes) require.NoError(t, err) diff --git a/x/auth/tx/builder.go b/x/auth/tx/builder.go index 52e35705f062..d3b83d5018bb 100644 --- a/x/auth/tx/builder.go +++ b/x/auth/tx/builder.go @@ -35,6 +35,7 @@ var ( _ client.TxBuilder = &wrapper{} _ middleware.HasExtensionOptionsTx = &wrapper{} _ ExtensionOptionsTxBuilder = &wrapper{} + _ tx.TipTx = &wrapper{} ) // ExtensionOptionsTxBuilder defines a TxBuilder that can also set extensions. @@ -156,6 +157,10 @@ func (w *wrapper) FeeGranter() sdk.AccAddress { return nil } +func (w *wrapper) GetTip() *tx.Tip { + return w.tx.AuthInfo.Tip +} + func (w *wrapper) GetMemo() string { return w.tx.Body.Memo } diff --git a/x/auth/tx/direct_aux.go b/x/auth/tx/direct_aux.go index c7324c18bb90..71a3fd5c0426 100644 --- a/x/auth/tx/direct_aux.go +++ b/x/auth/tx/direct_aux.go @@ -50,7 +50,7 @@ func (signModeDirectAuxHandler) GetSignBytes( AccountNumber: data.AccountNumber, Sequence: data.Sequence, Tip: protoTx.tx.AuthInfo.Tip, - PublicKey: protoTx.tx.AuthInfo.SignerInfos[data.SignerIndex].PublicKey, + PublicKey: signerInfo.PublicKey, } return signDocDirectAux.Marshal() diff --git a/x/auth/tx/legacy_amino_json.go b/x/auth/tx/legacy_amino_json.go index fea9d78a9bd3..01274f9f6ed9 100644 --- a/x/auth/tx/legacy_amino_json.go +++ b/x/auth/tx/legacy_amino_json.go @@ -46,9 +46,33 @@ func (s signModeLegacyAminoJSONHandler) GetSignBytes(mode signingtypes.SignMode, return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s does not support protobuf extension options", signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) } + addr := data.Address + if addr == "" { + return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "got empty address in %s handler", signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) + } + + tip := protoTx.GetTip() + isTipper := tip != nil && tip.Tipper == addr + + // We set a convention that if the tipper signs with LEGACY_AMINO_JSON, then + // they sign over empty fees and 0 gas. + if isTipper { + return legacytx.StdSignBytes( + data.ChainID, data.AccountNumber, data.Sequence, protoTx.GetTimeoutHeight(), + // The tipper signs over 0 fee and 0 gas, no feepayer, no feegranter by convention. + legacytx.StdFee{}, + tx.GetMsgs(), protoTx.GetMemo(), tip, + ), nil + } + return legacytx.StdSignBytes( data.ChainID, data.AccountNumber, data.Sequence, protoTx.GetTimeoutHeight(), - legacytx.StdFee{Amount: protoTx.GetFee(), Gas: protoTx.GetGas()}, - tx.GetMsgs(), protoTx.GetMemo(), + legacytx.StdFee{ + Amount: protoTx.GetFee(), + Gas: protoTx.GetGas(), + Payer: protoTx.tx.AuthInfo.Fee.Payer, + Granter: protoTx.tx.AuthInfo.Fee.Granter, + }, + tx.GetMsgs(), protoTx.GetMemo(), tip, ), nil } diff --git a/x/auth/tx/legacy_amino_json_test.go b/x/auth/tx/legacy_amino_json_test.go index 893d06f5ff2d..9734e7704eea 100644 --- a/x/auth/tx/legacy_amino_json_test.go +++ b/x/auth/tx/legacy_amino_json_test.go @@ -8,6 +8,7 @@ import ( cdctypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + "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" @@ -33,17 +34,79 @@ func buildTx(t *testing.T, bldr *wrapper) { } func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { - bldr := newBuilder() - buildTx(t, bldr) - tx := bldr.GetTx() - var ( - chainId = "test-chain" - accNum uint64 = 7 - seqNum uint64 = 7 + chainId = "test-chain" + accNum uint64 = 7 + seqNum uint64 = 7 + tip *tx.Tip = &tx.Tip{Tipper: addr1.String(), Amount: coins} ) + testcases := []struct { + name string + signer string + malleate func(*wrapper) + expectedSignBz []byte + }{ + { + "signer which is also fee payer (no tips)", addr1.String(), + func(w *wrapper) {}, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas}, []sdk.Msg{msg}, memo, nil), + }, + { + "signer which is also fee payer (with tips)", addr2.String(), + func(w *wrapper) { w.SetTip(tip) }, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas}, []sdk.Msg{msg}, memo, tip), + }, + { + "explicit fee payer", addr1.String(), + func(w *wrapper) { w.SetFeePayer(addr2) }, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Payer: addr2.String()}, []sdk.Msg{msg}, memo, nil), + }, + { + "explicit fee granter", addr1.String(), + func(w *wrapper) { w.SetFeeGranter(addr2) }, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Granter: addr2.String()}, []sdk.Msg{msg}, memo, nil), + }, + { + "explicit fee payer and fee granter", addr1.String(), + func(w *wrapper) { + w.SetFeePayer(addr2) + w.SetFeeGranter(addr2) + }, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{Amount: coins, Gas: gas, Payer: addr2.String(), Granter: addr2.String()}, []sdk.Msg{msg}, memo, nil), + }, + { + "signer which is also tipper", addr1.String(), + func(w *wrapper) { w.SetTip(tip) }, + legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{}, []sdk.Msg{msg}, memo, tip), + }, + } + handler := signModeLegacyAminoJSONHandler{} + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + bldr := newBuilder() + buildTx(t, bldr) + tx := bldr.GetTx() + tc.malleate(bldr) + + signingData := signing.SignerData{ + Address: tc.signer, + ChainID: chainId, + AccountNumber: accNum, + Sequence: seqNum, + } + signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + require.NoError(t, err) + + require.Equal(t, tc.expectedSignBz, signBz) + }) + } + + bldr := newBuilder() + buildTx(t, bldr) + tx := bldr.GetTx() signingData := signing.SignerData{ Address: addr1.String(), ChainID: chainId, @@ -51,18 +114,9 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { Sequence: seqNum, SignerIndex: 0, } - signBz, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) - require.NoError(t, err) - - expectedSignBz := legacytx.StdSignBytes(chainId, accNum, seqNum, timeout, legacytx.StdFee{ - Amount: coins, - Gas: gas, - }, []sdk.Msg{msg}, memo) - - require.Equal(t, expectedSignBz, signBz) // expect error with wrong sign mode - _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) + _, err := handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT, signingData, tx) require.Error(t, err) // expect error with extension options @@ -72,7 +126,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { require.NoError(t, err) bldr.tx.Body.ExtensionOptions = []*cdctypes.Any{any} tx = bldr.GetTx() - signBz, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.Error(t, err) // expect error with non-critical extension options @@ -80,7 +134,7 @@ func TestLegacyAminoJSONHandler_GetSignBytes(t *testing.T) { buildTx(t, bldr) bldr.tx.Body.NonCriticalExtensionOptions = []*cdctypes.Any{any} tx = bldr.GetTx() - signBz, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) + _, err = handler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, signingData, tx) require.Error(t, err) }