Skip to content

Commit a48de71

Browse files
authored
fix: validate decimals before conversion to prevent panic in GetBaseFee (#828)
* fix: validate decimals before conversion to prevent panic in GetBaseFee * when coininfo avoid panic on empty coininfo for old ctx * add test * doc
1 parent 65900e1 commit a48de71

File tree

3 files changed

+59
-14
lines changed

3 files changed

+59
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
- [\#800](https://github.com/cosmos/evm/pull/800) Fix denom exponent validation in virtual fee deduct in vm module.
3131
- [\#817](https://github.com/cosmos/evm/pull/817) Align GetCoinbaseAddress to handle empty proposer address in contexts like CheckTx where proposer doesn't exist.
3232
- [\#816](https://github.com/cosmos/evm/pull/816) Avoid nil pointer when RPC requests execute before evmCoinInfo initialization in PreBlock with defaultEvmCoinInfo fallback.
33+
- [\#828](https://github.com/cosmos/evm/pull/828) Validate decimals before conversion to prevent panic when coininfo is missing in historical queries.
3334

3435
## v0.5.0
3536

x/vm/wrappers/feemarket.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,21 @@ func NewFeeMarketWrapper(
3232
// GetBaseFee returns the base fee converted to 18 decimals.
3333
func (w FeeMarketWrapper) GetBaseFee(ctx sdk.Context, decimals types.Decimals) *big.Int {
3434
baseFee := w.FeeMarketKeeper.GetBaseFee(ctx)
35-
if baseFee.IsNil() {
36-
return nil
35+
if baseFee.IsNil() || baseFee.BigInt() == nil {
36+
return big.NewInt(0)
3737
}
38-
39-
return baseFee.MulInt(decimals.ConversionFactor()).TruncateInt().BigInt()
38+
if err := decimals.Validate(); err != nil {
39+
return big.NewInt(0)
40+
}
41+
conv := decimals.ConversionFactor()
42+
if conv.BigInt() == nil {
43+
return big.NewInt(0)
44+
}
45+
converted := baseFee.MulInt(conv).TruncateInt().BigInt()
46+
if converted == nil {
47+
return big.NewInt(0)
48+
}
49+
return converted
4050
}
4151

4252
// CalculateBaseFee returns the calculated base fee converted to 18 decimals.

x/vm/wrappers/feemarket_test.go

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestGetBaseFee(t *testing.T) {
4848
{
4949
name: "success - nil base fee",
5050
coinInfo: testconstants.ExampleChainCoinInfo[testconstants.SixDecimalsChainID],
51-
expResult: nil,
51+
expResult: big.NewInt(0),
5252
mockSetup: func(mfk *testutil.MockFeeMarketKeeper) {
5353
mfk.EXPECT().
5454
GetBaseFee(gomock.Any()).
@@ -85,24 +85,58 @@ func TestGetBaseFee(t *testing.T) {
8585
Return(sdkmath.LegacyNewDecWithPrec(1, 13)) // multiplied by 1e12 is still less than 1
8686
},
8787
},
88+
{
89+
name: "defensive - zero decimals returns zero without panic",
90+
coinInfo: evmtypes.EvmCoinInfo{
91+
Denom: "aevmos",
92+
ExtendedDenom: "aevmos",
93+
DisplayDenom: "evmos",
94+
Decimals: 0, // invalid/uninitialized
95+
},
96+
expResult: big.NewInt(0),
97+
mockSetup: func(mfk *testutil.MockFeeMarketKeeper) {
98+
mfk.EXPECT().
99+
GetBaseFee(gomock.Any()).
100+
Return(sdkmath.LegacyNewDec(1e18))
101+
},
102+
},
103+
{
104+
name: "defensive - invalid decimals (19) returns zero without panic",
105+
coinInfo: evmtypes.EvmCoinInfo{
106+
Denom: "aevmos",
107+
ExtendedDenom: "aevmos",
108+
DisplayDenom: "evmos",
109+
Decimals: 19, // exceeds max supported
110+
},
111+
expResult: big.NewInt(0),
112+
mockSetup: func(mfk *testutil.MockFeeMarketKeeper) {
113+
mfk.EXPECT().
114+
GetBaseFee(gomock.Any()).
115+
Return(sdkmath.LegacyNewDec(1e18))
116+
},
117+
},
88118
}
89119

90120
for _, tc := range testCases {
91121
t.Run(tc.name, func(t *testing.T) {
92-
// Setup EVM configurator to have access to the EVM coin info.
93-
configurator := evmtypes.NewEVMConfigurator()
94-
configurator.ResetTestConfig()
95-
err := configurator.WithEVMCoinInfo(tc.coinInfo).Configure()
96-
require.NoError(t, err, "failed to configure EVMConfigurator")
97-
98122
ctrl := gomock.NewController(t)
99123
mockFeeMarketKeeper := testutil.NewMockFeeMarketKeeper(ctrl)
100124
tc.mockSetup(mockFeeMarketKeeper)
101125

102126
feeMarketWrapper := wrappers.NewFeeMarketWrapper(mockFeeMarketKeeper)
103-
result := feeMarketWrapper.GetBaseFee(sdk.Context{}, evmtypes.Decimals(tc.coinInfo.Decimals))
104-
105-
require.Equal(t, tc.expResult, result)
127+
// skip EVMConfigurator for defensive cases
128+
if tc.coinInfo.Decimals == 0 || tc.coinInfo.Decimals > 18 {
129+
result := feeMarketWrapper.GetBaseFee(sdk.Context{}, evmtypes.Decimals(tc.coinInfo.Decimals))
130+
require.Equal(t, tc.expResult, result)
131+
} else {
132+
// Setup EVM configurator to have access to the EVM coin info.
133+
configurator := evmtypes.NewEVMConfigurator()
134+
configurator.ResetTestConfig()
135+
err := configurator.WithEVMCoinInfo(tc.coinInfo).Configure()
136+
require.NoError(t, err, "failed to configure EVMConfigurator")
137+
result := feeMarketWrapper.GetBaseFee(sdk.Context{}, evmtypes.Decimals(tc.coinInfo.Decimals))
138+
require.Equal(t, tc.expResult, result)
139+
}
106140
})
107141
}
108142
}

0 commit comments

Comments
 (0)