Skip to content

Commit

Permalink
refactor!: remove tips (#17787)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored and julienrbrt committed Sep 20, 2023
1 parent b1208ef commit e982c91
Show file tree
Hide file tree
Showing 54 changed files with 337 additions and 1,153 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/auth) [#17787](https://github.com/cosmos/cosmos-sdk/pull/17787) Remove Tip functionality.
* (types) `module.EndBlockAppModule` has been replaced by Core API `appmodule.HasEndBlocker` or `module.HasABCIEndBlock` when needing validator updates.
* (types) `module.BeginBlockAppModule` has been replaced by Core API `appmodule.HasBeginBlocker`.
* (types) [#17358](https://github.com/cosmos/cosmos-sdk/pull/17358) Remove deprecated `sdk.Handler`, use `baseapp.MsgServiceHandler` instead.
Expand Down
3 changes: 1 addition & 2 deletions api/cosmos/tx/signing/v1beta1/signing.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

240 changes: 122 additions & 118 deletions api/cosmos/tx/v1beta1/tx.pulsar.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type BaseApp struct {

mempool mempool.Mempool // application side mempool
anteHandler sdk.AnteHandler // ante handler for fee and auth
postHandler sdk.PostHandler // post handler, optional, e.g. for tips
postHandler sdk.PostHandler // post handler, optional

initChainer sdk.InitChainer // ABCI InitChain handler
preBlocker sdk.PreBlocker // logic to run before BeginBlocker
Expand Down
21 changes: 2 additions & 19 deletions client/tx/aux_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/cosmos/gogoproto/proto"
"google.golang.org/protobuf/types/known/anypb"

basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1"
txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
txsigning "cosmossdk.io/x/tx/signing"
"cosmossdk.io/x/tx/signing/aminojson"
Expand Down Expand Up @@ -130,12 +129,6 @@ func (b *AuxTxBuilder) SetSignMode(mode signing.SignMode) error {
return nil
}

// 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 in the AuxSignerData.
func (b *AuxTxBuilder) SetSignature(sig []byte) {
b.checkEmptyFields()
Expand Down Expand Up @@ -214,11 +207,7 @@ func (b *AuxTxBuilder) GetSignBytes() ([]byte, error) {
handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{
FileResolver: proto.HybridResolver,
})
legacyTip := b.auxSignerData.SignDoc.Tip
tip := &txv1beta1.Tip{
Amount: make([]*basev1beta1.Coin, len(legacyTip.Amount)),
Tipper: legacyTip.Tipper,
}

auxBody := &txv1beta1.TxBody{
Messages: body.Messages,
Memo: body.Memo,
Expand All @@ -230,12 +219,7 @@ func (b *AuxTxBuilder) GetSignBytes() ([]byte, error) {
ExtensionOptions: nil,
NonCriticalExtensionOptions: nil,
}
for i, coin := range legacyTip.Amount {
tip.Amount[i] = &basev1beta1.Coin{
Denom: coin.Denom,
Amount: coin.Amount.String(),
}
}

signBz, err = handler.GetSignBytes(
context.Background(),
txsigning.SignerData{
Expand All @@ -254,7 +238,6 @@ func (b *AuxTxBuilder) GetSignBytes() ([]byte, error) {
// over empty fees.
// ref: https://github.com/cosmos/cosmos-sdk/pull/10348
Fee: &txv1beta1.Fee{},
Tip: tip,
},
},
)
Expand Down
20 changes: 0 additions & 20 deletions client/tx/aux_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,25 +84,11 @@ func TestAuxTxBuilder(t *testing.T) {
},
true, "got unknown sign mode SIGN_MODE_UNSPECIFIED",
},
{
"GetSignBytes tipper should not be nil (if tip is set)",
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(&typestx.Tip{})
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
return err
},
true, "tipper cannot be empty",
},
{
"GetSignBytes works for DIRECT_AUX",
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
Expand All @@ -115,7 +101,6 @@ func TestAuxTxBuilder(t *testing.T) {
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
Expand All @@ -131,7 +116,6 @@ func TestAuxTxBuilder(t *testing.T) {
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
b.SetAddress(addr1.String())
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))

Expand All @@ -153,7 +137,6 @@ func TestAuxTxBuilder(t *testing.T) {
b.SetChainID(chainID)
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
b.SetAddress(addr1.String())
err := b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX)
require.NoError(t, err)
Expand All @@ -176,7 +159,6 @@ func TestAuxTxBuilder(t *testing.T) {
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
b.SetAddress(addr1.String())
err := b.SetSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
require.NoError(t, err)
Expand All @@ -196,7 +178,6 @@ func TestAuxTxBuilder(t *testing.T) {
b.SetChainID(chainID)
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetTip(tip)
b.SetAddress(addr1.String())
err := b.SetSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
require.NoError(t, err)
Expand Down Expand Up @@ -250,7 +231,6 @@ func checkCorrectData(t *testing.T, cdc codec.Codec, auxSignerData typestx.AuxSi
require.Equal(t, chainID, auxSignerData.SignDoc.ChainId)
require.Equal(t, msgAny, body.GetMessages()[0])
require.Equal(t, pkAny, auxSignerData.SignDoc.PublicKey)
require.Equal(t, tip, auxSignerData.SignDoc.Tip)
require.Equal(t, signMode, auxSignerData.Mode)
require.Equal(t, rawSig, auxSignerData.Sig)
}
21 changes: 0 additions & 21 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
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"
)

Expand All @@ -38,7 +37,6 @@ type Factory struct {
generateOnly bool
memo string
fees sdk.Coins
tip *tx.Tip
feeGranter sdk.AccAddress
feePayer sdk.AccAddress
gasPrices sdk.DecCoins
Expand Down Expand Up @@ -105,11 +103,6 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, e
feesStr, _ := flagSet.GetString(flags.FlagFees)
f = f.WithFees(feesStr)

tipsStr, _ := flagSet.GetString(flags.FlagTip)
// Add tips to factory. The tipper is necessarily the Msg signer, i.e.
// the from address.
f = f.WithTips(tipsStr, clientCtx.FromAddress.String())

gasPricesStr, _ := flagSet.GetString(flags.FlagGasPrices)
f = f.WithGasPrices(gasPricesStr)

Expand Down Expand Up @@ -169,20 +162,6 @@ func (f Factory) WithFees(fees string) Factory {
return f
}

// WithTips returns a copy of the Factory with an updated tip.
func (f Factory) WithTips(tip, tipper string) Factory {
parsedTips, err := sdk.ParseCoinsNormalized(tip)
if err != nil {
panic(err)
}

f.tip = &tx.Tip{
Tipper: tipper,
Amount: parsedTips,
}
return f
}

// WithGasPrices returns a copy of the Factory with updated gas prices.
func (f Factory) WithGasPrices(gasPrices string) Factory {
parsedGasPrices, err := sdk.ParseDecCoins(gasPrices)
Expand Down
2 changes: 0 additions & 2 deletions client/tx/legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package tx_test
import (
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/types"
typestx "github.com/cosmos/cosmos-sdk/types/tx"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
)

Expand All @@ -19,5 +18,4 @@ var (
msg1 = banktypes.NewMsgSend(addr1, addr2, types.NewCoins(types.NewInt64Coin("wack", 2)))

chainID = "test-chain"
tip = &typestx.Tip{Tipper: addr1.String(), Amount: testdata.NewTestFeeAmount()}
)
7 changes: 0 additions & 7 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,6 @@ func makeAuxSignerData(clientCtx client.Context, f Factory, msgs ...sdk.Msg) (tx
return tx.AuxSignerData{}, err
}

if f.tip != nil {
if _, err := sdk.AccAddressFromBech32(f.tip.Tipper); err != nil {
return tx.AuxSignerData{}, sdkerrors.ErrInvalidAddress.Wrap("tipper must be a bech32 address")
}
b.SetTip(f.tip)
}

err = b.SetSignMode(f.SignMode())
if err != nil {
return tx.AuxSignerData{}, err
Expand Down
15 changes: 0 additions & 15 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -388,22 +386,12 @@ func TestPreprocessHook(t *testing.T) {
extAny, err := codectypes.NewAnyWithValue(extVal)
requireT.NoError(err)

coin := sdk.Coin{
Denom: "atom",
Amount: math.NewInt(20),
}
newTip := &txtypes.Tip{
Amount: sdk.Coins{coin},
Tipper: "galaxy",
}

preprocessHook := client.PreprocessTxFn(func(chainID string, key keyring.KeyType, tx client.TxBuilder) error {
extensionBuilder, ok := tx.(authtx.ExtensionOptionsTxBuilder)
requireT.True(ok)

// Set new extension and tip
extensionBuilder.SetExtensionOptions(extAny)
tx.SetTip(newTip)

return nil
})
Expand Down Expand Up @@ -435,9 +423,6 @@ func TestPreprocessHook(t *testing.T) {

opt := hasExtOptsTx.GetExtensionOptions()[0]
requireT.Equal(opt, extAny)

tip := txb.GetTx().GetTip()
requireT.Equal(tip, newTip)
}

func testSigners(require *require.Assertions, tr signing.Tx, pks ...cryptotypes.PubKey) []signingtypes.SignatureV2 {
Expand Down
1 change: 0 additions & 1 deletion client/tx_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ type (
SetFeeAmount(amount sdk.Coins)
SetFeePayer(feePayer sdk.AccAddress)
SetGasLimit(limit uint64)
SetTip(tip *tx.Tip)
SetTimeoutHeight(height uint64)
SetFeeGranter(feeGranter sdk.AccAddress)
AddAuxSignerData(tx.AuxSignerData) error
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/develop/advanced/00-baseapp.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,8 @@ First, it retrieves the `sdk.Msg`'s fully-qualified type name, by checking the `

`PostHandler` is similar to `AnteHandler`, but it, as the name suggests, executes custom post tx processing logic after [`RunMsgs`](#runmsgs) is called. `PostHandler` receives the `Result` of the the `RunMsgs` in order to enable this customizable behavior.

Like `AnteHandler`s, `PostHandler`s are theoretically optional, one use case for `PostHandler`s is transaction tips (enabled by default in simapp).
Like `AnteHandler`s, `PostHandler`s are theoretically optional.

Other use cases like unused gas refund can also be enabled by `PostHandler`s.

```go reference
Expand Down
1 change: 0 additions & 1 deletion docs/docs/develop/advanced/01-transactions.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/proto/cosmos/tx/v1beta
The use case is a multi-signer transaction, where one of the signers is appointed to gather all signatures, broadcast the signature and pay for fees, and the others only care about the transaction body. This generally allows for a better multi-signing UX. If Alice, Bob and Charlie are part of a 3-signer transaction, then Alice and Bob can both use `SIGN_MODE_DIRECT_AUX` to sign over the `TxBody` and their own signer info (no need an additional step to gather other signers' ones, like in `SIGN_MODE_DIRECT`), without specifying a fee in their SignDoc. Charlie can then gather both signatures from Alice and Bob, and
create the final transaction by appending a fee. Note that the fee payer of the transaction (in our case Charlie) must sign over the fees, so must use `SIGN_MODE_DIRECT` or `SIGN_MODE_LEGACY_AMINO_JSON`.

A concrete use case is implemented in [transaction tips](./14-tips.md): the tipper may use `SIGN_MODE_DIRECT_AUX` to specify a tip in the transaction, without signing over the actual transaction fees. Then, the fee payer appends fees inside the tipper's desired `TxBody`, and as an exchange for paying the fees and broadcasting the transaction, receives the tipper's transaction tips as payment.

#### `SIGN_MODE_TEXTUAL`

Expand Down
Loading

0 comments on commit e982c91

Please sign in to comment.