-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: remove old names, use new solidity features, add file for poolClosing #567
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -37,4 +37,4 @@ dapp test -m ':ContractName\.' | |||
For deploying the Tinlake contracts to mainnet or a testnet, view our deploy scripts in [Tinlake Deploy](https://github.com/centrifuge/tinlake-deploy). | |||
|
|||
## Community | |||
Join our public Slack channel: [Centrifuge Slack](http://centrifuge.io/slack). | |||
Join our public Discord: [Centrifuge Discord](https://centrifuge.io/discord/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
src/borrower/feed/navfeed.sol
Outdated
@@ -156,7 +155,7 @@ contract NAVFeed is BaseNFTFeed, Interest, Buckets, FixedPoint { | |||
// On borrow: the discounted future value of the asset is computed based on the loan amount and addeed to the bucket with the according maturity Date | |||
function _borrow(uint loan, uint amount) internal returns(uint navIncrease) { | |||
// ceiling check uses existing loan debt | |||
require(ceiling(loan) >= safeAdd(borrowed[loan], amount), "borrow-amount-too-high"); | |||
require(ceiling(loan) >= amount, "borrow-amount-too-high"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if we actually need both methods:
function ceiling(uint loan) public view returns (uint) {
if (borrowed[loan] > currentCeiling(loan)) {
return 0;
}
return safeSub(currentCeiling(loan), borrowed[loan]);
}
function currentCeiling(uint loan) public view returns(uint) {
bytes32 nftID_ = nftID(loan);
return rmul(nftValues[nftID_], ceilingRatio[risk[nftID_]]);
}
We only use ceiling
here it seems. We could just use currentCeiling
and remove the ceiling
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this requires a bit more investigation whether we're not relying on either externally. I'll add a separate ticket for this.
@@ -23,7 +23,7 @@ contract NAVTest is DSTest, Math { | |||
Hevm hevm; | |||
|
|||
function setUp() public { | |||
hevm = Hevm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D); | |||
hevm = Hevm(HEVM_ADDRESS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice
Co-authored-by: Manuel Polzhofer <[email protected]>
Co-authored-by: Manuel Polzhofer <[email protected]>
distributor
=>reserve
rename (closes rename the distributor depenency to reserve in the shelf contract #563)epochTicker
=>coordinator
renameds-note
with custom eventsuint(-1)
=>type(uint256).max
hevm = Hevm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
=>hevm = Hevm(HEVM_ADDRESS);
file()
forpoolClosing
in coordinator