From abf7cfe93c7384c24e79fb496340f7ba70bf7105 Mon Sep 17 00:00:00 2001 From: zhenfei Date: Fri, 28 Jul 2023 07:33:12 -0400 Subject: [PATCH 1/5] add fixed cells for chunk_is_valid --- aggregator/src/core.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/aggregator/src/core.rs b/aggregator/src/core.rs index 8ebfc43946..92d77ccdab 100644 --- a/aggregator/src/core.rs +++ b/aggregator/src/core.rs @@ -733,10 +733,19 @@ pub(crate) fn chunk_is_valid( offset: &mut usize, ) -> Result<[AssignedCell; MAX_AGG_SNARKS], halo2_proofs::plonk::Error> { let mut res = vec![]; - - for i in 0..MAX_AGG_SNARKS { - let value = rlc_config.load_private(region, &Fr::from(i as u64), offset)?; - let is_valid = rlc_config.is_smaller_than(region, &value, num_of_valid_chunks, offset)?; + let mut cur_index = rlc_config.load_private(region, &Fr::zero(), offset)?; + let zero_cell = rlc_config.zero_cell(cur_index.cell().region_index); + region.constrain_equal(cur_index.cell(), zero_cell)?; + let one = rlc_config.load_private(region, &Fr::zero(), offset)?; + let one_cell = rlc_config.zero_cell(one.cell().region_index); + region.constrain_equal(one.cell(), one_cell)?; + + let is_valid = rlc_config.is_smaller_than(region, &cur_index, num_of_valid_chunks, offset)?; + res.push(is_valid); + for _ in 0..MAX_AGG_SNARKS - 1 { + cur_index = rlc_config.add(region, &cur_index, &one, offset)?; + let is_valid = + rlc_config.is_smaller_than(region, &cur_index, num_of_valid_chunks, offset)?; res.push(is_valid); } // constrain the chunks are ordered with real ones at the beginning. that is, From 9c5aecd86497c6bed8eac92932c8d79d81e4e341 Mon Sep 17 00:00:00 2001 From: zhenfei Date: Fri, 28 Jul 2023 07:39:01 -0400 Subject: [PATCH 2/5] fix typo --- aggregator/src/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggregator/src/core.rs b/aggregator/src/core.rs index 92d77ccdab..3a0da838b2 100644 --- a/aggregator/src/core.rs +++ b/aggregator/src/core.rs @@ -736,7 +736,7 @@ pub(crate) fn chunk_is_valid( let mut cur_index = rlc_config.load_private(region, &Fr::zero(), offset)?; let zero_cell = rlc_config.zero_cell(cur_index.cell().region_index); region.constrain_equal(cur_index.cell(), zero_cell)?; - let one = rlc_config.load_private(region, &Fr::zero(), offset)?; + let one = rlc_config.load_private(region, &Fr::one(), offset)?; let one_cell = rlc_config.zero_cell(one.cell().region_index); region.constrain_equal(one.cell(), one_cell)?; From 048afaa03dbb57c6c42248959ae1e429d017da14 Mon Sep 17 00:00:00 2001 From: zhenfei Date: Fri, 28 Jul 2023 07:41:43 -0400 Subject: [PATCH 3/5] well... need morning coffee --- aggregator/src/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggregator/src/core.rs b/aggregator/src/core.rs index 3a0da838b2..c6a8fa603d 100644 --- a/aggregator/src/core.rs +++ b/aggregator/src/core.rs @@ -737,7 +737,7 @@ pub(crate) fn chunk_is_valid( let zero_cell = rlc_config.zero_cell(cur_index.cell().region_index); region.constrain_equal(cur_index.cell(), zero_cell)?; let one = rlc_config.load_private(region, &Fr::one(), offset)?; - let one_cell = rlc_config.zero_cell(one.cell().region_index); + let one_cell = rlc_config.one_cell(one.cell().region_index); region.constrain_equal(one.cell(), one_cell)?; let is_valid = rlc_config.is_smaller_than(region, &cur_index, num_of_valid_chunks, offset)?; From d2b59a9106b0ffd6457ef4892c93414bac6a802a Mon Sep 17 00:00:00 2001 From: zhenfei Date: Fri, 28 Jul 2023 07:42:45 -0400 Subject: [PATCH 4/5] typo in readme --- aggregator/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggregator/README.md b/aggregator/README.md index e5888dea4a..bf869e3c5b 100644 --- a/aggregator/README.md +++ b/aggregator/README.md @@ -182,7 +182,7 @@ for i in 1 ... n: if is_padding: chunk_i.prev_state_root == chunk_i.post_state_root chunk_i.withdraw_root == chunk_{i-1}.withdraw_root - chunk_i.data_hash == [0u8; 32] + chunk_i.data_hash == keccak("") ``` 7. chunk[i]'s data_hash len is `0` when chunk[i] is padded From f1d34c54b3d782a489c5dd488b2ea7445d1353e1 Mon Sep 17 00:00:00 2001 From: zhenfei Date: Fri, 28 Jul 2023 08:38:03 -0400 Subject: [PATCH 5/5] fix soundness issue in is_smaller_than --- aggregator/src/aggregation/rlc/gates.rs | 34 ++++++++++++++++++++----- aggregator/src/tests/rlc/gates.rs | 6 ++--- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/aggregator/src/aggregation/rlc/gates.rs b/aggregator/src/aggregation/rlc/gates.rs index 2ce903d54c..8b3af6f16f 100644 --- a/aggregator/src/aggregation/rlc/gates.rs +++ b/aggregator/src/aggregation/rlc/gates.rs @@ -28,6 +28,12 @@ impl RlcConfig { 5, || Value::known(Fr::from(32)), )?; + region.assign_fixed( + || "const two to thirty two", + self.fixed, + 6, + || Value::known(Fr::from(1 << 32)), + )?; Ok(()) } @@ -85,6 +91,15 @@ impl RlcConfig { } } + #[inline] + pub(crate) fn two_to_thirty_two_cell(&self, region_index: RegionIndex) -> Cell { + Cell { + region_index, + row_offset: 6, + column: self.fixed.into(), + } + } + pub(crate) fn load_private( &self, region: &mut Region, @@ -399,7 +414,7 @@ impl RlcConfig { } // return a boolean if a is smaller than b - // requires that both a and b are smallish + // requires that both a and b are less than 32 bits pub(crate) fn is_smaller_than( &self, region: &mut Region, @@ -408,11 +423,18 @@ impl RlcConfig { offset: &mut usize, ) -> Result, Error> { // when a and b are both small (as in our use case) - // if a < b, (a-b) will under flow and the highest bit of (a-b) be one - // else, the highest bit of (a-b) be zero - let sub = self.sub(region, a, b, offset)?; - let bits = self.decomposition(region, &sub, offset)?; - Ok(bits[253].clone()) + // if a >= b, c = 2^32 + (a-b) will be >= 2^32 + // we bit decompose c and check the 33-th bit + let two_to_thirty_two = self.load_private(region, &Fr::from(1 << 32), offset)?; + let two_to_thirty_two_cell = + self.two_to_thirty_two_cell(two_to_thirty_two.cell().region_index); + region.constrain_equal(two_to_thirty_two_cell, two_to_thirty_two.cell())?; + + let ca = self.add(region, &two_to_thirty_two, a, offset)?; + let c = self.sub(region, &ca, b, offset)?; + let bits = self.decomposition(region, &c, offset)?; + let res = self.not(region, &bits[32], offset)?; + Ok(res) } } #[inline] diff --git a/aggregator/src/tests/rlc/gates.rs b/aggregator/src/tests/rlc/gates.rs index f614730e81..057b43f38f 100644 --- a/aggregator/src/tests/rlc/gates.rs +++ b/aggregator/src/tests/rlc/gates.rs @@ -146,8 +146,8 @@ impl Circuit for ArithTestCircuit { // unit test: is smaller than { for _ in 0..10 { - let a = Fr::from(rng.next_u64()); - let b = Fr::from(rng.next_u64()); + let a = Fr::from(rng.next_u32() as u64); + let b = Fr::from(rng.next_u32() as u64); let c = if a < b { Fr::one() } else { Fr::zero() }; let a = config.load_private(&mut region, &a, &mut offset)?; let b = config.load_private(&mut region, &b, &mut offset)?; @@ -157,7 +157,7 @@ impl Circuit for ArithTestCircuit { } // equality check - let a = Fr::from(rng.next_u64()); + let a = Fr::from(rng.next_u32() as u64); let b = a; let c = Fr::zero(); let a = config.load_private(&mut region, &a, &mut offset)?;