From 55249336212764da4b85634e7d35e8fedb147619 Mon Sep 17 00:00:00 2001
From: kevaundray <kevtheappdev@gmail.com>
Date: Mon, 27 Nov 2023 22:36:24 +0000
Subject: [PATCH] chore: deterministically deduplicate
 `cached_partial_non_native_field_multiplication` across wasm32 and native
 compilations (#3425)

This essentially fixes the implementation of < on the struct in question

Read comment below for more context.

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [ ] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [ ] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).
---
 .../circuit_builder/ultra_circuit_builder.cpp | 12 +----
 .../circuit_builder/ultra_circuit_builder.hpp | 51 +++++++++++++++++--
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp
index e51cacc67d4..047e3e8489e 100644
--- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp
+++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp
@@ -1665,17 +1665,10 @@ void UltraCircuitBuilder_<Arithmetization>::process_non_native_field_multiplicat
             c.b[j] = this->real_variable_index[c.b[j]];
         }
     }
-    std::sort(cached_partial_non_native_field_multiplications.begin(),
-              cached_partial_non_native_field_multiplications.end());
-
-    auto last = std::unique(cached_partial_non_native_field_multiplications.begin(),
-                            cached_partial_non_native_field_multiplications.end());
-
-    auto it = cached_partial_non_native_field_multiplications.begin();
+    cached_partial_non_native_field_multiplication::deduplicate(cached_partial_non_native_field_multiplications);
 
     // iterate over the cached items and create constraints
-    while (it != last) {
-        const auto input = *it;
+    for (const auto& input : cached_partial_non_native_field_multiplications) {
 
         w_l.emplace_back(input.a[1]);
         w_r.emplace_back(input.b[1]);
@@ -1701,7 +1694,6 @@ void UltraCircuitBuilder_<Arithmetization>::process_non_native_field_multiplicat
         w_4.emplace_back(input.hi_1);
         apply_aux_selectors(AUX_SELECTORS::NONE);
         ++this->num_gates;
-        ++it;
     }
 }
 
diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
index 25c35737a12..a8186c477dc 100644
--- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
+++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
@@ -206,18 +206,59 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization:
             return valid;
         }
 
+        static void deduplicate(std::vector<cached_partial_non_native_field_multiplication>& vec)
+        {
+            std::unordered_set<cached_partial_non_native_field_multiplication, Hash, std::equal_to<>> seen;
+
+            std::vector<cached_partial_non_native_field_multiplication> uniqueVec;
+
+            for (const auto& item : vec) {
+                if (seen.insert(item).second) {
+                    uniqueVec.push_back(item);
+                }
+            }
+
+            vec.swap(uniqueVec);
+        }
+
         bool operator<(const cached_partial_non_native_field_multiplication& other) const
         {
             if (a < other.a) {
                 return true;
             }
-            if (a == other.a) {
-                if (b < other.b) {
-                    return true;
-                }
+            if (other.a < a) {
+                return false;
+            }
+            if (b < other.b) {
+                return true;
             }
-            return false;
+            return other.b < b;
         }
+
+        struct Hash {
+            size_t operator()(const cached_partial_non_native_field_multiplication& obj) const
+            {
+                size_t combined_hash = 0;
+
+                // C++ does not have a standard way to hash values, so we use the
+                // common algorithm that boot uses.
+                // You can search for 'cpp hash_combine' to find more information.
+                // Here is one reference:
+                // https://stackoverflow.com/questions/2590677/how-do-i-combine-hash-values-in-c0x
+                auto hash_combiner = [](size_t lhs, size_t rhs) {
+                    return lhs ^ (rhs + 0x9e3779b9 + (lhs << 6) + (lhs >> 2));
+                };
+
+                for (const auto& elem : obj.a) {
+                    combined_hash = hash_combiner(combined_hash, std::hash<uint32_t>()(elem));
+                }
+                for (const auto& elem : obj.b) {
+                    combined_hash = hash_combiner(combined_hash, std::hash<uint32_t>()(elem));
+                }
+
+                return combined_hash;
+            }
+        };
     };
 
     struct non_native_field_multiplication_cross_terms {