Skip to content

Commit

Permalink
feat: configurable fee collector for DeductFeeDecorator (#407)
Browse files Browse the repository at this point in the history
* feat: configurable fee collector for DeductFeeDecorator

* docs: update changelog with PR number

* test: ensure increment of alt account, comments
  • Loading branch information
JimLarson committed Mar 8, 2024
1 parent f42d869 commit a1ec2f2
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 13 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG-Agoric.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,22 @@ Ref: https://keepachangelog.com/en/1.0.0/

# Changelog (Agoric fork)

## Unreleased

### 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

## `v0.46.16-alpha.agoric.2.1` - 2024-03-08

### Improvements

* (auth) #??? Configurable fee collector module account in DeductFeeDecorator. (backport #407)

## `v0.46.16-alpha.agoric.2` - 2024-02-08

* Agoric/agoric-sdk#8871 Have `tx gov submit-proposal` accept either new or legacy syntax
Expand Down
34 changes: 21 additions & 13 deletions x/auth/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand All @@ -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())
}
Expand Down
46 changes: 46 additions & 0 deletions x/auth/ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

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

0 comments on commit a1ec2f2

Please sign in to comment.