-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Implement committee_hash lightclient api #4736
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
Thanks for taking the task! I guess you haven't added the hashing part yet? The hash should be simply the keccak256 of the concatenated public keys of the sync committee. Here is the current prover implementation |
74ce77a
to
e7ae483
Compare
Maybe consider adding tests that covers the added endpoint? apart from that LGTM. |
it already covered the end point as long as we update the testData for lightclient, doubled checked it via the unit test output
|
@shresthagrawal Why don't you do the hashTreeRoot of the SyncCommittee instead of concat pubkeys + hash? It's kindof tangent to your classic object hashing in the beacon chain. Plus the hashTreeRoot of the SyncCommittee is usually already available due to state hashing. |
@dapplion yes that works, I already mentioned that in the GitHub issue earlier #4733 (comment) |
Unfortunately that test only seem to test some of the type machinery and not the actual implementation. I updated the implementation to throw an error like this:
Ran the test and also got:
You can also confirm this. What will be needed is e2e test, and the few existing ones can be found in |
1ab00f2
to
0a40aa5
Compare
@dadepo thanks, implemented it |
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.
LGTM
* Implement committee_hash lightclient api * Fix check-types in light-client package * Return committee hash instead of committee itself * Fix lint in light-client * Tested successfuly on local server * Address PR comments * Add lightclient e2e tests * Switch to committee root
Motivation
Description
syncCommitteeWitness
dbsyncCommittee
dbpart of #4733