Use withdrawal credentials as default fee recipient (#4968)

AllTests-mainnet.md

@@ -189,12 +189,12 @@ OK: 5/5 Fail: 0/5 Skip: 0/5
 OK: 3/3 Fail: 0/3 Skip: 0/3
 ## Fee recipient management [Beacon Node] [Preset: mainnet]
 ```diff
-+ Configuring the fee recpient [Beacon Node] [Preset: mainnet]                               OK
++ Configuring the fee recipient [Beacon Node] [Preset: mainnet]                              OK
 + Invalid Authorization Header [Beacon Node] [Preset: mainnet]                               OK
 + Invalid Authorization Token [Beacon Node] [Preset: mainnet]                                OK
 + Missing Authorization header [Beacon Node] [Preset: mainnet]                               OK
-+ Obtaining the fee recpient of a missing validator returns 404 [Beacon Node] [Preset: mainn OK
++ Obtaining the fee recipient of a missing validator returns 404 [Beacon Node] [Preset: main OK
-+ Obtaining the fee recpient of an unconfigured validator returns the suggested default [Bea OK
++ Obtaining the fee recipient of an unconfigured validator returns the suggested default [Be OK
 + Setting the fee recipient on a missing validator creates a record for it [Beacon Node] [Pr OK
 ```
 OK: 7/7 Fail: 0/7 Skip: 0/7

beacon_chain/conf.nim

@@ -1308,12 +1308,12 @@ proc loadEth2Network*(
 template loadEth2Network*(config: BeaconNodeConf): Eth2NetworkMetadata =
   loadEth2Network(config.eth2Network)
 
-func defaultFeeRecipient*(conf: AnyConf): Eth1Address =
+func defaultFeeRecipient*(conf: AnyConf): Opt[Eth1Address] =
   if conf.suggestedFeeRecipient.isSome:
-    conf.suggestedFeeRecipient.get
+    Opt.some conf.suggestedFeeRecipient.get
   else:
     # https://github.com/nim-lang/Nim/issues/19802
-    (static(default(Eth1Address)))
+    (static(Opt.none Eth1Address))
 
 proc loadJwtSecret*(
     rng: var HmacDrbgContext,

beacon_chain/consensus_object_pools/consensus_manager.nim

@@ -14,12 +14,14 @@ import
   ../el/el_manager,
   ../beacon_clock
 
-from ../spec/beaconstate import get_expected_withdrawals
+from ../spec/beaconstate import
+  get_expected_withdrawals, has_eth1_withdrawal_credential
 from ../spec/datatypes/capella import Withdrawal
 from ../spec/eth2_apis/dynamic_fee_recipients import
   DynamicFeeRecipientsStore, getDynamicFeeRecipient
 from ../validators/keystore_management import
-  KeymanagerHost, getSuggestedFeeRecipient, getSuggestedGasLimit
+  KeymanagerHost, getPerValidatorDefaultFeeRecipient, getSuggestedFeeRecipient,
+  getSuggestedGasLimit
 from ../validators/action_tracker import ActionTracker, getNextProposalSlot
 
 type
@@ -48,7 +50,7 @@ type
     # ----------------------------------------------------------------
     dynamicFeeRecipientsStore: ref DynamicFeeRecipientsStore
     validatorsDir: string
-    defaultFeeRecipient: Eth1Address
+    defaultFeeRecipient: Opt[Eth1Address]
     defaultGasLimit: uint64
 
     # Tracking last proposal forkchoiceUpdated payload information
@@ -66,7 +68,7 @@ func new*(T: type ConsensusManager,
           actionTracker: ActionTracker,
           dynamicFeeRecipientsStore: ref DynamicFeeRecipientsStore,
           validatorsDir: string,
-          defaultFeeRecipient: Eth1Address,
+          defaultFeeRecipient: Opt[Eth1Address],
           defaultGasLimit: uint64
          ): ref ConsensusManager =
   (ref ConsensusManager)(
@@ -296,10 +298,28 @@ proc getFeeRecipient*(
     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
+        else:
+          Opt.none Eth1Address
+      defaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
+        self.defaultFeeRecipient, withdrawalAddress)
     self.validatorsDir.getSuggestedFeeRecipient(
-        pubkey, self.defaultFeeRecipient).valueOr:
+        pubkey, defaultFeeRecipient).valueOr:
       # Ignore errors and use default - errors are logged in gsfr
-      self.defaultFeeRecipient
+      defaultFeeRecipient
 
 proc getGasLimit*(
     self: ConsensusManager, pubkey: ValidatorPubKey): uint64 =

beacon_chain/rpc/rest_key_management_api.nim

@@ -1,4 +1,4 @@
-# Copyright (c) 2021-2022 Status Research & Development GmbH
+# Copyright (c) 2021-2023 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).
@@ -126,7 +126,8 @@ proc handleAddRemoteValidatorReq(host: KeymanagerHost,
                                  keystore: RemoteKeystore): RequestItemStatus =
   let res = importKeystore(host.validatorPool[], host.validatorsDir, keystore)
   if res.isOk:
-    host.addValidator(res.get())
+    host.addValidator(
+      res.get(), host.getValidatorWithdrawalAddress(keystore.pubkey))
 
     RequestItemStatus(status: $KeystoreStatus.imported)
   else:
@@ -193,7 +194,8 @@ proc installKeymanagerHandlers*(router: var RestRouter, host: KeymanagerHost) =
           response.data.add(
             RequestItemStatus(status: $KeystoreStatus.duplicate))
       else:
-        host.addValidator(res.get())
+        host.addValidator(
+          res.get(), host.getValidatorWithdrawalAddress(res.get.pubkey))
         response.data.add(
           RequestItemStatus(status: $KeystoreStatus.imported))
 
@@ -333,7 +335,11 @@ proc installKeymanagerHandlers*(router: var RestRouter, host: KeymanagerHost) =
     let
       pubkey = pubkey.valueOr:
         return keymanagerApiError(Http400, InvalidValidatorPublicKey)
-      ethaddress = host.getSuggestedFeeRecipient(pubkey)
+      perValidatorDefaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
+        host.defaultFeeRecipient,
+        host.getValidatorWithdrawalAddress(pubkey))
+      ethaddress = host.getSuggestedFeeRecipient(
+        pubkey, perValidatorDefaultFeeRecipient)
 
     return if ethaddress.isOk:
       RestApiResponse.jsonResponse(ListFeeRecipientResponse(

beacon_chain/validator_client/common.nim

@@ -1,5 +1,5 @@
 # beacon_chain
-# Copyright (c) 2021-2022 Status Research & Development GmbH
+# Copyright (c) 2021-2023 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).
@@ -693,9 +693,16 @@ proc currentSlot*(vc: ValidatorClientRef): Slot =
 proc addValidator*(vc: ValidatorClientRef, keystore: KeystoreData) =
   let
     slot = vc.currentSlot()
+    withdrawalAddress =
+      if vc.keymanagerHost.isNil:
+        Opt.none Eth1Address
+      else:
+        vc.keymanagerHost[].getValidatorWithdrawalAddress(keystore.pubkey)
+    perValidatorDefaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
+      vc.config.defaultFeeRecipient, withdrawalAddress)
     feeRecipient = vc.config.validatorsDir.getSuggestedFeeRecipient(
-      keystore.pubkey, vc.config.defaultFeeRecipient).valueOr(
+      keystore.pubkey, perValidatorDefaultFeeRecipient).valueOr(
-        vc.config.defaultFeeRecipient)
+        perValidatorDefaultFeeRecipient)
     gasLimit = vc.config.validatorsDir.getSuggestedGasLimit(
       keystore.pubkey, vc.config.suggestedGasLimit).valueOr(
         vc.config.suggestedGasLimit)
@@ -730,8 +737,16 @@ proc getFeeRecipient*(vc: ValidatorClientRef, pubkey: ValidatorPubKey,
   if dynamicRecipient.isSome():
     Opt.some(dynamicRecipient.get())
   else:
-    let staticRecipient = getSuggestedFeeRecipient(
+    let
-      vc.config.validatorsDir, pubkey, vc.config.defaultFeeRecipient)
+      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:

beacon_chain/validators/keystore_management.nim

@@ -75,7 +75,7 @@ type
     keymanagerToken*: string
     validatorsDir*: string
     secretsDir*: string
-    defaultFeeRecipient*: Eth1Address
+    defaultFeeRecipient*: Opt[Eth1Address]
     defaultGasLimit*: uint64
     getValidatorAndIdxFn*: ValidatorPubKeyToDataFn
     getBeaconTimeFn*: GetBeaconTimeFn
@@ -101,7 +101,7 @@ func init*(T: type KeymanagerHost,
            keymanagerToken: string,
            validatorsDir: string,
            secretsDir: string,
-           defaultFeeRecipient: Eth1Address,
+           defaultFeeRecipient: Opt[Eth1Address],
            defaultGasLimit: uint64,
            getValidatorAndIdxFn: ValidatorPubKeyToDataFn,
            getBeaconTimeFn: GetBeaconTimeFn): T =
@@ -744,9 +744,9 @@ func gasLimitPath(validatorsDir: string,
   validatorsDir.validatorKeystoreDir(pubkey) / GasLimitFilename
 
 proc getSuggestedFeeRecipient*(
-    validatorsDir: string,
+    validatorsDir: string, pubkey: ValidatorPubKey,
-    pubkey: ValidatorPubKey,
+    defaultFeeRecipient: Eth1Address):
-    defaultFeeRecipient: Eth1Address): Result[Eth1Address, ValidatorConfigFileStatus] =
+    Result[Eth1Address, ValidatorConfigFileStatus] =
   # In this particular case, an error might be by design. If the file exists,
   # but doesn't load or parse that's a more urgent matter to fix. Many people
   # people might prefer, however, not to override their default suggested fee
@@ -1386,20 +1386,60 @@ proc setGasLimit*(host: KeymanagerHost,
   io2.writeFile(validatorKeystoreDir / GasLimitFilename, $gasLimit)
     .mapErr(proc(e: auto): string = "Failed to write gas limit file: " & $e)
 
+from ".."/spec/beaconstate import has_eth1_withdrawal_credential
+
+proc getValidatorWithdrawalAddress*(
+    host: KeymanagerHost, pubkey: ValidatorPubKey): Opt[Eth1Address] =
+  if host.getValidatorAndIdxFn.isNil:
+    Opt.none Eth1Address
+  else:
+    let validatorAndIndex = host.getValidatorAndIdxFn(pubkey)
+    if validatorAndIndex.isNone:
+      Opt.none Eth1Address
+    else:
+      template validator: auto = validatorAndIndex.get.validator
+      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
+
+func getPerValidatorDefaultFeeRecipient*(
+    defaultFeeRecipient: Opt[Eth1Address],
+    withdrawalAddress: Opt[Eth1Address]): Eth1Address =
+  defaultFeeRecipient.valueOr:
+    withdrawalAddress.valueOr:
+      (static(default(Eth1Address)))
+
 proc getSuggestedFeeRecipient*(
-    host: KeymanagerHost,
+    host: KeymanagerHost, pubkey: ValidatorPubKey,
-    pubkey: ValidatorPubKey): Result[Eth1Address, ValidatorConfigFileStatus] =
+    defaultFeeRecipient: Eth1Address):
-  host.validatorsDir.getSuggestedFeeRecipient(pubkey, host.defaultFeeRecipient)
+    Result[Eth1Address, ValidatorConfigFileStatus] {.deprecated.} =
+  host.validatorsDir.getSuggestedFeeRecipient(pubkey, defaultFeeRecipient)
+
+proc getSuggestedFeeRecipient(
+    host: KeyManagerHost, pubkey: ValidatorPubKey,
+    withdrawalAddress: Opt[Eth1Address]): Eth1Address =
+  # Enforce the gsfr(foo).valueOr(foo) pattern where feasible
+  let perValidatorDefaultFeeRecipient = getPerValidatorDefaultFeeRecipient(
+      host.defaultFeeRecipient, withdrawalAddress)
+  host.getSuggestedFeeRecipient(
+      pubkey, perValidatorDefaultFeeRecipient).valueOr:
+    perValidatorDefaultFeeRecipient
 
 proc getSuggestedGasLimit*(
     host: KeymanagerHost,
     pubkey: ValidatorPubKey): Result[uint64, ValidatorConfigFileStatus] =
   host.validatorsDir.getSuggestedGasLimit(pubkey, host.defaultGasLimit)
 
-proc addValidator*(host: KeymanagerHost, keystore: KeystoreData) =
+proc addValidator*(
+    host: KeymanagerHost, keystore: KeystoreData,
+    withdrawalAddress: Opt[Eth1Address]) =
   let
-    feeRecipient = host.getSuggestedFeeRecipient(keystore.pubkey).valueOr(
+    feeRecipient = host.getSuggestedFeeRecipient(
-      host.defaultFeeRecipient)
+      keystore.pubkey, withdrawalAddress)
     gasLimit = host.getSuggestedGasLimit(keystore.pubkey).valueOr(
       host.defaultGasLimit)
     v = host.validatorPool[].addValidator(keystore, feeRecipient, gasLimit)

tests/test_block_processor.nim

@@ -49,7 +49,7 @@ suite "Block processor" & preset():
       consensusManager = ConsensusManager.new(
         dag, attestationPool, quarantine, elManager, actionTracker,
         newClone(DynamicFeeRecipientsStore.init()), "",
-        default(Eth1Address), defaultGasLimit)
+        Opt.some default(Eth1Address), defaultGasLimit)
       state = newClone(dag.headState)
       cache = StateCache()
       b1 = addTestBlock(state[], cache).phase0Data

tests/test_keymanager_api.nim

@@ -1,5 +1,5 @@
 # beacon_chain
-# Copyright (c) 2021-2022 Status Research & Development GmbH
+# Copyright (c) 2021-2023 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).
@@ -109,13 +109,13 @@ const
 func specifiedFeeRecipient(x: int): Eth1Address =
   copyMem(addr result, unsafeAddr x, sizeof x)
 
-proc contains*(keylist: openArray[KeystoreInfo], key: ValidatorPubKey): bool =
+func contains*(keylist: openArray[KeystoreInfo], key: ValidatorPubKey): bool =
   for item in keylist:
     if item.validating_pubkey == key:
       return true
   false
 
-proc contains*(keylist: openArray[KeystoreInfo], key: string): bool =
+func contains*(keylist: openArray[KeystoreInfo], key: string): bool =
   let pubkey = ValidatorPubKey.fromHex(key).tryGet()
   contains(keylist, pubkey)
 
@@ -363,7 +363,7 @@ proc listRemoteValidators(validatorsDir,
           validatorsDir, secretsDir, err = err.msg
   validators
 
-proc `==`(a: seq[ValidatorPubKey],
+func `==`(a: seq[ValidatorPubKey],
           b: seq[KeystoreInfo | RemoteKeystoreInfo]): bool =
   if len(a) != len(b):
     return false
@@ -507,7 +507,7 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
     testFlavour = " [" & keymanager.ident & "]" & preset()
 
   suite "Serialization/deserialization" & testFlavour:
-    proc `==`(a, b: Kdf): bool =
+    func `==`(a, b: Kdf): bool =
       if (a.function != b.function) or (a.message != b.message):
         return false
       case a.function
@@ -523,14 +523,14 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
           (a.scryptParams.r == b.scryptParams.r) and
           (seq[byte](a.scryptParams.salt) == seq[byte](b.scryptParams.salt))
 
-    proc `==`(a, b: Checksum): bool =
+    func `==`(a, b: Checksum): bool =
       if a.function != b.function:
         return false
       case a.function
       of ChecksumFunctionKind.sha256Checksum:
         a.message.data == b.message.data
 
-    proc `==`(a, b: Cipher): bool =
+    func `==`(a, b: Cipher): bool =
       if (a.function != b.function) or
          (seq[byte](a.message) != seq[byte](b.message)):
         return false
@@ -538,11 +538,11 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
       of CipherFunctionKind.aes128CtrCipher:
         seq[byte](a.params.iv) == seq[byte](b.params.iv)
 
-    proc `==`(a, b: Crypto): bool =
+    func `==`(a, b: Crypto): bool =
       (a.kdf == b.kdf) and (a.checksum == b.checksum) and
         (a.cipher == b.cipher)
 
-    proc `==`(a, b: Keystore): bool =
+    func `==`(a, b: Keystore): bool =
       (a.crypto == b.crypto) and (a.pubkey == b.pubkey) and
         (string(a.path) == string(b.path)) and
         (a.description == b.description) and (a.uuid == b.uuid) and
@@ -1047,7 +1047,7 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
           response.status == 403
           responseJson["message"].getStr() == InvalidAuthorizationError
 
-    asyncTest "Obtaining the fee recpient of a missing validator returns 404" & testFlavour:
+    asyncTest "Obtaining the fee recipient of a missing validator returns 404" & testFlavour:
       let
         pubkey = ValidatorPubKey.fromHex(unusedPublicKeys[0]).expect("valid key")
         response = await client.listFeeRecipientPlain(
@@ -1068,7 +1068,7 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
       check:
         resultFromApi == feeRecipient
 
-    asyncTest "Obtaining the fee recpient of an unconfigured validator returns the suggested default" & testFlavour:
+    asyncTest "Obtaining the fee recipient of an unconfigured validator returns the suggested default" & testFlavour:
       let
         pubkey = ValidatorPubKey.fromHex(oldPublicKeys[0]).expect("valid key")
         resultFromApi = await client.listFeeRecipient(pubkey, correctTokenValue)
@@ -1076,7 +1076,7 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
       check:
         resultFromApi == defaultFeeRecipient
 
-    asyncTest "Configuring the fee recpient" & testFlavour:
+    asyncTest "Configuring the fee recipient" & testFlavour:
       let
         pubkey = ValidatorPubKey.fromHex(oldPublicKeys[1]).expect("valid key")
         firstFeeRecipient = specifiedFeeRecipient(2)