From d7c5bc0397240898d8fa8c7bfc9be37e886f5e97 Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Sun, 19 May 2024 04:49:43 +0300 Subject: [PATCH] [VC] Add builderBoostFactor support. (#6294) * Initial commit. * Replace localBlockValueBoost with builderBoostFactor. * Add test. * Update AllTests. * Update options.md * Recover `localBlockValueBoost` for BN-only mode. * Address review comments. --- AllTests-mainnet.md | 5 + beacon_chain/conf.nim | 10 +- beacon_chain/rpc/rest_validator_api.nim | 14 ++- .../spec/eth2_apis/rest_validator_calls.nim | 3 +- beacon_chain/validator_client/api.nim | 7 +- .../validator_client/block_service.nim | 1 + beacon_chain/validators/beacon_validators.nim | 81 +++++++++--- tests/all_tests.nim | 5 +- tests/test_beacon_validators.nim | 115 ++++++++++++++++++ 9 files changed, 219 insertions(+), 22 deletions(-) create mode 100644 tests/test_beacon_validators.nim diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 71e6332b9..534e465dd 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -80,6 +80,11 @@ OK: 7/7 Fail: 0/7 Skip: 0/7 + basics OK ``` OK: 2/2 Fail: 0/2 Skip: 0/2 +## Beacon validators test suite +```diff ++ builderBetterBid(builderBoostFactor) test OK +``` +OK: 1/1 Fail: 0/1 Skip: 0/1 ## Blinded block conversions ```diff + Bellatrix toSignedBlindedBlock OK diff --git a/beacon_chain/conf.nim b/beacon_chain/conf.nim index 248b77a0b..c79631c08 100644 --- a/beacon_chain/conf.nim +++ b/beacon_chain/conf.nim @@ -643,7 +643,8 @@ type # Flag name and semantics borrowed from Prysm # https://github.com/prysmaticlabs/prysm/pull/12227/files localBlockValueBoost* {. - desc: "Increase execution layer block values for builder bid comparison by a percentage" + desc: "Increase execution layer block values for builder bid " & + "comparison by a percentage" defaultValue: 10 name: "local-block-value-boost" .}: uint8 @@ -1031,6 +1032,13 @@ type defaultValue: false name: "distributed".}: bool + builderBoostFactor* {. + desc: "Percentage multiplier to apply to the builder's payload value " & + "when choosing between a builder payload header and payload " & + "from the paired execution node." + defaultValue: 100, + name: "builder-boost-factor".}: uint64 + beaconNodes* {. desc: "URL addresses to one or more beacon node HTTP REST APIs", defaultValue: @[defaultBeaconNodeUri] diff --git a/beacon_chain/rpc/rest_validator_api.nim b/beacon_chain/rpc/rest_validator_api.nim index b1b1c17e7..545994ecf 100644 --- a/beacon_chain/rpc/rest_validator_api.nim +++ b/beacon_chain/rpc/rest_validator_api.nim @@ -571,7 +571,8 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = router.api(MethodGet, "/eth/v3/validator/blocks/{slot}") do ( slot: Slot, randao_reveal: Option[ValidatorSig], graffiti: Option[GraffitiBytes], - skip_randao_verification: Option[string]) -> RestApiResponse: + skip_randao_verification: Option[string], + builder_boost_factor: Option[uint64]) -> RestApiResponse: let contentType = preferredContentType(jsonMediaType, sszMediaType).valueOr: return RestApiResponse.jsonError(Http406, ContentNotAcceptableError) @@ -630,6 +631,14 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = if not tres.executionValid: return RestApiResponse.jsonError(Http503, BeaconNodeInSyncError) tres + qboostFactor {.used.} = + if builder_boost_factor.isNone(): + 100'u64 + else: + let res = builder_boost_factor.get() + if res.isErr(): + return RestApiResponse.jsonError(Http400, ) + res.get() proposer = node.dag.getProposer(qhead, qslot).valueOr: return RestApiResponse.jsonError(Http400, ProposerNotFoundError) @@ -641,7 +650,8 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = when consensusFork >= ConsensusFork.Deneb: let message = (await node.makeMaybeBlindedBeaconBlockForHeadAndSlot( - consensusFork, qrandao, qgraffiti, qhead, qslot)).valueOr: + consensusFork, qrandao, qgraffiti, qhead, qslot, + qboostFactor)).valueOr: # HTTP 400 error is only for incorrect parameters. return RestApiResponse.jsonError(Http500, error) headers = consensusFork.getMaybeBlindedHeaders( diff --git a/beacon_chain/spec/eth2_apis/rest_validator_calls.nim b/beacon_chain/spec/eth2_apis/rest_validator_calls.nim index e7c907079..7ee76bb68 100644 --- a/beacon_chain/spec/eth2_apis/rest_validator_calls.nim +++ b/beacon_chain/spec/eth2_apis/rest_validator_calls.nim @@ -48,7 +48,8 @@ proc produceBlockV2Plain*( proc produceBlockV3Plain*( slot: Slot, randao_reveal: ValidatorSig, - graffiti: GraffitiBytes + graffiti: GraffitiBytes, + builder_boost_factor: uint64 ): RestPlainResponse {. rest, endpoint: "/eth/v3/validator/blocks/{slot}", accept: preferSSZ, meth: MethodGet.} diff --git a/beacon_chain/validator_client/api.nim b/beacon_chain/validator_client/api.nim index ceb43b923..75d171ddc 100644 --- a/beacon_chain/validator_client/api.nim +++ b/beacon_chain/validator_client/api.nim @@ -2118,6 +2118,7 @@ proc produceBlockV3*( slot: Slot, randao_reveal: ValidatorSig, graffiti: GraffitiBytes, + builder_boost_factor: uint64, strategy: ApiStrategyKind ): Future[ProduceBlockResponseV3] {.async.} = const @@ -2133,7 +2134,8 @@ proc produceBlockV3*( SlotDuration, ViableNodeStatus, {BeaconNodeRole.BlockProposalData}, - produceBlockV3Plain(it, slot, randao_reveal, graffiti)): + produceBlockV3Plain(it, slot, randao_reveal, graffiti, + builder_boost_factor)): if apiResponse.isErr(): handleCommunicationError() ApiResponse[ProduceBlockResponseV3].err(apiResponse.error) @@ -2189,7 +2191,8 @@ proc produceBlockV3*( SlotDuration, ViableNodeStatus, {BeaconNodeRole.BlockProposalData}, - produceBlockV3Plain(it, slot, randao_reveal, graffiti)): + produceBlockV3Plain(it, slot, randao_reveal, graffiti, + builder_boost_factor)): if apiResponse.isErr(): handleCommunicationError() false diff --git a/beacon_chain/validator_client/block_service.nim b/beacon_chain/validator_client/block_service.nim index 3e2e18cea..30198ac9e 100644 --- a/beacon_chain/validator_client/block_service.nim +++ b/beacon_chain/validator_client/block_service.nim @@ -243,6 +243,7 @@ proc publishBlockV3(vc: ValidatorClientRef, currentSlot, slot: Slot, maybeBlock = try: await vc.produceBlockV3(slot, randao_reveal, graffiti, + vc.config.builderBoostFactor, ApiStrategyKind.Best) except ValidatorApiError as exc: warn "Unable to retrieve block data", reason = exc.getFailureReason() diff --git a/beacon_chain/validators/beacon_validators.nim b/beacon_chain/validators/beacon_validators.nim index 334a55c89..c4375f3ca 100644 --- a/beacon_chain/validators/beacon_validators.nim +++ b/beacon_chain/validators/beacon_validators.nim @@ -97,6 +97,22 @@ type engineBid: Opt[EngineBid] builderBid: Opt[BuilderBid[SBBB]] + BoostFactorKind {.pure.} = enum + Local, Builder + + BoostFactor = object + case kind: BoostFactorKind + of BoostFactorKind.Local: + value8: uint8 + of BoostFactorKind.Builder: + value64: uint64 + +func init(t: typedesc[BoostFactor], value: uint8): BoostFactor = + BoostFactor(kind: BoostFactorKind.Local, value8: value) + +func init(t: typedesc[BoostFactor], value: uint64): BoostFactor = + BoostFactor(kind: BoostFactorKind.Builder, value64: value) + proc getValidator*(validators: auto, pubkey: ValidatorPubKey): Opt[ValidatorAndIndex] = let idx = validators.findIt(it.pubkey == pubkey) @@ -1107,8 +1123,8 @@ proc collectBids( engineBid: engineBid, builderBid: builderBid) -func builderBetterBid( - localBlockValueBoost: uint8, builderValue: UInt256, engineValue: Wei): bool = +func builderBetterBid(localBlockValueBoost: uint8, + builderValue: UInt256, engineValue: Wei): bool = # Scale down to ensure no overflows; if lower few bits would have been # otherwise decisive, was close enough not to matter. Calibrate to let # uint8-range percentages avoid overflowing. @@ -1121,13 +1137,47 @@ func builderBetterBid( scaledBuilderValue > scaledEngineValue * (localBlockValueBoost.uint16 + 100).u256 +func builderBetterBid*(builderBoostFactor: uint64, + builderValue: UInt256, engineValue: Wei): bool = + if builderBoostFactor == 0'u64: + false + elif builderBoostFactor == 100'u64: + builderValue >= engineValue + elif builderBoostFactor == high(uint64): + true + else: + let + multiplier = builderBoostFactor.u256 + multipledBuilderValue = builderValue * multiplier + overflow = + if builderValue == UInt256.zero: + false + else: + builderValue != multipledBuilderValue div multiplier + + if overflow: + # In case of overflow we will use `builderValue`. + true + else: + (multipledBuilderValue div 100) >= engineValue + +func builderBetterBid(boostFactor: BoostFactor, builderValue: UInt256, + engineValue: Wei): bool = + case boostFactor.kind + of BoostFactorKind.Local: + builderBetterBid(boostFactor.value8, builderValue, engineValue) + of BoostFactorKind.Builder: + builderBetterBid(boostFactor.value64, builderValue, engineValue) + proc proposeBlockAux( SBBB: typedesc, EPS: typedesc, node: BeaconNode, validator: AttachedValidator, validator_index: ValidatorIndex, head: BlockRef, slot: Slot, randao: ValidatorSig, fork: Fork, genesis_validators_root: Eth2Digest, - localBlockValueBoost: uint8): Future[BlockRef] {.async: (raises: [CancelledError]).} = + localBlockValueBoost: uint8 +): Future[BlockRef] {.async: (raises: [CancelledError]).} = let + boostFactor = BoostFactor.init(localBlockValueBoost) graffitiBytes = node.getGraffitiBytes(validator) payloadBuilderClient = node.getPayloadBuilderClient(validator_index.distinctBase).valueOr(nil) @@ -1139,7 +1189,7 @@ proc proposeBlockAux( useBuilderBlock = if collectedBids.builderBid.isSome(): collectedBids.engineBid.isNone() or builderBetterBid( - localBlockValueBoost, + boostFactor, collectedBids.builderBid.value().executionPayloadValue, collectedBids.engineBid.value().executionPayloadValue) else: @@ -1257,11 +1307,13 @@ proc proposeBlockAux( return newBlockRef.get() -proc proposeBlock(node: BeaconNode, - validator: AttachedValidator, - validator_index: ValidatorIndex, - head: BlockRef, - slot: Slot): Future[BlockRef] {.async: (raises: [CancelledError]).} = +proc proposeBlock( + node: BeaconNode, + validator: AttachedValidator, + validator_index: ValidatorIndex, + head: BlockRef, + slot: Slot +): Future[BlockRef] {.async: (raises: [CancelledError]).} = if head.slot >= slot: # We should normally not have a head newer than the slot we're proposing for # but this can happen if block proposal is delayed @@ -1972,7 +2024,8 @@ proc registerDuties*(node: BeaconNode, wallSlot: Slot) {.async: (raises: [Cancel proc makeMaybeBlindedBeaconBlockForHeadAndSlotImpl[ResultType]( node: BeaconNode, consensusFork: static ConsensusFork, randao_reveal: ValidatorSig, graffiti: GraffitiBytes, - head: BlockRef, slot: Slot): Future[ResultType] {.async: (raises: [CancelledError]).} = + head: BlockRef, slot: Slot, + builderBoostFactor: uint64): Future[ResultType] {.async: (raises: [CancelledError]).} = let proposer = node.dag.getProposer(head, slot).valueOr: return ResultType.err( @@ -1981,7 +2034,6 @@ proc makeMaybeBlindedBeaconBlockForHeadAndSlotImpl[ResultType]( payloadBuilderClient = node.getPayloadBuilderClient(proposer.distinctBase).valueOr(nil) - localBlockValueBoost = node.config.localBlockValueBoost collectedBids = await collectBids(consensusFork.SignedBlindedBeaconBlock, @@ -1993,7 +2045,7 @@ proc makeMaybeBlindedBeaconBlockForHeadAndSlotImpl[ResultType]( useBuilderBlock = if collectedBids.builderBid.isSome(): collectedBids.engineBid.isNone() or builderBetterBid( - localBlockValueBoost, + BoostFactor.init(builderBoostFactor), collectedBids.builderBid.value().executionPayloadValue, collectedBids.engineBid.value().executionPayloadValue) else: @@ -2039,11 +2091,12 @@ proc makeMaybeBlindedBeaconBlockForHeadAndSlotImpl[ResultType]( proc makeMaybeBlindedBeaconBlockForHeadAndSlot*( node: BeaconNode, consensusFork: static ConsensusFork, randao_reveal: ValidatorSig, graffiti: GraffitiBytes, - head: BlockRef, slot: Slot): auto = + head: BlockRef, slot: Slot, builderBoostFactor: uint64): auto = type ResultType = Result[tuple[ blck: consensusFork.MaybeBlindedBeaconBlock, executionValue: Opt[UInt256], consensusValue: Opt[UInt256]], string] makeMaybeBlindedBeaconBlockForHeadAndSlotImpl[ResultType]( - node, consensusFork, randao_reveal, graffiti, head, slot) + node, consensusFork, randao_reveal, graffiti, head, slot, + builderBoostFactor) diff --git a/tests/all_tests.nim b/tests/all_tests.nim index 5371e5fc9..18f698da6 100644 --- a/tests/all_tests.nim +++ b/tests/all_tests.nim @@ -58,9 +58,10 @@ import # Unit test ./consensus_spec/all_tests as consensus_all_tests, ./slashing_protection/test_fixtures, ./slashing_protection/test_slashing_protection_db, - ./test_validator_client + ./test_validator_client, + ./test_beacon_validators when not defined(windows): import ./test_keymanager_api -summarizeLongTests("AllTests") \ No newline at end of file +summarizeLongTests("AllTests") diff --git a/tests/test_beacon_validators.nim b/tests/test_beacon_validators.nim new file mode 100644 index 000000000..3a376a136 --- /dev/null +++ b/tests/test_beacon_validators.nim @@ -0,0 +1,115 @@ +# 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 unittest2, results, chronos, stint +import ../beacon_chain/validators/beacon_validators, + ../beacon_chain/spec/datatypes/base, + ../beacon_chain/spec/eth2_apis/eth2_rest_serialization + +suite "Beacon validators test suite": + test "builderBetterBid(builderBoostFactor) test": + const TestVectors = + [ + ( + # zero comparison + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 0'u64, + false + ), + ( + # less or equal + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 100'u64, + true + ), + ( + # overflow #1 + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 101'u64, + true + ), + ( + # overflow #2 + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 0xffffffffffffffff'u64, + true + ), + ( + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 0'u64, + false + ), + ( + # less + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 100'u64, + false + ), + ( + # overflow #1 + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 101'u64, + true + ), + ( + # overflow #2 + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + 0xffffffffffffffff'u64, + true + ), + ( + # zeros + "0", + "0", + 0'u64, + false + ), + ( + # 10 * (50 div 100) < 6 + "a", + "6", + 50'u64, + false + ), + ( + # 10 * (50 div 100) >= 5 + "a", + "5", + 50'u64, + true + ), + ( + # 5 * (150 div 100) < 8 + "5", + "8", + 150'u64, + false + ), + ( + # 5 * (150 div 100) >= 7 + "5", + "7", + 150'u64, + true + ), + ] + + for index, vector in TestVectors.pairs(): + let + builderValue = strictParse(vector[0], UInt256, 16).get() + engineValue = Wei(strictParse(vector[1], UInt256, 16).get()) + check builderBetterBid(vector[2], builderValue, engineValue) == vector[3]