Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nrpc re-org handling #2994

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

nrpc re-org handling #2994

wants to merge 5 commits into from

Conversation

advaita-saha
Copy link
Contributor

Adds re-org handling capacity to nrpc along with fixes for #2796

@advaita-saha advaita-saha changed the title Nrpc re-org handling nrpc re-org handling Jan 13, 2025
@advaita-saha advaita-saha changed the title nrpc re-org handling nrpc re-org handling Jan 13, 2025
nrpc/nrpc.nim Outdated Show resolved Hide resolved
@advaita-saha advaita-saha requested a review from tersec January 14, 2025 10:41

info "newPayload Request sent",
blockNumber = int(payload.blockNumber), response = payloadResponse.status

if payloadResponse.status != PayloadExecutionStatus.valid:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is too strict -- syncing and accepted are fine too.

This should trigger on what https://github.com/ethereum/consensus-specs/blob/v1.5.0-beta.0/sync/optimistic.md#helpers calls INVALIDATED: INVALID or INVALID_BLOCK_HASH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But syncing and accepted shouldn't be expected the behaviour if nrpc sync is running and syncing properly

Copy link
Contributor

Choose a reason for hiding this comment

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

If the EL starts out as not synced, then in general an EL which does snap sync will go some hours before it starts responding with anything but syncing or accepted.

nimbus-eth1/nrpc/nrpc.nim

Lines 212 to 217 in cee5fe9

if fcuResponse.payloadStatus.status != PayloadExecutionStatus.valid:
error "Forkchoice not validated", status = fcuResponse.payloadStatus.status
quit(QuitFailure)
else:
info "forkchoiceUpdated Request sent",
response = fcuResponse.payloadStatus.status

has a similar issue here.

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.

2 participants