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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion noir/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
21 changes: 2 additions & 19 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 @@ -223,16 +212,12 @@ impl BlackBoxFuncCall {
proof,
public_inputs,
key_hash,
..
} => {
let mut inputs = Vec::new();
inputs.extend(key.iter().copied());
inputs.extend(proof.iter().copied());
inputs.extend(public_inputs.iter().copied());
inputs.push(*key_hash);
// NOTE: we do not return an input aggregation object as it will either be non-existent for the first recursive aggregation
// or the output aggregation object of a previous recursive aggregation. We do not simulate recursive aggregation
// thus the input aggregation object will always be unassigned until proving
inputs
}
}
Expand All @@ -245,9 +230,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 +239,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 @@ -104,18 +104,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 entirely handled by the backend and is not solved by the ACVM
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
9 changes: 2 additions & 7 deletions noir/noir_stdlib/src/lib.nr
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,8 @@ unconstrained pub fn println<T>(input: T) {
}

#[foreign(recursive_aggregation)]
pub fn verify_proof<N>(
_verification_key: [Field],
_proof: [Field],
_public_inputs: [Field],
_key_hash: Field,
_input_aggregation_object: [Field; N]
) -> [Field; N] {}
pub fn verify_proof<N>(_verification_key: [Field], _proof: [Field], _public_inputs: [Field], _key_hash: Field) {}

// Asserts that the given value is known at compile-time.
// Useful for debugging for-loop bounds.
#[builtin(assert_constant)]
Expand Down
Loading