-
Notifications
You must be signed in to change notification settings - Fork 270
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: Poseidon2 gates for Ultra arithmetisation #7494
Changes from 16 commits
b1518fe
49eca12
a5ccefa
1d86341
bb178f0
269da29
f1edf5f
4b98f77
5058fa0
bf37f09
b116e00
e688422
3488ac4
83ecd92
2bef500
42bae7d
191b587
c7b0005
376a736
798cf19
148b8fc
e91dfd8
42f472e
e21b3e4
0a16e60
7144552
65e9613
a659abf
43241d1
c03a0c0
71ecc37
b3edc4b
ed7a9eb
d01ff27
aae0648
b6e27b9
03ca5fa
dc486a0
b56cb85
002d88e
21cfb25
05b3061
465912c
11592c9
e74d3f7
c60ac06
166a8e4
698f3fe
0fb6a7a
1054f9e
ce890b2
8eaf4a4
8b18801
2ad9fd3
05ad521
6dd0949
524f88b
4af936b
843368c
edefb2e
5fe0a4d
3630ef2
333cf11
61b4789
c959fcb
b251dc7
50041c8
1992e8e
6406a6f
3eb0054
b2872d5
75a5abc
59d79b8
96e9e31
b7b0248
25f4264
a8f5fdb
f6521c6
a2544bc
d355e00
a3a2454
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
barretenberg_module(commitment_schemes_recursion commitment_schemes stdlib_primitives) | ||
barretenberg_module(commitment_schemes_recursion commitment_schemes stdlib_poseidon2) | ||
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 do we need stdlib poseidon and not primitives for commitment schemes? 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. the module doesnt link without poseidon now that it's enabled and stdlib_poseidon2 already includes stdlib_primitives |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,10 +77,4 @@ template <typename FF, size_t NUM_WIRES, size_t NUM_SELECTORS> class ExecutionTr | |
void set_fixed_size(uint32_t size_in) { fixed_size = size_in; } | ||
}; | ||
|
||
class TranslatorArith { | ||
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. This could just be moved to Translator, randomly stumbled across it |
||
public: | ||
static constexpr size_t NUM_WIRES = 81; | ||
static constexpr size_t NUM_SELECTORS = 0; | ||
}; | ||
|
||
} // namespace bb |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
|
||
# np.set_printoptions(formatter={'int': hex}) | ||
|
||
EXTENDED_RELATION_LENGTH = 13 | ||
EXTENDED_RELATION_LENGTH = 12 | ||
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. interesting that this decreased 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. A prior line of work decreased it (using different etas rather than powers of eta) but because nothing in the logic changed, that PR could just remove one value in the combiner tests and this file wasnt modified. Now I had to rerun the script and realised we have one extra eval, but my PR didnt specifically decreased this |
||
|
||
class Row: | ||
# Construct a set of 'all' polynomials with a very simple structure | ||
|
@@ -20,38 +20,40 @@ def __init__(self, base_poly): | |
self.q_elliptic = base_poly + 2 * 8 | ||
self.q_aux = base_poly + 2 * 9 | ||
self.q_lookup = base_poly + 2 * 10 | ||
self.sigma_1 = base_poly + 2 * 11 | ||
self.sigma_2 = base_poly + 2 * 12 | ||
self.sigma_3 = base_poly + 2 * 13 | ||
self.sigma_4 = base_poly + 2 * 14 | ||
self.id_1 = base_poly + 2 * 15 | ||
self.id_2 = base_poly + 2 * 16 | ||
self.id_3 = base_poly + 2 * 17 | ||
self.id_4 = base_poly + 2 * 18 | ||
self.table_1 = base_poly + 2 * 19 | ||
self.table_2 = base_poly + 2 * 20 | ||
self.table_3 = base_poly + 2 * 21 | ||
self.table_4 = base_poly + 2 * 22 | ||
self.lagrange_first = base_poly + 2 * 23 | ||
self.lagrange_last = base_poly + 2 * 24 | ||
self.w_l = base_poly + 2 * 25 | ||
self.w_r = base_poly + 2 * 26 | ||
self.w_o = base_poly + 2 * 27 | ||
self.w_4 = base_poly + 2 * 28 | ||
self.sorted_accum = base_poly + 2 * 29 | ||
self.z_perm = base_poly + 2 * 30 | ||
self.z_lookup = base_poly + 2 * 31 | ||
self.table_1_shift = base_poly + 2 * 32 | ||
self.table_2_shift = base_poly + 2 * 33 | ||
self.table_3_shift = base_poly + 2 * 34 | ||
self.table_4_shift = base_poly + 2 * 35 | ||
self.w_l_shift = base_poly + 2 * 36 | ||
self.w_r_shift = base_poly + 2 * 37 | ||
self.w_o_shift = base_poly + 2 * 38 | ||
self.w_4_shift = base_poly + 2 * 39 | ||
self.sorted_accum_shift = base_poly + 2 * 40 | ||
self.z_perm_shift = base_poly + 2 * 41 | ||
self.z_lookup_shift = base_poly + 2 * 42 | ||
self.q_poseidon2_external_1 = base_poly + 2 * 11 | ||
self.q_poseidon2_external_2 = base_poly + 2 * 12 | ||
self.sigma_1 = base_poly + 2 * 13 | ||
self.sigma_2 = base_poly + 2 * 14 | ||
self.sigma_3 = base_poly + 2 * 15 | ||
self.sigma_4 = base_poly + 2 * 16 | ||
self.id_1 = base_poly + 2 * 17 | ||
self.id_2 = base_poly + 2 * 18 | ||
self.id_3 = base_poly + 2 * 19 | ||
self.id_4 = base_poly + 2 * 20 | ||
self.table_1 = base_poly + 2 * 21 | ||
self.table_2 = base_poly + 2 * 22 | ||
self.table_3 = base_poly + 2 * 23 | ||
self.table_4 = base_poly + 2 * 24 | ||
self.lagrange_first = base_poly + 2 * 25 | ||
self.lagrange_last = base_poly + 2 * 26 | ||
self.w_l = base_poly + 2 * 27 | ||
self.w_r = base_poly + 2 * 28 | ||
self.w_o = base_poly + 2 * 29 | ||
self.w_4 = base_poly + 2 * 30 | ||
self.sorted_accum = base_poly + 2 * 31 | ||
self.z_perm = base_poly + 2 * 32 | ||
self.z_lookup = base_poly + 2 * 33 | ||
self.table_1_shift = base_poly + 2 * 34 | ||
self.table_2_shift = base_poly + 2 * 35 | ||
self.table_3_shift = base_poly + 2 * 36 | ||
self.table_4_shift = base_poly + 2 * 37 | ||
self.w_l_shift = base_poly + 2 * 38 | ||
self.w_r_shift = base_poly + 2 * 39 | ||
self.w_o_shift = base_poly + 2 * 40 | ||
self.w_4_shift = base_poly + 2 * 41 | ||
self.sorted_accum_shift = base_poly + 2 * 42 | ||
self.z_perm_shift = base_poly + 2 * 43 | ||
self.z_lookup_shift = base_poly + 2 * 44 | ||
|
||
def arith_relation(self): | ||
return self.q_m * self.w_l * self.w_r + self.q_l * self.w_l + self.q_r * self.w_r + self.q_o * self.w_o + self.q_c | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,11 +88,12 @@ TEST_F(ClientIVCRecursionTests, Basic) | |
// Generate the recursive verification circuit | ||
verifier.verify(proof); | ||
|
||
info("Recursive Verifier: num gates = ", builder->num_gates); | ||
|
||
EXPECT_EQ(builder->failed(), false) << builder->err(); | ||
|
||
EXPECT_TRUE(CircuitChecker::check(*builder)); | ||
|
||
// Print the number of gates post finalisation | ||
info("Recursive Verifier: num gates = ", builder->num_gates); | ||
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. maybe could have both actually and mark which one is pre/post finalization |
||
} | ||
|
||
} // namespace bb::stdlib::recursion::honk |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,16 +129,14 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve | |
// multivariate evaluations at u | ||
const size_t log_circuit_size = numeric::get_msb(static_cast<uint32_t>(key->circuit_size)); | ||
auto sumcheck = Sumcheck(log_circuit_size, transcript); | ||
|
||
RelationSeparator alpha; | ||
for (size_t idx = 0; idx < alpha.size(); idx++) { | ||
alpha[idx] = transcript->template get_challenge<FF>("alpha_" + std::to_string(idx)); | ||
} | ||
|
||
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1041): Once hashing produces constraints for Ultra in | ||
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. put this in the PR description 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'm not opposed to sprinkling TODOs in the code FWIW it helps keep them top of mind (as long as they link an issue that can then be used for cleanup) 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. this TODO is supposed to be resolved by this PR |
||
// the transcript, a fixed number of gate_challenges must be generated by the prover/verifier in order to achieve a | ||
// verification circuit that is independent of proof size. | ||
auto gate_challenges = std::vector<FF>(log_circuit_size); | ||
for (size_t idx = 0; idx < log_circuit_size; idx++) { | ||
auto gate_challenges = std::vector<FF>(CONST_PROOF_SIZE_LOG_N); | ||
for (size_t idx = 0; idx < CONST_PROOF_SIZE_LOG_N; idx++) { | ||
gate_challenges[idx] = transcript->template get_challenge<FF>("Sumcheck:gate_challenge_" + std::to_string(idx)); | ||
} | ||
auto [multivariate_challenge, claimed_evaluations, sumcheck_verified] = | ||
|
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.
remove?
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.
Erm I guess it's good info generally when running e2e tests
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.
note that it calls get_num_gates() and not builder.num_gates. As a sidenote, maybe we can change the name of get_num_gates() to estimate_num_finalized_gates()