Skip to content

Commit

Permalink
feat: Add Tips middleware (cosmos#10208)
Browse files Browse the repository at this point in the history
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

R4R

Closes: cosmos#9912 

This PR introduces 1 new middleware:
- `TipsMiddleware`: transfer tip from tipper to feePayer when relevant.

It also makes sure in the DIRECT_AUX sign mode handler that the fee payer cannot use that sign mode.

Depends on:
- [x]  cosmos#10028 
- [x] cosmos#10268 
- [x] cosmos#10322 
- [x] cosmos#10346 

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

---

### 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)
  • Loading branch information
amaury1093 authored and blewater committed Dec 8, 2021
1 parent 3f988c4 commit b73fbc0
Show file tree
Hide file tree
Showing 15 changed files with 402 additions and 32 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion client/tx_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
17 changes: 17 additions & 0 deletions types/tx/aux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions types/tx/aux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
})
}
Expand Down
1 change: 1 addition & 0 deletions x/auth/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
24 changes: 13 additions & 11 deletions x/auth/middleware/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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++ {
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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()
Expand Down
22 changes: 10 additions & 12 deletions x/auth/middleware/testutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
94 changes: 94 additions & 0 deletions x/auth/middleware/tips.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading

0 comments on commit b73fbc0

Please sign in to comment.