RENEGADE Audit #3: Focus on Proof Linking

# RENEGADE Audit #3: Focus on Proof Linking To understand the code and logic in the trading-related [proof linking file](https://github.com/renegade-fi/renegade/blob/main/circuits/src/zk_circuits/proof_linking.rs) in the [renegade/circuits/src/zk_circuits folder](https://github.com/renegade-fi/renegade/blob/main/circuits/src/zk_circuits/), I investigated the following functionalities: ## 1. Functionality 1 ### Description The specialized [PLONK based proof linking file](https://github.com/renegade-fi/mpc-jellyfish/blob/main/plonk/src/proof_system/proof_linking.rs) in the [mpc-jellyfish/plonk/src/proof_system folder](https://github.com/renegade-fi/renegade/blob/main/circuits/src/zk_circuits/). In particular, the function `fn link-proofs <T: PlonkTranscript<F>> `[(line 80 from this file)](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L80) is used in [line 58](https://github.com/renegade-fi/renegade/blob/997f9c4e66c3f095ad55056c3b7a50df972e5662/circuits/src/zk_circuits/proof_linking.rs#L58) of the [proof linking file of interest](https://github.com/renegade-fi/renegade/blob/997f9c4e66c3f095ad55056c3b7a50df972e5662/circuits/src/zk_circuits/proof_linking.rs). In fact, each function in the implementation of the `struct PlonkKzgSnark` is internally called/needed by the implementation of the function `fn link-proofs <T: PlonkTranscript<F>>`. #### Group observations related to the implementation of [PlonkKzgSnark in lines L73-L222](https://github.com/renegade-fi/mpc-jellyfish/blob/main/plonk/src/proof_system/proof_linking.rs#L73-L222) (This is the PLONK KZG Prover!): - **Logical Issue**: It seems quite restrictive that there is [only one group_layout variable (of type GroupLayout)](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L83). In particular, given only a group layout it means it applies to both proofs/commitments/witnesses that the prover would like to link together. In turn, this means that we cannot have any special case where one witness (polynomial) is a subvector of the other witness (polynomial). Is the Renegade team really OK with this restrictive logic? - **Possible Issue:** Does the polynomial division [in line 133 ](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L133) `Ok(&diff / &vanishing_poly)` output an error as it probably should when the vanishing polynomial does not divide the diff polynomial? - **Low Security Issue:** The function `fn compute_quotient_challenge<T: PlonkTranscript<E::BaseField>>` defined [in line 185 ](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L185C4-L185C68) is not computing the FS transform correctly as a few public parameters are missing: some identifier of the prover key, some unique identifier of the GroupLayout (e.g., the order of the roots of unity over which the linking proof is defined, etc.). However, I do not think it would be easy to mount an attack even if these items are missing. - **Possible Issue**: Question for me to further think about: Are there any exploitable consequences for the fact that the [LinkingProofs struct](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L34-L38) contains only the argument elements without the corresponding public input (the two commitments to the "matching" polynomials)? Can there ever be that a LinkingProof with a public input that does not correspond to it be passed around as an alleged correct PLONK proof and be accepted? #### Group observations related to the implementation of [PlonkKzgSnark in lines L228-L287](https://github.com/renegade-fi/mpc-jellyfish/blob/main/plonk/src/proof_system/proof_linking.rs#L228-L287) (This is the PLONK KZG Verifier!): - **Question for Renegade team** The first two parameters of the function `fn verify_link_proof<T: PlonkTranscript<E::BaseField>>` (see [line 240](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L240)) could have simply been of type `&Commitment`. I really do not see why `&Proof` is used. Type `Proof` requires multiple commitments to be instantiated and, in [the Prover for linking proofs](https://github.com/renegade-fi/mpc-jellyfish/blob/main/plonk/src/proof_system/proof_linking.rs#L73-L222) all but one commitment are not used. ## 2. Functionality 2 ### Description The function `get_group_layout()` called [here ](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L123) is actually defined in [this file](https://github.com/renegade-fi/mpc-jellyfish/blob/main/relation/src/proof_linking/mod.rs). #### Group observations - **Possible low security issue:** When [defining the constructor for GroupLayout](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L31) there should be some checks whether the parameters alignment, offset and size work together at all. - **Question for internal discussion:** I do not inderstand the logic behind the function `range_in_nth_roots()` [here](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L37). - **Question for internal discussion:** I do not understand the logic [behind line 51 ](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L51). - **Question for Renegade team** What is the relation between gates, n_gates and and link_gates in [the definition of struct CircuitLayout](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L58) and on [this line](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L84)? Why do Renegade use addition? Why is circuit size computed [this way](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/relation/src/proof_linking/mod.rs#L85-L86)? ## 3. Understanding the [main file of interest](https://github.com/renegade-fi/renegade/blob/zksecurity-audit-4-21-25/circuits/src/zk_circuits/proof_linking.rs) #### ISSUE 1: Background: [This file](https://github.com/renegade-fi/renegade/blob/zksecurity-audit-4-21-25/circuits/src/zk_circuits/proof_linking.rs) mostly contains functions which have one of two main functionalities: *Functionality A:* Create a linking proof between a commitment (proof) and a re-bliding proof ([from line 28 to line 124](https://github.com/renegade-fi/renegade/blob/zksecurity-audit-4-21-25/circuits/src/zk_circuits/proof_linking.rs#L28-L124)) or create a linking proof between a commitment (proof) and a match settle proof (from [line 130 to line 388](https://github.com/renegade-fi/renegade/blob/zksecurity-audit-4-21-25/circuits/src/zk_circuits/proof_linking.rs#L130-L388)). Overall, creating a linking proof follows the same pattern: * Requires parameters` MAX_BALANCES, MAX_ORDERS, MERKLE_HEIGHT` (either as constants or as variables). **ISSUE 1.1:** I believe it should be enough for these parameters to be set as variables. It would be hepful if the Renegade team clarifies why they need to have designated functions where these parameters are set as system wide constants as well. * Inputs are either tuples (commitment hint, reblind hint) or (commitment hint, settle match hint) where each component is of type `ProofLinkHint` (i.e., a commitment and a witness polynomial). * Output is of type `Result<PlonkLinkProof, ProverError>` where PlonkLinkProof is a type of Proof which itself is a full PLONK proof, i.e., all the commitments which make up a standard PLONK proof. *Functionality B:* Validate a linking proof defined in item A. Note that the inputs for a validate function are (commitment proof, reblind/settle match proof, linking proof) of type `(PlonkProof, PlonkProof, PlonkLinkProof)`. In order to check the linking proof validity, the code always extracts only the commitments of interest from the full PLONK proof. **ISSUE 1.2:** There is a discrepancy between the inputs to the functions creating linking proofs and the functions validating the linking proofs. The former takes as inputs only the minimal necessary commitments while the latter takes as inputs full PLONK proofs. Renegade team should provide good documentation regarding for the correct use of the above functions, otherwise this discrepancy is confusing/can lead to erroneous use. ### ISSUE 2: For each validation function there is a clear corresponding link proof creation function, but, there are two extra link proof creation functions which do not have any such correspondence (see below). Moreover, it is quite unclear what additional functionality do these functions model/bring compared to what is already defined. **Recommendations:** The code should be revised for possible duplicated functionality. Also the code should be better documented with more clear explanations and highlighted differences among different existing functions. The extra functions are defined starting on line 298 and on line 303, respectively ([fn link_sized_commitments_match_settle_with_commitments](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L289) and [fn link_commitments_match_settle_with_commitments](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L303)). Moreover, each of them uses ValidMatchSettleWithCommitments as described in [line 316](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L316) but there is no corresponding validation function to use ValidMatchSettleWithCommitments to derive the verification key. **Technical question for internal discussion:** For the code on [this line](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L316) let pk = ValidMatchSettleWithCommitments::<MAX_BALANCES, MAX_ORDERS>::proving_key();` I could not find an implementation for the structure `ValidMatchSettleWithCommitments` which includes the function `proving_key()`. When using VS Code to track down the definition of `proving_key()` in the context above I am redirected to this `[file/line](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuit-types/src/traits.rs#L819) which is counter-intuitive to me. ### ISSUE 3: I would like another pair of eyes on this but, as far as I could tell, the function `get_circuit_layout()` which is called on lines [121](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L121) and [385](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L385) and is defined [here](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuit-types/src/traits.rs#L849) seems to create an empty circuit. If that is the case, I cannot understand how it can be of use when called in the locations I mentioned above. ### ISSUE 4: Currently the FS transform for the linking proof, in particular the derivation of the opening challenge, takes into account only the two commitments related to commitment proof and to the re-blind/match settle proof. See, for example, [here](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L250) which redirects [here](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L80) where t[he function compute_quotient_challenge()](https://github.com/renegade-fi/mpc-jellyfish/blob/7be2cb7e66e692e7555ec3f469bbb27adc407c52/plonk/src/proof_system/proof_linking.rs#L185) is called. However, as seen in any of the validation functions, for example, [here](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L174-L175), the commitment proof and the match settle proofs are part of a full PLONK proof (or, at least this is what the definition says) and, to my best recommendation I believe the respective full PLONK proofs should be taken into account into the FS transform when computing the opening challenge for the linking proof. Without that it seems that malicious parties who can see previous successful linking proofs can replicate those proofs and claim them as their own which does not seem right. I believe this is at least **a medium security issue**. ### Other open questions/issues: - **Internal question to discuss** Can any of the sizing parameters be exploited? - **Possible question for Renegade team** What is the difference between atomic settle and non-atomic settle? Just from the code I have been looking at I cannot tell at all the difference and I do not know/understand why separate creation/validation functions for these two sub-types of settle matching are needed? - **Possible Low Issue** There is [one use of panic!](https://github.com/renegade-fi/renegade/blob/d880cda602e5da95ec01ec793ab0fca3993411d3/circuits/src/zk_circuits/proof_linking.rs#L382) which probably should be avoided.