DistributedCollective / DistributedCollective/Sovryn-smart-contracts

Refactor code to provide better fees calculations in LoanOpenings.sol

Open
#190 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.