Skip to content

Conversation

cody-wang-cb
Copy link
Contributor

@cody-wang-cb cody-wang-cb commented Sep 30, 2025

📝 Summary

Adds a configurable maximum limit for the number of flashblocks that can be produced per block, ensures the maximum cannot exceed configured value.

💡 Motivation and Context

The flashblock count was previously determined purely by timing calculations (block_time / flashblock_interval), which doesn't prevent the count go over 10 when using flashblock_interval < 200 for a 2s block time chain.
By capping the flashblocks it adds another layer of insurance that it won't lead to accidental unwanted behaviors in terms of the number of flashblocks

Changes

  • CLI Flag: Added --flashblocks.max-per-block with default value of 10
  • Environment Variable: FLASHBLOCKS_MAX_PER_BLOCK support
  • Configuration: Added max_flashblocks_per_block field to FlashblocksConfig
  • Logic: Applied cap in both fixed and dynamic timing modes:
    • Fixed mode: Cap applied in flashblocks_per_block() method
    • Dynamic mode: Cap applied at the end of timing calculations
  • Tests: Added comprehensive test coverage for both timing modes

Implementation Details

The cap is applied in two places:

  1. FlashBlocksConfigExt::flashblocks_per_block() - handles fixed timing mode
  2. End of calculate_flashblocks() - handles dynamic timing mode calculations

This approach ensures consistent behavior across both modes without restructuring the existing timing logic.

Testing

  • ✅ test_max_flashblocks_per_block_cap - tests fixed timing mode with cap of 3
  • ✅ test_max_flashblocks_dynamic_timing - tests dynamic timing mode with cap of 5
  • ✅ All existing flashblocks tests continue to pass

@avalonche
Copy link
Collaborator

what's the use case for this? It could easily lead to misconfiguration using flashblock_interval < 200 for a 2s block time and the default caps it at 10. Could the default be unlimited?

@cody-wang-cb
Copy link
Contributor Author

@avalonche by capping the flashblocks it adds another layer of insurance that it won't lead to accidental unwanted behaviors in terms of the number of flashblocks
On Base we had to tune the interval to be shorter because of the higher time drift due to the higher latency on the engine API calls, however while we were tuning the params it sent more than 10 flashblocks for a period of time when the params were not tuned well enough
So feel like this could have been somewhat useful for this case, and maybe other teams would want it as well

@avalonche
Copy link
Collaborator

I think it's possible to add a "buffer" for each flashblock to tune the interval instead of capping a maximum flashblock number. Would that achieve the same result? This seems more like a patch solution to the unintended behaviour of another hacky solution and easily leads to misconfiguration.

@SozinM
Copy link
Collaborator

SozinM commented Oct 4, 2025

@cody-wang-cb i think there is easier way to address this:
We have cli param to specify blocks time
And we could modify this line:

        let Ok(time_drift) = target_time.duration_since(now) else {
            error!(
                target: "payload_builder",
                message = "FCU arrived too late or system clock are unsynced",
                ?target_time,
                ?now,
            );
            return (
                self.config.flashblocks_per_block(),
                self.config.specific.interval,
            );
        };

And do
time_drift.map(|drift| drift.max(self.config.block_time))
To ensure that we will have max time as expected

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