diff --git a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs index 3628712721..8a23c6050d 100644 --- a/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs +++ b/beacon_node/beacon_chain/src/payload_envelope_verification/import.rs @@ -233,10 +233,14 @@ impl BeaconChain { return Err(EnvelopeError::BlockRootUnknown { block_root }); } - // TODO(gloas) no fork choice logic yet // Take an exclusive write-lock on fork choice. It's very important to prevent deadlocks by // avoiding taking other locks whilst holding this lock. - // let fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); + let mut fork_choice = parking_lot::RwLockUpgradableReadGuard::upgrade(fork_choice_reader); + + // Update the node's payload_status from PENDING to FULL in fork choice. + fork_choice + .on_execution_payload(block_root) + .map_err(|e| EnvelopeError::InternalError(format!("{e:?}")))?; // TODO(gloas) Do we need this check? Do not import a block that doesn't descend from the finalized root. // let signed_block = check_block_is_finalized_checkpoint_or_descendant(self, &fork_choice, signed_block)?; @@ -311,10 +315,9 @@ impl BeaconChain { drop(db_span); - // TODO(gloas) drop fork choice lock // The fork choice write-lock is dropped *after* the on-disk database has been updated. // This prevents inconsistency between the two at the expense of concurrency. - // drop(fork_choice); + drop(fork_choice); // We're declaring the envelope "imported" at this point, since fork choice and the DB know // about it. diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 9744b9fa08..4bea489b1c 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -21,6 +21,7 @@ use types::{ AbstractExecPayload, AttestationShufflingId, AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecPayload, ExecutionBlockHash, Hash256, IndexedAttestationRef, RelativeEpoch, SignedBeaconBlock, Slot, + consts::gloas::{PAYLOAD_STATUS_FULL, PAYLOAD_STATUS_PENDING}, }; #[derive(Debug)] @@ -386,6 +387,13 @@ where // If the current slot is not provided, use the value that was last provided to the store. let current_slot = current_slot.unwrap_or_else(|| fc_store.get_current_slot()); + // TODO(gloas) this probably isnt true, but AI is hallucinating, and we probably dont need + // this right now anyways + // The anchor is a trusted finalized block. Pre-Gloas blocks always have their payload + // inline (FULL). For Gloas anchors, we trust the finalized state was fully processed, + // so FULL is appropriate. + let payload_status = PAYLOAD_STATUS_FULL; + let proto_array = ProtoArrayForkChoice::new::( current_slot, finalized_block_slot, @@ -395,6 +403,7 @@ where current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, + payload_status, )?; let mut fork_choice = Self { @@ -632,6 +641,17 @@ where .map_err(Error::FailedToProcessInvalidExecutionPayload) } + /// Register that a valid execution payload envelope has been received for `block_root`, + /// updating the node's `payload_status` from PENDING to FULL. + pub fn on_execution_payload( + &mut self, + block_root: Hash256, + ) -> Result<(), Error> { + self.proto_array + .on_execution_payload(block_root) + .map_err(Error::FailedToProcessValidExecutionPayload) + } + /// Add `block` to the fork choice DAG. /// /// - `block_root` is the root of `block. @@ -908,6 +928,13 @@ where execution_status, unrealized_justified_checkpoint: Some(unrealized_justified_checkpoint), unrealized_finalized_checkpoint: Some(unrealized_finalized_checkpoint), + // Pre-Gloas blocks have their payload inline, so they are always FULL. + // Gloas blocks start as PENDING until the payload envelope arrives. + payload_status: if block.fork_name_unchecked().gloas_enabled() { + PAYLOAD_STATUS_PENDING + } else { + PAYLOAD_STATUS_FULL + }, }, current_slot, self.justified_checkpoint(), diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index e9deb6759f..7595d61ab5 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -89,6 +89,7 @@ impl ForkChoiceTestDefinition { junk_shuffling_id.clone(), junk_shuffling_id, ExecutionStatus::Optimistic(ExecutionBlockHash::zero()), + types::consts::gloas::PAYLOAD_STATUS_FULL, ) .expect("should create fork choice struct"); let equivocating_indices = BTreeSet::new(); @@ -211,6 +212,8 @@ impl ForkChoiceTestDefinition { ), unrealized_justified_checkpoint: None, unrealized_finalized_checkpoint: None, + // Test blocks default to FULL payload status. + payload_status: types::consts::gloas::PAYLOAD_STATUS_FULL, }; fork_choice .process_block::( diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 5bfcdae463..9d8174285a 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -10,6 +10,7 @@ use superstruct::superstruct; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, Slot, + consts::gloas::{PAYLOAD_STATUS_FULL, PAYLOAD_STATUS_PENDING, PayloadStatus}, }; // Define a "legacy" implementation of `Option` which uses four bytes for encoding the union @@ -109,6 +110,13 @@ pub struct ProtoNode { pub unrealized_justified_checkpoint: Option, #[ssz(with = "four_byte_option_checkpoint")] pub unrealized_finalized_checkpoint: Option, + /// The payload status of this block (Gloas). + /// + /// - `PAYLOAD_STATUS_PENDING` (0): The payload has not yet been received. + /// - `PAYLOAD_STATUS_EMPTY` (1): The slot has passed without a payload (pre-Gloas blocks + /// or empty Gloas slots). + /// - `PAYLOAD_STATUS_FULL` (2): A valid execution payload has been received. + pub payload_status: PayloadStatus, } #[derive(PartialEq, Debug, Encode, Decode, Serialize, Deserialize, Copy, Clone)] @@ -332,6 +340,7 @@ impl ProtoArray { execution_status: block.execution_status, unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + payload_status: block.payload_status, }; // If the parent has an invalid execution status, return an error before adding the block to @@ -369,6 +378,32 @@ impl ProtoArray { Ok(()) } + /// Register that an execution payload has been received and validated for `block_root`. + /// + /// Updates the node's `payload_status` from `PENDING` to `FULL`. + /// + /// Returns an error if the block is unknown to fork choice. + pub fn on_execution_payload( + &mut self, + block_root: Hash256, + ) -> Result<(), Error> { + let index = self + .indices + .get(&block_root) + .copied() + .ok_or(Error::NodeUnknown(block_root))?; + let node = self + .nodes + .get_mut(index) + .ok_or(Error::InvalidNodeIndex(index))?; + + if node.payload_status == PAYLOAD_STATUS_PENDING { + node.payload_status = PAYLOAD_STATUS_FULL; + } + + Ok(()) + } + /// Updates the `block_root` and all ancestors to have validated execution payloads. /// /// Returns an error if: diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 3edf1e0644..371e8ff7b7 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -17,7 +17,7 @@ use std::{ }; use types::{ AttestationShufflingId, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, - Slot, + Slot, consts::gloas::PayloadStatus, }; pub const DEFAULT_PRUNE_THRESHOLD: usize = 256; @@ -159,6 +159,8 @@ pub struct Block { pub execution_status: ExecutionStatus, pub unrealized_justified_checkpoint: Option, pub unrealized_finalized_checkpoint: Option, + /// The payload status for this block (Gloas fork choice). + pub payload_status: PayloadStatus, } impl Block { @@ -422,6 +424,7 @@ impl ProtoArrayForkChoice { current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, execution_status: ExecutionStatus, + payload_status: PayloadStatus, ) -> Result { let mut proto_array = ProtoArray { prune_threshold: DEFAULT_PRUNE_THRESHOLD, @@ -445,6 +448,7 @@ impl ProtoArrayForkChoice { execution_status, unrealized_justified_checkpoint: Some(justified_checkpoint), unrealized_finalized_checkpoint: Some(finalized_checkpoint), + payload_status, }; proto_array @@ -484,6 +488,18 @@ impl ProtoArrayForkChoice { .map_err(|e| format!("Failed to process invalid payload: {:?}", e)) } + /// Register that a valid execution payload envelope has been received for `block_root`. + /// + /// See `ProtoArray::on_execution_payload` for documentation. + pub fn on_execution_payload( + &mut self, + block_root: Hash256, + ) -> Result<(), String> { + self.proto_array + .on_execution_payload(block_root) + .map_err(|e| format!("on_execution_payload error: {:?}", e)) + } + pub fn process_attestation( &mut self, validator_index: usize, @@ -873,6 +889,7 @@ impl ProtoArrayForkChoice { execution_status: block.execution_status, unrealized_justified_checkpoint: block.unrealized_justified_checkpoint, unrealized_finalized_checkpoint: block.unrealized_finalized_checkpoint, + payload_status: block.payload_status, }) } @@ -1136,6 +1153,7 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, + types::consts::gloas::PAYLOAD_STATUS_FULL, ) .unwrap(); @@ -1155,6 +1173,7 @@ mod test_compute_deltas { execution_status, unrealized_justified_checkpoint: Some(genesis_checkpoint), unrealized_finalized_checkpoint: Some(genesis_checkpoint), + payload_status: types::consts::gloas::PAYLOAD_STATUS_FULL, }, genesis_slot + 1, genesis_checkpoint, @@ -1180,6 +1199,7 @@ mod test_compute_deltas { execution_status, unrealized_justified_checkpoint: None, unrealized_finalized_checkpoint: None, + payload_status: types::consts::gloas::PAYLOAD_STATUS_FULL, }, genesis_slot + 1, genesis_checkpoint, @@ -1280,6 +1300,7 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, + types::consts::gloas::PAYLOAD_STATUS_FULL, ) .unwrap(); @@ -1308,6 +1329,7 @@ mod test_compute_deltas { execution_status, unrealized_justified_checkpoint: Some(genesis_checkpoint), unrealized_finalized_checkpoint: Some(genesis_checkpoint), + payload_status: types::consts::gloas::PAYLOAD_STATUS_FULL, }, Slot::from(block.slot), genesis_checkpoint,