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.

Sign up for GitHub

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

Jump to bottom

Hstaykov/add test for weigh justification and finalization #262

Merged
merged 12 commits into from
Jan 24, 2024
Merged

Hstaykov/add test for weigh justification and finalization #262

merged 12 commits into from
Jan 24, 2024

Conversation

HristoStaykov
Copy link
Contributor

@HristoStaykov HristoStaykov commented Nov 6, 2023 •
edited
Loading

Added implementation for compute_shuffled_index and weigh_justification_and_finalization + initial test.

@HristoStaykov HristoStaykov force-pushed the hstaykov/add_test_for_weigh_justification_and_finalization branch 3 times, most recently from 8034ac8 to 858f385 Compare November 12, 2023 17:56
@Owliie
Copy link
Contributor

Owliie commented Nov 20, 2023

Reuse vendor/consensus-spec-tests for the test files

@Xearty
Copy link
Contributor

Xearty commented Nov 20, 2023

Build weigh_justification_and_finalization as a circuit

@Xearty
Copy link
Contributor

Xearty commented Nov 20, 2023

Rename *_imp files to *_impl

@Xearty
Copy link
Contributor

Xearty commented Nov 20, 2023

Are we sure we want all of this in vendor?

@HristoStaykov HristoStaykov requested a review from Xearty January 17, 2024 13:38
Xearty
Xearty reviewed Jan 23, 2024
View reviewed changes
vendor/zkllvm-metacraft-circuits/src/circuits_impl/weigh_justification_and_finalization.h
Comment on lines +33 to +38
void inline verify_finalized_checkpoint(Root beacon_state_root,
CheckpointVariable finalized_checkpoint,
BeaconStateLeafProof proof) {
ssz_verify_proof(
beacon_state_root, hash_tree_root(finalized_checkpoint), proof, BEACON_STATE_FINALIZED_CHECKPOINT_GINDEX);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Inline this code

Xearty
Xearty reviewed Jan 23, 2024
View reviewed changes
...lvm-metacraft-circuits/src/tests/compute_shuffled_index_test/compute_shuffled_index_test.cpp
};

std::vector<path> result;
path my_path("/consensus-spec-tests");
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work outside of a docker image. Try using a relative path.

@HristoStaykov HristoStaykov requested a review from Xearty January 23, 2024 13:54
@HristoStaykov
build(zkllvm-circuits/cmake): Enable address sanitizer
a890e35
@HristoStaykov
feat(zkllvm-circuits/compute_shuffled_index_runable): Implement logic…
3abb01e 
… for reading mappings from yaml config files
@HristoStaykov
build(zkllvm-circuits/cmake): Created new directory structure.
b1eeb92 
Refactor in order to keep the circuits,
tests and test case data in separate folders.
@HristoStaykov
test(zkllvm-circuits/compute_shuffled_index_runable): Added iteration…
c16ddc2 
… over the test input files.
@HristoStaykov
feat(zkllvm-circuits/compute_shuffled_index_runable): Utils directory…
f996c18 
… restructuring.

Move common utility functions that will be used
in all circuits to common place.
@HristoStaykov
feat(zkllvm-circuits/compute_shuffled_index_runable): Successful proc…
0313131 
…essing of test cases with picosha header only lib.

Code foramtting added.
@HristoStaykov
feat(zkllvm-circuits/compute_shuffled_index_circuit): Compiled comput…
a594956 
…e_shuffled_index.cpp as circuit.

By modifying picosha2.h to use only std::array instead of std::vector
we are now able to compile compute_shuffled_index's code as circuit.
@HristoStaykov
feat(zkllvm-circuits/weigh_justification_and_finalization): Added imp…
ef516ba 
…lementation.

Added Functionality for:
get_current_epoch, assert_epoch_is_not_genesis_epoch, verify_justification_bits,
get_previous_epoch, compute_start_slot_at_epoch_in_block_roots,
verify_epoch_start_slot_root_in_block_roots, verify_finalized_checkpoint

weigh_justification_and_finalization implementation complete + first test passing.
@HristoStaykov
feat(zkllvm-circuits/weigh_justification_and_finalization): Code rest…
9e8e85b 
…ructuring.

Circuit code extracted from test to circuit_imp dir.
std::cout-s cleanup.
Code foramtting applied.
Cleanup debug code.
@HristoStaykov
feat(zkllvm-circuits/weigh_justification_and_finalization_test): Adde…
36e520f 
…d json parsing.

Initial ssz conversion to JSON step added.
Initial process of JSON file.
@HristoStaykov
feat(zkllvm-circuits/compute_shuffled_index_circuit): Move content of…
e09ce55 
… implementation in imp folder.
@HristoStaykov
Review comments addressed.
94bdc53 
Usage of tests from vendor/consensus-spec-tests.
Code formatting applied.
Removed unnecessary copying of buffer.
Add ssz files not part of consensus-spec-tests.
Added scripts for code formatting, compilation + tests run
and running arbitrary processes in a container with all environment.
@HristoStaykov HristoStaykov force-pushed the hstaykov/add_test_for_weigh_justification_and_finalization branch from 63a74e0 to 94bdc53 Compare January 24, 2024 13:22
@HristoStaykov HristoStaykov merged commit 600c1f3 into main Jan 24, 2024
14 checks passed
@HristoStaykov HristoStaykov deleted the hstaykov/add_test_for_weigh_justification_and_finalization branch January 24, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Owliie Owliie Owliie approved these changes

@Xearty Xearty Xearty approved these changes

@zah zah Awaiting requested review from zah

@Dimo99 Dimo99 Awaiting requested review from Dimo99

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HristoStaykov @Owliie @Xearty