Apply gas limit rules correctly

Also, deduplicate some logic, to ease maintenance and improve
correctness.

Note that the 'diff' approach had a bug when 'diff' was negative. That
approach needed a math.abs() to correctly validate the limit.

Author: carver

eth/_utils/headers.py

@@ -15,6 +15,7 @@
     GENESIS_PARENT_HASH,
     GAS_LIMIT_EMA_DENOMINATOR,
     GAS_LIMIT_ADJUSTMENT_FACTOR,
+    GAS_LIMIT_MAXIMUM,
     GAS_LIMIT_MINIMUM,
     GAS_LIMIT_USAGE_ADJUSTMENT_NUMERATOR,
     GAS_LIMIT_USAGE_ADJUSTMENT_DENOMINATOR,
@@ -99,13 +100,13 @@ def fill_header_params_from_parent(
     return header_kwargs
 
 
-def compute_gas_limit_bounds(parent: BlockHeaderAPI) -> Tuple[int, int]:
+def compute_gas_limit_bounds(previous_limit: int) -> Tuple[int, int]:
     """
     Compute the boundaries for the block gas limit based on the parent block.
     """
-    boundary_range = parent.gas_limit // GAS_LIMIT_ADJUSTMENT_FACTOR
-    upper_bound = parent.gas_limit + boundary_range
-    lower_bound = max(GAS_LIMIT_MINIMUM, parent.gas_limit - boundary_range)
+    boundary_range = previous_limit // GAS_LIMIT_ADJUSTMENT_FACTOR
+    upper_bound = min(GAS_LIMIT_MAXIMUM, previous_limit + boundary_range)
+    lower_bound = max(GAS_LIMIT_MINIMUM, previous_limit - boundary_range)
     return lower_bound, upper_bound
 
 

eth/abc.py

@@ -4016,13 +4016,6 @@ def validate_seal(self, header: BlockHeaderAPI) -> None:
         """
         ...
 
-    @abstractmethod
-    def validate_gaslimit(self, header: BlockHeaderAPI) -> None:
-        """
-        Validate the gas limit on the given ``header``.
-        """
-        ...
-
     @abstractmethod
     def validate_uncles(self, block: BlockAPI) -> None:
         """

eth/chains/base.py

@@ -31,9 +31,6 @@
 from eth._utils.datatypes import (
     Configurable,
 )
-from eth._utils.headers import (
-    compute_gas_limit_bounds,
-)
 from eth._utils.rlp import (
     validate_imported_block_unchanged,
 )
@@ -543,28 +540,11 @@ def validate_block(self, block: BlockAPI) -> None:
         vm.validate_seal(block.header)
         vm.validate_seal_extension(block.header, ())
         self.validate_uncles(block)
-        self.validate_gaslimit(block.header)
 
     def validate_seal(self, header: BlockHeaderAPI) -> None:
         vm = self.get_vm(header)
         vm.validate_seal(header)
 
-    def validate_gaslimit(self, header: BlockHeaderAPI) -> None:
-        parent_header = self.get_block_header_by_hash(header.parent_hash)
-        low_bound, high_bound = compute_gas_limit_bounds(parent_header)
-        if header.gas_limit < low_bound:
-            raise ValidationError(
-                f"The gas limit on block {encode_hex(header.hash)} "
-                f"is too low: {header.gas_limit}. "
-                f"It must be at least {low_bound}"
-            )
-        elif header.gas_limit > high_bound:
-            raise ValidationError(
-                f"The gas limit on block {encode_hex(header.hash)} "
-                f"is too high: {header.gas_limit}. "
-                f"It must be at most {high_bound}"
-            )
-
     def validate_uncles(self, block: BlockAPI) -> None:
         has_uncles = len(block.uncles) > 0
         should_have_uncles = block.header.uncles_hash != EMPTY_UNCLE_HASH

eth/validation.py

@@ -24,6 +24,9 @@
 
 from eth_utils.toolz import itertoolz
 
+from eth._utils.headers import (
+    compute_gas_limit_bounds,
+)
 from eth.abc import VirtualMachineAPI
 from eth.constants import (
     SECPK1_N,
@@ -247,3 +250,15 @@ def validate_header_params_for_configuration(header_params: Dict[str, Any]) -> N
             f"({', '.join(tuple(sorted(ALLOWED_HEADER_FIELDS)))}). "
             f"The provided fields ({', '.join(tuple(sorted(extra_fields)))}) are not supported"
         )
+
+
+def validate_gas_limit(gas_limit: int, parent_gas_limit: int) -> None:
+    low_bound, high_bound = compute_gas_limit_bounds(parent_gas_limit)
+    if gas_limit < low_bound:
+        raise ValidationError(
+            f"The gas limit {gas_limit} is too low. It must be at least {low_bound}"
+        )
+    elif gas_limit > high_bound:
+        raise ValidationError(
+            f"The gas limit {gas_limit} is too high. It must be at most {high_bound}"
+        )

eth/vm/base.py

@@ -48,9 +48,6 @@
     PowConsensus,
 )
 from eth.constants import (
-    GAS_LIMIT_ADJUSTMENT_FACTOR,
-    GAS_LIMIT_MAXIMUM,
-    GAS_LIMIT_MINIMUM,
     GENESIS_PARENT_HASH,
     MAX_PREV_HEADER_DEPTH,
     MAX_UNCLES,
@@ -74,6 +71,7 @@
 )
 from eth.validation import (
     validate_length_lte,
+    validate_gas_limit,
 )
 from eth.vm.execution_context import (
     ExecutionContext,
@@ -613,19 +611,7 @@ def validate_gas(
             cls,
             header: BlockHeaderAPI,
             parent_header: BlockHeaderAPI) -> None:
-
-        gas_limit = header.gas_limit
-        parent_gas_limit = parent_header.gas_limit
-
-        if gas_limit < GAS_LIMIT_MINIMUM:
-            raise ValidationError(f"Gas limit {gas_limit} is below minimum {GAS_LIMIT_MINIMUM}")
-        if gas_limit > GAS_LIMIT_MAXIMUM:
-            raise ValidationError(f"Gas limit {gas_limit} is above maximum {GAS_LIMIT_MAXIMUM}")
-        diff = gas_limit - parent_gas_limit
-        if diff > (parent_gas_limit // GAS_LIMIT_ADJUSTMENT_FACTOR):
-            raise ValidationError(
-                f"Gas limit {gas_limit} difference to parent {parent_gas_limit} is too big {diff}"
-            )
+        validate_gas_limit(header.gas_limit, parent_header.gas_limit)
 
     def validate_seal(self, header: BlockHeaderAPI) -> None:
         try:

eth/vm/forks/london/__init__.py

@@ -6,13 +6,13 @@
     BlockHeaderAPI,
 )
 from eth.rlp.blocks import BaseBlock
+from eth.validation import validate_gas_limit
 from eth.vm.forks.berlin import BerlinVM
 from eth.vm.state import BaseState
 
 from .blocks import LondonBlock
 from .constants import (
     ELASTICITY_MULTIPLIER,
-    MINIMUM_GAS_LIMIT,
 )
 from .headers import (
     LondonBackwardsHeader,
@@ -45,37 +45,15 @@ def validate_gas(
             parent_header: BlockHeaderAPI) -> None:
 
         if parent_header.base_fee_per_gas is not None:
-            # This is not the first block of the fork
+            # Follow normal gas limit rules if the previous block had a base fee
             parent_gas_limit = parent_header.gas_limit
-            parent_gas_target = parent_gas_limit // ELASTICITY_MULTIPLIER
         else:
-            # On the fork block, don't account for the ELASTICITY_MULTIPLIER
-            # to avoid unduly halving the gas target.
+            # On the fork block, double the gas limit.
+            # That way, the gas target (which is half the London limit) equals the
+            # previous (pre-London) gas limit.
             parent_gas_limit = parent_header.gas_limit * ELASTICITY_MULTIPLIER
-            parent_gas_target = parent_header.gas_limit
 
-        if header.gas_used > header.gas_limit:
-            raise ValidationError(
-                f"Block used too much gas: {header.gas_used} "
-                f"(max: {header.gas_limit})"
-            )
-
-        if header.gas_limit > parent_gas_limit + (parent_gas_limit // 1024):
-            raise ValidationError(
-                f"Gas limit increased too much (from {parent_gas_limit} "
-                f"to {header.gas_limit})"
-            )
-
-        if header.gas_limit < parent_gas_target - (parent_gas_target // 1024):
-            raise ValidationError(
-                f"Gas limit decreased too much (from {parent_gas_target} "
-                f"to {header.gas_limit})"
-            )
-
-        if header.gas_limit < MINIMUM_GAS_LIMIT:
-            raise ValidationError(
-                f"Gas limit is lower than the minimum ({header.gas_limit} < {MINIMUM_GAS_LIMIT})"
-            )
+        validate_gas_limit(header.gas_limit, parent_gas_limit)
 
         expected_base_fee_per_gas = calculate_expected_base_fee_per_gas(parent_header)
         if expected_base_fee_per_gas != header.base_fee_per_gas:

eth/vm/forks/london/constants.py

@@ -10,5 +10,4 @@
 
 BASE_FEE_MAX_CHANGE_DENOMINATOR = 8
 INITIAL_BASE_FEE = 1000000000
-MINIMUM_GAS_LIMIT = 5000
 ELASTICITY_MULTIPLIER = 2

tests/core/vm/test_vm.py

@@ -37,7 +37,7 @@ def noproof_consensus_chain(vm_class):
         MiningChain,
         api.fork_at(vm_class, 0),
         api.disable_pow_check(),
-        api.genesis(),
+        api.genesis(params=dict(gas_limit=100000)),
     )
 
 
@@ -127,3 +127,53 @@ def test_validate_header_fails_on_invalid_parent(noproof_consensus_chain):
 
     with pytest.raises(ValidationError, match="Blocks must be numbered consecutively"):
         vm.validate_header(block2.header.copy(block_number=3), block1.header)
+
+
+def test_validate_gas_limit_almost_too_low(noproof_consensus_chain):
+    block1 = noproof_consensus_chain.mine_block()
+    block2 = noproof_consensus_chain.mine_block()
+
+    barely_valid_low_gas_limit = block1.header.gas_limit - (block1.header.gas_limit // 1024)
+    barely_valid_header = block2.header.copy(gas_limit=barely_valid_low_gas_limit)
+
+    vm = noproof_consensus_chain.get_vm(block2.header)
+
+    vm.validate_header(barely_valid_header, block1.header)
+
+
+def test_validate_gas_limit_too_low(noproof_consensus_chain):
+    block1 = noproof_consensus_chain.mine_block()
+    block2 = noproof_consensus_chain.mine_block()
+
+    invalid_low_gas_limit = block1.header.gas_limit - (block1.header.gas_limit // 1024) - 1
+    invalid_header = block2.header.copy(gas_limit=invalid_low_gas_limit)
+
+    vm = noproof_consensus_chain.get_vm(block2.header)
+
+    with pytest.raises(ValidationError, match="[Gg]as limit"):
+        vm.validate_header(invalid_header, block1.header)
+
+
+def test_validate_gas_limit_almost_too_high(noproof_consensus_chain):
+    block1 = noproof_consensus_chain.mine_block()
+    block2 = noproof_consensus_chain.mine_block()
+
+    barely_valid_high_gas_limit = block1.header.gas_limit + (block1.header.gas_limit // 1024)
+    barely_valid_header = block2.header.copy(gas_limit=barely_valid_high_gas_limit)
+
+    vm = noproof_consensus_chain.get_vm(block2.header)
+
+    vm.validate_header(barely_valid_header, block1.header)
+
+
+def test_validate_gas_limit_too_high(noproof_consensus_chain):
+    block1 = noproof_consensus_chain.mine_block()
+    block2 = noproof_consensus_chain.mine_block()
+
+    invalid_high_gas_limit = block1.header.gas_limit + (block1.header.gas_limit // 1024) + 1
+    invalid_header = block2.header.copy(gas_limit=invalid_high_gas_limit)
+
+    vm = noproof_consensus_chain.get_vm(block2.header)
+
+    with pytest.raises(ValidationError, match="[Gg]as limit"):
+        vm.validate_header(invalid_header, block1.header)