Limit batch buffer capacity by QuentinI · Pull Request #348 · EspressoSystems/optimism-espresso-integration

QuentinI · 2026-02-05T17:59:47Z

Closes #<ISSUE_NUMBER>

This PR:

Limits batch buffer capacity
Introduces headBatch field in streamer to avoid getting stuck if batch buffer is completely filled with future batches
Puts undecided batches into the same queue as decided to avoid time-based non-determinism

This PR does not:

Key places to review:

gemini-code-assist · 2026-02-05T18:00:04Z

Summary of Changes

Hello @QuentinI, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces significant refinements to the batch processing mechanism within the espresso package. By implementing a fixed capacity for the batch buffer and introducing a dedicated headBatch concept in the streamer, the changes aim to enhance stability and predictability. The updated logic ensures that the system can gracefully handle scenarios where the buffer is full or when batches require further L1 information, preventing potential deadlocks and improving the overall robustness of the batch streaming process.

Highlights

Batch Buffer Capacity: The batch buffer now has a defined capacity, preventing it from growing indefinitely and potentially causing performance issues or memory exhaustion. A new error ErrAtCapacity is introduced to signal when the buffer is full.
Streamer Head Batch Management: A new headBatch field has been added to the BatchStreamer to explicitly manage the next batch to be processed. This helps avoid situations where the streamer might get stuck if the buffer is completely filled with future batches, ensuring a deterministic flow.
Simplified Batch Processing Logic: The BatchStreamer's CheckBatch method no longer handles insertion logic, focusing solely on validity. The processRemainingBatches function has been removed, and the logic for handling undecided batches and inserting them into the buffer has been integrated directly into processEspressoTransaction and HasNext.
Improved Batch Insertion: The BatchBuffer's Insert method now includes a custom comparison function for slices.BinarySearchFunc to correctly handle cases where multiple batches might have the same number but different hashes, ensuring proper ordering and preventing duplicates.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

espresso/batch_buffer.go
- Removed cmp import and added errors import.
- Removed BatchFuture constant and introduced ErrAtCapacity.
- Added capacity field to BatchBuffer struct and updated NewBatchBuffer to accept a capacity argument.
- Added Capacity() method to BatchBuffer.
- Modified Insert method to check buffer capacity and return ErrAtCapacity if full.
- Updated Insert method to use a custom BinarySearchFunc for more precise batch insertion and duplicate handling.
- Removed the TryInsert method.
espresso/streamer.go
- Added errors and math imports.
- Defined BatchBufferCapacity constant (1024).
- Added skipPos and headBatch fields to the BatchStreamer struct.
- Updated NewEspressoStreamer to initialize BatchBuffer with the new capacity and set skipPos.
- Removed the RemainingBatches map.
- Modified CheckBatch to return only BatchValidity, removing the integer position return.
- Removed the processRemainingBatches function.
- Updated fetchHotShotRange to handle ErrAtCapacity by setting skipPos.
- Refactored processEspressoTransaction to return an error, manage headBatch, and handle buffer insertions directly.
- Modified Next method to retrieve and clear headBatch, and reset hotShotPos if skipPos was active.
- Updated HasNext method to manage headBatch directly, peek from the buffer, and re-evaluate batch validity (Drop, Undecided, Past) to find the next valid batch.

Activity

QuentinI created this pull request to address an issue related to batch buffer capacity and processing logic.
The PR description clearly outlines the key changes, including limiting batch buffer capacity, introducing a headBatch field, and modifying how undecided batches are handled.
The changes involve refactoring core batch processing functions and data structures in espresso/batch_buffer.go and espresso/streamer.go.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a capacity limit for the batch buffer, which is a crucial step to prevent unbounded memory usage. The changes also include a more robust handling of undecided batches by placing them in the main buffer for later re-evaluation, which helps in reducing non-determinism. The introduction of headBatch and skipPos in the streamer provides a graceful mechanism to manage a full buffer without getting stuck. Overall, the logic is sound and addresses the intended problem effectively. I have a few suggestions to enhance robustness and code clarity.

espresso/batch_buffer.go

espresso/streamer.go

shenkeyao · 2026-02-12T02:39:45Z

espresso/batch_buffer.go

+		// Note: we use a custom comparison function that returns 0 only if the batches are actually
+		// the same to ensure that newer batches with the same number are stored later in the buffer


I was wondering about the case where a new batch has the same hash as an existing batch but a different number. Can we simplify the second if-clause to check the hash only?

(And would a test be helpful for this case?)

a new batch has the same hash as an existing batch but a different number

That would be a hash collision, block number is part of the hash.

Can we simplify the second if-clause to check the hash only?

In practice yes, equal hashes imply equal numbers absent any collisions, but I feel like it would read worse, because the second if clause would have none of the operands of the first if clause.

shenkeyao · 2026-02-12T02:47:16Z

espresso/streamer.go

-			s.processEspressoTransaction(ctx, txn.Payload)
+			err := s.processEspressoTransaction(ctx, txn.Payload)
+			if errors.Is(err, ErrAtCapacity) {
+				s.skipPos = start


Can we exit here with the error without continuing the iteration?

No, because we may still be looking for a missing head batch.

Makes sense! A follow-up question: should we set skipPos conditionally, i.e., compare it with the current value first before using the new start value?

Indeed, thank you for the catch! Fixed and added a regression test.

jjeangal

Overall looks good me!
One remark that I have is about the way the testing is handled. There seems to have been a lot of additions and new tests added which is a very good thing but would there be ways to optimize or delete duplicated test logic, etc?

espresso/batch_buffer.go

jjeangal · 2026-02-13T16:32:28Z

espresso/batch_buffer.go

+		// the same to ensure that newer batches with the same number are stored later in the buffer
+		if a.Number() > t.Number() {
+			return 1
+		} else if a.Number() == t.Number() && a.Hash() == t.Hash() {


With your comment above in mind we would only need to compare the hash right? Not that big of a deal tho I guess.

Yes, I deliberately left the Numbers there because I thought it would read better (see my response to similar question raised by Keyao), but it seems like 2/2 reviewers find it more confusing... Removed.

espresso/batch_buffer.go

skipPos was unconditionally assigned to start on each ErrAtCapacity, causing later fetch ranges to overwrite the value set by earlier ones. After a rewind, batches dropped in the earlier ranges were permanently skipped. Use min(skipPos, start) to preserve the earliest position. Co-authored-by: OpenCode <noreply@opencode.ai>

QuentinI · 2026-02-13T17:46:49Z

@jjeangal re: tests - I must admit I haven't checked that I'm not adding any duplicates very thoroughly, I removed a couple tests that were already covered. Everything that remains should be purely new coverage.

Return ErrDuplicateBatch from BatchBuffer.Insert so the streamer can log a warning when a duplicate hash is encountered. Update tests accordingly. Co-authored-by: OpenCode <noreply@opencode.ai>

jjeangal

Thanks for addressing the comments. LGTM!

* Add cap to batch buffer * Gemini's suggestions * No recursion * Fix typo * Fmt * Rewrite test * Add tests * Remove unused function * Fix skipPos overwrite across multiple fetch ranges skipPos was unconditionally assigned to start on each ErrAtCapacity, causing later fetch ranges to overwrite the value set by earlier ones. After a rewind, batches dropped in the earlier ranges were permanently skipped. Use min(skipPos, start) to preserve the earliest position. Co-authored-by: OpenCode <noreply@opencode.ai> * Comments * Log warning on duplicate batch insertion Return ErrDuplicateBatch from BatchBuffer.Insert so the streamer can log a warning when a duplicate hash is encountered. Update tests accordingly. Co-authored-by: OpenCode <noreply@opencode.ai> --------- Co-authored-by: OpenCode <noreply@opencode.ai>

Add cap to batch buffer

9344db9

gemini-code-assist bot reviewed Feb 5, 2026

View reviewed changes

espresso/batch_buffer.go Outdated Show resolved Hide resolved

espresso/streamer.go Show resolved Hide resolved

espresso/streamer.go Show resolved Hide resolved

Gemini's suggestions

12afb24

QuentinI force-pushed the ag/capped-batcher branch from 4808a80 to 12afb24 Compare February 5, 2026 18:08

QuentinI added 6 commits February 5, 2026 19:15

No recursion

3149223

Fix typo

1ccafe8

Fmt

1df08f3

Rewrite test

dcf4fc7

Add tests

73c1b56

Remove unused function

e76b92e

QuentinI mentioned this pull request Feb 10, 2026

Audit fixes #350

Merged

shenkeyao reviewed Feb 12, 2026

View reviewed changes

jjeangal reviewed Feb 13, 2026

View reviewed changes

QuentinI and others added 2 commits February 13, 2026 18:32

Comments

fdb74a5

Log warning on duplicate batch insertion

60fb7ea

Return ErrDuplicateBatch from BatchBuffer.Insert so the streamer can log a warning when a duplicate hash is encountered. Update tests accordingly. Co-authored-by: OpenCode <noreply@opencode.ai>

jjeangal approved these changes Feb 13, 2026

View reviewed changes

QuentinI merged commit 9b83f58 into celo-integration-rebase-14.2 Feb 14, 2026
38 of 40 checks passed

QuentinI deleted the ag/capped-batcher branch February 14, 2026 01:48

		// Note: we use a custom comparison function that returns 0 only if the batches are actually
		// the same to ensure that newer batches with the same number are stored later in the buffer

Conversation

QuentinI commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR:

This PR does not:

Key places to review:

Uh oh!

gemini-code-assist bot commented Feb 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shenkeyao Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

QuentinI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

shenkeyao Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

QuentinI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

shenkeyao Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

QuentinI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

jjeangal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjeangal Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

QuentinI Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

QuentinI commented Feb 13, 2026

Uh oh!

jjeangal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

QuentinI commented Feb 5, 2026 •

edited

Loading

QuentinI Feb 13, 2026 •

edited

Loading