Skip to content

Review 5186 - #20

Merged
potuz merged 2 commits into
potuz:proposer_dafrom
nflaig:review-proposer-da
May 13, 2026
Merged

Review 5186#20
potuz merged 2 commits into
potuz:proposer_dafrom
nflaig:review-proposer-da

Conversation

@nflaig

@nflaig nflaig commented May 7, 2026

Copy link
Copy Markdown

No description provided.

@nflaig

nflaig commented May 12, 2026

Copy link
Copy Markdown
Author

cc @potuz

Comment thread specs/gloas/fork-choice.md Outdated
assert head.payload_status != PAYLOAD_STATUS_PENDING
if head.payload_status == PAYLOAD_STATUS_EMPTY:
def should_build_on_full(store: Store, root: Root) -> bool:
if not should_extend_payload(store, root):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This feels weird to call all this function when it's just equivalent to is_payload_verified for the proposer. OTOH, the payload could be verified but the head still be empty, this is the reason I decided to go with head in my PR instead of just root.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, this is indeed a bug in my refactor, addressed this in 795050c

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

for context

image

@potuz potuz left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM

@potuz
potuz merged commit a1eaa17 into potuz:proposer_da May 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants