DistributedCollective / DistributedCollective/Sovryn-smart-contracts
Refactor code to provide better fees calculations in LoanOpenings.sol
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 132
- Forks
- 57
- PR merge metrics
- No merged PRs in 30d
Description
Description
For example, let’s say the fee rate is 0.3% and someone
wants to deposit around 1,000 DAI for borrowing around 2 ETH. If we calculate the depositAmount
from borrowAmount, 1,000 DAI could borrow 2 × (1, 000∕1, 003) = 1.99401795 ETH. But, if we
calculate the borrowAmount from depositAmount, 2× (1− 0.3%) = 1.994 ETH could be borrowed. Here,
we see the 1.99401795 − 1.994 = 0.00001795 ETH difference, which is due to the inconsistent fee
calculation. A fair calculation should be 𝑑𝑒𝑝𝑜𝑠𝑖𝑡𝐴𝑚𝑜𝑢𝑛𝑡 = 𝑑𝑒𝑝𝑜𝑠𝑖𝑡𝐴𝑚𝑜𝑢𝑛𝑡∕(1 − 0.3%).
Recommendation Fix the fee calculation on the depositAmount side as 𝑑𝑒𝑝𝑜𝑠𝑖𝑡𝐴𝑚𝑜𝑢𝑛𝑡 =
𝑑𝑒𝑝𝑜𝑠𝑖𝑡𝐴𝑚𝑜𝑢𝑛𝑡∕(1 − 𝑓 𝑒𝑒).
;
Besides the new formula reduces calculations saving some gas for every operation.
Problem
Changing the formula in the contract leads to the some tests failures due to discrepancies in the returning results for tests from other contracts which requires further research and probably redesign/refactoring of the code to comply with DRY design principle and respecting data encapsulation to fix such inconsistencies.
Links
https://github.com/DistributedCollective/Sovryn-smart-contracts/blob/878614979f06314a1574260a9c5e7e88cfd6b448/contracts/modules/LoanOpenings.sol#L123
https://github.com/DistributedCollective/Sovryn-smart-contracts/blob/878614979f06314a1574260a9c5e7e88cfd6b448/contracts/modules/LoanOpenings.sol#L143
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with contracts/modules/LoanOpenings.sol at the linked lines 123 and 143, then reproduce the test failures caused by changing the depositAmount fee formula. Trace the affected tests in the other contracts and determine the refactoring needed to address the stated DRY and data-encapsulation concerns. Done means the revised calculation is consistent and the affected tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- blockchain
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100