In reserve.sol, both a local balance_ variable which is incremented and subtracted at deposits and payouts, and the direct balanceOf(pot) value are being used in calculations involving the maker adapter integration: L118, L143. If someone were to send at least 1 wei dai to the reserve, balanceOf(pot) will be larger than balance_, which can lead to blocking payouts or deposits from a failing safeSub in _payoutAction, causing a system deadlock.

While the maker stability fee is correctly calculated by the following expression in clerk.debt():

rmul(art, rmul(rpow(safeAdd(jug.base(), duty), safeSub(block.timestamp, rho), ONE), rateIdx));

the function clerk.stabilityFee fails to take jug.base() into account, which can lead to an incorrect calculation of the cdp debt in assessor.remainingCredit, in the worst case reverting a safeSub, blocking calls to assessor.seniorBalance() and subsequently deadlocks epoch execution.

n.b. that in practice, the jug.base() variable has always been zero since the deployment of multicollateral dai and stability fees have been adjusted on a per ilk basis.

When due some rounding error seniorTranche.tokenSupply() is left with dust (e.g. 1 instead of 0), the check in assessor.sol#L167 during _calcSeniorTokenPrice is missed and the seniorTokenPrice can end up being 0 (in case the pool only has junior investors). This can lead to multiple issues,

• a division by `0` at `clerk.sol#L231` where the seniorTokenPrice ends up being 0
• when a senior investor tries to redeem they will be stuck calling disburse due to another division by zero in `tranche.sol#L167`.

This scenario only occurs when seniorTokenRatio is allowed to be `0` and the pool only consists out of junior investors.

It's taken to account in reBalance but the `seniorRatio` is not updated in repaymentUpdate and so it would be wrong adjusting the senior balance and debt. Needs more analysis on the borrower side.

The total assets held by the tinlake system at any point in time is given by reserves + NAV; idle deposits kept in the reserve and the net asset value of all outstanding loans (see https://developer.centrifuge.io/learn/understanding-tinlake/#nav for details).

However, in between the two stages of borrowing, shelf.borrow and shelf.withdraw, a loan is considered active and counted towards the NAV, while the borrowed amount still sits in the reserve, effectively being counted twice.

This poses a particular problem for pools with Maker integration, as when additional DROP are minted to provide collateral for the CDP, the seniorRatio is adjusted too far down, as assets are overvalued, leaving senior investors with too little interest accruing stake in the loan (seniorBalance becomes too low).

Luckily, this inaccuracy in the seniorRatio only remains until reserve.balance() is called, which happens at every epoch, so senior investors only miss out on a portion of one epochs interest.

The method assessor.dripSeniorDebt() applies interest on the seniorDebt, i.e. the value of of current outstanding loans that are generating interest for DROP holders.

However, the storage variable lastUpdateSeniorInterest is only updated when seniorDebt actually has increased:

if (newSeniorDebt > seniorDebt_) {
    seniorDebt_ = newSeniorDebt;
    lastUpdateSeniorInterest = block.timestamp;
}

which means that the initialization of seniorDebt does not update this variable, and a retroactive, instantaneous interest is applied since the opening of the pool.

In pools without maker integration, this scenario does not occur since the lastUpdateSeniorInterest variable is updated in borrowUpdate (of which dripSeniorDebt() is a subcall), but changes to seniorBalance as a result of changeSeniorAsset will lead to too much interest being applied.

The calcSeniorTokenPrice() and calcSeniorTokenPrice() methods on the Assessor use different values for the NAV calculation. In calcSeniorTokenPrice() the approximated NAV is used, in calcJuniorTokenPrice() the current NAV is used. Callers of these methods in the Assessor may receive inconsistent results.

calcJuniorTokenPrice() is not used within tinlake. calcSeniorTokenPrice is used by the Clerk. The currentNAV in the current version of the navfeed is too expensive to use in this context, so calcJuniorTokenPrice should be modified to use the approximated NAV.

Events should be used in places where a general state-change takes place, (e.g. such as file or rely calls) in order to monitor state-changes and make the history queryable.

When calling closeEpoch() there is no indication of whether the epoch successfully executed or went into a submission period. Currently a caller will always need to call submissionsPeriod afterward. It would be more convenient if this was simply returned by the closeEpoch() method. In case the epoch cannot be closed immediately, it would be good to return the error code to inform the caller what constraint cannot be fulfilled.

All state variables which are assigned in the constructor and never changed should be marked as immutable. This will save gas by putting them directly in the contract code instead of in the storage and make the code more readable.

The global variable address self which is set to address(this) in the constructor and never changed is redundant and more expensive then a simple address(this) statement. It should be replaced.

The argument order for RestrictedTokenFab.newRestrictedToken(string name, string symbol) is reversed from the underlying call. It should be reversed in order to prevent confusion and comply with the order defined by the ETC20 interface.

memberAdmin

poolAdmin

Both files have updateMembers() functions for testing and test names which indicate they are being used, but the pluralized version of the tests are in fact calling the singular updateMember() helpers.

• setting lastUpdateSeniorInterest at the end of repaymentUpdate / borrowUpdate
• repeated checks in dripSeniorDebt / chargeInterest / seniorDebt()

Both _calcSeniorTokenPrice and _calcJuniorTokenPrice have comments stating that the maker creditline is included in the token price calculations, however this is not the case for either method.

The totalBalance method of the Reserve simply returns the value stored in the (public) balance_ storage variable. This method could be auto-generated by solidity if balance_ was renamed to totalBalance, resulting in a minor code simplification.

Using enums instead of just constants improves compile time guarantees and readability of the code.

If either function is being called with two parameters, the second parameter is ignored and defaults to reserve.totalBalance(). To prevent confusion, this parameter could be removed entirely from the function.

The deployer for the borrower module has an interface named NFTFeedLike, this expects an init() method to be available. However the actual NFTFeed has no such method, and the interface should instead be renamed to NAVFeedLike to reflect the true intention.

The Reserve contract is responsible for securing and tracking the amount of loanable currency in the system. The reserve's current balance is tracked in a cached storage variable (balance_) which is updated on each call to deposit or hardDeposit. This approach is more gas efficient that checking the balance on the token directly (with a call to balanceOf), but does make some assumptions about token semantics that may not always hold. As an example, if the currency token takes a transfer fee the balance_ variable will be incorrect with respect to the actual balance of the reserve.

New currency tokens should be carefully audited to make sure that they do not violate the expectations of the Reserve.

As there can be multiple asset classes with different risk profiles in a Tinlake pool, it is possible to have loans whose interest rates are lower than DROP returns. In such settings, asset originators can use this in their favor and borrow money and provide senior investments with borrowed funds. As long as the system can sustain their DROP yields (from other, higher interest loans), this provides a net yield for these actors, in effect extracted from TIN holders in the system.

Borrowers can also collude with a portion of the TIN investors to extract value from other TIN holders by performing the following scenario:

1. Before borrowing, acquire TIN by supplying an order and wait for the epoch to settle.
2. Take out a large loan, increasing the NAV of the system, and therefore TIN value.
3. Cash out TIN position at higher price (requires sufficient funds in the reserve).
4. Repay loan prematurely, before too much interest has accrued.

This attack is essentially possible since NAV calculations assumes loans will be paid back with interest in the future, and not be paid back prematurely.