Skip to content

Bind circuits of new proof aggregation flow to proper vk hashes #1524

Description

@cedoor

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactoringimproving a software's internal structure without changing its external behavior or functionalitysecurityRelevant to security

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions