perf: shortcircuit if new payload is worse (#2224)

crates/payload/basic/src/lib.rs

@@ -255,21 +255,6 @@ pub struct BasicPayloadJob<Client, Pool, Tasks> {
     payload_task_guard: PayloadTaskGuard,
 }
 
-// === impl BasicPayloadJob ===
-
-impl<Client, Pool, Tasks> BasicPayloadJob<Client, Pool, Tasks> {
-    /// Checks if the new payload is better than the current best.
-    ///
-    /// This compares the total fees of the blocks, higher is better.
-    fn is_better(&self, new_payload: &BuiltPayload) -> bool {
-        if let Some(best_payload) = &self.best_payload {
-            new_payload.fees() > best_payload.fees()
-        } else {
-            true
-        }
-    }
-}
-
 impl<Client, Pool, Tasks> Stream for BasicPayloadJob<Client, Pool, Tasks>
 where
     Client: StateProviderFactory + Clone + Unpin + 'static,
@@ -297,10 +282,11 @@ where
             let _cancel = cancel.clone();
             let guard = this.payload_task_guard.clone();
             let payload_config = this.config.clone();
+            let best_payload = this.best_payload.clone();
             this.executor.spawn_blocking(Box::pin(async move {
                 // acquire the permit for executing the task
                 let _permit = guard.0.acquire().await;
-                build_payload(client, pool, payload_config, cancel, tx)
+                build_payload(client, pool, payload_config, cancel, best_payload, tx)
             }));
             this.pending_block = Some(PendingPayload { _cancel, payload: rx });
         }
@@ -308,12 +294,20 @@ where
         // poll the pending block
         if let Some(mut fut) = this.pending_block.take() {
             match fut.poll_unpin(cx) {
-                Poll::Ready(Ok(payload)) => {
+                Poll::Ready(Ok(outcome)) => {
                     this.interval.reset();
-                    if this.is_better(&payload) {
-                        let payload = Arc::new(payload);
-                        this.best_payload = Some(payload.clone());
-                        return Poll::Ready(Some(Ok(payload)))
+                    match outcome {
+                        BuildOutcome::Better(payload) => {
+                            trace!("built better payload");
+                            let payload = Arc::new(payload);
+                            this.best_payload = Some(payload);
+                        }
+                        BuildOutcome::Aborted { .. } => {
+                            trace!("skipped payload build of worse block");
+                        }
+                        BuildOutcome::Cancelled => {
+                            unreachable!("the cancel signal never fired")
+                        }
                     }
                 }
                 Poll::Ready(Err(err)) => {
@@ -352,11 +346,11 @@ struct PendingPayload {
     /// The marker to cancel the job on drop
     _cancel: Cancelled,
     /// The channel to send the result to.
-    payload: oneshot::Receiver<Result<BuiltPayload, PayloadBuilderError>>,
+    payload: oneshot::Receiver<Result<BuildOutcome, PayloadBuilderError>>,
 }
 
 impl Future for PendingPayload {
-    type Output = Result<BuiltPayload, PayloadBuilderError>;
+    type Output = Result<BuildOutcome, PayloadBuilderError>;
 
     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let res = ready!(self.payload.poll_unpin(cx));
@@ -400,13 +394,23 @@ struct PayloadConfig {
     chain_spec: Arc<ChainSpec>,
 }
 
+enum BuildOutcome {
+    /// Successfully built a better block.
+    Better(BuiltPayload),
+    /// Aborted payload building because resulted in worse block wrt. fees.
+    Aborted { fees: U256 },
+    /// Build job was cancelled
+    Cancelled,
+}
+
 /// Builds the payload and sends the result to the given channel.
 fn build_payload<Pool, Client>(
     client: Client,
     pool: Pool,
     config: PayloadConfig,
     cancel: Cancelled,
-    to_job: oneshot::Sender<Result<BuiltPayload, PayloadBuilderError>>,
+    best_payload: Option<Arc<BuiltPayload>>,
+    to_job: oneshot::Sender<Result<BuildOutcome, PayloadBuilderError>>,
 ) where
     Client: StateProviderFactory,
     Pool: TransactionPool,
@@ -417,7 +421,8 @@ fn build_payload<Pool, Client>(
         pool: Pool,
         config: PayloadConfig,
         cancel: Cancelled,
-    ) -> Result<BuiltPayload, PayloadBuilderError>
+        best_payload: Option<Arc<BuiltPayload>>,
+    ) -> Result<BuildOutcome, PayloadBuilderError>
     where
         Client: StateProviderFactory,
         Pool: TransactionPool,
@@ -455,7 +460,7 @@ fn build_payload<Pool, Client>(
 
             // check if the job was cancelled, if so we can exit early
             if cancel.is_cancelled() {
-                return Err(PayloadBuilderError::BuildJobCancelled)
+                return Ok(BuildOutcome::Cancelled)
             }
 
             // convert tx to a signed transaction
@@ -501,6 +506,12 @@ fn build_payload<Pool, Client>(
             executed_txs.push(tx.into_signed());
         }
 
+        // check if we have a better block
+        if !is_better_payload(best_payload.as_deref(), total_fees) {
+            // can skip building the block
+            return Ok(BuildOutcome::Aborted { fees: total_fees })
+        }
+
         let mut withdrawals_root = None;
 
         // get balance changes from withdrawals
@@ -555,7 +566,19 @@ fn build_payload<Pool, Client>(
         };
 
         let sealed_block = block.seal_slow();
-        Ok(BuiltPayload::new(attributes.id, sealed_block, total_fees))
+        Ok(BuildOutcome::Better(BuiltPayload::new(attributes.id, sealed_block, total_fees)))
+    }
+    let _ = to_job.send(try_build(client, pool, config, cancel, best_payload));
+}
+
+/// Checks if the new payload is better than the current best.
+///
+/// This compares the total fees of the blocks, higher is better.
+#[inline(always)]
+fn is_better_payload(best_payload: Option<&BuiltPayload>, new_fees: U256) -> bool {
+    if let Some(best_payload) = best_payload {
+        new_fees > best_payload.fees()
+    } else {
+        true
     }
-    let _ = to_job.send(try_build(client, pool, config, cancel));
 }