From ff464e49cf130484ecfe464f1eac4fed71512cc9 Mon Sep 17 00:00:00 2001 From: zah Date: Wed, 15 Feb 2023 17:10:31 +0200 Subject: [PATCH] Implement the set of gas_limit end-points in the Keymanager API (#4612) Fixes #3946 --- AllTests-mainnet.md | 13 +- beacon_chain/conf.nim | 11 ++ .../consensus_manager.nim | 15 +- beacon_chain/nimbus_beacon_node.nim | 3 +- beacon_chain/nimbus_signing_node.nim | 4 +- beacon_chain/nimbus_validator_client.nim | 1 + beacon_chain/rpc/rest_constants.nim | 2 + beacon_chain/rpc/rest_key_management_api.nim | 69 +++++++- .../eth2_apis/eth2_rest_serialization.nim | 1 + .../spec/eth2_apis/rest_keymanager_calls.nim | 70 ++++++++ .../spec/eth2_apis/rest_keymanager_types.nim | 11 ++ beacon_chain/validator_client/common.nim | 25 ++- .../validators/keystore_management.nim | 83 ++++++++- beacon_chain/validators/validator_duties.nim | 11 +- beacon_chain/validators/validator_pool.nim | 25 ++- docs/the_nimbus_book/src/options.md | 1 + tests/test_block_processor.nim | 4 +- tests/test_keymanager_api.nim | 163 ++++++++++++++++++ 18 files changed, 475 insertions(+), 37 deletions(-) diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 28b88e128..6d5f4ca96 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -219,6 +219,17 @@ OK: 3/3 Fail: 0/3 Skip: 0/3 + should raise on unknown data OK ``` OK: 7/7 Fail: 0/7 Skip: 0/7 +## Gas limit management [Beacon Node] [Preset: mainnet] +```diff ++ Configuring the gas limit [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 gas limit of a missing validator returns 404 [Beacon Node] [Preset: mainnet] OK ++ Obtaining the gas limit of an unconfigured validator returns the suggested default [Beacon OK ++ Setting the gas limit on a missing validator creates a record for it [Beacon Node] [Preset OK +``` +OK: 7/7 Fail: 0/7 Skip: 0/7 ## Gossip fork transition ```diff + Gossip fork transition OK @@ -619,4 +630,4 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 OK: 9/9 Fail: 0/9 Skip: 0/9 ---TOTAL--- -OK: 344/349 Fail: 0/349 Skip: 5/349 +OK: 351/356 Fail: 0/356 Skip: 5/356 diff --git a/beacon_chain/conf.nim b/beacon_chain/conf.nim index d1d73b65f..b23ef2562 100644 --- a/beacon_chain/conf.nim +++ b/beacon_chain/conf.nim @@ -48,6 +48,7 @@ const defaultSigningNodeRequestTimeout* = 60 defaultBeaconNode* = "http://127.0.0.1:" & $defaultEth2RestPort defaultBeaconNodeUri* = parseUri(defaultBeaconNode) + defaultGasLimit* = 30_000_000 defaultListenAddressDesc* = $defaultListenAddress defaultAdminListenAddressDesc* = $defaultAdminListenAddress @@ -582,6 +583,11 @@ type desc: "Suggested fee recipient" name: "suggested-fee-recipient" .}: Option[Address] + suggestedGasLimit* {. + desc: "Suggested gas limit" + defaultValue: defaultGasLimit + name: "suggested-gas-limit" .}: uint64 + payloadBuilderEnable* {. desc: "Enable external payload builder" defaultValue: false @@ -885,6 +891,11 @@ type desc: "Suggested fee recipient" name: "suggested-fee-recipient" .}: Option[Address] + suggestedGasLimit* {. + desc: "Suggested gas limit" + defaultValue: 30_000_000 + name: "suggested-gas-limit" .}: uint64 + keymanagerEnabled* {. desc: "Enable the REST keymanager API" defaultValue: false diff --git a/beacon_chain/consensus_object_pools/consensus_manager.nim b/beacon_chain/consensus_object_pools/consensus_manager.nim index b82e33825..0798f5574 100644 --- a/beacon_chain/consensus_object_pools/consensus_manager.nim +++ b/beacon_chain/consensus_object_pools/consensus_manager.nim @@ -18,7 +18,7 @@ from ../spec/datatypes/capella import Withdrawal from ../spec/eth2_apis/dynamic_fee_recipients import DynamicFeeRecipientsStore, getDynamicFeeRecipient from ../validators/keystore_management import - KeymanagerHost, getSuggestedFeeRecipient + KeymanagerHost, getSuggestedFeeRecipient, getSuggestedGasLimit from ../validators/action_tracker import ActionTracker, getNextProposalSlot type @@ -57,6 +57,7 @@ type dynamicFeeRecipientsStore: ref DynamicFeeRecipientsStore validatorsDir: string defaultFeeRecipient: Eth1Address + defaultGasLimit: uint64 # Tracking last proposal forkchoiceUpdated payload information # ---------------------------------------------------------------- @@ -74,7 +75,8 @@ func new*(T: type ConsensusManager, actionTracker: ActionTracker, dynamicFeeRecipientsStore: ref DynamicFeeRecipientsStore, validatorsDir: string, - defaultFeeRecipient: Eth1Address + defaultFeeRecipient: Eth1Address, + defaultGasLimit: uint64 ): ref ConsensusManager = (ref ConsensusManager)( dag: dag, @@ -85,7 +87,8 @@ func new*(T: type ConsensusManager, dynamicFeeRecipientsStore: dynamicFeeRecipientsStore, validatorsDir: validatorsDir, forkchoiceUpdatedInfo: Opt.none ForkchoiceUpdatedInformation, - defaultFeeRecipient: defaultFeeRecipient + defaultFeeRecipient: defaultFeeRecipient, + defaultGasLimit: defaultGasLimit ) # Consensus Management @@ -333,6 +336,12 @@ proc getFeeRecipient*( # Ignore errors and use default - errors are logged in gsfr self.defaultFeeRecipient +proc getGasLimit*( + self: ConsensusManager, pubkey: ValidatorPubKey): uint64 = + self.validatorsDir.getSuggestedGasLimit( + pubkey, self.defaultGasLimit).valueOr: + self.defaultGasLimit + from ../spec/datatypes/bellatrix import PayloadID proc runProposalForkchoiceUpdated*( diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim index 1686c7391..45a20c5d1 100644 --- a/beacon_chain/nimbus_beacon_node.nim +++ b/beacon_chain/nimbus_beacon_node.nim @@ -315,7 +315,7 @@ proc initFullNode( dag, attestationPool, quarantine, node.eth1Monitor, ActionTracker.init(rng, config.subscribeAllSubnets), node.dynamicFeeRecipientsStore, config.validatorsDir, - config.defaultFeeRecipient) + config.defaultFeeRecipient, config.suggestedGasLimit) blockProcessor = BlockProcessor.new( config.dumpEnabled, config.dumpDirInvalid, config.dumpDirIncoming, rng, taskpool, consensusManager, node.validatorMonitor, getBeaconTime) @@ -639,6 +639,7 @@ proc init*(T: type BeaconNode, config.validatorsDir, config.secretsDir, config.defaultFeeRecipient, + config.suggestedGasLimit, getValidatorAndIdx, getBeaconTime) else: nil diff --git a/beacon_chain/nimbus_signing_node.nim b/beacon_chain/nimbus_signing_node.nim index b5bd1f0e6..e76cdd518 100644 --- a/beacon_chain/nimbus_signing_node.nim +++ b/beacon_chain/nimbus_signing_node.nim @@ -103,7 +103,9 @@ proc initValidators(sn: var SigningNode): bool = let feeRecipient = default(Eth1Address) case keystore.kind of KeystoreKind.Local: - discard sn.attachedValidators.addValidator(keystore, feeRecipient) + discard sn.attachedValidators.addValidator(keystore, + feeRecipient, + defaultGasLimit) publicKeyIdents.add("\"0x" & keystore.pubkey.toHex() & "\"") of KeystoreKind.Remote: error "Signing node do not support remote validators", diff --git a/beacon_chain/nimbus_validator_client.nim b/beacon_chain/nimbus_validator_client.nim index 7607c48b7..7b0fc9dbd 100644 --- a/beacon_chain/nimbus_validator_client.nim +++ b/beacon_chain/nimbus_validator_client.nim @@ -278,6 +278,7 @@ proc asyncInit(vc: ValidatorClientRef): Future[ValidatorClientRef] {.async.} = vc.config.validatorsDir, vc.config.secretsDir, vc.config.defaultFeeRecipient, + vc.config.suggestedGasLimit, nil, vc.beaconClock.getBeaconTimeFn) diff --git a/beacon_chain/rpc/rest_constants.nim b/beacon_chain/rpc/rest_constants.nim index cb13428b6..1f18641f1 100644 --- a/beacon_chain/rpc/rest_constants.nim +++ b/beacon_chain/rpc/rest_constants.nim @@ -50,6 +50,8 @@ const "Unable to decode voluntary exit object(s)" InvalidFeeRecipientRequestError* = "Bad request. Request was malformed and could not be processed" + InvalidGasLimitRequestError* = + "Bad request. Request was malformed and could not be processed" VoluntaryExitValidationError* = "Invalid voluntary exit, it will never pass validation so it's rejected" VoluntaryExitValidationSuccess* = diff --git a/beacon_chain/rpc/rest_key_management_api.nim b/beacon_chain/rpc/rest_key_management_api.nim index 84c4efbb7..4fd649bbe 100644 --- a/beacon_chain/rpc/rest_key_management_api.nim +++ b/beacon_chain/rpc/rest_key_management_api.nim @@ -343,7 +343,7 @@ proc installKeymanagerHandlers*(router: var RestRouter, host: KeymanagerHost) = case ethaddress.error of noSuchValidator: keymanagerApiError(Http404, "No matching validator found") - of invalidFeeRecipientFile: + of malformedConfigFile: keymanagerApiError(Http500, "Error reading fee recipient file") # https://ethereum.github.io/keymanager-APIs/#/Fee%20Recipient/SetFeeRecipient @@ -390,6 +390,73 @@ proc installKeymanagerHandlers*(router: var RestRouter, host: KeymanagerHost) = keymanagerApiError( Http500, "Failed to remove fee recipient file: " & res.error) + # https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit/getGasLimit + router.api(MethodGet, "/eth/v1/validator/{pubkey}/gas_limit") do ( + pubkey: ValidatorPubKey) -> RestApiResponse: + let authStatus = checkAuthorization(request, host) + if authStatus.isErr(): + return authErrorResponse authStatus.error + + let + pubkey = pubkey.valueOr: + return keymanagerApiError(Http400, InvalidValidatorPublicKey) + gasLimit = host.getSuggestedGasLimit(pubkey) + + return if gasLimit.isOk: + RestApiResponse.jsonResponse(GetValidatorGasLimitResponse( + pubkey: pubkey, + gas_limit: gasLimit.get)) + else: + case gasLimit.error + of noSuchValidator: + keymanagerApiError(Http404, "No matching validator found") + of malformedConfigFile: + keymanagerApiError(Http500, "Error reading gas limit file") + + # https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit/setGasLimit + router.api(MethodPost, "/eth/v1/validator/{pubkey}/gas_limit") do ( + pubkey: ValidatorPubKey, + contentBody: Option[ContentBody]) -> RestApiResponse: + let authStatus = checkAuthorization(request, host) + if authStatus.isErr(): + return authErrorResponse authStatus.error + let + pubkey = pubkey.valueOr: + return keymanagerApiError(Http400, InvalidValidatorPublicKey) + gasLimitReq = + block: + if contentBody.isNone(): + return keymanagerApiError(Http400, InvalidGasLimitRequestError) + let dres = decodeBody(SetGasLimitRequest, contentBody.get()) + if dres.isErr(): + return keymanagerApiError(Http400, InvalidGasLimitRequestError) + dres.get() + + status = host.setGasLimit(pubkey, gasLimitReq.gas_limit) + + return if status.isOk: + RestApiResponse.response("", Http202, "text/plain") + else: + keymanagerApiError( + Http500, "Failed to set gas limit: " & status.error) + + # https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit/deleteGasLimit + router.api(MethodDelete, "/eth/v1/validator/{pubkey}/gas_limit") do ( + pubkey: ValidatorPubKey) -> RestApiResponse: + let authStatus = checkAuthorization(request, host) + if authStatus.isErr(): + return authErrorResponse authStatus.error + let + pubkey = pubkey.valueOr: + return keymanagerApiError(Http400, InvalidValidatorPublicKey) + res = host.removeGasLimitFile(pubkey) + + return if res.isOk: + RestApiResponse.response("", Http204, "text/plain") + else: + keymanagerApiError( + Http500, "Failed to remove gas limit file: " & res.error) + # TODO: These URLs will be changed once we submit a proposal for # /eth/v2/remotekeys that supports distributed keys. router.api(MethodGet, "/eth/v1/remotekeys/distributed") do () -> RestApiResponse: diff --git a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim index c90111dee..6b8b7ac2d 100644 --- a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim +++ b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim @@ -85,6 +85,7 @@ type PrepareBeaconProposer | ProposerSlashing | SetFeeRecipientRequest | + SetGasLimitRequest | bellatrix_mev.SignedBlindedBeaconBlock | capella_mev.SignedBlindedBeaconBlock | SignedValidatorRegistrationV1 | diff --git a/beacon_chain/spec/eth2_apis/rest_keymanager_calls.nim b/beacon_chain/spec/eth2_apis/rest_keymanager_calls.nim index e57802d45..af0c27a95 100644 --- a/beacon_chain/spec/eth2_apis/rest_keymanager_calls.nim +++ b/beacon_chain/spec/eth2_apis/rest_keymanager_calls.nim @@ -99,6 +99,23 @@ proc deleteFeeRecipientPlain*(pubkey: ValidatorPubKey, meth: MethodDelete.} ## https://ethereum.github.io/keymanager-APIs/#/Fee%20Recipient/DeleteFeeRecipient +proc listGasLimitPlain*(pubkey: ValidatorPubKey): RestPlainResponse {. + rest, endpoint: "/eth/v1/validator/{pubkey}/gas_limit", + meth: MethodGet.} + ## https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit + +proc setGasLimitPlain*(pubkey: ValidatorPubKey, + body: SetGasLimitRequest): RestPlainResponse {. + rest, endpoint: "/eth/v1/validator/{pubkey}/gas_limit", + meth: MethodPost.} + ## https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit/setGasLimit + +proc deleteGasLimitPlain *(pubkey: ValidatorPubKey, + body: EmptyBody): RestPlainResponse {. + rest, endpoint: "/eth/v1/validator/{pubkey}/gas_limit", + meth: MethodDelete.} + ## https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit/deleteGasLimit + proc listRemoteDistributedKeysPlain*(): RestPlainResponse {. rest, endpoint: "/eth/v1/remotekeys/distributed", meth: MethodGet.} @@ -183,3 +200,56 @@ proc deleteFeeRecipient*(client: RestClientRef, raiseKeymanagerGenericError(resp) else: raiseUnknownStatusError(resp) + +proc listGasLimit*(client: RestClientRef, + pubkey: ValidatorPubKey, + token: string): Future[uint64] {.async.} = + let resp = await client.listGasLimitPlain( + pubkey, + extraHeaders = @[("Authorization", "Bearer " & token)]) + + case resp.status: + of 200: + let res = decodeBytes(DataEnclosedObject[ListGasLimitResponse], + resp.data, + resp.contentType) + if res.isErr: + raise newException(RestError, $res.error) + return res.get.data.gas_limit + of 400, 401, 403, 404, 500: + raiseKeymanagerGenericError(resp) + else: + raiseUnknownStatusError(resp) + +proc setGasLimit*(client: RestClientRef, + pubkey: ValidatorPubKey, + gasLimit: uint64, + token: string) {.async.} = + let resp = await client.setGasLimitPlain( + pubkey, + SetGasLimitRequest(gasLimit: gasLimit), + extraHeaders = @[("Authorization", "Bearer " & token)]) + + case resp.status: + of 202: + discard + of 400, 401, 403, 404, 500: + raiseKeymanagerGenericError(resp) + else: + raiseUnknownStatusError(resp) + +proc deleteGasLimit*(client: RestClientRef, + pubkey: ValidatorPubKey, + token: string) {.async.} = + let resp = await client.deleteGasLimitPlain( + pubkey, + EmptyBody(), + extraHeaders = @[("Authorization", "Bearer " & token)]) + + case resp.status: + of 204: + discard + of 400, 401, 403, 404, 500: + raiseKeymanagerGenericError(resp) + else: + raiseUnknownStatusError(resp) diff --git a/beacon_chain/spec/eth2_apis/rest_keymanager_types.nim b/beacon_chain/spec/eth2_apis/rest_keymanager_types.nim index b356e661b..ee61912a6 100644 --- a/beacon_chain/spec/eth2_apis/rest_keymanager_types.nim +++ b/beacon_chain/spec/eth2_apis/rest_keymanager_types.nim @@ -45,6 +45,10 @@ type GetDistributedKeystoresResponse* = object data*: seq[DistributedKeystoreInfo] + GetValidatorGasLimitResponse* = object + pubkey*: ValidatorPubKey + gas_limit*: uint64 + ImportRemoteKeystoresBody* = object remote_keys*: seq[RemoteKeystoreInfo] @@ -72,6 +76,13 @@ type pubkey*: ValidatorPubKey ethaddress*: Eth1Address + ListGasLimitResponse* = object + pubkey*: ValidatorPubKey + gas_limit*: uint64 + + SetGasLimitRequest* = object + gas_limit*: uint64 + KeystoreStatus* = enum error = "error" notActive = "not_active" diff --git a/beacon_chain/validator_client/common.nim b/beacon_chain/validator_client/common.nim index 74a973a37..a7b492426 100644 --- a/beacon_chain/validator_client/common.nim +++ b/beacon_chain/validator_client/common.nim @@ -34,7 +34,6 @@ const HISTORICAL_DUTIES_EPOCHS* = 2'u64 TIME_DELAY_FROM_SLOT* = 79.milliseconds SUBSCRIPTION_BUFFER_SLOTS* = 2'u64 - VALIDATOR_DEFAULT_GAS_LIMIT* = 30_000_000'u64 # Stand-in, reasonable default EPOCHS_BETWEEN_VALIDATOR_REGISTRATION* = 1 DelayBuckets* = [-Inf, -4.0, -2.0, -1.0, -0.5, -0.1, -0.05, @@ -49,7 +48,8 @@ type proposers*: seq[ValidatorPubKey] RegistrationKind* {.pure.} = enum - Cached, IncorrectTime, MissingIndex, MissingFee, ErrorSignature, NoSignature + Cached, IncorrectTime, MissingIndex, MissingFee, MissingGasLimit + ErrorSignature, NoSignature PendingValidatorRegistration* = object registration*: SignedValidatorRegistrationV1 @@ -508,8 +508,11 @@ proc addValidator*(vc: ValidatorClientRef, keystore: KeystoreData) = feeRecipient = vc.config.validatorsDir.getSuggestedFeeRecipient( keystore.pubkey, vc.config.defaultFeeRecipient).valueOr( vc.config.defaultFeeRecipient) + gasLimit = vc.config.validatorsDir.getSuggestedGasLimit( + keystore.pubkey, vc.config.suggestedGasLimit).valueOr( + vc.config.suggestedGasLimit) - discard vc.attachedValidators[].addValidator(keystore, feeRecipient) + discard vc.attachedValidators[].addValidator(keystore, feeRecipient, gasLimit) proc removeValidator*(vc: ValidatorClientRef, pubkey: ValidatorPubKey) {.async.} = @@ -545,6 +548,12 @@ proc getFeeRecipient*(vc: ValidatorClientRef, pubkey: ValidatorPubKey, else: Opt.none(Eth1Address) +proc getGasLimit*(vc: ValidatorClientRef, + pubkey: ValidatorPubKey): uint64 = + getSuggestedGasLimit( + vc.config.validatorsDir, pubkey, vc.config.suggestedGasLimit).valueOr: + vc.config.suggestedGasLimit + proc prepareProposersList*(vc: ValidatorClientRef, epoch: Epoch): seq[PrepareBeaconProposer] = var res: seq[PrepareBeaconProposer] @@ -585,7 +594,7 @@ proc isExpired*(vc: ValidatorClientRef, else: true -proc getValidatorRegistraion( +proc getValidatorRegistration( vc: ValidatorClientRef, validator: AttachedValidator, timestamp: Time, @@ -613,13 +622,13 @@ proc getValidatorRegistraion( debug "Could not get fee recipient for registration data", validator = shortLog(validator) return err(RegistrationKind.MissingFee) - + let gasLimit = vc.getGasLimit(validator.pubkey) var registration = SignedValidatorRegistrationV1( message: ValidatorRegistrationV1( fee_recipient: ExecutionAddress(data: distinctBase(feeRecipient.get())), - gas_limit: VALIDATOR_DEFAULT_GAS_LIMIT, + gas_limit: gasLimit, timestamp: uint64(timestamp.toUnix()), pubkey: validator.pubkey ) @@ -667,11 +676,12 @@ proc prepareRegistrationList*( errors = 0 indexMissing = 0 feeMissing = 0 + gasLimit = 0 cached = 0 timed = 0 for validator in vc.attachedValidators[].items(): - let res = vc.getValidatorRegistraion(validator, timestamp, fork) + let res = vc.getValidatorRegistration(validator, timestamp, fork) if res.isOk(): let preg = res.get() if preg.future.isNil(): @@ -687,6 +697,7 @@ proc prepareRegistrationList*( of RegistrationKind.ErrorSignature: inc(errors) of RegistrationKind.MissingIndex: inc(indexMissing) of RegistrationKind.MissingFee: inc(feeMissing) + of RegistrationKind.MissingGasLimit: inc(gasLimit) succeed = len(registrations) diff --git a/beacon_chain/validators/keystore_management.nim b/beacon_chain/validators/keystore_management.nim index 06856f331..4237f015e 100644 --- a/beacon_chain/validators/keystore_management.nim +++ b/beacon_chain/validators/keystore_management.nim @@ -34,6 +34,7 @@ const RemoteKeystoreFileName* = "remote_keystore.json" NetKeystoreFileName* = "network_keystore.json" FeeRecipientFilename* = "suggested_fee_recipient.hex" + GasLimitFilename* = "suggested_gas_limit.json" KeyNameSize* = 98 # 0x + hexadecimal key representation 96 characters. MaxKeystoreFileSize* = 65536 @@ -75,6 +76,7 @@ type validatorsDir*: string secretsDir*: string defaultFeeRecipient*: Eth1Address + defaultGasLimit*: uint64 getValidatorAndIdxFn*: ValidatorPubKeyToDataFn getBeaconTimeFn*: GetBeaconTimeFn @@ -94,6 +96,7 @@ func init*(T: type KeymanagerHost, validatorsDir: string, secretsDir: string, defaultFeeRecipient: Eth1Address, + defaultGasLimit: uint64, getValidatorAndIdxFn: ValidatorPubKeyToDataFn, getBeaconTimeFn: GetBeaconTimeFn): T = T(validatorPool: validatorPool, @@ -102,6 +105,7 @@ func init*(T: type KeymanagerHost, validatorsDir: validatorsDir, secretsDir: secretsDir, defaultFeeRecipient: defaultFeeRecipient, + defaultGasLimit: defaultGasLimit, getValidatorAndIdxFn: getValidatorAndIdxFn, getBeaconTimeFn: getBeaconTimeFn) @@ -699,9 +703,9 @@ iterator listLoadableKeystores*(config: AnyConf): KeystoreData = yield el type - FeeRecipientStatus* = enum + ValidatorConfigFileStatus* = enum noSuchValidator - invalidFeeRecipientFile + malformedConfigFile func validatorKeystoreDir( validatorsDir: string, pubkey: ValidatorPubKey): string = @@ -711,10 +715,14 @@ func feeRecipientPath(validatorsDir: string, pubkey: ValidatorPubKey): string = validatorsDir.validatorKeystoreDir(pubkey) / FeeRecipientFilename +func gasLimitPath(validatorsDir: string, + pubkey: ValidatorPubKey): string = + validatorsDir.validatorKeystoreDir(pubkey) / GasLimitFilename + proc getSuggestedFeeRecipient*( validatorsDir: string, pubkey: ValidatorPubKey, - defaultFeeRecipient: Eth1Address): Result[Eth1Address, FeeRecipientStatus] = + 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 @@ -735,10 +743,37 @@ proc getSuggestedFeeRecipient*( except CatchableError as exc: # Because the nonexistent validator case was already checked, any failure # at this point is serious enough to alert the user. - warn "getSuggestedFeeRecipient: failed loading fee recipient file; falling back to default fee recipient", - feeRecipientPath, + warn "Failed to load fee recipient file; falling back to default fee recipient", + feeRecipientPath, defaultFeeRecipient, err = exc.msg - err invalidFeeRecipientFile + err malformedConfigFile + +proc getSuggestedGasLimit*( + validatorsDir: string, + pubkey: ValidatorPubKey, + defaultGasLimit: uint64): Result[uint64, 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 gas + # limit per validator, so don't warn very loudly, if at all. + if not dirExists(validatorsDir.validatorKeystoreDir(pubkey)): + return err noSuchValidator + + let gasLimitPath = validatorsDir.gasLimitPath(pubkey) + if not fileExists(gasLimitPath): + return ok defaultGasLimit + try: + ok parseBiggestUInt(strutils.strip( + readFile(gasLimitPath), leading = false, trailing = true)) + except SerializationError as e: + warn "Invalid local gas limit file", gasLimitPath, + err= e.formatMsg(gasLimitPath) + err malformedConfigFile + except CatchableError as exc: + warn "Failed to load gas limit file; falling back to default gas limit", + gasLimitPath, defaultGasLimit, + err = exc.msg + err malformedConfigFile type KeystoreGenerationErrorKind* = enum @@ -1280,6 +1315,10 @@ func feeRecipientPath*(host: KeymanagerHost, pubkey: ValidatorPubKey): string = host.validatorsDir.feeRecipientPath(pubkey) +func gasLimitPath*(host: KeymanagerHost, + pubkey: ValidatorPubKey): string = + host.validatorsDir.gasLimitPath(pubkey) + proc removeFeeRecipientFile*(host: KeymanagerHost, pubkey: ValidatorPubKey): Result[void, string] = let path = host.feeRecipientPath(pubkey) @@ -1290,6 +1329,16 @@ proc removeFeeRecipientFile*(host: KeymanagerHost, return ok() +proc removeGasLimitFile*(host: KeymanagerHost, + pubkey: ValidatorPubKey): Result[void, string] = + let path = host.gasLimitPath(pubkey) + if fileExists(path): + let res = io2.removeFile(path) + if res.isErr: + return err res.error.ioErrorMsg + + return ok() + proc setFeeRecipient*(host: KeymanagerHost, pubkey: ValidatorPubKey, feeRecipient: Eth1Address): Result[void, string] = let validatorKeystoreDir = host.validatorKeystoreDir(pubkey) @@ -1299,16 +1348,34 @@ proc setFeeRecipient*(host: KeymanagerHost, pubkey: ValidatorPubKey, feeRecipien io2.writeFile(validatorKeystoreDir / FeeRecipientFilename, $feeRecipient) .mapErr(proc(e: auto): string = "Failed to write fee recipient file: " & $e) +proc setGasLimit*(host: KeymanagerHost, + pubkey: ValidatorPubKey, + gasLimit: uint64): Result[void, string] = + let validatorKeystoreDir = host.validatorKeystoreDir(pubkey) + + ? secureCreatePath(validatorKeystoreDir).mapErr(proc(e: auto): string = + "Could not create wallet directory [" & validatorKeystoreDir & "]: " & $e) + + io2.writeFile(validatorKeystoreDir / GasLimitFilename, $gasLimit) + .mapErr(proc(e: auto): string = "Failed to write gas limit file: " & $e) + proc getSuggestedFeeRecipient*( host: KeymanagerHost, - pubkey: ValidatorPubKey): Result[Eth1Address, FeeRecipientStatus] = + pubkey: ValidatorPubKey): Result[Eth1Address, ValidatorConfigFileStatus] = host.validatorsDir.getSuggestedFeeRecipient(pubkey, host.defaultFeeRecipient) +proc getSuggestedGasLimit*( + host: KeymanagerHost, + pubkey: ValidatorPubKey): Result[uint64, ValidatorConfigFileStatus] = + host.validatorsDir.getSuggestedGasLimit(pubkey, host.defaultGasLimit) + proc addValidator*(host: KeymanagerHost, keystore: KeystoreData) = let feeRecipient = host.getSuggestedFeeRecipient(keystore.pubkey).valueOr( host.defaultFeeRecipient) - v = host.validatorPool[].addValidator(keystore, feeRecipient) + gasLimit = host.getSuggestedGasLimit(keystore.pubkey).valueOr( + host.defaultGasLimit) + v = host.validatorPool[].addValidator(keystore, feeRecipient, gasLimit) if not isNil(host.getValidatorAndIdxFn): let data = host.getValidatorAndIdxFn(keystore.pubkey) diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index 6fe053190..81e321766 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -119,8 +119,9 @@ proc addValidators*(node: BeaconNode) = Opt.none(ValidatorIndex) feeRecipient = node.consensusManager[].getFeeRecipient( keystore.pubkey, index, epoch) + gasLimit = node.consensusManager[].getGasLimit(keystore.pubkey) - v = node.attachedValidators[].addValidator(keystore, feeRecipient) + v = node.attachedValidators[].addValidator(keystore, feeRecipient, gasLimit) v.updateValidator(data) proc getValidatorForDuties*( @@ -323,6 +324,10 @@ proc getFeeRecipient(node: BeaconNode, epoch: Epoch): Eth1Address = node.consensusManager[].getFeeRecipient(pubkey, Opt.some(validatorIdx), epoch) +proc getGasLimit(node: BeaconNode, + pubkey: ValidatorPubKey): uint64 = + node.consensusManager[].getGasLimit(pubkey) + from web3/engine_api_types import PayloadExecutionStatus from ../spec/datatypes/capella import BeaconBlock, ExecutionPayload from ../spec/datatypes/eip4844 import BeaconBlock, ExecutionPayload @@ -1292,15 +1297,13 @@ from std/times import epochTime proc getValidatorRegistration( node: BeaconNode, validator: AttachedValidator, epoch: Epoch): Future[Result[SignedValidatorRegistrationV1, string]] {.async.} = - # Stand-in, reasonable default - const gasLimit = 30000000 - let validatorIdx = validator.index.valueOr: # The validator index will be missing when the validator was not # activated for duties yet. We can safely skip the registration then. return let feeRecipient = node.getFeeRecipient(validator.pubkey, validatorIdx, epoch) + let gasLimit = node.getGasLimit(validator.pubkey) var validatorRegistration = SignedValidatorRegistrationV1( message: ValidatorRegistrationV1( fee_recipient: ExecutionAddress(data: distinctBase(feeRecipient)), diff --git a/beacon_chain/validators/validator_pool.nim b/beacon_chain/validators/validator_pool.nim index cdeaf9cfa..5168286f6 100644 --- a/beacon_chain/validators/validator_pool.nim +++ b/beacon_chain/validators/validator_pool.nim @@ -118,7 +118,7 @@ template count*(pool: ValidatorPool): int = proc addLocalValidator( pool: var ValidatorPool, keystore: KeystoreData, - feeRecipient: Eth1Address): AttachedValidator = + feeRecipient: Eth1Address, gasLimit: uint64): AttachedValidator = doAssert keystore.kind == KeystoreKind.Local let v = AttachedValidator( kind: ValidatorKind.Local, @@ -132,14 +132,16 @@ proc addLocalValidator( notice "Local validator attached", pubkey = v.pubkey, validator = shortLog(v), - initial_fee_recipient = feeRecipient.toHex() + initial_fee_recipient = feeRecipient.toHex(), + initial_gas_limit = gasLimit validators.set(pool.count().int64) v proc addRemoteValidator(pool: var ValidatorPool, keystore: KeystoreData, clients: seq[(RestClientRef, RemoteSignerInfo)], - feeRecipient: Eth1Address): AttachedValidator = + feeRecipient: Eth1Address, + gasLimit: uint64): AttachedValidator = doAssert keystore.kind == KeystoreKind.Remote let v = AttachedValidator( kind: ValidatorKind.Remote, @@ -153,7 +155,8 @@ proc addRemoteValidator(pool: var ValidatorPool, keystore: KeystoreData, pubkey = v.pubkey, validator = shortLog(v), remote_signer = $keystore.remotes, - initial_fee_recipient = feeRecipient.toHex() + initial_fee_recipient = feeRecipient.toHex(), + initial_gas_limit = gasLimit validators.set(pool.count().int64) @@ -161,7 +164,8 @@ proc addRemoteValidator(pool: var ValidatorPool, keystore: KeystoreData, proc addRemoteValidator(pool: var ValidatorPool, keystore: KeystoreData, - feeRecipient: Eth1Address): AttachedValidator = + feeRecipient: Eth1Address, + gasLimit: uint64): AttachedValidator = let httpFlags = if RemoteKeystoreFlag.IgnoreSSLVerification in keystore.flags: @@ -182,18 +186,21 @@ proc addRemoteValidator(pool: var ValidatorPool, res.add((client.get(), remote)) res - pool.addRemoteValidator(keystore, clients, feeRecipient) + pool.addRemoteValidator(keystore, clients, feeRecipient, gasLimit) proc addValidator*(pool: var ValidatorPool, keystore: KeystoreData, - feeRecipient: Eth1Address): AttachedValidator = + feeRecipient: Eth1Address, + gasLimit: uint64): AttachedValidator = pool.validators.withValue(keystore.pubkey, v): notice "Adding already-known validator", validator = shortLog(v[]) return v[] case keystore.kind - of KeystoreKind.Local: pool.addLocalValidator(keystore, feeRecipient) - of KeystoreKind.Remote: pool.addRemoteValidator(keystore, feeRecipient) + of KeystoreKind.Local: + pool.addLocalValidator(keystore, feeRecipient, gasLimit) + of KeystoreKind.Remote: + pool.addRemoteValidator(keystore, feeRecipient, gasLimit) proc getValidator*(pool: ValidatorPool, validatorKey: ValidatorPubKey): AttachedValidator = diff --git a/docs/the_nimbus_book/src/options.md b/docs/the_nimbus_book/src/options.md index fe2956ce5..f53888e19 100644 --- a/docs/the_nimbus_book/src/options.md +++ b/docs/the_nimbus_book/src/options.md @@ -112,6 +112,7 @@ The following options are available: --validator-monitor-totals Publish metrics to single 'totals' label for better collection performance when monitoring many validators (BETA) [=false]. --suggested-fee-recipient Suggested fee recipient. + --suggested-gas-limit Suggested gas limit [=30000000]. --payload-builder Enable external payload builder [=false]. --payload-builder-url Payload builder URL. diff --git a/tests/test_block_processor.nim b/tests/test_block_processor.nim index c3b31e9c7..fe7fec4c8 100644 --- a/tests/test_block_processor.nim +++ b/tests/test_block_processor.nim @@ -12,7 +12,7 @@ import std/[options, sequtils], unittest2, eth/keys, taskpools, - ../beacon_chain/beacon_clock, + ../beacon_chain/[conf, beacon_clock], ../beacon_chain/spec/[beaconstate, forks, helpers, state_transition], ../beacon_chain/spec/datatypes/eip4844, ../beacon_chain/gossip_processing/block_processor, @@ -48,7 +48,7 @@ suite "Block processor" & preset(): consensusManager = ConsensusManager.new( dag, attestationPool, quarantine, eth1Monitor, actionTracker, newClone(DynamicFeeRecipientsStore.init()), "", - default(Eth1Address)) + 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 baffad632..610cac0dc 100644 --- a/tests/test_keymanager_api.nim +++ b/tests/test_keymanager_api.nim @@ -52,6 +52,7 @@ const defaultBasePort = 49000 correctTokenValue = "some secret token" defaultFeeRecipient = Eth1Address.fromHex("0x000000000000000000000000000000000000DEAD") + defaultGasLimit = 30_000_000 newPrivateKeys = [ "0x598c9b81749ba7bb8eb37781027359e3ffe87d0e1579e21c453ce22af0c05e35", @@ -1083,6 +1084,168 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} = check: finalResultFromApi == defaultFeeRecipient + suite "Gas limit management" & testFlavour: + asyncTest "Missing Authorization header" & testFlavour: + let pubkey = ValidatorPubKey.fromHex(oldPublicKeys[0]).expect("valid key") + + block: + let + response = await client.listGasLimitPlain(pubkey) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + block: + let + response = await client.setGasLimitPlain( + pubkey, + default SetGasLimitRequest) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + block: + let + response = await client.deleteGasLimitPlain(pubkey, EmptyBody()) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + asyncTest "Invalid Authorization Header" & testFlavour: + let pubkey = ValidatorPubKey.fromHex(oldPublicKeys[0]).expect("valid key") + + block: + let + response = await client.listGasLimitPlain( + pubkey, + extraHeaders = @[("Authorization", "UnknownAuthScheme X")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + block: + let + response = await client.setGasLimitPlain( + pubkey, + default SetGasLimitRequest, + extraHeaders = @[("Authorization", "UnknownAuthScheme X")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + + block: + let + response = await client.deleteGasLimitPlain( + pubkey, + EmptyBody(), + extraHeaders = @[("Authorization", "UnknownAuthScheme X")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 401 + responseJson["message"].getStr() == InvalidAuthorizationError + + asyncTest "Invalid Authorization Token" & testFlavour: + let pubkey = ValidatorPubKey.fromHex(oldPublicKeys[0]).expect("valid key") + + block: + let + response = await client.listGasLimitPlain( + pubkey, + extraHeaders = @[("Authorization", "Bearer InvalidToken")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 403 + responseJson["message"].getStr() == InvalidAuthorizationError + + block: + let + response = await client.setGasLimitPlain( + pubkey, + default SetGasLimitRequest, + extraHeaders = @[("Authorization", "Bearer InvalidToken")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 403 + responseJson["message"].getStr() == InvalidAuthorizationError + + block: + let + response = await client.deleteGasLimitPlain( + pubkey, + EmptyBody(), + extraHeaders = @[("Authorization", "Bearer InvalidToken")]) + responseJson = Json.decode(response.data, JsonNode) + + check: + response.status == 403 + responseJson["message"].getStr() == InvalidAuthorizationError + + asyncTest "Obtaining the gas limit of a missing validator returns 404" & testFlavour: + let + pubkey = ValidatorPubKey.fromHex(unusedPublicKeys[0]).expect("valid key") + response = await client.listGasLimitPlain( + pubkey, + extraHeaders = @[("Authorization", "Bearer " & correctTokenValue)]) + + check: + response.status == 404 + + asyncTest "Setting the gas limit on a missing validator creates a record for it" & testFlavour: + let + pubkey = ValidatorPubKey.fromHex(unusedPublicKeys[1]).expect("valid key") + gasLimit = 20_000_000'u64 + + await client.setGasLimit(pubkey, gasLimit, correctTokenValue) + let resultFromApi = await client.listGasLimit(pubkey, correctTokenValue) + + check: + resultFromApi == gasLimit + + asyncTest "Obtaining the gas limit of an unconfigured validator returns the suggested default" & testFlavour: + let + pubkey = ValidatorPubKey.fromHex(oldPublicKeys[0]).expect("valid key") + resultFromApi = await client.listGasLimit(pubkey, correctTokenValue) + + check: + resultFromApi == defaultGasLimit + + asyncTest "Configuring the gas limit" & testFlavour: + let + pubkey = ValidatorPubKey.fromHex(oldPublicKeys[1]).expect("valid key") + firstGasLimit = 40_000_000'u64 + + await client.setGasLimit(pubkey, firstGasLimit, correctTokenValue) + + let firstResultFromApi = await client.listGasLimit(pubkey, correctTokenValue) + check: + firstResultFromApi == firstGasLimit + + let secondGasLimit = 50_000_000'u64 + await client.setGasLimit(pubkey, secondGasLimit, correctTokenValue) + + let secondResultFromApi = await client.listGasLimit(pubkey, correctTokenValue) + check: + secondResultFromApi == secondGasLimit + + await client.deleteGasLimit(pubkey, correctTokenValue) + let finalResultFromApi = await client.listGasLimit(pubkey, correctTokenValue) + check: + finalResultFromApi == defaultGasLimit + suite "ImportRemoteKeys/ListRemoteKeys/DeleteRemoteKeys" & testFlavour: asyncTest "Importing list of remote keys" & testFlavour: let