Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rm π solvency #185

Merged
merged 12 commits into from
Nov 13, 2023
Merged

Conversation

enricobottazzi
Copy link
Member

No description provided.

@enricobottazzi
Copy link
Member Author

enricobottazzi commented Nov 10, 2023

@alxkzmn can you help me on the contracts side of this? As part of the PR I added a script gen_commitment in zk_prover that output the commitment to the MerkleSumTree in a json file that can be consumed by contracts.

To dos:

  • verifyInclusionProof should also require that rootSums used as public input match the ones committed to in the previous phase
  • rename rootSums to rootBalances
  • Update tests to consume commitment_solidity_calldata.json
  • Add testing to cover the case in which the CEX commits to a fake rootBalances during the commitment phase that will make verifyInclusionProof to fail. Similar to the attack described here => Refactor the contract according to V1 consolidation spec #178 (comment)

@alxkzmn
Copy link
Contributor

alxkzmn commented Nov 11, 2023

@alxkzmn can you help me on the contracts side of this? As part of the PR I added a script gen_commitment in zk_prover that output the commitment to the MerkleSumTree in a json file that can be consumed by contracts.

To dos:

* [x]  `verifyInclusionProof` should also require that `rootSums` used as public input match the ones committed to in the previous phase

* [x]  rename `rootSums` to `rootBalances`

* [x]  Update tests to consume `commitment_solidity_calldata.json`

* [x]  Add testing to cover the case in which the CEX commits to a fake `rootBalances` during the commitment phase that will make `verifyInclusionProof` to fail. Similar to the attack described here => [Refactor the contract according to V1 consolidation spec #178 (comment)](https://github.com/summa-dev/summa-solvency/pull/178#discussion_r1381385661)

Done!

@enricobottazzi enricobottazzi marked this pull request as ready for review November 13, 2023 12:49
@enricobottazzi enricobottazzi merged commit c8d9781 into v1-improvements-and-consolidation Nov 13, 2023
4 checks passed
@enricobottazzi enricobottazzi deleted the rm-π-solvency branch November 21, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants