Contracts optimizations (#42)

* use max lookup bits for circuit degree * remove y coord check * tweak poseidon * half number of poseidon hash inputs * add step aggregation * remove committee poseidon from pi commit in contracts * tweak circuit accordingly * re-gen step verifier sol * update tests * post merge fixes * cargo fix * cargo fix + fmt * cargo fix + fmt * regenerate committee update contract * remove commented code Co-authored-by: Willem Olding <[email protected]> --------- Co-authored-by: Willem Olding <[email protected]>
ChainSafe · Dec 13, 2023 · 9cc6298 · 9cc6298
1 parent 5c2ac0d
commit 9cc6298
Show file tree

Hide file tree

Showing 22 changed files with 97 additions and 121 deletions.
diff --git a/contract-tests/tests/rotation_input_encoding.rs b/contract-tests/tests/rotation_input_encoding.rs
@@ -29,19 +29,12 @@ where
     [(); Spec::SYNC_COMMITTEE_SIZE]:,
 {
     fn from(args: CommitteeRotationArgs<Spec>) -> Self {
-        let poseidon_commitment_be = poseidon_committee_commitment_from_compressed(
+        let poseidon_commitment = poseidon_committee_commitment_from_compressed(
             &args.pubkeys_compressed.iter().cloned().collect_vec(),
-        )
-        .unwrap()
-        .into_iter()
-        .rev() // need to reverse to match the endianness of the solidity encoding
-        .collect_vec()
-        .try_into()
-        .unwrap();
-
-        // Endianess here is super confusing
-        // This should be solved by having `committee_poseidong` only be `uint256`
-        // See https://github.com/ChainSafe/Spectre/pull/42
+        );
+        let sync_committee_poseidon =
+            ethers::prelude::U256::from_little_endian(&poseidon_commitment.to_bytes());
+
 
         let mut pk_vector: Vector<Vector<u8, 48>, { Spec::SYNC_COMMITTEE_SIZE }> = args
             .pubkeys_compressed
@@ -61,7 +54,7 @@ where
 
         RotateInput {
             sync_committee_ssz,
-            sync_committee_poseidon: poseidon_commitment_be,
+            sync_committee_poseidon,
         }
     }
 }
@@ -100,7 +93,8 @@ mod tests {
         let (_, witness) = read_test_files_and_gen_witness(&path);
         let accumulator = [bn256::Fr::zero(); 12]; // this can be anything.. The test is just checking it gets correctly concatenated to the start of the encoded input
 
-        let instance = CommitteeUpdateCircuit::<Minimal, bn256::Fr>::instance(&witness, LIMB_BITS);
+        let instance =
+            CommitteeUpdateCircuit::<Minimal, bn256::Fr>::get_instances(&witness, LIMB_BITS);
         let finalized_block_root = witness
             .finalized_header
             .clone()

diff --git a/contract-tests/tests/spectre.rs b/contract-tests/tests/spectre.rs
@@ -22,7 +22,7 @@ const SLOTS_PER_SYNC_COMMITTEE_PERIOD: usize = EPOCHS_PER_SYNC_COMMITTEE_PERIOD
 #[tokio::test]
 async fn test_deploy_spectre() -> anyhow::Result<()> {
     let (_anvil_instance, ethclient) = make_client();
-    let _contract = deploy_spectre_mock_verifiers(ethclient, 0, [0; 32], 0).await?;
+    let _contract = deploy_spectre_mock_verifiers(ethclient, 0, U256::zero(), 0).await?;
     Ok(())
 }
 
@@ -79,7 +79,7 @@ async fn test_contract_initialization_and_first_step(
 async fn deploy_spectre_mock_verifiers<M: Middleware + 'static>(
     ethclient: Arc<M>,
     initial_sync_period: usize,
-    initial_sync_committee_poseidon: [u8; 32],
+    initial_sync_committee_poseidon: U256,
     slots_per_period: usize,
 ) -> anyhow::Result<Spectre<M>> {
     let step_verifier = StepMockVerifier::deploy(ethclient.clone(), ())?

diff --git a/contract-tests/tests/step_input_encoding.rs b/contract-tests/tests/step_input_encoding.rs
@@ -55,20 +55,25 @@ async fn test_step_instance_commitment_evm_equivalence(
     path: PathBuf,
 ) -> anyhow::Result<()> {
     let (witness, _) = read_test_files_and_gen_witness(&path);
-    let instance = StepCircuit::<Minimal, bn256::Fr>::instance_commitment(&witness, LIMB_BITS);
-    let poseidon_commitment_le =
-        poseidon_committee_commitment_from_uncompressed(&witness.pubkeys_uncompressed)?;
+    let instance = StepCircuit::<Minimal, bn256::Fr>::get_instances(&witness, LIMB_BITS);
+    let poseidon_commitment =
+        poseidon_committee_commitment_from_uncompressed(&witness.pubkeys_uncompressed);
 
     let (_anvil_instance, ethclient) = make_client();
     let contract = SyncStepExternal::deploy(ethclient, ())?.send().await?;
 
     let result = contract
-        .to_input_commitment(SyncStepInput::from(witness), poseidon_commitment_le)
+        .to_input_commitment(SyncStepInput::from(witness))
         .call()
         .await?;
-    let mut result_bytes = [0_u8; 32];
+    let mut result_bytes = [0u8; 32];
     result.to_little_endian(&mut result_bytes);
 
-    assert_eq!(bn256::Fr::from_bytes(&result_bytes).unwrap(), instance);
+    assert_eq!(
+        bn256::Fr::from_bytes(&result_bytes).unwrap(),
+        instance[0][0]
+    );
+    assert_eq!(poseidon_commitment, instance[0][1]);
+
     Ok(())
 }
diff --git a/contracts/rust-abi/lib.rs b/contracts/rust-abi/lib.rs
@@ -1,6 +1,6 @@
 #![allow(incomplete_features)]
 #![feature(generic_const_exprs)]
-use ethers::contract::abigen;
+use ethers::{contract::abigen, types::U256};
 use itertools::Itertools;
 use lightclient_circuits::{
     poseidon::poseidon_committee_commitment_from_compressed,
@@ -61,10 +61,10 @@ where
     [(); Spec::SYNC_COMMITTEE_SIZE]:,
 {
     fn from(args: CommitteeRotationArgs<Spec>) -> Self {
-        let poseidon_commitment_le = poseidon_committee_commitment_from_compressed(
+        let sync_committee_poseidon = poseidon_committee_commitment_from_compressed(
             &args.pubkeys_compressed.iter().cloned().collect_vec(),
-        )
-        .unwrap();
+        );
+        let sync_committee_poseidon = U256::from_little_endian(&sync_committee_poseidon.to_bytes());
 
         let mut pk_vector: Vector<Vector<u8, 48>, { Spec::SYNC_COMMITTEE_SIZE }> = args
             .pubkeys_compressed
@@ -84,17 +84,7 @@ where
 
         RotateInput {
             sync_committee_ssz,
-            sync_committee_poseidon: poseidon_commitment_le,
+            sync_committee_poseidon,
         }
     }
 }
-
-// pub fn poseidon_committee_commitment_from_compressed(pubkeys_compressed: &[Vec<u8>]) -> [u8; 32] {
-//     let pubkeys_x = pubkeys_compressed.iter().cloned().map(|mut bytes| {
-//         bytes[0] &= 0b00011111;
-//         bls12_381::Fq::from_bytes_be(&bytes.try_into().unwrap())
-//             .expect("bad bls12_381::Fq encoding")
-//     });
-//     let poseidon_commitment = fq_array_poseidon_native::<Fr>(pubkeys_x, LIMB_BITS).unwrap();
-//     poseidon_commitment.to_bytes()
-// }
diff --git a/contracts/script/DeploySpectre.s.sol b/contracts/script/DeploySpectre.s.sol
@@ -4,15 +4,15 @@ pragma solidity ^0.8.0;
 import "forge-std/Script.sol";
 
 import {Spectre} from "../src/Spectre.sol";
-import {Verifier as SyncStepVerifier} from "../snark-verifiers/sync_step.sol"; 
 import {Verifier as CommitteeUpdateVerifier} from "../snark-verifiers/committee_update_verifier.sol"; 
+import {Verifier as SyncStepVerifier} from "../snark-verifiers/sync_step.sol"; 
 
 contract DeploySpectre is Script {
 
     function run() external {
         uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
         uint256 initialSyncPeriod = vm.envUint("INITIAL_SYNC_PERIOD");
-        bytes32 initialCommitteePoseidon = vm.envBytes32("INITIAL_COMMITTEE_POSEIDON");
+        uint256 initialCommitteePoseidon = vm.envUint("INITIAL_COMMITTEE_POSEIDON");
         uint256 slotsPerPeriod = vm.envUint("SLOTS_PER_PERIOD");
 
         vm.startBroadcast(deployerPrivateKey);

diff --git a/contracts/script/DeploySpectreTestnet.s.sol b/contracts/script/DeploySpectreTestnet.s.sol
@@ -4,15 +4,15 @@ pragma solidity ^0.8.0;
 import "forge-std/Script.sol";
 
 import {Spectre} from "../src/Spectre.sol";
-import {Verifier as SyncStepVerifier} from "../snark-verifiers/sync_step.sol"; 
 import {Verifier as CommitteeUpdateVerifier} from "../snark-verifiers/committee_update_verifier.sol"; 
+import {Verifier as SyncStepVerifier} from "../snark-verifiers/sync_step.sol";
 
 contract DeploySpectre is Script {
 
     function run() external {
         uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY");
         uint256 initialSyncPeriod = vm.envUint("INITIAL_SYNC_PERIOD");
-        bytes32 initialCommitteePoseidon = vm.envBytes32("INITIAL_COMMITTEE_POSEIDON");
+        uint256 initialCommitteePoseidon = vm.envUint("INITIAL_COMMITTEE_POSEIDON");
         uint256 slotsPerPeriod = vm.envUint("SLOTS_PER_PERIOD");
 
         vm.startBroadcast(deployerPrivateKey);

diff --git a/contracts/snark-verifiers/committee_update_verifier.sol b/contracts/snark-verifiers/committee_update_verifier.sol
diff --git a/contracts/snark-verifiers/sync_step.sol b/contracts/snark-verifiers/sync_step.sol
diff --git a/contracts/src/RotateLib.sol b/contracts/src/RotateLib.sol
@@ -7,7 +7,7 @@ library RotateLib {
 
     struct RotateInput {
         bytes32 syncCommitteeSSZ;
-        bytes32 syncCommitteePoseidon;
+        uint256 syncCommitteePoseidon;
     }
 
     /**
@@ -23,7 +23,7 @@ library RotateLib {
             inputs[i] = accumulator[i];
         }
 
-        inputs[accumulator.length] = uint256(args.syncCommitteePoseidon);
+        inputs[accumulator.length] = args.syncCommitteePoseidon;
 
         uint256 syncCommitteeSSZNumeric = uint256(args.syncCommitteeSSZ);
         for (uint256 i = 0; i < 32; i++) {

diff --git a/contracts/src/Spectre.sol b/contracts/src/Spectre.sol
@@ -14,7 +14,7 @@ contract Spectre {
     uint256 internal immutable SLOTS_PER_PERIOD;
 
     /// Maps from a sync period to the poseidon commitment for the sync committee.
-    mapping(uint256 => bytes32) public syncCommitteePoseidons;
+    mapping(uint256 => uint256) public syncCommitteePoseidons;
 
     /// Maps from a slot to a beacon block header root.
     mapping(uint256 => bytes32) public blockHeaderRoots;
@@ -32,7 +32,7 @@ contract Spectre {
         address _stepVerifierAddress,
         address _committeeUpdateVerifierAddress,
         uint256 _initialSyncPeriod,
-        bytes32 _initialSyncCommitteePoseidon,
+        uint256 _initialSyncCommitteePoseidon,
         uint256 _slotsPerPeriod
     ) {
         stepVerifier = SyncStepVerifier(_stepVerifierAddress);
@@ -50,9 +50,9 @@ contract Spectre {
         if (syncCommitteePoseidons[currentPeriod] == 0) {
             revert("Sync committee not yet set for this period");
         }
-        uint256 instanceCommitment = input.toInputCommitment(syncCommitteePoseidons[currentPeriod]);
+        uint256 instanceCommitment = input.toInputCommitment();
 
-        bool success = stepVerifier.verify([instanceCommitment], proof);
+        bool success = stepVerifier.verify([instanceCommitment, syncCommitteePoseidons[currentPeriod]], proof);
         if (!success) {
             revert("Proof verification failed");
         }
@@ -74,8 +74,8 @@ contract Spectre {
         // that checks the new committee is in the beacon state 'next_sync_committee' field. It also allows trusting the finalizedSlot which is
         // used to calculate the sync period that the new committee belongs to.
         uint256 attestingPeriod = getSyncCommitteePeriod(stepInput.attestedSlot);
-        uint256 instanceCommitment = stepInput.toInputCommitment(syncCommitteePoseidons[attestingPeriod]);
-        bool stepSuccess = stepVerifier.verify([instanceCommitment], stepProof);
+        uint256 instanceCommitment = stepInput.toInputCommitment();
+        bool stepSuccess = stepVerifier.verify([instanceCommitment, syncCommitteePoseidons[attestingPeriod]], stepProof);
         if (!stepSuccess) {
             revert("Step proof verification failed");
         }

diff --git a/contracts/src/SyncStepLib.sol b/contracts/src/SyncStepLib.sol
@@ -16,17 +16,15 @@ library SyncStepLib {
     * @notice Compute the public input commitment for the sync step given this input.
     *         This must always match the prodecure used in lightclient-circuits/src/sync_step_circuit.rs - SyncStepCircuit::instance()
     * @param args The arguments for the sync step
-    * @param keysPoseidonCommitment The commitment to the keys used in the sync step
     * @return The public input commitment that can be sent to the verifier contract.
      */
-    function toInputCommitment(SyncStepInput memory args, bytes32 keysPoseidonCommitment) internal pure returns (uint256) {
+    function toInputCommitment(SyncStepInput memory args) internal pure returns (uint256) {
         bytes32 h = sha256(abi.encodePacked(
             EndianConversions.toLittleEndian64(args.attestedSlot),
             EndianConversions.toLittleEndian64(args.finalizedSlot),
             EndianConversions.toLittleEndian64(args.participation),
             args.finalizedHeaderRoot,
-            args.executionPayloadRoot,
-            keysPoseidonCommitment
+            args.executionPayloadRoot
         ));
         uint256 commitment = uint256(EndianConversions.toLittleEndian(uint256(h)));
         return commitment & ((uint256(1) << 253) - 1); // truncated to 253 bits

diff --git a/contracts/src/interfaces/SyncStepVerifier.sol b/contracts/src/interfaces/SyncStepVerifier.sol
@@ -2,5 +2,5 @@
 pragma solidity ^0.8.0;
 
 interface SyncStepVerifier {
-    function verify(uint256[1] calldata input, bytes calldata proof) external returns (bool);
+    function verify(uint256[2] calldata input, bytes calldata proof) external returns (bool);
 }
diff --git a/contracts/src/mocks/SyncStepMockVerifier.sol b/contracts/src/mocks/SyncStepMockVerifier.sol
@@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
 import { SyncStepVerifier } from "../interfaces/SyncStepVerifier.sol";
 
 contract SyncStepMockVerifier is SyncStepVerifier {
-    function verify(uint256[1] calldata _input, bytes calldata _proof) external override returns (bool) {
+    function verify(uint256[2] calldata _input, bytes calldata _proof) external override returns (bool) {
         return true;
     }
 }
diff --git a/contracts/test/SyncStepExternal.sol b/contracts/test/SyncStepExternal.sol
@@ -1,17 +1,19 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import { SyncStepLib } from "../src/SyncStepLib.sol";
+import {SyncStepLib} from "../src/SyncStepLib.sol";
 
 /**
-* @title SyncStepExternal
-* @dev This contract exists solely for the purpose of exposing the SyncStepLib functions
-*      so they can be used in the Rust test suite. It should not be part of a production deployment
-*/
+ * @title SyncStepExternal
+ * @dev This contract exists solely for the purpose of exposing the SyncStepLib functions
+ *      so they can be used in the Rust test suite. It should not be part of a production deployment
+ */
 contract SyncStepExternal {
     using SyncStepLib for SyncStepLib.SyncStepInput;
 
-    function toInputCommitment(SyncStepLib.SyncStepInput calldata args, bytes32 keysPoseidonCommitment) public pure returns (uint256) {
-        return args.toInputCommitment(keysPoseidonCommitment);
+    function toInputCommitment(
+        SyncStepLib.SyncStepInput calldata args
+    ) public pure returns (uint256) {
+        return args.toInputCommitment();
     }
 }
diff --git a/lightclient-circuits/config/sync_step_testnet.json b/lightclient-circuits/config/sync_step_testnet.json
@@ -2,24 +2,21 @@
   "params": {
     "k": 22,
     "num_advice_per_phase": [
-      6
+      3
     ],
     "num_fixed": 1,
     "num_lookup_advice_per_phase": [
       1,
       0,
       0
     ],
-    "lookup_bits": 8,
+    "lookup_bits": 21,
     "num_instance_columns": 1
   },
   "break_points": [
     [
-      4194292,
       4194294,
-      4194292,
-      4194293,
-      4194293
+      4194294
     ]
   ]
 }
diff --git a/lightclient-circuits/src/committee_update_circuit.rs b/lightclient-circuits/src/committee_update_circuit.rs
@@ -156,7 +156,7 @@ impl<S: Spec, F: Field> CommitteeUpdateCircuit<S, F> {
         ssz_merkleize_chunks(builder, hasher, pubkeys_hashes)
     }
 
-    pub fn instance(
+    pub fn get_instances(
         args: &witness::CommitteeRotationArgs<S>,
         limb_bits: usize,
     ) -> Vec<Vec<bn256::Fr>>
@@ -169,8 +169,7 @@ impl<S: Spec, F: Field> CommitteeUpdateCircuit<S, F> {
                 .expect("bad bls12_381::Fq encoding")
         });
 
-        let poseidon_commitment =
-            fq_array_poseidon_native::<bn256::Fr>(pubkeys_x, limb_bits).unwrap();
+        let poseidon_commitment = fq_array_poseidon_native::<bn256::Fr>(pubkeys_x, limb_bits);
 
         let mut pk_vector: Vector<Vector<u8, 48>, { S::SYNC_COMMITTEE_SIZE }> = args
             .pubkeys_compressed
@@ -315,7 +314,7 @@ mod tests {
         )
         .unwrap();
 
-        let instance = CommitteeUpdateCircuit::<Testnet, Fr>::instance(&witness, LIMB_BITS);
+        let instance = CommitteeUpdateCircuit::<Testnet, Fr>::get_instances(&witness, LIMB_BITS);
 
         let timer = start_timer!(|| "committee_update mock prover");
         let prover = MockProver::<Fr>::run(K, &circuit, instance).unwrap();