Skip to content

Commit

Permalink
chore!: Remove aggregation objects from RecursionConstraint (#3885)
Browse files Browse the repository at this point in the history
This removes the aggregation objects which are currently unused in the
RecursionConstraint implementation. Next we will update the ACVM opcode
to no longer use the aggregation object fields and update the
serialization.

# 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).

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
vezenovm and kevaundray committed Jan 8, 2024
1 parent 0dfc32f commit 9a80008
Show file tree
Hide file tree
Showing 13 changed files with 12 additions and 120 deletions.
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

0 comments on commit 9a80008

Please sign in to comment.