# Jake's Audit Notes: IDEX AMM Client: IDEX AMM https://github.com/idexio/idex-contracts-silverton: main / c26dfb7 Audit type: Silverton ## Whitepaper & specification about the protocol https://docs.google.com/document/d/1llJxFet8RUlGwukkpelXt6ZM3fkUc7EONxmu6FnTzj4/edit# Fork of Pancake Swap which implements "hybrid liquidity pools". Order matching is completed off chain and supports limit orders with partial fills. ## Review of the protocol/implementation ### Exchange.sol [1] **Short invalidateOrderNonce period** [`invalidateOrderNonce()`](https://github.com/idexio/idex-contracts-silverton/blob/c26dfb78a5421e21c475db593fa337946a272333/contracts/Exchange.sol#L1186) is used to invalidate nonces in the event of the off chain Dispatch wallet being compromised. In this function, the provided time for invalidation must be less than `getOneDayFromNowInMs()`. This means that a user only has 24 hours lock time available. In the event of a high gas utilisation event on Ethereum, this could be prohibitive for a user to execute frequently. Moreover this assumes that a compromise would be detected and mitigated within a 24 hour window. Recommendations: [a] Consider allowing a longer period for users to prevent unauthorised spending of provided liquidity in the event of off chain service compromise. ### Governance.sol [1] **Centralisation of Power** All upgrades are controlled by an Admin address. Recommendations: [a] Consider moving to a DAO to control upgrades of functionality, or make this clear to the users of the service. ### Owned.sol [1] **Manual access grant/changes** OpenZeppelin provides a well used Access control library for managing RBAC user access. The implementation rolls its own. Recommendations: [a] Consider using https://docs.openzeppelin.com/contracts/2.x/access-control ### AssetRegistry.sol [1] **Maximum of 255 Asset Symbols** [`loadAssetBySymbol()`](https://github.com/idexio/idex-contracts-silverton/blob/c26dfb78a5421e21c475db593fa337946a272333/contracts/libraries/AssetRegistry.sol#L63) uses a `uint8` to iterate over the assets to load by symbol. This means there's an upper bound of 255 supported crypto assets in the exchange. If that is exceeded this function will not return the full range of assets by symbol. Recommendations: [a] Use a larger iterator to future proof the deployment to have more than 255 assets. ### Constants.sol [1] **Incorrect constant calculation** [`maxChainPropagationPeriod`]( https://github.com/idexio/idex-contracts-silverton/blob/c26dfb78a5421e21c475db593fa337946a272333/contracts/libraries/Constants.sol#L10) is set to one week with three second block intervals. This is not refelctive of actual block intervals on Ethereum in the real world, which typically come at 12-15 second intervals. As this is a security feature for nonce invalidations, further investigation is warrented to verify the impact of mistimed blocks which could be result in a 75% shorter time calculation than expected. Recommendations: [a] Verify the impact of incorrectly calculated block time for Ethereum deployments, and consider making this constant reflective of the block interval for the EVM chain that this is deployed on. ## Comments - This is going to be expensive to operate for the `onlyDispatcher` role. Has a L2 service been considered?