QA Report

[code423n4]

QA Report

1. Open TODOS

Consider resolving the TODOs before deploying.

PooledCreditLine/PooledCreditLine.sol:794:        // TODO: Can directly transfer to borrower and cut down on 1 transfer

2. Implicit constant visibility

Consider explicitly marking those as internal:

PooledCreditLine/PooledCreditLine.sol:33:    uint256 constant YEAR_IN_SECONDS = 365 days;
PooledCreditLine/PooledCreditLine.sol:34:    uint256 constant SCALING_FACTOR = 1e18;

3. All initialize() functions are front-runnable in the solution.

I suggest adding some access control to them:

PooledCreditLine/LenderPool.sol:
  90:     function initialize() external initializer {}

PooledCreditLine/PooledCreditLine.sol:
  407:     function initialize(

Verification/twitterVerifier.sol:
  57:     function initialize(

The one in LenderPool.sol doesn't do anything though

4. Typo in revert strings

Replace:

PooledCreditLine/LenderPool.sol:219:            revert("cant withdraw");
PooledCreditLine/PooledCreditLine.sol:484:        require(_protocolFeeCollector != address(0), 'cant be 0 address');
Verification/twitterVerifier.sol:125:        require(bytes(_userdata).length != 0, 'User doesnt exists');
Verification/twitterVerifier.sol:137:        require(bytes(_userdata).length != 0, 'User does not exists');

with

PooledCreditLine/LenderPool.sol:219:            revert("can't withdraw");
PooledCreditLine/PooledCreditLine.sol:484:        require(_protocolFeeCollector != address(0), "can't be 0 address");
Verification/twitterVerifier.sol:125:        require(bytes(_userdata).length != 0, 'User does not exist');
Verification/twitterVerifier.sol:137:        require(bytes(_userdata).length != 0, 'User does not exist');

5. A revert string shouldn't be empty

PooledCreditLine/LenderPool.sol:207:                require(_isLiquidationWithdrawn, "");

6. approve should be replace with safeApprove

approve is subject to a known front-running attack. Consider using safeApprove instead:

PooledCreditLine/LenderPool.sol:110:            SAVINGS_ACCOUNT.approve(_token, address(POOLED_CREDIT_LINE), type(uint256).max);
PooledCreditLine/PooledCreditLine.sol:777:            IERC20(_collateralAsset).approve(_strategy, _amount);
PooledCreditLine/PooledCreditLine.sol:840:        IERC20(_borrowAsset).approve(_strategy, _amount);

7. Immutable addresses should be 0-checked

Consider adding an address(0) check here:

File: LenderPool.sol
82:     constructor(
83:         address _pooledCreditLine,
84:         address _savingsAccount
85:     ) {
86:         POOLED_CREDIT_LINE = IPooledCreditLine(_pooledCreditLine); //@audit missing address(0) check on immutable address
87:         SAVINGS_ACCOUNT = ISavingsAccount(_savingsAccount); //@audit missing address(0) check on immutable address
88:     }

File: PooledCreditLine.sol
393:     constructor(address _lenderPool) {
394:         lenderPool = _lenderPool; //@audit missing address(0) check on immutable address
395:     }

8. Missing comments

The following comments are missing (see @audit tags):

PooledCreditLine/PooledCreditLine.sol:
   298:      * @param _max maximum threshold of the parameter //@audit missing @return bool
   405:      * @param _protocolFeeCollector address to which protocol fee is charged to  //@audit missing @param _borrowerVerifier & @param _verification
  1040:      * @param _id identifier for the credit line //@audit missing @return address & @return uint256

[ritik99]

It seems that an incorrect branch has been reviewed by the warden, the line numbers mentioned are not the same as the one which were shared for the contest

[HardlyDifficult]

Merging with #50

[HardlyDifficult]

Merging with #48