Skip to content

Commit

Permalink
Problem: fee deduction not compatible with parallel execution (#447)
Browse files Browse the repository at this point in the history
* Problem: fee deduction not compatible with parallel execution

Solution:
- use virtual send

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix build

* fix test

* cleanup

---------

Signed-off-by: yihuang <[email protected]>
  • Loading branch information
yihuang authored Apr 2, 2024
1 parent 59586a5 commit be00d15
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [#434](https://github.com/crypto-org-chain/ethermint/pull/434) No need gasPrice when patch gasUsed for `eth_getTransactionReceipt`.
* (rpc) [#439](https://github.com/crypto-org-chain/ethermint/pull/439), [#441](https://github.com/crypto-org-chain/ethermint/pull/441) Align trace response for failed tx with go-ethereum.
* (statedb) [#446](https://github.com/crypto-org-chain/ethermint/pull/446) Re-use the cache store implementation with sdk.
* (evm) [#447](https://github.com/crypto-org-chain/ethermint/pull/447) Deduct fee through virtual bank transfer.

### State Machine Breaking

Expand Down
5 changes: 3 additions & 2 deletions app/ante/nativefee.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/types"

evmkeeper "github.com/evmos/ethermint/x/evm/keeper"
evmtypes "github.com/evmos/ethermint/x/evm/types"
)

// DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx.
Expand All @@ -20,12 +21,12 @@ import (
// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator
type DeductFeeDecorator struct {
accountKeeper ante.AccountKeeper
bankKeeper types.BankKeeper
bankKeeper evmtypes.BankKeeper
feegrantKeeper ante.FeegrantKeeper
txFeeChecker ante.TxFeeChecker
}

func NewDeductFeeDecorator(ak ante.AccountKeeper, bk types.BankKeeper, fk ante.FeegrantKeeper, tfc ante.TxFeeChecker) DeductFeeDecorator {
func NewDeductFeeDecorator(ak ante.AccountKeeper, bk evmtypes.BankKeeper, fk ante.FeegrantKeeper, tfc ante.TxFeeChecker) DeductFeeDecorator {
if tfc == nil {
tfc = checkTxFeeWithValidatorMinGasPrices
}
Expand Down
19 changes: 18 additions & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ func NewEthermintApp(

// NOTE: fee market module must go last in order to retrieve the block gas used.
app.ModuleManager.SetOrderEndBlockers(
banktypes.ModuleName,
crisistypes.ModuleName,
govtypes.ModuleName,
stakingtypes.ModuleName,
Expand All @@ -672,7 +673,6 @@ func NewEthermintApp(
ibcexported.ModuleName,
ibctransfertypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
distrtypes.ModuleName,
slashingtypes.ModuleName,
minttypes.ModuleName,
Expand Down Expand Up @@ -1033,6 +1033,23 @@ func (app *EthermintApp) RegisterNodeService(clientCtx client.Context, cfg confi
node.RegisterNodeService(clientCtx, app.GRPCQueryRouter(), cfg)
}

// GetStoreKey is used by unit test
func (app *EthermintApp) GetStoreKey(name string) storetypes.StoreKey {
key, ok := app.keys[name]
if ok {
return key
}
tkey, ok := app.tkeys[name]
if ok {
return tkey
}
mkey, ok := app.memKeys[name]
if ok {
return mkey
}
return app.okeys[name]
}

// RegisterSwaggerAPI registers swagger route with API Server
func RegisterSwaggerAPI(_ client.Context, rtr *mux.Router) {
root, err := fs.Sub(docs.SwaggerUI, "swagger-ui")
Expand Down
27 changes: 27 additions & 0 deletions testutil/base_test_suite.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package testutil

import (
"encoding/binary"
"encoding/json"
"math/big"
"time"

coreheader "cosmossdk.io/core/header"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
abci "github.com/cometbft/cometbft/abci/types"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/cosmos-sdk/baseapp"
Expand All @@ -17,6 +19,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
Expand All @@ -41,6 +44,30 @@ type BaseTestSuite struct {
App *app.EthermintApp
}

func (suite *BaseTestSuite) MintFeeCollectorVirtual(coins sdk.Coins) {
// add some virtual balance to the fee collector for refunding
addVirtualCoins(
suite.Ctx.ObjectStore(suite.App.GetStoreKey(banktypes.ObjectStoreKey)),
suite.Ctx.TxIndex(),
authtypes.NewModuleAddress(authtypes.FeeCollectorName),
coins,
)
}

func addVirtualCoins(store storetypes.ObjKVStore, txIndex int, addr sdk.AccAddress, amt sdk.Coins) {
key := make([]byte, len(addr)+8)
copy(key, addr)
binary.BigEndian.PutUint64(key[len(addr):], uint64(txIndex))

var coins sdk.Coins
value := store.Get(key)
if value != nil {
coins = value.(sdk.Coins)
}
coins = coins.Add(amt...)
store.Set(key, coins)
}

func (suite *BaseTestSuite) SetupTest() {
suite.SetupTestWithCb(suite.T(), nil)
}
Expand Down
7 changes: 6 additions & 1 deletion x/evm/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ func TestHandlerTestSuite(t *testing.T) {
}

func (suite *HandlerTestSuite) SetupTest() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdkmath.NewInt(100000000000000)))

t := suite.T()
suite.SetupTestWithCb(t, func(app *app.EthermintApp, genesis app.GenesisState) app.GenesisState {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdkmath.NewInt(100000000000000)))
b32address := sdk.MustBech32ifyAddressBytes(sdk.GetConfig().GetBech32AccountAddrPrefix(), suite.ConsPubKey.Address().Bytes())
balances := []banktypes.Balance{
{
Expand Down Expand Up @@ -72,6 +73,10 @@ func (suite *HandlerTestSuite) SetupTest() {
genesis[authtypes.ModuleName] = app.AppCodec().MustMarshalJSON(&authGenesis)
return genesis
})

// add some virtual balance to the fee collector for refunding
suite.MintFeeCollectorVirtual(coins)

suite.ethSigner = ethtypes.LatestSignerForChainID(suite.App.EvmKeeper.ChainID())
}

Expand Down
2 changes: 1 addition & 1 deletion x/evm/keeper/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64

// refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees

err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, authtypes.FeeCollectorName, msg.From.Bytes(), refundedCoins)
err := k.bankKeeper.SendCoinsFromModuleToAccountVirtual(ctx, authtypes.FeeCollectorName, msg.From.Bytes(), refundedCoins)
if err != nil {
err = errorsmod.Wrapf(errortypes.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error())
return errorsmod.Wrapf(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String())
Expand Down
9 changes: 7 additions & 2 deletions x/evm/keeper/state_transition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ type StateTransitionTestSuite struct {
}

func (suite *StateTransitionTestSuite) SetupTest() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdkmath.NewInt(int64(params.TxGas)-1)))

t := suite.T()
suite.EVMTestSuiteWithAccountAndQueryClient.SetupTestWithCb(t, func(a *app.EthermintApp, genesis app.GenesisState) app.GenesisState {
suite.SetupTestWithCb(t, func(a *app.EthermintApp, genesis app.GenesisState) app.GenesisState {
feemarketGenesis := feemarkettypes.DefaultGenesisState()
feemarketGenesis.Params.NoBaseFee = true
genesis[feemarkettypes.ModuleName] = a.AppCodec().MustMarshalJSON(feemarketGenesis)
Expand All @@ -57,7 +59,6 @@ func (suite *StateTransitionTestSuite) SetupTest() {
genesis[authtypes.ModuleName] = a.AppCodec().MustMarshalJSON(&authGenesis)
if suite.mintFeeCollector {
// mint some coin to fee collector
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdkmath.NewInt(int64(params.TxGas)-1)))
balances := []banktypes.Balance{
{
Address: suite.App.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName).String(),
Expand All @@ -73,6 +74,10 @@ func (suite *StateTransitionTestSuite) SetupTest() {
}
return genesis
})

if suite.mintFeeCollector {
suite.MintFeeCollectorVirtual(coins)
}
}

func TestStateTransitionTestSuite(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions x/evm/keeper/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,12 @@ func CheckSenderBalance(
}

// DeductFees deducts fees from the given account.
func DeductFees(bankKeeper authtypes.BankKeeper, ctx sdk.Context, acc sdk.AccountI, fees sdk.Coins) error {
func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc sdk.AccountI, fees sdk.Coins) error {
if !fees.IsValid() {
return errorsmod.Wrapf(errortypes.ErrInsufficientFee, "invalid fee amount: %s", fees)
}

err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees)
err := bankKeeper.SendCoinsFromAccountToModuleVirtual(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees)
if err != nil {
return errorsmod.Wrapf(errortypes.ErrInsufficientFunds, err.Error())
}
Expand Down
2 changes: 2 additions & 0 deletions x/evm/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ type BankKeeper interface {
authtypes.BankKeeper
GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin
SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromModuleToAccountVirtual(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromAccountToModuleVirtual(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
MintCoins(ctx context.Context, moduleName string, amt sdk.Coins) error
BurnCoins(ctx context.Context, moduleName string, amt sdk.Coins) error
BlockedAddr(addr sdk.AccAddress) bool
Expand Down

0 comments on commit be00d15

Please sign in to comment.