Epic: FiRMv2 #90
Draft
Epic: FiRMv2 #90
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nourharidy
reviewed
Nov 7, 2024
| interface IEscrow { | ||
| function initialize(IERC20 _token, address beneficiary) external; | ||
| function onDeposit() external; | ||
| function onDepositCallBack() external returns(uint); |
| interface IDebtManager { | ||
| function debt(address borrower) external view returns(uint); | ||
| function totalDebt() external view returns(uint); | ||
| function dbrDeficit(address borrower) external view returns(uint); |
There was a problem hiding this comment.
Debt managers shouldn't have to deal with DBR and therefore this function shouldn't be on the interface. Maybe it can be replaced with a boolean inDeficit(address) returns (bool) function which would be universal whether using DBR or not.
| function dbrDeficit(address borrower) external view returns(uint); | ||
| function increaseDebt(address borrower, uint amount) external; | ||
| function decreaseDebt(address borrower, uint amount) external returns(uint); | ||
| function replenish(address borrower, uint amount) external; |
There was a problem hiding this comment.
Seems unused? imo replenishments can happen through the debt manager contract directly. Doesn't have to be done through the market anyway.
| keccak256( | ||
| abi.encode( | ||
| keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), | ||
| keccak256(bytes("DBR MARKET")), |
There was a problem hiding this comment.
Suggested change
| keccak256(bytes("DBR MARKET")), | |
| keccak256(bytes("FiRM MARKET")), |
| function withdrawInternal(address from, address to, uint amount) internal { | ||
| uint limit = getWithdrawalLimitInternal(from); | ||
| require(limit >= amount, "Insufficient withdrawal limit"); | ||
| require(dbr.deficitOf(from) == 0, "Can't withdraw with DBR deficit"); |
There was a problem hiding this comment.
Does this rule still apply? I think it should be replaced with the user's debtManager's deficit instead, no?
* Liquidation incentive linearly increase as borrower collateral factor decreases * Liquidation fee linearly decreases as borrower collateral factor decreases * Remove collateral factor, instead add max liquidation amount
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Base branch for firmv2