Document and update halo2wrong dep

Cargo.toml

@@ -14,16 +14,16 @@ members = [
 ]
 
 [patch.crates-io]
-# halo2_proofs = { git = "https://github.com/appliedzkp/halo2.git", tag = "v2022_03_06" }
+halo2_proofs = { git = "https://github.com/appliedzkp/halo2.git", tag = "v2022_03_06" }
 # halo2_proofs = { git = "https://github.com/appliedzkp/halo2.git", rev = "9d3ab0cf7be57a828df6cedb8f58476b483fdde1" }
 # This fork makes bitvec 0.20.x work with funty 1.1 and funty 1.2.  Without
 # this fork, bitvec 0.20.x is incompatible with funty 1.2, which we depend on,
 # and leads to a compilation error.  This can be removed once the upstream PR
 # is resolved: https://github.com/bitvecto-rs/bitvec/pull/141
 bitvec = { git = "https://github.com/ed255/bitvec.git", rev = "5cfc5fa8496c66872d21905e677120fc3e79693c" }
 
-[patch."https://github.com/appliedzkp/halo2.git"]
-halo2_proofs = { path = "../halo2/halo2_proofs" }
+# [patch."https://github.com/appliedzkp/halo2.git"]
+# halo2_proofs = { path = "../halo2/halo2_proofs" }
 
 # Definition of benchmarks profile to use.
 [profile.bench]

zkevm-circuits/Cargo.toml

@@ -8,7 +8,7 @@ edition = "2018"
 
 [dependencies]
 ff = "0.11"
-halo2_proofs = { git = "https://github.com/appliedzkp/halo2.git", rev = "9d3ab0cf7be57a828df6cedb8f58476b483fdde1" }
+halo2_proofs = { git = "https://github.com/appliedzkp/halo2.git", tag = "v2022_03_06" }
 pairing = { git = 'https://github.com/appliedzkp/pairing', package = "pairing_bn256" }
 bigint = "4"
 num = "0.4"
@@ -29,6 +29,7 @@ keccak256 = { path = "../keccak256"}
 log = "0.4.14"
 env_logger = "0.9"
 # ecdsa = { git = "https://github.com/appliedzkp/halo2wrong.git", rev = "00b90e8c3198de0a73cdb6308f7dbea2bd6f24cc", features = ["kzg"] }
+# TODO: Replace by github path once https://github.com/appliedzkp/halo2wrong/pull/25 is merged
 ecdsa = { path = "../../halo2wrong/ecdsa", features = ["kzg"] }
 secp256k1 = { path = "../../halo2wrong/ecc/secp256k1", features = ["kzg"] }
 ecc = { path = "../../halo2wrong/ecc/circuit", features = ["kzg"] }
@@ -42,7 +43,6 @@ rlp = "0.5"
 num-bigint = { version = "0.4" }
 log = "0.4"
 subtle = "2.4"
-plotters = { version = "0.3.0", optional = true }
 
 [dev-dependencies]
 criterion = "0.3"
@@ -62,4 +62,3 @@ harness = false
 [features]
 default = []
 test = []
-dev-graph = ["halo2_proofs/dev-graph", "plotters"]

zkevm-circuits/src/tx_circuit.rs

@@ -1,5 +1,4 @@
-// TODO Remove this
-#![allow(missing_docs)]
+//! The transaction circuit implementation.
 
 mod sign_verify;
 
@@ -8,7 +7,6 @@ use eth_types::{Address, Bytes, Field, ToBigEndian, ToLittleEndian, ToScalar, Wo
 use ff::PrimeField;
 use group::GroupEncoding;
 use halo2_proofs::{
-    arithmetic::CurveAffine,
     circuit::{AssignedCell, Layouter, Region, SimpleFloorPlanner},
     plonk::{Advice, Circuit, Column, ConstraintSystem, Error},
     poly::Rotation,
@@ -38,6 +36,7 @@ lazy_static! {
     ]);
 }
 
+/// Transaction to be verified by the TxCircuit
 #[derive(Clone, Default, Debug)]
 pub struct Transaction {
     /// Sender address
@@ -63,8 +62,11 @@ pub struct Transaction {
     /// Transaction nonce
     pub nonce: U256,
 
+    /// "v" value of the transaction signature
     pub v: u64,
+    /// "r" value of the transaction signature
     pub r: U256,
+    /// "s" value of the transaction signature
     pub s: U256,
 }
 
@@ -93,7 +95,10 @@ fn recover_pk(
         error!("Message hash parsing from slice failed: {:?}", e);
         Error::Synthesis
     })?;
-    let recovery_id = libsecp256k1::RecoveryId::parse(v).expect("FIXME");
+    let recovery_id = libsecp256k1::RecoveryId::parse(v).map_err(|e| {
+        error!("secp256k1::RecoveriId::parse error: {:?}", e);
+        Error::Synthesis
+    })?;
     let pk = libsecp256k1::recover(&msg_hash, &signature, &recovery_id).map_err(|e| {
         error!("Public key recovery failed: {:?}", e);
         Error::Synthesis
@@ -103,7 +108,9 @@ fn recover_pk(
     pk_le.copy_from_slice(&pk_be[1..]);
     pk_le[..32].reverse();
     pk_le[32..].reverse();
-    let pk = Secp256k1Affine::from_bytes(&secp256k1::Serialized(pk_le));
+    let mut pk_bytes = secp256k1::Serialized::default();
+    pk_bytes.as_mut().copy_from_slice(&pk_le[..]);
+    let pk = Secp256k1Affine::from_bytes(&pk_bytes);
     ct_option_ok_or(pk, Error::Synthesis).map_err(|e| {
         error!("Invalid public key little endian bytes");
         e
@@ -168,18 +175,32 @@ fn tx_to_sign_data(tx: &Transaction, chain_id: u64) -> Result<SignData, Error> {
 
 // TODO: Deduplicate with
 // `zkevm-circuits/src/evm_circuit/table.rs::TxContextFieldTag`.
+/// Tag used to identify each field in the transaction in a row of the
+/// transaction table.
 #[derive(Clone, Copy, Debug)]
 pub enum TxFieldTag {
+    /// Unused tag
     Null = 0,
+    /// Nonce
     Nonce,
+    /// Gas
     Gas,
+    /// GasPrice
     GasPrice,
+    /// CallerAddress
     CallerAddress,
+    /// CalleeAddress
     CalleeAddress,
+    /// IsCreate
     IsCreate,
+    /// Value
     Value,
+    /// CallDataLength
     CallDataLength,
+    /// TxSignHash: Hash of the transaction without the signature, used for
+    /// signing.
     TxSignHash,
+    /// CallData
     CallData,
 }
 
@@ -425,7 +446,7 @@ mod tx_circuit_tests {
     };
     use ethers_signers::{LocalWallet, Signer};
     use group::{Curve, Group};
-    use halo2_proofs::{dev::MockProver, pairing::bn256::Fr};
+    use halo2_proofs::{arithmetic::CurveAffine, dev::MockProver, pairing::bn256::Fr};
     use pretty_assertions::assert_eq;
     use rand::{CryptoRng, Rng, SeedableRng};
     use rand_chacha::ChaCha20Rng;
@@ -456,20 +477,6 @@ mod tx_circuit_tests {
             chain_id,
         };
 
-        #[cfg(feature = "dev-graph")]
-        {
-            use plotters::prelude::*;
-            let root = BitMapBackend::new("tx-circuit.png", (16384, 65536)).into_drawing_area();
-            root.fill(&WHITE).unwrap();
-            let root = root.titled("TxCircuit", ("sans-serif", 60)).unwrap();
-            halo2_proofs::dev::CircuitLayout::default()
-                .show_labels(true)
-                .mark_equality_cells(true)
-                .show_equality_constraints(true)
-                .render(20, &circuit, &root)
-                .unwrap();
-        }
-
         let prover = match MockProver::run(k, &circuit, power_of_randomness) {
             Ok(prover) => prover,
             Err(e) => panic!("{:#?}", e),
@@ -513,7 +520,7 @@ mod tx_circuit_tests {
     }
 
     #[test]
-    fn test_tx_pk_recovery() {
+    fn test_tx_circuit() {
         const NUM_TXS: usize = 2;
         const MAX_TXS: usize = 2;
         const MAX_CALLDATA: usize = 32;

zkevm-circuits/src/tx_circuit/sign_verify.rs

@@ -1,6 +1,3 @@
-// TODO Remove this
-#![allow(missing_docs)]
-
 use crate::{
     evm_circuit::util::{not, RandomLinearCombination, Word},
     gadget::is_zero::{IsZeroChip, IsZeroConfig, IsZeroInstruction},
@@ -11,8 +8,8 @@ use ecdsa::ecdsa::{AssignedEcdsaSig, AssignedPublicKey, EcdsaChip};
 use group::{ff::Field, prime::PrimeCurveAffine, Curve};
 use halo2_proofs::{
     arithmetic::{BaseExt, CurveAffine},
-    circuit::{AssignedCell, Layouter, Region, SimpleFloorPlanner},
-    plonk::{Advice, Circuit, Column, ConstraintSystem, Error, Expression, Selector},
+    circuit::{AssignedCell, Layouter, Region},
+    plonk::{Advice, Column, ConstraintSystem, Error, Expression, Selector},
     poly::Rotation,
 };
 use integer::{
@@ -22,6 +19,7 @@ use integer::{
 use itertools::Itertools;
 use keccak256::plain::Keccak;
 use lazy_static::lazy_static;
+use log::error;
 use maingate::{
     Assigned, AssignedValue, MainGate, MainGateConfig, MainGateInstructions, RangeChip,
     RangeConfig, RangeInstructions, RegionCtx, UnassignedValue,
@@ -30,7 +28,11 @@ use pairing::arithmetic::{Coordinates, FieldExt};
 use secp256k1::Secp256k1Affine;
 use std::{cmp::min, convert::TryInto, io::Cursor, marker::PhantomData};
 
+/// Power of randomness vector size required for the SignVerifyChip
 pub const POW_RAND_SIZE: usize = 63;
+
+/// Number of rows required for a verification of the SignVerifyChip in the
+/// "signature address verify" region.
 pub const VERIF_HEIGHT: usize = 1;
 
 /// Auxiliary Gadget to verify a that a message hash is signed by the public
@@ -40,14 +42,14 @@ pub(crate) struct SignVerifyChip<F: FieldExt, const MAX_VERIF: usize> {
     pub aux_generator: Secp256k1Affine,
     pub window_size: usize,
     pub _marker: PhantomData<F>,
-    // ecdsa_chip: EcdsaChip<Secp256k1Affine, F>,
 }
 
 const KECCAK_IS_ENABLED: usize = 0;
 const KECCAK_INPUT_RLC: usize = 1;
 const KECCAK_INPUT_LEN: usize = 2;
 const KECCAK_OUTPUT_RLC: usize = 3;
 
+const NUMBER_OF_LIMBS: usize = 4;
 const BIT_LEN_LIMB: usize = 72;
 
 /// Return an expression that builds an integer element in the field from the
@@ -84,7 +86,7 @@ fn integer_eq_bytes_le<F: FieldExt>(
 fn copy_integer<F: FieldExt, W: WrongExt>(
     region: &mut Region<'_, F>,
     name: &str,
-    src: AssignedInteger<W, F>,
+    src: AssignedInteger<W, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
     dst: &[Column<Advice>; 4],
     offset: usize,
 ) -> Result<(), Error> {
@@ -276,7 +278,8 @@ impl<F: FieldExt> SignVerifyConfig<F> {
         });
 
         // ECDSA config
-        let (rns_base, rns_scalar) = GeneralEccChip::<Secp256k1Affine, F>::rns(BIT_LEN_LIMB);
+        let (rns_base, rns_scalar) =
+            GeneralEccChip::<Secp256k1Affine, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>::rns();
         let main_gate_config = MainGate::<F>::configure(meta);
         let mut overflow_bit_lengths: Vec<usize> = vec![];
         overflow_bit_lengths.extend(rns_base.overflow_lengths());
@@ -418,7 +421,7 @@ fn integer_to_bytes_le<F: FieldExt, W: WrongExt>(
     main_gate: &MainGate<F>,
     range_chip: &RangeChip<F>,
     pows_256: &[AssignedValue<F>],
-    int: &AssignedInteger<W, F>,
+    int: &AssignedInteger<W, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
 ) -> Result<[AssignedValue<F>; 32], Error> {
     let mut int_le = Vec::new();
     int_le.extend(int.limbs()[0].decompose(9, 8).expect("bad decompose"));
@@ -429,15 +432,18 @@ fn integer_to_bytes_le<F: FieldExt, W: WrongExt>(
         .iter()
         .map(|b| range_chip.range_value(ctx, &UnassignedValue::from(Some(*b)), 8))
         .try_collect()
-        .expect("FIXME");
+        .map_err(|e| {
+            error!("RangeChip::range_value error: {:?}", e);
+            e
+        })?;
     let int_le: [AssignedValue<F>; 32] = int_le.try_into().expect("vec to array of size 32");
     for (j, positions) in [1..9, 1..9, 1..9, 1..5].iter().enumerate() {
         let mut acc = int_le[j * 9].clone();
         for i in positions.clone() {
-            let shifted = main_gate.mul(ctx, int_le[j * 9 + i].clone(), pows_256[i - 1].clone())?;
-            acc = main_gate.add(ctx, acc, shifted)?;
+            let shifted = main_gate.mul(ctx, &int_le[j * 9 + i], &pows_256[i - 1])?;
+            acc = main_gate.add(ctx, &acc, &shifted)?;
         }
-        main_gate.assert_equal(ctx, acc, int.limbs()[j].clone())?;
+        main_gate.assert_equal(ctx, &acc, &(&int.limbs()[j]).into())?;
     }
     Ok(int_le)
 }
@@ -446,7 +452,7 @@ impl<F: FieldExt, const MAX_VERIF: usize> SignVerifyChip<F, MAX_VERIF> {
     pub fn assign_aux(
         &self,
         region: &mut Region<'_, F>,
-        ecc_chip: &mut GeneralEccChip<Secp256k1Affine, F>,
+        ecc_chip: &mut GeneralEccChip<Secp256k1Affine, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
     ) -> Result<(), Error> {
         let ctx_offset = &mut 0;
         let ctx = &mut RegionCtx::new(region, ctx_offset);
@@ -461,9 +467,9 @@ impl<F: FieldExt, const MAX_VERIF: usize> SignVerifyChip<F, MAX_VERIF> {
         ctx: &mut RegionCtx<F>,
         main_gate: &MainGate<F>,
         range_chip: &RangeChip<F>,
-        ecc_chip: &GeneralEccChip<Secp256k1Affine, F>,
-        scalar_chip: &IntegerChip<secp256k1::Fq, F>,
-        ecdsa_chip: &EcdsaChip<Secp256k1Affine, F>,
+        ecc_chip: &GeneralEccChip<Secp256k1Affine, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
+        scalar_chip: &IntegerChip<secp256k1::Fq, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
+        ecdsa_chip: &EcdsaChip<Secp256k1Affine, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>,
         sign_data: &SignData,
     ) -> Result<AssignedECDSA<F>, Error> {
         let SignData {
@@ -668,8 +674,9 @@ impl<F: FieldExt, const MAX_VERIF: usize> SignVerifyChip<F, MAX_VERIF> {
         // TODO: Figure out the best value for RangeChip base_bit_len, when we want to
         // range on 8 bits.
         let range_chip = RangeChip::new(config.range_config.clone(), 8);
-        let mut ecc_chip =
-            GeneralEccChip::<Secp256k1Affine, F>::new(config.ecc_chip_config(), BIT_LEN_LIMB);
+        let mut ecc_chip = GeneralEccChip::<Secp256k1Affine, F, NUMBER_OF_LIMBS, BIT_LEN_LIMB>::new(
+            config.ecc_chip_config(),
+        );
         let scalar_chip = ecc_chip.scalar_field_chip();
 
         layouter.assign_region(
@@ -711,8 +718,6 @@ impl<F: FieldExt, const MAX_VERIF: usize> SignVerifyChip<F, MAX_VERIF> {
                 Ok(())
             },
         )?;
-        println!("DBG MAX_VERIF {}", MAX_VERIF);
-        println!("DBG assigned_ecdsas.len {}", assigned_ecdsas.len());
 
         let mut assigned_sig_verifs = Vec::new();
         layouter.assign_region(
@@ -799,14 +804,6 @@ lazy_static! {
         let randomness = secp256k1::Fq::one();
         let (sig_r, sig_s) = sign(randomness, sk, msg_hash);
 
-        println!(
-            "DBG sign_data: {:?}",
-            SignData {
-                signature: (sig_r, sig_s),
-                pk,
-                msg_hash,
-            }
-        );
         SignData {
             signature: (sig_r, sig_s),
             pk,
@@ -836,7 +833,9 @@ fn pub_key_hash_to_address<F: FieldExt>(pk_hash: &[u8]) -> F {
 mod sign_verify_tests {
     use super::*;
     use group::Group;
-    use halo2_proofs::{dev::MockProver, pairing::bn256::Fr};
+    use halo2_proofs::{
+        circuit::SimpleFloorPlanner, dev::MockProver, pairing::bn256::Fr, plonk::Circuit,
+    };
     use pretty_assertions::assert_eq;
     use rand::{RngCore, SeedableRng};
     use rand_xorshift::XorShiftRng;