diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e9bae8473af..3142a9b79f90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/auth/ante) [#26813](https://github.com/cosmos/cosmos-sdk/pull/26813) `DeductFeeDecorator` now deducts fees to the recipient module it was configured with. Previously it used the package-level `FeeRecipientModule`, which every `NewDeductFeeDecorator` call reset to `fee_collector`, so building a second ante chain silently redirected the fees of a decorator configured via `WithFeeRecipientModule`. * (client/tx) [#26759](https://github.com/cosmos/cosmos-sdk/issues/26759) Populate the multisig bit array in simulation txs so `--gas auto` works for multisig senders. * (blockstm) [#26772](https://github.com/cosmos/cosmos-sdk/pull/26772) Panic with a descriptive error when accessing an unregistered store instead of silently using store index zero. * (x/genutil) [#26741](https://github.com/cosmos/cosmos-sdk/issues/26741) Preserve vote extension enable height when exporting genesis state. diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 64e6abe19ac6..362750859764 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -11,11 +11,12 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// FeeRecipientModule holds the module name that receives deducted tx fees. -// It is set by NewDeductFeeDecorator (default: fee_collector) and updated by -// WithFeeRecipientModule. Other modules can read this to verify that fees are -// being routed to the expected destination. -var FeeRecipientModule string +// FeeRecipientModule holds the module name that receives deducted tx fees +// (default: fee_collector). It is updated by WithFeeRecipientModule and used +// by DeductFees. Other modules can read this to verify that fees are being +// routed to the expected destination. DeductFeeDecorator itself always +// deducts to the recipient it was configured with. +var FeeRecipientModule = types.FeeCollectorName // TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, // the effective fee should be deducted later, and the priority should be returned in abci response. @@ -38,8 +39,6 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee tfc = checkTxFeeWithValidatorMinGasPrices } - FeeRecipientModule = types.FeeCollectorName - return DeductFeeDecorator{ accountKeeper: ak, bankKeeper: bk, @@ -130,7 +129,7 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) + err := deductFeesToModule(dfd.bankKeeper, ctx, deductFeesFromAcc, fee, dfd.feeRecipientModule) if err != nil { return err } @@ -151,11 +150,17 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee // DeductFees deducts fees from the given account and sends them to the // module configured via FeeRecipientModule. func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc sdk.AccountI, fees sdk.Coins) error { + return deductFeesToModule(bankKeeper, ctx, acc, fees, FeeRecipientModule) +} + +// deductFeesToModule deducts fees from the given account and sends them to +// the given module account. +func deductFeesToModule(bankKeeper types.BankKeeper, ctx sdk.Context, acc sdk.AccountI, fees sdk.Coins, recipientModule string) error { if !fees.IsValid() { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), FeeRecipientModule, fees) + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), recipientModule, fees) if err != nil { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, "%s", err.Error()) } diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 0a577fb66acf..729f8302bcc9 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -196,3 +196,28 @@ func TestDeductFees_WithFeeRecipientModule(t *testing.T) { }) } } + +// A decorator configured with a custom fee recipient must keep using it even +// when another DeductFeeDecorator is constructed afterwards. +func TestDeductFees_RecipientNotClobberedByLaterDecorator(t *testing.T) { + s := SetupTestSuite(t, false) + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + accs := s.CreateTestAccounts(1) + + msg := testdata.NewTestMsg(accs[0].acc.GetAddress()) + feeAmount := testdata.NewTestFeeAmount() + require.NoError(t, s.txBuilder.SetMsgs(msg)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(testdata.NewTestGasLimit()) + tx, err := s.CreateTestTx(s.ctx, []cryptotypes.PrivKey{accs[0].priv}, []uint64{0}, []uint64{0}, s.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT) + require.NoError(t, err) + + custom := ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, nil, nil).WithFeeRecipientModule("mint") + // e.g. a second ante chain built by the app + _ = ante.NewDeductFeeDecorator(s.accountKeeper, s.bankKeeper, nil, nil) + + s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), accs[0].acc.GetAddress(), "mint", feeAmount).Return(nil) + + _, err = sdk.ChainAnteDecorators(custom)(s.ctx, tx, false) + require.NoError(t, err) +}