Skip to content

Commit

Permalink
feat: Add fee.{payer,granter} and tip fields to StdSignDoc (#10348)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people authored Oct 22, 2021
1 parent 994219a commit 956d8b4
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 39 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions types/tx/tips.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion x/auth/migrations/legacytx/amino_signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion x/auth/migrations/legacytx/amino_signing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
15 changes: 13 additions & 2 deletions x/auth/migrations/legacytx/stdsign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion x/auth/migrations/legacytx/stdsignmsg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions x/auth/migrations/legacytx/stdtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down
63 changes: 56 additions & 7 deletions x/auth/migrations/legacytx/stdtx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

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

Expand Down
4 changes: 2 additions & 2 deletions x/auth/signing/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions x/auth/tx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/tx/direct_aux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
28 changes: 26 additions & 2 deletions x/auth/tx/legacy_amino_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 956d8b4

Please sign in to comment.