Skip to content

Gloas serve post block state for finalized/justified state requests#9092

Merged
mergify[bot] merged 7 commits intosigp:unstablefrom
eserilev:gloas-serve-finalized-post-block-state
Apr 8, 2026
Merged

Gloas serve post block state for finalized/justified state requests#9092
mergify[bot] merged 7 commits intosigp:unstablefrom
eserilev:gloas-serve-finalized-post-block-state

Conversation

@eserilev
Copy link
Copy Markdown
Member

@eserilev eserilev commented Apr 4, 2026 •
edited
Loading

Finalized/justified states for gloas are always the post block state. This PR allows us to serve checkpoint sync data for Gloas

@eserilev eserilev added the ready-for-review The code is ready for review label Apr 5, 2026
@eserilev eserilev mentioned this pull request Apr 5, 2026
Open
40 tasks
pawanjay176
pawanjay176 reviewed Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Just a nit and some additional documentation suggestions.

I also think we should add http api tests that verifies we get the right state pre and post gloas. We could do that once we sort out the remaining db/fork choice stuff and make an issue for it now. What do you think?

Comment thread beacon_node/http_api/src/state_id.rs Outdated
Comment thread beacon_node/http_api/src/state_id.rs
michaelsproul
michaelsproul approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

This is good, and I think helps with some of my DB stuff.

Agree with Pawan that we should open an issue to test this more thoroughly. A lot of these state-related paths are going to need hammering with tests.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 8, 2026
@mergify mergify Bot added the queued label Apr 8, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 8, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 2 minutes 16 seconds in the queue, with no time running CI.

Required conditions to merge

Reason

Pull request #9092 has been dequeued

The pull request rule doesn't match anymore. The following conditions don't match anymore:

  • label=ready-for-merge

Hint

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

@michaelsproul michaelsproul removed the ready-for-merge This PR is ready to merge. label Apr 8, 2026
@michaelsproul
Copy link
Copy Markdown
Member

michaelsproul commented Apr 8, 2026

@mergify dequeue

mergify Bot added a commit that referenced this pull request Apr 8, 2026
@mergify
Merge of #9092
456b6fe
@mergify mergify Bot mentioned this pull request Apr 8, 2026
Closed
11 tasks
@mergify mergify Bot added dequeued and removed queued labels Apr 8, 2026
@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Apr 8, 2026
@mergify mergify Bot removed the dequeued label Apr 8, 2026
michaelsproul
michaelsproul reviewed Apr 8, 2026
View reviewed changes
Comment thread beacon_node/http_api/src/state_id.rs Outdated

if chain
.spec
.fork_name_at_slot::<T::EthSpec>(slot)
Copy link
Copy Markdown
Member

@michaelsproul michaelsproul Apr 8, 2026

Choose a reason for hiding this comment

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

Should maybe be block.slot to account for the case where a pre-Gloas state is finalized at the Gloas fork boundary epoch?

Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 Apr 8, 2026

Choose a reason for hiding this comment

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

I think this is correct. the fork boundary block would be pre-gloas so there would be no envelope. so the more correct state to return there should be advanced to the epoch boundary like we do pre-gloas

Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 Apr 8, 2026

Choose a reason for hiding this comment

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

Used block.slot for both finalized and justified in 51e08d0

michaelsproul
michaelsproul approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks good now. Just one nit, but I think this is fine

pawanjay176
pawanjay176 approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

LGTM. applied michael's suggestion and also changed the regular comment to a doc comment so its easier to check with rust-analyzer. Gonna merge when CI passes

@pawanjay176 pawanjay176 mentioned this pull request Apr 8, 2026
Closed
@pawanjay176 pawanjay176 removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Apr 8, 2026
@pawanjay176 pawanjay176 added the ready-for-merge This PR is ready to merge. label Apr 8, 2026
@mergify mergify Bot added the queued label Apr 8, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 8, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 29 minutes 43 seconds in the queue, including 28 minutes 10 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Apr 8, 2026
@mergify
Merge of #9092
fd8d794
@mergify mergify Bot mentioned this pull request Apr 8, 2026
Closed
10 tasks
@mergify mergify Bot merged commit 2749e18 into sigp:unstable Apr 8, 2026
39 checks passed
@mergify mergify Bot removed the queued label Apr 8, 2026
michaelsproul added a commit to michaelsproul/lighthouse that referenced this pull request Apr 21, 2026
@michaelsproul
Revert "Gloas serve post block state for finalized/justified state re…
716caf1 
…quests (sigp#9092)"

This reverts commit 2749e18.
@michaelsproul michaelsproul mentioned this pull request Apr 21, 2026
Merged
mergify Bot pushed a commit that referenced this pull request Apr 21, 2026
@michaelsproul
Revert Gloas API changes from 9092 (#9151)
02c2841 
This reverts commit 2749e18, from:

- #9092

We no longer need those changes since the abolition of pending/full states.


  


Co-Authored-By: Michael Sproul <michael@sigmaprime.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelsproul michaelsproul michaelsproul approved these changes

@pawanjay176 pawanjay176 pawanjay176 approved these changes

Assignees

No one assigned

Labels

gloas HTTP-API ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eserilev @michaelsproul @pawanjay176