chore(op-payload-builder): Refactor optimism_payload into smaller, reusable functions

[0xKitsune]

Addresses #10935

This PR refactors the OP block building logic from the optimism_payload function into smaller methods implemented on the OptimismPayloadBuilder struct.

This change enhances the re-usability of the default OptimismPayloadBuilder logic, which is useful when implementing a custom OP Stack Payload builder.

[emhane]

is this linked to an issue?

[0xKitsune]

My mistake, I just realized that I had not opened an issue. I opened a new issue just now and linked it in the PR description.
Please let me know your thoughts on the direction for this PR, happy to adjust if there is a better approach!

[emhane]

My mistake, I just realized that I had not opened an issue. I opened a new issue just now and linked it in the PR description. Please let me know your thoughts on the direction for this PR, happy to adjust if there is a better approach!

valid issue! thanks for implementing!

[emhane]

lgtm

[emhane]

could you please run this against hive also

[0xKitsune]

I ran Hive against these these changes locally and received the following results. It looks like there are some expected failures defined in .github/assetshive/expected_failures.yaml, is there a preferred output that you would like me to produce?

Also please let me know if there is a workflow or set of commands that I should run to evaluate the results against the expected outcomes.

 # Pass
-  "smoke/genesis" ""
    - Pass
-  "smoke/network" ""
    - Pass
 - "ethereum/engine" "engine-auth"
    - Pass
 - "devp2p" "discv4"
    - Pass
 - "ethereum/engine" "engine-exchange-capabilities"
    - Pass

 # Fail
- "ethereum/rpc-compat" ""
    - 21 tests failed
 - "ethereum/engine" "engine-withdrawals"
    - 18 tests failed
 - "ethereum/engine" "engine-api"
    - 8 tests failed
- "ethereum/engine" "cancun"
    - 12 tests failed
 - "ethereum/sync" ""
    - 1 test failed
 - "devp2p" "eth"
    - 5 tests failed

[0xKitsune]

@emhane Any thoughts on how best to proceed to get this merged in?

[emhane]

I ran Hive against these these changes locally and received the following results. It looks like there are some expected failures defined in .github/assetshive/expected_failures.yaml, is there a preferred output that you would like me to produce?

Also please let me know if there is a workflow or set of commands that I should run to evaluate the results against the expected outcomes.

 # Pass
-  "smoke/genesis" ""
    - Pass
-  "smoke/network" ""
    - Pass
 - "ethereum/engine" "engine-auth"
    - Pass
 - "devp2p" "discv4"
    - Pass
 - "ethereum/engine" "engine-exchange-capabilities"
    - Pass

 # Fail
- "ethereum/rpc-compat" ""
    - 21 tests failed
 - "ethereum/engine" "engine-withdrawals"
    - 18 tests failed
 - "ethereum/engine" "engine-api"
    - 8 tests failed
- "ethereum/engine" "cancun"
    - 12 tests failed
 - "ethereum/sync" ""
    - 1 test failed
 - "devp2p" "eth"
    - 5 tests failed

it shouldn't be performing worse than the latest run on main, which it seems to do now. on main only the engine tests fail 18 rpc compat tests fail for example. https://github.com/paradigmxyz/reth/actions/runs/10875392078

try merging latest main, run again and compare to latest run on main :)

[0xKitsune]

After merging the latest changes on main and rerunning hive against this branch, it looks like the result is the same compared to running against reth:latest. Thanks for the help! Let me know if you want me to produce a specific output or this looks good to merge in.

[mattsse]

I don't necessarily agree with some decisions in this PR

for example:

this is redundant:

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

Line 85 in 6688078

evm_config: EvmConfig,

all these functions accept &self, when not required.

and I find this a bit harder to read now