Skip to content

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Sep 1, 2025

Purpose:

correct the block cost accounting in create_block_generator2(). This function is still disabled by default.

The variable block_cost is an estimate of the current cost of the block + the cost of the current batch of transactions that haven't been added yet.

Current Behavior:

The block_cost estimate is assumed to match the final cost returned by the BlockBuilder. This is not a safe assumption.

New Behavior:

Disregard block_cost once the block is built, and just look at cost instead, which is the cost returned by the BlockBuilder object.

Testing Notes:

The new test exercises the path where the last batch is attempted to be added to the block, but fails. In this scenario, the estimated block cost is still incremented with the next transaction, even though it's never added. This case would cause the assertion to fail (before this PR removes it).

@arvidn arvidn force-pushed the create_block_generator2 branch from 00281bc to 6075d97 Compare September 1, 2025 16:51
@arvidn arvidn added Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Sep 1, 2025
@arvidn arvidn marked this pull request as ready for review September 1, 2025 18:02
@arvidn arvidn requested a review from a team as a code owner September 1, 2025 18:02
@arvidn arvidn requested a review from AmineKhaldi September 1, 2025 18:02
@arvidn arvidn closed this Sep 2, 2025
@arvidn arvidn reopened this Sep 2, 2025
@arvidn arvidn requested a review from wjblanke September 2, 2025 15:19
Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

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

aok

@arvidn arvidn added the ready_to_merge Submitter and reviewers think this is ready label Sep 3, 2025
@pmaslana pmaslana merged commit 6df066f into main Sep 4, 2025
2135 of 2148 checks passed
@pmaslana pmaslana deleted the create_block_generator2 branch September 4, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants