Can re-open gaps, and note right-side gap shrink

Added hypothesis test to make sure gap invariants hold (non-overlapping,
and consecutive)

Author: carver

eth/db/chain_gaps.py

@@ -1,7 +1,11 @@
 import enum
-from typing import Tuple
+from typing import Iterable, Tuple
 
 from eth_typing import BlockNumber
+from eth_utils import (
+    ValidationError,
+    to_tuple,
+)
 
 from eth.exceptions import GapTrackingCorrupted
 from eth.typing import BlockRange, ChainGaps
@@ -12,17 +16,87 @@ class GapChange(enum.Enum):
     NewGap = enum.auto()
     GapFill = enum.auto()
     GapSplit = enum.auto()
-    GapShrink = enum.auto()
+    GapLeftShrink = enum.auto()
+    GapRightShrink = enum.auto()
     TailWrite = enum.auto()
 
 
-GAP_WRITES = (GapChange.GapFill, GapChange.GapSplit, GapChange.GapShrink)
+GAP_WRITES = (
+    GapChange.GapFill,
+    GapChange.GapSplit,
+    GapChange.GapLeftShrink,
+    GapChange.GapRightShrink,
+)
 GENESIS_CHAIN_GAPS = ((), BlockNumber(1))
 
 GapInfo = Tuple[GapChange, ChainGaps]
 
 
-def calculate_gaps(newly_persisted: BlockNumber, base_gaps: ChainGaps) -> GapInfo:
+@to_tuple
+def _join_overlapping_gaps(unjoined_gaps: Tuple[BlockRange, ...]) -> Iterable[BlockRange]:
+    """
+    After introducing a new gap, join any that overlap.
+    Input must already be sorted.
+    """
+    unyielded_low = None
+    unyielded_high = None
+    for low, high in unjoined_gaps:
+        if unyielded_high is not None:
+            if low < unyielded_low:
+                raise ValidationError(f"Unsorted input! {unjoined_gaps!r}")
+            elif unyielded_low <= low <= unyielded_high + 1:
+                unyielded_high = max(high, unyielded_high)
+                continue
+            else:
+                yield unyielded_low, unyielded_high
+
+        unyielded_low = low
+        unyielded_high = high
+
+    if unyielded_high is not None:
+        yield unyielded_low, unyielded_high
+
+
+def reopen_gap(decanonicalized: BlockNumber, base_gaps: ChainGaps) -> ChainGaps:
+    """
+    Add a new gap, for a header that was decanonicalized.
+    """
+    current_gaps, tip_child = base_gaps
+
+    if tip_child <= decanonicalized:
+        return base_gaps
+
+    new_raw_gaps = current_gaps + ((decanonicalized, decanonicalized), )
+
+    # join overlapping gaps
+    joined_gaps = _join_overlapping_gaps(sorted(new_raw_gaps))
+
+    # is the last gap overlapping with the tip child? if so, merge it
+    if joined_gaps[-1][1] + 1 >= tip_child:
+        return joined_gaps[:-1], joined_gaps[-1][0]
+    else:
+        return joined_gaps, tip_child
+
+
+def is_block_number_in_gap(block_number: BlockNumber, gaps: ChainGaps) -> bool:
+    """
+    Check if a block number is found in the given gaps
+    """
+    gap_ranges, tip_child = gaps
+    for low, high in gap_ranges:
+        if low > block_number:
+            return False
+        elif high >= block_number:
+            return True
+        # this range was below the block number, continue looking at the next range
+
+    return block_number >= tip_child
+
+
+def fill_gap(newly_persisted: BlockNumber, base_gaps: ChainGaps) -> GapInfo:
+    """
+    Remove a gap, for a new header that was canonicalized.
+    """
 
     current_gaps, tip_child = base_gaps
 
@@ -45,11 +119,12 @@ def calculate_gaps(newly_persisted: BlockNumber, base_gaps: ChainGaps) -> GapInf
         ]
 
         if len(matching_gaps) > 1:
+            first_match, second_match, *_ = matching_gaps
             raise GapTrackingCorrupted(
                 "Corrupted chain gap tracking",
                 f"No. {newly_persisted} appears to be missing in multiple gaps",
-                f"1st gap goes from {matching_gaps[0][1][0]} to {matching_gaps[0][1][1]}"
-                f"2nd gap goes from {matching_gaps[1][1][0]} to {matching_gaps[1][1][1]}"
+                f"1st gap is {first_match[1]}, 2nd gap is {second_match[1]}",
+                f"all matching gaps: {matching_gaps}",
             )
         elif len(matching_gaps) == 0:
             # Looks like we are just overwriting an existing header.
@@ -62,11 +137,11 @@ def calculate_gaps(newly_persisted: BlockNumber, base_gaps: ChainGaps) -> GapInf
             elif newly_persisted == gap[0]:
                 # we are shrinking the gap at the start
                 updated_center = ((BlockNumber(gap[0] + 1), gap[1],),)
-                gap_change = GapChange.GapShrink
+                gap_change = GapChange.GapLeftShrink
             elif newly_persisted == gap[1]:
                 # we are shrinking the gap at the tail
                 updated_center = ((gap[0], BlockNumber(gap[1] - 1),),)
-                gap_change = GapChange.GapShrink
+                gap_change = GapChange.GapRightShrink
             elif gap[0] < newly_persisted < gap[1]:
                 # we are dividing the gap
                 first_new_gap = (gap[0], BlockNumber(newly_persisted - 1))

eth/db/header.py

@@ -30,11 +30,11 @@
     GENESIS_PARENT_HASH,
 )
 from eth.db.chain_gaps import (
-    calculate_gaps,
     GapChange,
     GapInfo,
     GAP_WRITES,
     GENESIS_CHAIN_GAPS,
+    fill_gap,
 )
 from eth.exceptions import (
     CanonicalHeadNotFound,
@@ -79,7 +79,7 @@ def _update_header_chain_gaps(
             if base_gaps is None:
                 base_gaps = cls._get_header_chain_gaps(db)
 
-            gap_change, gaps = calculate_gaps(persisted_header.block_number, base_gaps)
+            gap_change, gaps = fill_gap(persisted_header.block_number, base_gaps)
 
             if gap_change is not GapChange.NoChange:
                 db.set(
@@ -297,7 +297,8 @@ def _handle_gap_change(cls,
         if gap_change not in GAP_WRITES:
             return
 
-        if gap_change is GapChange.GapFill:
+        # Check if this change will link up the chain to the right
+        if gap_change in (GapChange.GapFill, GapChange.GapRightShrink):
             next_child_number = BlockNumber(header.block_number + 1)
             expected_child = cls._get_canonical_block_header_by_number(db, next_child_number)
             if header.hash != expected_child.parent_hash:

tests/database/test_header_db.py

@@ -1,12 +1,23 @@
 import enum
+from functools import partial
 import operator
 import random
 
+from hypothesis import (
+    example,
+    given,
+    strategies as st,
+)
 import pytest
 
-from eth_utils.toolz import accumulate
+from eth_utils.toolz import (
+    accumulate,
+    compose,
+    sliding_window,
+)
 
 from eth_utils import (
+    to_set,
     to_tuple,
     keccak,
     ValidationError,
@@ -17,9 +28,16 @@
     GENESIS_DIFFICULTY,
     GENESIS_GAS_LIMIT,
 )
-from eth.db.chain_gaps import GapChange, GENESIS_CHAIN_GAPS
+from eth.db.chain_gaps import (
+    GapChange,
+    GENESIS_CHAIN_GAPS,
+    fill_gap,
+    reopen_gap,
+    is_block_number_in_gap,
+)
 from eth.exceptions import (
     CanonicalHeadNotFound,
+    GapTrackingCorrupted,
     HeaderNotFound,
     ParentNotFound,
 )
@@ -37,7 +55,7 @@ def headerdb(base_db):
     return HeaderDB(base_db)
 
 
-@pytest.fixture
+@pytest.fixture(scope="module")
 def genesis_header():
     return BlockHeader(
         difficulty=GENESIS_DIFFICULTY,
@@ -123,6 +141,13 @@ def test_headerdb_persist_disconnected_headers(headerdb, genesis_header):
         headerdb.get_block_header_by_hash(headers[2].hash)
 
 
+def test_corrupt_gaps():
+    with pytest.raises(GapTrackingCorrupted, match="(1, 3)"):
+        fill_gap(2, (((1, 3), (2, 4)), 6))
+    with pytest.raises(GapTrackingCorrupted, match="(2, 4)"):
+        fill_gap(2, (((1, 3), (2, 4)), 6))
+
+
 class StepAction(enum.Enum):
     PERSIST_CHECKPOINT = enum.auto()
     PERSIST_HEADERS = enum.auto()
@@ -132,6 +157,30 @@ class StepAction(enum.Enum):
     VERIFY_PERSIST_RAISES = enum.auto()
 
 
+def _all_gap_numbers(chain_gaps, highest_block_number):
+    """List all the missing headers, the block numbers in gaps"""
+    gap_ranges, tail = chain_gaps
+    for low, high in gap_ranges:
+        yield from range(low, high + 1)
+    yield from range(tail, highest_block_number + 1)
+
+
+def _validate_gap_invariants(gaps):
+    # 1. gaps are sorted
+    for low, high in gaps[0]:
+        assert high >= low, gaps
+
+    # 2. gaps are not overrlapping
+    for low_range, high_range in sliding_window(2, gaps[0]):
+        # the top of the low range must not be sequential with the bottom of the high range
+        assert low_range[1] + 1 < high_range[0], gaps
+
+    # 3. final gap does not overlap with the tail
+    if len(gaps[0]):
+        final_gap_range = gaps[0][-1]
+        assert final_gap_range[1] + 1 < gaps[1], gaps
+
+
 @pytest.mark.parametrize(
     'steps',
     (
@@ -296,12 +345,12 @@ def _get_chain(id):
                 (GapChange.TailWrite, ((), 2)),
                 (GapChange.NewGap, (((2, 9),), 11)),
                 (GapChange.GapSplit, (((2, 4), (6, 9),), 11)),
-                (GapChange.GapShrink, (((3, 4), (6, 9),), 11)),
-                (GapChange.GapShrink, (((3, 3), (6, 9),), 11)),
+                (GapChange.GapLeftShrink, (((3, 4), (6, 9),), 11)),
+                (GapChange.GapRightShrink, (((3, 3), (6, 9),), 11)),
                 (GapChange.GapFill, (((6, 9),), 11)),
-                (GapChange.GapShrink, (((6, 8),), 11)),
-                (GapChange.GapShrink, (((6, 7),), 11)),
-                (GapChange.GapShrink, (((6, 6),), 11)),
+                (GapChange.GapRightShrink, (((6, 8),), 11)),
+                (GapChange.GapRightShrink, (((6, 7),), 11)),
+                (GapChange.GapRightShrink, (((6, 6),), 11)),
                 (GapChange.GapFill, ((), 11)),
             )
         ),
@@ -325,6 +374,75 @@ def test_gap_tracking(headerdb, genesis_header, written_headers, evolving_gaps):
         assert change == evolving_gaps[idx][0]
 
 
+@given(st.lists(
+    st.tuples(
+        # True to insert a header (ie~ remove a gap), False to remove a header (ie~ add a gap)
+        st.booleans(),
+        st.integers(min_value=1, max_value=19),  # constrain to try to cause collisions
+    )
+))
+@example([(True, 2), (True, 4), (False, 4)])
+def test_gap_continuity(changes):
+    MAX_BLOCK_NUM = 21
+
+    # method to get all the block numbers that are in a gap right now
+    _all_missing = compose(set, partial(_all_gap_numbers, highest_block_number=MAX_BLOCK_NUM))
+
+    @to_set
+    def _all_inserted(chain_gaps):
+        """List all the inserted headers, the block numbers not in gaps"""
+        missing = _all_missing(chain_gaps)
+        for block_num in range(MAX_BLOCK_NUM + 1):
+            if block_num not in missing:
+                yield block_num
+
+    chain_gaps = GENESIS_CHAIN_GAPS
+
+    for do_insert, block_num in changes:
+        starts_inserted = _all_inserted(chain_gaps)
+        starts_missing = _all_missing(chain_gaps)
+
+        if do_insert:
+            to_insert = block_num
+            _, chain_gaps = fill_gap(to_insert, chain_gaps)
+            assert not is_block_number_in_gap(to_insert, chain_gaps)
+
+            # Make sure that at most this one block number was filled
+            finished_inserted = _all_inserted(chain_gaps)
+            assert to_insert in finished_inserted
+            new_inserts = finished_inserted - starts_inserted
+            if block_num in starts_inserted:
+                assert new_inserts == set()
+            else:
+                assert new_inserts == {block_num}
+
+            # Make sure that no new gaps were created
+            finished_missing = _all_missing(chain_gaps)
+            assert to_insert not in finished_missing
+            new_missing = finished_missing - starts_missing
+            assert new_missing == set()
+        else:
+            to_remove = block_num
+            # Note that removing a header is inserting a gap
+            chain_gaps = reopen_gap(to_remove, chain_gaps)
+            assert is_block_number_in_gap(to_remove, chain_gaps)
+
+            # Make sure that no gaps were filled
+            finished_inserted = _all_inserted(chain_gaps)
+            new_inserts = finished_inserted - starts_inserted
+            assert new_inserts == set()
+
+            # Make sure that at most this one block number gap was reopened
+            finished_missing = _all_missing(chain_gaps)
+            new_missing = finished_missing - starts_missing
+            if block_num in starts_missing:
+                assert new_missing == set()
+            else:
+                assert new_missing == {block_num}
+
+        _validate_gap_invariants(chain_gaps)
+
+
 @pytest.mark.parametrize(
     'chain_length',
     (0, 1, 2, 3),