Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(l1): implement forkchoiceupdatedv2, newpayloadv2 and getpayloadv2 #1297

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/blockchain/dev/block_producer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utils::engine_client::{errors::EngineClientError, EngineClient};
use bytes::Bytes;
use ethereum_types::{Address, H256};
use ethrex_rpc::types::fork_choice::{ForkChoiceState, PayloadAttributesV3};
use ethrex_rpc::types::fork_choice::{ForkChoiceState, PayloadAttributes};
use sha2::{Digest, Sha256};
use std::time::{SystemTime, UNIX_EPOCH};

Expand All @@ -25,7 +25,7 @@ pub async fn start_block_producer(
finalized_block_hash: head_block_hash,
};

let payload_attributes = PayloadAttributesV3 {
let payload_attributes = PayloadAttributes {
timestamp: SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(),
suggested_fee_recipient: coinbase_address,
..Default::default()
Expand Down Expand Up @@ -62,6 +62,7 @@ pub async fn start_block_producer(
execution_payload_response.execution_payload,
execution_payload_response
.blobs_bundle
.unwrap_or_default()
.commitments
.iter()
.map(|commitment| {
Expand Down
18 changes: 9 additions & 9 deletions crates/blockchain/dev/utils/engine_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ use errors::{
use ethereum_types::H256;
use ethrex_rpc::{
engine::{
fork_choice::ForkChoiceUpdatedV3,
payload::{GetPayloadV3Request, NewPayloadV3Request},
fork_choice::{ForkChoiceUpdated, ForkChoiceUpdatedV3},
payload::{GetPayloadRequest, GetPayloadV3Request, NewPayloadV3Request},
ExchangeCapabilitiesRequest,
},
types::{
fork_choice::{ForkChoiceResponse, ForkChoiceState, PayloadAttributesV3},
payload::{ExecutionPayloadResponse, ExecutionPayloadV3, PayloadStatus},
fork_choice::{ForkChoiceResponse, ForkChoiceState, PayloadAttributes},
payload::{ExecutionPayload, ExecutionPayloadResponse, PayloadStatus},
},
utils::{RpcErrorResponse, RpcRequest, RpcSuccessResponse},
};
Expand Down Expand Up @@ -77,12 +77,12 @@ impl EngineClient {
pub async fn engine_forkchoice_updated_v3(
&self,
state: ForkChoiceState,
payload_attributes: Option<PayloadAttributesV3>,
payload_attributes: Option<PayloadAttributes>,
) -> Result<ForkChoiceResponse, EngineClientError> {
let request = ForkChoiceUpdatedV3 {
let request = ForkChoiceUpdatedV3(ForkChoiceUpdated {
fork_choice_state: state,
payload_attributes: Ok(payload_attributes),
}
})
.try_into()
.map_err(|e| {
ForkChoiceUpdateError::ConversionError(format!("Failed to convert to RPC request: {e}"))
Expand All @@ -104,7 +104,7 @@ impl EngineClient {
&self,
payload_id: u64,
) -> Result<ExecutionPayloadResponse, EngineClientError> {
let request = GetPayloadV3Request { payload_id }.into();
let request = GetPayloadV3Request(GetPayloadRequest { payload_id }).into();

match self.send_request(request).await {
Ok(RpcResponse::Success(result)) => serde_json::from_value(result.result)
Expand All @@ -119,7 +119,7 @@ impl EngineClient {

pub async fn engine_new_payload_v3(
&self,
execution_payload: ExecutionPayloadV3,
execution_payload: ExecutionPayload,
expected_blob_versioned_hashes: Vec<H256>,
parent_beacon_block_root: H256,
) -> Result<PayloadStatus, EngineClientError> {
Expand Down
4 changes: 3 additions & 1 deletion crates/blockchain/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ pub fn create_payload(args: &BuildPayloadArgs, storage: &Store) -> Result<Block,
withdrawals_root: chain_config
.is_shanghai_activated(args.timestamp)
.then_some(compute_withdrawals_root(&args.withdrawals)),
blob_gas_used: Some(0),
blob_gas_used: chain_config
.is_cancun_activated(args.timestamp)
.then_some(0),
excess_blob_gas: chain_config.is_cancun_activated(args.timestamp).then_some(
calc_excess_blob_gas(
parent_block.excess_blob_gas.unwrap_or_default(),
Expand Down
230 changes: 179 additions & 51 deletions crates/networking/rpc/engine/fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,72 +4,74 @@ use ethrex_blockchain::{
latest_canonical_block_hash,
payload::{create_payload, BuildPayloadArgs},
};
use ethrex_core::types::{BlockHeader, ChainConfig};
use serde_json::Value;
use tracing::{info, warn};

use crate::{
types::{
fork_choice::{ForkChoiceResponse, ForkChoiceState, PayloadAttributesV3},
fork_choice::{ForkChoiceResponse, ForkChoiceState, PayloadAttributes},
payload::PayloadStatus,
},
utils::RpcRequest,
RpcApiContext, RpcErr, RpcHandler,
};

#[derive(Debug)]
pub struct ForkChoiceUpdatedV3 {
pub struct ForkChoiceUpdated {
pub fork_choice_state: ForkChoiceState,
#[allow(unused)]
pub payload_attributes: Result<Option<PayloadAttributesV3>, String>,
pub payload_attributes: Result<Option<PayloadAttributes>, String>,
}

impl TryFrom<ForkChoiceUpdatedV3> for RpcRequest {
type Error = String;

fn try_from(val: ForkChoiceUpdatedV3) -> Result<Self, Self::Error> {
match val.payload_attributes {
Ok(attrs) => Ok(RpcRequest {
method: "engine_forkchoiceUpdatedV3".to_string(),
params: Some(vec![
serde_json::json!(val.fork_choice_state),
serde_json::json!(attrs),
]),
..Default::default()
}),
Err(err) => Err(err),
}
}
}

impl RpcHandler for ForkChoiceUpdatedV3 {
// TODO(#853): Allow fork choice to be executed even if fork choice updated v3 was not correctly parsed.
fn parse(params: &Option<Vec<Value>>) -> Result<Self, RpcErr> {
impl ForkChoiceUpdated {
// TODO(#853): Allow fork choice to be executed even if fork choice updated was not correctly parsed.
fn parse(params: &Option<Vec<Value>>) -> Result<ForkChoiceUpdated, RpcErr> {
let params = params
.as_ref()
.ok_or(RpcErr::BadParams("No params provided".to_owned()))?;
if params.len() != 2 {
return Err(RpcErr::BadParams("Expected 2 params".to_owned()));
}
Ok(ForkChoiceUpdatedV3 {
Ok(ForkChoiceUpdated {
fork_choice_state: serde_json::from_value(params[0].clone())?,
payload_attributes: serde_json::from_value(params[1].clone())
.map_err(|e| e.to_string()),
})
}

fn handle(&self, context: RpcApiContext) -> Result<Value, RpcErr> {
fn try_from(fork_choice_updated: &dyn ForkChoiceUpdatedImpl) -> Result<RpcRequest, String> {
let request = fork_choice_updated.request();
match &request.payload_attributes {
Ok(attrs) => Ok(RpcRequest {
method: fork_choice_updated.method(),
params: Some(vec![
serde_json::json!(request.fork_choice_state),
serde_json::json!(attrs),
]),
..Default::default()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd prefer if we used some explicit fields here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of prefer that too... but it's like repeating arbitrary code:

            id: RpcRequestId::Number(1),
            jsonrpc: "2.0".to_string(),

}),
Err(err) => Err(err.to_string()),
}
}

fn handle(
fork_choice_updated: &dyn ForkChoiceUpdatedImpl,
context: RpcApiContext,
) -> Result<Value, RpcErr> {
let request = fork_choice_updated.request();
let storage = &context.storage;
info!(
"New fork choice request with head: {}, safe: {}, finalized: {}.",
self.fork_choice_state.head_block_hash,
self.fork_choice_state.safe_block_hash,
self.fork_choice_state.finalized_block_hash
request.fork_choice_state.head_block_hash,
request.fork_choice_state.safe_block_hash,
request.fork_choice_state.finalized_block_hash
);

let head_block = match apply_fork_choice(
&context.storage,
self.fork_choice_state.head_block_hash,
self.fork_choice_state.safe_block_hash,
self.fork_choice_state.finalized_block_hash,
storage,
request.fork_choice_state.head_block_hash,
request.fork_choice_state.safe_block_hash,
request.fork_choice_state.finalized_block_hash,
) {
Ok(head) => head,
Err(error) => {
Expand All @@ -89,7 +91,7 @@ impl RpcHandler for ForkChoiceUpdatedV3 {
"Missing latest canonical block".to_owned(),
));
};
let sync_head = self.fork_choice_state.head_block_hash;
let sync_head = request.fork_choice_state.head_block_hash;
tokio::spawn(async move {
// If we can't get hold of the syncer, then it means that there is an active sync in process
if let Ok(mut syncer) = context.syncer.try_lock() {
Expand All @@ -112,34 +114,25 @@ impl RpcHandler for ForkChoiceUpdatedV3 {

// Build block from received payload. This step is skipped if applying the fork choice state failed
let mut response = ForkChoiceResponse::from(PayloadStatus::valid_with_hash(
self.fork_choice_state.head_block_hash,
request.fork_choice_state.head_block_hash,
));

match &self.payload_attributes {
match &request.payload_attributes {
// Payload may be invalid but we had to apply fork choice state nevertheless.
Err(e) => return Err(RpcErr::InvalidPayloadAttributes(e.into())),
Ok(None) => (),
Ok(Some(attributes)) => {
info!("Fork choice updated includes payload attributes. Creating a new payload.");
let chain_config = context.storage.get_chain_config()?;
if !chain_config.is_cancun_activated(attributes.timestamp) {
return Err(RpcErr::UnsuportedFork(
"forkChoiceV3 used to build pre-Cancun payload".to_string(),
));
}
if attributes.timestamp <= head_block.timestamp {
return Err(RpcErr::InvalidPayloadAttributes(
"invalid timestamp".to_string(),
));
}
let chain_config = storage.get_chain_config()?;
fork_choice_updated.validate(attributes, chain_config, head_block)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is the only thing that is different between the FCU calls? What if there is any actions besides validating?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this call the only difference are the validations and the version field in the PayloadArgs here. If some other action or validation is required we can extend the ForkChoiceUpdatedImpl trait to handle it in each version that implements it

let args = BuildPayloadArgs {
parent: self.fork_choice_state.head_block_hash,
parent: request.fork_choice_state.head_block_hash,
timestamp: attributes.timestamp,
fee_recipient: attributes.suggested_fee_recipient,
random: attributes.prev_randao,
withdrawals: attributes.withdrawals.clone(),
beacon_root: Some(attributes.parent_beacon_block_root),
version: 3,
beacon_root: attributes.parent_beacon_block_root,
version: fork_choice_updated.version(),
};
let payload_id = args.id();
response.set_id(payload_id);
Expand All @@ -157,3 +150,138 @@ impl RpcHandler for ForkChoiceUpdatedV3 {
serde_json::to_value(response).map_err(|error| RpcErr::Internal(error.to_string()))
}
}

trait ForkChoiceUpdatedImpl {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we just call it ForkChoiceUpdated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a struct already named like that

fn method(&self) -> String;
fn request(&self) -> &ForkChoiceUpdated;
fn version(&self) -> u8;
fn validate(
&self,
attributes: &PayloadAttributes,
chain_config: ChainConfig,
head_block: BlockHeader,
) -> Result<(), RpcErr>;
}

#[derive(Debug)]
pub struct ForkChoiceUpdatedV2(ForkChoiceUpdated);

impl ForkChoiceUpdatedImpl for ForkChoiceUpdatedV2 {
fn method(&self) -> String {
"engine_forkchoiceUpdatedV2".to_string()
}

fn request(&self) -> &ForkChoiceUpdated {
&self.0
}

fn version(&self) -> u8 {
2
}

fn validate(
&self,
attributes: &PayloadAttributes,
chain_config: ChainConfig,
head_block: BlockHeader,
) -> Result<(), RpcErr> {
if attributes.parent_beacon_block_root.is_some() {
return Err(RpcErr::InvalidPayloadAttributes(
"forkChoiceV2 with Beacon Root".to_string(),
));
}
if !chain_config.is_shanghai_activated(attributes.timestamp) {
return Err(RpcErr::UnsuportedFork(
"forkChoiceV2 used to build pre-Shanghai payload".to_string(),
));
}
if chain_config.is_cancun_activated(attributes.timestamp) {
return Err(RpcErr::UnsuportedFork(
"forkChoiceV2 used to build Cancun payload".to_string(),
));
}
if attributes.timestamp <= head_block.timestamp {
return Err(RpcErr::InvalidPayloadAttributes(
"invalid timestamp".to_string(),
));
}
Ok(())
}
}

impl TryFrom<ForkChoiceUpdatedV2> for RpcRequest {
type Error = String;

fn try_from(val: ForkChoiceUpdatedV2) -> Result<Self, Self::Error> {
ForkChoiceUpdated::try_from(&val)
}
}

impl RpcHandler for ForkChoiceUpdatedV2 {
fn parse(params: &Option<Vec<Value>>) -> Result<Self, RpcErr> {
Ok(Self(ForkChoiceUpdated::parse(params)?))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do you validate that params is in fact v2?

Copy link
Contributor Author

@ElFantasma ElFantasma Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difference is the parent_beacon_block_root that shouldn't be present in V2. And it is checked in v2 validate() here

}

fn handle(&self, context: RpcApiContext) -> Result<Value, RpcErr> {
ForkChoiceUpdated::handle(self, context)
}
}

#[derive(Debug)]
pub struct ForkChoiceUpdatedV3(pub ForkChoiceUpdated);

impl ForkChoiceUpdatedImpl for ForkChoiceUpdatedV3 {
fn method(&self) -> String {
"engine_forkchoiceUpdatedV3".to_string()
}

fn request(&self) -> &ForkChoiceUpdated {
&self.0
}

fn version(&self) -> u8 {
3
}

fn validate(
&self,
attributes: &PayloadAttributes,
chain_config: ChainConfig,
head_block: BlockHeader,
) -> Result<(), RpcErr> {
if attributes.parent_beacon_block_root.is_none() {
return Err(RpcErr::InvalidPayloadAttributes(
"Null Parent Beacon Root".to_string(),
));
}
if !chain_config.is_cancun_activated(attributes.timestamp) {
return Err(RpcErr::UnsuportedFork(
"forkChoiceV3 used to build pre-Cancun payload".to_string(),
));
}
if attributes.timestamp <= head_block.timestamp {
return Err(RpcErr::InvalidPayloadAttributes(
"invalid timestamp".to_string(),
));
}
Ok(())
}
}

impl TryFrom<ForkChoiceUpdatedV3> for RpcRequest {
type Error = String;

fn try_from(val: ForkChoiceUpdatedV3) -> Result<Self, Self::Error> {
ForkChoiceUpdated::try_from(&val)
}
}

impl RpcHandler for ForkChoiceUpdatedV3 {
fn parse(params: &Option<Vec<Value>>) -> Result<Self, RpcErr> {
Ok(Self(ForkChoiceUpdated::parse(params)?))
}

fn handle(&self, context: RpcApiContext) -> Result<Value, RpcErr> {
ForkChoiceUpdated::handle(self, context)
}
}
Loading