diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 6f7d9dee6d..56b8e5dc63 100644 --- a/AllTests-mainnet.md +++ b/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 recpient of an unconfigured validator returns the suggested default [Bea OK ++ Obtaining the fee recipient of a missing validator returns 404 [Beacon Node] [Preset: main 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 diff --git a/beacon_chain/conf.nim b/beacon_chain/conf.nim index 356998cf47..b0d229aba7 100644 --- a/beacon_chain/conf.nim +++ b/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, diff --git a/beacon_chain/consensus_object_pools/consensus_manager.nim b/beacon_chain/consensus_object_pools/consensus_manager.nim index 38cf1d2028..12d5c2c789 100644 --- a/beacon_chain/consensus_object_pools/consensus_manager.nim +++ b/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 = diff --git a/beacon_chain/rpc/rest_key_management_api.nim b/beacon_chain/rpc/rest_key_management_api.nim index 4fd649bbe1..8186eb919b 100644 --- a/beacon_chain/rpc/rest_key_management_api.nim +++ b/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( diff --git a/beacon_chain/validator_client/common.nim b/beacon_chain/validator_client/common.nim index d90b2c4bec..a3e7385e9e 100644 --- a/beacon_chain/validator_client/common.nim +++ b/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( - vc.config.defaultFeeRecipient) + keystore.pubkey, perValidatorDefaultFeeRecipient).valueOr( + 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( - vc.config.validatorsDir, pubkey, vc.config.defaultFeeRecipient) + 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: diff --git a/beacon_chain/validators/keystore_management.nim b/beacon_chain/validators/keystore_management.nim index dad746826d..5a31f84697 100644 --- a/beacon_chain/validators/keystore_management.nim +++ b/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, - pubkey: ValidatorPubKey, - defaultFeeRecipient: Eth1Address): Result[Eth1Address, ValidatorConfigFileStatus] = + validatorsDir: string, pubkey: ValidatorPubKey, + defaultFeeRecipient: Eth1Address): + 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, - pubkey: ValidatorPubKey): Result[Eth1Address, ValidatorConfigFileStatus] = - host.validatorsDir.getSuggestedFeeRecipient(pubkey, host.defaultFeeRecipient) + host: KeymanagerHost, pubkey: ValidatorPubKey, + defaultFeeRecipient: Eth1Address): + 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( - host.defaultFeeRecipient) + feeRecipient = host.getSuggestedFeeRecipient( + keystore.pubkey, withdrawalAddress) gasLimit = host.getSuggestedGasLimit(keystore.pubkey).valueOr( host.defaultGasLimit) v = host.validatorPool[].addValidator(keystore, feeRecipient, gasLimit) diff --git a/tests/test_block_processor.nim b/tests/test_block_processor.nim index 3c7ce5999d..17ecec6017 100644 --- a/tests/test_block_processor.nim +++ b/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 diff --git a/tests/test_keymanager_api.nim b/tests/test_keymanager_api.nim index a2f8ba65e4..0fbe27a815 100644 --- a/tests/test_keymanager_api.nim +++ b/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)