From f5791122f6d50bb33e164de7d7bae8f3ea6e6afc Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 20 Oct 2021 11:58:38 +0200 Subject: [PATCH] widen allowed specs for validator client The validator client was only able to connect to beacon nodes exposing the exact same set of spec constants that are locally known via their config/spec REST API. However, that set of spec constants is dynamic. As the validator client only requires a subset of relevant constants, this may lead to compatible specs being rejected. This patch widens the allowed specs by only verifying that the required set of constants are present in the spec response, ignoring any spec constants that are not locally known, and ignoring missing spec constants that are locally known but not included by the remote beacon node when not relevant for operation of the validator client. --- .../eth2_apis/eth2_rest_serialization.nim | 9 ++++- .../spec/eth2_apis/rest_config_calls.nim | 4 +++ beacon_chain/spec/eth2_apis/rest_types.nim | 34 +++++++++++++++++++ beacon_chain/validator_client/api.nim | 3 +- beacon_chain/validator_client/common.nim | 2 +- 5 files changed, 49 insertions(+), 3 deletions(-) diff --git a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim index 1ffbd578f..4b0c1a1ec 100644 --- a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim +++ b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim @@ -71,6 +71,12 @@ type GetBlockV2Response | GetStateV2Response + # These types may be extended with additional fields in the future. + # Locally unknown fields are silently ignored when decoding them. + ExtensibleDecodeTypes* = + GetSpecResponse | + GetSpecVCResponse + SszDecodeTypes* = GetPhase0StateSszResponse | GetAltairStateSszResponse | @@ -957,10 +963,11 @@ proc encodeBytes*[T: EncodeArrays](value: T, proc decodeBytes*[T: DecodeTypes](t: typedesc[T], value: openarray[byte], contentType: string): RestResult[T] = + const isExtensibleType = t is ExtensibleDecodeTypes case contentType of "application/json": try: - ok RestJson.decode(value, T) + ok RestJson.decode(value, T, allowUnknownFields = isExtensibleType) except SerializationError as exc: err("Serialization error") else: diff --git a/beacon_chain/spec/eth2_apis/rest_config_calls.nim b/beacon_chain/spec/eth2_apis/rest_config_calls.nim index cb21397f7..c6b8dd065 100644 --- a/beacon_chain/spec/eth2_apis/rest_config_calls.nim +++ b/beacon_chain/spec/eth2_apis/rest_config_calls.nim @@ -20,6 +20,10 @@ proc getSpec*(): RestResponse[GetSpecResponse] {. rest, endpoint: "/eth/v1/config/spec", meth: MethodGet.} ## https://ethereum.github.io/beacon-APIs/#/Config/getSpec +proc getSpecVC*(): RestResponse[GetSpecVCResponse] {. + rest, endpoint: "/eth/v1/config/spec", meth: MethodGet.} + ## https://ethereum.github.io/beacon-APIs/#/Config/getSpec + proc getDepositContract*(): RestResponse[GetDepositContractResponse] {. rest, endpoint: "/eth/v1/config/deposit_contract", meth: MethodGet.} ## https://ethereum.github.io/beacon-APIs/#/Config/getDepositContract diff --git a/beacon_chain/spec/eth2_apis/rest_types.nim b/beacon_chain/spec/eth2_apis/rest_types.nim index df4d5fabb..580590c4d 100644 --- a/beacon_chain/spec/eth2_apis/rest_types.nim +++ b/beacon_chain/spec/eth2_apis/rest_types.nim @@ -306,6 +306,39 @@ type DOMAIN_SYNC_COMMITTEE*: DomainType DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF*: DomainType + # The `RestSpec` is a dynamic dictionary that includes version-specific spec + # constants. New versions may introduce new constants, and remove old ones. + # The Nimbus validator client fetches the remote spec to determine whether it + # is connected to a compatible beacon node. For this purpose, it only needs to + # verify a small set of relevant spec constants. To avoid rejecting a remote + # spec that includes all of those relevant spec constants, but that does not + # include all of the locally known spec constants, a separate type is defined + # that includes just the spec constants relevant for the validator client. + # Extra spec constants are silently ignored. + RestSpecVC* = object + # /!\ Keep in sync with `validator_client/api.nim` > `checkCompatible`. + MAX_VALIDATORS_PER_COMMITTEE*: uint64 + SLOTS_PER_EPOCH*: uint64 + SECONDS_PER_SLOT*: uint64 + EPOCHS_PER_ETH1_VOTING_PERIOD*: uint64 + SLOTS_PER_HISTORICAL_ROOT*: uint64 + EPOCHS_PER_HISTORICAL_VECTOR*: uint64 + EPOCHS_PER_SLASHINGS_VECTOR*: uint64 + HISTORICAL_ROOTS_LIMIT*: uint64 + VALIDATOR_REGISTRY_LIMIT*: uint64 + MAX_PROPOSER_SLASHINGS*: uint64 + MAX_ATTESTER_SLASHINGS*: uint64 + MAX_ATTESTATIONS*: uint64 + MAX_DEPOSITS*: uint64 + MAX_VOLUNTARY_EXITS*: uint64 + DOMAIN_BEACON_PROPOSER*: DomainType + DOMAIN_BEACON_ATTESTER*: DomainType + DOMAIN_RANDAO*: DomainType + DOMAIN_DEPOSIT*: DomainType + DOMAIN_VOLUNTARY_EXIT*: DomainType + DOMAIN_SELECTION_PROOF*: DomainType + DOMAIN_AGGREGATE_AND_PROOF*: DomainType + RestDepositContract* = object chain_id*: string address*: string @@ -371,6 +404,7 @@ type GetProposerDutiesResponse* = DataRootEnclosedObject[seq[RestProposerDuty]] GetSyncCommitteeDutiesResponse* = DataEnclosedObject[seq[RestSyncCommitteeDuty]] GetSpecResponse* = DataEnclosedObject[RestSpec] + GetSpecVCResponse* = DataEnclosedObject[RestSpecVC] GetStateFinalityCheckpointsResponse* = DataEnclosedObject[RestBeaconStatesFinalityCheckpoints] GetStateForkResponse* = DataEnclosedObject[Fork] GetStateRootResponse* = DataEnclosedObject[Eth2Digest] diff --git a/beacon_chain/validator_client/api.nim b/beacon_chain/validator_client/api.nim index c2b11a8d4..fee81f52b 100644 --- a/beacon_chain/validator_client/api.nim +++ b/beacon_chain/validator_client/api.nim @@ -16,7 +16,7 @@ proc checkCompatible*(vc: ValidatorClientRef, let info = try: debug "Requesting beacon node network configuration" - let res = await node.client.getSpec() + let res = await node.client.getSpecVC() res.data.data except CancelledError as exc: error "Configuration request was interrupted" @@ -55,6 +55,7 @@ proc checkCompatible*(vc: ValidatorClientRef, let genesisFlag = (genesis != vc.beaconGenesis) let configFlag = + # /!\ Keep in sync with `spec/eth2_apis/rest_types.nim` > `RestSpecVC`. info.MAX_VALIDATORS_PER_COMMITTEE != MAX_VALIDATORS_PER_COMMITTEE or info.SLOTS_PER_EPOCH != SLOTS_PER_EPOCH or info.SECONDS_PER_SLOT != SECONDS_PER_SLOT or diff --git a/beacon_chain/validator_client/common.nim b/beacon_chain/validator_client/common.nim index ead934be3..65bb0c298 100644 --- a/beacon_chain/validator_client/common.nim +++ b/beacon_chain/validator_client/common.nim @@ -69,7 +69,7 @@ type BeaconNodeServer* = object client*: RestClientRef endpoint*: string - config*: Option[RestSpec] + config*: Option[RestSpecVC] ident*: Option[string] genesis*: Option[RestGenesis] syncInfo*: Option[RestSyncInfo]