Enable ResolvePayloadFuture to be generic to the PayloadJob trait

[loocapro]

Describe the feature

I am using the reth Payload crate to modify the default behaviour of engine Api payload jobs. I am implementing the PayloadJobGenerator passing my own PayloadJob.

Currently the PayloadJob::resolve()returns a ResolvePayloadFuture which is tied to the BuiltPayload reth type currently consumed by the engine API.

I 'd love that the ResolvePayloadFuture could be generic so that I could return a BuiltPayload and some other parameter that I am calculating from my PayloadJobs.

@mattsse do you have any better way to achieve this?

I'd love to pick this up myself.

Additional context

No response

[mattsse]

how are you using your type?

because, by default, the generator is controlled by the service:

reth/crates/payload/builder/src/service.rs

Lines 159 to 162 in 460b5f6

	pub struct PayloadBuilderService<Gen>
	where
	Gen: PayloadJobGenerator,
	{

which is wired to the engine API

[loocapro]

how are you using your type?

because, by default, the generator is controlled by the service:

reth/crates/payload/builder/src/service.rs

Lines 159 to 162 in 460b5f6

	pub struct PayloadBuilderService<Gen>
	where
	Gen: PayloadJobGenerator,
	{

which is wired to the engine API

I am just implementing a custom generator, with a custom job, and spawning a custom payload service, but still using the reth PayloadService from the payload crate.

Then I interact with the reth payload service via the payload handle resolve function, which unfortunately can only return a BuiltPayload future.

[mattsse]

gotcha,
so currently this format is fixed, but I think we actually want this to be generic, because optimism would also benefit from this.

I need to think about this a bit more because it would introduce a few more changes wrt generics

[loocapro]

Okk, please let me know. If it's not a huge change i d like to take it.

[loocapro]

Hi @mattsse sorry to hassle you, have you got any news on this?

Currently I have solved it by duplicating the PayloadService code but I am a bit worried about future conflicts.

[mattsse]

this would require significant changes across the codebase and is more or less blocked by this #5501

perhaps we can at least introduce a new associated type to the trait for now and require that it is Into<BuiltPayload>

But I also think you should be able to work around this for your use case with some extra code bypassing the payload handle, which doesn't have generics

[loocapro]

bypassing the payload handle, which doesn't have generics

Thanks a lot Matt. Could you please show me how would you by pass the payload handle?

[idatsy]

Would this be resolved by just making the PayloadBuilderHandle::new pub so that extending spawn_payload_builder_service lets you roll your own builder logic consuming the to_service channel directly?

That or make PayloadBuilderHandle a pub trait where users can impl their own resolve, best_payload, payload_attributes, send_new_payload and new_payload?

[mattsse]

Would this be resolved by just making the PayloadBuilderHandle::new pub so that extending

that would def make this possible, yeah

let's do that first.

then I think we want the return type of the Future Also Into<Payload>, though we'd need to break that again eventually

[idatsy]

Would this be resolved by just making the PayloadBuilderHandle::new pub so that extending

that would def make this possible, yeah

let's do that first.

then I think we want the return type of the Future Also Into<Payload>, though we'd need to break that again eventually

Awesome, I'll PR that for your right away, will drastically simplify my (and I assume others) payload building logic :)

[idatsy]

Would this be resolved by just making the PayloadBuilderHandle::new pub so that extending

that would def make this possible, yeah

let's do that first.

then I think we want the return type of the Future Also Into<Payload>, though we'd need to break that again eventually

I left out the Into<Payload> change as I'm not sure exactly what you have in mind there: #5578

[loocapro]

Thanks @idatsy.

This is not going to solve my issue as I am using the payload crate (generator + service + handle) and I like how it is structured. I don't even think that new function should be exposed to the outside as it could expose the internal implementation to the outside world and it would totally miss the purpose of encapsulation.

@mattsse I love the idea of restricting the ResolvePayloadFuture to Into<Payload>, will try to get a pr up soon.

[github-actions]

This issue is stale because it has been open for 21 days with no activity.

[github-actions]

This issue is stale because it has been open for 21 days with no activity.

[mattsse]

marking this as completed because this is now possible:

reth/crates/payload/builder/src/service.rs

Line 201 in f23250d

<Gen::Job as PayloadJob>::BuiltPayload: Into<Engine::BuiltPayload>,

reth/crates/payload/builder/src/traits.rs

Lines 27 to 28 in f23250d

	/// Represents the built payload type that is returned to the CL.
	type BuiltPayload: BuiltPayload + std::fmt::Debug;