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

Handle unavailable data outside of prune window #3169

Merged
merged 1 commit into from
Dec 21, 2022
Merged

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Dec 19, 2022

I believe this captures the intention of the discussion from #3141. This does not actually change the spec, it just clarifies what to do in a few scenarios. namely:

  1. It notes that retrieve_blobs_sidecar might fail on the p2p network outside of the prune window. Thus is_data_available would fail in this scenario due to the exception thrown
  2. It also notes that "Blocks that have been previously validated as available SHOULD be considered available even if the associated BlobsSidecar has subsequently been pruned." This is to avoid interactions with pruning and re-orgs past the depth. If a client has validated that a block's sidecar is available, and then re-orgs to this sidecar at a depth past the prune window, the client does not need to re-assert the availability.

Copy link
Contributor

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

Lgtm. I think there's some value in updating /phase0/weak-subjectivity.md to include eip-4844 considerations. It can be another PR

@djrtwo djrtwo merged commit 4f77493 into dev Dec 21, 2022
@djrtwo djrtwo deleted the false-after-prune branch December 21, 2022 00:15
@dapplion
Copy link
Collaborator

From interop event discussions there is still some confusion regarding the practical implications of this PR.

  • As of now consensus implementations can sync from genesis (some emit warnings about WS unsafety)
  • As a result of this PR, should a spec-compliant implementation prevent syncing from genesis?

Since that has significant UX implications from status-quo, it might be explicitly stated somewhere in the spec.

@dankrad
Copy link
Contributor

dankrad commented Jan 24, 2023

As a result of this PR, should a spec-compliant implementation prevent syncing from genesis?

I think it has always been pretty clearly stated that syncing from genesis is not safe because of weak subjectivity? So how is this new?

The safe way to sync is to get a checkpoint within the weak subjectivity window.

If you sync from genesis, you make the assumption that no exited key attack has happened. This would warrant at least a warning by the client -- probably better it should be an override flag in order to force the client into this sync mode. I think the same is true for syncing from genesis without checking data availability, but since in practice they cover the same blocks, the override flag for the two can be combined.

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.

6 participants