Activate wire protocol eth/66 (#993)

* Activate wire protocol eth/66

and:
 Disentangle protocol_eth66.nim from import sections

why:
  Importing the protocol_eth66 module is not necessary. There is
  no need to know too many details of the underlying wire protocol. All
  that is needed will be exported by blockchain_sync.nim.

* fixes, and rebase

* Update nimbus/p2p/blockchain_sync.nim

Co-authored-by: Kim De Mey <kim.demey@gmail.com>

* Fixes and rebase

Co-authored-by: Kim De Mey <kim.demey@gmail.com>

hive_integration/nodocker/graphql/graphql_sim.nim

@@ -10,7 +10,7 @@
 import
   std/[os, json],
   eth/[p2p, trie/db], ../../../nimbus/db/db_chain,
-  ../../../nimbus/sync/protocol_eth65,
+  ../../../nimbus/sync/protocol_ethxx,
   ../../../nimbus/[genesis, config, conf_utils, context],
   ../../../nimbus/graphql/ethapi, ../../../tests/test_helpers,
   ../../../nimbus/utils/tx_pool,

hive_integration/nodocker/rpc/rpc_sim.nim

@@ -15,7 +15,7 @@ import
   stew/results,
   chronos, json_rpc/[rpcserver, rpcclient],
   ../../../nimbus/db/db_chain,
-  ../../../nimbus/sync/protocol_eth65,
+  ../../../nimbus/p2p/protocol_ethxx,
   ../../../nimbus/[config, context, genesis],
   ../../../nimbus/rpc/[common, p2p, debug],
   ../../../tests/test_helpers,

nimbus/nimbus.nim

@@ -17,9 +17,9 @@ import
   eth/common as eth_common, eth/p2p as eth_p2p,
   chronos, json_rpc/rpcserver, chronicles,
   eth/p2p/rlpx_protocols/les_protocol,
+  ./sync/protocol_ethxx,
   ./p2p/blockchain_sync, eth/net/nat, eth/p2p/peer_pool,
   ./p2p/clique/[clique_desc, clique_sealer],
-  ./sync/protocol_eth65,
   config, genesis, rpc/[common, p2p, debug, engine_api], p2p/chain,
   eth/trie/db, metrics, metrics/[chronos_httpserver, chronicles_support],
   graphql/ethapi, context, utils/tx_pool,

nimbus/p2p/blockchain_sync.nim

@@ -8,9 +8,11 @@
 import
   std/[sets, options, random, hashes, sequtils],
   chronos, chronicles,
+  ../sync/protocol_ethxx,
   eth/common/eth_types,
-  eth/[p2p, p2p/private/p2p_types, p2p/rlpx, p2p/peer_pool],
+  eth/[p2p, p2p/private/p2p_types, p2p/rlpx, p2p/peer_pool]
-  ../sync/protocol_eth65
+
+{.push raises:[Defect].}
 
 const
   minPeersToStartSync* = 2 # Wait for consensus of at least this
@@ -22,6 +24,9 @@ type
     syncNotEnoughPeers
     syncTimeOut
 
+  BlockchainSyncDefect* = object of Defect
+    ## Catch and relay exception
+
   WantedBlocksState = enum
     Initial,
     Requested,
@@ -44,6 +49,17 @@ type
     trustedPeers: HashSet[Peer]
     hasOutOfOrderBlocks: bool
 
+template catchException(info: string; code: untyped) =
+  try:
+    code
+  except CatchableError as e:
+    raise (ref CatchableError)(msg: e.msg)
+  except Defect as e:
+    raise (ref Defect)(msg: e.msg)
+  except Exception as e:
+    raise newException(
+      BlockchainSyncDefect, info & "(): " & $e.name & " -- " & e.msg)
+
 proc hash*(p: Peer): Hash = hash(cast[pointer](p))
 
 proc endIndex(b: WantedBlocks): BlockNumber =
@@ -89,13 +105,15 @@ proc availableWorkItem(ctx: SyncContext): int =
 
   # Create new work item when queue was increased, reset when selected work item
   # is at Persisted state.
-  var numBlocks = (ctx.endBlockNumber - nextRequestedBlock).toInt
+  var numBlocks = (ctx.endBlockNumber - nextRequestedBlock).truncate(int)
   if numBlocks > maxHeadersFetch:
     numBlocks = maxHeadersFetch
   ctx.workQueue[result] = WantedBlocks(startIndex: nextRequestedBlock, numBlocks: numBlocks.uint, state: Initial)
 
-proc persistWorkItem(ctx: SyncContext, wi: var WantedBlocks): ValidationResult =
+proc persistWorkItem(ctx: SyncContext, wi: var WantedBlocks): ValidationResult
-  result = ctx.chain.persistBlocks(wi.headers, wi.bodies)
+    {.gcsafe, raises:[Defect,CatchableError].} =
+  catchException("persistBlocks"):
+    result = ctx.chain.persistBlocks(wi.headers, wi.bodies)
   case result
   of ValidationResult.OK:
     ctx.finalizedBlock = wi.endIndex
@@ -106,7 +124,8 @@ proc persistWorkItem(ctx: SyncContext, wi: var WantedBlocks): ValidationResult =
   wi.headers = @[]
   wi.bodies = @[]
 
-proc persistPendingWorkItems(ctx: SyncContext): (int, ValidationResult) =
+proc persistPendingWorkItems(ctx: SyncContext): (int, ValidationResult)
+    {.gcsafe, raises:[Defect,CatchableError].} =
   var nextStartIndex = ctx.finalizedBlock + 1
   var keepRunning = true
   var hasOutOfOrderBlocks = false
@@ -134,7 +153,8 @@ proc persistPendingWorkItems(ctx: SyncContext): (int, ValidationResult) =
 
   ctx.hasOutOfOrderBlocks = hasOutOfOrderBlocks
 
-proc returnWorkItem(ctx: SyncContext, workItem: int): ValidationResult =
+proc returnWorkItem(ctx: SyncContext, workItem: int): ValidationResult
+    {.gcsafe, raises:[Defect,CatchableError].} =
   let wi = addr ctx.workQueue[workItem]
   let askedBlocks = wi.numBlocks.int
   let receivedBlocks = wi.headers.len
@@ -172,7 +192,8 @@ proc returnWorkItem(ctx: SyncContext, workItem: int): ValidationResult =
       receivedBlocks
     return ValidationResult.Error
 
-proc newSyncContext(chain: AbstractChainDB, peerPool: PeerPool): SyncContext =
+proc newSyncContext(chain: AbstractChainDB, peerPool: PeerPool): SyncContext
+    {.gcsafe, raises:[Defect,CatchableError].} =
   new result
   result.chain = chain
   result.peerPool = peerPool
@@ -193,7 +214,7 @@ proc getBestBlockNumber(p: Peer): Future[BlockNumber] {.async.} =
     reverse: true)
 
   tracePacket ">> Sending eth.GetBlockHeaders (0x03)", peer=p,
-    startBlock=request.startBlock.hash, max=request.maxResults
+    startBlock=request.startBlock.hash.toHex, max=request.maxResults
   let latestBlock = await p.getBlockHeaders(request)
 
   if latestBlock.isSome:
@@ -324,7 +345,7 @@ proc peersAgreeOnChain(a, b: Peer): Future[bool] {.async.} =
     reverse: true)
 
   tracePacket ">> Sending eth.GetBlockHeaders (0x03)", peer=a,
-    startBlock=request.startBlock.hash, max=request.maxResults
+    startBlock=request.startBlock.hash.toHex, max=request.maxResults
   let latestBlock = await a.getBlockHeaders(request)
 
   result = latestBlock.isSome and latestBlock.get.headers.len > 0

nimbus/sync/protocol_eth66.nim

@@ -6,9 +6,9 @@
 #  * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT)
 # at your option. This file may not be copied, modified, or distributed except according to those terms.
 
-## This module implements Ethereum Wire Protocol version 65, `eth/65`.
+## This module implements Ethereum Wire Protocol version 66, `eth/66`.
 ## Specification:
-##   https://github.com/ethereum/devp2p/blob/master/caps/eth.md
+##   `eth/66 <https://github.com/ethereum/devp2p/blob/master/caps/eth.md>`_
 
 import
   chronos, stint, chronicles, stew/byteutils, macros,
@@ -26,6 +26,10 @@ type
     forkHash: array[4, byte] # The RLP encoding must be exactly 4 bytes.
     forkNext: BlockNumber    # The RLP encoding must be variable-length
 
+  NewBlockAndTotalDiff* = object
+    ethBlock: EthBlock
+    totalDifficulty: DifficultyInt
+
   PeerState = ref object
     initialized*: bool
     bestBlockHash*: KeccakHash
@@ -40,7 +44,7 @@ const
   maxBodiesFetch* = 128
   maxReceiptsFetch* = 256
   maxHeadersFetch* = 192
-  ethVersion = 65
+  ethVersion* = 66
 
 func toHex*(x: KeccakHash): string = x.data.toHex
 macro tracePacket*(msg: static[string], args: varargs[untyped]) =
@@ -50,7 +54,7 @@ macro tracePacket*(msg: static[string], args: varargs[untyped]) =
 
 p2pProtocol eth(version = ethVersion,
                 peerState = PeerState,
-                useRequestIds = false):
+                useRequestIds = true):
 
   onPeerConnected do (peer: Peer):
     let
@@ -60,8 +64,7 @@ p2pProtocol eth(version = ethVersion,
       chainForkId = chain.getForkId(bestBlock.blockNumber)
       forkId = ForkId(
         forkHash: chainForkId.crc.toBytesBe,
-        forkNext: chainForkId.nextFork.u256,
+        forkNext: chainForkId.nextFork.u256)
-      )
 
     tracePacket ">> Sending eth.Status (0x00) [eth/" & $ethVersion & "]",
       peer, td=bestBlock.difficulty,
@@ -184,7 +187,8 @@ p2pProtocol eth(version = ethVersion,
     # because either `p2pProtocol` or RLPx doesn't work with an alias.)
     tracePacket "<< Discarding eth.NewBlock (0x07)",
       peer, totalDifficulty,
-      blockNumber=bh.header.blockNumber, blockDifficulty=bh.header.difficulty
+      blockNumber = bh.header.blockNumber,
+      blockDifficulty = bh.header.difficulty
     discard
 
   # User message 0x08: NewPooledTransactionHashes.

nimbus/sync/protocol_ethxx.nim

@@ -0,0 +1,12 @@
+# Nimbus
+# Copyright (c) 2018-2021 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at
+#     https://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at
+#     https://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed
+# except according to those terms.
+
+import ./protocol_eth66
+export protocol_eth66

tests/graphql/README.md

@@ -0,0 +1,5 @@
+per protocool folders apply, e.g.
+
+- eth65/queries.toml (obsoleted *eth65* test specs)
+- eth66/queries.toml
+- ...

tests/graphql/eth66/queries.toml

@@ -486,7 +486,7 @@ mutation {
 """
   result = """
 {
-  "protocolVersion":65
+  "protocolVersion":66
 }
 """
 

tests/test_graphql.nim

@@ -12,7 +12,7 @@ import
   stew/byteutils, unittest2,
   eth/[p2p, common, trie/db, rlp],
   graphql, ../nimbus/graphql/ethapi, graphql/test_common,
-  ../nimbus/sync/protocol_eth65,
+  ../nimbus/sync/protocol_ethxx,
   ../nimbus/[genesis, config, chain_config, context],
   ../nimbus/db/[db_chain],
   ../nimbus/p2p/chain, ./test_helpers,
@@ -25,7 +25,7 @@ type
     uncles: seq[BlockHeader]
 
 const
-  caseFolder = "tests" / "graphql"
+  caseFolder = "tests" / "graphql" / "eth" & $ethVersion
   dataFolder  = "tests" / "fixtures" / "eth_tests" / "BlockchainTests" / "ValidBlocks" / "bcUncleTest"
 
 proc toBlock(n: JsonNode, key: string): EthBlock =

tests/test_merge.nim

@@ -12,12 +12,12 @@ import
   unittest2, nimcrypto, eth/common as eth_common,
   json_rpc/[rpcserver, rpcclient], web3/[conversions, engine_api_types],
   eth/[trie/db, p2p/private/p2p_types],
-  ../nimbus/sync/protocol_eth65,
+  ../nimbus/sync/protocol_ethxx,
   ../nimbus/rpc/[common, p2p, hexstrings, rpc_types, rpc_utils, engine_api],
   ../nimbus/db/[db_chain],
   ../nimbus/[chain_config, config, context, genesis, sealer],
   ../nimbus/utils/[tx_pool],
-  ../nimbus/p2p/[chain],
+  ../nimbus/p2p/chain,
   ../nimbus/merge/mergetypes,
   ./test_helpers
 

tests/test_rpc.nim

@@ -14,8 +14,8 @@ import
   ../nimbus/[constants, config, genesis, utils, transaction,
              vm_state, vm_types],
   ../nimbus/db/[accounts_cache, db_chain],
+  ../nimbus/sync/protocol_ethxx,
   ../nimbus/p2p/[chain, executor, executor/executor_helpers],
-  ../nimbus/sync/protocol_eth65,
   ../nimbus/utils/[difficulty, tx_pool],
   ../nimbus/[context, chain_config],
    ./test_helpers, ./macro_assembler
@@ -217,7 +217,7 @@ proc rpcMain*() =
       # same expression as the client can hide issues when the value is wrong
       # in both places.  When the expected value genuinely changes, it'll be
       # obvious.  Just change this number.
-      check res == "65"
+      check res == $ethVersion
 
     test "eth_syncing":
       let res = await client.eth_syncing()