Fix VC not always be able to obtain feeRecipient value. (#5781)

Use state's validator value to obtain feeRecipient value.
Make feeRecipient and gasLimit calculation equal for BN and VC.

beacon_chain/conf.nim

@@ -968,7 +968,7 @@ type
 
     suggestedGasLimit* {.
       desc: "Suggested gas limit"
-      defaultValue: 30_000_000
+      defaultValue: defaultGasLimit
       name: "suggested-gas-limit" .}: uint64
 
     keymanagerEnabled* {.

beacon_chain/consensus_object_pools/common_tools.nim

@@ -0,0 +1,66 @@
+# beacon_chain
+# Copyright (c) 2024 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.
+
+{.push raises: [].}
+
+import
+  std/typetraits,
+  results,
+  ../spec/datatypes/base
+
+from ../spec/eth2_apis/dynamic_fee_recipients import
+  DynamicFeeRecipientsStore, getDynamicFeeRecipient
+from ../validators/keystore_management import
+     getPerValidatorDefaultFeeRecipient, getSuggestedGasLimit,
+     getSuggestedFeeRecipient
+from ../spec/beaconstate import has_eth1_withdrawal_credential
+from ../spec/presets import Eth1Address
+
+export Eth1Address, DynamicFeeRecipientsStore
+
+proc getFeeRecipient*(
+    dynamicFeeRecipientsStore: ref DynamicFeeRecipientsStore,
+    pubkey: ValidatorPubKey,
+    validatorIdx: Opt[ValidatorIndex],
+    stateValidator: Opt[Validator],
+    configFeeRecipient: Opt[Eth1Address],
+    configValidatorsDir: string,
+    epoch: Epoch
+): Eth1Address =
+
+  let dynFeeRecipient =
+    if validatorIdx.isSome:
+      dynamicFeeRecipientsStore[].getDynamicFeeRecipient(
+        validatorIdx.get(), epoch)
+    else:
+      Opt.none(Eth1Address)
+
+  dynFeeRecipient.valueOr:
+    let
+      withdrawalAddress =
+        if stateValidator.isSome():
+          let validator = stateValidator.get()
+          if has_eth1_withdrawal_credential(validator):
+            var address: distinctBase(Eth1Address)
+            address[0 .. ^1] = validator.withdrawal_credentials.data[12 .. ^1]
+            Opt.some Eth1Address(address)
+          else:
+            Opt.none Eth1Address
+        else:
+          Opt.none Eth1Address
+      defaultFeeRecipient =
+        getPerValidatorDefaultFeeRecipient(configFeeRecipient,
+          withdrawalAddress)
+    getSuggestedFeeRecipient(
+      configValidatorsDir, pubkey, defaultFeeRecipient).valueOr:
+      defaultFeeRecipient
+
+proc getGasLimit*(configValidatorsDir: string,
+                  configGasLimit: uint64,
+                  pubkey: ValidatorPubKey): uint64 =
+  getSuggestedGasLimit(configValidatorsDir, pubkey, configGasLimit).valueOr:
+    configGasLimit

beacon_chain/consensus_object_pools/consensus_manager.nim

@@ -12,7 +12,8 @@ import
   ../spec/datatypes/base,
   ../consensus_object_pools/[blockchain_dag, block_quarantine, attestation_pool],
   ../el/el_manager,
-  ../beacon_clock
+  ../beacon_clock,
+  ./common_tools
 
 from ../spec/beaconstate import
   get_expected_withdrawals, has_eth1_withdrawal_credential
@@ -296,41 +297,23 @@ proc checkNextProposer*(self: ref ConsensusManager, wallSlot: Slot):
 proc getFeeRecipient*(
     self: ConsensusManager, pubkey: ValidatorPubKey,
     validatorIdx: Opt[ValidatorIndex], epoch: Epoch): Eth1Address =
-  let dynFeeRecipient = if validatorIdx.isSome:
-    self.dynamicFeeRecipientsStore[].getDynamicFeeRecipient(
-      validatorIdx.get(), epoch)
-  else:
-    Opt.none(Eth1Address)
 
-  dynFeeRecipient.valueOr:
-    let
-      withdrawalAddress =
-        if validatorIdx.isSome:
-          withState(self.dag.headState):
-            if validatorIdx.get < forkyState.data.validators.lenu64:
-              let validator = forkyState.data.validators.item(validatorIdx.get)
-              if has_eth1_withdrawal_credential(validator):
-                var address: distinctBase(Eth1Address)
-                address[0..^1] = validator.withdrawal_credentials.data[12..^1]
-                Opt.some Eth1Address address
-              else:
-                Opt.none Eth1Address
-            else:
-              Opt.none Eth1Address
+  let validator =
+    if validatorIdx.isSome():
+      withState(self.dag.headState):
+        if validatorIdx.get < forkyState.data.validators.lenu64:
+          Opt.some forkyState.data.validators.item(validatorIdx.get)
         else:
-          Opt.none Eth1Address
-      defaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
-        self.defaultFeeRecipient, withdrawalAddress)
-    self.validatorsDir.getSuggestedFeeRecipient(
-        pubkey, defaultFeeRecipient).valueOr:
-      # Ignore errors and use default - errors are logged in gsfr
-      defaultFeeRecipient
+          Opt.none Validator
+    else:
+      Opt.none Validator
 
-proc getGasLimit*(
-    self: ConsensusManager, pubkey: ValidatorPubKey): uint64 =
-  self.validatorsDir.getSuggestedGasLimit(
-      pubkey, self.defaultGasLimit).valueOr:
-    self.defaultGasLimit
+  getFeeRecipient(self.dynamicFeeRecipientsStore, pubkey, validatorIdx,
+                  validator, self.defaultFeeRecipient, self.validatorsDir,
+                  epoch)
+
+proc getGasLimit*(self: ConsensusManager, pubkey: ValidatorPubKey): uint64 =
+  getGasLimit(self.validatorsDir, self.defaultGasLimit, pubkey)
 
 from ../spec/datatypes/bellatrix import PayloadID
 

beacon_chain/validator_client/common.nim

@@ -16,7 +16,7 @@ import
   ".."/spec/[eth2_merkleization, helpers, signatures, validator],
   ".."/spec/eth2_apis/[eth2_rest_serialization, rest_beacon_client,
                        dynamic_fee_recipients],
-  ".."/consensus_object_pools/block_pools_types,
+  ".."/consensus_object_pools/[block_pools_types, common_tools],
   ".."/validators/[keystore_management, validator_pool, slashing_protection,
                    validator_duties],
   ".."/[conf, beacon_clock, version, nimbus_binary_common]
@@ -948,34 +948,17 @@ proc removeValidator*(vc: ValidatorClientRef,
         res
     await allFutures(pending)
 
-proc getFeeRecipient*(vc: ValidatorClientRef, pubkey: ValidatorPubKey,
-                      validatorIdx: ValidatorIndex,
-                      epoch: Epoch): Opt[Eth1Address] =
-  let dynamicRecipient = vc.dynamicFeeRecipientsStore[].getDynamicFeeRecipient(
-                           validatorIdx, epoch)
-  if dynamicRecipient.isSome():
-    Opt.some(dynamicRecipient.get())
-  else:
-    let
-      withdrawalAddress =
-        if vc.keymanagerHost.isNil:
-          Opt.none Eth1Address
-        else:
-          vc.keymanagerHost[].getValidatorWithdrawalAddress(pubkey)
-      perValidatorDefaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
-        vc.config.defaultFeeRecipient, withdrawalAddress)
-      staticRecipient = getSuggestedFeeRecipient(
-        vc.config.validatorsDir, pubkey, perValidatorDefaultFeeRecipient)
-    if staticRecipient.isOk():
-      Opt.some(staticRecipient.get())
-    else:
-      Opt.none(Eth1Address)
+proc getFeeRecipient(vc: ValidatorClientRef, validator: AttachedValidator,
+                     epoch: Epoch): Eth1Address =
+  getFeeRecipient(vc.dynamicFeeRecipientsStore, validator.pubkey,
+                  validator.index, validator.validator,
+                  vc.config.defaultFeeRecipient(),
+                  vc.config.validatorsDir(), epoch)
 
-proc getGasLimit*(vc: ValidatorClientRef,
-                  pubkey: ValidatorPubKey): uint64 =
-  getSuggestedGasLimit(
-    vc.config.validatorsDir, pubkey, vc.config.suggestedGasLimit).valueOr:
-      vc.config.suggestedGasLimit
+proc getGasLimit(vc: ValidatorClientRef,
+                 validator: AttachedValidator): uint64 =
+  getGasLimit(vc.config.validatorsDir, vc.config.suggestedGasLimit,
+              validator.pubkey)
 
 proc prepareProposersList*(vc: ValidatorClientRef,
                            epoch: Epoch): seq[PrepareBeaconProposer] =
@@ -984,10 +967,9 @@ proc prepareProposersList*(vc: ValidatorClientRef,
     if validator.index.isSome():
       let
         index = validator.index.get()
-        feeRecipient = vc.getFeeRecipient(validator.pubkey, index, epoch)
-      if feeRecipient.isSome():
-        res.add(PrepareBeaconProposer(validator_index: index,
-                                      fee_recipient: feeRecipient.get()))
+        feeRecipient = vc.getFeeRecipient(validator, epoch)
+      res.add(PrepareBeaconProposer(validator_index: index,
+                                    fee_recipient: feeRecipient))
   res
 
 proc isDefault*(reg: SignedValidatorRegistrationV1): bool =
@@ -1043,18 +1025,13 @@ proc getValidatorRegistration(
       # Want to send it to relay, but not recompute perfectly fine cache
       return ok(PendingValidatorRegistration(registration: cached, future: nil))
 
-    let feeRecipient = vc.getFeeRecipient(validator.pubkey, vindex,
-                                          currentSlot.epoch())
-    if feeRecipient.isNone():
-      debug "Could not get fee recipient for registration data",
-            validator = shortLog(validator)
-      return err(RegistrationKind.MissingFee)
-    let gasLimit = vc.getGasLimit(validator.pubkey)
+    let
+      feeRecipient = vc.getFeeRecipient(validator, currentSlot.epoch())
+      gasLimit = vc.getGasLimit(validator)
     var registration =
       SignedValidatorRegistrationV1(
         message: ValidatorRegistrationV1(
-          fee_recipient:
-            ExecutionAddress(data: distinctBase(feeRecipient.get())),
+          fee_recipient: ExecutionAddress(data: distinctBase(feeRecipient)),
           gas_limit: gasLimit,
           timestamp: uint64(timestamp.toUnix()),
           pubkey: validator.pubkey

beacon_chain/validators/validator_pool.nim

@@ -79,6 +79,10 @@ type
     doppelActivity*: Opt[Epoch]
       ## The last time we attempted to perform a duty with this validator
 
+    validator*: Opt[Validator]
+      ## Copy of validator's entry from head state. Used by validator client,
+      ## to calculate feeRecipient address.
+
     lastWarning*: Opt[Slot]
 
   SignResponse* = Web3SignerDataResponse
@@ -255,6 +259,7 @@ proc updateValidator*(
   ## Update activation information for a validator
   if validator.index != Opt.some data.index:
     validator.index = Opt.some data.index
+    validator.validator = Opt.some data.validator
 
   if validator.activationEpoch != data.validator.activation_epoch:
     # In theory, activation epoch could change but that's rare enough that it