Skip to content

Conversation

@SozinM
Copy link
Contributor

@SozinM SozinM commented Feb 20, 2025

πŸ“ Summary

Could be merged after #436
Closes flashbots/op-rbuilder#23

πŸ’‘ Motivation and Context


βœ… I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

have an error with lifetime of &self with executor.spawn_blocking
@SozinM SozinM force-pushed the msozin/op-rbuilder/remove-task-trait branch from ffdb8b0 to 3ba33f1 Compare February 21, 2025 10:05
@SozinM SozinM force-pushed the msozin/op-rbuilder/remove-task-trait branch from aa222d2 to 49682b0 Compare February 21, 2025 11:24
Fix hanging test
@SozinM SozinM force-pushed the msozin/op-rbuilder/remove-task-trait branch from 80d0691 to 55d7299 Compare February 24, 2025 06:21

let (tx, rx) = oneshot::channel();
self.build_complete = Some(rx);
let args = BuildArguments {
Copy link
Contributor

@ferranbt ferranbt Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagined that it would be PayloadGenerator the one spawning the job instead of the PayloadBuilder impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could move this too into the OpPayloadBuilderVanilla.
I will add config and cancel to the try_build and then see if we could refactor this somehow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could move this too into the OpPayloadBuilderVanilla.

Forget about the highlighted line, I could not highlight the correct part of the code since it was not modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like i read initial message wrong.
You man to move spawning into new_payload_job of BlockPayloadJobGenerator, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally i would change interface in this place a bit.
I would like to see the following:
Instead of resolving per-job we would me payload resolving logic into BlockPayloadJobGenerator
The BlockPayloadJobGenerator would coordinate all BlockPayloadJob spawned for this payload building and will ultimately return response to PayloadBuilderService

@SozinM
Copy link
Contributor Author

SozinM commented Feb 25, 2025

Will do it differently on top of #443 chain

@SozinM SozinM closed this Feb 26, 2025
@SozinM SozinM deleted the msozin/op-rbuilder/remove-task-trait branch March 11, 2025 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants