Cache and resend, rather than recreate, builder API registrations (#4040)

Author: tersec

beacon_chain/consensus_object_pools/block_dag.nim

@@ -11,7 +11,6 @@ else:
   {.push raises: [].}
 
 import
-  std/options,
   chronicles,
   ../spec/datatypes/[phase0, altair, bellatrix],
   ../spec/forks

beacon_chain/consensus_object_pools/blockchain_dag.nim

@@ -1952,7 +1952,7 @@ proc preInit*(
     tailStateSlot = getStateField(tailState, slot)
 
   let genesisBlockRoot = withState(genesisState):
-    if state.root != getStateRoot(tailState):
+    if forkyState.root != getStateRoot(tailState):
       # Different tail and genesis
       if state.data.slot >= getStateField(tailState, slot):
         fatal "Tail state must be newer or the same as genesis state"

beacon_chain/spec/beaconstate.nim

@@ -11,7 +11,7 @@ else:
   {.push raises: [].}
 
 import
-  std/[algorithm, collections/heapqueue, math, options, sequtils, tables],
+  std/[algorithm, collections/heapqueue, math, sequtils, tables],
   stew/assign2,
   json_serialization/std/sets,
   chronicles,

beacon_chain/spec/state_transition_block.nim

@@ -23,7 +23,7 @@ else:
   {.push raises: [].}
 
 import
-  std/[algorithm, options, sequtils, sets, tables],
+  std/[algorithm, sequtils, sets, tables],
   chronicles, metrics,
   ../extras,
   ./datatypes/[phase0, altair, bellatrix],
@@ -172,7 +172,7 @@ proc check_proposer_slashing*(
     state: var ForkedHashedBeaconState; proposer_slashing: SomeProposerSlashing;
     flags: UpdateFlags): Result[ValidatorIndex, cstring] =
   withState(state):
-    check_proposer_slashing(state.data, proposer_slashing, flags)
+    check_proposer_slashing(forkyState.data, proposer_slashing, flags)
 
 # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.3/specs/phase0/beacon-chain.md#proposer-slashings
 proc process_proposer_slashing*(

beacon_chain/validators/validator_duties.nim

@@ -1280,6 +1280,8 @@ proc getValidatorRegistration(
 
   return ok validatorRegistration
 
+from std/sequtils import toSeq
+
 proc registerValidators(node: BeaconNode, epoch: Epoch) {.async.} =
   try:
     if  (not node.config.payloadBuilderEnable) or
@@ -1303,10 +1305,20 @@ proc registerValidators(node: BeaconNode, epoch: Epoch) {.async.} =
         builderStatus = restBuilderStatus
       return
 
-    # TODO cache the generated registrations and keep resending the previous ones
+    # The async aspect of signing the registrations can cause the attached
+    # validators to change during the loop.
+    let attachedValidatorPubkeys =
+      toSeq(node.attachedValidators[].validators.keys)
+
     # https://github.com/ethereum/builder-specs/blob/v0.2.0/specs/validator.md#validator-registration
     var validatorRegistrations: seq[SignedValidatorRegistrationV1]
-    for validator in node.attachedValidators[].validators.values:
+    for key in attachedValidatorPubkeys:
+      # Time passed during awaits; REST keymanager API might have removed it
+      if key notin node.attachedValidators[].validators:
+        continue
+
+      let validator = node.attachedValidators[].validators[key]
+
       if validator.index.isNone:
         continue
 
@@ -1321,14 +1333,23 @@ proc registerValidators(node: BeaconNode, epoch: Epoch) {.async.} =
             state.data.validators.item(validator.index.get).exit_epoch:
           continue
 
-      let validatorRegistration =
-        await node.getValidatorRegistration(validator, epoch)
-      if validatorRegistration.isErr:
-        error "registerValidators: validatorRegistration failed",
-               validatorRegistration
-        continue
+      if validator.externalBuilderRegistration.isSome:
+        validatorRegistrations.add validator.externalBuilderRegistration.get
+      else:
+        let validatorRegistration =
+          await node.getValidatorRegistration(validator, epoch)
+        if validatorRegistration.isErr:
+          error "registerValidators: validatorRegistration failed",
+                 validatorRegistration
+          continue
+
+        # Time passed during await; REST keymanager API might have removed it
+        if key notin node.attachedValidators[].validators:
+          continue
 
-      validatorRegistrations.add validatorRegistration.get
+        node.attachedValidators[].validators[key].externalBuilderRegistration =
+          Opt.some validatorRegistration.get
+        validatorRegistrations.add validatorRegistration.get
 
     let registerValidatorResult =
       awaitWithTimeout(node.payloadBuilderRestClient.registerValidator(validatorRegistrations),

beacon_chain/validators/validator_pool.nim

@@ -58,7 +58,11 @@ type
 
     # Cache the latest slot signature - the slot signature is used to determine
     # if the validator will be aggregating (in the near future)
-    slotSignature*: Option[tuple[slot: Slot, signature: ValidatorSig]]
+    slotSignature*: Opt[tuple[slot: Slot, signature: ValidatorSig]]
+
+    # For the external payload builder; each epoch, the external payload
+    # builder should be informed of current validators
+    externalBuilderRegistration*: Opt[SignedValidatorRegistrationV1]
 
     startSlot*: Slot
 
@@ -93,8 +97,10 @@ proc addLocalValidator*(pool: var ValidatorPool, item: KeystoreData,
                         index: Opt[ValidatorIndex], slot: Slot) =
   doAssert item.kind == KeystoreKind.Local
   let pubkey = item.pubkey
-  let v = AttachedValidator(kind: ValidatorKind.Local, pubkey: pubkey,
-                            index: index, data: item, startSlot: slot)
+  let v = AttachedValidator(
+    kind: ValidatorKind.Local, pubkey: pubkey, index: index, data: item,
+    externalBuilderRegistration: Opt.none SignedValidatorRegistrationV1,
+    startSlot: slot)
   pool.validators[pubkey] = v
   notice "Local validator attached", pubkey, validator = shortLog(v),
                                      start_slot = slot
@@ -109,9 +115,11 @@ proc addRemoteValidator*(pool: var ValidatorPool, item: KeystoreData,
                          index: Opt[ValidatorIndex], slot: Slot) =
   doAssert item.kind == KeystoreKind.Remote
   let pubkey = item.pubkey
-  let v = AttachedValidator(kind: ValidatorKind.Remote, pubkey: pubkey,
-                            index: index, data: item, clients: clients,
-                            startSlot: slot)
+  let v = AttachedValidator(
+    kind: ValidatorKind.Remote, pubkey: pubkey, index: index, data: item,
+    clients: clients,
+    externalBuilderRegistration: Opt.none SignedValidatorRegistrationV1,
+    startSlot: slot)
   pool.validators[pubkey] = v
   notice "Remote validator attached", pubkey, validator = shortLog(v),
                                       remote_signer = $item.remotes,
@@ -403,7 +411,7 @@ proc getSlotSignature*(v: AttachedValidator, fork: Fork,
   if signature.isErr:
     return signature
 
-  v.slotSignature = some((slot, signature.get))
+  v.slotSignature = Opt.some((slot, signature.get))
   return signature
 
 # https://github.com/ethereum/builder-specs/blob/v0.2.0/specs/builder.md#signing

research/simutils.nim

@@ -87,7 +87,7 @@ proc loadGenesis*(validators: Natural, validate: bool):
         quit 1
 
       if forkyState.data.validators.len != validators:
-        echo &"Supplied genesis file has {state.data.validators.len} validators, while {validators} where requested, running anyway"
+        echo &"Supplied genesis file has {forkyState.data.validators.len} validators, while {validators} where requested, running anyway"
 
       echo &"Loaded {genesisFn}..."
 

research/state_sim.nim

@@ -63,11 +63,11 @@ cli do(slots = SLOTS_PER_EPOCH * 5,
         write(stdout, ".")
 
       if last:
-        withState(state[]): writeJson("state.json", state.data)
+        withState(state[]): writeJson("state.json", forkyState.data)
     else:
       withState(state[]):
-        if state.data.slot mod json_interval.uint64 == 0:
-          writeJson(jsonName(prefix, state.data.slot), state.data)
+        if forkyState.data.slot mod json_interval.uint64 == 0:
+          writeJson(jsonName(prefix, forkyState.data.slot), forkyState.data)
           write(stdout, ":")
         else:
           write(stdout, ".")
@@ -107,7 +107,7 @@ cli do(slots = SLOTS_PER_EPOCH * 5,
 
       withState(state[]):
         let
-          slot = state.data.slot
+          slot = forkyState.data.slot
           epoch = slot.epoch
           committees_per_slot =
             get_committee_count_per_slot(state.data, epoch, cache)