add strict mode to light client processor (#3894)

The light client sync protocol employs heuristics to ensure it does not
become stuck during non-finality or low sync committee participation.
These can enable use cases that prefer availability of recent data
over security. For our syncing use case, though, security is preferred.
An option is added to light client processor to configure this tradeoff.

AllTests-mainnet.md

@@ -287,14 +287,20 @@ OK: 9/9 Fail: 0/9 Skip: 0/9
 OK: 3/3 Fail: 0/3 Skip: 0/3
 ## Light client processor [Preset: mainnet]
 ```diff
-+ Duplicate bootstrap [Preset: mainnet]                                                      OK
++ Duplicate bootstrap (Optimistic) [Preset: mainnet]                                         OK
-+ Invalid bootstrap [Preset: mainnet]                                                        OK
++ Duplicate bootstrap (Strict) [Preset: mainnet]                                             OK
-+ Missing bootstrap (finality update) [Preset: mainnet]                                      OK
++ Invalid bootstrap (Optimistic) [Preset: mainnet]                                           OK
-+ Missing bootstrap (optimistic update) [Preset: mainnet]                                    OK
++ Invalid bootstrap (Strict) [Preset: mainnet]                                               OK
-+ Missing bootstrap (update) [Preset: mainnet]                                               OK
++ Missing bootstrap (finality update) (Optimistic) [Preset: mainnet]                         OK
-+ Sync [Preset: mainnet]                                                                     OK
++ Missing bootstrap (finality update) (Strict) [Preset: mainnet]                             OK
++ Missing bootstrap (optimistic update) (Optimistic) [Preset: mainnet]                       OK
++ Missing bootstrap (optimistic update) (Strict) [Preset: mainnet]                           OK
++ Missing bootstrap (update) (Optimistic) [Preset: mainnet]                                  OK
++ Missing bootstrap (update) (Strict) [Preset: mainnet]                                      OK
++ Sync (Optimistic) [Preset: mainnet]                                                        OK
++ Sync (Strict) [Preset: mainnet]                                                            OK
 ```
-OK: 6/6 Fail: 0/6 Skip: 0/6
+OK: 12/12 Fail: 0/12 Skip: 0/12
 ## ListKeys requests [Preset: mainnet]
 ```diff
 + Correct token provided [Preset: mainnet]                                                   OK
@@ -580,4 +586,4 @@ OK: 1/1 Fail: 0/1 Skip: 0/1
 OK: 9/9 Fail: 0/9 Skip: 0/9
 
 ---TOTAL---
-OK: 321/326 Fail: 0/326 Skip: 5/326
+OK: 327/332 Fail: 0/332 Skip: 5/332

beacon_chain/beacon_node_light_client.nim

@@ -42,8 +42,8 @@ proc initLightClient*(
       config.safeSlotsToImportOptimistically)
 
     lightClient = createLightClient(
-      node.network, rng, config, cfg,
+      node.network, rng, config, cfg, forkDigests, getBeaconTime,
-      forkDigests, getBeaconTime, genesis_validators_root)
+      genesis_validators_root, LightClientFinalizationMode.Strict)
 
   if config.lightClientEnable.get:
     proc shouldSyncOptimistically(slot: Slot): bool =

beacon_chain/gossip_processing/light_client_processor.nim

@@ -33,6 +33,33 @@ type
   VoidCallback* =
     proc() {.gcsafe, raises: [Defect].}
 
+  LightClientFinalizationMode* {.pure.} = enum
+    Strict
+      ## Only finalize light client data that:
+      ## - has been signed by a supermajority (2/3) of the sync committee
+      ## - has a valid finality proof
+      ##
+      ## Optimizes for security, but may become stuck if there is any of:
+      ## - non-finality for an entire sync committee period
+      ## - low sync committee participation for an entire sync committee period
+      ## Such periods need to be covered by an out-of-band syncing mechanism.
+      ##
+      ## Note that a compromised supermajority of the sync committee is able to
+      ## sign arbitrary light client data, even after being slashed. The light
+      ## client cannot validate the slashing status of sync committee members.
+      ## Likewise, voluntarily exited validators may sign bad light client data
+      ## for the sync committee periods in which they used to be selected.
+
+    Optimistic
+      ## Attempt to finalize light client data not satisfying strict conditions
+      ## if there is no progress for an extended period of time and if there are
+      ## repeated messages indicating that it is the best available data on the
+      ## network for the affected time period.
+      ##
+      ## Optimizes for availability of recent data, but may end up on incorrect
+      ## forks if run in a hostile network environment (no honest peers), or if
+      ## the low sync committee participation is being exploited by bad actors.
+
   LightClientProcessor* = object
     ## This manages the processing of received light client objects
     ##
@@ -48,13 +75,6 @@ type
     ##
     ## are then verified and added to:
     ## - `LightClientStore`
-    ##
-    ## The processor will also attempt to force-update the light client state
-    ## if no update seems to be available on the network, that is both signed by
-    ## a supermajority of sync committee members and also improves finality.
-    ## This logic is triggered if there is no progress for an extended period
-    ## of time, and there are repeated messages indicating that this is the best
-    ## available data on the network during that time period.
 
     # Config
     # ----------------------------------------------------------------
@@ -72,6 +92,10 @@ type
     cfg: RuntimeConfig
     genesis_validators_root: Eth2Digest
 
+    case finalizationMode: LightClientFinalizationMode
+    of LightClientFinalizationMode.Strict:
+      discard
+    of LightClientFinalizationMode.Optimistic:
       lastProgressTick: BeaconTime # Moment when last update made progress
       lastDuplicateTick: BeaconTime # Moment when last duplicate update received
       numDuplicatesSinceProgress: int # Number of duplicates since last progress
@@ -94,6 +118,7 @@ proc new*(
     dumpDirInvalid, dumpDirIncoming: string,
     cfg: RuntimeConfig,
     genesis_validators_root: Eth2Digest,
+    finalizationMode: LightClientFinalizationMode,
     store: ref Option[LightClientStore],
     getBeaconTime: GetBeaconTimeFn,
     getTrustedBlockRoot: GetTrustedBlockRootCallback,
@@ -112,8 +137,8 @@ proc new*(
     onFinalizedHeader: onFinalizedHeader,
     onOptimisticHeader: onOptimisticHeader,
     cfg: cfg,
-    genesis_validators_root: genesis_validators_root
+    genesis_validators_root: genesis_validators_root,
-  )
+    finalizationMode: finalizationMode)
 
 # Storage
 # ------------------------------------------------------------------------------
@@ -146,6 +171,7 @@ proc tryForceUpdate(
     store = self.store
 
   if store[].isSome:
+    doAssert self.finalizationMode == LightClientFinalizationMode.Optimistic
     case store[].get.try_light_client_store_force_update(wallSlot)
     of NoUpdate:
       discard
@@ -192,11 +218,12 @@ proc processObject(
 
   if res.isErr:
     when obj is altair.LightClientUpdate:
-      if store[].isSome and store[].get.best_valid_update.isSome:
+      if self.finalizationMode == LightClientFinalizationMode.Optimistic and
-        # `best_valid_update` gets set when no supermajority / improved finality
+          store[].isSome and store[].get.best_valid_update.isSome:
-        # is available. In that case, we will wait for a better update that once
+        # `best_valid_update` gets set when no supermajority / finality proof
-        # again fulfills those conditions. If none is received within reasonable
+        # is available. In that case, we will wait for a better update.
-        # time, the light client store is force-updated to `best_valid_update`.
+        # If none is made available within reasonable time, the light client
+        # is force-updated using the best known data to ensure sync progress.
         case res.error
         of BlockError.Duplicate:
           if wallTime >= self.lastDuplicateTick + duplicateRateLimit:
@@ -215,6 +242,7 @@ proc processObject(
     return res
 
   when obj is altair.LightClientBootstrap | altair.LightClientUpdate:
+    if self.finalizationMode == LightClientFinalizationMode.Optimistic:
       self.lastProgressTick = wallTime
       self.lastDuplicateTick = wallTime + duplicateCountDelay
       self.numDuplicatesSinceProgress = 0

beacon_chain/light_client.nim

@@ -20,7 +20,7 @@ import
   ./sync/light_client_manager,
   "."/[beacon_clock, conf_light_client]
 
-export eth2_network, conf_light_client
+export LightClientFinalizationMode, eth2_network, conf_light_client
 
 logScope: topics = "lightcl"
 
@@ -60,7 +60,8 @@ proc createLightClient(
     cfg: RuntimeConfig,
     forkDigests: ref ForkDigests,
     getBeaconTime: GetBeaconTimeFn,
-    genesis_validators_root: Eth2Digest
+    genesis_validators_root: Eth2Digest,
+    finalizationMode: LightClientFinalizationMode
 ): LightClient =
   let lightClient = LightClient(
     network: network,
@@ -85,7 +86,7 @@ proc createLightClient(
 
   lightClient.processor = LightClientProcessor.new(
     dumpEnabled, dumpDirInvalid, dumpDirIncoming,
-    cfg, genesis_validators_root,
+    cfg, genesis_validators_root, finalizationMode,
     lightClient.store, getBeaconTime, getTrustedBlockRoot,
     onStoreInitialized, onFinalizedHeader, onOptimisticHeader)
 
@@ -141,12 +142,13 @@ proc createLightClient*(
     cfg: RuntimeConfig,
     forkDigests: ref ForkDigests,
     getBeaconTime: GetBeaconTimeFn,
-    genesis_validators_root: Eth2Digest
+    genesis_validators_root: Eth2Digest,
+    finalizationMode: LightClientFinalizationMode
 ): LightClient =
   createLightClient(
     network, rng,
     config.dumpEnabled, config.dumpDirInvalid, config.dumpDirIncoming,
-    cfg, forkDigests, getBeaconTime, genesis_validators_root)
+    cfg, forkDigests, getBeaconTime, genesis_validators_root, finalizationMode)
 
 proc createLightClient*(
     network: Eth2Node,
@@ -155,12 +157,13 @@ proc createLightClient*(
     cfg: RuntimeConfig,
     forkDigests: ref ForkDigests,
     getBeaconTime: GetBeaconTimeFn,
-    genesis_validators_root: Eth2Digest
+    genesis_validators_root: Eth2Digest,
+    finalizationMode: LightClientFinalizationMode
 ): LightClient =
   createLightClient(
     network, rng,
     dumpEnabled = false, dumpDirInvalid = ".", dumpDirIncoming = ".",
-    cfg, forkDigests, getBeaconTime, genesis_validators_root)
+    cfg, forkDigests, getBeaconTime, genesis_validators_root, finalizationMode)
 
 proc start*(lightClient: LightClient) =
   notice "Starting light client",

beacon_chain/nimbus_light_client.nim

@@ -95,8 +95,8 @@ programMain:
       config.safeSlotsToImportOptimistically)
 
     lightClient = createLightClient(
-      network, rng, config, cfg,
+      network, rng, config, cfg, forkDigests, getBeaconTime,
-      forkDigests, getBeaconTime, genesis_validators_root)
+      genesis_validators_root, LightClientFinalizationMode.Optimistic)
 
   info "Listening to incoming network requests"
   network.initBeaconSync(cfg, forkDigests, genesisBlockRoot, getBeaconTime)

tests/test_light_client_processor.nim

@@ -79,6 +79,9 @@ suite "Light client processor" & preset():
         0.82
     addBlocks(numFilledEpochsPerPeriod * SLOTS_PER_EPOCH, syncCommitteeRatio)
 
+  for finalizationMode in LightClientFinalizationMode:
+    let testNameSuffix = " (" & $finalizationMode & ")" & preset()
+
     setup:
       var time = chronos.seconds(0)
       proc getBeaconTime(): BeaconTime =
@@ -87,16 +90,16 @@ suite "Light client processor" & preset():
         time = chronos.seconds((slot * SECONDS_PER_SLOT).int64)
 
       var numOnStoreInitializedCalls = 0
-    proc onStoreInitialized() = inc numOnStoreInitializedCalls
+      func onStoreInitialized() = inc numOnStoreInitializedCalls
 
       let store = (ref Option[LightClientStore])()
       var
         processor = LightClientProcessor.new(
-        false, "", "", cfg, genesis_validators_root,
+          false, "", "", cfg, genesis_validators_root, finalizationMode,
           store, getBeaconTime, getTrustedBlockRoot, onStoreInitialized)
         res: Result[bool, BlockError]
 
-  test "Sync" & preset():
+    test "Sync" & testNameSuffix:
       let bootstrap = dag.getLightClientBootstrap(trustedBlockRoot)
       check bootstrap.isOk
       setTimeToSlot(bootstrap.get.header.slot)
@@ -106,7 +109,8 @@ suite "Light client processor" & preset():
         res.isOk
         numOnStoreInitializedCalls == 1
 
-    for period in lowPeriod .. lastPeriodWithSupermajority:
+      # Reduce stack size by making this a `proc`
+      proc applyPeriodWithSupermajority(period: SyncCommitteePeriod) =
         let update = dag.getLightClientUpdateForPeriod(period)
         check update.isSome
         setTimeToSlot(update.get.signature_slot)
@@ -121,7 +125,11 @@ suite "Light client processor" & preset():
             store[].get.finalized_header == bootstrap.get.header
           store[].get.optimistic_header == update.get.attested_header
 
-    for period in lastPeriodWithSupermajority + 1 .. highPeriod:
+      for period in lowPeriod .. lastPeriodWithSupermajority:
+        applyPeriodWithSupermajority(period)
+
+      # Reduce stack size by making this a `proc`
+      proc applyPeriodWithoutSupermajority(period: SyncCommitteePeriod) =
         let update = dag.getLightClientUpdateForPeriod(period)
         check update.isSome
         setTimeToSlot(update.get.signature_slot)
@@ -129,21 +137,48 @@ suite "Light client processor" & preset():
         for i in 0 ..< 2:
           res = processor[].storeObject(
             MsgSource.gossip, getBeaconTime(), update.get)
+          if finalizationMode == LightClientFinalizationMode.Optimistic or
+              period == lastPeriodWithSupermajority + 1:
+            if finalizationMode == LightClientFinalizationMode.Optimistic or
+                i == 0:
               check:
                 res.isOk
                 store[].isSome
                 store[].get.best_valid_update.isSome
                 store[].get.best_valid_update.get == update.get
-
+            else:
-        proc applyDuplicate() = # Reduce stack size by making this a `proc`
-          res = processor[].storeObject(
-            MsgSource.gossip, getBeaconTime(), update.get)
               check:
                 res.isErr
                 res.error == BlockError.Duplicate
                 store[].isSome
                 store[].get.best_valid_update.isSome
                 store[].get.best_valid_update.get == update.get
+          else:
+            check:
+              res.isErr
+              res.error == BlockError.MissingParent
+              store[].isSome
+              store[].get.best_valid_update.isSome
+              store[].get.best_valid_update.get != update.get
+
+          proc applyDuplicate() = # Reduce stack size by making this a `proc`
+            res = processor[].storeObject(
+              MsgSource.gossip, getBeaconTime(), update.get)
+            if finalizationMode == LightClientFinalizationMode.Optimistic or
+                period == lastPeriodWithSupermajority + 1:
+              check:
+                res.isErr
+                res.error == BlockError.Duplicate
+                store[].isSome
+                store[].get.best_valid_update.isSome
+                store[].get.best_valid_update.get == update.get
+            else:
+              check:
+                res.isErr
+                res.error == BlockError.MissingParent
+                store[].isSome
+                store[].get.best_valid_update.isSome
+                store[].get.best_valid_update.get != update.get
 
           applyDuplicate()
           time += chronos.minutes(15)
@@ -154,6 +189,7 @@ suite "Light client processor" & preset():
 
           res = processor[].storeObject(
             MsgSource.gossip, getBeaconTime(), update.get)
+          if finalizationMode == LightClientFinalizationMode.Optimistic:
             check:
               res.isErr
               res.error == BlockError.Duplicate
@@ -162,7 +198,27 @@ suite "Light client processor" & preset():
             if store[].get.finalized_header == update.get.attested_header:
               break
             check store[].get.finalized_header == update.get.finalized_header
+          elif period == lastPeriodWithSupermajority + 1:
+            check:
+              res.isErr
+              res.error == BlockError.Duplicate
+              store[].isSome
+              store[].get.best_valid_update.isSome
+              store[].get.best_valid_update.get == update.get
+          else:
+            check:
+              res.isErr
+              res.error == BlockError.MissingParent
+              store[].isSome
+              store[].get.best_valid_update.isSome
+              store[].get.best_valid_update.get != update.get
+        if finalizationMode == LightClientFinalizationMode.Optimistic:
           check store[].get.finalized_header == update.get.attested_header
+        else:
+          check store[].get.finalized_header != update.get.attested_header
+
+      for period in lastPeriodWithSupermajority + 1 .. highPeriod:
+        applyPeriodWithoutSupermajority(period)
 
       let
         previousFinalized = store[].get.finalized_header
@@ -173,16 +229,19 @@ suite "Light client processor" & preset():
         MsgSource.gossip, getBeaconTime(), finalityUpdate.get)
       if res.isOk:
         check:
+          finalizationMode == LightClientFinalizationMode.Optimistic
           store[].isSome
           store[].get.finalized_header == previousFinalized
           store[].get.best_valid_update.isSome
           store[].get.best_valid_update.get.matches(finalityUpdate.get)
           store[].get.optimistic_header == finalityUpdate.get.attested_header
-    else:
+      elif finalizationMode == LightClientFinalizationMode.Optimistic:
         check res.error == BlockError.Duplicate
+      else:
+        check res.error == BlockError.MissingParent
       check numOnStoreInitializedCalls == 1
 
-  test "Invalid bootstrap" & preset():
+    test "Invalid bootstrap" & testNameSuffix:
       var bootstrap = dag.getLightClientBootstrap(trustedBlockRoot)
       check bootstrap.isOk
       bootstrap.get.header.slot.inc()
@@ -194,7 +253,7 @@ suite "Light client processor" & preset():
         res.error == BlockError.Invalid
         numOnStoreInitializedCalls == 0
 
-  test "Duplicate bootstrap" & preset():
+    test "Duplicate bootstrap" & testNameSuffix:
       let bootstrap = dag.getLightClientBootstrap(trustedBlockRoot)
       check bootstrap.isOk
       setTimeToSlot(bootstrap.get.header.slot)
@@ -210,7 +269,7 @@ suite "Light client processor" & preset():
         res.error == BlockError.Duplicate
         numOnStoreInitializedCalls == 1
 
-  test "Missing bootstrap (update)" & preset():
+    test "Missing bootstrap (update)" & testNameSuffix:
       let update = dag.getLightClientUpdateForPeriod(lowPeriod)
       check update.isSome
       setTimeToSlot(update.get.signature_slot)
@@ -221,7 +280,7 @@ suite "Light client processor" & preset():
         res.error == BlockError.MissingParent
         numOnStoreInitializedCalls == 0
 
-  test "Missing bootstrap (finality update)" & preset():
+    test "Missing bootstrap (finality update)" & testNameSuffix:
       let finalityUpdate = dag.getLightClientFinalityUpdate()
       check finalityUpdate.isSome
       setTimeToSlot(finalityUpdate.get.signature_slot)
@@ -232,7 +291,7 @@ suite "Light client processor" & preset():
         res.error == BlockError.MissingParent
         numOnStoreInitializedCalls == 0
 
-  test "Missing bootstrap (optimistic update)" & preset():
+    test "Missing bootstrap (optimistic update)" & testNameSuffix:
       let optimisticUpdate = dag.getLightClientOptimisticUpdate()
       check optimisticUpdate.isSome
       setTimeToSlot(optimisticUpdate.get.signature_slot)