Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove AddSingularBatch from ChannelOut interface (prefer AddBlock) #12079

Conversation

mdehoog
Copy link
Contributor

@mdehoog mdehoog commented Sep 24, 2024

Description

Removes the AddSingularBatch function from the ChannelOut interface. This was duplicative of the AddBlock method.

There wasn't a case where AddSingularBatch was called without calling BlockToSingularBatch beforehand (apart from in tests), so this PR is a simplification of the exposed interface. It also lets future ChannelOut implementations access the Block if needed, as only exposing AddBlock forces callers to always pass a block.

Tests

Existing tests have been updated to the newly exposed interface. There were some tests that were specifically testing AddSingularBatch, so a new test-only interface has been added to expose this as an internal method addSingularBatch. Also one of the benchmark tests was updated to call AddBlock rather than AddSingularBatch which required a small helper function to convert from a SinglularBatch to a Block.

Additional context

I'd like to access the block (to check for withdrawal logs in the Bloom) in a ChannelOut for L3 batch submission. Exposing the alternate AddSingularBatch was preventing this.

@mdehoog mdehoog force-pushed the michael/channel-out-remove-add-singular-batch branch 10 times, most recently from 13b5ca3 to 064b121 Compare September 24, 2024 06:54
@mdehoog mdehoog marked this pull request as ready for review September 24, 2024 07:06
@mdehoog mdehoog requested review from a team as code owners September 24, 2024 07:06
@tynes tynes requested a review from geoknee October 2, 2024 20:53
@tynes
Copy link
Contributor

tynes commented Oct 2, 2024

This seems like a sane change, @geoknee do you mind reviewing this when you have some time?

@mdehoog mdehoog force-pushed the michael/channel-out-remove-add-singular-batch branch from beb5c7d to ecee6a0 Compare October 5, 2024 03:03
@mdehoog
Copy link
Contributor Author

mdehoog commented Oct 5, 2024

(rebased on latest develop branch)

@geoknee geoknee added this pull request to the merge queue Oct 7, 2024
Merged via the queue into ethereum-optimism:develop with commit d062c1c Oct 7, 2024
60 checks passed
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.

3 participants