From c4956143b0faa5c059fd69162497365f851e71ac Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:07:52 -0400 Subject: [PATCH] fix: enforce unsupported fork rules on get_payload_v3 (#4562) --- crates/payload/basic/src/lib.rs | 4 ++ crates/payload/builder/src/lib.rs | 4 ++ crates/payload/builder/src/service.rs | 41 +++++++++++++++++++++ crates/payload/builder/src/test_utils.rs | 4 ++ crates/payload/builder/src/traits.rs | 3 ++ crates/rpc/rpc-engine-api/src/engine_api.rs | 18 +++++++++ 6 files changed, 74 insertions(+) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index d7fbecd06..4b125238a 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -412,6 +412,10 @@ where build_empty_payload(&self.client, self.config.clone()).map(Arc::new) } + fn payload_attributes(&self) -> Result { + Ok(self.config.attributes.clone()) + } + fn resolve(&mut self) -> (Self::ResolvePayloadFuture, KeepPayloadJobAlive) { let best_payload = self.best_payload.take(); let maybe_better = self.pending_block.take(); diff --git a/crates/payload/builder/src/lib.rs b/crates/payload/builder/src/lib.rs index b1b6e8989..d0bfeb1c9 100644 --- a/crates/payload/builder/src/lib.rs +++ b/crates/payload/builder/src/lib.rs @@ -86,6 +86,10 @@ //! Ok(Arc::new(payload)) //! } //! +//! fn payload_attributes(&self) -> Result { +//! Ok(self.attributes.clone()) +//! } +//! //! fn resolve(&mut self) -> (Self::ResolvePayloadFuture, KeepPayloadJobAlive) { //! let payload = self.best_payload(); //! (futures_util::future::ready(payload), KeepPayloadJobAlive::No) diff --git a/crates/payload/builder/src/service.rs b/crates/payload/builder/src/service.rs index 310e28763..3a46f011b 100644 --- a/crates/payload/builder/src/service.rs +++ b/crates/payload/builder/src/service.rs @@ -48,6 +48,16 @@ impl PayloadStore { ) -> Option, PayloadBuilderError>> { self.inner.best_payload(id).await } + + /// Returns the payload attributes associated with the given identifier. + /// + /// Note: this returns the attributes of the payload and does not resolve the job. + pub async fn payload_attributes( + &self, + id: PayloadId, + ) -> Option> { + self.inner.payload_attributes(id).await + } } impl From for PayloadStore { @@ -94,6 +104,18 @@ impl PayloadBuilderHandle { rx.await.ok()? } + /// Returns the payload attributes associated with the given identifier. + /// + /// Note: this returns the attributes of the payload and does not resolve the job. + pub async fn payload_attributes( + &self, + id: PayloadId, + ) -> Option> { + let (tx, rx) = oneshot::channel(); + self.to_service.send(PayloadServiceCommand::PayloadAttributes(id, tx)).ok()?; + rx.await.ok()? + } + /// Sends a message to the service to start building a new payload for the given payload. /// /// This is the same as [PayloadBuilderHandle::new_payload] but does not wait for the result and @@ -178,6 +200,17 @@ where self.payload_jobs.iter().find(|(_, job_id)| *job_id == id).map(|(j, _)| j.best_payload()) } + /// Returns the payload attributes for the given payload. + fn payload_attributes( + &self, + id: PayloadId, + ) -> Option> { + self.payload_jobs + .iter() + .find(|(_, job_id)| *job_id == id) + .map(|(j, _)| j.payload_attributes()) + } + /// Returns the best payload for the given identifier that has been built so far and terminates /// the job if requested. fn resolve(&mut self, id: PayloadId) -> Option { @@ -262,6 +295,9 @@ where PayloadServiceCommand::BestPayload(id, tx) => { let _ = tx.send(this.best_payload(id)); } + PayloadServiceCommand::PayloadAttributes(id, tx) => { + let _ = tx.send(this.payload_attributes(id)); + } PayloadServiceCommand::Resolve(id, tx) => { let _ = tx.send(this.resolve(id)); } @@ -287,6 +323,11 @@ enum PayloadServiceCommand { ), /// Get the best payload so far BestPayload(PayloadId, oneshot::Sender, PayloadBuilderError>>>), + /// Get the payload attributes for the given payload + PayloadAttributes( + PayloadId, + oneshot::Sender>>, + ), /// Resolve the payload and return the payload Resolve(PayloadId, oneshot::Sender>), } diff --git a/crates/payload/builder/src/test_utils.rs b/crates/payload/builder/src/test_utils.rs index 20b290de8..0257c4c0b 100644 --- a/crates/payload/builder/src/test_utils.rs +++ b/crates/payload/builder/src/test_utils.rs @@ -68,6 +68,10 @@ impl PayloadJob for TestPayloadJob { ))) } + fn payload_attributes(&self) -> Result { + Ok(self.attr.clone()) + } + fn resolve(&mut self) -> (Self::ResolvePayloadFuture, KeepPayloadJobAlive) { let fut = futures_util::future::ready(self.best_payload()); (fut, KeepPayloadJobAlive::No) diff --git a/crates/payload/builder/src/traits.rs b/crates/payload/builder/src/traits.rs index 4fddbebec..ab118709f 100644 --- a/crates/payload/builder/src/traits.rs +++ b/crates/payload/builder/src/traits.rs @@ -26,6 +26,9 @@ pub trait PayloadJob: Future> + Send + /// Note: This is never called by the CL. fn best_payload(&self) -> Result, PayloadBuilderError>; + /// Returns the payload attributes for the payload being built. + fn payload_attributes(&self) -> Result; + /// Called when the payload is requested by the CL. /// /// This is invoked on [`engine_getPayloadV2`](https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#engine_getpayloadv2) and [`engine_getPayloadV1`](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#engine_getpayloadv1). diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index a96144c22..88513e8de 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -208,6 +208,24 @@ where &self, payload_id: PayloadId, ) -> EngineApiResult { + // First we fetch the payload attributes to check the timestamp + let attributes = self + .inner + .payload_store + .payload_attributes(payload_id) + .await + .ok_or(EngineApiError::UnknownPayload)??; + + // From the Engine API spec: + // + // + // 1. Client software **MUST** return `-38005: Unsupported fork` error if the `timestamp` of + // the built payload does not fall within the time frame of the Cancun fork. + if !self.inner.chain_spec.is_cancun_activated_at_timestamp(attributes.timestamp) { + return Err(EngineApiError::UnsupportedFork) + } + + // Now resolve the payload Ok(self .inner .payload_store