Skip to content

Commit

Permalink
names
Browse files Browse the repository at this point in the history
  • Loading branch information
nullity00 committed Jun 21, 2024
1 parent 9548856 commit 4b66f3d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 34 deletions.
27 changes: 13 additions & 14 deletions versionA.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ Auditors:
- [5. Clippy](#5-clippy)
- [B - Fuzz Testing](#b---fuzz-testing)
- [C - Code Coverage](#c---code-coverage)


# Protocol Summary

Expand Down Expand Up @@ -214,7 +213,7 @@ We recommend that a range check is done inside the circuit or inside the smart c

#### Refer

- [Guarantee usernames stays inside field](https://github.com/zBlock-2/summa-solvency-schneier/issues/13) by [sebastiantf]()
- [Guarantee usernames stays inside field](https://github.com/zBlock-2/summa-solvency-schneier/issues/13) by [sebastiantf](https://github.com/sebastiantf)
- [Possible Overflow in username in big_intify_username combined with calling big_uint_to_fp](https://github.com/zBlock-2/summa-solvency-diffie/issues/16) by [parsley]()

## 2. High: Sum balance overflow
Expand All @@ -223,7 +222,7 @@ The lack of in-circuit range check for the sum of balances posesses a risk of ov

#### Refer

- [Sum Balance Overflow](https://github.com/zBlock-2/summa-solvency-diffie/issues/10) by [zeroqn]()
- [Sum Balance Overflow](https://github.com/zBlock-2/summa-solvency-diffie/issues/10) by [zeroqn](https://github.com/zeroqn)

## 3. High: Inconsistency in range checks

Expand All @@ -233,7 +232,7 @@ Root's max balance is `(NLEVEL - 1) * m` as can be inferred from circuits/contra

#### Refer

- [Inconsistency in range checks](https://github.com/zBlock-2/summa-solvency-Turing/issues/14) by [y5yash]()
- [Inconsistency in range checks](https://github.com/zBlock-2/summa-solvency-Turing/issues/14) by [y5yash](https://github.com/Y5Yash)

## Low

Expand All @@ -260,7 +259,7 @@ In the code [here](https://github.com/summa-dev/summa-solvency/blob/master/zk_pr

#### Refer

- [Mixed endian usage in code](https://github.com/zBlock-2/summa-solvency-diffie/issues/17) by [bbresearcher]()
- [Mixed endian usage in code](https://github.com/zBlock-2/summa-solvency-diffie/issues/17) by [bbresearcher](https://github.com/bbresearcher)

## Informational

Expand All @@ -270,7 +269,7 @@ The [range check](https://github.com/summa-dev/summa-solvency/blob/52373464b7ac4

#### Refer

- [Range check uses lookup_any instead of lookup](https://github.com/zBlock-2/summa-solvency-schneier/issues/18) By [obatirou]()
- [Range check uses lookup_any instead of lookup](https://github.com/zBlock-2/summa-solvency-schneier/issues/18) By [obatirou](https://github.com/obatirou)

## 2. Informational: `InclusionVerifier.yul`not generated

Expand All @@ -280,7 +279,7 @@ It is mentioned in the Summa Book on the [summa-solvency page](https://summa.git

#### Refer

- [`InclusionVerifier.yul`not generated](https://github.com/zBlock-2/summa-solvency-schneier/issues/16) by [flyingnobita]()
- [`InclusionVerifier.yul`not generated](https://github.com/zBlock-2/summa-solvency-schneier/issues/16) by [flyingnobita](https://github.com/flyingnobita)

## 3. Informational: Improvement to public inputs in contract

Expand All @@ -290,7 +289,7 @@ Since they are not homogenous or not values that have the same meaning, it might

#### Refer

- [Improvement to public inputs in contract](https://github.com/zBlock-2/summa-solvency-schneier/issues/12) By [sebastiantf]()
- [Improvement to public inputs in contract](https://github.com/zBlock-2/summa-solvency-schneier/issues/12) By [sebastiantf](https://github.com/sebastiantf)

## 4. Informational: Use only mapping for `addressOwnershipProofs`

Expand All @@ -300,7 +299,7 @@ We recommend removing the array, modifying the mapping to store the `AddressOwne

#### Refer

- [Use only mapping for `addressOwnershipProofs`](https://github.com/zBlock-2/summa-solvency-schneier/issues/11) by [sebastiantf]()
- [Use only mapping for `addressOwnershipProofs`](https://github.com/zBlock-2/summa-solvency-schneier/issues/11) by [sebastiantf](https://github.com/sebastiantf)

## 5. Informational: `Summa.sol` : Improvements to generating the `addressHash`

Expand Down Expand Up @@ -329,8 +328,8 @@ According to [Coingecko](https://www.coingecko.com/en/exchanges/binance), Binanc

#### Refer

- [Potential `Summa::submitCommitment()` Gas limits](https://github.com/zBlock-2/summa-solvency-schneier/issues/4) by [sebastiantf]()
- [test: submitting large no. of cryptocurrencies in single commitment](https://github.com/zBlock-2/summa-solvency-schneier/pull/5) by [sebastiantf]()
- [Potential `Summa::submitCommitment()` Gas limits](https://github.com/zBlock-2/summa-solvency-schneier/issues/4) by [sebastiantf](https://github.com/sebastiantf)
- [test: submitting large no. of cryptocurrencies in single commitment](https://github.com/zBlock-2/summa-solvency-schneier/pull/5) by [sebastiantf](https://github.com/sebastiantf)

## 8. Informational: Use constants to denote magic numbers in PoseidonChip

Expand All @@ -345,7 +344,7 @@ The [Poseidon chip](https://github.com/summa-dev/summa-solvency/blob/master/zk_p

#### Refer

- [Magic numbers used in code of MST Circuit to create PoseidonChip](https://github.com/zBlock-2/summa-solvency-diffie/issues/15) by [bbresearcher]()
- [Magic numbers used in code of MST Circuit to create PoseidonChip](https://github.com/zBlock-2/summa-solvency-diffie/issues/15) by [bbresearcher](https://github.com/bbresearcher)

## 9. Informational: Missing validation for `timestamp`, `mstLevels` and `currenciesCount`

Expand Down Expand Up @@ -390,7 +389,7 @@ Likewise we would also need the following checks :
#### Refer
- [Review of the `Summa.sol` smart contract](https://github.com/zBlock-2/summa-solvency-diffie/issues/12) by [hrishibhat]()
- [Review of the `Summa.sol` smart contract](https://github.com/zBlock-2/summa-solvency-diffie/issues/12) by [hrishibhat](https://github.com/hrishibhat)
# Final remarks
Expand Down Expand Up @@ -523,4 +522,4 @@ We raised the following pull requests to increase code coverage & emphasize on t
- [PR#3](https://github.com/zBlock-2/summa-solvency-diffie/pull/3) to increase code coverage for `merkle_sum_tree`
- [PR#17](https://github.com/zBlock-2/summa-solvency-schneier/pull/17) to add end-to-end testing with full prover and verifier (instead of mock prover).
- [PR#8](https://github.com/zBlock-2/summa-solvency-schneier/pull/8/files) to include cost estimation for circuits using `CircuitCost`
- [PR#5](https://github.com/zBlock-2/summa-solvency-schneier/pull/5) is a stress test to determine the potential gas limits of `Summa::submitCommitment()`
- [PR#5](https://github.com/zBlock-2/summa-solvency-schneier/pull/5) is a stress test to determine the potential gas limits of `Summa::submitCommitment()`
41 changes: 21 additions & 20 deletions versionB.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ Auditors:
- [5. Cargo Audit](#5-cargo-audit)
- [6. Clippy](#6-clippy)


# Protocol Summary

_For a hgih-level overview of the Summa protocol, [see this](./README.md#overview-of-the-summa-proof-of-solvency-protocol)_.
Expand Down Expand Up @@ -188,7 +187,7 @@ A sniffer can see whether a user has a specific token because balance polynomial

#### Refer

- [The inclusion proof reveals whether a user holds a specific token](https://github.com/zBlock-2/summa-solvency/issues/21) by [qpzm]()
- [The inclusion proof reveals whether a user holds a specific token](https://github.com/zBlock-2/summa-solvency/issues/21) by [qpzm](https://github.com/qpzm)

## 2. High: Incorrect verifying key contract permutation length can be considered valid by `validateVKPermutationsLength`

Expand All @@ -198,7 +197,7 @@ This can lead to vkContracts with different permutation commitments from the cir

#### Refer

- [Wrong verifying key contract permutation length can be considered valid by `validateVKPermutationsLength`](https://github.com/zBlock-2/summa-solvency/issues/10) by [obatirou]()
- [Wrong verifying key contract permutation length can be considered valid by `validateVKPermutationsLength`](https://github.com/zBlock-2/summa-solvency/issues/10) by [obatirou](https://github.com/obatirou)

## 3. High: Fake user Inclusion Proof verified in contract

Expand All @@ -208,7 +207,7 @@ As all the cell values are zero, it might not be a critical issue in the CEX use

#### Refer

- [Fake user Inclusion Proof verified in contract](https://github.com/zBlock-2/summa-solvency/issues/9) by [rkdud007](), [qpzm]()
- [Fake user Inclusion Proof verified in contract](https://github.com/zBlock-2/summa-solvency/issues/9) by [rkdud007](), [qpzm](https://github.com/qpzm)

## 4. High: Missing username range check in `big_intify_username` & `big_uint_to_fp`

Expand All @@ -230,9 +229,9 @@ We recommend that a range check is done inside the circuit or inside the smart c

#### Refer

- [Username overflow](https://github.com/zBlock-2/summa-solvency/issues/2) by [bbresearcher]()
- [Possible Overflow in username in big_intify_username combined with calling big_uint_to_fp](https://github.com/zBlock-2/summa-solvency-diffie/issues/16) by [bbresearcher]()
- [Guarantee usernames stays inside field](https://github.com/zBlock-2/summa-solvency-schneier/issues/13) by [sebastiantf]()
- [Username overflow](https://github.com/zBlock-2/summa-solvency/issues/2) by [bbresearcher](https://github.com/bbresearcher)
- [Possible Overflow in username in big_intify_username combined with calling big_uint_to_fp](https://github.com/zBlock-2/summa-solvency-diffie/issues/16) by [bbresearcher](https://github.com/bbresearcher)
- [Guarantee usernames stays inside field](https://github.com/zBlock-2/summa-solvency-schneier/issues/13) by [sebastiantf](https://github.com/sebastiantf)

## Medium

Expand All @@ -256,7 +255,7 @@ Furthermore, [`verifyProof`](https://github.com/summa-dev/summa-solvency/blob/fe

#### Refer

- [The return value of `GrandSumVerifier` should be tested](https://github.com/zBlock-2/summa-solvency/issues/4) by [qpzm]()
- [The return value of `GrandSumVerifier` should be tested](https://github.com/zBlock-2/summa-solvency/issues/4) by [qpzm](https://github.com/qpzm)

## Low

Expand All @@ -268,7 +267,7 @@ To Reproduce this, [change `balanceByteRange` to 9](https://github.com/summa-dev

#### Refer

- [Incorrect permutation length in `validateVKPermuationsLength`](https://github.com/zBlock-2/summa-solvency/issues/6) by [zeroqn]()
- [Incorrect permutation length in `validateVKPermuationsLength`](https://github.com/zBlock-2/summa-solvency/issues/6) by [zeroqn](https://github.com/zeroqn)

## 2. Low: CSV parsing allows duplicate crypto tokens

Expand All @@ -293,7 +292,7 @@ Running the command `cargo run --release --example summa_solvency_flow` still ge

#### Refer

- [CSV parsing allows duplicate crypto tokens](https://github.com/zBlock-2/summa-solvency/issues/5) by [bbresearcher]()
- [CSV parsing allows duplicate crypto tokens](https://github.com/zBlock-2/summa-solvency/issues/5) by [bbresearcher](https://github.com/bbresearcher)

## 3. Low: The return value of `GrandSumVerifier.verifyProof` isn't tested

Expand All @@ -319,13 +318,13 @@ This [test data](https://github.com/summa-dev/summa-solvency/blob/fec83a747ead21
2. user1 balance1
```

The test result, however, shows that 4 balances break the range check.
The test result, however, shows that 4 balances break the range check.

For example, user2 balance0 is zero, but the test error contains `Perform range check on balance 0 of user 2`. Refer to the assertion [here](https://github.com/summa-dev/summa-solvency/blob/fec83a747ead213261aecfaf4a01b43fff9731ee/prover/src/circuits/tests.rs#L453-L484)

#### Refer

- [Range check tests are unreliable regarding the number of overflows](https://github.com/zBlock-2/summa-solvency/issues/3) by [qpzm]() , [pia]()
- [Range check tests are unreliable regarding the number of overflows](https://github.com/zBlock-2/summa-solvency/issues/3) by [qpzm](https://github.com/qpzm) , [rkdud007](https://github.com/rkdud007)()

## Informational

Expand Down Expand Up @@ -374,7 +373,7 @@ The following sanity check `require(bytes(cryptocurrencies[i].chain).length <= c
#### Refer
- [Security concerns in `Summa.sol`](https://github.com/zBlock-2/summa-solvency/issues/23) by [hrishibhat]()
- [Security concerns in `Summa.sol`](https://github.com/zBlock-2/summa-solvency/issues/23) by [hrishibhat](https://github.com/hrishibhat)
## 2. Informational: Update memory locations in verifiers to save gas
Expand All @@ -384,26 +383,25 @@ However, it can start from `0x180`.
The maximum memory size used in both verifiers is `0x180`, so it is recommended to save constants from memory `0x180`. The saved memory is 160 and 128 bytes respectively.
#### Refer
- [Update memory locations in verifiers to save gas](https://github.com/zBlock-2/summa-solvency/issues/18) by [qpzm]() and [pia]()
#### Refer
- [Update memory locations in verifiers to save gas](https://github.com/zBlock-2/summa-solvency/issues/18) by [qpzm](https://github.com/qpzm) and [rkdud007](https://github.com/rkdud007)()
## 3. Informational: Dynamic Errors not handled in Box of Errors for Function Results
When `Box` pointer is used on function returning a result, the underlying error type is only known at runtime and not statically known. There are instances in code where `Box` is used but there is no handling of dynamic errors in the function.
#### Refer
- [Dynamic Errors not handled in Box of Errors for Function Results](https://github.com/zBlock-2/summa-solvency/issues/8) by [sachindkagrawal15]()
- [Dynamic Errors not handled in Box of Errors for Function Results](https://github.com/zBlock-2/summa-solvency/issues/8) by [sachindkagrawal15](https://github.com/sachindkagrawal15)
## 4. Informational: Use of unwrap in core files and improper error handling
Unwrap is usually a shortcut to bypass implementing proper error handling in Rust. Using Unwrap in production code is not recommended as it crashes the application & hides actual bugs. It is highly undesirable in core files and is a Rust bad practice. Proper error handling/transmission should be implemented.
#### Refer
#### Refer
- [Use of unwrap in core files and improper error handling](https://github.com/zBlock-2/summa-solvency/issues/7) by [sachindkagrawal15]()
- [Use of unwrap in core files and improper error handling](https://github.com/zBlock-2/summa-solvency/issues/7) by [sachindkagrawal15](https://github.com/sachindkagrawal15)
## Final remarks
Expand Down Expand Up @@ -491,4 +489,7 @@ Highlighter works on a set of rules to look for error prone areas such as incorr
### 6. Clippy
`clippy` is a linter for Rust that checks your code for common mistakes and style issues. It provides helpful suggestions to improve your code quality and maintainability. Using `clippy` helps ensure your Rust code is clean, efficient, and follows best practices. Here's the [report](https://github.com/zBlock-2/audit-report/blob/main/appendix/V2/clippy/output.md).
`clippy` is a linter for Rust that checks your code for common mistakes and style issues. It provides helpful suggestions to improve your code quality and maintainability. Using `clippy` helps ensure your Rust code is clean, efficient, and follows best practices. Here's the [report](https://github.com/zBlock-2/audit-report/blob/main/appendix/V2/clippy/output.md).
## B - Fuzzing

0 comments on commit 4b66f3d

Please sign in to comment.