fix modifying common.Big1 (#649)

* add regression test

* apply fix

* use cmp

* reorder equal params

* improve readability by removing extra var (#650)

consensus/dummy/dynamic_fees.go

@@ -145,34 +145,31 @@ func CalcBaseFee(config *params.ChainConfig, parent *types.Header, timestamp uin
 		return newRollupWindow, baseFee, nil
 	}
 
+	num := new(big.Int)
+
 	if totalGas > parentGasTarget {
 		// If the parent block used more gas than its target, the baseFee should increase.
-		gasUsedDelta := new(big.Int).SetUint64(totalGas - parentGasTarget)
-		x := new(big.Int).Mul(parent.BaseFee, gasUsedDelta)
-		y := x.Div(x, parentGasTargetBig)
-		baseFeeDelta := math.BigMax(
-			x.Div(y, baseFeeChangeDenominator),
-			common.Big1,
-		)
+		num.SetUint64(totalGas - parentGasTarget)
+		num.Mul(num, parent.BaseFee)
+		num.Div(num, parentGasTargetBig)
+		num.Div(num, baseFeeChangeDenominator)
+		baseFeeDelta := math.BigMax(num, common.Big1)
 
 		baseFee.Add(baseFee, baseFeeDelta)
 	} else {
 		// Otherwise if the parent block used less gas than its target, the baseFee should decrease.
-		gasUsedDelta := new(big.Int).SetUint64(parentGasTarget - totalGas)
-		x := new(big.Int).Mul(parent.BaseFee, gasUsedDelta)
-		y := x.Div(x, parentGasTargetBig)
-		baseFeeDelta := math.BigMax(
-			x.Div(y, baseFeeChangeDenominator),
-			common.Big1,
-		)
-
+		num.SetUint64(parentGasTarget - totalGas)
+		num.Mul(num, parent.BaseFee)
+		num.Div(num, parentGasTargetBig)
+		num.Div(num, baseFeeChangeDenominator)
+		baseFeeDelta := math.BigMax(num, common.Big1)
 		// If [roll] is greater than [rollupWindow], apply the state transition to the base fee to account
 		// for the interval during which no blocks were produced.
 		// We use roll/rollupWindow, so that the transition is applied for every [rollupWindow] seconds
 		// that has elapsed between the parent and this block.
 		if roll > rollupWindow {
 			// Note: roll/rollupWindow must be greater than 1 since we've checked that roll > rollupWindow
-			baseFeeDelta = baseFeeDelta.Mul(baseFeeDelta, new(big.Int).SetUint64(roll/rollupWindow))
+			baseFeeDelta = new(big.Int).Mul(baseFeeDelta, new(big.Int).SetUint64(roll/rollupWindow))
 		}
 		baseFee.Sub(baseFee, baseFeeDelta)
 	}

consensus/dummy/dynamic_fees_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/ava-labs/coreth/core/types"
 	"github.com/ava-labs/coreth/params"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/math"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/stretchr/testify/assert"
@@ -564,3 +565,20 @@ func TestDynamicFeesEtna(t *testing.T) {
 	// lower than the prior base fee minimum.
 	require.Less(nextBaseFee.Int64(), params.ApricotPhase4MinBaseFee)
 }
+
+func TestCalcBaseFeeRegression(t *testing.T) {
+	parentTimestamp := uint64(1)
+	timestamp := parentTimestamp + params.RollupWindow + 1000
+
+	parentHeader := &types.Header{
+		Time:    parentTimestamp,
+		GasUsed: 14_999_999,
+		Number:  big.NewInt(1),
+		BaseFee: big.NewInt(1),
+		Extra:   make([]byte, params.DynamicFeeExtraDataSize),
+	}
+
+	_, _, err := CalcBaseFee(params.TestChainConfig, parentHeader, timestamp)
+	require.NoError(t, err)
+	require.Equalf(t, 0, common.Big1.Cmp(big.NewInt(1)), "big1 should be 1, got %s", common.Big1)
+}