Review of the Summa.sol smart contract.

[hrishibhat]

The Summa contract allows for centralized exchanges to prove to their users of the inclusion of their balances in the Merke Sum Tree.
The Summa contract is currently setup with the configuration that includes:

• mstLevels the number of levels to be used in an exchange's MST
• currenciesCount number of cryptocurrencies supported.
• balanceByteRange is the bytes used to represent the balance.

The CEX Owner submits proof of address ownership through submitProofOfAddressOwnership. This is an optimistic proof that these addresses belong to the exchange and the verification is done off-chain due to the large computational requirement that may not be feasible on-chain.

The CEX owner submits a commitment using submitCommitment about its liabilities in the form of a struct that uses the Merkle root, balances of the root, and the timestamp.

Once the commitment is submitted the user can then verify using the verifyInclusionProof function with relevant proof issued by the CEX and the public inputs against the exchange commitment for the respective timestamp that their balances are accurately represented in the CEX's Merkle tree. The verification is done by an external Inclusion verifier contract.

Observations and Security concerns:

• No timestamp validation:

    function submitCommitment(
        uint256 mstRoot,
        uint256[] memory rootBalances,
        Cryptocurrency[] memory cryptocurrencies,
        uint256 timestamp // @audit : Future timestamp can be used. This can be used to manipulate 

timestamp is expected to be time at which the exchange has taken snapshot of all the balances but this timestamp is not validated. As this can be set to a future timestamp. This may lead to potential manipulations by the exchange owner by combining off-chain and on-chain processes:

• Creates inconsistencies/confusion by not maintaining a chronological order in the commitment.
• Delaying the proof verification by promising a future commitment.
  To mitigate this add the following vallidation checks to timestamp
• Add a check to make sure the timestamp is not in the future.
• Store the last submitted timestamp and check the new timestamp is larger than the previous timestamp.

+ uint256 public lastSubmitted;

    function submitCommitment(
        uint256 mstRoot,
        uint256[] memory rootBalances,
        Cryptocurrency[] memory cryptocurrencies,
        uint256 timestamp
    ) public onlyOwner {
        require(mstRoot != 0, "Invalid MST root");
+       require(timestamp < block.timestamp, "Cannot submit future commitment");
+        require(timestamp > lastSubmitted, "Incorrect timestamp");
        lastSubmitted = timestamp;
        ....

mstLevels and currenciesCount are fixed:
mstLevels and currenciesCount are currently set within the summa config inside the constructor. However this could limit flexibility if the number of accounts or currencies changes over time. Allow for dynamic resizing of mstLevels and currenciesCount.
While this may require commitment versioning so as to not impact the previous versions of commitments when the mstLevels or currencies change.

No validation for mstLevels or currency length when adding commitments:
Commitment submissions do check if the cryptocurrencies length matches with the configured range of currenciesCount allowing for any potential manipulations with these. Although this may not have a significant impact since the user could verify the currencies, also not all the currencies may need to be included for a commitment but a there can be a sanity check
require(cryptocurrencies.length <= currenciesCount, "Exceeds currency count");
Additional check can also be added to make sure the mst levels resulting out of the rootBalances/currenciesCount does not exceed the configured mstLevels.

[alxkzmn]

Great idea regarding ensuring the chronological order! As for preventing the future timestamps to be used, there can be an issue with the chains other than the one where contract is deployed. Even though a block.timestamp corresponds to a particular moment in time, the exact value itself only makes sense for a block of this specific chain. Everyone would have to agree on a reliable conversion mechanism to be able to convert from one chain's block timestamp to another.