ethereum/trinity

Incorrect pivot when peer sends an old header tip that we pivoted over

carver opened this issue · 0 comments

  • Trinity version: e87c309
  • OS: linux
  • Python version (output of python --version): 3.8.5

What is wrong?

Beam sync tried to import an old header, which made it think it was lagging the chain (even though it was caught up), and try to pivot. Logs:

    INFO  2020-08-30 21:40:21,417   BaseBodyChainSyncer  Imported block 10766586 (128 txs) in 7.66 seconds, lagging 0 blocks | 32s
    INFO  2020-08-30 21:41:21,292   BaseBodyChainSyncer  Received a header before processing its parent during regular sync. Canonical head is <BlockHeader #10766586 a65ec379>, the received header is <BlockHeader #10738264 6e769213>, with parent <BlockHeader #10738263 aaa8a6cb>. This might be a fork, importing to determine if it is the longest chain
    INFO  2020-08-30 21:41:41,701   BaseBodyChainSyncer  Import starved for 80.3s, waiting for <BlockHeader #10738264 6e769213> body
 WARNING  2020-08-30 21:41:45,417       BeamSyncService  Beam Sync is lagging by 28328 blocks. Pivoting...

The above sync started long after 10738264, at: INFO 2020-08-30 21:33:43,795 BaseBodyChainSyncer Imported block 10766553 (198 txs) in 13.56 seconds, lagging 10 blocks | 1m41s

The checkpoint header was <BlockHeader #10735000 8cecad06>, before the fallback header.

How can it be fixed

Maybe filter out headers that are too old in HeaderLaunchpointSyncer. Something like:

diff --git a/trinity/sync/beam/chain.py b/trinity/sync/beam/chain.py
index 284e406e3..cc8b78a87 100644
--- a/trinity/sync/beam/chain.py
+++ b/trinity/sync/beam/chain.py
@@ -580,8 +580,18 @@ class HeaderLaunchpointSyncer(HeaderSyncerAPI):
         )
         yield self._launchpoint_headers
 
+        min_block_number = min(header.block_number for header in self._launchpoint_headers)
+
         async for headers in self._real_syncer.new_sync_headers(max_batch_size):
-            yield headers
+            valid_headers = tuple(h for h in headers if h.block_number >= min_block_number)
+            if len(valid_headers) < len(headers):
+                dropped_headers = tuple(h for h in headers if h.block_number < min_block_number)
+                self.logger.warning(
+                    "Dropped these headers that are prior to the checkpoint: %s",
+                    dropped_headers,
+                )
+            if len(valid_headers):
+                yield valid_headers
 
     def get_target_header_hash(self) -> Hash32:
         return self._real_syncer.get_target_header_hash()