Illuminate contest - ElKu's results

1) The coding style should be consistent throughout the project.

For example, the function declarations are generally done in multiline, but in some cases written over a single long line. References below:

a. function setPrincipal(uint8 p, address u, uint256 m, address a) external authorized(admin) returns (bool) { b. function mint(address u, uint256 m, uint256 uA, uint256 ptA, uint256 minRatio, uint256 maxRatio) external returns (uint256, uint256, uint256) { c. function mintWithUnderlying(address u, uint256 m, uint256 a, uint256 ptBought, uint256 minRatio, uint256 maxRatio) external returns (uint256, uint256, uint256) { d. function burn(address u, uint256 m, uint256 minRatio, uint256 maxRatio) external returns (uint256, uint256, uint256) { e. function burnForUnderlying(address u, uint256 m, uint256 minRatio, uint256 maxRatio) external returns (uint256, uint256) { f. function approve(address[] calldata u, address[] calldata a) external authorized(admin) returns (bool) {

2) Use type(uint256).max to get the max value of a uint256, for readability

uint256 max = 2**256 - 1; on lines 84 and 112

3) Markets, Principles and Pools are immutable:

Once the market is created or the principle is set, it cannot be changed.

Impact:

If the admin makes a mistake in setting these addresses the first time, then there is no way to change it later.

Mitigation:

Since the admin is assumed to be trusted and has been given enough authority, he should be able to change the market addresses too. Since the purpose for admin to be able to change the market address is that he can rectify his mistake(for example a typing error the first time he sets it), we can put a maximum timePeriod for this. When the address is set for the first time, we set a variable as follows:

//Admin can  change the address multiple times for the next one hour. 
uint256 adminFlexibilityTimestamp = block.timestamp + 3600;  

4) Redundant code statements:

Redundant code statements in the if else statement. The code could be rewritten as:

if (p == uint8(MarketPlace.Principals.Yield)) {
            // Purchase yield PTs to lender.sol (address(this))
            uint256 returned = yield(u, y, a - calculateFee(a), address(this));
            // Mint and distribute equivalent illuminate PTs
            IERC5095(principalToken(u, m)).mint(msg.sender, returned);
        } else {
            // Purchase illuminate PTs directly to msg.sender
            uint256 returned = yield(u, y, a - calculateFee(a), msg.sender);
        }
        emit Lend(p, u, m, returned);
        return returned;
}

5) Swap the operands inside the if statement for better readability.

if (address(0) != (address(uToken))) {

.Swap operands and remove the extra bracket from the 2nd operand.

if (address(uToken) != address(0))

6) Mistakes in natspec comments.

a) /// @notice sets the feenominator to the given value

is a copy pasted text from setFee function.

Spelling mistakes in natspec comments: a) intializes b) withdrawl c) prinicpal

7) Spelling mistakes in normal comments:

a) unsighed b) gauruntee c) remaing d) for for e) prinicipal

1) Use custom errors instead of require statements in Lender.sol:

This reduces both deployment and runtime gas usage. See this link for an example.

require (when != 0, 'no withdrawal scheduled'); and require (block.timestamp >= when, 'withdrawal still on hold');

2) Cache the length of the array o before using it in for loop

for (uint256 i = 0; i < o.length; ) {

could be rewritten as:

uint256 len = o.length;
for (uint256 i = 0; i < len; ) 

3) Use calldata instead of memory for external functions

Declare read-only dynamic inputs as calldata instead of memory for external functions.

a) address[8] memory t, b) uint256[] memory a,

These can be rewritten as: a) address[8] calldata t b) uint256[] calldata a