-
Notifications
You must be signed in to change notification settings - Fork 233
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
Adding Kzg Specs, integration to gossipValidation
and it's adjoining tests
#6322
base: wip-peerdas
Are you sure you want to change the base?
Conversation
…into kzgpeerdas
@@ -20,6 +29,8 @@ type | |||
Coset* = array[FIELD_ELEMENTS_PER_CELL, BLSFieldElement] | |||
CosetEvals* = array[FIELD_ELEMENTS_PER_CELL, BLSFieldElement] | |||
Cell* = KzgCell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Cell
used anywhere, or should it be, in place of KzgCell
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, using Cell
should be used ideally and consistently everywhere, I probably used KzgCell
as as experimental debug strategy to locate the buggy non-upstreamed funcs in c-kzg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can definitely fix this
presetPath/"eip7594"/"networking"/"get_custody_columns"/"pyspec_tests" | ||
for kind, path in walkDir(basePath, relative = true, checkDir = true): | ||
runGetCustodyColumns(suiteName, basePath/path) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bunch of odd empty-looking lines at end
@@ -0,0 +1,83 @@ | |||
# beacon_chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this specifically a 7594-related test? If so, it could be more clearly labeled/named as such and apparently not included in all_eip7594_fixtures
. I'm not sure if there are other test fixtures you'd intended to at some point add to that or not, or if consensus-specs
will in general include networking tests so that there will be a more general purpose to something like test_fixture_networking
, but currently it's not clear to me the purpose of its organization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay so this is consensus-specs
specific and i think i named it exactly based on that, the dir was indeed named as networking
, similar to merkle_proof
, etc. and yes if there are further networking fixture tests, it could easily go in there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do understand that eip7594
is sort of hard-coded in the presetPath but that can be altered later, once more eips/specs are tested under this networking dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the same rationale as to naming test_fixture_kzg
, and previously just hardcoding deneb
in one of the dir paths
…6428) * move proofs and cells to ref * move returned value to ref
This branch is for parallely adding kzg/ other EIP7594 (peerDAS) specs/ tests as and when things are completed. Once things look decent enough, they are going to be merged into the
wip-peerdas
branch.