Merge pull request #190 from kroma-network/fix/fix-bug-on-constraint-…

…system

fix(zk): fix bug on constraint system

tachyon/zk/plonk/BUILD.bazel

@@ -1,4 +1,4 @@
-load("//bazel:tachyon_cc.bzl", "tachyon_cc_library")
+load("//bazel:tachyon_cc.bzl", "tachyon_cc_library", "tachyon_cc_unittest")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -20,3 +20,12 @@ tachyon_cc_library(
  "//tachyon/zk/plonk/permutation:permutation_argument",
  ],
 )
+
+tachyon_cc_unittest(
+ name = "plonk_unittests",
+ srcs = ["constraint_system_unittest.cc"],
+ deps = [
+ ":constraint_system",
+ "//tachyon/math/finite_fields/test:gf7",
+ ],
+)

tachyon/zk/plonk/circuit/phase.h

@@ -19,7 +19,7 @@
 
 namespace tachyon::zk {
 
-// NOTE(lightscale-luke): Rotation class is copyable, assignable, and occupy 8
+// NOTE(lightscale-luke): Phase class is copyable, assignable, and occupy 8
 // bits per instance. Prefer to pass them by value.
 class TACHYON_EXPORT Phase {
  public:
@@ -34,8 +34,12 @@ class TACHYON_EXPORT Phase {
 
  uint8_t value() const { return value_; }
 
- bool operator==(const Phase& other) const { return value_ == other.value_; }
- bool operator!=(const Phase& other) const { return value_ != other.value_; }
+ bool operator==(Phase other) const { return value_ == other.value_; }
+ bool operator!=(Phase other) const { return value_ != other.value_; }
+ bool operator>(Phase other) const { return value_ > other.value_; }
+ bool operator>=(Phase other) const { return value_ >= other.value_; }
+ bool operator<(Phase other) const { return value_ < other.value_; }
+ bool operator<=(Phase other) const { return value_ <= other.value_; }
 
  std::string ToString() const { return base::NumberToString(value_); }
 
@@ -47,7 +51,7 @@ constexpr static Phase kFirstPhase = Phase(0);
 constexpr static Phase kSecondPhase = Phase(1);
 
 template <typename H>
-H AbslHashValue(H h, const Phase& phase) {
+H AbslHashValue(H h, Phase phase) {
  return H::combine(std::move(h), phase.value());
 }
 

tachyon/zk/plonk/circuit/table_unittest.cc

@@ -13,7 +13,6 @@ template <typename ColumnKeyType>
 class TableTest : public testing::Test {
  public:
  constexpr static size_t kMaxDegree = (size_t{1} << 3) - 1;
- constexpr static Phase kFirstPhase = Phase(0);
 
  using F = math::bn254::G1AffinePoint::ScalarField;
  using Evals = math::UnivariateEvaluations<F, kMaxDegree>;

tachyon/zk/plonk/constraint_system.h

@@ -412,15 +412,15 @@ class ConstraintSystem {
  Phase ComputeMaxPhase() const {
  auto max_phase_it = std::max_element(advice_column_phases_.begin(),
  advice_column_phases_.end());
- if (max_phase_it == advice_column_phases_.end()) return Phase(0);
+ if (max_phase_it == advice_column_phases_.end()) return kFirstPhase;
  return *max_phase_it;
  }
 
  std::vector<Phase> GetPhases() const {
  Phase max_phase = ComputeMaxPhase();
- return base::CreateVector(size_t{max_phase.value()}, [](size_t i) {
- return Phase(static_cast<uint8_t>(i));
- });
+ return base::CreateVector(
+  static_cast<size_t>(max_phase.value() + 1),
+  [](size_t i) { return Phase(static_cast<uint8_t>(i)); });
  }
 
  // Compute the degree of the constraint system (the maximum degree of all
@@ -459,7 +459,7 @@ class ConstraintSystem {
  factors = std::max(size_t{3}, factors);
 
  // Each polynomial is evaluated at most an additional time during
- // multiopen (at x₃ to produce qₑᵥₐₗₛ):
+ // multiopen (at x₃ to produce q_evals):
  ++factors;
 
  // h(x) is derived by the other evaluations so it does not reveal
@@ -479,10 +479,10 @@ class ConstraintSystem {
  // account for e.g. blinding factors.
  size_t ComputeMinimumRows() const {
  return ComputeBlindingFactors() // m blinding factors
- + 1 // for l_{-(m + 1)} (lₗₐₛₜ)
+ + 1 // for l_{-(m + 1)} (l_last)
  + 1 // for l₀ (just for extra breathing room for the permutation
  // argument, to essentially force a separation in the
- // permutation polynomial between the roles of lₗₐₛₜ, l₀
+ // permutation polynomial between the roles of l_last, l₀
  // and the interstitial values.)
  + 1; // for at least one row
  }
@@ -497,7 +497,7 @@ class ConstraintSystem {
  return query.column() == column && query.rotation() == at;
  });
  if (!index.has_value()) return false;
- *index = index.value();
+ *index_out = index.value();
  return true;
  }
 

tachyon/zk/plonk/constraint_system_unittest.cc

@@ -0,0 +1,191 @@
+#include "tachyon/zk/plonk/constraint_system.h"
+
+#include "gtest/gtest.h"
+
+#include "tachyon/math/finite_fields/test/gf7.h"
+
+namespace tachyon::zk {
+
+namespace {
+
+class ConstraintSystemTest : public testing::Test {
+ public:
+ static void SetUpTestSuite() { math::GF7::Init(); }
+};
+
+} // namespace
+
+TEST_F(ConstraintSystemTest, EnableConstant) {
+ ConstraintSystem<math::GF7> constraint_system;
+ std::vector<FixedColumnKey> expected_constants;
+ std::vector<AnyColumnKey> expected_permutation_columns;
+ EXPECT_EQ(constraint_system.constants(), expected_constants);
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+
+ FixedColumnKey column = constraint_system.CreateFixedColumn();
+ constraint_system.EnableConstant(column);
+ expected_constants.push_back(column);
+ EXPECT_EQ(constraint_system.constants(), expected_constants);
+ expected_permutation_columns.push_back(AnyColumnKey(column));
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+
+ constraint_system.EnableConstant(column);
+ EXPECT_EQ(constraint_system.constants(), expected_constants);
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+}
+
+// TODO(chokobole): Add tests for Lookup and LookupAny.
+
+TEST_F(ConstraintSystemTest, QueryFixedIndex) {
+ ConstraintSystem<math::GF7> constraint_system;
+ FixedColumnKey column = constraint_system.CreateFixedColumn();
+ Rotation rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryFixedIndex(column, rotation), 0);
+ EXPECT_EQ(constraint_system.QueryFixedIndex(column, rotation), 0);
+
+ column = constraint_system.CreateFixedColumn();
+ rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryFixedIndex(column, rotation), 1);
+
+ rotation = Rotation::Next();
+ EXPECT_EQ(constraint_system.QueryFixedIndex(column, rotation), 2);
+}
+
+TEST_F(ConstraintSystemTest, QueryAdviceIndex) {
+ ConstraintSystem<math::GF7> constraint_system;
+ AdviceColumnKey column = constraint_system.CreateAdviceColumn();
+ Rotation rotation = Rotation::Cur();
+
+ EXPECT_EQ(constraint_system.QueryAdviceIndex(column, rotation), 0);
+ EXPECT_EQ(constraint_system.QueryAdviceIndex(column, rotation), 0);
+
+ column = constraint_system.CreateAdviceColumn();
+ rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryAdviceIndex(column, rotation), 1);
+
+ rotation = Rotation::Next();
+ EXPECT_EQ(constraint_system.QueryAdviceIndex(column, rotation), 2);
+}
+
+TEST_F(ConstraintSystemTest, QueryInstanceIndex) {
+ ConstraintSystem<math::GF7> constraint_system;
+ InstanceColumnKey column = constraint_system.CreateInstanceColumn();
+ Rotation rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryInstanceIndex(column, rotation), 0);
+ EXPECT_EQ(constraint_system.QueryInstanceIndex(column, rotation), 0);
+
+ column = constraint_system.CreateInstanceColumn();
+ rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryInstanceIndex(column, rotation), 1);
+
+ rotation = Rotation::Next();
+ EXPECT_EQ(constraint_system.QueryInstanceIndex(column, rotation), 2);
+}
+
+TEST_F(ConstraintSystemTest, Phases) {
+ ConstraintSystem<math::GF7> constraint_system;
+ EXPECT_DEATH(constraint_system.CreateAdviceColumn(kSecondPhase), "");
+
+ std::vector<Phase> phases = {kFirstPhase};
+ EXPECT_EQ(constraint_system.ComputeMaxPhase(), kFirstPhase);
+ EXPECT_EQ(constraint_system.GetPhases(), phases);
+
+ constraint_system.CreateAdviceColumn(kFirstPhase);
+ EXPECT_EQ(constraint_system.ComputeMaxPhase(), kFirstPhase);
+ EXPECT_EQ(constraint_system.GetPhases(), phases);
+
+ constraint_system.CreateAdviceColumn(kSecondPhase);
+ phases.push_back(kSecondPhase);
+ EXPECT_EQ(constraint_system.ComputeMaxPhase(), kSecondPhase);
+ EXPECT_EQ(constraint_system.GetPhases(), phases);
+}
+
+namespace {
+
+template <typename ColumnKeyType>
+class ConstraintSystemTypedTest : public testing::Test {
+ public:
+ static void SetUpTestSuite() { math::GF7::Init(); }
+};
+
+} // namespace
+
+using ColumnKeyTypes =
+ testing::Types<FixedColumnKey, AdviceColumnKey, InstanceColumnKey>;
+TYPED_TEST_SUITE(ConstraintSystemTypedTest, ColumnKeyTypes);
+
+TYPED_TEST(ConstraintSystemTypedTest, EnableEquality) {
+ using ColumnKeyTy = TypeParam;
+
+ ConstraintSystem<math::GF7> constraint_system;
+ std::vector<AnyColumnKey> expected_permutation_columns;
+ std::vector<FixedQueryData> fixed_queries;
+ std::vector<AdviceQueryData> advice_queries;
+ std::vector<size_t> num_advice_queries;
+ std::vector<InstanceQueryData> instance_queries;
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+
+ ColumnKeyTy column;
+ if constexpr (std::is_same_v<ColumnKeyTy, FixedColumnKey>) {
+ column = constraint_system.CreateFixedColumn();
+ fixed_queries.push_back(FixedQueryData(Rotation::Cur(), column));
+ } else if constexpr (std::is_same_v<ColumnKeyTy, AdviceColumnKey>) {
+ column = constraint_system.CreateAdviceColumn();
+ num_advice_queries.push_back(0);
+ EXPECT_EQ(constraint_system.num_advice_queries(), num_advice_queries);
+ ++num_advice_queries[column.index()];
+ advice_queries.push_back(AdviceQueryData(Rotation::Cur(), column));
+ } else {
+ column = constraint_system.CreateInstanceColumn();
+ instance_queries.push_back(InstanceQueryData(Rotation::Cur(), column));
+ }
+ constraint_system.EnableEquality(column);
+ expected_permutation_columns.push_back(AnyColumnKey(column));
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+ EXPECT_EQ(constraint_system.fixed_queries(), fixed_queries);
+ EXPECT_EQ(constraint_system.advice_queries(), advice_queries);
+ EXPECT_EQ(constraint_system.num_advice_queries(), num_advice_queries);
+ EXPECT_EQ(constraint_system.instance_queries(), instance_queries);
+
+ constraint_system.EnableEquality(column);
+ EXPECT_EQ(constraint_system.permutation().columns(),
+ expected_permutation_columns);
+ EXPECT_EQ(constraint_system.fixed_queries(), fixed_queries);
+ EXPECT_EQ(constraint_system.advice_queries(), advice_queries);
+ EXPECT_EQ(constraint_system.num_advice_queries(), num_advice_queries);
+ EXPECT_EQ(constraint_system.instance_queries(), instance_queries);
+}
+
+TYPED_TEST(ConstraintSystemTypedTest, QueryAnyIndex) {
+ using ColumnKeyTy = TypeParam;
+
+ ConstraintSystem<math::GF7> constraint_system;
+ std::function<ColumnKeyTy()> create_column = [&constraint_system]() {
+ if constexpr (std::is_same_v<ColumnKeyTy, FixedColumnKey>) {
+ return constraint_system.CreateFixedColumn();
+ } else if constexpr (std::is_same_v<ColumnKeyTy, AdviceColumnKey>) {
+ return constraint_system.CreateAdviceColumn();
+ } else {
+ return constraint_system.CreateInstanceColumn();
+ }
+ };
+
+ ColumnKeyTy column = create_column();
+ Rotation rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryAnyIndex(column, rotation), 0);
+ EXPECT_EQ(constraint_system.QueryAnyIndex(column, rotation), 0);
+
+ column = create_column();
+ rotation = Rotation::Cur();
+ EXPECT_EQ(constraint_system.QueryAnyIndex(column, rotation), 1);
+
+ rotation = Rotation::Next();
+ EXPECT_EQ(constraint_system.QueryAnyIndex(column, rotation), 2);
+}
+
+} // namespace tachyon::zk