You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The list of possible improvements (in order of implementation priority):
1. Change/simplify naming approach of the contracts. See the Cashier repository for a reference. Concrete steps:
a. Merge interfaces:
i. ICreditLine and ICreditLineConfigurable ones.
ii ILiquidityPool and ILiquidityPoolAccountable ones.
b. Rename contracts:
i. CreditLineConfigurable => CreditLine.
ii. LiquidityPoolAccountable => LiquidityPool.
c. Merge primary contracts with the UUPS-ones and rename the result contracts with simpler names, e.g.:
i. CreditLineConfigurableUUPS => CreditLine;
ii. LiquidityPoolAccountableUUPS => LiquidityPool;
iii. LendingMarketUUPS => LendingMarket.
2. Consider removing the takeLoan() function that is called by the borrower. After that the addonAmount internal calculation logic, function calculateAddonAmount(), and the related fields of the BorrowerConfig and CreditLineConfig structures can be removed as well. The related discussion in Slack: link.
3. Consider improving and add new role types. Allow multiple owners for the credit line contract and maybe other contracts through the AccessControl parent contract. Currently there can be only one account with the OWNER_ROLE role: the lender account passed in the initialization parameters. And there is no mechanism to transfer the ownership to another account. A fix can be added to the following code snippet within an internal initializing function that currently does not assign the admin role for the OWNER_ROLE role:
4. Add possibility to configure alias accounts and other roles on the contracts in a batch. This step can be implemented along with the one above.
5. Consider removing of the _addonsBalance logic from the liquidity pool contract. At the same time if the _addonTreasury address is zero related functions must be reverted.
6. Double check the possibility of zero/empty token transfers and consider removing them or reverting with an explicit error.
7. Move the late fee amount calculation to the credit line.
8. Make sure the value of the addonAmount parameter of the takeLoanFor() function is not greater than expected for the internal function call. Check other similar cases if appropriate.
9. Implement internal functions to check that the loan is closed and that the loan exists instead of checking the Loan.State.trackedBalance or Loan.State.borrower fields to zero. Use them in the modifiers (e.g. onlyOngoingLoan ) and in external functions, which can then also be used in tests.
10. Consider to make the isLenderOrAlias() function external and introduce the appropriate internal function. Check that the new approach is more effective in terms of the code size and/or gas usage. Consider other public function the same way.
11. Add the check that a liquidity pool has enough tokens for a loan. Currently in this case a transaction is reverted with a panic due to arithmetic underflow.
12. Simplify mock contracts for testing, remove redundant functions.
13. Add more tests cases for the revokeInstallmentLoan() function. Use test cases of the revokeLoan() function.
14. Add missing tests for the case of discounting frozen loan.
15. Rename outstandingBalance => trackedBalance in places where we actually use or get the second one unrounded.
17. Consider historical backward compatibility. What do we do in the case of breaking changes? Do we keep older functions forever?
18. Add the ability to roll back a repayment. This may be necessary in case of errors on the backend side.
19. Add the ability to make a payment in the past (for example, if the loan status is yesterday, a week ago, etc.). This may be necessary in case of errors on the backend side.
20. Consider removing the repayLoan() function.
21. Consider replacing the type of all the Terms structure fields with the native EVM type. This may result in lower gas consumption and smaller code size.
The text was updated successfully, but these errors were encountered:
The list of possible improvements (in order of implementation priority):
Cashier
repository for a reference. Concrete steps:ICreditLine
andICreditLineConfigurable
ones.ILiquidityPool
andILiquidityPoolAccountable
ones.CreditLineConfigurable
=>CreditLine
.LiquidityPoolAccountable
=>LiquidityPool
.CreditLineConfigurableUUPS
=>CreditLine
;LiquidityPoolAccountableUUPS
=>LiquidityPool
;LendingMarketUUPS
=>LendingMarket
.takeLoan()
function that is called by the borrower. After that theaddonAmount
internal calculation logic, functioncalculateAddonAmount()
, and the related fields of theBorrowerConfig
andCreditLineConfig
structures can be removed as well. The related discussion in Slack: link.AccessControl
parent contract. Currently there can be only one account with theOWNER_ROLE
role: the lender account passed in the initialization parameters. And there is no mechanism to transfer the ownership to another account. A fix can be added to the following code snippet within an internal initializing function that currently does not assign the admin role for theOWNER_ROLE
role:_addonsBalance
logic from the liquidity pool contract. At the same time if the_addonTreasury
address is zero related functions must be reverted.zero
/empty
token transfers and consider removing them or reverting with an explicit error.addonAmount
parameter of thetakeLoanFor()
function is not greater than expected for the internal function call. Check other similar cases if appropriate.Loan.State.trackedBalance
orLoan.State.borrower
fields to zero. Use them in the modifiers (e.g.onlyOngoingLoan
) and in external functions, which can then also be used in tests.isLenderOrAlias()
function external and introduce the appropriate internal function. Check that the new approach is more effective in terms of the code size and/or gas usage. Consider other public function the same way.revokeInstallmentLoan()
function. Use test cases of therevokeLoan()
function.outstandingBalance
=>trackedBalance
in places where we actually use or get the second one unrounded.borrowAmount
=>borrowedAmount
, except maybe structs for backward compatibility.repayLoan()
function.Terms
structure fields with the native EVM type. This may result in lower gas consumption and smaller code size.The text was updated successfully, but these errors were encountered: