Skip to content
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

chore!: Remove aggregation objects from RecursionConstraint #3885

Merged
merged 9 commits into from
Jan 8, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,7 @@ void handle_blackbox_func_call(Circuit::Opcode::BlackBoxFuncCall const& arg, aci
.proof = map(arg.proof, [](auto& e) { return e.witness.value; }),
.public_inputs = map(arg.public_inputs, [](auto& e) { return e.witness.value; }),
.key_hash = arg.key_hash.witness.value,
.input_aggregation_object = {},
.output_aggregation_object = {},
.nested_aggregation_object = {},
};
if (arg.input_aggregation_object.has_value()) {
for (size_t i = 0; i < RecursionConstraint::AGGREGATION_OBJECT_SIZE; ++i) {
c.input_aggregation_object[i] = (*arg.input_aggregation_object)[i].witness.value;
}
}
for (size_t i = 0; i < RecursionConstraint::AGGREGATION_OBJECT_SIZE; ++i) {
c.output_aggregation_object[i] = arg.output_aggregation_object[i].value;
}
af.recursion_constraints.push_back(c);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,6 @@ struct RecursionConstraint {
std::vector<uint32_t> proof;
std::vector<uint32_t> public_inputs;
uint32_t key_hash;
// TODO(maxim):This is now unused, but we keep it here for backwards compatibility
std::array<uint32_t, AGGREGATION_OBJECT_SIZE> input_aggregation_object;
// TODO(maxim): This is now unused, but we keep it here for backwards compatibility
std::array<uint32_t, AGGREGATION_OBJECT_SIZE> output_aggregation_object;
// TODO(maxim): This is currently not being used on the Noir level at all,
// TODO(maxim): but we keep it here for backwards compatibility
// TODO(maxim): The object is now currently contained by the `proof` field
// TODO(maxim): and is handled when serializing ACIR to a barretenberg circuit
std::array<uint32_t, AGGREGATION_OBJECT_SIZE> nested_aggregation_object;

friend bool operator==(RecursionConstraint const& lhs, RecursionConstraint const& rhs) = default;
};
Expand Down Expand Up @@ -100,9 +91,6 @@ template <typename B> inline void read(B& buf, RecursionConstraint& constraint)
read(buf, constraint.proof);
read(buf, constraint.public_inputs);
read(buf, constraint.key_hash);
read(buf, constraint.input_aggregation_object);
read(buf, constraint.output_aggregation_object);
read(buf, constraint.nested_aggregation_object);
}

template <typename B> inline void write(B& buf, RecursionConstraint const& constraint)
Expand All @@ -112,9 +100,6 @@ template <typename B> inline void write(B& buf, RecursionConstraint const& const
write(buf, constraint.proof);
write(buf, constraint.public_inputs);
write(buf, constraint.key_hash);
write(buf, constraint.input_aggregation_object);
write(buf, constraint.output_aggregation_object);
write(buf, constraint.nested_aggregation_object);
}

} // namespace acir_format
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@ Builder create_outer_circuit(std::vector<Builder>& inner_circuits)
.proof = proof_indices,
.public_inputs = inner_public_inputs,
.key_hash = key_hash_start_idx,
.input_aggregation_object = input_aggregation_object,
.output_aggregation_object = output_aggregation_object,
.nested_aggregation_object = nested_aggregation_object,
};
recursion_constraints.push_back(recursion_constraint);

Expand Down
14 changes: 0 additions & 14 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ struct BlackBoxFuncCall {
std::vector<Circuit::FunctionInput> proof;
std::vector<Circuit::FunctionInput> public_inputs;
Circuit::FunctionInput key_hash;
std::optional<std::vector<Circuit::FunctionInput>> input_aggregation_object;
std::vector<Circuit::Witness> output_aggregation_object;

friend bool operator==(const RecursiveAggregation&, const RecursiveAggregation&);
std::vector<uint8_t> bincodeSerialize() const;
Expand Down Expand Up @@ -2659,12 +2657,6 @@ inline bool operator==(const BlackBoxFuncCall::RecursiveAggregation& lhs,
if (!(lhs.key_hash == rhs.key_hash)) {
return false;
}
if (!(lhs.input_aggregation_object == rhs.input_aggregation_object)) {
return false;
}
if (!(lhs.output_aggregation_object == rhs.output_aggregation_object)) {
return false;
}
return true;
}

Expand Down Expand Up @@ -2697,8 +2689,6 @@ void serde::Serializable<Circuit::BlackBoxFuncCall::RecursiveAggregation>::seria
serde::Serializable<decltype(obj.proof)>::serialize(obj.proof, serializer);
serde::Serializable<decltype(obj.public_inputs)>::serialize(obj.public_inputs, serializer);
serde::Serializable<decltype(obj.key_hash)>::serialize(obj.key_hash, serializer);
serde::Serializable<decltype(obj.input_aggregation_object)>::serialize(obj.input_aggregation_object, serializer);
serde::Serializable<decltype(obj.output_aggregation_object)>::serialize(obj.output_aggregation_object, serializer);
}

template <>
Expand All @@ -2711,10 +2701,6 @@ Circuit::BlackBoxFuncCall::RecursiveAggregation serde::Deserializable<
obj.proof = serde::Deserializable<decltype(obj.proof)>::deserialize(deserializer);
obj.public_inputs = serde::Deserializable<decltype(obj.public_inputs)>::deserialize(deserializer);
obj.key_hash = serde::Deserializable<decltype(obj.key_hash)>::deserialize(deserializer);
obj.input_aggregation_object =
serde::Deserializable<decltype(obj.input_aggregation_object)>::deserialize(deserializer);
obj.output_aggregation_object =
serde::Deserializable<decltype(obj.output_aggregation_object)>::deserialize(deserializer);
return obj;
}

Expand Down
8 changes: 0 additions & 8 deletions noir/acvm-repo/acir/codegen/acir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ namespace Circuit {
std::vector<Circuit::FunctionInput> proof;
std::vector<Circuit::FunctionInput> public_inputs;
Circuit::FunctionInput key_hash;
std::optional<std::vector<Circuit::FunctionInput>> input_aggregation_object;
std::vector<Circuit::Witness> output_aggregation_object;

friend bool operator==(const RecursiveAggregation&, const RecursiveAggregation&);
std::vector<uint8_t> bincodeSerialize() const;
Expand Down Expand Up @@ -2295,8 +2293,6 @@ namespace Circuit {
if (!(lhs.proof == rhs.proof)) { return false; }
if (!(lhs.public_inputs == rhs.public_inputs)) { return false; }
if (!(lhs.key_hash == rhs.key_hash)) { return false; }
if (!(lhs.input_aggregation_object == rhs.input_aggregation_object)) { return false; }
if (!(lhs.output_aggregation_object == rhs.output_aggregation_object)) { return false; }
return true;
}

Expand Down Expand Up @@ -2324,8 +2320,6 @@ void serde::Serializable<Circuit::BlackBoxFuncCall::RecursiveAggregation>::seria
serde::Serializable<decltype(obj.proof)>::serialize(obj.proof, serializer);
serde::Serializable<decltype(obj.public_inputs)>::serialize(obj.public_inputs, serializer);
serde::Serializable<decltype(obj.key_hash)>::serialize(obj.key_hash, serializer);
serde::Serializable<decltype(obj.input_aggregation_object)>::serialize(obj.input_aggregation_object, serializer);
serde::Serializable<decltype(obj.output_aggregation_object)>::serialize(obj.output_aggregation_object, serializer);
}

template <>
Expand All @@ -2336,8 +2330,6 @@ Circuit::BlackBoxFuncCall::RecursiveAggregation serde::Deserializable<Circuit::B
obj.proof = serde::Deserializable<decltype(obj.proof)>::deserialize(deserializer);
obj.public_inputs = serde::Deserializable<decltype(obj.public_inputs)>::deserialize(deserializer);
obj.key_hash = serde::Deserializable<decltype(obj.key_hash)>::deserialize(deserializer);
obj.input_aggregation_object = serde::Deserializable<decltype(obj.input_aggregation_object)>::deserialize(deserializer);
obj.output_aggregation_object = serde::Deserializable<decltype(obj.output_aggregation_object)>::deserialize(deserializer);
return obj;
}

Expand Down
17 changes: 2 additions & 15 deletions noir/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,6 @@ pub enum BlackBoxFuncCall {
/// The circuit implementing this opcode can use this hash to ensure that the
/// key provided to the circuit matches the key produced by the circuit creator
key_hash: FunctionInput,
/// An aggregation object is blob of data that the top-level verifier must run some proof system specific
/// algorithm on to complete verification. The size is proof system specific and will be set by the backend integrating this opcode.
/// The input aggregation object is only not `None` when we are verifying a previous recursive aggregation in
/// the current circuit. If this is the first recursive aggregation there is no input aggregation object.
/// It is left to the backend to determine how to handle when there is no input aggregation object.
input_aggregation_object: Option<Vec<FunctionInput>>,
/// This is the result of a recursive aggregation and is what will be fed into the next verifier.
/// The next verifier can either perform a final verification (returning true or false)
/// or perform another recursive aggregation where this output aggregation object
/// will be the input aggregation object of the next recursive aggregation.
output_aggregation_object: Vec<Witness>,
},
}

Expand Down Expand Up @@ -245,9 +234,7 @@ impl BlackBoxFuncCall {
| BlackBoxFuncCall::Blake3 { outputs, .. }
| BlackBoxFuncCall::Keccak256 { outputs, .. }
| BlackBoxFuncCall::Keccakf1600 { outputs, .. }
| BlackBoxFuncCall::RecursiveAggregation {
output_aggregation_object: outputs, ..
} => outputs.to_vec(),
=> outputs.to_vec(),
BlackBoxFuncCall::AND { output, .. }
| BlackBoxFuncCall::XOR { output, .. }
| BlackBoxFuncCall::SchnorrVerify { output, .. }
Expand All @@ -256,7 +243,7 @@ impl BlackBoxFuncCall {
| BlackBoxFuncCall::EcdsaSecp256r1 { output, .. } => vec![*output],
BlackBoxFuncCall::FixedBaseScalarMul { outputs, .. }
| BlackBoxFuncCall::PedersenCommitment { outputs, .. } => vec![outputs.0, outputs.1],
BlackBoxFuncCall::RANGE { .. } => vec![],
BlackBoxFuncCall::RANGE { .. } | BlackBoxFuncCall::RecursiveAggregation { .. } => vec![],
BlackBoxFuncCall::Keccak256VariableLength { outputs, .. } => outputs.to_vec(),
}
}
Expand Down
6 changes: 1 addition & 5 deletions noir/acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,14 @@ pub(super) fn transform_internal(
| acir::circuit::opcodes::BlackBoxFuncCall::XOR { output, .. } => {
transformer.mark_solvable(*output);
}
acir::circuit::opcodes::BlackBoxFuncCall::RANGE { .. } => (),
acir::circuit::opcodes::BlackBoxFuncCall::RANGE { .. } | acir::circuit::opcodes::BlackBoxFuncCall::RecursiveAggregation { .. } => (),
acir::circuit::opcodes::BlackBoxFuncCall::SHA256 { outputs, .. }
| acir::circuit::opcodes::BlackBoxFuncCall::Keccak256 { outputs, .. }
| acir::circuit::opcodes::BlackBoxFuncCall::Keccak256VariableLength {
outputs,
..
}
| acir::circuit::opcodes::BlackBoxFuncCall::Keccakf1600 { outputs, .. }
| acir::circuit::opcodes::BlackBoxFuncCall::RecursiveAggregation {
output_aggregation_object: outputs,
..
}
| acir::circuit::opcodes::BlackBoxFuncCall::Blake2s { outputs, .. }
| acir::circuit::opcodes::BlackBoxFuncCall::Blake3 { outputs, .. } => {
for witness in outputs {
Expand Down
10 changes: 2 additions & 8 deletions noir/acvm-repo/acvm/src/pwg/blackbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,7 @@ pub(crate) fn solve(
BlackBoxFuncCall::FixedBaseScalarMul { low, high, outputs } => {
fixed_base_scalar_mul(backend, initial_witness, *low, *high, *outputs)
}
BlackBoxFuncCall::RecursiveAggregation { output_aggregation_object, .. } => {
// Solve the output of the recursive aggregation to zero to prevent missing assignment errors
// The correct value will be computed by the backend
for witness in output_aggregation_object {
insert_value(witness, FieldElement::zero(), initial_witness)?;
}
Ok(())
}
// Recursive aggregation will be entirel handled by the backend and is not solved by the ACVM
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
BlackBoxFuncCall::RecursiveAggregation { .. } => Ok(()),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -227,23 +227,11 @@ impl GeneratedAcir {
BlackBoxFuncCall::Keccakf1600 { inputs: inputs[0].clone(), outputs }
}
BlackBoxFunc::RecursiveAggregation => {
let has_previous_aggregation = self.opcodes.iter().any(|op| {
matches!(
op,
AcirOpcode::BlackBoxFuncCall(BlackBoxFuncCall::RecursiveAggregation { .. })
)
});

let input_aggregation_object =
if !has_previous_aggregation { None } else { Some(inputs[4].clone()) };

BlackBoxFuncCall::RecursiveAggregation {
verification_key: inputs[0].clone(),
proof: inputs[1].clone(),
public_inputs: inputs[2].clone(),
key_hash: inputs[3][0],
input_aggregation_object,
output_aggregation_object: outputs,
}
}
};
Expand Down