-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Origin tags implemented in biggroup #10002
Changes from 6 commits
a5070a7
2fb14d6
1d8fa55
5568ad8
9845bfa
a5ed2dc
878ac0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
#include "../bit_array/bit_array.hpp" | ||
#include "../circuit_builders/circuit_builders.hpp" | ||
#include "barretenberg/stdlib/primitives/biggroup/biggroup.hpp" | ||
#include "barretenberg/transcript/origin_tag.hpp" | ||
|
||
namespace bb::stdlib::element_default { | ||
|
||
|
@@ -116,6 +117,8 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::operator+(const element& other) con | |
bool_ct result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity); | ||
result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); | ||
result.set_point_at_infinity(result_is_infinity); | ||
|
||
result.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); | ||
return result; | ||
} | ||
|
||
|
@@ -186,6 +189,7 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::operator-(const element& other) con | |
bool_ct result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity); | ||
result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity); | ||
result.set_point_at_infinity(result_is_infinity); | ||
result.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); | ||
return result; | ||
} | ||
|
||
|
@@ -749,6 +753,25 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::batch_mul(const std::vector<element | |
const bool with_edgecases) | ||
{ | ||
auto [points, scalars] = handle_points_at_infinity(_points, _scalars); | ||
OriginTag tag{}; | ||
const auto empty_tag = OriginTag(); | ||
|
||
// handle_points_at_infinity_method can remove some constant points, which messes with this code under | ||
// CircuitSimulator | ||
for (size_t i = 0; i < _points.size(); i++) { | ||
tag = OriginTag(tag, OriginTag(_points[i].get_origin_tag(), _scalars[i].get_origin_tag())); | ||
} | ||
for (size_t i = 0; i < scalars.size(); i++) { | ||
// If batch_mul actually performs batch multiplication on the points and scalars, subprocedures can do | ||
// operations like addition or subtraction of points, which can trigger OriginTag security mechanisms even | ||
// though the final result satisfies the security logic | ||
// For example result = submitted_in_round_0 *challenge_from_round_0 +submitted_in_round_1 * | ||
// challenge_in_round_1 will trigger it, because the addition of submitted_in_round_0 to submitted_in_round_1 is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't understand why this is dangerous There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are going to detect when two submitted values from 2 rounds meet without a challenge in the future |
||
// dangerous by itself. To avoid this, we remove the tags, merge them separately and set the result | ||
// appropriately | ||
points[i].set_origin_tag(empty_tag); | ||
scalars[i].set_origin_tag(empty_tag); | ||
} | ||
|
||
if constexpr (IsSimulator<C>) { | ||
// TODO(https://github.com/AztecProtocol/barretenberg/issues/663) | ||
|
@@ -760,7 +783,9 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::batch_mul(const std::vector<element | |
result += (element_t(points[i].get_value()) * scalars[i].get_value()); | ||
} | ||
result = result.normalize(); | ||
return from_witness(context, result); | ||
auto nonnative_result = from_witness(context, result); | ||
nonnative_result.set_origin_tag(tag); | ||
return nonnative_result; | ||
} else { | ||
// Perform goblinized batched mul if available; supported only for BN254 | ||
if (with_edgecases) { | ||
|
@@ -807,6 +832,7 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::batch_mul(const std::vector<element | |
} | ||
accumulator = accumulator - offset_generators.second; | ||
|
||
accumulator.set_origin_tag(tag); | ||
return accumulator; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,16 +160,20 @@ struct OriginTag { | |
OriginTag(OriginTag&& other) = default; | ||
OriginTag& operator=(const OriginTag& other) = default; | ||
OriginTag& operator=(OriginTag&& other) = default; | ||
~OriginTag() = default; | ||
|
||
OriginTag(size_t, size_t, bool is_submitted [[maybe_unused]] = true) {} | ||
OriginTag(size_t parent_index [[maybe_unused]], | ||
size_t child_index [[maybe_unused]], | ||
bool is_submitted [[maybe_unused]] = true) | ||
{} | ||
|
||
OriginTag(const OriginTag&, const OriginTag&) {} | ||
template <class... T> OriginTag(const OriginTag&, const T&...) {} | ||
bool operator==(const OriginTag& other) const; | ||
void poison() {} | ||
void unpoison() {} | ||
bool is_poisoned() const { return false; } | ||
bool is_empty() const { return true; }; | ||
static bool is_poisoned() { return false; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why was this changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I got some errors, but don't remember any more |
||
static bool is_empty() { return true; }; | ||
}; | ||
inline std::ostream& operator<<(std::ostream& os, OriginTag const&) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this merge function associative? could just do
OriginTag(tag_union, point.get_origin_tag(), scalar.get_origin_tag())
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not associative, we'd want to get a submitted_value*challenge, get its tag and then merge it with the previous one