From e21c1cc41e0047e368468781ca9e80f141a79364 Mon Sep 17 00:00:00 2001 From: sifnoc Date: Tue, 31 Oct 2023 11:06:28 +0000 Subject: [PATCH] chore: add audit comments --- contracts/src/Summa.sol | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/contracts/src/Summa.sol b/contracts/src/Summa.sol index 2134c4f0..82159afc 100644 --- a/contracts/src/Summa.sol +++ b/contracts/src/Summa.sol @@ -37,14 +37,17 @@ contract Summa is Ownable { uint256 amount; } - //Verifier contracts + // Verifier contracts IVerifier private immutable solvencyVerifier; IVerifier private immutable inclusionVerifier; - //All address ownership proofs submitted by the CEX + // All address ownership proofs submitted by the CEX AddressOwnershipProof[] public addressOwnershipProofs; - //Convenience mapping to check if an address has already been verified + // Convenience mapping to check if an address has already been verified + /* + Boolean type is better than uint256 for this mapping, at least more than 2,100 gas is saved per call + */ mapping(bytes32 => uint256) public ownershipProofByAddress; // MST roots corresponding to successfully verified solvency proofs by timestamp @@ -64,6 +67,15 @@ contract Summa is Ownable { inclusionVerifier = _inclusionVerifier; } + /* + // While this duplicate method might elevate deployment costs, the trade-off is a reduction in usage and maintenance costs. + function submitProofOfAddressOwnership( + AddressOwnershipProof memory _addressOwnershipProof + ) public onlyOwner { + ... + } + */ + /** * @dev Submit an optimistic proof of address ownership for a CEX. The proof is subject to an off-chain verification as it's not feasible to verify the signatures of non-EVM chains in an Ethereum smart contract. * @param _addressOwnershipProofs The list of address ownership proofs @@ -77,6 +89,9 @@ contract Summa is Ownable { ); uint256 index = ownershipProofByAddress[addressHash]; require(index == 0, "Address already verified"); + /* + Is there any reason to assign value `i + 1` to `ownershipProofByAddress[addressHash]`? + */ ownershipProofByAddress[addressHash] = i + 1; addressOwnershipProofs.push(_addressOwnershipProofs[i]); require( @@ -125,6 +140,10 @@ contract Summa is Ownable { emit SolvencyProofSubmitted(timestamp, inputs[0], assets); } + /* + It would be helpful to provide a description of the public inputs for the `verifySolvencyProof` and `verifyInclusionProof` methods. + */ + /** * Verify the proof of CEX solvency * @param proof ZK proof