Refactor the contract according to V1 consolidation spec #178
Conversation
* feat: create bash script for updating verifier interface files in backend * fix: error propagation with try operator; remove unnecessaries * refactor: changed data type in 'MstInclusionProof' * fix: generate solvency verifier contract * chore: remove left over * chore: update README * fix: remove left over; assert term * fix: update README; small fixes * feat: Signer accepts address or file path for init
contracts/src/Summa.sol
Outdated
|
|
||
| event AddressOwnershipProofSubmitted( | ||
| AddressOwnershipProof[] addressOwnershipProofs | ||
| ); | ||
| event SolvencyProofSubmitted( | ||
| uint256 indexed timestamp, | ||
| uint256 mstRoot, | ||
| Asset[] assets | ||
| uint256 rootSum, | ||
| Asset assets |
There was a problem hiding this comment.
Why this is no longer an array?
There was a problem hiding this comment.
Because we're now having a single asset per tree per the new specs.
There was a problem hiding this comment.
Roll back to multi-assets as discussed privately
contracts/src/Summa.sol
Outdated
| Asset asset; | ||
| } | ||
|
|
||
| //User inclusion proof verifier |
contracts/src/Summa.sol
Outdated
| @@ -77,6 +90,7 @@ contract Summa is Ownable { | |||
| ); | |||
| 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) | |||
contracts/test/Summa.ts
Outdated
| @@ -255,113 +242,85 @@ describe("Summa Contract", () => { | |||
| const calldata: any = JSON.parse(jsonData); | |||
|
|
|||
| mstRoot = calldata.public_inputs[0]; | |||
There was a problem hiding this comment.
We will no longer have solvency_proof_solidity_calldata.json and its generator script in the future. How do you think we should modify the flow accordingly? One suggestion that I have is to modify the gen_inclusion_verifier.rs example with generate_mst_commitment.rs that outputs something like commitment_calldata.json
There was a problem hiding this comment.
I think it's a good idea.
contracts/test/Summa.ts
Outdated
| }); | ||
|
|
||
| it("should verify the proof of inclusion for the given public input", async () => { | ||
| await summa.submitProofOfAddressOwnership(ownedAddresses); | ||
| await submitProofOfSolvency(summa, mstRoot, solvencyProof); | ||
| await submitCommitment(summa, mstRoot, rootSum); |
There was a problem hiding this comment.
I realize that there's a security issue here. You hardcode rootSum as 10000000 when sending the commitment. But this is not the correct root sum associated with mstRoot. To fix that, the ProofOfInclusion should also output the rootSum as public output and the verification function should check that it matches the committed one
| }); | ||
|
|
||
| describe("verify proof of inclusion", () => { | ||
| let mstRoot: BigNumber; | ||
| let rootSum: BigNumber; |
There was a problem hiding this comment.
I cannot comment on the right line but solvencyProof should no longer exist here
| * @param timestamp The timestamp at which the CEX took the snapshot of its assets and liabilities | ||
| */ | ||
| function submitProofOfSolvency( | ||
| function submitCommitment( |
There was a problem hiding this comment.
Here we should check that Asset amount is greater than rootSums
There was a problem hiding this comment.
Resolved in private discussion
enricobottazzi
merged commit 19dcb54
into
summa-dev:v1-improvements-and-consolidation
No description provided.