Skip to content

Rename max IL transactions bytes constant per spec change - #9741

Merged
mergify[bot] merged 11 commits into
sigp:unstablefrom
conache:rename-il-transactions-bytes-constant
Aug 11, 2026
Merged

mergify[bot] merged 11 commits into
sigp:unstablefrom
conache:rename-il-transactions-bytes-constant

Conversation

@conache

@conache conache commented Aug 3, 2026 •

Copy link
Copy Markdown

Proposed Changes

  • Rename MAX_BYTES_PER_INCLUSION_LIST to MAX_TRANSACTIONS_BYTES_PER_INCLUSION_LIST per consensus-specs#5508 PR.

  • Add the missing IL config values to the Config struct, so config file values can modify them instead of Lighthouse silently using its hardcoded defaults:

    • MAX_TRANSACTIONS_BYTES_PER_INCLUSION_LIST
    • MAX_REQUEST_INCLUSION_LIST
    • MIN_SLOTS_FOR_INCLUSION_LISTS_REQUESTS

@conache conache changed the title Heze: rename max IL transactions bytes constant Rename max IL transactions bytes constant per spec change Aug 3, 2026
@chong-he chong-he added ready-for-review The code is ready for review heze labels Aug 4, 2026
@eserilev eserilev added the focil Fork choice enforced inclusion lists label Aug 4, 2026
@mergify

mergify Bot commented Aug 5, 2026

Copy link
Copy Markdown

Some required checks have failed. Could you please take a look @conache? 🙏

@mergify mergify Bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 5, 2026
@mergify mergify Bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 10, 2026

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is fine but i think we should extend the scope of this PR a little bit

Lighthouse never reads this value from a config file. The number 8192 is hardcoded. If a devnet config sets a different value, Lighthouse ignores it and uses 8192 anyway, with no warning.

The fix would be to add the field to the Config struct, and remove the key from the UPSTREAM_KEYS_NOT_IN_LIGHTHOUSE list. Then config files control the value, like every other networking parameter. We should double check that other IL config values are also in the Config struct

@eserilev eserilev added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 10, 2026
@conache

conache commented Aug 10, 2026

Copy link
Copy Markdown
Author

this is fine but i think we should extend the scope of this PR a little bit

Lighthouse never reads this value from a config file. The number 8192 is hardcoded. If a devnet config sets a different value, Lighthouse ignores it and uses 8192 anyway, with no warning.

The fix would be to add the field to the Config struct, and remove the key from the UPSTREAM_KEYS_NOT_IN_LIGHTHOUSE list. Then config files control the value, like every other networking parameter. We should double check that other IL config values are also in the Config struct

Thank you for the context! This makes sense. I added the suggested change for MAX_TRANSACTIONS_BYTES_PER_INCLUSION_LIST.

I also double-checked the other IL config values and found out that the following had the same problem (and added fixes for them):

  • MAX_REQUEST_INCLUSION_LIST
  • MIN_SLOTS_FOR_INCLUSION_LISTS_REQUESTS

@conache
conache requested a review from eserilev August 10, 2026 16:17
@conache
conache requested a review from eserilev August 11, 2026 08:01

@eserilev eserilev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@eserilev eserilev removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Aug 11, 2026
@eserilev eserilev added the ready-for-merge This PR is ready to merge. label Aug 11, 2026
@mergify mergify Bot added the queued label Aug 11, 2026
@mergify

mergify Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 28 minutes 51 seconds in the queue, including 26 minutes 48 seconds running CI.

Required conditions to merge

@mergify
mergify Bot merged commit 326376b into sigp:unstable Aug 11, 2026
38 checks passed
@mergify mergify Bot removed the queued label Aug 11, 2026
@conache
conache deleted the rename-il-transactions-bytes-constant branch August 11, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

focil Fork choice enforced inclusion lists heze ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants