VC: Fix forks management behavior. (#6698)

* Fix issue "ignore _VERSION configuration value if _EPOCH == FAR_FUTURE_EPOCH".
Add `OptionalForks` constant which should provide default values for _VERSION/_EPOCH.
Fix Fork schedule should update Fork's configuration with _EPOCH values.

* Fix compilation error.

* Add comment why and how `OptionalForks` should be maintained.

beacon_chain/spec/eth2_apis/rest_fork_config.nim

@@ -48,7 +48,8 @@ func getOrDefault*(info: VCRuntimeConfig, name: string, default: Epoch): Epoch =
 
 func getForkVersion(
     info: VCRuntimeConfig,
-    consensusFork: ConsensusFork
+    consensusFork: ConsensusFork,
+    optionalForks: set[ConsensusFork] = {}
 ): Result[Version, string] =
   let
     key = consensusFork.forkVersionConfigKey()
@@ -56,6 +57,9 @@ func getForkVersion(
       try:
         info[key]
       except KeyError:
+        if consensusFork in optionalForks:
+          "0xFFFFFFFF"
+        else:
           return err("Forks configuration missing value " & $consensusFork)
   var value: Version
   try:
@@ -64,8 +68,11 @@ func getForkVersion(
     return err(key & " is invalid, reason " & exc.msg)
   ok(value)
 
-func getForkEpoch(info: VCRuntimeConfig,
+func getForkEpoch(
-                  consensusFork: ConsensusFork): Result[Epoch, string] =
+    info: VCRuntimeConfig,
+    consensusFork: ConsensusFork,
+    optionalForks: set[ConsensusFork] = {}
+): Result[Epoch, string] =
   if consensusFork > ConsensusFork.Phase0:
     let
       key = consensusFork.forkEpochConfigKey()
@@ -73,6 +80,9 @@ func getForkEpoch(info: VCRuntimeConfig,
         try:
           info[key]
         except KeyError:
+          if consensusFork in optionalForks:
+            "18446744073709551615"
+          else:
             return err("Forks configuration missing value " & $consensusFork)
       numValue = Base10.decode(uint64, stringValue).valueOr:
         return err(key & " is invalid, reason " & $error)
@@ -84,7 +94,8 @@ template toString(epoch: Epoch): string =
   Base10.toString(uint64(epoch))
 
 func getConsensusForkConfig*(
-    info: VCRuntimeConfig
+    info: VCRuntimeConfig,
+    optionalForks: set[ConsensusFork] = {}
 ): Result[VCForkConfig, string] =
   ## This extracts all `_FORK_VERSION` and `_FORK_EPOCH` constants
   var
@@ -92,8 +103,8 @@ func getConsensusForkConfig*(
     presence: set[ConsensusFork]
   for fork in ConsensusFork:
     let
-      forkVersion = ? info.getForkVersion(fork)
+      forkVersion = ? info.getForkVersion(fork, optionalForks)
-      forkEpoch = ? info.getForkEpoch(fork)
+      forkEpoch = ? info.getForkEpoch(fork, optionalForks)
     config[fork] = ForkConfigItem(version: forkVersion, epoch: forkEpoch)
     presence.incl(fork)
 

beacon_chain/validator_client/common.nim

@@ -46,6 +46,16 @@ const
 
   ZeroTimeDiff* = TimeDiff(nanoseconds: 0'i64)
 
+static: doAssert(high(ConsensusFork) == ConsensusFork.Electra,
+          "Update OptionalForks constant!")
+const
+  OptionalForks* = {ConsensusFork.Electra}
+    ## When a new ConsensusFork is added and before this fork is activated on
+    ## `mainnet`, it should be part of `OptionalForks`.
+    ## In this case, the client will ignore missing <FORKNAME>_VERSION
+    ## and <FORKNAME>_EPOCH constants from the data reported by BN via
+    ## `/eth/v1/config/spec` API call.
+
 type
   ServiceState* {.pure.} = enum
     Initialized, Running, Error, Closing, Closed
@@ -1487,24 +1497,25 @@ proc validateForkCompatibility(
     forkConfig: VCForkConfig
 ): Result[void, string] =
   let
-    item =
+    storedConfig =
       try:
         vc.forkConfig.get()[consensusFork]
       except KeyError:
         raiseAssert "Fork should be present in configuration"
 
-  if forkVersion != item.version:
+  if forkEpoch != storedConfig.epoch:
-    return err("Beacon node has conflicting " &
-               consensusFork.forkVersionConfigKey() & " value")
-
-  if forkEpoch != item.epoch:
     if forkEpoch == FAR_FUTURE_EPOCH:
       return err("Beacon node do not know about " &
                  $consensusFork & " starting epoch")
     else:
-      if item.epoch != FAR_FUTURE_EPOCH:
+      if storedConfig.epoch != FAR_FUTURE_EPOCH:
         return err("Beacon node has conflicting " &
                    consensusFork.forkEpochConfigKey() & " value")
+  else:
+    if forkEpoch != FAR_FUTURE_EPOCH:
+      if forkVersion != storedConfig.version:
+        return err("Beacon node has conflicting " &
+                   consensusFork.forkVersionConfigKey() & " value")
   ok()
 
 proc updateRuntimeConfig*(
@@ -1512,7 +1523,7 @@ proc updateRuntimeConfig*(
     node: BeaconNodeServerRef,
     info: VCRuntimeConfig
 ): Result[void, string] =
-  let forkConfig = ? info.getConsensusForkConfig()
+  let forkConfig = ? info.getConsensusForkConfig(OptionalForks)
 
   if vc.forkConfig.isNone():
     vc.forkConfig = Opt.some(forkConfig)
@@ -1526,11 +1537,32 @@ proc updateRuntimeConfig*(
         # Save newly discovered forks.
         if localForkConfig[fork].epoch == FAR_FUTURE_EPOCH:
           localForkConfig[fork].epoch = item.epoch
+          localForkConfig[fork].version = item.version
       except KeyError:
         raiseAssert "All the forks should be present inside forks configuration"
     vc.forkConfig = Opt.some(localForkConfig)
   ok()
 
+proc updateForkConfig*(vc: ValidatorClientRef) =
+  if vc.forkConfig.isNone():
+    return
+
+  var config = vc.forkConfig.get()
+  for fork in ConsensusFork:
+    let configItem =
+      try:
+        config[fork]
+      except KeyError:
+        raiseAssert "All the forks should be present inside forks configuration"
+    for scheduleItem in vc.forks:
+      if scheduleItem.current_version == configItem.version:
+        if configItem.epoch == FAR_FUTURE_EPOCH:
+          # Fork schedule knows about Fork's epoch.
+          config[fork] = ForkConfigItem(version: scheduleItem.current_version,
+                                        epoch: scheduleItem.epoch)
+        break
+  vc.forkConfig = Opt.some(config)
+
 proc `+`*(slot: Slot, epochs: Epoch): Slot =
   slot + uint64(epochs) * SLOTS_PER_EPOCH
 

beacon_chain/validator_client/fork_service.nim

@@ -69,6 +69,7 @@ proc pollForFork(vc: ValidatorClientRef) {.async: (raises: [CancelledError]).} =
 
   if (len(vc.forks) == 0) or (vc.forks != sortedForks):
     vc.forks = sortedForks
+    vc.updateForkConfig()
     notice "Fork schedule updated", fork_schedule = sortedForks
     vc.forksAvailable.fire()