Omniscia rain protocol Audit
SeedERC20 Code Style Findings
SeedERC20 Code Style Findings
SER-01C: Potential Legibility Optimization
| Type | Severity | Location |
|---|---|---|
| Code Style |  | SeedERC20.sol:L99, L101, L104 |
Description:
The linked constant declarations are sequential and are meant to represent different states of the contract.
Example:
97/// Phase constants.98/// Contract is uninitialized.99uint256 private constant PHASE_UNINITIALIZED = 0;100/// Minimum seed funds have not yet been reached so seeding is in progress.101uint256 private constant PHASE_SEEDING = 1;102/// Minimum seed funds were reached so now tokens can be redeemed but not103/// purchased from or refunded to this contract.104uint256 private constant PHASE_REDEEMING = 2;Recommendation:
We advise an enum declaration to be utilized instead, enabling a more uniform state representation style across the contract that is also extensible. Enums under the hood are represented by 0-based index entries based on their order of declaration (i.e. enum Foo { A, B } would cause A = 0, B = 1). Type casting is also available from an enum to a uint256 and any other numeric literal, fulfilling the purposes of the contract. In general, enum usage should be considered across the contract as this finding will not be repeated.
Alleviation:
After discussion with the Rain Protocol team, we concluded that such a change would force the code to utilize the uint8 data type underneath the enum and thus lead to increased gas costs across the board. As a result, we consider this exhibit nullified.
SER-02C: Potentially Convoluted Redemption Protection Mechanism
| Type | Severity | Location |
|---|---|---|
| Gas Optimization | | SeedERC20.sol:L303-L318 |
Description:
The redemption protection mechanism the redeem function imposes relies on a complex highwater maintenance method whereby the maximum reserve supply ever present in the contract is tracked and used as a safety guard.
Example:
276/// Burn seed tokens for pro-rata reserve assets.277///278/// ```279/// (units * reserve held by seed contract) / total seed token supply280/// = reserve transfer to `msg.sender`281/// ```282///283/// The recipient or someone else must first transfer reserve assets to the284/// `SeedERC20` contract.285/// The recipient MUST be a TRUSTED contract or third party.286/// This contract has no control over the reserve assets once they are287/// transferred away at the start of `Phase.ONE`.288/// It is the caller's responsibility to monitor the reserve balance of the289/// `SeedERC20` contract.290///291/// For example, if `SeedERC20` is used as a seeder for a `Trust` contract292/// (in this repo) it will receive a refund or refund + fee.293/// @param units_ Amount of seed units to burn and redeem for reserve294/// assets.295/// @param safetyRelease_ Amount of reserve above the high water mark the296/// redeemer is willing to writeoff - e.g. pool dust for a failed raise.297function redeem(uint256 units_, uint256 safetyRelease_)298 external299 onlyPhase(PHASE_REDEEMING)300{301 uint256 currentReserveBalance_ = reserve.balanceOf(address(this));302
303 // Guard against someone accidentally calling redeem before the reserve304 // has been returned. It's possible for the highwater to never hit the305 // `safeExit`, notably and most easily in the case of a failed raise306 // there will be pool dust trapped in the LBP, so the user can specify307 // some `safetyRelease` as reserve they are willing to write off. A308 // less likely scenario is that reserve is sent to the seed contract309 // across several transactions, interleaved with other seeders310 // redeeming, thus producing a very low highwater. In this case the311 // process is identical but manual review and a larger safety release312 // will be required.313 uint256 highwater_ = highwater;314 if (highwater_ < currentReserveBalance_) {315 highwater_ = currentReserveBalance_;316 highwater = highwater_;317 }318 require(highwater_ + safetyRelease_ >= safeExit, "RESERVE_BALANCE");319
320 IERC20[] memory assets_ = new IERC20[](1);321 assets_[0] = reserve;322 _redeem(assets_, units_);323}Recommendation:
We advise a paradigm similar to DEX implementations to be utilized here whereby the user supplies the units_ they wish to redeem as well as the minimumReserve_ they wish to acquire, increasing the accuracy of the protection mechanism and removing the necessity of a complex highest-balance measurement maintenance system.
Alleviation:
The Rain Protocol team stated that they wish to retain the convoluted nature in place as it can protect against misconfigured "minimum" values that would otherwise be present. Additionally, they stated that they intend to remove seeding altogether in a future update. Given those facts, we consider this exhibit acknowledged.