diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index d95ce898f0..f7aeb19e56 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -75,13 +75,12 @@ OK: 1/1 Fail: 0/1 Skip: 0/1 ## Block pool processing [Preset: mainnet] ```diff + Adding the same block twice returns a Duplicate error [Preset: mainnet] OK -+ Randao skip and non-skip OK + Simple block add&get [Preset: mainnet] OK + basic ops OK + updateHead updates head and headState [Preset: mainnet] OK + updateState sanity [Preset: mainnet] OK ``` -OK: 6/6 Fail: 0/6 Skip: 0/6 +OK: 5/5 Fail: 0/5 Skip: 0/5 ## Block processor [Preset: mainnet] ```diff + Reverse order block add & get [Preset: mainnet] OK @@ -615,4 +614,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: 343/348 Fail: 0/348 Skip: 5/348 diff --git a/beacon_chain/extras.nim b/beacon_chain/extras.nim index f31d5ecdb9..c3674df391 100644 --- a/beacon_chain/extras.nim +++ b/beacon_chain/extras.nim @@ -27,9 +27,6 @@ type ## Skip verification of BLS signatures in block processing. ## Predominantly intended for use in testing, e.g. to allow extra coverage. ## Also useful to avoid unnecessary work when replaying known, good blocks. - skipRandaoVerification ##\ - ## Skip verification of the proposer's randao reveal in block processing, but do ensure - ## that they set the randao reveal to the point at infinity. skipStateRootValidation ##\ ## Skip verification of block state root. strictVerification ##\ diff --git a/beacon_chain/rpc/rest_utils.nim b/beacon_chain/rpc/rest_utils.nim index 6249af1287..865c9ced43 100644 --- a/beacon_chain/rpc/rest_utils.nim +++ b/beacon_chain/rpc/rest_utils.nim @@ -308,3 +308,22 @@ const jsonMediaType* = MediaType.init("application/json") sszMediaType* = MediaType.init("application/octet-stream") textEventStreamMediaType* = MediaType.init("text/event-stream") + +proc verifyRandao*( + node: BeaconNode, slot: Slot, proposer: ValidatorIndex, + randao: ValidatorSig, skip_randao_verification: bool): bool = + let + proposer_pubkey = node.dag.validatorKey(proposer) + if proposer_pubkey.isNone(): + return false + + if skip_randao_verification: + randao == ValidatorSig.infinity() + else: + let + fork = node.dag.forkAtEpoch(slot.epoch) + genesis_validators_root = node.dag.genesis_validators_root + + verify_epoch_signature( + fork, genesis_validators_root, slot.epoch, proposer_pubkey.get(), + randao) diff --git a/beacon_chain/rpc/rest_validator_api.nim b/beacon_chain/rpc/rest_validator_api.nim index 37c1fb17fe..89c911e178 100644 --- a/beacon_chain/rpc/rest_validator_api.nim +++ b/beacon_chain/rpc/rest_validator_api.nim @@ -308,8 +308,9 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = # https://ethereum.github.io/beacon-APIs/#/Validator/produceBlockV2 router.api(MethodGet, "/eth/v2/validator/blocks/{slot}") do ( - slot: Slot, randao_reveal: Option[ValidatorSig], - graffiti: Option[GraffitiBytes], skip_randao_verification: Option[string]) -> RestApiResponse: + slot: Slot, randao_reveal: Option[ValidatorSig], + graffiti: Option[GraffitiBytes], + skip_randao_verification: Option[string]) -> RestApiResponse: let message = block: let qslot = block: @@ -363,13 +364,18 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonError(Http503, BeaconNodeInSyncError, $res.error()) res.get() - let proposer = node.dag.getProposer(qhead, qslot) + let + proposer = node.dag.getProposer(qhead, qslot) if proposer.isNone(): return RestApiResponse.jsonError(Http400, ProposerNotFoundError) + + if not node.verifyRandao( + qslot, proposer.get(), qrandao, qskip_randao_verification): + return RestApiResponse.jsonError(Http400, InvalidRandaoRevealValue) + let res = await makeBeaconBlockForHeadAndSlot[bellatrix.ExecutionPayload]( - node, qrandao, proposer.get(), qgraffiti, qhead, qslot, - qskip_randao_verification) + node, qrandao, proposer.get(), qgraffiti, qhead, qslot) if res.isErr(): return RestApiResponse.jsonError(Http400, res.error()) res.get() @@ -378,8 +384,9 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = # https://ethereum.github.io/beacon-APIs/#/Validator/produceBlindedBlock # https://github.com/ethereum/beacon-APIs/blob/v2.3.0/apis/validator/blinded_block.yaml router.api(MethodGet, "/eth/v1/validator/blinded_blocks/{slot}") do ( - slot: Slot, randao_reveal: Option[ValidatorSig], - graffiti: Option[GraffitiBytes]) -> RestApiResponse: + slot: Slot, randao_reveal: Option[ValidatorSig], + graffiti: Option[GraffitiBytes], + skip_randao_verification: Option[string]) -> RestApiResponse: ## Requests a beacon node to produce a valid blinded block, which can then ## be signed by a validator. A blinded block is a block with only a ## transactions root, rather than a full transactions list. @@ -408,6 +415,15 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonError(Http400, InvalidSlotValueError, "Slot cannot be in the future") res + let qskip_randao_verification = + if skip_randao_verification.isNone(): + false + else: + let res = skip_randao_verification.get() + if res.isErr() or res.get() != "": + return RestApiResponse.jsonError(Http400, + InvalidSkipRandaoVerificationValue) + true let qrandao = if randao_reveal.isNone(): return RestApiResponse.jsonError(Http400, MissingRandaoRevealValue) @@ -439,6 +455,10 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = if proposer.isNone(): return RestApiResponse.jsonError(Http400, ProposerNotFoundError) + if not node.verifyRandao( + qslot, proposer.get(), qrandao, qskip_randao_verification): + return RestApiResponse.jsonError(Http400, InvalidRandaoRevealValue) + template responsePlain(response: untyped): untyped = if contentType == sszMediaType: RestApiResponse.sszResponse(response) diff --git a/beacon_chain/spec/state_transition_block.nim b/beacon_chain/spec/state_transition_block.nim index eab94851e9..e57833985f 100644 --- a/beacon_chain/spec/state_transition_block.nim +++ b/beacon_chain/spec/state_transition_block.nim @@ -92,10 +92,7 @@ proc process_randao( # Verify RANDAO reveal let epoch = state.get_current_epoch() - if skipRandaoVerification in flags: - if body.randao_reveal.toRaw != ValidatorSig.infinity.toRaw: - return err("process_randao: expected point-at-infinity for skipRandaoVerification") - elif skipBlsValidation notin flags: + if skipBlsValidation notin flags and body.randao_reveal isnot TrustedSig: let proposer_pubkey = state.validators.item(proposer_index.get).pubkey # `state_transition.makeBeaconBlock` ensures this is run with a trusted @@ -103,10 +100,7 @@ proc process_randao( # epoch signatures still have to be verified. if not verify_epoch_signature( state.fork, state.genesis_validators_root, epoch, proposer_pubkey, - when body.randao_reveal is ValidatorSig: - body.randao_reveal - else: - isomorphicCast[ValidatorSig](body.randao_reveal)): + body.randao_reveal): return err("process_randao: invalid epoch signature") diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index 26eb15a2aa..a82b1cb395 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -432,7 +432,6 @@ proc makeBeaconBlockForHeadAndSlot*[EP]( node: BeaconNode, randao_reveal: ValidatorSig, validator_index: ValidatorIndex, graffiti: GraffitiBytes, head: BlockRef, slot: Slot, - skip_randao_verification_bool: bool, execution_payload: Opt[EP], transactions_root: Opt[Eth2Digest], execution_payload_root: Opt[Eth2Digest]): @@ -455,9 +454,9 @@ proc makeBeaconBlockForHeadAndSlot*[EP]( let state = maybeState.get payloadFut = - if executionPayload.isSome: + if execution_payload.isSome: let fut = newFuture[Opt[EP]]("given-payload") - fut.complete(executionPayload) + fut.complete(execution_payload) fut elif slot.epoch < node.dag.cfg.BELLATRIX_FORK_EPOCH or not ( @@ -510,10 +509,7 @@ proc makeBeaconBlockForHeadAndSlot*[EP]( (static(default(SignedBLSToExecutionChangeList))), noRollback, # Temporary state - no need for rollback cache, - # makeBeaconBlock doesn't verify BLS at all, but does have special case - # for skipRandaoVerification separately - verificationFlags = - if skip_randao_verification_bool: {skipRandaoVerification} else: {}, + verificationFlags = {}, transactions_root = transactions_root, execution_payload_root = execution_payload_root).mapErr do (error: cstring) -> string: # This is almost certainly a bug, but it's complex enough that there's a @@ -530,21 +526,10 @@ proc makeBeaconBlockForHeadAndSlot*[EP]( node: BeaconNode, randao_reveal: ValidatorSig, validator_index: ValidatorIndex, graffiti: GraffitiBytes, head: BlockRef, slot: Slot): - Future[ForkedBlockResult] {.async.} = - return await makeBeaconBlockForHeadAndSlot[EP]( - node, randao_reveal, validator_index, graffiti, head, slot, - false, execution_payload = Opt.none(EP), - transactions_root = Opt.none(Eth2Digest), - execution_payload_root = Opt.none(Eth2Digest)) - -proc makeBeaconBlockForHeadAndSlot*[EP]( - node: BeaconNode, randao_reveal: ValidatorSig, - validator_index: ValidatorIndex, graffiti: GraffitiBytes, head: BlockRef, - slot: Slot, skip_randao_verification: bool): - Future[ForkedBlockResult] {.async.} = - return await makeBeaconBlockForHeadAndSlot[EP]( + Future[ForkedBlockResult] = + return makeBeaconBlockForHeadAndSlot[EP]( node, randao_reveal, validator_index, graffiti, head, slot, - skip_randao_verification, execution_payload = Opt.none(EP), + execution_payload = Opt.none(EP), transactions_root = Opt.none(Eth2Digest), execution_payload_root = Opt.none(Eth2Digest)) @@ -695,7 +680,6 @@ proc getBlindedBlockParts( let newBlock = await makeBeaconBlockForHeadAndSlot[bellatrix.ExecutionPayload]( node, randao, validator_index, graffiti, head, slot, - skip_randao_verification_bool = false, execution_payload = Opt.some shimExecutionPayload, transactions_root = Opt.some executionPayloadHeader.get.transactions_root, execution_payload_root = diff --git a/tests/test_blockchain_dag.nim b/tests/test_blockchain_dag.nim index c190ef4e60..03df4047c3 100644 --- a/tests/test_blockchain_dag.nim +++ b/tests/test_blockchain_dag.nim @@ -206,74 +206,6 @@ suite "Block pool processing" & preset(): # Getting an EpochRef should not result in states being stored db.getStateRoot(stateCheckpoint.bid.root, stateCheckpoint.slot).isOk() - test "Randao skip and non-skip": - process_slots( - dag.cfg, state[], getStateField( - state[], slot) + 1, cache, info, {}).expect("can advance 1") - - let - proposer_index = get_beacon_proposer_index( - state[], cache, getStateField(state[], slot)) - privKey = MockPrivKeys[proposer_index.get] - randao_reveal = get_epoch_signature( - getStateField(state[], fork), - getStateField(state[], genesis_validators_root), - getStateField(state[], slot).epoch, privKey).toValidatorSig() - var bad_randao_reveal = randao_reveal - bad_randao_reveal.blob[5] += 1 - - block: # bad randao + no skip = bad - let - tmpState = assignClone(state[]) - message = makeBeaconBlock( - dag.cfg, tmpState[], proposer_index.get(), - bad_randao_reveal, - getStateField(tmpState[], eth1_data), - default(GraffitiBytes), @[], @[], BeaconBlockExits(), - default(SyncAggregate), default(ExecutionPayload), - default(SignedBLSToExecutionChangeList), - noRollback, cache) - check: message.isErr - - block: # bad randao + skip = bad - let - tmpState = assignClone(state[]) - message = makeBeaconBlock( - dag.cfg, tmpState[], proposer_index.get(), - bad_randao_reveal, - getStateField(tmpState[], eth1_data), - default(GraffitiBytes), @[], @[], BeaconBlockExits(), - default(SyncAggregate), default(ExecutionPayload), - default(SignedBLSToExecutionChangeList), - noRollback, cache, {skipRandaoVerification}) - check: message.isErr - - block: # infinity + no skip = bad - let - tmpState = assignClone(state[]) - message = makeBeaconBlock( - dag.cfg, tmpState[], proposer_index.get(), - ValidatorSig.infinity(), - getStateField(tmpState[], eth1_data), - default(GraffitiBytes), @[], @[], BeaconBlockExits(), - default(SyncAggregate), default(ExecutionPayload), - default(SignedBLSToExecutionChangeList), - noRollback, cache, {}) - check: message.isErr - - block: # Infinity + skip = ok! - let - tmpState = assignClone(state[]) - message = makeBeaconBlock( - dag.cfg, tmpState[], proposer_index.get(), - ValidatorSig.infinity(), - getStateField(tmpState[], eth1_data), - default(GraffitiBytes), @[], @[], BeaconBlockExits(), - default(SyncAggregate), default(ExecutionPayload), - default(SignedBLSToExecutionChangeList), - noRollback, cache, {skipRandaoVerification}) - check: message.isOk - test "Adding the same block twice returns a Duplicate error" & preset(): let b10 = dag.addHeadBlock(verifier, b1, nilPhase0Callback)