Addressing some review comments

AztecProtocol · Nov 28, 2023 · 3c63ce3 · 3c63ce3
1 parent 2218e47
commit 3c63ce3
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 11 deletions.
diff --git a/barretenberg/cpp/pil/avm/avm_mini.pil b/barretenberg/cpp/pil/avm/avm_mini.pil
@@ -4,7 +4,6 @@ namespace avmMini(%N);
 
     //===== CONSTANT POLYNOMIALS ==================================================
     pol constant clk(i) { i };
-    pol constant positive(i) { (i + 1) };
     pol constant first = [1] + [0]*; // Used mostly to toggle off the first row consisting
                                      // only in first element of shifted polynomials.
 
@@ -71,12 +70,13 @@ namespace avmMini(%N);
     // We need: m_lastAccess == 1 ==> m_addr' > m_addr
     // The above implies: m_addr' == m_addr ==> m_lastAccess == 0
     // This condition does not apply on the last row.
+    // clk + 1 used as an expression for positive integers
     // TODO: Uncomment when lookups are supported
-    // (1 - first) * (1 - last) * m_lastAccess { (m_addr' - m_addr) } in positive; // Gated inclusion check. Is it supported?
+    // (1 - first) * (1 - last) * m_lastAccess { (m_addr' - m_addr) } in clk + 1; // Gated inclusion check. Is it supported?
 
     // TODO: following constraint
     // m_addr' == m_addr && m_clk == m_clk' ==> m_sub_clk' - m_sub_clk > 0
-    // Can be enforced with (1 - first) * (1 - last) * (1 - m_lastAccess) { 6 * (m_clk' - m_clk) + m_sub_clk' - m_sub_clk } in positive
+    // Can be enforced with (1 - first) * (1 - last) * (1 - m_lastAccess) { 6 * (m_clk' - m_clk) + m_sub_clk' - m_sub_clk } in clk + 1
 
     // Alternatively to the above, one could require 
     // that m_addr' - m_addr is 0 or 1 (needs to add placeholders m_addr values):

diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/generated/AvmMini_trace.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/generated/AvmMini_trace.cpp
@@ -66,32 +66,32 @@ void AvmMiniTraceBuilder::insertInMemTrace(uint32_t m_clk, uint32_t m_sub_clk, u
 
 void AvmMiniTraceBuilder::loadAInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 0, addr, val, false);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkLoadA, addr, val, false);
 }
 
 void AvmMiniTraceBuilder::loadBInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 1, addr, val, false);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkLoadB, addr, val, false);
 }
 
 void AvmMiniTraceBuilder::loadCInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 2, addr, val, false);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkLoadC, addr, val, false);
 }
 
 void AvmMiniTraceBuilder::storeAInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 3, addr, val, true);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkStoreA, addr, val, true);
 }
 
 void AvmMiniTraceBuilder::storeBInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 4, addr, val, true);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkStoreB, addr, val, true);
 }
 
 void AvmMiniTraceBuilder::storeCInMemTrace(uint32_t addr, FF val)
 {
-    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), 5, addr, val, true);
+    insertInMemTrace(static_cast<uint32_t>(mainTrace.size()), SubClkStoreC, addr, val, true);
 }
 
 // Addition over finite field with direct memory access.
@@ -146,7 +146,7 @@ void AvmMiniTraceBuilder::callDataCopy(uint32_t s0, uint32_t s1, uint32_t d0, st
     // This offset points to the first storing operation (pertaining to intermediate register Ia).
     // s0 + offset:       Ia memory store operation
     // s0 + offset + 1:   Ib memory store operation
-    // s0 + offset + 1:   Ic memory store operation
+    // s0 + offset + 2:   Ic memory store operation
 
     uint32_t offset = 0;
 
@@ -228,7 +228,7 @@ std::vector<FF> AvmMiniTraceBuilder::returnOP(uint32_t s0, uint32_t s1)
     // This offset points to the first loading operation (pertaining to intermediate register Ia).
     // s0 + offset:       Ia memory load operation
     // s0 + offset + 1:   Ib memory load operation
-    // s0 + offset + 1:   Ic memory load operation
+    // s0 + offset + 2:   Ic memory load operation
 
     uint32_t offset = 0;
     std::vector<FF> returnMem;

diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/generated/AvmMini_trace.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/generated/AvmMini_trace.hpp
@@ -24,6 +24,13 @@ class AvmMiniTraceBuilder {
     static const size_t N = 256;
     static const size_t MemSize = 1024;
 
+    static const uint32_t SubClkLoadA = 0;
+    static const uint32_t SubClkLoadB = 1;
+    static const uint32_t SubClkLoadC = 2;
+    static const uint32_t SubClkStoreA = 3;
+    static const uint32_t SubClkStoreB = 4;
+    static const uint32_t SubClkStoreC = 5;
+
     AvmMiniTraceBuilder();
 
     // Temporary helper to initialize memory.