aligned_layer icon indicating copy to clipboard operation
aligned_layer copied to clipboard

feat(aggregation-mode): proving system id for proof commitments

Open MarcosNicolau opened this issue 1 month ago • 2 comments

Description

This prs adds the proving system id as part of the proof commitments. This is to avoid a confusion between leaf hashes (proof commitments) and compression hashes (authenticity paths) when verifying that proofs have been indeed verified by the aggregation program. This also fixes an special case were a proof for one proving system could be miss identified as being verified by another one.

As stated by 3MI audit:

The leaf hash (as computed in e.g. sp1/lib.rs line 12, ref. Finding 9) can be confused for a compression hash (e.g. line 51) when the public input consists of 32 bytes. Since no guarantees on the depth of the tree are made, and no check on the length of the authentication path are performed in the verification of a batch, this leads to internal nodes of the merkle tree being considered as proven/verified program IDs.

Suggestion: Provide clear domain separation between leaf and compression hashes in the merkle tree.

How to test

  1. Start ethereum package:
make ethereum_package_start
  1. Start batcher:
make batcher_start_ethereum_package
  1. Send SP1 and Risc0 proofs:
make batcher_send_sp1_burst BURST_SIZE=1
make batcher_send_risc0_burst BURST_SIZE=1
  1. Run proof aggregator for SP1 and RISC0:
make proof_aggregator_start AGGREGATOR=sp1
make proof_aggregator_start AGGREGATOR=risc0
  1. Verify the proofs have been aggregated:
make verify_aggregated_proof_sp1 FROM_BLOCK=0
make verify_aggregated_proof_risc0 FROM_BLOCK=0

Note: you might need to wait until the fulu fork gets activated on ethereum package (on epoch = 1), you can inspect that with:

curl -X 'GET' \
  'http://localhost:58801/eth/v1/beacon/states/head/fork' \
  -H 'accept: application/json'

Test L2 example

Follow the instructions here.

Type of change

Please delete options that are not relevant.

  • [ ] New feature
  • [ ] Bug fix
  • [ ] Optimization
  • [ ] Refactor

Checklist

  • [ ] “Hotfix” to testnet, everything else to staging
  • [ ] Linked to Github Issue
  • [ ] This change depends on code or research by an external entity
    • [ ] Acknowledgements were updated to give credit
  • [ ] Unit tests added
  • [ ] This change requires new documentation.
    • [ ] Documentation has been added/updated.
  • [ ] This change is an Optimization
    • [ ] Benchmarks added/run
  • [ ] Has a known issue
    • Link to the open issue addressing it
  • [ ] If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • [ ] This PR adds compatibility for operator for both versions and do not change crates/docs/examples
    • [ ] This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

MarcosNicolau avatar Nov 25 '25 13:11 MarcosNicolau

Add IDs to interface constants Make proving system u16

I've added the proving system as a u16. I don't think having an enum in the function is the right approach, since that would require:

  1. Upgrading the contract when we want a new entry in the enum
  2. Enums are defined as u8s, so if we make the proving systems ids uint16 then we can't really use enums.
  3. We could add as many verifiers as we want (different version of sp1 for example) off-chain without having to change anything in the contract
  4. It can be ambigous to call the function via solidity (one would need to import the interface which isn't very confrotable as it would mean cloning the aligned repo and adding it as a dep in foundry)

I admit enums are nicer for ux, but I feel they are not essential here. The IDs are already documented in comments, and the rust sdk explains them clearly.

MarcosNicolau avatar Nov 27 '25 17:11 MarcosNicolau