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

Potential Summa::submitCommitment() Gas limits #4

Open
sebastiantf opened this issue Feb 21, 2024 · 3 comments · May be fixed by #5
Open

Potential Summa::submitCommitment() Gas limits #4

sebastiantf opened this issue Feb 21, 2024 · 3 comments · May be fixed by #5

Comments

@sebastiantf
Copy link
Collaborator

sebastiantf commented Feb 21, 2024

Description

Summa::submitCommitment() takes in two arrays and loops once over them:

uint256[] memory rootBalances,
Cryptocurrency[] memory cryptocurrencies,

for (uint i = 0; i < cryptocurrencies.length; i++) {
require(
bytes(cryptocurrencies[i].chain).length != 0 &&
bytes(cryptocurrencies[i].name).length != 0,
"Invalid cryptocurrency"
);
require(
rootBalances[i] != 0,
"All root sums should be greater than zero"
);
cryptocurrencyNames[i] = cryptocurrencies[i].name;
blockchainNames[i] = cryptocurrencies[i].chain;
}

rootBalances array contains the root balances of each cryptocurrency.
cryptocurrencies array contains details of each cryptocurrency: name, chain

There could be practical limitations to the number of rootBalances and cryptocurrencies that could be submitted in a single txn, imposed by block gas limits

According to Coingecko, Binance hosts 376 cryptocurrencies.

Current tests in the Summa contract codebase doesn't test this limitation as it only seems to be using 2 cryptocurrencies. See here, here and here

It shouldn't be too hard to add a test that tests such limits and it is WIP. But if it turns out that the contract is not able to handle such large number of cryptocurrencies, it would be a limitation to be aware of. It might be necessary to split the submission into multiple commitments for the same timestamp.

This would warrant if the circuit implementation supports this kind of split submission. Afaik, a single commitment is for the entire state of the exchange at a given time. If I'm wrong, please correct me, and feel free to close this issue.

@sebastiantf
Copy link
Collaborator Author

After testing in #5, seems like 402 is the max no. of cryptocurrencies before overflowing 30M block gas limit

@sebastiantf
Copy link
Collaborator Author

Also related: Are there / Could there be limits to the number of currencies that can be used in the circuit as well that prohibit practical use due to increase in proof size / verification cost.
#8 is a small effort towards that. But as mentioned there, more insights on proof size / verification cost is derived from cost_model

@sifnoc
Copy link
Contributor

sifnoc commented Apr 3, 2024

Also related: Are there / Could there be limits to the number of currencies that can be used in the circuit as well that prohibit practical use due to increase in proof size / verification cost.

There is another limitation to the number of currencies from the verifier contract. As you may already know, the size of the verifier contract is linearly correlated with the number of currencies it supports. Practically, the inclusion of additional currencies is limited to around 24 to 25 due to the contract code size limit.

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 a pull request may close this issue.

2 participants