Skip to content

Commit 2155b9b

Browse files
jhelisonjhelison
authored
Merge pull request #1 from KiiChain/feat/fee-abstraction
Apply the external fee payment to EVM gas refunds logic
2 parents d781e11 + bdff4ca commit 2155b9b

2 files changed

Lines changed: 244 additions & 3 deletions

File tree

‎x/vm/keeper/gas.go‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import (
1515
"github.com/cosmos/evm/x/vm/types"
1616
)
1717

18+
// ContextPaidFeesKey is a key used to store the paid fee in the context
19+
type ContextPaidFeesKey struct{}
20+
1821
// GetEthIntrinsicGas returns the intrinsic gas cost for the transaction
1922
func (k *Keeper) GetEthIntrinsicGas(ctx sdk.Context, msg core.Message, cfg *params.ChainConfig, isContractCreation bool) (uint64, error) {
2023
height := big.NewInt(ctx.BlockHeight())
@@ -32,22 +35,55 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64
3235
// Return EVM tokens for remaining gas, exchanged at the original rate.
3336
remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice())
3437

38+
// Check if gas is zero
39+
if msg.Gas() == 0 {
40+
// If gas is zero, we cannot refund anything, so we return early
41+
return nil
42+
}
43+
3544
switch remaining.Sign() {
3645
case -1:
3746
// negative refund errors
3847
return errorsmod.Wrapf(types.ErrInvalidRefund, "refunded amount value cannot be negative %d", remaining.Int64())
3948
case 1:
40-
// positive amount refund
49+
// Attempt to extract the paid coin from the context
50+
// This is used when fee abstraction is applied into the fee payment
51+
// If no value is found under the context, the original denom is used
52+
if val := ctx.Value(ContextPaidFeesKey{}); val != nil {
53+
// We check if a coin exists under the value and if it's not empty
54+
if paidCoins, ok := val.(sdk.Coins); ok && !paidCoins.IsZero() {
55+
// We know that only a single coin is used for EVM payments
56+
if len(paidCoins) != 1 {
57+
// This should never happen, but if it does, we return an error
58+
return errorsmod.Wrapf(types.ErrInvalidRefund, "expected a single coin for EVM refunds, got %d", len(paidCoins))
59+
}
60+
paidCoin := paidCoins[0]
61+
62+
// Extract the coin information
63+
denom = paidCoin.Denom
64+
amount := paidCoin.Amount.BigInt()
65+
66+
// Calculate the amount to refund
67+
// This is calculated as:
68+
// remaining = amount * leftoverGas / gasUsed
69+
remaining = new(big.Int).Div(
70+
new(big.Int).Mul(amount, new(big.Int).SetUint64(leftoverGas)),
71+
new(big.Int).SetUint64(msg.Gas()),
72+
)
73+
}
74+
}
75+
76+
// Positive amount refund
4177
refundedCoins := sdk.Coins{sdk.NewCoin(denom, sdkmath.NewIntFromBigInt(remaining))}
4278

43-
// refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees
79+
// Refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees
4480
err := k.bankWrapper.SendCoinsFromModuleToAccount(ctx, authtypes.FeeCollectorName, msg.From().Bytes(), refundedCoins)
4581
if err != nil {
4682
err = errorsmod.Wrapf(errortypes.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error())
4783
return errorsmod.Wrapf(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String())
4884
}
4985
default:
50-
// no refund, consume gas and update the tx gas meter
86+
// No refund, consume gas and update the tx gas meter
5187
}
5288

5389
return nil

‎x/vm/keeper/gas_test.go‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
package keeper_test
2+
3+
import (
4+
"math/big"
5+
6+
sdkmath "cosmossdk.io/math"
7+
sdk "github.com/cosmos/cosmos-sdk/types"
8+
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
9+
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
10+
"github.com/cosmos/evm/testutil/integration/os/factory"
11+
"github.com/cosmos/evm/testutil/integration/os/grpc"
12+
testkeyring "github.com/cosmos/evm/testutil/integration/os/keyring"
13+
erc20mocks "github.com/cosmos/evm/x/erc20/types/mocks"
14+
"github.com/cosmos/evm/x/vm/keeper"
15+
"github.com/cosmos/evm/x/vm/types"
16+
"go.uber.org/mock/gomock"
17+
)
18+
19+
const (
20+
DefaultCoreMsgGasUsage = 21000
21+
DefaultGasPrice = 120000
22+
)
23+
24+
// TestGasRefundGas tests the refund gas exclusively without going though the state transition
25+
// The gas part on the name refers to the file name to not generate a duplicated test name
26+
func (suite *KeeperTestSuite) TestGasRefundGas() {
27+
// Create a txFactory
28+
grpcHandler := grpc.NewIntegrationHandler(suite.network)
29+
txFactory := factory.New(suite.network, grpcHandler)
30+
31+
// Create a core message to use for the test
32+
keyring := testkeyring.New(2)
33+
sender := keyring.GetKey(0)
34+
recipient := keyring.GetAddr(1)
35+
coreMsg, err := txFactory.GenerateGethCoreMsg(
36+
sender.Priv,
37+
types.EvmTxArgs{
38+
To: &recipient,
39+
Amount: big.NewInt(100),
40+
GasPrice: big.NewInt(120000),
41+
},
42+
)
43+
suite.Require().NoError(err)
44+
45+
// Produce all the test cases
46+
testCases := []struct {
47+
name string
48+
leftoverGas uint64 // The coreMsg always uses 21000 gas limit
49+
malleate func(sdk.Context) sdk.Context
50+
expectedRefund sdk.Coins
51+
errContains string
52+
}{
53+
{
54+
name: "Refund the full value as no gas was used",
55+
leftoverGas: DefaultCoreMsgGasUsage,
56+
expectedRefund: sdk.NewCoins(
57+
sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(DefaultCoreMsgGasUsage*DefaultGasPrice)),
58+
),
59+
},
60+
{
61+
name: "Refund half the value as half gas was used",
62+
leftoverGas: DefaultCoreMsgGasUsage / 2,
63+
expectedRefund: sdk.NewCoins(
64+
sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt((DefaultCoreMsgGasUsage*DefaultGasPrice)/2)),
65+
),
66+
},
67+
{
68+
name: "No refund as no gas was left over used",
69+
leftoverGas: 0,
70+
expectedRefund: sdk.NewCoins(
71+
sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(0)),
72+
),
73+
},
74+
{
75+
name: "Refund with context fees, refunding the full value",
76+
leftoverGas: DefaultCoreMsgGasUsage,
77+
malleate: func(ctx sdk.Context) sdk.Context {
78+
// Set the fee abstraction paid fee key with a single coin
79+
return ctx.WithValue(
80+
keeper.ContextPaidFeesKey{},
81+
sdk.NewCoins(
82+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)),
83+
),
84+
)
85+
},
86+
expectedRefund: sdk.NewCoins(
87+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)),
88+
),
89+
},
90+
{
91+
name: "Refund with context fees, refunding the half the value",
92+
leftoverGas: DefaultCoreMsgGasUsage / 2,
93+
malleate: func(ctx sdk.Context) sdk.Context {
94+
// Set the fee abstraction paid fee key with a single coin
95+
return ctx.WithValue(
96+
keeper.ContextPaidFeesKey{},
97+
sdk.NewCoins(
98+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)),
99+
),
100+
)
101+
},
102+
expectedRefund: sdk.NewCoins(
103+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000/2)),
104+
),
105+
},
106+
{
107+
name: "Refund with context fees, no refund",
108+
leftoverGas: 0,
109+
malleate: func(ctx sdk.Context) sdk.Context {
110+
// Set the fee abstraction paid fee key with a single coin
111+
return ctx.WithValue(
112+
keeper.ContextPaidFeesKey{},
113+
sdk.NewCoins(
114+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)),
115+
),
116+
)
117+
},
118+
expectedRefund: sdk.NewCoins(
119+
sdk.NewCoin("acoin", sdkmath.NewInt(0)),
120+
),
121+
},
122+
{
123+
name: "Error - More than one coin being passed",
124+
leftoverGas: DefaultCoreMsgGasUsage,
125+
malleate: func(ctx sdk.Context) sdk.Context {
126+
// Set the fee abstraction paid fee key with a single coin
127+
return ctx.WithValue(
128+
keeper.ContextPaidFeesKey{},
129+
sdk.NewCoins(
130+
sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)),
131+
sdk.NewCoin("atwo", sdkmath.NewInt(750_000_000)),
132+
),
133+
)
134+
},
135+
expectedRefund: sdk.NewCoins(
136+
sdk.NewCoin("acoin", sdkmath.NewInt(0)), // We say as zero to skip the mock bank check
137+
),
138+
errContains: "expected a single coin for EVM refunds, got 2",
139+
},
140+
}
141+
142+
// Iterate though the test cases
143+
for _, tc := range testCases {
144+
suite.Run(tc.name, func() {
145+
// Generate a cached context to not leak data between tests
146+
ctx, _ := suite.network.GetContext().CacheContext()
147+
148+
// Create a new controller for the mock
149+
ctrl := gomock.NewController(suite.T())
150+
defer ctrl.Finish()
151+
152+
// Apply the malleate function to the context
153+
if tc.malleate != nil {
154+
ctx = tc.malleate(ctx)
155+
}
156+
157+
// Create a new mock bank keeper
158+
mockBankKeeper := erc20mocks.NewMockBankKeeper(ctrl)
159+
160+
// Apply the expect, but only if expected refund is not zero
161+
if !tc.expectedRefund.IsZero() {
162+
mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
163+
DoAndReturn(func(ctx sdk.Context, senderModule string, recipient sdk.AccAddress, coins sdk.Coins) error {
164+
if !coins.Equal(tc.expectedRefund) {
165+
suite.T().Errorf("expected %s, got %s", tc.expectedRefund, coins)
166+
}
167+
168+
return nil
169+
})
170+
}
171+
172+
// Initialize a new EVM keeper with the mock bank keeper
173+
// We need to redo this every time, since we will apply the mocked bank keeper at this step
174+
evmKeeper := keeper.NewKeeper(
175+
suite.network.App.AppCodec(),
176+
suite.network.App.GetKey(types.StoreKey),
177+
suite.network.App.GetTKey(types.StoreKey),
178+
authtypes.NewModuleAddress(govtypes.ModuleName),
179+
suite.network.App.AccountKeeper,
180+
mockBankKeeper,
181+
suite.network.App.StakingKeeper,
182+
suite.network.App.FeeMarketKeeper,
183+
suite.network.App.Erc20Keeper,
184+
"",
185+
suite.network.App.GetSubspace(types.ModuleName),
186+
)
187+
188+
// Call the msg, not further checks are needed, all balance checks are done in the mock
189+
err := evmKeeper.RefundGas(
190+
ctx,
191+
coreMsg,
192+
tc.leftoverGas,
193+
suite.network.GetBaseDenom(),
194+
)
195+
196+
// Check the error
197+
if tc.errContains != "" {
198+
suite.Require().ErrorContains(err, tc.errContains, "RefundGas should return an error")
199+
} else {
200+
suite.Require().NoError(err, "RefundGas should not return an error")
201+
}
202+
})
203+
}
204+
205+
}

0 commit comments

Comments
 (0)