feat(payload): abstract payload builder in trait #10965
Conversation
greged93
requested review from
mattsse,
Rjected,
onbjerg,
rkrasiuk,
fgimenez and
gakonst
as code owners
crates/payload/builder/src/traits.rs
Outdated
| Pin<Box<dyn Future<Output = Result<P, PayloadBuilderError>> + Send + Sync>>; | ||
|
|
||
| /// A type that can request, subscribe to and resolve payloads. | ||
| #[async_trait::async_trait] |
There was a problem hiding this comment.
I'm not too sure about this, are you ok with using the async_trait macro or would you rather explicitly returning a future?
There was a problem hiding this comment.
async_trait is fine here, then we could use this as dyn trait as well.
crates/payload/builder/src/traits.rs
Outdated
| Pin<Box<dyn Future<Output = Result<P, PayloadBuilderError>> + Send + Sync>>; | ||
|
|
||
| /// A type that can request, subscribe to and resolve payloads. | ||
| #[async_trait::async_trait] |
There was a problem hiding this comment.
async_trait is fine here, then we could use this as dyn trait as well.
crates/payload/builder/src/traits.rs
Outdated
|
|
||
| /// A type that can request, subscribe to and resolve payloads. | ||
| #[async_trait::async_trait] | ||
| pub trait PayloadBuilder { |
There was a problem hiding this comment.
can we move this to payload-primitives perhaps?
crates/payload/builder/src/traits.rs
Outdated
|
|
||
| /// A type that can request, subscribe to and resolve payloads. | ||
| #[async_trait::async_trait] | ||
| pub trait PayloadBuilder { |
There was a problem hiding this comment.
this should be Send + Unpin
| let (tx, rx) = oneshot::channel(); | ||
| let _ = self.to_service.send(PayloadServiceCommand::Subscribe(tx)); | ||
| Ok(PayloadEvents { receiver: rx.await? }) | ||
| } | ||
| } | ||
|
|
||
| impl<T> PayloadBuilderHandle<T> | ||
| where | ||
| T: PayloadTypes + 'static, |
There was a problem hiding this comment.
let's also make PayloadTypes 'static
greged93
force-pushed
the
feat/abstract-payload-builder
branch
from
eb123d2
to
07b720a
Compare
greged93
force-pushed
the
feat/abstract-payload-builder
branch
from
07b720a
to
9c469d0
Compare
|
I noticed there were 2 |
greged93
force-pushed
the
feat/abstract-payload-builder
branch
from
9c469d0
to
d5c6ea1
Compare
greged93
force-pushed
the
feat/abstract-payload-builder
branch
from
d5c6ea1
to
aa161a2
Compare
greged93
force-pushed
the
feat/abstract-payload-builder
branch
from
aa161a2
to
c16ec18
Compare
crates/payload/builder/src/lib.rs
Outdated
| @@ -28,7 +28,7 @@ | |||
| //! use std::pin::Pin; | |||
| //! use std::task::{Context, Poll}; | |||
| //! use reth_payload_builder::{EthBuiltPayload, KeepPayloadJobAlive, EthPayloadBuilderAttributes, PayloadJob, PayloadJobGenerator}; | |||
| //! use reth_payload_builder::error::PayloadBuilderError; | |||
| //! use reth_payload_primitives::PayloadBuilderError; | |||
There was a problem hiding this comment.
we should re-export the PayloadBuilderError in L125 then this wouldn't break too much and some crates don't require an additional dep
Abstracts the current
PayloadBuilderHandlebehind aPayloadBuildertrait. First step towards being able to have thePayloadBuilderbe a GAT on theNodeComponentstrait.Resolves #10956.