From 29aacf926e87680430ba90815e216b88e2e44e1f Mon Sep 17 00:00:00 2001 From: Alex Kuzmin Date: Wed, 8 Nov 2023 17:16:16 +0800 Subject: [PATCH] Refactor smart contract --- contracts/src/Summa.sol | 100 +++++++++++++-------- contracts/test/Summa.ts | 188 +++++++++++++++++++++------------------- 2 files changed, 163 insertions(+), 125 deletions(-) diff --git a/contracts/src/Summa.sol b/contracts/src/Summa.sol index 67a00963..cff887d3 100644 --- a/contracts/src/Summa.sol +++ b/contracts/src/Summa.sol @@ -4,9 +4,7 @@ pragma solidity ^0.8.18; // Uncomment this line to use console.log //import "hardhat/console.sol"; -import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "./interfaces/IVerifier.sol"; @@ -29,36 +27,46 @@ contract Summa is Ownable { * @dev Struct representing an asset owned by the CEX * @param assetName The name of the asset * @param chain The name of the chain name where the asset lives (e.g., ETH, BTC) - * @param amount The total amount of the asset that the CEX holds on a given chain */ struct Asset { string assetName; string chain; - uint256 amount; } /** - * @dev Struct representing a commitment submitted by the CEX + * @dev Struct representing a commitment submitted by the CEX. * @param mstRoot Merkle sum tree root of the CEX's liabilities - * @param rootSum The total sum of the tree asset + * @param rootSums The total sums of the assets included in the tree + * @param assetChains The chains where the CEX holds the assets included into the tree + * @param assetNames The names of the assets included into the tree */ struct Commitment { uint256 mstRoot; - uint256 rootSum; - Asset asset; + uint256[] rootSums; + string[] assetNames; + string[] assetChains; } - //User inclusion proof verifier + // User inclusion proof verifier IVerifier private immutable inclusionVerifier; - // All address ownership proofs submitted by the CEX + // List of all address ownership proofs submitted by the CEX AddressOwnershipProof[] public addressOwnershipProofs; + function getAddressOwnershipProof( + bytes32 addressHash + ) public view returns (AddressOwnershipProof memory) { + require( + _ownershipProofByAddress[addressHash] > 0, + "Address not verified" + ); + // -1 comes from the fact that 0 is reserved to distinguish the case when the proof has not yet been submitted + return + addressOwnershipProofs[_ownershipProofByAddress[addressHash] - 1]; + } + // 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; + mapping(bytes32 => uint256) private _ownershipProofByAddress; // Solvency commitments by timestamp submitted by the CEX mapping(uint256 => Commitment) public commitments; @@ -66,11 +74,11 @@ contract Summa is Ownable { event AddressOwnershipProofSubmitted( AddressOwnershipProof[] addressOwnershipProofs ); - event SolvencyProofSubmitted( + event LiabilitiesCommitmentSubmitted( uint256 indexed timestamp, uint256 mstRoot, - uint256 rootSum, - Asset assets + uint256[] rootSums, + Asset[] assets ); constructor(IVerifier _inclusionVerifier) { @@ -86,13 +94,14 @@ contract Summa is Ownable { ) public onlyOwner { for (uint i = 0; i < _addressOwnershipProofs.length; i++) { bytes32 addressHash = keccak256( - abi.encode(_addressOwnershipProofs[i].cexAddress) + abi.encodePacked(_addressOwnershipProofs[i].cexAddress) ); - uint256 index = ownershipProofByAddress[addressHash]; - require(index == 0, "Address already verified"); - //Offsetting the index by 1 to distinguish with the case when the proof hasn't been submitted (the storage slot would be zero) - ownershipProofByAddress[addressHash] = i + 1; + uint256 proofIndex = _ownershipProofByAddress[addressHash]; + require(proofIndex == 0, "Address already verified"); + addressOwnershipProofs.push(_addressOwnershipProofs[i]); + _ownershipProofByAddress[addressHash] = addressOwnershipProofs + .length; require( bytes(_addressOwnershipProofs[i].cexAddress).length != 0 && bytes(_addressOwnershipProofs[i].chain).length != 0 && @@ -108,31 +117,50 @@ contract Summa is Ownable { /** * @dev Submit proof of solvency for a CEX * @param mstRoot Merkle sum tree root of the CEX's liabilities - * @param rootSum The total sum of the given asset across all the CEX liabilities - * @param asset The assets owned by the CEX + * @param rootSums The total sums of the assets included into the Merkle sum tree + * @param assets The assets included into the Merkle sum tree * @param timestamp The timestamp at which the CEX took the snapshot of its assets and liabilities */ function submitCommitment( uint256 mstRoot, - uint256 rootSum, - Asset memory asset, + uint256[] memory rootSums, + Asset[] memory assets, uint256 timestamp ) public onlyOwner { + require(mstRoot != 0, "Invalid MST root"); require( - addressOwnershipProofs.length != 0, - "The CEX has not submitted any address ownership proofs" + rootSums.length == assets.length, + "Root asset sums and asset number mismatch" ); + string[] memory assetNames = new string[](assets.length); + string[] memory assetChains = new string[](assets.length); + for (uint i = 0; i < assets.length; i++) { + require( + bytes(assets[i].chain).length != 0 && + bytes(assets[i].assetName).length != 0, + "Invalid asset" + ); + require( + rootSums[i] != 0, + "All root sums should be greater than zero" + ); + assetNames[i] = assets[i].assetName; + assetChains[i] = assets[i].chain; + } - require( - bytes(asset.chain).length != 0 && - bytes(asset.assetName).length != 0, - "Invalid asset" + commitments[timestamp] = Commitment( + mstRoot, + rootSums, + assetNames, + assetChains ); - require(rootSum != 0, "Root sum should be greater than zero"); - - commitments[timestamp] = Commitment(mstRoot, rootSum, asset); - emit SolvencyProofSubmitted(timestamp, mstRoot, rootSum, asset); + emit LiabilitiesCommitmentSubmitted( + timestamp, + mstRoot, + rootSums, + assets + ); } /** diff --git a/contracts/test/Summa.ts b/contracts/test/Summa.ts index e01c498d..d8d6da15 100644 --- a/contracts/test/Summa.ts +++ b/contracts/test/Summa.ts @@ -12,17 +12,18 @@ describe("Summa Contract", () => { function submitCommitment( summa: Summa, mstRoot: BigNumber, - rootSum: BigNumber, - asset = { - chain: "ETH", - assetName: "ETH", - amount: BigNumber.from(556863), - } + rootSums: BigNumber[], + assets = [ + { + chain: "ETH", + assetName: "ETH", + }, + ] ): any { return summa.submitCommitment( mstRoot, - rootSum, - asset, + rootSums, + assets, BigNumber.from(1693559255) ); } @@ -99,14 +100,14 @@ describe("Summa Contract", () => { ownedAddresses = [ { chain: "ETH", - cexAddress: defaultAbiCoder.encode(["address"], [account1.address]), + cexAddress: account1.address.toString(), signature: "0x089b32327d332c295dc3b8873c205b72153211de6dc1c51235782b091cefb9d06d6df2661b86a7d441cd322f125b84901486b150e684221a7b7636eb8182af551b", message: message, }, { chain: "ETH", - cexAddress: defaultAbiCoder.encode(["address"], [account2.address]), + cexAddress: account2.address.toString(), signature: "0xb17a9e25265d3b88de7bfad81e7accad6e3d5612308ff83cc0fef76a34152b0444309e8fc3dea5139e49b6fc83a8553071a7af3d0cfd3fb8c1aea2a4c171729c1c", message: message, @@ -120,38 +121,44 @@ describe("Summa Contract", () => { .withArgs((ownedAddresses: any) => { return ( ownedAddresses[0].chain == "ETH" && - ownedAddresses[0].cexAddress == - defaultAbiCoder.encode(["address"], [account1.address]) && + ownedAddresses[0].cexAddress == account1.address && ownedAddresses[0].signature == "0x089b32327d332c295dc3b8873c205b72153211de6dc1c51235782b091cefb9d06d6df2661b86a7d441cd322f125b84901486b150e684221a7b7636eb8182af551b" && ownedAddresses[0].message == message && ownedAddresses[1].chain == "ETH" && - ownedAddresses[1].cexAddress == - defaultAbiCoder.encode(["address"], [account2.address]) && + ownedAddresses[1].cexAddress == account2.address && ownedAddresses[1].signature == "0xb17a9e25265d3b88de7bfad81e7accad6e3d5612308ff83cc0fef76a34152b0444309e8fc3dea5139e49b6fc83a8553071a7af3d0cfd3fb8c1aea2a4c171729c1c" && ownedAddresses[1].message == message ); }); - let proofOfAddressOwnership0 = await summa.addressOwnershipProofs(0); - expect(proofOfAddressOwnership0.chain).to.be.equal("ETH"); - expect(proofOfAddressOwnership0.cexAddress).to.be.equal( - defaultAbiCoder.encode(["address"], [account1.address]) + const addr1Hash = ethers.utils.solidityKeccak256( + ["string"], + [account1.address] ); - expect(proofOfAddressOwnership0.signature).to.be.equal( - "0x089b32327d332c295dc3b8873c205b72153211de6dc1c51235782b091cefb9d06d6df2661b86a7d441cd322f125b84901486b150e684221a7b7636eb8182af551b" + let proofOfAddressOwnership1 = await summa.getAddressOwnershipProof( + addr1Hash ); - expect(proofOfAddressOwnership0.message).to.be.equal(message); - let proofOfAddressOwnership1 = await summa.addressOwnershipProofs(1); expect(proofOfAddressOwnership1.chain).to.be.equal("ETH"); - expect(proofOfAddressOwnership1.cexAddress).to.be.equal( - defaultAbiCoder.encode(["address"], [account2.address]) - ); + expect(proofOfAddressOwnership1.cexAddress).to.be.equal(account1.address); expect(proofOfAddressOwnership1.signature).to.be.equal( - "0xb17a9e25265d3b88de7bfad81e7accad6e3d5612308ff83cc0fef76a34152b0444309e8fc3dea5139e49b6fc83a8553071a7af3d0cfd3fb8c1aea2a4c171729c1c" + "0x089b32327d332c295dc3b8873c205b72153211de6dc1c51235782b091cefb9d06d6df2661b86a7d441cd322f125b84901486b150e684221a7b7636eb8182af551b" ); expect(proofOfAddressOwnership1.message).to.be.equal(message); + const addr2Hash = ethers.utils.solidityKeccak256( + ["string"], + [account2.address] + ); + let proofOfAddressOwnership2 = await summa.getAddressOwnershipProof( + addr2Hash + ); + expect(proofOfAddressOwnership2.chain).to.be.equal("ETH"); + expect(proofOfAddressOwnership2.cexAddress).to.be.equal(account2.address); + expect(proofOfAddressOwnership2.signature).to.be.equal( + "0xb17a9e25265d3b88de7bfad81e7accad6e3d5612308ff83cc0fef76a34152b0444309e8fc3dea5139e49b6fc83a8553071a7af3d0cfd3fb8c1aea2a4c171729c1c" + ); + expect(proofOfAddressOwnership2.message).to.be.equal(message); }); it("should revert if the caller is not the owner", async () => { @@ -194,6 +201,16 @@ describe("Summa Contract", () => { summa.submitProofOfAddressOwnership(ownedAddresses) ).to.be.revertedWith("Invalid proof of address ownership"); }); + + it("should revert if requesting proof for unverified address", async () => { + const addr1Hash = ethers.utils.solidityKeccak256( + ["string"], + [account1.address] + ); + await expect( + summa.getAddressOwnershipProof(addr1Hash) + ).to.be.revertedWith("Address not verified"); + }); }); describe("verify proof of solvency", () => { @@ -218,48 +235,37 @@ describe("Summa Contract", () => { ownedAddresses = [ { chain: "ETH", - cexAddress: defaultAbiCoder.encode(["address"], [account1.address]), + cexAddress: account1.address.toString(), signature: "0x089b32327d332c295dc3b8873c205b72153211de6dc1c51235782b091cefb9d06d6df2661b86a7d441cd322f125b84901486b150e684221a7b7636eb8182af551b", message: message, }, { chain: "ETH", - cexAddress: defaultAbiCoder.encode(["address"], [account2.address]), + cexAddress: account2.address.toString(), signature: "0xb17a9e25265d3b88de7bfad81e7accad6e3d5612308ff83cc0fef76a34152b0444309e8fc3dea5139e49b6fc83a8553071a7af3d0cfd3fb8c1aea2a4c171729c1c", message: message, }, ]; - const jsonData = fs.readFileSync( - path.resolve( - __dirname, - "../../zk_prover/examples/solvency_proof_solidity_calldata.json" - ), - "utf-8" + mstRoot = BigNumber.from( + "0x2e021d9bf99c5bd7267488b6a7a5cf5f7d00222a41b6a9b971899c44089e0c5" ); - const calldata: any = JSON.parse(jsonData); - - mstRoot = calldata.public_inputs[0]; rootSum = BigNumber.from(10000000); }); it("should verify the proof of solvency for the given public input", async () => { await summa.submitProofOfAddressOwnership(ownedAddresses); - await expect(submitCommitment(summa, mstRoot, rootSum)) - .to.emit(summa, "SolvencyProofSubmitted") + await expect(submitCommitment(summa, mstRoot, [rootSum])) + .to.emit(summa, "LiabilitiesCommitmentSubmitted") .withArgs( BigNumber.from(1693559255), mstRoot, - rootSum, - (asset: Summa.AssetStruct) => { - return ( - asset.chain == "ETH" && - asset.assetName == "ETH" && - BigNumber.from(556863).eq(asset.amount as BigNumber) - ); + [rootSum], + (assets: [Summa.AssetStruct]) => { + return assets[0].chain == "ETH" && assets[0].assetName == "ETH"; } ); }); @@ -268,54 +274,71 @@ describe("Summa Contract", () => { await expect( summa.connect(account2).submitCommitment( mstRoot, - BigNumber.from(1000000000), - { - chain: "ETH", - assetName: "ETH", - amount: BigNumber.from(556863), - }, + [BigNumber.from(1000000000)], + [ + { + chain: "ETH", + assetName: "ETH", + }, + ], BigNumber.from(1693559255) ) ).to.be.revertedWith("Ownable: caller is not the owner"); }); - it("should not verify the proof of solvency if the CEX hasn't proven the address ownership", async () => { - await expect( - submitCommitment(summa, mstRoot, rootSum) - ).to.be.revertedWith( - "The CEX has not submitted any address ownership proofs" - ); - }); - it("should revert with invalid root sum", async () => { rootSum = BigNumber.from(0); await summa.submitProofOfAddressOwnership(ownedAddresses); await expect( - submitCommitment(summa, mstRoot, rootSum) - ).to.be.revertedWith("Root sum should be greater than zero"); + submitCommitment(summa, mstRoot, [rootSum]) + ).to.be.revertedWith("All root sums should be greater than zero"); }); it("should revert with invalid assets", async () => { await summa.submitProofOfAddressOwnership(ownedAddresses); await expect( - submitCommitment(summa, mstRoot, rootSum, { - chain: "", - assetName: "ETH", - amount: BigNumber.from(556863), - }) + submitCommitment( + summa, + mstRoot, + [rootSum], + [ + { + chain: "", + assetName: "ETH", + }, + ] + ) ).to.be.revertedWith("Invalid asset"); await expect( - submitCommitment(summa, mstRoot, rootSum, { - chain: "ETH", - assetName: "", - amount: BigNumber.from(556863), - }) + submitCommitment( + summa, + mstRoot, + [rootSum], + [ + { + chain: "ETH", + assetName: "", + }, + ] + ) ).to.be.revertedWith("Invalid asset"); }); + + it("should not submit invalid root", async () => { + await expect( + submitCommitment(summa, BigNumber.from(0), [rootSum]) + ).to.be.revertedWith("Invalid MST root"); + }); + + it("should revert if asset and sum count don't match", async () => { + await expect( + submitCommitment(summa, mstRoot, [rootSum, rootSum]) + ).to.be.revertedWith("Root asset sums and asset number mismatch"); + }); }); describe("verify proof of inclusion", () => { @@ -326,7 +349,6 @@ describe("Summa Contract", () => { let account1: SignerWithAddress; let account2: SignerWithAddress; let inclusionProof: string; - let solvencyProof: string; let ownedAddresses: Summa.AddressOwnershipProofStruct[]; const message = ethers.utils.defaultAbiCoder.encode( ["string"], @@ -356,18 +378,6 @@ describe("Summa Contract", () => { }, ]; - const solvencyJson = fs.readFileSync( - path.resolve( - __dirname, - "../../zk_prover/examples/solvency_proof_solidity_calldata.json" - ), - "utf-8" - ); - const solvencyCalldata: any = JSON.parse(solvencyJson); - - mstRoot = solvencyCalldata.public_inputs[0]; - solvencyProof = solvencyCalldata.proof; - const inclusionJson = fs.readFileSync( path.resolve( __dirname, @@ -385,7 +395,7 @@ describe("Summa Contract", () => { it("should verify the proof of inclusion for the given public input", async () => { await summa.submitProofOfAddressOwnership(ownedAddresses); - await submitCommitment(summa, mstRoot, rootSum); + await submitCommitment(summa, mstRoot, [rootSum]); expect( await verifyInclusionProof(summa, inclusionProof, leafHash, mstRoot) ).to.be.equal(true); @@ -393,7 +403,7 @@ describe("Summa Contract", () => { it("should not verify with invalid MST root", async () => { await summa.submitProofOfAddressOwnership(ownedAddresses); - await submitCommitment(summa, mstRoot, rootSum); + await submitCommitment(summa, mstRoot, [rootSum]); mstRoot = BigNumber.from(0); await expect( verifyInclusionProof(summa, inclusionProof, leafHash, mstRoot) @@ -411,7 +421,7 @@ describe("Summa Contract", () => { leafHash = BigNumber.from(0); await summa.submitProofOfAddressOwnership(ownedAddresses); - await submitCommitment(summa, mstRoot, rootSum); + await submitCommitment(summa, mstRoot, [rootSum]); expect( await verifyInclusionProof(summa, inclusionProof, leafHash, mstRoot) ).to.be.equal(false); @@ -421,7 +431,7 @@ describe("Summa Contract", () => { inclusionProof = inclusionProof.replace("1", "2"); await summa.submitProofOfAddressOwnership(ownedAddresses); - await submitCommitment(summa, mstRoot, rootSum); + await submitCommitment(summa, mstRoot, [rootSum]); expect( await verifyInclusionProof(summa, inclusionProof, leafHash, mstRoot) ).to.be.equal(false);