diff --git a/beacon_node/beacon_chain/src/envelope_verification.rs b/beacon_node/beacon_chain/src/envelope_verification.rs index 222823285a..0fecb91df4 100644 --- a/beacon_node/beacon_chain/src/envelope_verification.rs +++ b/beacon_node/beacon_chain/src/envelope_verification.rs @@ -263,10 +263,10 @@ impl GossipVerifiedEnvelope { } // builder index matches committed bid - if envelope.builder_index != execution_bid.builder_index { + if envelope.raw_builder_index() != execution_bid.builder_index { return Err(EnvelopeError::BuilderIndexMismatch { committed_bid: execution_bid.builder_index, - envelope: envelope.builder_index, + envelope: envelope.raw_builder_index(), }); } @@ -308,13 +308,15 @@ impl GossipVerifiedEnvelope { )?; let fork = proposer.fork; + // True builder index accounting for self-building. + let proposer_index = parent_block.message().proposer_index(); + let builder_index = envelope.builder_index(proposer_index); + let signature_is_valid = { let pubkey_cache = chain.validator_pubkey_cache.read(); let builder_pubkey = pubkey_cache - .get(envelope.builder_index as usize) - .ok_or_else(|| EnvelopeError::UnknownValidator { - builder_index: envelope.builder_index, - })?; + .get(builder_index as usize) + .ok_or_else(|| EnvelopeError::UnknownValidator { builder_index })?; signed_envelope.verify_signature( builder_pubkey, &fork, diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 66f39a28f5..89fdc33009 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -372,6 +372,8 @@ where E: EthSpec, F: Fn(usize) -> Option>, { + let proposer_index = state.latest_block_header().proposer_index; + let builder_index = signed_envelope.message.builder_index(proposer_index); let domain = spec.get_domain( state.current_epoch(), Domain::BeaconBuilder, @@ -379,9 +381,8 @@ where state.genesis_validators_root(), ); let message = signed_envelope.message.signing_root(domain); - let pubkey = get_pubkey(signed_envelope.message.builder_index as usize).ok_or( - Error::ValidatorUnknown(signed_envelope.message.builder_index), - )?; + let pubkey = + get_pubkey(builder_index as usize).ok_or(Error::ValidatorUnknown(builder_index))?; Ok(SignatureSet::single_pubkey( &signed_envelope.signature, @@ -400,6 +401,7 @@ where E: EthSpec, F: Fn(usize) -> Option>, { + // TODO(EIP-7732): needs to handle self building! let domain = spec.get_domain( state.current_epoch(), Domain::BeaconBuilder, diff --git a/consensus/types/src/core/consts.rs b/consensus/types/src/core/consts.rs index b6d63c47a8..6876159c0f 100644 --- a/consensus/types/src/core/consts.rs +++ b/consensus/types/src/core/consts.rs @@ -25,3 +25,6 @@ pub mod bellatrix { pub mod deneb { pub use kzg::VERSIONED_HASH_VERSION_KZG; } +pub mod gloas { + pub const BUILDER_INDEX_SELF_BUILD: u64 = u64::MAX; +} diff --git a/consensus/types/src/execution/execution_payload_envelope.rs b/consensus/types/src/execution/execution_payload_envelope.rs index 64e03cec5a..a82517a16e 100644 --- a/consensus/types/src/execution/execution_payload_envelope.rs +++ b/consensus/types/src/execution/execution_payload_envelope.rs @@ -1,3 +1,4 @@ +use crate::consts::gloas::BUILDER_INDEX_SELF_BUILD; use crate::test_utils::TestRandom; use crate::{ EthSpec, ExecutionPayloadGloas, ExecutionRequests, ForkName, Hash256, KzgCommitments, @@ -17,8 +18,10 @@ use tree_hash_derive::TreeHash; pub struct ExecutionPayloadEnvelope { pub payload: ExecutionPayloadGloas, pub execution_requests: ExecutionRequests, + // The builder index is private so that callers are forced to handle the case where it equals + // BUILDER_INDEX_SELF_BUILD. #[serde(with = "serde_utils::quoted_u64")] - pub builder_index: u64, + builder_index: u64, pub beacon_block_root: Hash256, pub slot: Slot, pub blob_kzg_commitments: KzgCommitments, @@ -27,6 +30,28 @@ pub struct ExecutionPayloadEnvelope { impl SignedRoot for ExecutionPayloadEnvelope {} +impl ExecutionPayloadEnvelope { + /// Fetch the validator index of the builder of this execution payload. + /// + /// This falls back to the provided `proposer_index` if the builder index indicates + /// self-building. + pub fn builder_index(&self, proposer_index: u64) -> u64 { + if self.builder_index == BUILDER_INDEX_SELF_BUILD { + proposer_index + } else { + self.builder_index + } + } + + /// Fetch the raw builder index, which may be `BUILDER_INDEX_SELF_BUILD` to indicate + /// self-building. + /// + /// This method should be used sparingly. + pub fn raw_builder_index(&self) -> u64 { + self.builder_index + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/consensus/types/src/execution/signed_execution_payload_envelope.rs b/consensus/types/src/execution/signed_execution_payload_envelope.rs index a055d3d541..58f0ae0565 100644 --- a/consensus/types/src/execution/signed_execution_payload_envelope.rs +++ b/consensus/types/src/execution/signed_execution_payload_envelope.rs @@ -39,12 +39,14 @@ impl SignedExecutionPayloadEnvelope { /// /// The `parent_state` is the post-state of the beacon block with /// block_root = self.message.beacon_block_root - /// TODO(EIP-7732): maybe delete this function later + /// TODO(EIP-7732): maybe delete this function later (it is inefficient) pub fn verify_signature_with_state( &self, parent_state: &BeaconState, spec: &ChainSpec, ) -> Result { + let proposer_index = parent_state.latest_block_header().proposer_index; + let builder_index = self.message.builder_index(proposer_index) as usize; let domain = spec.get_domain( parent_state.current_epoch(), Domain::BeaconBuilder, @@ -53,23 +55,18 @@ impl SignedExecutionPayloadEnvelope { ); let pubkey = parent_state .validators() - .get(self.message.builder_index as usize) + .get(builder_index) .and_then(|v| { let pk: Option = v.pubkey.decompress().ok(); pk }) - .ok_or(BeaconStateError::UnknownValidator( - self.message.builder_index as usize, - ))?; + .ok_or(BeaconStateError::UnknownValidator(builder_index))?; let message = self.message.signing_root(domain); Ok(self.signature.verify(&pubkey, message)) } /// Verify `self.signature`. - /// - /// If the root of `block.message` is already known it can be passed in via `object_root_opt`. - /// Otherwise, it will be computed locally. pub fn verify_signature( &self, pubkey: &PublicKey, @@ -77,9 +74,11 @@ impl SignedExecutionPayloadEnvelope { genesis_validators_root: Hash256, spec: &ChainSpec, ) -> bool { + // Signed envelopes using the new BeaconBuilder domain per the spec: + // https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.1/specs/gloas/beacon-chain.md#new-verify_execution_payload_envelope_signature let domain = spec.get_domain( self.epoch(), - Domain::BeaconProposer, + Domain::BeaconBuilder, fork, genesis_validators_root, );