From a382498cfe48f07ba830503b225df37848b4f722 Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 17 Feb 2023 14:35:12 +0100 Subject: [PATCH] batch-verify BLS to execution change messages (#4637) --- .../block_clearance.nim | 2 +- .../consensus_object_pools/exit_pool.nim | 4 +- .../gossip_processing/batch_validation.nim | 36 ++++++++++++++++++ .../gossip_processing/eth2_processor.nim | 12 +++--- .../gossip_processing/gossip_validation.nim | 26 +++++++++++-- beacon_chain/nimbus_beacon_node.nim | 8 ++-- beacon_chain/spec/beaconstate.nim | 29 ++++++++++++++ beacon_chain/spec/signatures.nim | 3 +- beacon_chain/spec/signatures_batch.nim | 38 ++++++++++++++++++- beacon_chain/spec/state_transition_block.nim | 29 +------------- beacon_chain/validators/message_router.nim | 2 +- 11 files changed, 144 insertions(+), 45 deletions(-) diff --git a/beacon_chain/consensus_object_pools/block_clearance.nim b/beacon_chain/consensus_object_pools/block_clearance.nim index 2a5f4d855..dd164c1f5 100644 --- a/beacon_chain/consensus_object_pools/block_clearance.nim +++ b/beacon_chain/consensus_object_pools/block_clearance.nim @@ -260,7 +260,7 @@ proc addHeadBlock*( var sigs: seq[SignatureSet] if (let e = sigs.collectSignatureSets( signedBlock, dag.db.immutableValidators, - dag.clearanceState, cache); e.isErr()): + dag.clearanceState, dag.cfg.genesisFork(), cache); e.isErr()): # A PublicKey or Signature isn't on the BLS12-381 curve info "Unable to load signature sets", err = e.error() diff --git a/beacon_chain/consensus_object_pools/exit_pool.nim b/beacon_chain/consensus_object_pools/exit_pool.nim index 8eba1259a..e42489c3c 100644 --- a/beacon_chain/consensus_object_pools/exit_pool.nim +++ b/beacon_chain/consensus_object_pools/exit_pool.nim @@ -15,6 +15,8 @@ import ../spec/[helpers, state_transition_block], "."/[attestation_pool, blockchain_dag] +from ../spec/beaconstate import check_bls_to_execution_change + export base, deques, blockchain_dag const @@ -184,7 +186,7 @@ proc validateValidatorChangeMessage( proc validateValidatorChangeMessage( cfg: RuntimeConfig, state: ForkyBeaconState, msg: SignedBLSToExecutionChange): bool = - check_bls_to_execution_change(cfg, state, msg).isOk + check_bls_to_execution_change(cfg.genesisFork, state, msg, {}).isOk proc getValidatorChangeMessagesForBlock( subpool: var Deque, cfg: RuntimeConfig, state: ForkyBeaconState, diff --git a/beacon_chain/gossip_processing/batch_validation.nim b/beacon_chain/gossip_processing/batch_validation.nim index 8daf6808a..57c4332cb 100644 --- a/beacon_chain/gossip_processing/batch_validation.nim +++ b/beacon_chain/gossip_processing/batch_validation.nim @@ -456,3 +456,39 @@ proc scheduleContributionChecks*( contribution.beacon_block_root, contributionKey, contributionSig) ok((aggregatorFut, proofFut, contributionFut, contributionSig)) + +proc scheduleBlsToExecutionChangeCheck*( + batchCrypto: ref BatchCrypto, + genesisFork: Fork, genesis_validators_root: Eth2Digest, + signedBLSToExecutionChange: SignedBLSToExecutionChange): Result[tuple[ + blsToExecutionFut: Future[BatchResult], + sig: CookedSig], cstring] = + ## Schedule crypto verification of all signatures in a + ## SignedBLSToExecutionChange message + ## + ## The buffer is processed: + ## - when eager processing is enabled and the batch is full + ## - otherwise after 10ms (BatchAttAccumTime) + ## + ## This returns an error if crypto sanity checks failed + ## and a future with the deferred check otherwise. + + # Must be genesis fork + doAssert genesis_fork.previous_version == genesis_fork.current_version + + let + # Only called when matching already-known withdrawal credentials, so it's + # resistant to allowing loadWithCache DoSing + validatorChangePubkey = + signedBLSToExecutionChange.message.from_bls_pubkey.loadWithCache.valueOr: + return err("scheduleBlsToExecutionChangeCheck: cannot load BLS to withdrawals pubkey") + + validatorChangeSig = signedBLSToExecutionChange.signature.load().valueOr: + return err("scheduleBlsToExecutionChangeCheck: invalid validator change signature") + validatorChangeFut = batchCrypto.withBatch("scheduleContributionAndProofChecks.contribution"): + bls_to_execution_change_signature_set( + genesis_fork, genesis_validators_root, + signedBLSToExecutionChange.message, + validatorChangePubkey, validatorChangeSig) + + ok((validatorChangeFut, validatorChangeSig)) diff --git a/beacon_chain/gossip_processing/eth2_processor.nim b/beacon_chain/gossip_processing/eth2_processor.nim index ffa525e04..de6950dac 100644 --- a/beacon_chain/gossip_processing/eth2_processor.nim +++ b/beacon_chain/gossip_processing/eth2_processor.nim @@ -403,15 +403,17 @@ proc processSignedAggregateAndProof*( err(v.error()) proc processBlsToExecutionChange*( - self: var Eth2Processor, src: MsgSource, - blsToExecutionChange: SignedBLSToExecutionChange): ValidationRes = + self: ref Eth2Processor, src: MsgSource, + blsToExecutionChange: SignedBLSToExecutionChange): + Future[ValidationRes] {.async.} = logScope: blsToExecutionChange = shortLog(blsToExecutionChange) debug "BLS to execution change received" - let v = self.validatorChangePool[].validateBlsToExecutionChange( - blsToExecutionChange, self.getCurrentBeaconTime().slotOrZero.epoch) + let v = await self.validatorChangePool[].validateBlsToExecutionChange( + self.batchCrypto, blsToExecutionChange, + self.getCurrentBeaconTime().slotOrZero.epoch) if v.isOk(): trace "BLS to execution change validated" @@ -422,7 +424,7 @@ proc processBlsToExecutionChange*( debug "Dropping BLS to execution change", validationError = v.error beacon_attester_slashings_dropped.inc(1, [$v.error[0]]) - v + return v proc processAttesterSlashing*( self: var Eth2Processor, src: MsgSource, diff --git a/beacon_chain/gossip_processing/gossip_validation.nim b/beacon_chain/gossip_processing/gossip_validation.nim index 8d09b9953..5f6d37764 100644 --- a/beacon_chain/gossip_processing/gossip_validation.nim +++ b/beacon_chain/gossip_processing/gossip_validation.nim @@ -814,8 +814,9 @@ proc validateAggregate*( # https://github.com/ethereum/consensus-specs/blob/v1.3.0-rc.2/specs/capella/p2p-interface.md#bls_to_execution_change proc validateBlsToExecutionChange*( - pool: ValidatorChangePool, signed_address_change: SignedBLSToExecutionChange, - wallEpoch: Epoch): Result[void, ValidationError] = + pool: ValidatorChangePool, batchCrypto: ref BatchCrypto, + signed_address_change: SignedBLSToExecutionChange, + wallEpoch: Epoch): Future[Result[void, ValidationError]] {.async.} = # [IGNORE] `current_epoch >= CAPELLA_FORK_EPOCH`, where `current_epoch` is # defined by the current wall-clock time. if not (wallEpoch >= pool.dag.cfg.CAPELLA_FORK_EPOCH): @@ -834,11 +835,28 @@ proc validateBlsToExecutionChange*( return errIgnore("validateBlsToExecutionChange: can't validate against pre-Capella state") else: let res = check_bls_to_execution_change( - pool.dag.cfg, forkyState.data, signed_address_change) + pool.dag.cfg.genesisFork, forkyState.data, signed_address_change, + {skipBlsValidation}) if res.isErr: return errReject(res.error) - ok() + # BLS to execution change signatures are batch-verified + let deferredCrypto = batchCrypto.scheduleBlsToExecutionChangeCheck( + pool.dag.cfg.genesisFork, pool.dag.genesis_validators_root, + signed_address_change) + if deferredCrypto.isErr(): + return checkedReject(deferredCrypto.error) + + let (cryptoFut, sig) = deferredCrypto.get() + case await cryptoFut + of BatchResult.Invalid: + return checkedReject("validateBlsToExecutionChange: invalid signature") + of BatchResult.Timeout: + return errIgnore("validateBlsToExecutionChange: timeout checking signature") + of BatchResult.Valid: + discard # keep going only in this case + + return ok() # https://github.com/ethereum/consensus-specs/blob/v1.3.0-rc.2/specs/phase0/p2p-interface.md#attester_slashing proc validateAttesterSlashing*( diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim index fbe502ed7..809be184a 100644 --- a/beacon_chain/nimbus_beacon_node.nim +++ b/beacon_chain/nimbus_beacon_node.nim @@ -1543,11 +1543,13 @@ proc installMessageValidators(node: BeaconNode) = installSyncCommitteeeValidators(forkDigests.eip4844) template installBlsToExecutionChangeValidators(digest: auto) = - node.network.addValidator( + node.network.addAsyncValidator( getBlsToExecutionChangeTopic(digest), - proc(msg: SignedBLSToExecutionChange): ValidationResult = + proc(msg: SignedBLSToExecutionChange): + Future[ValidationResult] {.async.} = return toValidationResult( - node.processor[].processBlsToExecutionChange(MsgSource.gossip, msg))) + await node.processor.processBlsToExecutionChange( + MsgSource.gossip, msg))) installBlsToExecutionChangeValidators(forkDigests.capella) if node.dag.cfg.DENEB_FORK_EPOCH != FAR_FUTURE_EPOCH: diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 656bff848..454efffcd 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -696,6 +696,35 @@ proc check_attestation*( ok() +# https://github.com/ethereum/consensus-specs/blob/v1.3.0-rc.2/specs/capella/beacon-chain.md#new-process_bls_to_execution_change +proc check_bls_to_execution_change*( + genesisFork: Fork, state: capella.BeaconState | eip4844.BeaconState, + signed_address_change: SignedBLSToExecutionChange, flags: UpdateFlags): + Result[void, cstring] = + let address_change = signed_address_change.message + + if not (address_change.validator_index < state.validators.lenu64): + return err("process_bls_to_execution_change: invalid validator index") + + var withdrawal_credentials = + state.validators.item(address_change.validator_index).withdrawal_credentials + + if not (withdrawal_credentials.data[0] == BLS_WITHDRAWAL_PREFIX): + return err("process_bls_to_execution_change: invalid withdrawal prefix") + + if not (withdrawal_credentials.data.toOpenArray(1, 31) == + eth2digest(address_change.from_bls_pubkey.blob).data.toOpenArray(1, 31)): + return err("process_bls_to_execution_change: invalid withdrawal credentials") + + doAssert flags + {skipBlsValidation} == {skipBlsValidation} + if skipBlsValidation notin flags and + not verify_bls_to_execution_change_signature( + genesisFork, state.genesis_validators_root, signed_address_change, + address_change.from_bls_pubkey, signed_address_change.signature): + return err("process_bls_to_execution_change: invalid signature") + + ok() + func get_proposer_reward*(state: ForkyBeaconState, attestation: SomeAttestation, base_reward_per_increment: Gwei, diff --git a/beacon_chain/spec/signatures.nim b/beacon_chain/spec/signatures.nim index 08c17cd13..7d166304d 100644 --- a/beacon_chain/spec/signatures.nim +++ b/beacon_chain/spec/signatures.nim @@ -368,12 +368,13 @@ proc verify_builder_signature*( blsVerify(pubkey, signing_root.data, signature) # https://github.com/ethereum/consensus-specs/blob/v1.3.0-rc.2/specs/capella/beacon-chain.md#new-process_bls_to_execution_change -func compute_bls_to_execution_change_signing_root( +func compute_bls_to_execution_change_signing_root*( genesisFork: Fork, genesis_validators_root: Eth2Digest, msg: BLSToExecutionChange): Eth2Digest = # So the epoch doesn't matter when calling get_domain doAssert genesisFork.previous_version == genesisFork.current_version + # Fork-agnostic domain since address changes are valid across forks let domain = get_domain( genesisFork, DOMAIN_BLS_TO_EXECUTION_CHANGE, GENESIS_EPOCH, genesis_validators_root) diff --git a/beacon_chain/spec/signatures_batch.nim b/beacon_chain/spec/signatures_batch.nim index baad56f49..31ae8f632 100644 --- a/beacon_chain/spec/signatures_batch.nim +++ b/beacon_chain/spec/signatures_batch.nim @@ -202,11 +202,21 @@ func contribution_and_proof_signature_set*( SignatureSet.init(pubkey, signing_root, signature) -func collectSignatureSets*( +func bls_to_execution_change_signature_set*( + genesisFork: Fork, genesis_validators_root: Eth2Digest, + msg: BLSToExecutionChange, + pubkey: CookedPubKey, signature: CookedSig): SignatureSet = + let signing_root = compute_bls_to_execution_change_signing_root( + genesisFork, genesis_validators_root, msg) + + SignatureSet.init(pubkey, signing_root, signature) + +proc collectSignatureSets*( sigs: var seq[SignatureSet], signed_block: ForkySignedBeaconBlock, validatorKeys: auto, state: ForkedHashedBeaconState, + genesis_fork: Fork, cache: var StateCache): Result[void, cstring] = ## Collect all signature verifications that process_block would normally do ## except deposits, in one go. @@ -218,7 +228,8 @@ func collectSignatureSets*( ## - Attester slashings ## - Attestations ## - VoluntaryExits - ## - SyncCommittee (altair+) + ## - SyncCommittee (Altair+) + ## - BLS to execution changes (Capella+) ## ## We do not include deposits as they can be invalid while still leaving the ## block valid @@ -235,6 +246,8 @@ func collectSignatureSets*( return err("collectSignatureSets: invalid proposer index") epoch = signed_block.message.slot.epoch() + doAssert genesis_fork.previous_version == genesis_fork.current_version + # 1. Block proposer # ---------------------------------------------------- sigs.add block_signature_set( @@ -395,6 +408,27 @@ func collectSignatureSets*( signed_block.message.body.sync_aggregate.sync_committee_signature.load().valueOr do: return err("collectSignatureSets: cannot load signature")) + block: + # 8. BLS to execution changes + when typeof(signed_block).toFork() >= ConsensusFork.Capella: + withState(state): + when stateFork >= ConsensusFork.Capella: + for bls_change in signed_block.message.body.bls_to_execution_changes: + let sig = bls_change.signature.load.valueOr: + return err("collectSignatureSets: cannot load BLS to execution change signature") + + # Otherwise, expensive loadWithCache can be spammed with irrelevant pubkeys + ? check_bls_to_execution_change( + genesis_fork, forkyState.data, bls_change, {skipBlsValidation}) + + let validator_pubkey = + bls_change.message.from_bls_pubkey.loadWithCache.valueOr: + return err("collectSignatureSets: cannot load BLS to execution change pubkey") + + sigs.add bls_to_execution_change_signature_set( + genesis_fork, genesis_validators_root, bls_change.message, + validator_pubkey, sig) + ok() proc batchVerify*(verifier: var BatchVerifier, sigs: openArray[SignatureSet]): bool = diff --git a/beacon_chain/spec/state_transition_block.nim b/beacon_chain/spec/state_transition_block.nim index 0d6dcabe5..bec016ac5 100644 --- a/beacon_chain/spec/state_transition_block.nim +++ b/beacon_chain/spec/state_transition_block.nim @@ -387,36 +387,11 @@ proc process_voluntary_exit*( ? initiate_validator_exit(cfg, state, exited_validator, cache) ok() -# https://github.com/ethereum/consensus-specs/blob/v1.3.0-alpha.1/specs/capella/beacon-chain.md#new-process_bls_to_execution_change -proc check_bls_to_execution_change*( - cfg: RuntimeConfig, state: capella.BeaconState | eip4844.BeaconState, - signed_address_change: SignedBLSToExecutionChange): Result[void, cstring] = - let address_change = signed_address_change.message - - if not (address_change.validator_index < state.validators.lenu64): - return err("process_bls_to_execution_change: invalid validator index") - - var withdrawal_credentials = - state.validators.item(address_change.validator_index).withdrawal_credentials - - if not (withdrawal_credentials.data[0] == BLS_WITHDRAWAL_PREFIX): - return err("process_bls_to_execution_change: invalid withdrawal prefix") - - if not (withdrawal_credentials.data.toOpenArray(1, 31) == - eth2digest(address_change.from_bls_pubkey.blob).data.toOpenArray(1, 31)): - return err("process_bls_to_execution_change: invalid withdrawal credentials") - - if not verify_bls_to_execution_change_signature( - cfg.genesisFork, state.genesis_validators_root, signed_address_change, - address_change.from_bls_pubkey, signed_address_change.signature): - return err("process_bls_to_execution_change: invalid signature") - - ok() - proc process_bls_to_execution_change*( cfg: RuntimeConfig, state: var (capella.BeaconState | eip4844.BeaconState), signed_address_change: SignedBLSToExecutionChange): Result[void, cstring] = - ? check_bls_to_execution_change(cfg, state, signed_address_change) + ? check_bls_to_execution_change( + cfg.genesisFork, state, signed_address_change, {}) let address_change = signed_address_change.message var withdrawal_credentials = state.validators.item(address_change.validator_index).withdrawal_credentials diff --git a/beacon_chain/validators/message_router.nim b/beacon_chain/validators/message_router.nim index 5c9d0c5de..ad3a72e79 100644 --- a/beacon_chain/validators/message_router.nim +++ b/beacon_chain/validators/message_router.nim @@ -459,7 +459,7 @@ proc routeBlsToExecutionChange*( bls_to_execution_change: SignedBLSToExecutionChange): Future[SendResult] {.async.} = block: - let res = router[].processor[].processBlsToExecutionChange( + let res = await router.processor.processBlsToExecutionChange( MsgSource.api, bls_to_execution_change) if not res.isGoodForSending: warn "BLS to execution change request failed validation",