feat: local engine #10803

greged93 · 2024-09-10T09:13:20Z

Implements a local engine service with the goal to replace the current MiningTask.

greged93 · 2024-09-10T09:15:57Z

crates/engine/local/src/service.rs

+                    let attr = <N::Engine as PayloadTypes>::PayloadBuilderAttributes::try_new(
+                        head, attributes,
+                    )
+                    .map_err(|_| eyre::eyre!("failed to fetch payload attributes"))?;


I can't figure out how to update the attributes here. My current understanding was:

Pass an initial PayloadAttributes and parent hash to the engine

Use this to create the PayloadBuilderAttributes

Update the PayloadAttributes and the head once block building is done

However I can't figure out how to update these attributes. Maybe I need to add a setter for them?

onbjerg

smol doc nits, havent looked over the entire pr yet as i'm not too familiar with this code myself

onbjerg · 2024-09-10T10:17:41Z

crates/engine/local/src/miner.rs

+/// A mining mode for the local dev engine.
+#[derive(Debug)]
+pub enum MiningMode {
+    /// In this mode a block is build as soon as


Suggested change

/// In this mode a block is build as soon as

/// In this mode a block is built as soon as

onbjerg · 2024-09-10T10:17:51Z

crates/engine/local/src/miner.rs

+    /// In this mode a block is build as soon as
+    /// a valid transaction reaches the pool.
+    Instant(Fuse<ReceiverStream<TxHash>>),
+    /// In this mode a block is build at a fixed interval


Suggested change

/// In this mode a block is build at a fixed interval

/// In this mode a block is built at a fixed interval.

onbjerg · 2024-09-10T10:18:00Z

crates/engine/local/src/miner.rs

+}
+
+impl MiningMode {
+    /// Constructor for an [`MiningMode::Instant`]


Suggested change

/// Constructor for an [`MiningMode::Instant`]

/// Constructor for a [`MiningMode::Instant`]

onbjerg · 2024-09-10T10:18:05Z

crates/engine/local/src/miner.rs

+        Self::Instant(ReceiverStream::new(rx).fuse())
+    }
+
+    /// Constructor for an [`MiningMode::Interval`]


Suggested change

/// Constructor for an [`MiningMode::Interval`]

/// Constructor for a [`MiningMode::Interval`]

mattsse

this is a nice first draft,

I think we can solve the attributes issue by requiring a type that can create a PayloadAttributes instance

mattsse · 2024-09-10T11:46:53Z

crates/engine/local/src/service.rs

+impl<N> Future for LocalEngineService<N>
+where
+    N: EngineNodeTypes,
+{
+    type Output = ();
+
+    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {


if it's easier we can also do this as an async fn function over tokio select, which would make sense here because we do a lot of sequential processing

greged93 · 2024-09-13T16:08:52Z

crates/payload/primitives/src/traits.rs

+/// A builder that can return the current payload attribute.
+pub trait PayloadAttributesBuilder: std::fmt::Debug + Send + Sync + 'static {
+    /// The payload attributes type returned by the builder.
+    type PayloadAttributes: PayloadAttributes;
+
+    /// Return a new payload attribute from the builder.
+    fn build(&self) -> Self::PayloadAttributes;
+}


added a payload attributes builder as suggested, for now it doesn't have setters and is sitting in the traits of the payload as I felt this was most appropriate, let me know

mattsse

this looks pretty good already, some nits

need to think about how to impl the payloadattr builder

mattsse · 2024-09-13T17:07:09Z

crates/engine/local/src/miner.rs

+        match this {
+            Self::Instant(rx) => {
+                // drain all transactions notifications
+                if let Poll::Ready(Some(_)) = pin!(rx).poll_next(cx) {


you can use poll_next_unpin

mattsse · 2024-09-13T17:13:52Z

crates/payload/primitives/src/traits.rs

+    type PayloadAttributes: PayloadAttributes;
+
+    /// Return a new payload attribute from the builder.
+    fn build(&self) -> Self::PayloadAttributes;


this makes sense, we probably want to provide some additional context as arguments eventually, e.g. the most recent block, but we can figure this out later.

mattsse · 2024-09-13T17:14:37Z

crates/payload/primitives/src/traits.rs

+    type PayloadAttributes: PayloadAttributes;
+
+    /// Return a new payload attribute from the builder.
+    fn build(&self) -> Self::PayloadAttributes;


I wanna make this return a result, in case the impl needs to do some io (e.g. fetch current head, etc)

mattsse · 2024-09-13T17:15:49Z

crates/engine/local/src/service.rs

+            loop {
+                // Wait for the interval or the pool to receive a transaction
+                (&mut engine.mode).await;
+
+                // Start a new payload building job
+                let new_head = engine.build_and_save_payload().await;
+
+                if new_head.is_err() {
+                    debug!(target: "local_engine", err = ?new_head.unwrap_err(), "failed payload
+                building");
+                    continue
+                }
+
+                // Update the head
+                engine.head = new_head.expect("not error");
+            }


this should be a standalone function fn run() that should be spawned

greged93 · 2024-09-16T09:31:50Z

let me know if there is anything else you think needs changing @mattsse

crates/engine/local/src/service.rs

mattsse · 2024-09-19T11:52:11Z

this is a good first step

greged93 requested review from mattsse, Rjected, rkrasiuk, fgimenez and gakonst as code owners September 10, 2024 09:13

greged93 commented Sep 10, 2024

View reviewed changes

greged93 requested review from onbjerg and DaniPopes as code owners September 10, 2024 09:29

onbjerg reviewed Sep 10, 2024

View reviewed changes

onbjerg added C-enhancement New feature or request A-engine Related to the engine implementation labels Sep 10, 2024

mattsse requested changes Sep 10, 2024

View reviewed changes

greged93 force-pushed the feat/local-engine branch from 3921928 to 8994e17 Compare September 13, 2024 16:07

greged93 commented Sep 13, 2024

View reviewed changes

greged93 force-pushed the feat/local-engine branch from 86b3bf6 to 3e2aca9 Compare September 13, 2024 16:11

mattsse requested changes Sep 13, 2024

View reviewed changes

greged93 force-pushed the feat/local-engine branch 2 times, most recently from 6c144ce to 879f77c Compare September 14, 2024 07:12

klkvr reviewed Sep 18, 2024

View reviewed changes

crates/engine/local/src/service.rs Show resolved Hide resolved

klkvr approved these changes Sep 19, 2024

View reviewed changes

mattsse approved these changes Sep 19, 2024

View reviewed changes

mattsse added this pull request to the merge queue Sep 19, 2024

github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 19, 2024

greged93 added 5 commits September 19, 2024 14:25

wip

4872c2d

wip LocalEngineService

6f60653

add default executed block to the TestPayloadJob

395357e

add miner modes

fa78cdb

fix issues in Future implementation

83703e6

greged93 added 13 commits September 19, 2024 14:28

fix issues in Future implementation

be95da8

fix issues in Future implementation

c9138f9

add tests

7fff270

fix lints

a279ecd

renaming

452dfca

add retry limit

ba351cf

fix lints

a106206

spelling

16bb6d2

update async loop + add PayloadAttributesBuilder

c5194d2

use send_and_resolve_payload from the PayloadBuilderHandle

c419624

remove unused seal_slow

7fa4609

fix comments

41c2f73

fix rebasing

a3c4497

greged93 force-pushed the feat/local-engine branch from 879f77c to a3c4497 Compare September 19, 2024 12:47

mattsse enabled auto-merge September 19, 2024 12:53

mattsse added this pull request to the merge queue Sep 19, 2024

Merged via the queue into paradigmxyz:main with commit 6688078 Sep 19, 2024
37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: local engine #10803

feat: local engine #10803

greged93 commented Sep 10, 2024

greged93 Sep 10, 2024 •

edited

Loading

onbjerg left a comment •

edited

Loading

onbjerg Sep 10, 2024

onbjerg Sep 10, 2024

onbjerg Sep 10, 2024

onbjerg Sep 10, 2024

mattsse left a comment

mattsse Sep 10, 2024

greged93 Sep 13, 2024

mattsse left a comment

mattsse Sep 13, 2024

mattsse Sep 13, 2024

mattsse Sep 13, 2024

mattsse Sep 13, 2024

greged93 commented Sep 16, 2024

mattsse commented Sep 19, 2024

	/// In this mode a block is build as soon as
	/// In this mode a block is built as soon as

	/// In this mode a block is build at a fixed interval
	/// In this mode a block is built at a fixed interval.

	/// Constructor for an [`MiningMode::Instant`]
	/// Constructor for a [`MiningMode::Instant`]

	/// Constructor for an [`MiningMode::Interval`]
	/// Constructor for a [`MiningMode::Interval`]

feat: local engine #10803

feat: local engine #10803

Conversation

greged93 commented Sep 10, 2024

greged93 Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

onbjerg left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greged93 commented Sep 16, 2024

mattsse commented Sep 19, 2024

greged93 Sep 10, 2024 •

edited

Loading

onbjerg left a comment •

edited

Loading