Skip to content

Commit

Permalink
chore: deterministically deduplicate `cached_partial_non_native_field…
Browse files Browse the repository at this point in the history
…_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).
  • Loading branch information
kevaundray authored Nov 27, 2023
1 parent 4973cfb commit 5524933
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 5524933

Please sign in to comment.