Follow-up from #1516 (refactor: new proof aggregation).
The circuits introduced in the new proof aggregation flow need to be bound to their proper verification key (vk) hashes to ensure correctness and security of the aggregation pipeline.
Context
The dkg_aggregator circuit (circuits/bin/recursive_aggregation/dkg_aggregator/src/main.nr) takes nodes_fold_key_hash and c5_key_hash as public inputs, passes them through compute_vk_hash, and returns the result as key_hash — the first element of its public output tuple:
fn main(...) -> pub (Field, [Field; H], [Field; H], Field)
// ^^^^^ key_hash ... aggregated_pk_commit
The on-chain verifier BfvPkVerifier.verify() (packages/enclave-contracts/contracts/verifiers/bfv/BfvPkVerifier.sol) currently only checks the last public input:
if (publicInputs[publicInputs.length - 1] != pkCommitment) {
return false;
}
return circuitVerifier.verify(rawProof, publicInputs);
key_hash is present in publicInputs but never validated against a canonical expected value. This means a malicious aggregator could, in principle, substitute a different nodes_fold_vk (e.g. a weaker or hand-crafted circuit) and produce a valid outer proof that the contract accepts — as long as the aggregated_pk_commit at the end is consistent.
Proposed fix
Store the canonical key_hash (derived from the known nodes_fold and c5 circuit VKs via compute_vk_hash) as an immutable in BfvPkVerifier and check it explicitly:
contract BfvPkVerifier is IPkVerifier {
ICircuitVerifier public immutable circuitVerifier;
bytes32 public immutable expectedKeyHash;
constructor(address _circuitVerifier, bytes32 _expectedKeyHash) {
circuitVerifier = ICircuitVerifier(_circuitVerifier);
expectedKeyHash = _expectedKeyHash;
}
function verify(bytes32 pkCommitment, bytes calldata proof)
external view override returns (bool)
{
(bytes memory rawProof, bytes32[] memory publicInputs) =
abi.decode(proof, (bytes, bytes32[]));
if (publicInputs.length == 0) return false;
// Pin sub-circuit VKs: publicInputs[0] is key_hash from dkg_aggregator
if (bytes32(publicInputs[0]) != expectedKeyHash) return false;
// Pin the aggregated pk commitment
if (publicInputs[publicInputs.length - 1] != pkCommitment) return false;
return circuitVerifier.verify(rawProof, publicInputs);
}
}
expectedKeyHash is computed at deploy time using the same compute_vk_hash([nodes_fold_vk_hash, c5_vk_hash]) formula the circuit uses. When circuits are upgraded, the outer DkgAggregatorVerifier (Honk verifier) is already redeployed — BfvPkVerifier redeploys alongside it with the new hash, so there's no additional operational burden.
The same pattern applies to BfvDecryptionVerifier for the decryption aggregation path.
Follow-up from #1516 (refactor: new proof aggregation).
The circuits introduced in the new proof aggregation flow need to be bound to their proper verification key (vk) hashes to ensure correctness and security of the aggregation pipeline.
Context
The
dkg_aggregatorcircuit (circuits/bin/recursive_aggregation/dkg_aggregator/src/main.nr) takesnodes_fold_key_hashandc5_key_hashas public inputs, passes them throughcompute_vk_hash, and returns the result askey_hash— the first element of its public output tuple:The on-chain verifier
BfvPkVerifier.verify()(packages/enclave-contracts/contracts/verifiers/bfv/BfvPkVerifier.sol) currently only checks the last public input:key_hashis present inpublicInputsbut never validated against a canonical expected value. This means a malicious aggregator could, in principle, substitute a differentnodes_fold_vk(e.g. a weaker or hand-crafted circuit) and produce a valid outer proof that the contract accepts — as long as theaggregated_pk_commitat the end is consistent.Proposed fix
Store the canonical
key_hash(derived from the knownnodes_foldandc5circuit VKs viacompute_vk_hash) as an immutable inBfvPkVerifierand check it explicitly:expectedKeyHashis computed at deploy time using the samecompute_vk_hash([nodes_fold_vk_hash, c5_vk_hash])formula the circuit uses. When circuits are upgraded, the outerDkgAggregatorVerifier(Honk verifier) is already redeployed —BfvPkVerifierredeploys alongside it with the new hash, so there's no additional operational burden.The same pattern applies to
BfvDecryptionVerifierfor the decryption aggregation path.