chore(builder): remove flashblocks extra context #448
Conversation
🟡 Heimdall Review Status
|
| Pool: PoolBounds, | ||
| Client: ClientBounds, | ||
| { | ||
| #[allow(clippy::too_many_arguments)] |
There was a problem hiding this comment.
ah i dont love this, but i guess its fine for now
There was a problem hiding this comment.
Yea I'm not sure this is worth it
Farukest
force-pushed
the
chore/remove-flashblocks-extra-context
branch
from
8a04383 to
f625850
Compare
All done @haardikk21 let me know if anything else needs |
haardikk21
left a comment
haardikk21
left a comment
There was a problem hiding this comment.
LGTM, will wait for another review before merge
Farukest
force-pushed
the
chore/remove-flashblocks-extra-context
branch
from
f625850 to
5f82ea2
Compare
danyalprout
left a comment
danyalprout
left a comment
There was a problem hiding this comment.
Looks reasonable. Can you please fix the tests/clippy and I'll approve
Remove FlashblocksExtraCtx struct and inline its fields directly into OpPayloadBuilderCtx. This simplifies the code by eliminating an unnecessary layer of indirection. - Move all FlashblocksExtraCtx fields into OpPayloadBuilderCtx - Replace with_extra_ctx() with with_next_targets() - Update all ctx.extra.xxx usages to ctx.xxx - Return tuple instead of struct from build_next_flashblock() - Remove redundant flashblock_index() and target_flashblock_count() getters Closes base#443
haardikk21
force-pushed
the
chore/remove-flashblocks-extra-context
branch
from
a12683c to
d043ba2
Compare
refcell
left a comment
refcell
left a comment
There was a problem hiding this comment.
I don't think this is worth it. The previous encapsulation of flashblock-related context made OpPayloadBuilderCtx a lot more approachable. We're pretty much just inlining for the sake of inlining here with no real benefit / reduction in logic. I'd prefer not to merge this change.
|
Going to close this for now. Preference for the encapsulated way |
5 participants
Summary
Remove the
FlashblocksExtraCtxstruct and inline its fields directly intoOpPayloadBuilderCtx. This eliminates an unnecessary layer of indirection since flashblocks is now the only builder mode.Changes
FlashblocksExtraCtxfields intoOpPayloadBuilderCtxwith_extra_ctx()withwith_next_flashblock()ctx.extra.xxxusages toctx.xxxbuild_next_flashblock()Test plan
cargo check -p op-rbuilderpassescargo check -p op-rbuilder --testspassesCloses #443