From 66ae3ec4bad90765b9f213bc59d0e7d06117cbbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?I=C3=B1igo=20Querejeta=20Azurmendi?=
 <31273774+iquerejeta@users.noreply.github.com>
Date: Mon, 1 Apr 2024 16:27:54 +0200
Subject: [PATCH] Fix number of rows (#3)

* Account for empty regions

* Bump rust toolchain

Unfortunately the wasm backend makes a breaking change. We need newer version to support bumpalo
---
 halo2_proofs/src/dev/cost_model.rs | 36 ++++++++++++++++++++----------
 halo2_proofs/tests/plonk_api.rs    |  4 ++--
 rust-toolchain                     |  2 +-
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/halo2_proofs/src/dev/cost_model.rs b/halo2_proofs/src/dev/cost_model.rs
index 2c3ab4f788..b19189e2b5 100644
--- a/halo2_proofs/src/dev/cost_model.rs
+++ b/halo2_proofs/src/dev/cost_model.rs
@@ -54,7 +54,7 @@ pub struct CostOptions {
 
     /// Minimum value of k needed to account for the circuit
     pub min_k: usize,
-	
+
     /// Expanded rows count, which does not account for compression (where
     /// multiple regions can use the same rows).
     pub expanded_rows_count: usize,
@@ -139,7 +139,7 @@ impl Shuffle {
 
 /// Information about how a gate (see plonk::circuit::Gate) is used in the
 /// circuit.
-#[derive(Clone, Debug, Deserialize, Serialize)] 
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct GateUsage {
     /// The name of the gate.
     pub name: String,
@@ -353,16 +353,29 @@ pub fn from_circuit_to_cost_model_options<F: Ord + Field + FromUniformBytes<64>,
         let mut expanded_rows_count = 0;
         let mut compressed_rows_count = 0;
         for region in prover.regions {
-            if region.name.contains("table") { // Account for lookup tables separately
-                let (start, end) = region.rows.expect("region.rows should be known by now");
-                min_k = std::cmp::max(min_k, end - start);
-            } else {
-                let (start, end) = region.rows.expect("region.rows should be known by now");
-                expanded_rows_count = expanded_rows_count + ((end - start) + 1);
-                compressed_rows_count = std::cmp::max(compressed_rows_count, end + 1);
-            }
+            // If `region.rows == None`, then that region has no rows (e.g. just copy constraints)
+            if let Some((start, end)) = region.rows {
+                // Note that `end` is the index of the last row, so when
+                // counting rows, this last row needs to be counted via `end +
+                // 1`.
+
+                // A region is a _table region_ if all of its columns are `Fixed`
+                // columns (see that `plonk::circuit::TableColumn` is a wrapper
+                // around `Column<Fixed>`). All of a table region's rows are
+                // counted towards `table_rows_count.`
+                if region
+                    .columns
+                    .iter()
+                    .all(|c| *c.column_type() == crate::plonk::Any::Fixed)
+                {
+                    min_k = std::cmp::max(min_k, (end + 1) - start);
+                } else {
+                    expanded_rows_count += (end + 1) - start;
+                    compressed_rows_count = std::cmp::max(compressed_rows_count, end + 1);
+                }
 
-            min_k = std::cmp::max(min_k, compressed_rows_count);
+                min_k = std::cmp::max(min_k, compressed_rows_count);
+            }
         }
         (expanded_rows_count, compressed_rows_count, min_k)
     };
@@ -390,7 +403,6 @@ pub fn from_circuit_to_cost_model_options<F: Ord + Field + FromUniformBytes<64>,
         })
         .collect();
 
-
     CostOptions {
         advice,
         instance,
diff --git a/halo2_proofs/tests/plonk_api.rs b/halo2_proofs/tests/plonk_api.rs
index 28ffb399ff..16f2031e35 100644
--- a/halo2_proofs/tests/plonk_api.rs
+++ b/halo2_proofs/tests/plonk_api.rs
@@ -622,7 +622,7 @@ fn plonk_api() {
             //panic!("{:#?}", pk.get_vk().pinned());
             assert_eq!(
                 format!("{:#?}", pk.get_vk().pinned()),
-                r#####"PinnedVerificationKey {
+                r#"PinnedVerificationKey {
     base_modulus: "0x40000000000000000000000000000000224698fc0994a8dd8c46eb2100000001",
     scalar_modulus: "0x40000000000000000000000000000000224698fc094cf91b992d30ed00000001",
     domain: PinnedEvaluationDomain {
@@ -1015,7 +1015,7 @@ fn plonk_api() {
             (0x3d907e0591343bd285c2c846f3e871a6ac70d80ec29e9500b8cb57f544e60202, 0x1034e48df35830244cabea076be8a16d67d7896e27c6ac22b285d017105da9c3),
         ],
     },
-}"#####
+}"#
             );
         }
     }
diff --git a/rust-toolchain b/rust-toolchain
index 65ee095984..5e3a425662 100644
--- a/rust-toolchain
+++ b/rust-toolchain
@@ -1 +1 @@
-1.67.0
+1.73.0