penumbra-zone / penumbra

Penumbra is a fully private proof-of-stake network and decentralized exchange for the Cosmos ecosystem.
https://penumbra.zone
Apache License 2.0
376 stars 294 forks source link

Insufficient randomization in swap balance commitment #4482

Open TalDerei opened 4 months ago

TalDerei commented 4 months ago

The swap balance commitment cv is not sufficiently hiding in relation to the circuit balance commitment integrity check since (1) cv is deterministically derived from the fee commitment cvf, and (2) pedersen commitments are additively homomorphic. Combining these facts would enable the search space for the private asset values v1 and v2, after Sealed-Bid Batch Swaps are implemented, to be brute forced. The balance commitment needs to be derived with a new blinding factor to break this determinism.

This references component A1 in the ECC audit log, and A2 still needs to be done.

cc @redshiftzero

TalDerei commented 4 months ago

@redshiftzero erwan mentioned offline that this issue, in addition to describing the high-level goal, should be fleshed in greater detail to include:

redshiftzero commented 4 months ago

The suggested remediation here is to use an additional blinding factor $v_i$ for cv in swaps which we'd witness in the SwapCircuit. I was thinking we'd derive the balance commitment as:

$cv = [−v_1​] G_1​+[−v_2​] G_2​ + [vi] G{vi} + c{v_f}$

(compared to now where we do $cv = [−v_1​] G_1​+[−v_2​] G2​ + c{v_f}$)

$G_{v_i}$ would be a new constant generator point we'd want to specify

hdevalence commented 4 months ago

I don’t understand why we need a new generator here?

TalDerei commented 4 months ago

I don’t understand why we need a new generator here?

https://github.com/penumbra-zone/penumbra/pull/4481#issuecomment-2138341101

I was independently questioning this as well since the discrete log is still hard over a large prime field and using a vector commitment, and that's why the PR currently uses the same generator and divergences from the ECCs recommended remediation.