Skip to content

Update consensus spec to v1.6.0-alpha.4 and implement data column support#15590

Merged
terencechain merged 2 commits intodevelopfrom
v1.6.0-alpha4-spec-tests
Aug 16, 2025
Merged

Update consensus spec to v1.6.0-alpha.4 and implement data column support#15590
terencechain merged 2 commits intodevelopfrom
v1.6.0-alpha4-spec-tests

Conversation

@terencechain
Copy link
Collaborator

  • Bumped consensus spec version from v1.6.0-alpha.1 to v1.6.0-alpha.4
  • Added SpectestDataColumnSidecarRequirements for forkchoice spectest data column verification
  • Implemented runDataColumnStep() function to handle data column sidecar processing
  • Clear committee cache at every fork choice test given fulu uses committess in core processing

@terencechain terencechain force-pushed the v1.6.0-alpha4-spec-tests branch from d12eda8 to 8bbb5cd Compare August 13, 2025 19:51
columnFiles = append(columnFiles, step.DataColumns...)
}
if len(columnFiles) == 0 {
return
Copy link
Member

Choose a reason for hiding this comment

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

Are there cases where there are not files? Could this be a false positive if the test runs on an empty directory but there were files expected? Perhaps this should fail here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ya good point, will change it to fail

Co-Authored-By: Preston Van Loon <pvanloon@offchainlabs.com>
@terencechain terencechain force-pushed the v1.6.0-alpha4-spec-tests branch from ed913c5 to 764d8d4 Compare August 14, 2025 16:20
}
}

if err := dv.NotFromFutureSlot(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think maybe i'm not familiar with the design, but maybe on the verifier we can have a function that returns the verification function based on the verification value instead of manually adding them below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think that's a good idea, ill do it in another pr 🫡

if !*step.Valid {
return func(t *testing.T, err error) {
require.ErrorIs(t, err, expect)
if expect.Error() == "data columns failed verification" {
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems potentially fragile, we should have a named variables if it's suppose to be be special I think

Copy link
Contributor

Choose a reason for hiding this comment

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

you added one on line 322 i guess

@terencechain terencechain added this pull request to the merge queue Aug 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2025
@terencechain terencechain added this pull request to the merge queue Aug 16, 2025
Merged via the queue into develop with commit 6528fb9 Aug 16, 2025
15 checks passed
@terencechain terencechain deleted the v1.6.0-alpha4-spec-tests branch August 16, 2025 16:14
jihoonsong pushed a commit to jihoonsong/prysm that referenced this pull request Aug 27, 2025
…port (OffchainLabs#15590)

* Update consensus spec to v1.6.0-alpha.4 and implement data column support for forkchoice spectests

* Apply suggestion from @prestonvanloon

Co-Authored-By: Preston Van Loon <pvanloon@offchainlabs.com>

---------

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
fernantho pushed a commit to fernantho/prysm that referenced this pull request Sep 26, 2025
…port (OffchainLabs#15590)

* Update consensus spec to v1.6.0-alpha.4 and implement data column support for forkchoice spectests

* Apply suggestion from @prestonvanloon

Co-Authored-By: Preston Van Loon <pvanloon@offchainlabs.com>

---------

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
fernantho pushed a commit to fernantho/prysm that referenced this pull request Sep 26, 2025
…port (OffchainLabs#15590)

* Update consensus spec to v1.6.0-alpha.4 and implement data column support for forkchoice spectests

* Apply suggestion from @prestonvanloon

Co-Authored-By: Preston Van Loon <pvanloon@offchainlabs.com>

---------

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
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.

3 participants

Comments