Resolved review comments.

vendor/zkllvm-metacraft-circuits/src/circuit_utils/base_types.h

@@ -72,9 +72,6 @@ struct JustificationBitsVariable {
     }
 };
 
-constexpr unsigned int MAX_MERKLE_DEPTH = 512;
-
 Epoch get_current_epoch(Slot slot) {
-    auto slots_per_epoch = SLOTS_PER_EPOCH;
-    return slot / slots_per_epoch;
+    return slot / SLOTS_PER_EPOCH;
 }

vendor/zkllvm-metacraft-circuits/src/circuit_utils/circuit_byte_utils.h

@@ -14,7 +14,6 @@ namespace circuit_byte_utils {
     using sha256_t = typename hashes::sha2<256>::block_type;
 
     bool sha256_equals(sha256_t hash1, sha256_t hash2) {
-
         bool result = true;
         for (auto i = 0; i < countof(hash1); ++i) {
             result = result && (hash1[i] == hash2[i]);
@@ -28,7 +27,12 @@ namespace circuit_byte_utils {
         static_assert(std::is_integral<typename std::remove_reference<T>::type>::value, "T must be integral");
         assert_true(n < sizeof(T));
 
-        return T(val >> (n * 8));
+        return Byte(val >> (n * 8));
+    }
+
+    Byte get_nth_byte(sha256_t val, size_t int_count, size_t byte_count) {
+        // TODO: implement when we start using crypto3's sha256
+        return Byte {};
     }
 
     bool get_nth_bit(uint64_t value, short i) {
@@ -37,35 +41,35 @@ namespace circuit_byte_utils {
 
     Byte set_nth_bit(Byte& value, short i) {
         assert_true(i < 8);
-        value = value | (Byte(1) << i);
-        return value;
+        return value | (Byte(1) << i);
     }
 
-    template<typename T>
-    void sha256_to_bytes_array(sha256_t sha, T& out) {
+    Bytes32 sha256_to_bytes_array(sha256_t sha) {
+        Bytes32 out;
         assert_true(out.size() >= sizeof(sha));
-        for (int int_count = 0; int_count < sizeof(sha) / sizeof(sha[0]); int_count++) {
+        for (int int_count = 0; int_count < countof(sha); int_count++) {
 
             for (int byte_count = 0; byte_count < sizeof(sha[0]); byte_count++) {
-                out[int_count * sizeof(sha[0]) + byte_count] = get_nth_byte(sha[int_count], byte_count);
+                out[int_count * sizeof(sha[0]) + byte_count] = get_nth_byte(sha, int_count, byte_count);
             }
         }
+        return out;
     }
 
     template<std::size_t N, typename T, std::size_t InputSize>
-    std::array<T, N> take_n_elements(const std::array<T, InputSize>& val) {
+    std::array<T, N> take(const std::array<T, InputSize>& val) {
         static_assert(N <= InputSize);
         std::array<T, N> ret {};
         std::copy(val.begin(), val.begin() + N, ret.begin());
 
         return ret;
     }
 
-    template<typename T, bool little_endian = true>
+    template<typename T, bool LittleEndian = true>
     std::array<Byte, sizeof(T)> int_to_bytes(const T& paramInt) {
         static_assert(std::is_integral_v<typename std::remove_reference_t<T>>, "T must be integral");
         std::array<Byte, sizeof(T)> bytes {};
-        if constexpr (little_endian) {
+        if constexpr (LittleEndian) {
             for (int i = 0; i < sizeof(T); ++i) {
                 bytes[i] = (paramInt >> (i * 8));
             }
@@ -77,17 +81,17 @@ namespace circuit_byte_utils {
         return bytes;
     }
 
-    template<typename T>
-    T bytes_to_int(const std::array<Byte, sizeof(T)>& paramVec, bool little_endian = true) {
+    template<typename T, bool LittleEndian = true>
+    T bytes_to_int(const std::array<Byte, sizeof(T)>& bytes) {
         static_assert(std::is_integral<typename std::remove_reference<T>::type>::value, "T must be integral");
         T result = 0;
-        if (little_endian) {
+        if constexpr (LittleEndian) {
             for (int i = sizeof(T) - 1; i >= 0; i--) {
-                result = (result << 8) + paramVec[i];
+                result = (result << 8) + bytes[i];
             }
         } else {
             for (unsigned i = 0; i < sizeof(T); ++i) {
-                result = (result << 8) + paramVec[i];
+                result = (result << 8) + bytes[i];
             }
         }
         return result;
@@ -124,21 +128,21 @@ namespace circuit_byte_utils {
     template<size_t N>
     struct CanConvertToBytes<std::array<Byte, N>> : std::true_type { };
 
-#define IS_HASHABLE(T)                                                                       \
+#define ASSERT_IS_HASHABLE(T)                                                                \
     using PureType = std::remove_cv_t<std::remove_reference_t<T>>;                           \
     static_assert(!std::is_pointer_v<PureType>, "The argument must not be a pointer.");      \
     static_assert(CanConvertToBytes<PureType>::value, "The argument cannot be serialized."); \
     static_assert(!HasPadding<PureType>::value, "The argument must not contain padding.");
 
     template<typename T, size_t N>
     void to_bytes(const T& val, std::array<Byte, N>& buffer, size_t offset) {
-        IS_HASHABLE(T);
+        ASSERT_IS_HASHABLE(T);
         memcpy(&buffer[offset], (char*)&val, sizeof(T));
     }
 
     template<typename T, size_t N>
     void to_bytes(const T& val, std::array<Byte, N>& buffer, size_t offset, size_t total_bytes) {
-        IS_HASHABLE(T);
+        ASSERT_IS_HASHABLE(T);
         assert_true(N >= offset + total_bytes);
         memcpy(&buffer[offset], (char*)&val, total_bytes);
     }
@@ -163,7 +167,7 @@ namespace circuit_byte_utils {
 
         // TODO: use crypto3 sha after stack smash bug is fixed.
         //!!! auto digest = hash<hashes::sha2<256>>();
-        //!!! sha256_to_bytes_array();
+        //!!! sha256_to_bytes();
 
         Bytes32 hashed;
         picosha2::hash256(buffer.begin(), buffer.end(), hashed.begin(), hashed.end());
@@ -177,7 +181,7 @@ namespace circuit_byte_utils {
 
         // TODO: use crypto3 sha after stack smash bug is fixed.
         //!!! auto digest = hash<hashes::sha2<256>>();
-        //!!! sha256_to_bytes_array();
+        //!!! sha256_to_bytes();
 
         Bytes32 hashed;
         picosha2::hash256(buffer.begin(), buffer.begin() + NBytesToHash, hashed.begin(), hashed.end());

vendor/zkllvm-metacraft-circuits/src/circuit_utils/ssz_utils.h

@@ -6,7 +6,7 @@ using namespace circuit_byte_utils;
 
 namespace ssz_utils {
 
-    template<uint32_t MERKLE_DEPTH>
+    template<size_t MERKLE_DEPTH>
     Bytes32
         ssz_restore_merkle_root(const Bytes32& leaf, const std::array<Bytes32, MERKLE_DEPTH>& branch, uint64_t gindex) {
         auto hash = leaf;
@@ -36,7 +36,7 @@ namespace ssz_utils {
                           const Bytes32& leaf,
                           const std::array<Bytes32, MERKLE_DEPTH>& branch,
                           const uint64_t gindex) {
-        auto expected_root = ssz_restore_merkle_root<MERKLE_DEPTH>(leaf, branch, gindex);
+        auto expected_root = ssz_restore_merkle_root(leaf, branch, gindex);
         assert_true(root == expected_root);
     }
 
@@ -56,7 +56,7 @@ namespace ssz_utils {
         Bytes32 ret_val {};
         for (auto i = 0; i < 4; i++) {
             if (checkpoint.bits[i]) {
-                set_nth_bit(ret_val[0], i);
+                ret_val[0] = set_nth_bit(ret_val[0], i);
             }
         }
 

vendor/zkllvm-metacraft-circuits/src/circuits_impl/compute_shuffled_index_impl.h

@@ -11,26 +11,22 @@
 
 using namespace circuit_byte_utils;
 
-uint64_t compute_shuffled_index_impl(uint64_t index, uint64_t index_count, Bytes32 seed, int SHUFFLE_ROUND_COUNT = 90) {
+uint64_t compute_shuffled_index_impl(uint64_t index, uint64_t index_count, Bytes32 seed, int shuffle_round_count = 90) {
     assert_true(index < index_count);
 
     // Swap or not (https://link.springer.com/content/pdf/10.1007%2F978-3-642-32009-5_1.pdf)
     // See the 'generalized domain' algorithm on page 3
-    for (Byte current_round = 0; current_round < SHUFFLE_ROUND_COUNT; current_round++) {
+    for (Byte current_round = 0; current_round < shuffle_round_count; current_round++) {
 
-        Bytes32 source_round_hash = calc_hash(seed, current_round);
-        auto first8bytes = take_n_elements<8>(source_round_hash);
-
-        auto first8bytes_int = bytes_to_int<uint64_t>(first8bytes);
-        auto pivot = first8bytes_int % index_count;
+        auto pivot = bytes_to_int<uint64_t>(take<8>(calc_hash(seed, current_round))) % index_count;
         uint64_t flip = (pivot + index_count - index) % index_count;
         auto position = std::max(index, flip);
 
-        Bytes32 seed_hash = calc_hash(seed, current_round, int_to_bytes(uint32_t(position >> 8)));
-        auto byte_value = seed_hash[(position % 256) >> 3];
-        auto bit = (byte_value >> (position % 8)) % 2;
+        Bytes32 seed_hash = calc_hash(seed, current_round, int_to_bytes(uint32_t(position / 256)));
+        auto byte = seed_hash[(position % 256) / 8];
+        auto bit = (byte >> (position % 8)) % 2;
 
-        if (bit != 0) {
+        if (bit == 1) {
             index = flip;
         }
     }

vendor/zkllvm-metacraft-circuits/src/circuits_impl/weigh_justification_and_finalization.h

@@ -12,10 +12,6 @@ using namespace ssz_utils;
 
 namespace weigh_justification_and_finalization_ {
 
-    void assert_epoch_is_not_genesis_epoch(Epoch epoch) {
-        assert_true(epoch >= 1);
-    }
-
     Epoch get_previous_epoch(Epoch current_epoch) {
         return current_epoch - 1;
     }
@@ -29,27 +25,16 @@ namespace weigh_justification_and_finalization_ {
                                                      Root block_root,
                                                      MerkleProof<18>
                                                          proof) {
-        auto start_block_roots_gindex = DEPTH18_START_BLOCK_ROOTS_GINDEX;
         auto index_in_block_roots = compute_start_slot_at_epoch_in_block_roots(epoch);
-        auto gindex = start_block_roots_gindex + index_in_block_roots;
+        auto gindex = DEPTH18_START_BLOCK_ROOTS_GINDEX + index_in_block_roots;
         ssz_verify_proof(beacon_state_root, block_root, proof, gindex);
     }
 
-    void verify_finalized_checkpoint(Root beacon_state_root,
-                                     CheckpointVariable finalized_checkpoint,
-                                     BeaconStateLeafProof proof) {
-        auto finalized_checkpoint_leaf = hash_tree_root(finalized_checkpoint);
-        auto gindex = BEACON_STATE_FINALIZED_CHECKPOINT_GINDEX;
-        ssz_verify_proof(beacon_state_root, finalized_checkpoint_leaf, proof, gindex);
-    }
-
-    void assert_slot_is_not_first_in_epoch(Slot slot) {
-        auto slot_in_epoch = slot % SLOTS_PER_EPOCH;
-        assert_true(slot_in_epoch != 0);
-    }
-
-    bool is_supermajority_link(Gwei target_balance, Gwei total_active_balance) {
-        return target_balance * 3 >= total_active_balance * 2;
+    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);
     }
 
     void process_justifications(const Gwei& total_active_balance,
@@ -62,27 +47,17 @@ namespace weigh_justification_and_finalization_ {
                                 // Outputs:
                                 CheckpointVariable& out_checkpoint,
                                 JustificationBitsVariable& out_justification_bits) {
-        const auto previous_epoch_supermajority_link_pred =
-            is_supermajority_link(previous_epoch_target_balance, total_active_balance);
-        const auto current_epoch_supermajority_link_pred =
-            is_supermajority_link(current_epoch_target_balance, total_active_balance);
-
-        auto new_current_justified_checkpoint =
-            previous_epoch_supermajority_link_pred ? previous_epoch_checkpoint : current_justified_checkpoint;
-
-        new_current_justified_checkpoint =
-            current_epoch_supermajority_link_pred ? current_epoch_checkpoint : new_current_justified_checkpoint;
-
-        const auto new_second_justification_bit =
-            previous_epoch_supermajority_link_pred ? true : justification_bits.bits[0];
-
-        auto new_justification_bits = justification_bits;
-        new_justification_bits.shift_right(1);
-        new_justification_bits.bits[1] = new_second_justification_bit;
-        new_justification_bits.bits[0] = current_epoch_supermajority_link_pred;
-
-        out_checkpoint = new_current_justified_checkpoint;
-        out_justification_bits = new_justification_bits;
+        out_justification_bits = justification_bits;
+        out_justification_bits.shift_right(1);
+        out_justification_bits.bits[0] = false;
+        if (previous_epoch_target_balance * 3 >= total_active_balance * 2) {
+            out_checkpoint = previous_epoch_checkpoint;
+            out_justification_bits.bits[1] = true;
+        }
+        if (current_epoch_target_balance * 3 >= total_active_balance * 2) {
+            out_checkpoint = current_epoch_checkpoint;
+            out_justification_bits.bits[0] = true;
+        }
     }
 
     CheckpointVariable process_finalizations(const JustificationBitsVariable& justification_bits,
@@ -91,17 +66,19 @@ namespace weigh_justification_and_finalization_ {
                                              const Epoch& current_epoch,
                                              const CheckpointVariable& finalized_checkpoint) {
 
-        auto new_finalized_checkpoint =
-            ((justification_bits.test_range(1, 4) && (previous_justified_checkpoint.epoch + 3 == current_epoch)) ||
-             (justification_bits.test_range(1, 3) && (previous_justified_checkpoint.epoch + 2 == current_epoch))) ?
-                previous_justified_checkpoint :
-                finalized_checkpoint;
-
-        new_finalized_checkpoint =
-            ((justification_bits.test_range(0, 3) && (current_justified_checkpoint.epoch + 2 == current_epoch)) ||
-             (justification_bits.test_range(0, 2) && (current_justified_checkpoint.epoch + 1 == current_epoch))) ?
-                current_justified_checkpoint :
-                new_finalized_checkpoint;
+        CheckpointVariable new_finalized_checkpoint;
+        // The 2nd/3rd/4th most recent epochs are justified, the 2nd using the 4th as source
+        if (justification_bits.test_range(1, 4) && (previous_justified_checkpoint.epoch + 3 == current_epoch))
+            new_finalized_checkpoint = previous_justified_checkpoint;
+        // The 2nd/3rd most recent epochs are justified, the 2nd using the 3rd as source
+        if (justification_bits.test_range(1, 3) && (previous_justified_checkpoint.epoch + 2 == current_epoch))
+            new_finalized_checkpoint = previous_justified_checkpoint;
+        // The 1st/2nd/3rd most recent epochs are justified, the 1st using the 3rd as source
+        if (justification_bits.test_range(0, 3) && (current_justified_checkpoint.epoch + 2 == current_epoch))
+            new_finalized_checkpoint = current_justified_checkpoint;
+        // The 1st/2nd most recent epochs are justified, the 1st using the 2nd as source
+        if (justification_bits.test_range(0, 2) && (current_justified_checkpoint.epoch + 1 == current_epoch))
+            new_finalized_checkpoint = current_justified_checkpoint;
 
         return new_finalized_checkpoint;
     }
@@ -130,7 +107,7 @@ namespace weigh_justification_and_finalization_ {
         CheckpointVariable& out_new_current_justified_checkpoint,
         CheckpointVariable& out_new_finalized_checkpoint,
         JustificationBitsVariable& out_new_justification_bits) {
-        assert_slot_is_not_first_in_epoch(slot);
+        assert_true((slot % SLOTS_PER_EPOCH) != 0);
         ssz_verify_proof(beacon_state_root, hash_tree_root(slot), slot_proof, BEACON_STATE_SLOT_GINDEX);
 
         ssz_verify_proof(beacon_state_root,
@@ -149,7 +126,7 @@ namespace weigh_justification_and_finalization_ {
                          BEACON_STATE_JUSTIFICATION_BITS_GINDEX);
 
         auto current_epoch = get_current_epoch(slot);
-        assert_epoch_is_not_genesis_epoch(current_epoch);
+        assert_true(current_epoch >= 1);
 
         auto previous_epoch = get_previous_epoch(current_epoch);
 

vendor/zkllvm-metacraft-circuits/src/tests/compute_shuffled_index_test/compute_shuffled_index_test.cpp

@@ -83,7 +83,7 @@ int main(int argc, char* argv[]) {
     path my_path("/consensus-spec-tests");
     try {
         find_matching_files(my_path, std::vector<std::string> {"minimal", "shuffling", "mapping.yaml"}, result);
-    } catch (const non_existing_path& e) {
+    } catch (const NonExistentPath& e) {
         std::cerr << "ERROR: non existing path " << e.what() << "\n";
         return 1;
     }
@@ -96,7 +96,7 @@ int main(int argc, char* argv[]) {
 
     try {
         find_matching_files(my_path, std::vector<std::string> {"mainnet", "shuffling", "mapping.yaml"}, result);
-    } catch (const non_existing_path& e) {
+    } catch (const NonExistentPath& e) {
         std::cerr << "ERROR: non existing path " << e.what() << "\n";
         return 1;
     }

vendor/zkllvm-metacraft-circuits/src/tests/weigh_justification_and_finalization_test/weigh_justification_and_finalization_test.cpp

@@ -210,7 +210,7 @@ void test_circuit_ssz_json() {
     path my_path("/consensus-spec-tests");
     try {
         find_matching_files(my_path, std::vector<std::string> {"ssz_snappy.json"}, result);
-    } catch (const non_existing_path& e) {
+    } catch (const NonExistentPath& e) {
         std::cerr << "ERROR: non existing path " << e.what() << "\n";
         exit(1);
     }

vendor/zkllvm-metacraft-circuits/src/utils/byte_utils.h

@@ -73,8 +73,7 @@ namespace byte_utils {
 
         std::array<Byte, SIZE> bytes;
         for (size_t i = 0; i < bytes.size(); ++i) {
-            const size_t j = i * 2;
-            auto hexByte = hex.substr(j, 2);
+            auto hexByte = hex.substr(i * 2, 2);
             auto byte = hexToByte(hexByte.data());
             bytes[i] = byte;
         }

vendor/zkllvm-metacraft-circuits/src/utils/file_utils.h

@@ -2,9 +2,9 @@
 using namespace boost::filesystem;
 
 namespace file_utils {
-    struct non_existing_path : public std::runtime_error {
+    struct NonExistentPath : public std::runtime_error {
         template<typename T>
-        non_existing_path(T&& arg) : runtime_error(std::forward<T>(arg)) {
+        NonExistentPath(T&& arg) : runtime_error(std::forward<T>(arg)) {
         }
     };
 
@@ -19,7 +19,7 @@ namespace file_utils {
         };
 
         if (!exists(dir_path))
-            throw non_existing_path(dir_path.string());
+            throw NonExistentPath(dir_path.string());
 
         directory_iterator end_itr;    // default construction yields past-the-end
         for (directory_iterator itr(dir_path); itr != end_itr; ++itr) {