Skip to content

Conversation

@dylanlott
Copy link
Contributor

@dylanlott dylanlott commented Nov 21, 2025

fix: make block query cutoff properly configurable

At some point in the past, the block query cutoff configuration value was removed and a hard-coded value was added in.

This PR adds the block query cutoff back into the config and uses the config value in the calculate_deadline​ function.

Builder example environment documentation needs to be updated to reflect this change.

Copy link
Contributor Author

dylanlott commented Nov 21, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, disregard the other review. This should be called BLOCK_QUERY_CUTOFF_BUFFER

@dylanlott dylanlott marked this pull request as ready for review November 21, 2025 17:45
@dylanlott dylanlott requested a review from prestwich as a code owner November 21, 2025 17:45
@dylanlott dylanlott requested a review from Evalir November 21, 2025 17:45
Copy link
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seconds is not enough resolution. needs to be milliseconds

@dylanlott dylanlott force-pushed the dylan/fix-deadline-cutoff branch from aa0fb98 to 48769bd Compare November 24, 2025 19:28
@dylanlott dylanlott force-pushed the dylan/fix-deadline-cutoff branch from d6fa407 to 376d078 Compare November 24, 2025 22:26
@dylanlott dylanlott force-pushed the dylan/fix-deadline-cutoff branch from cb227d0 to 376d078 Compare November 25, 2025 04:28
@dylanlott dylanlott mentioned this pull request Nov 25, 2025
@dylanlott dylanlott force-pushed the dylan/fix-deadline-cutoff branch from 88c0627 to c7121a6 Compare November 25, 2025 05:06
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's better to explicitly circuit break the building process altogether if the deadline ends up being too close to Instant::now() instead of this being implicit, because it indicates we've ran out of time anyway.

/// current timestamp in UNIX seconds.
/// An `Instant` representing the simulation deadline as calculated by determining
/// the milliseconds left in the current slot and adding that to the current
/// timestamp in UNIX seconds.
Copy link
Member

@prestwich prestwich Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're trying to improve resultion, we need to add to the current timestamp in unix milliseconds. using seconds here gives us all the rounding problems we already had

@prestwich prestwich marked this pull request as draft November 25, 2025 20:33
@prestwich
Copy link
Member

converting to draft until the duration math is removed and the bin-base changes are in

@dylanlott dylanlott force-pushed the dylan/fix-deadline-cutoff branch from 99a5e4a to aee5c4a Compare November 26, 2025 02:52
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.

5 participants