diff --git a/CHANGELOG-Agoric.md b/CHANGELOG-Agoric.md index 62746480090b..67158b64ae73 100644 --- a/CHANGELOG-Agoric.md +++ b/CHANGELOG-Agoric.md @@ -37,7 +37,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Unreleased -### API Breaking Changes +### Improvements + +* (auth) [#407](https://github.com/agoric-labs/cosmos-sdk/pull/407) Configurable fee collector module account in DeductFeeDecorator. + +### API Breaking * (auth, bank) Agoric/agoric-sdk#8989 Remove deprecated lien support diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 4ab9dda9ee1a..947ceb9aa880 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -17,22 +17,30 @@ type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // Call next AnteHandler if fees successfully deducted // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type DeductFeeDecorator struct { - accountKeeper AccountKeeper - bankKeeper types.BankKeeper - feegrantKeeper FeegrantKeeper - txFeeChecker TxFeeChecker + accountKeeper AccountKeeper + bankKeeper types.BankKeeper + feegrantKeeper FeegrantKeeper + txFeeChecker TxFeeChecker + feeCollectorName string } func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { + return NewDeductFeeDecoratorWithName(ak, bk, fk, tfc, types.FeeCollectorName) +} + +// NewDeductFeeDecoratorWithName returns a DeductFeeDecorator using a custom fee collector module account. +// Agoric note: for collecting fees in the reserve account. +func NewDeductFeeDecoratorWithName(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker, feeCollectorName string) DeductFeeDecorator { if tfc == nil { tfc = checkTxFeeWithValidatorMinGasPrices } return DeductFeeDecorator{ - accountKeeper: ak, - bankKeeper: bk, - feegrantKeeper: fk, - txFeeChecker: tfc, + accountKeeper: ak, + bankKeeper: bk, + feegrantKeeper: fk, + txFeeChecker: tfc, + feeCollectorName: feeCollectorName, } } @@ -73,8 +81,8 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { - return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) + if addr := dfd.accountKeeper.GetModuleAddress(dfd.feeCollectorName); addr == nil { + return fmt.Errorf("fee collector module account (%s) has not been set", dfd.feeCollectorName) } feePayer := feeTx.FeePayer() @@ -103,7 +111,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 := DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee, dfd.feeCollectorName) if err != nil { return err } @@ -122,12 +130,12 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } // DeductFees deducts fees from the given account. -func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error { +func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins, feeCollectorName string) error { if !fees.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), feeCollectorName, fees) if err != nil { return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index c63706137e0a..5223c4d9d8b8 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -5,6 +5,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/bank/testutil" ) @@ -146,3 +147,48 @@ func (s *AnteTestSuite) TestDeductFees() { s.Require().Nil(err, "Tx errored after account has been set with sufficient funds") } + +func (s *AnteTestSuite) TestDeductFees_WithName() { + s.SetupTest(false) // setup + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + s.Require().NoError(s.txBuilder.SetMsgs(msg)) + s.txBuilder.SetFeeAmount(feeAmount) + s.txBuilder.SetGasLimit(gasLimit) + + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx, err := s.CreateTestTx(privs, accNums, accSeqs, s.ctx.ChainID()) + s.Require().NoError(err) + + // Set transacting account with sufficient funds + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr1) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(200))) + err = testutil.FundAccount(s.app.BankKeeper, s.ctx, addr1, coins) + s.Require().NoError(err) + + feeCollectorAcc := s.app.AccountKeeper.GetModuleAccount(s.ctx, types.FeeCollectorName) + // pick a simapp module account + altCollectorName := "distribution" + altCollectorAcc := s.app.AccountKeeper.GetModuleAccount(s.ctx, altCollectorName) + s.Require().True(s.app.BankKeeper.GetAllBalances(s.ctx, feeCollectorAcc.GetAddress()).Empty()) + altBalance := s.app.BankKeeper.GetAllBalances(s.ctx, altCollectorAcc.GetAddress()) + + // Run the transaction through a handler chain that deducts fees into altCollectorAcc. + dfd := ante.NewDeductFeeDecoratorWithName(s.app.AccountKeeper, s.app.BankKeeper, nil, nil, altCollectorName) + antehandler := sdk.ChainAnteDecorators(dfd) + _, err = antehandler(s.ctx, tx, false) + s.Require().NoError(err) + + s.Require().True(s.app.BankKeeper.GetAllBalances(s.ctx, feeCollectorAcc.GetAddress()).Empty()) + newAltBalance := s.app.BankKeeper.GetAllBalances(s.ctx, altCollectorAcc.GetAddress()) + s.Require().True(newAltBalance.IsAllGTE(altBalance)) + s.Require().False(newAltBalance.IsEqual(altBalance)) +}