Fix: Base Fee Off By One (#2015)

* checkpoint

* add migration handler and test

* add impl and fixed existing tests

* add integration test

* cleanup

* more cleanup

* prev base fee per gas avoids returning nil to avoid method handler crashed on replay

* remove unused prefix

* fix test

* reserve storage slot 0

* fix

* set new names and register migration handler

* fix

aclmapping/evm/mappings_test.go

@@ -126,7 +126,7 @@ func (suite *KeeperTestSuite) TestMsgEVMTransaction() {
 			ctx, err := suite.preprocessor.AnteHandle(handlerCtx, tx.GetTx(), false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil })
 			suite.Require().Nil(err)
 			cms.ResetEvents()
-			suite.App.EvmKeeper.SetDynamicBaseFeePerGas(ctx, sdk.ZeroDec())
+			suite.App.EvmKeeper.SetCurrBaseFeePerGas(ctx, sdk.ZeroDec())
 			_, err = suite.msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), tc.msg)
 			suite.Require().Nil(err)
 

contracts/src/EVMCompatibilityTester.sol

@@ -15,6 +15,9 @@ contract EVMCompatibilityTester {
     event StringSet(address indexed performer, string value);
     event LogIndexEvent(address indexed performer, uint256 value);
     event BytesSet(address indexed performer, bytes value);
+    // Example of contract storing and retrieving data
+    uint256 private storedData; // needs to be first storage variable
+    mapping(uint256 => uint256) public gasGuzzler;
 
     struct MsgDetails {
         address sender;
@@ -23,9 +26,6 @@ contract EVMCompatibilityTester {
         uint256 gas;
     }
 
-    // Example of contract storing and retrieving data
-    uint256 private storedData;
-
     // deployer of the contract
     address public owner;
 
@@ -180,5 +180,20 @@ contract EVMCompatibilityTester {
         }
         return fee;
     }
+
+    // useGas will at least use gasToUse amount of gas
+    function useGas(uint256 gasToUse) public {
+        // while gasleft() > gasUse, use use storage to use gas
+        uint256 counter = 0;
+        uint256 startGas = gasleft();
+        uint256 gasUsed = 0;
+        while (gasUsed < gasToUse) {
+            uint256 randomNumber = uint256(keccak256(abi.encodePacked(block.number, block.prevrandao, counter)));
+            counter++;
+            gasGuzzler[randomNumber] = randomNumber;
+            uint256 endGas = gasleft();
+            gasUsed = startGas - endGas;
+        }
+    }
 }
 

contracts/test/EVMCompatabilityTest.js

@@ -312,7 +312,6 @@ describe("EVM Test", function () {
         expect(found).to.be.true;
       });
 
-
       it("Should set the string correctly and emit an event", async function () {
         await delay()
         const txResponse = await evmTester.setStringVar("test", { gasPrice: ethers.parseUnits('100', 'gwei') });
@@ -499,13 +498,53 @@ describe("EVM Test", function () {
         const zero = ethers.parseUnits('0', 'ether')
         const highgp = ethers.parseUnits("400", "gwei");
         const gp = ethers.parseUnits("100", "gwei");
+        const oneGwei = ethers.parseUnits("1", "gwei");
 
         const testCases = [
           ["No truncation from max priority fee", gp, gp],
           ["With truncation from max priority fee", gp, highgp],
           ["With complete truncation from max priority fee", zero, highgp]
         ];
 
+        it("Check base fee on the right block", async function () {
+          await delay()
+          // check if base fee is 1gwei, otherwise wait
+          let iterations = 0
+          while (true) {
+            const block = await ethers.provider.getBlock("latest");
+            const baseFee = Number(block.baseFeePerGas);
+            if (baseFee === Number(oneGwei)) {
+              break;
+            }
+            iterations += 1
+            if(iterations > 10) {
+              throw new Error("base fee hasn't dropped to 1gwei in 10 iterations")
+            }
+            await sleep(1000);
+          }
+          // use at least 1000000 gas to increase base fee
+          const txResponse = await evmTester.useGas(1000000, { gasPrice: ethers.parseUnits('100', 'gwei') });
+          const receipt = await txResponse.wait();
+          const blockHeight = receipt.blockNumber;
+
+          // make sure block base fee is 1gwei and the block after
+          // has a base fee higher than 1gwei
+          const block = await ethers.provider.getBlock(blockHeight);
+          const baseFee = Number(block.baseFeePerGas);
+          expect(baseFee).to.equal(oneGwei);
+          // wait for the next block
+          while (true) {
+            const bn = await ethers.provider.getBlockNumber();
+            if(bn !== blockHeight){
+              break
+            }
+            await sleep(500)
+          }
+          const nextBlock = await ethers.provider.getBlock(blockHeight + 1);
+          const nextBaseFee = Number(nextBlock.baseFeePerGas);
+          expect(nextBaseFee).to.be.greaterThan(oneGwei);
+        });
+
         it("Should be able to send many EIP-1559 txs", async function () {
           const gp = ethers.parseUnits("100", "gwei");
           const zero = ethers.parseUnits('0', 'ether')

evmrpc/block.go

@@ -277,7 +277,7 @@ func EncodeTmBlock(
 	txHash := common.HexToHash(block.Block.DataHash.String())
 	resultHash := common.HexToHash(block.Block.LastResultsHash.String())
 	miner := common.HexToAddress(block.Block.ProposerAddress.String())
-	baseFeePerGas := k.GetDynamicBaseFeePerGas(ctx).TruncateInt().BigInt()
+	baseFeePerGas := k.GetCurrBaseFeePerGas(ctx).TruncateInt().BigInt()
 	var blockGasUsed int64
 	chainConfig := types.DefaultChainConfig().EthereumConfig(k.ChainID(ctx))
 	transactions := []interface{}{}

evmrpc/info.go

@@ -76,7 +76,7 @@ func (i *InfoAPI) Accounts() (result []common.Address, returnErr error) {
 func (i *InfoAPI) GasPrice(ctx context.Context) (result *hexutil.Big, returnErr error) {
 	startTime := time.Now()
 	defer recordMetrics("eth_GasPrice", i.connectionType, startTime, returnErr == nil)
-	baseFee := i.keeper.GetDynamicBaseFeePerGas(i.ctxProvider(LatestCtxHeight)).TruncateInt().BigInt()
+	baseFee := i.keeper.GetCurrBaseFeePerGas(i.ctxProvider(LatestCtxHeight)).TruncateInt().BigInt()
 	totalGasUsed, err := i.getCongestionData(ctx, nil)
 	if err != nil {
 		return nil, err
@@ -229,7 +229,7 @@ func (i *InfoAPI) safeGetBaseFee(targetHeight int64) (res *big.Int) {
 			res = nil
 		}
 	}()
-	baseFee := i.keeper.GetDynamicBaseFeePerGas(i.ctxProvider(targetHeight))
+	baseFee := i.keeper.GetCurrBaseFeePerGas(i.ctxProvider(targetHeight))
 	res = baseFee.TruncateInt().BigInt()
 	return
 }

evmrpc/simulate.go

@@ -436,7 +436,7 @@ func (b *Backend) getHeader(blockNumber *big.Int) *ethtypes.Header {
 	header := &ethtypes.Header{
 		Difficulty:    common.Big0,
 		Number:        blockNumber,
-		BaseFee:       b.keeper.GetDynamicBaseFeePerGas(b.ctxProvider(LatestCtxHeight)).TruncateInt().BigInt(),
+		BaseFee:       b.keeper.GetCurrBaseFeePerGas(b.ctxProvider(LatestCtxHeight)).TruncateInt().BigInt(),
 		GasLimit:      b.config.GasCap,
 		Time:          uint64(time.Now().Unix()),
 		ExcessBlobGas: &zeroExcessBlobGas,

x/evm/ante/fee.go

@@ -104,7 +104,7 @@ func (fc EVMFeeCheckDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
 
 // minimum fee per gas required for a tx to be processed
 func (fc EVMFeeCheckDecorator) getBaseFee(ctx sdk.Context) *big.Int {
-	return fc.evmKeeper.GetDynamicBaseFeePerGas(ctx).TruncateInt().BigInt()
+	return fc.evmKeeper.GetCurrBaseFeePerGas(ctx).TruncateInt().BigInt()
 }
 
 // lowest allowed fee per gas, base fee will not be lower than this

x/evm/keeper/evm_test.go

@@ -39,15 +39,15 @@ func TestInternalCallCreateContract(t *testing.T) {
 	_, err = k.HandleInternalEVMCall(ctx, req)
 	require.Equal(t, "sei does not support EVM->CW->EVM call pattern", err.Error())
 	ctx = ctx.WithIsEVM(false)
-	oldBaseFee := k.GetDynamicBaseFeePerGas(ctx)
-	k.SetDynamicBaseFeePerGas(ctx, sdk.ZeroDec())
+	oldBaseFee := k.GetCurrBaseFeePerGas(ctx)
+	k.SetCurrBaseFeePerGas(ctx, sdk.ZeroDec())
 	_, err = k.HandleInternalEVMCall(ctx, req)
 	require.Nil(t, err)
 	receipt, err := k.GetTransientReceipt(ctx, [32]byte{1, 2, 3})
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	// reset base fee
-	k.SetDynamicBaseFeePerGas(ctx, oldBaseFee)
+	k.SetCurrBaseFeePerGas(ctx, oldBaseFee)
 }
 
 func TestInternalCall(t *testing.T) {

x/evm/keeper/fee.go

@@ -10,10 +10,12 @@ func (k *Keeper) AdjustDynamicBaseFeePerGas(ctx sdk.Context, blockGasUsed uint64
 	if ctx.ConsensusParams() == nil || ctx.ConsensusParams().Block == nil {
 		return nil
 	}
-	currentBaseFee := k.GetDynamicBaseFeePerGas(ctx)
+	prevBaseFee := k.GetNextBaseFeePerGas(ctx)
+	// set the resulting base fee for block n-1 on block n
+	k.SetCurrBaseFeePerGas(ctx, prevBaseFee)
 	targetGasUsed := sdk.NewDec(int64(k.GetTargetGasUsedPerBlock(ctx)))
-	if targetGasUsed.IsZero() {
-		return &currentBaseFee
+	if targetGasUsed.IsZero() { // avoid division by zero
+		return &prevBaseFee // return the previous base fee as is
 	}
 	minimumFeePerGas := k.GetParams(ctx).MinimumFeePerGas
 	maximumFeePerGas := k.GetParams(ctx).MaximumFeePerGas
@@ -32,14 +34,14 @@ func (k *Keeper) AdjustDynamicBaseFeePerGas(ctx sdk.Context, blockGasUsed uint64
 		denominator := blockGasLimit.Sub(targetGasUsed)
 		percentageFull := numerator.Quo(denominator)
 		adjustmentFactor := k.GetMaxDynamicBaseFeeUpwardAdjustment(ctx).Mul(percentageFull)
-		newBaseFee = currentBaseFee.Mul(sdk.NewDec(1).Add(adjustmentFactor))
+		newBaseFee = prevBaseFee.Mul(sdk.NewDec(1).Add(adjustmentFactor))
 	} else {
 		// downward adjustment
 		numerator := targetGasUsed.Sub(blockGasUsedDec)
 		denominator := targetGasUsed
 		percentageEmpty := numerator.Quo(denominator)
 		adjustmentFactor := k.GetMaxDynamicBaseFeeDownwardAdjustment(ctx).Mul(percentageEmpty)
-		newBaseFee = currentBaseFee.Mul(sdk.NewDec(1).Sub(adjustmentFactor))
+		newBaseFee = prevBaseFee.Mul(sdk.NewDec(1).Sub(adjustmentFactor))
 	}
 
 	// Ensure the new base fee is not lower than the minimum fee
@@ -53,13 +55,13 @@ func (k *Keeper) AdjustDynamicBaseFeePerGas(ctx sdk.Context, blockGasUsed uint64
 	}
 
 	// Set the new base fee for the next height
-	k.SetDynamicBaseFeePerGas(ctx.WithBlockHeight(ctx.BlockHeight()+1), newBaseFee)
+	k.SetNextBaseFeePerGas(ctx, newBaseFee)
 
 	return &newBaseFee
 }
 
 // dont have height be a prefix, just store the current base fee directly
-func (k *Keeper) GetDynamicBaseFeePerGas(ctx sdk.Context) sdk.Dec {
+func (k *Keeper) GetCurrBaseFeePerGas(ctx sdk.Context) sdk.Dec {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.BaseFeePerGasPrefix)
 	if bz == nil {
@@ -77,11 +79,38 @@ func (k *Keeper) GetDynamicBaseFeePerGas(ctx sdk.Context) sdk.Dec {
 	return d
 }
 
-func (k *Keeper) SetDynamicBaseFeePerGas(ctx sdk.Context, baseFeePerGas sdk.Dec) {
+func (k *Keeper) SetCurrBaseFeePerGas(ctx sdk.Context, baseFeePerGas sdk.Dec) {
 	store := ctx.KVStore(k.storeKey)
 	bz, err := baseFeePerGas.MarshalJSON()
 	if err != nil {
 		panic(err)
 	}
 	store.Set(types.BaseFeePerGasPrefix, bz)
 }
+
+func (k *Keeper) SetNextBaseFeePerGas(ctx sdk.Context, baseFeePerGas sdk.Dec) {
+	store := ctx.KVStore(k.storeKey)
+	bz, err := baseFeePerGas.MarshalJSON()
+	if err != nil {
+		panic(err)
+	}
+	store.Set(types.NextBaseFeePerGasPrefix, bz)
+}
+
+func (k *Keeper) GetNextBaseFeePerGas(ctx sdk.Context) sdk.Dec {
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get(types.NextBaseFeePerGasPrefix)
+	if bz == nil {
+		minFeePerGas := k.GetMinimumFeePerGas(ctx)
+		if minFeePerGas.IsNil() {
+			minFeePerGas = types.DefaultParams().MinimumFeePerGas
+		}
+		return minFeePerGas
+	}
+	d := sdk.Dec{}
+	err := d.UnmarshalJSON(bz)
+	if err != nil {
+		panic(err)
+	}
+	return d
+}

x/evm/keeper/fee_test.go

@@ -13,12 +13,12 @@ import (
 func TestBaseFeePerGas(t *testing.T) {
 	k := &testkeeper.EVMTestApp.EvmKeeper
 	ctx := testkeeper.EVMTestApp.GetContextForDeliverTx([]byte{})
-	require.Equal(t, k.GetMinimumFeePerGas(ctx), k.GetDynamicBaseFeePerGas(ctx))
-	require.True(t, k.GetDynamicBaseFeePerGas(ctx).LTE(k.GetMaximumFeePerGas(ctx)))
-	originalbf := k.GetDynamicBaseFeePerGas(ctx)
-	k.SetDynamicBaseFeePerGas(ctx, sdk.OneDec())
-	require.Equal(t, sdk.NewDecFromInt(sdk.NewInt(1)), k.GetDynamicBaseFeePerGas(ctx))
-	k.SetDynamicBaseFeePerGas(ctx, originalbf)
+	require.Equal(t, k.GetMinimumFeePerGas(ctx), k.GetCurrBaseFeePerGas(ctx))
+	require.True(t, k.GetCurrBaseFeePerGas(ctx).LTE(k.GetMaximumFeePerGas(ctx)))
+	originalbf := k.GetCurrBaseFeePerGas(ctx)
+	k.SetCurrBaseFeePerGas(ctx, sdk.OneDec())
+	require.Equal(t, sdk.NewDecFromInt(sdk.NewInt(1)), k.GetCurrBaseFeePerGas(ctx))
+	k.SetCurrBaseFeePerGas(ctx, originalbf)
 }
 
 func TestAdjustBaseFeePerGas(t *testing.T) {
@@ -163,7 +163,7 @@ func TestAdjustBaseFeePerGas(t *testing.T) {
 			ctx = ctx.WithConsensusParams(&tmproto.ConsensusParams{
 				Block: &tmproto.BlockParams{MaxGas: int64(tc.blockGasLimit)},
 			})
-			k.SetDynamicBaseFeePerGas(ctx, sdk.NewDecFromInt(sdk.NewInt(int64(tc.currentBaseFee))))
+			k.SetNextBaseFeePerGas(ctx, sdk.NewDecFromInt(sdk.NewInt(int64(tc.currentBaseFee))))
 			p := k.GetParams(ctx)
 			p.MinimumFeePerGas = sdk.NewDec(int64(tc.minimumFee))
 			p.MaximumFeePerGas = sdk.NewDec(int64(tc.maximumFee))
@@ -174,7 +174,10 @@ func TestAdjustBaseFeePerGas(t *testing.T) {
 			k.AdjustDynamicBaseFeePerGas(ctx, tc.blockGasUsed)
 			expected := sdk.NewDecFromInt(sdk.NewInt(int64(tc.expectedBaseFee)))
 			height := ctx.BlockHeight()
-			require.Equal(t, expected, k.GetDynamicBaseFeePerGas(ctx.WithBlockHeight(height+1)), "base fee did not match expected value")
+			gotCurrentBaseFee := k.GetCurrBaseFeePerGas(ctx.WithBlockHeight(height))
+			require.InDelta(t, tc.currentBaseFee, gotCurrentBaseFee.MustFloat64(), 0.001, "base fee did not match expected value")
+			gotPrevBlockBaseFee := k.GetNextBaseFeePerGas(ctx.WithBlockHeight(height))
+			require.Equal(t, expected, gotPrevBlockBaseFee, 0.001, "prev block base fee did not match expected value")
 		})
 	}
 }
@@ -187,15 +190,28 @@ func TestGetDynamicBaseFeePerGasWithNilMinFee(t *testing.T) {
 	store.Delete(types.BaseFeePerGasPrefix)
 
 	// Clear the dynamic base fee from store
-	fee := k.GetDynamicBaseFeePerGas(ctx)
+	fee := k.GetCurrBaseFeePerGas(ctx)
 	require.Equal(t, types.DefaultParams().MinimumFeePerGas, fee)
 	require.False(t, fee.IsNil())
 
 	// Test case 2: When dynamic base fee exists
 	expectedFee := sdk.NewDec(100)
-	k.SetDynamicBaseFeePerGas(ctx, expectedFee)
+	k.SetCurrBaseFeePerGas(ctx, expectedFee)
 
-	fee = k.GetDynamicBaseFeePerGas(ctx)
+	fee = k.GetCurrBaseFeePerGas(ctx)
 	require.Equal(t, expectedFee, fee)
 	require.False(t, fee.IsNil())
 }
+
+func TestGetPrevBlockBaseFeePerGasWithNilMinFee(t *testing.T) {
+	k, ctx := testkeeper.MockEVMKeeper()
+
+	// Test case 1: When dynamic base fee doesn't exist and minimum fee is nil
+	store := ctx.KVStore(k.GetStoreKey())
+	store.Delete(types.BaseFeePerGasPrefix)
+
+	// Clear the dynamic base fee from store
+	fee := k.GetCurrBaseFeePerGas(ctx)
+	require.Equal(t, types.DefaultParams().MinimumFeePerGas, fee)
+	require.False(t, fee.IsNil())
+}

x/evm/keeper/keeper.go

@@ -210,7 +210,7 @@ func (k *Keeper) GetVMBlockContext(ctx sdk.Context, gp core.GasPool) (*vm.BlockC
 		BlockNumber: big.NewInt(ctx.BlockHeight()),
 		Time:        uint64(ctx.BlockHeader().Time.Unix()),
 		Difficulty:  utils.Big0, // only needed for PoW
-		BaseFee:     k.GetDynamicBaseFeePerGas(ctx).TruncateInt().BigInt(),
+		BaseFee:     k.GetCurrBaseFeePerGas(ctx).TruncateInt().BigInt(),
 		BlobBaseFee: utils.Big1, // Cancun not enabled
 		Random:      &rh,
 	}, nil

x/evm/keeper/params_test.go

@@ -15,7 +15,7 @@ func TestParams(t *testing.T) {
 	ctx := testkeeper.EVMTestApp.GetContextForDeliverTx([]byte{}).WithBlockTime(time.Now())
 	require.Equal(t, "usei", k.GetBaseDenom(ctx))
 	require.Equal(t, types.DefaultPriorityNormalizer, k.GetPriorityNormalizer(ctx))
-	require.Equal(t, types.DefaultMinFeePerGas, k.GetDynamicBaseFeePerGas(ctx))
+	require.Equal(t, types.DefaultMinFeePerGas, k.GetCurrBaseFeePerGas(ctx))
 	require.Equal(t, types.DefaultBaseFeePerGas, k.GetBaseFeePerGas(ctx))
 	require.Equal(t, types.DefaultMinFeePerGas, k.GetMinimumFeePerGas(ctx))
 	require.Equal(t, types.DefaultMaxFeePerGas, k.GetMaximumFeePerGas(ctx))

x/evm/migrations/migrate_base_fee_off_by_one.go

@@ -0,0 +1,12 @@
+package migrations
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/sei-protocol/sei-chain/x/evm/keeper"
+)
+
+func MigrateBaseFeeOffByOne(ctx sdk.Context, k *keeper.Keeper) error {
+	baseFee := k.GetCurrBaseFeePerGas(ctx)
+	k.SetNextBaseFeePerGas(ctx, baseFee)
+	return nil
+}

x/evm/migrations/migrate_base_fee_off_by_one_test.go

@@ -0,0 +1,22 @@
+package migrations_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	testkeeper "github.com/sei-protocol/sei-chain/testutil/keeper"
+	"github.com/sei-protocol/sei-chain/x/evm/migrations"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMigrateBaseFeeOffByOne(t *testing.T) {
+	k := testkeeper.EVMTestApp.EvmKeeper
+	ctx := testkeeper.EVMTestApp.GetContextForDeliverTx([]byte{}).WithBlockHeight(8)
+	bf := sdk.NewDec(100)
+	k.SetCurrBaseFeePerGas(ctx, bf)
+	require.Equal(t, k.GetMinimumFeePerGas(ctx), k.GetNextBaseFeePerGas(ctx))
+	// do the migration
+	require.Nil(t, migrations.MigrateBaseFeeOffByOne(ctx, &k))
+	require.Equal(t, bf, k.GetNextBaseFeePerGas(ctx))
+	require.Equal(t, bf, k.GetCurrBaseFeePerGas(ctx))
+}