diff --git a/CHANGELOG.md b/CHANGELOG.md index 085c0acb8cdf..e778c4b38af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#9837](https://github.com/cosmos/cosmos-sdk/issues/9837) `--generate-only` flag will accept the keyname now. * [\#10326](https://github.com/cosmos/cosmos-sdk/pull/10326) `x/authz` add query all grants by granter query. * [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) Add `fee.{payer,granter}` and `tip` fields to StdSignDoc for signing tipped transactions. +* [\#10208](https://github.com/cosmos/cosmos-sdk/pull/10208) Add `TipsTxMiddleware` for transferring tips. * [\#10379](https://github.com/cosmos/cosmos-sdk/pull/10379) Add validation to `x/upgrade` CLI `software-upgrade` command `--plan-info` value. ### Improvements @@ -105,7 +106,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * 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. - +* [\#10208](https://github.com/cosmos/cosmos-sdk/pull/10208) The `x/auth/signing.Tx` interface now also includes a new `GetTip() *tx.Tip` method for verifying tipped transactions. The `x/auth/types` expected BankKeeper interface now expects the `SendCoins` method too. ### Client Breaking Changes diff --git a/client/tx_config.go b/client/tx_config.go index cb82a06e6686..e49dc1a6e603 100644 --- a/client/tx_config.go +++ b/client/tx_config.go @@ -44,7 +44,7 @@ type ( SetGasLimit(limit uint64) SetTip(tip *tx.Tip) SetTimeoutHeight(height uint64) - SetFeePayer(feeGranter sdk.AccAddress) + SetFeePayer(feePayer sdk.AccAddress) SetFeeGranter(feeGranter sdk.AccAddress) AddAuxSignerData(tx.AuxSignerData) error } diff --git a/types/tx/aux.go b/types/tx/aux.go index 2c9a71262252..73189ff840c5 100644 --- a/types/tx/aux.go +++ b/types/tx/aux.go @@ -48,6 +48,23 @@ func (a *AuxSignerData) ValidateBasic() error { return a.GetSignDoc().ValidateBasic() } +// GetSignaturesV2 gets the SignatureV2 of the aux signer. +func (a *AuxSignerData) GetSignatureV2() (signing.SignatureV2, error) { + pk, ok := a.SignDoc.PublicKey.GetCachedValue().(cryptotypes.PubKey) + if !ok { + return signing.SignatureV2{}, sdkerrors.ErrInvalidType.Wrapf("expected %T, got %T", (cryptotypes.PubKey)(nil), pk) + } + + return signing.SignatureV2{ + PubKey: pk, + Data: &signing.SingleSignatureData{ + SignMode: a.Mode, + Signature: a.Sig, + }, + Sequence: a.SignDoc.Sequence, + }, nil +} + // UnpackInterfaces implements the UnpackInterfaceMessages.UnpackInterfaces method func (a *AuxSignerData) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { return a.GetSignDoc().UnpackInterfaces(unpacker) diff --git a/types/tx/aux_test.go b/types/tx/aux_test.go index 02b39f0f356e..7120126c0215 100644 --- a/types/tx/aux_test.go +++ b/types/tx/aux_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -77,6 +78,12 @@ func TestAuxSignerData(t *testing.T) { require.Error(t, err) } else { require.NoError(t, err) + sigV2, err := tc.sd.GetSignatureV2() + require.NoError(t, err) + require.Equal(t, tc.sd.Mode, sigV2.Data.(*signing.SingleSignatureData).SignMode) + require.Equal(t, tc.sd.Sig, sigV2.Data.(*signing.SingleSignatureData).Signature) + require.Equal(t, tc.sd.SignDoc.Sequence, sigV2.Sequence) + require.True(t, tc.sd.SignDoc.PublicKey.GetCachedValue().(cryptotypes.PubKey).Equals(sigV2.PubKey)) } }) } diff --git a/x/auth/middleware/middleware.go b/x/auth/middleware/middleware.go index 60820bb46cc5..be8af2176215 100644 --- a/x/auth/middleware/middleware.go +++ b/x/auth/middleware/middleware.go @@ -93,6 +93,7 @@ func NewDefaultTxHandler(options TxHandlerOptions) (tx.Handler, error) { ValidateSigCountMiddleware(options.AccountKeeper), SigGasConsumeMiddleware(options.AccountKeeper, sigGasConsumer), SigVerificationMiddleware(options.AccountKeeper, options.SignModeHandler), + NewTipMiddleware(options.BankKeeper), IncrementSequenceMiddleware(options.AccountKeeper), ), nil } diff --git a/x/auth/middleware/middleware_test.go b/x/auth/middleware/middleware_test.go index e3b04983b6f5..d88e5cc6c1da 100644 --- a/x/auth/middleware/middleware_test.go +++ b/x/auth/middleware/middleware_test.go @@ -23,13 +23,15 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) +var testCoins = sdk.Coins{sdk.NewInt64Coin("atom", 10000000)} + // Test that simulate transaction accurately estimates gas cost func (s *MWTestSuite) TestSimulateGasCost() { ctx := s.SetupTest(false) // reset txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 3) + accounts := s.createTestAccounts(ctx, 3, testCoins) msgs := []sdk.Msg{ testdata.NewTestMsg(accounts[0].acc.GetAddress(), accounts[1].acc.GetAddress()), testdata.NewTestMsg(accounts[2].acc.GetAddress(), accounts[0].acc.GetAddress()), @@ -166,7 +168,7 @@ func (s *MWTestSuite) TestTxHandlerAccountNumbers() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 2) + accounts := s.createTestAccounts(ctx, 2, testCoins) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() @@ -248,7 +250,7 @@ func (s *MWTestSuite) TestTxHandlerAccountNumbersAtBlockHeightZero() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 2) + accounts := s.createTestAccounts(ctx, 2, testCoins) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() @@ -331,7 +333,7 @@ func (s *MWTestSuite) TestTxHandlerSequences() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 3) + accounts := s.createTestAccounts(ctx, 3, testCoins) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() @@ -524,7 +526,7 @@ func (s *MWTestSuite) TestTxHandlerMemoGas() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 1) + accounts := s.createTestAccounts(ctx, 1, testCoins) msgs := []sdk.Msg{testdata.NewTestMsg(accounts[0].acc.GetAddress())} privs, accNums, accSeqs := []cryptotypes.PrivKey{accounts[0].priv}, []uint64{0}, []uint64{0} @@ -594,7 +596,7 @@ func (s *MWTestSuite) TestTxHandlerMultiSigner() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 3) + accounts := s.createTestAccounts(ctx, 3, testCoins) msg1 := testdata.NewTestMsg(accounts[0].acc.GetAddress(), accounts[1].acc.GetAddress()) msg2 := testdata.NewTestMsg(accounts[2].acc.GetAddress(), accounts[0].acc.GetAddress()) msg3 := testdata.NewTestMsg(accounts[1].acc.GetAddress(), accounts[2].acc.GetAddress()) @@ -667,7 +669,7 @@ func (s *MWTestSuite) TestTxHandlerBadSignBytes() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 2) + accounts := s.createTestAccounts(ctx, 2, testCoins) msg0 := testdata.NewTestMsg(accounts[0].acc.GetAddress()) // Variable data per test case @@ -793,7 +795,7 @@ func (s *MWTestSuite) TestTxHandlerSetPubKey() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 2) + accounts := s.createTestAccounts(ctx, 2, testCoins) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() @@ -971,7 +973,7 @@ func (s *MWTestSuite) TestTxHandlerSigLimitExceeded() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 8) + accounts := s.createTestAccounts(ctx, 8, testCoins) var addrs []sdk.AccAddress var privs []cryptotypes.PrivKey for i := 0; i < 8; i++ { @@ -1029,7 +1031,7 @@ func (s *MWTestSuite) TestCustomSignatureVerificationGasConsumer() { s.Require().NoError(err) // Same data for every test cases - accounts := s.createTestAccounts(ctx, 1) + accounts := s.createTestAccounts(ctx, 1, testCoins) txBuilder.SetFeeAmount(testdata.NewTestFeeAmount()) txBuilder.SetGasLimit(testdata.NewTestGasLimit()) txBuilder.SetMsgs(testdata.NewTestMsg(accounts[0].acc.GetAddress())) @@ -1073,7 +1075,7 @@ func (s *MWTestSuite) TestTxHandlerReCheck() { txBuilder := s.clientCtx.TxConfig.NewTxBuilder() // Same data for every test cases - accounts := s.createTestAccounts(ctx, 1) + accounts := s.createTestAccounts(ctx, 1, testCoins) feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() diff --git a/x/auth/middleware/testutil_test.go b/x/auth/middleware/testutil_test.go index 4f174f6e69f2..e5fd3043a7f3 100644 --- a/x/auth/middleware/testutil_test.go +++ b/x/auth/middleware/testutil_test.go @@ -26,8 +26,9 @@ import ( // testAccount represents an account used in the tests in x/auth/middleware. type testAccount struct { - acc authtypes.AccountI - priv cryptotypes.PrivKey + acc authtypes.AccountI + priv cryptotypes.PrivKey + accNum uint64 } // MWTestSuite is a test suite to be used with middleware tests. @@ -42,7 +43,7 @@ type MWTestSuite struct { // returns context and app with params set on account keeper func createTestApp(t *testing.T, isCheckTx bool) (*simapp.SimApp, sdk.Context) { app := simapp.Setup(t, isCheckTx) - ctx := app.BaseApp.NewContext(isCheckTx, tmproto.Header{}).WithBlockGasMeter(sdk.NewInfiniteGasMeter()) + ctx := app.BaseApp.NewContext(isCheckTx, tmproto.Header{Height: app.LastBlockHeight() + 1}).WithBlockGasMeter(sdk.NewInfiniteGasMeter()) app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) return app, ctx @@ -52,7 +53,6 @@ func createTestApp(t *testing.T, isCheckTx bool) (*simapp.SimApp, sdk.Context) { func (s *MWTestSuite) SetupTest(isCheckTx bool) sdk.Context { var ctx sdk.Context s.app, ctx = createTestApp(s.T(), isCheckTx) - ctx = ctx.WithBlockHeight(1) // Set up TxConfig. encodingConfig := simapp.MakeTestEncodingConfig() @@ -89,25 +89,23 @@ func (s *MWTestSuite) SetupTest(isCheckTx bool) sdk.Context { // createTestAccounts creates `numAccs` accounts, and return all relevant // information about them including their private keys. -func (s *MWTestSuite) createTestAccounts(ctx sdk.Context, numAccs int) []testAccount { +func (s *MWTestSuite) createTestAccounts(ctx sdk.Context, numAccs int, coins sdk.Coins) []testAccount { var accounts []testAccount for i := 0; i < numAccs; i++ { priv, _, addr := testdata.KeyTestPubAddr() acc := s.app.AccountKeeper.NewAccountWithAddress(ctx, addr) - err := acc.SetAccountNumber(uint64(i)) + accNum := uint64(i) + err := acc.SetAccountNumber(accNum) s.Require().NoError(err) s.app.AccountKeeper.SetAccount(ctx, acc) - someCoins := sdk.Coins{ - sdk.NewInt64Coin("atom", 10000000), - } - err = s.app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, someCoins) + err = s.app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, coins) s.Require().NoError(err) - err = s.app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, someCoins) + err = s.app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, coins) s.Require().NoError(err) - accounts = append(accounts, testAccount{acc, priv}) + accounts = append(accounts, testAccount{acc, priv, accNum}) } return accounts diff --git a/x/auth/middleware/tips.go b/x/auth/middleware/tips.go new file mode 100644 index 000000000000..bcef9e62e0cd --- /dev/null +++ b/x/auth/middleware/tips.go @@ -0,0 +1,94 @@ +package middleware + +import ( + "context" + + abci "github.com/tendermint/tendermint/abci/types" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +type tipsTxHandler struct { + next tx.Handler + bankKeeper types.BankKeeper +} + +// NewTipMiddleware returns a new middleware for handling transactions with +// tips. +func NewTipMiddleware(bankKeeper types.BankKeeper) tx.Middleware { + return func(txh tx.Handler) tx.Handler { + return tipsTxHandler{txh, bankKeeper} + } +} + +var _ tx.Handler = tipsTxHandler{} + +// CheckTx implements tx.Handler.CheckTx. +func (txh tipsTxHandler) CheckTx(ctx context.Context, sdkTx sdk.Tx, req abci.RequestCheckTx) (abci.ResponseCheckTx, error) { + res, err := txh.next.CheckTx(ctx, sdkTx, req) + if err != nil { + return abci.ResponseCheckTx{}, err + } + + tipTx, ok := sdkTx.(tx.TipTx) + if !ok || tipTx.GetTip() == nil { + return res, err + } + + if err := txh.transferTip(ctx, tipTx); err != nil { + return abci.ResponseCheckTx{}, err + } + + return res, err +} + +// DeliverTx implements tx.Handler.DeliverTx. +func (txh tipsTxHandler) DeliverTx(ctx context.Context, sdkTx sdk.Tx, req abci.RequestDeliverTx) (abci.ResponseDeliverTx, error) { + res, err := txh.next.DeliverTx(ctx, sdkTx, req) + if err != nil { + return abci.ResponseDeliverTx{}, err + } + + tipTx, ok := sdkTx.(tx.TipTx) + if !ok || tipTx.GetTip() == nil { + return res, err + } + + if err := txh.transferTip(ctx, tipTx); err != nil { + return abci.ResponseDeliverTx{}, err + } + + return res, err +} + +// SimulateTx implements tx.Handler.SimulateTx method. +func (txh tipsTxHandler) SimulateTx(ctx context.Context, sdkTx sdk.Tx, req tx.RequestSimulateTx) (tx.ResponseSimulateTx, error) { + res, err := txh.next.SimulateTx(ctx, sdkTx, req) + if err != nil { + return tx.ResponseSimulateTx{}, err + } + + tipTx, ok := sdkTx.(tx.TipTx) + if !ok || tipTx.GetTip() == nil { + return res, err + } + + if err := txh.transferTip(ctx, tipTx); err != nil { + return tx.ResponseSimulateTx{}, err + } + + return res, err +} + +// transferTip transfers the tip from the tipper to the fee payer. +func (txh tipsTxHandler) transferTip(ctx context.Context, tipTx tx.TipTx) error { + sdkCtx := sdk.UnwrapSDKContext(ctx) + tipper, err := sdk.AccAddressFromBech32(tipTx.GetTip().Tipper) + if err != nil { + return err + } + + return txh.bankKeeper.SendCoins(sdkCtx, tipper, tipTx.FeePayer(), tipTx.GetTip().Amount) +} diff --git a/x/auth/middleware/tips_test.go b/x/auth/middleware/tips_test.go new file mode 100644 index 000000000000..b7f3c9ae8866 --- /dev/null +++ b/x/auth/middleware/tips_test.go @@ -0,0 +1,205 @@ +package middleware_test + +import ( + "time" + + abci "github.com/tendermint/tendermint/abci/types" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + + "github.com/cosmos/cosmos-sdk/client" + clienttx "github.com/cosmos/cosmos-sdk/client/tx" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" +) + +var initialRegens = sdk.NewCoins(sdk.NewCoin("regen", sdk.NewInt(1000))) +var initialAtoms = sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(1000))) + +// setupAcctsForTips sets up 2 accounts: +// - tipper has 1000 regens +// - feePayer has 1000 atoms and 1000 regens +func (s *MWTestSuite) setupAcctsForTips(ctx sdk.Context) (sdk.Context, []testAccount) { + accts := s.createTestAccounts(ctx, 2, initialRegens) + feePayer := accts[1] + err := s.app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initialAtoms) + s.Require().NoError(err) + err = s.app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, feePayer.acc.GetAddress(), initialAtoms) + s.Require().NoError(err) + + // Create dummy proposal for tipper to vote on. + prop, err := govtypes.NewProposal(govtypes.NewTextProposal("foo", "bar"), 1, time.Now(), time.Now().Add(time.Hour)) + s.Require().NoError(err) + s.app.GovKeeper.SetProposal(ctx, prop) + s.app.GovKeeper.ActivateVotingPeriod(ctx, prop) + + // Move to next block to commit previous data to state. + s.app.EndBlock(abci.RequestEndBlock{Height: ctx.BlockHeight()}) + s.app.Commit() + + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + s.app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: ctx.BlockHeight()}}) + + return ctx, accts +} + +func (s *MWTestSuite) TestTips() { + var msg sdk.Msg + + testcases := []struct { + name string + tip sdk.Coins + fee sdk.Coins + gasLimit uint64 + expErr bool + expErrStr string + }{ + { + "wrong tip denom", + sdk.NewCoins(sdk.NewCoin("foobar", sdk.NewInt(1000))), initialAtoms, 200000, + true, "0foobar is smaller than 1000foobar: insufficient funds", + }, + { + "insufficient tip from tipper", + sdk.NewCoins(sdk.NewCoin("regen", sdk.NewInt(5000))), initialAtoms, 200000, + true, "1000regen is smaller than 5000regen: insufficient funds", + }, + { + "insufficient fees from feePayer", + initialRegens, sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(5000))), 200000, + true, "1000atom is smaller than 5000atom: insufficient funds: insufficient funds", + }, + { + "insufficient gas", + initialRegens, initialAtoms, 100, + true, "out of gas in location: ReadFlat; gasWanted: 100, gasUsed: 1000: out of gas", + }, + { + "happy case", + initialRegens, initialAtoms, 200000, + false, "", + }, + } + + for _, tc := range testcases { + tc := tc + s.Run(tc.name, func() { + ctx := s.SetupTest(false) // reset + ctx, accts := s.setupAcctsForTips(ctx) + tipper, feePayer := accts[0], accts[1] + + msg = govtypes.NewMsgVote(tipper.acc.GetAddress(), 1, govtypes.OptionYes) + + auxSignerData := s.mkTipperAuxSignerData(tipper.priv, msg, tc.tip, signing.SignMode_SIGN_MODE_DIRECT_AUX, tipper.accNum, 0, ctx.ChainID()) + feePayerTxBuilder := s.mkFeePayerTxBuilder(s.clientCtx, auxSignerData, feePayer.priv, signing.SignMode_SIGN_MODE_DIRECT, tx.Fee{Amount: tc.fee, GasLimit: tc.gasLimit}, feePayer.accNum, 0, ctx.ChainID()) + + _, res, err := s.app.SimDeliver(s.clientCtx.TxConfig.TxEncoder(), feePayerTxBuilder.GetTx()) + + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrStr) + } else { + s.Require().NoError(err) + s.Require().NotNil(res) + + // Move to next block to commit previous data to state. + s.app.EndBlock(abci.RequestEndBlock{Height: ctx.BlockHeight()}) + s.app.Commit() + + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + s.app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: ctx.BlockHeight()}}) + + // Make sure tip is correctly transferred to feepayer, and fee is paid. + expTipperRegens := initialRegens.Sub(tc.tip) + expFeePayerRegens := initialRegens.Add(tc.tip...) + expFeePayerAtoms := initialAtoms.Sub(tc.fee) + s.Require().True(expTipperRegens.AmountOf("regen").Equal(s.app.BankKeeper.GetBalance(ctx, tipper.acc.GetAddress(), "regen").Amount)) + s.Require().True(expFeePayerRegens.AmountOf("regen").Equal(s.app.BankKeeper.GetBalance(ctx, feePayer.acc.GetAddress(), "regen").Amount)) + s.Require().True(expFeePayerAtoms.AmountOf("atom").Equal(s.app.BankKeeper.GetBalance(ctx, feePayer.acc.GetAddress(), "atom").Amount)) + // Make sure MsgVote has been submitted by tipper. + votes := s.app.GovKeeper.GetAllVotes(ctx) + s.Require().Len(votes, 1) + s.Require().Equal(tipper.acc.GetAddress().String(), votes[0].Voter) + } + }) + } +} + +func (s *MWTestSuite) mkTipperAuxSignerData( + tipperPriv cryptotypes.PrivKey, msg sdk.Msg, tip sdk.Coins, + signMode signing.SignMode, accNum, accSeq uint64, chainID string, +) tx.AuxSignerData { + tipperAddr := sdk.AccAddress(tipperPriv.PubKey().Address()).String() + b := clienttx.NewAuxTxBuilder() + b.SetAddress(tipperAddr) + b.SetAccountNumber(accNum) + b.SetSequence(accSeq) + err := b.SetMsgs(msg) + s.Require().NoError(err) + b.SetTip(&tx.Tip{Amount: tip, Tipper: tipperAddr}) + err = b.SetSignMode(signMode) + s.Require().NoError(err) + b.SetSequence(accSeq) + err = b.SetPubKey(tipperPriv.PubKey()) + s.Require().NoError(err) + b.SetChainID(chainID) + + signBz, err := b.GetSignBytes() + s.Require().NoError(err) + sig, err := tipperPriv.Sign(signBz) + s.Require().NoError(err) + b.SetSignature(sig) + + auxSignerData, err := b.GetAuxSignerData() + s.Require().NoError(err) + + return auxSignerData +} + +func (s *MWTestSuite) mkFeePayerTxBuilder( + clientCtx client.Context, + auxSignerData tx.AuxSignerData, + feePayerPriv cryptotypes.PrivKey, signMode signing.SignMode, + fee tx.Fee, accNum, accSeq uint64, chainID string, +) client.TxBuilder { + txBuilder := clientCtx.TxConfig.NewTxBuilder() + err := txBuilder.AddAuxSignerData(auxSignerData) + s.Require().NoError(err) + txBuilder.SetFeePayer(sdk.AccAddress(feePayerPriv.PubKey().Address())) + txBuilder.SetFeeAmount(fee.Amount) + txBuilder.SetGasLimit(fee.GasLimit) + + // Calling SetSignatures with empty sig to populate AuthInfo. + tipperSigsV2, err := auxSignerData.GetSignatureV2() + s.Require().NoError(err) + feePayerSigV2 := signing.SignatureV2{ + PubKey: feePayerPriv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: signMode, + Signature: nil, + }} + sigsV2 := append([]signing.SignatureV2{tipperSigsV2}, feePayerSigV2) + txBuilder.SetSignatures(sigsV2...) + + // Actually sign the data. + signerData := authsigning.SignerData{ + Address: sdk.AccAddress(feePayerPriv.PubKey().Address()).String(), + ChainID: chainID, + AccountNumber: accNum, + Sequence: accSeq, + SignerIndex: 1, + } + feePayerSigV2, err = clienttx.SignWithPrivKey( + signMode, signerData, + txBuilder, feePayerPriv, clientCtx.TxConfig, accSeq) + s.Require().NoError(err) + sigsV2 = append([]signing.SignatureV2{tipperSigsV2}, feePayerSigV2) + err = txBuilder.SetSignatures(sigsV2...) + s.Require().NoError(err) + + return txBuilder +} diff --git a/x/auth/migrations/legacytx/stdtx.go b/x/auth/migrations/legacytx/stdtx.go index 68f3b330eae8..33f7328da5e4 100644 --- a/x/auth/migrations/legacytx/stdtx.go +++ b/x/auth/migrations/legacytx/stdtx.go @@ -6,6 +6,7 @@ import ( cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx" txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -15,6 +16,7 @@ var ( _ sdk.Tx = (*StdTx)(nil) _ sdk.TxWithMemo = (*StdTx)(nil) _ sdk.FeeTx = (*StdTx)(nil) + _ tx.TipTx = (*StdTx)(nil) _ codectypes.UnpackInterfacesMessage = (*StdTx)(nil) _ codectypes.UnpackInterfacesMessage = (*StdSignature)(nil) @@ -232,6 +234,9 @@ func (tx StdTx) FeeGranter() sdk.AccAddress { return nil } +// GetTip always returns nil for StdTx +func (tx StdTx) GetTip() *tx.Tip { return nil } + func (tx StdTx) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { for _, m := range tx.Msgs { err := codectypes.UnpackInterfaces(m, unpacker) diff --git a/x/auth/signing/sig_verifiable_tx.go b/x/auth/signing/sig_verifiable_tx.go index 2d8aeb49db9a..698d2ac918cb 100644 --- a/x/auth/signing/sig_verifiable_tx.go +++ b/x/auth/signing/sig_verifiable_tx.go @@ -3,6 +3,7 @@ package signing import ( cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -22,5 +23,6 @@ type Tx interface { types.TxWithMemo types.FeeTx + tx.TipTx types.TxWithTimeoutHeight } diff --git a/x/auth/testutil/suite.go b/x/auth/testutil/suite.go index b9bcc8a9c609..e1080f067ab4 100644 --- a/x/auth/testutil/suite.go +++ b/x/auth/testutil/suite.go @@ -89,6 +89,7 @@ func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { msigAddr := sdk.AccAddress(multisigPk.Address()) msg2 := testdata.NewTestMsg(msigAddr) err := txBuilder.SetMsgs(msg, msg2) + txBuilder.SetFeeAmount(testdata.NewTestFeeAmount()) s.Require().NoError(err) // check that validation fails diff --git a/x/auth/tx/direct_aux.go b/x/auth/tx/direct_aux.go index 71a3fd5c0426..63aba24918f8 100644 --- a/x/auth/tx/direct_aux.go +++ b/x/auth/tx/direct_aux.go @@ -44,11 +44,24 @@ func (signModeDirectAuxHandler) GetSignBytes( return nil, sdkerrors.ErrInvalidRequest.Wrapf("got empty pubkey for signer #%d in %s handler", data.SignerIndex, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX) } + addr := data.Address + if addr == "" { + return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "got empty address in %s handler", signingtypes.SignMode_SIGN_MODE_DIRECT_AUX) + } + + feePayer := protoTx.FeePayer().String() + + // Fee payer cannot use SIGN_MODE_DIRECT_AUX, because SIGN_MODE_DIRECT_AUX + // does not sign over fees, which would create malleability issues. + if feePayer == data.Address { + return nil, sdkerrors.ErrUnauthorized.Wrapf("fee payer %s cannot sign with %s", feePayer, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX) + } + signDocDirectAux := types.SignDocDirectAux{ BodyBytes: protoTx.getBodyBytes(), ChainId: data.ChainID, AccountNumber: data.AccountNumber, - Sequence: data.Sequence, + Sequence: signerInfo.Sequence, Tip: protoTx.tx.AuthInfo.Tip, PublicKey: signerInfo.PublicKey, } diff --git a/x/auth/tx/direct_aux_test.go b/x/auth/tx/direct_aux_test.go index df7460c5ba78..6bd67024a259 100644 --- a/x/auth/tx/direct_aux_test.go +++ b/x/auth/tx/direct_aux_test.go @@ -17,6 +17,7 @@ import ( func TestDirectAuxHandler(t *testing.T) { privKey, pubkey, addr := testdata.KeyTestPubAddr() + _, feePayerPubKey, feePayerAddr := testdata.KeyTestPubAddr() interfaceRegistry := codectypes.NewInterfaceRegistry() interfaceRegistry.RegisterImplementations((*sdk.Msg)(nil), &testdata.TestMsg{}) marshaler := codec.NewProtoCodec(interfaceRegistry) @@ -24,9 +25,10 @@ func TestDirectAuxHandler(t *testing.T) { txConfig := NewTxConfig(marshaler, []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_DIRECT_AUX}) txBuilder := txConfig.NewTxBuilder() + chainID := "test-chain" memo := "sometestmemo" msgs := []sdk.Msg{testdata.NewTestMsg(addr)} - accSeq := uint64(2) // Arbitrary account sequence + accNum, accSeq := uint64(1), uint64(2) // Arbitrary account number/sequence any, err := codectypes.NewAnyWithValue(pubkey) require.NoError(t, err) @@ -40,26 +42,46 @@ func TestDirectAuxHandler(t *testing.T) { Data: sigData, Sequence: accSeq, } + feePayerSig := signingtypes.SignatureV2{ + PubKey: feePayerPubKey, + Data: sigData, + Sequence: accSeq, + } fee := txtypes.Fee{Amount: sdk.NewCoins(sdk.NewInt64Coin("atom", 150)), GasLimit: 20000} + tip := &txtypes.Tip{Amount: sdk.NewCoins(sdk.NewInt64Coin("tip-token", 10))} err = txBuilder.SetMsgs(msgs...) require.NoError(t, err) txBuilder.SetMemo(memo) txBuilder.SetFeeAmount(fee.Amount) + txBuilder.SetFeePayer(feePayerAddr) txBuilder.SetGasLimit(fee.GasLimit) + txBuilder.SetTip(tip) - err = txBuilder.SetSignatures(sig) + err = txBuilder.SetSignatures(sig, feePayerSig) require.NoError(t, err) signingData := signing.SignerData{ Address: addr.String(), - ChainID: "test-chain", - AccountNumber: 1, + ChainID: chainID, + AccountNumber: accNum, SignerIndex: 0, } + feePayerSigningData := signing.SignerData{ + Address: feePayerAddr.String(), + ChainID: chainID, + AccountNumber: accNum, + SignerIndex: 1, + } modeHandler := signModeDirectAuxHandler{} + + t.Log("verify fee payer cannot use SIGN_MODE_DIRECT_AUX") + _, err = modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, feePayerSigningData, txBuilder.GetTx()) + require.EqualError(t, err, fmt.Sprintf("fee payer %s cannot sign with %s: unauthorized", feePayerAddr.String(), signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)) + + t.Log("verify GetSignBytes with generating sign bytes by marshaling signDocDirectAux") signBytes, err := modeHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_DIRECT_AUX, signingData, txBuilder.GetTx()) require.NoError(t, err) require.NotNil(t, signBytes) @@ -79,12 +101,13 @@ func TestDirectAuxHandler(t *testing.T) { } bodyBytes := marshaler.MustMarshal(txBody) - t.Log("verify GetSignBytes with generating sign bytes by marshaling signDocDirectAux") signDocDirectAux := txtypes.SignDocDirectAux{ - AccountNumber: 1, + AccountNumber: accNum, BodyBytes: bodyBytes, ChainId: "test-chain", PublicKey: any, + Sequence: accSeq, + Tip: tip, } expectedSignBytes, err := signDocDirectAux.Marshal() diff --git a/x/auth/types/expected_keepers.go b/x/auth/types/expected_keepers.go index 0ab3f9c4308e..7f242c63fd3a 100644 --- a/x/auth/types/expected_keepers.go +++ b/x/auth/types/expected_keepers.go @@ -6,5 +6,6 @@ import ( // BankKeeper defines the contract needed for supply related APIs (noalias) type BankKeeper interface { + SendCoins(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error }