Audit list

[rndquu]

Audit list

For the new production deployment we decided to deploy the following contracts (and related ones):

Dollar token related contracts for audit:

1. UbiquityDollarToken.sol
2. ERC20Ubiquity.sol

Diamond related contracts for audit:

1. Diamond.sol
2. AccessControlFacet.sol
3. LibAccessControl.sol
4. AccessControlInternal.sol
5. DiamondCutFacet.sol
6. LibDiamond.sol
7. LibAppStorage.sol
8. OwnershipFacet.sol
9. TWAPOracleDollar3poolFacet.sol
10. LibTWAPOracle.sol
11. UbiquityPoolFacet.sol
12. LibUbiquityPool.sol

Pre-audit notes

Architecture

The whole protocol consists of 3 main parts:

1. Dollar token which is meant to be a USD-pegged stablecoin. There exists the old (already deployed) version of the Dollar token but we decided to redeploy the Dollar token for the contract to be upgradeable.
2. Governance token for managing the Ubiquity DAO. We've already deployed the Governance token and we decided to keep using the already deployed version (although we've already prepared an upgradeable version of the Governance token) in order to preserve the UBQ (Governance) token transaction history.
3. Diamond proxy (ERC-2535) where the main facets are the following:
   a) AccessControlFacet: responsible for RBAC
   b) OwnershipFacet
   c) TWAPOracleDollar3poolFacet: responsible for fetching the Dollar/USD quote from the Curve's metapool. There is the old Curve's Dollar-3CRVLP metapool but we will redeploy it because we plan to use the upgradeable Dollar token version (while the old metapool uses the old Dollar token).
   d) UbiquityPoolFacet: responsible for minting/redeeming Dollar tokens in exchange for collateral tokens. Initially we plan to use only the LUSD token as collateral.

Useful links:

1. https://github.com/ubiquity/ubiquity-dollar/wiki/23.-Smart-Contracts-Architecture
2. https://github.com/ubiquity/ubiquity-dollar/wiki/22.-Smart-Contract-API (up to date)

Things to double-check

1. Diamond proxy is based (as far as I remember) on https://github.com/mudgen/diamond-3-hardhat so it makes sense to compare the original implementation with our own.
2. Check that UUPS upgradeability is implemented without errors in ERC20Ubiquity.
3. There are some places in the LibUbiquityPool (one, two) where balanceOf(address(this)) is used which means that tokens can be transferred directly to the contract hence balances and calculations can possibly be manipulated.
4. Check that chainlink price fetch is implemented without errors.
5. Check that LibTWAPOracle updates average prices correctly. The old Curve's metapool (which we plan to redeploy) has 40k Dollars in liquidity so we should make sure that it's hard to manipulate Curve's TWAP with 40k of liquidity which is pretty low.

@pavlovcik Notice:

1. I think we can drastically reduce the list as some of the contracts are copy-pasted from already audited contracts. We just need to compare them with the original versions. In the ideal world the contracts above must be audited. Let me know if we need to reduce the list.
2. The list may be updated when Collateral TWAP price miscalculation #830 or dApp: create UI for UbiquityPool #833 is ready

[rndquu]

@gitcoindev @molecula451 Feedback is welcome

[molecula451]

i think we should list a list of common known patterns, that are not mean to be an issue, i've been triggered by pavlovciks telegram post looking about newer solidity version which (im in favor) as a part of a major upgrade, but that we shouldn't run, we should just gradually refactor to newer versions if we're really up to

e.g of a known issue (that is not really an issue), but that we are aware

require in favor of custom error()

I am not against require, and im not against either custom error, i like both, so since we use a lot of require in the code, we keep this is a pattern, im pretty sure they gonna tell you to update to custom error and marked as an issue, like this, many others...

what you think rndqnuu

[molecula451]

overflow? known issue, we use safemath

[rndquu]

i think we should list a list of common known patterns, that are not mean to be an issue

It doesn't help with the audit. When you get a list of low category issues (like require vs custom error) you either fix it either write "Acknowledged" and don't fix anything to be consistent with the codebase. Overall I mean that preparing such a list in advance is not really necessary because in the end you will get a list of issues you described and will have to deal with them somehow anyway.

[0x4007]

1. I see these links are from your fork. Perhaps it makes sense to link from the origin?
2. This will be used to receive a price quotation. I imagine if there are small adjustments made between now and when the audit starts it should not change the price much (but I expect you to update the links accordingly and let us know if this occurs.)
3. I plan to link this issue directly to the Sherlock team so that we are all in sync.

[rndquu]

1. Yes, that's because Governance token contracts are introduced in this PR which is not yet merged. When it's merged I'll update URLs to be relative to the https://github.com/ubiquity/ubiquity-dollar repo.
2. ok
3. ok

[rndquu]

I've updated all the URLs to be relative to the ubiquity-dollar repo.

[molecula451]

all the URLs point out to the ubiquity-dollar repo but now the list has been clean up

[0x4007]

If there are critical issues in the UbiquityGovernance or UbiquityAlgorithmicDollarManager contracts then we will obviously redeploy the UBQ (Governance) token.

Those aren't on the audit todo list so this condition will never be met, right?

I think we can drastically reduce the list as some of the contracts are copy-pasted from already audited contracts. We just need to compare them with the original versions. In the ideal world the contracts above must be audited. Let me know if we need to reduce the list.

It isn't clear to me what you think should be removed but I already made peace with doing the full list you requested regarding costs

[rndquu]

If there are critical issues in the UbiquityGovernance or UbiquityAlgorithmicDollarManager contracts then we will obviously redeploy the UBQ (Governance) token.

Those aren't on the audit todo list so this condition will never be met, right?

UbiquityGovernance and UbiquityAlgorithmicDollarManager contracts are in the audit list (under the "Governance token related contracts for audit" section) so if there are critical issues there then we will have to redeploy.

I think we can drastically reduce the list as some of the contracts are copy-pasted from already audited contracts. We just need to compare them with the original versions. In the ideal world the contracts above must be audited. Let me know if we need to reduce the list.

It isn't clear to me what you think should be removed but I already made peace with doing the full list you requested regarding costs

We could remove Diamond proxy related contracts (because they are mostly copy-pasted from https://github.com/mudgen/diamond-3-hardhat) and some of the facets like ManagerFacet which is responsible solely for updating state variables.

[rndquu]

@pavlovcik @gitcoindev @molecula451

FYI I removed some of the contracts from the final audit list for the following reasons:

1. UbiquityGovernance.sol (deployed at 0x4e38D89362f7e5db0096CE44ebD021c3962aA9a0): plain (almost) ERC20 token
2. UbiquityAlgorithmicDollarManager.sol (deployed at 0x4DA97a8b831C345dBe6d16FF7432DF2b7b776d98): contains 0 business logic and used only for setting state variables.
3. ERC20Ubiquity.sol: plain (almost) ERC20 token. The only thing that can go wrong is improperly set access control roles but we'll check it ourselves.
4. TWAPOracle.sol: TWAP oracle implementation of deprecated (i.e. already deployed) contract.
5. ManagerFacet.sol: contains 0 business logic and used only for setting state variables.
6. DiamondLoupeFacet.sol: contains only view methods useful for frontend.

[molecula451]

im in favor too, the cleaned up list makes the most reasonable contracts to be audited, and some of the contracts that rnqnuu mentions are deprecated

[molecula451]

Upcoming judge day !! 🔥 🔥 😄 Dollar (UBQ) 🚀