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

update pre-capella hive test vectors #40

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

Conversation

njgheorghita
Copy link
Contributor

This pr updates all pre-capella HeaderWithProof ContentValue test vectors for use in hive. These vectors are pulled automatically in hive, so it will likely cause failures on any clients who haven't activated/implemented the new type.

There is still some ongoing discussion to the exact type to be used in post-capella proofs, so I decided to skip those for now.

Any confirmation / validation that these test vectors can be correctly decoded & the proofs validated should happen before merging this.

@KolbyML
Copy link
Member

KolbyML commented Feb 19, 2025

It is fine if we want to delay post-capella for now, you will need to comment out the post-capella test vectors for the time being then, as our Hive code expects decodable test data. After this is merged we will have to update Trin's json-rpc code to use the updated HeaderWithProof types, then update hive to use the latest version of ethportal-api

@KolbyML
Copy link
Member

KolbyML commented Feb 19, 2025

Ideally we sync up merging this and updating trin/hive to use the latest HeaderWithProof format around the same time window, else when hive tries to run the history test suite it will fail to decode the new test data

@morph-dev
Copy link
Contributor

I agree with Kolby, all test-vectors should be decodable by new types. Since post-capella is tricky (with potential upcoming changes), I would just comment all post-capella entries for now.

Or if you want, you can delete them (as they will be available via git history), but I think we can just comment them out.

@kdeme
Copy link
Collaborator

kdeme commented Feb 19, 2025

The changes to the test vectors look good. I'd be fine with commenting out the rest (I understand now that even 1 failed decoding would make the whole suite not run at all?)

@KolbyML
Copy link
Member

KolbyML commented Feb 19, 2025

I understand now that even 1 failed decoding would make the whole suite not run at all?

Yes and also if we don't update which HeaderWithProof types Trin uses, then update hive

Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

Can confirm that Ultralight is able to decode all of these just fine.

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.

5 participants