negative guard; update unit tests

Author: i-norden

consensus/misc/basefee.go

@@ -59,30 +59,38 @@ func VerifyEIP1559BaseFee(config *params.ChainConfig, header, parent *types.Head
 }
 
 // CalcBaseFee returns the baseFee for the current block provided the parent header and config parameters
-func CalcBaseFee(config *params.ChainConfig, parent *types.Header) *big.Int {
+func CalcBaseFee(config *params.ChainConfig, parent *types.Header) (baseFee *big.Int) {
 	height := new(big.Int).Add(parent.Number, common.Big1)
 	// If we are before EIP1559 activation, the baseFee is nil
 	if !config.IsEIP1559(height) {
-		return nil
+		return
 	}
+	// never let baseFee drop below 0
+	defer func() {
+		if baseFee.Cmp(common.Big0) < 0 {
+			baseFee.Set(common.Big0)
+		}
+	}()
 	// If we are at the block of EIP1559 activation then the BaseFee is set to the initial value
 	if config.EIP1559Block.Cmp(height) == 0 {
-		return new(big.Int).SetUint64(config.EIP1559.InitialBaseFee)
+		baseFee = new(big.Int).SetUint64(config.EIP1559.InitialBaseFee)
+		return
 	}
 
 	// Otherwise,
 	// BASEFEE = PARENT_BASEFEE + PARENT_BASEFEE * delta // PARENT_EIP1559_GAS_TARGET // BASEFEE_MAX_CHANGE_DENOMINATOR
 	// Where delta = PARENT_GAS_USED - PARENT_EIP1559_GAS_TARGET (possibly negative)
 	parentGasTarget := CalcEIP1559GasTarget(config, parent.Number, new(big.Int).SetUint64(parent.GasLimit))
 	delta := new(big.Int).Sub(new(big.Int).SetUint64(parent.GasUsed), parentGasTarget)
-	// If delta is 0, BASEFEE is unchanged
+	// If PARENT_GAS_USAGE == TARGET_GAS_USAGE; BASEFEE = PARENT_BASEFEE - 1
 	if delta.Cmp(common.Big0) == 0 {
-		return new(big.Int).Sub(parent.BaseFee, common.Big1)
+		baseFee = new(big.Int).Sub(parent.BaseFee, common.Big1)
+		return
 	}
 	mul := new(big.Int).Mul(parent.BaseFee, delta)
 	div := new(big.Int).Div(mul, parentGasTarget)
 	div2 := new(big.Int).Div(div, new(big.Int).SetUint64(config.EIP1559.EIP1559BaseFeeMaxChangeDenominator))
-	baseFee := new(big.Int).Add(parent.BaseFee, div2)
+	baseFee = new(big.Int).Add(parent.BaseFee, div2)
 
 	// A valid BASEFEE is one such that
 	// abs(BASEFEE - PARENT_BASEFEE) <= max(1, PARENT_BASEFEE // BASEFEE_MAX_CHANGE_DENOMINATOR)

consensus/misc/basefee_test.go

@@ -297,6 +297,12 @@ func TestCalcBaseFee(t *testing.T) {
 			12000000,
 			big.NewInt(1025000000),
 		},
+
+		// Low parent baseFee tests
+
+		// parent gas usage >>>> parent gas limit
+		// parent baseFee == 0
+		// as expected, increment baseFee by 1
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
@@ -307,87 +313,75 @@ func TestCalcBaseFee(t *testing.T) {
 			1000000000000000,
 			big.NewInt(1),
 		},
-
-		// Low parent baseFee tests
+		// parent gas usage <<<< parent gas limit
+		// parent baseFee == 0
+		// would normally expect decrement
+		// but with parent baseFee == 0 the base fee calculation
+		// reduces down to parent basefee + 0 = 0
+		// this means the diff between parent and new basefee == 0 which is less than the minimum of |1|
+		// so we default to incrementing by 1, it is a positive increment since the diff == 0 is non-negative
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
 			1000,
 			big.NewInt(2000),
-			big.NewInt(1),
+			big.NewInt(0),
+			1000000000000000,
 			1,
-			8,
-			big.NewInt(2),
+			big.NewInt(1),
 		},
+		// parent gas usage == parent gas limit
+		// parent baseFee == 0
+		// would normally expect decrement of 1
+		// but that would go to negative numbers so it remains at 0
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
 			1000,
 			big.NewInt(2000),
-			big.NewInt(1),
+			big.NewInt(0),
 			1,
-			9,
-			big.NewInt(2),
+			1,
+			big.NewInt(0),
 		},
+		// parent gas usage >>>> parent gas limit
+		// parent baseFee == 1
+		// as expected, increment baseFee by 1
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
 			1000,
 			big.NewInt(2000),
 			big.NewInt(1),
-			100000000,
-			899999999,
+			1,
+			1000000000000000,
 			big.NewInt(2),
 		},
+		// parent gas usage <<<< parent gas limit
+		// parent baseFee == 1
+		// as expected, decrement by 1
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
 			1000,
 			big.NewInt(2000),
 			big.NewInt(1),
-			100000000,
-			900000000,
-			big.NewInt(2),
-		},
-		{
-			params.EIP1559ChainConfig,
-			big.NewInt(1000),
-			1000,
-			big.NewInt(2000),
-			big.NewInt(2),
+			1000000000000000,
 			1,
-			4,
-			big.NewInt(3),
+			big.NewInt(0),
 		},
+		// parent gas usage == parent gas limit
+		// parent baseFee == 1
+		// as expected, decrement by 1 when gas usage equals the gas target
 		{
 			params.EIP1559ChainConfig,
 			big.NewInt(1000),
 			1000,
 			big.NewInt(2000),
-			big.NewInt(2),
+			big.NewInt(1),
 			1,
-			5,
-			big.NewInt(3),
-		},
-		{
-			params.EIP1559ChainConfig,
-			big.NewInt(1000),
-			1000,
-			big.NewInt(2000),
-			big.NewInt(2),
-			100000000,
-			499999999,
-			big.NewInt(3),
-		},
-		{
-			params.EIP1559ChainConfig,
-			big.NewInt(1000),
-			1000,
-			big.NewInt(2000),
-			big.NewInt(2),
-			100000000,
-			500000000,
-			big.NewInt(3),
+			1,
+			big.NewInt(0),
 		},
 	}
 	for i, test := range testConditions {
@@ -404,13 +398,13 @@ func TestCalcBaseFee(t *testing.T) {
 		gasTarget := CalcBaseFee(&config, parent)
 		if gasTarget != nil {
 			if test.baseFee != nil && gasTarget.Cmp(test.baseFee) != 0 {
-				t.Errorf("test %d expected BaseFee %d got %d", i+1, test.baseFee.Uint64(), gasTarget.Uint64())
+				t.Errorf("test %d expected BaseFee %d got %d", i+1, test.baseFee.Int64(), gasTarget.Int64())
 			}
 			if test.baseFee == nil {
-				t.Errorf("test %d expected nil BaseFee got %d", i+1, gasTarget.Uint64())
+				t.Errorf("test %d expected nil BaseFee got %d", i+1, gasTarget.Int64())
 			}
 		} else if test.baseFee != nil {
-			t.Errorf("test %d expected BaseFee %d got nil", i+1, test.baseFee.Uint64())
+			t.Errorf("test %d expected BaseFee %d got nil", i+1, test.baseFee.Int64())
 		}
 	}
 }