Skip to content
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

Support future yield borrow #116

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Support future yield borrow #116

wants to merge 7 commits into from

Conversation

Xuefeng-Zhu
Copy link
Contributor

No description provided.

@Xuefeng-Zhu Xuefeng-Zhu changed the title Integrate alchemix Support future yield borrow Oct 27, 2021
if (_totalCredit < _amount) {
uint256 _remainingAmount = _amount.sub(_totalCredit);
_cdp.totalDebt = _cdp.totalDebt.add(_remainingAmount).add(
_remainingAmount.mul(borrowFee).div(PERCENT_RESOLUTION)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Debt is increased with _remainingAmount.mul(borrowFee).div(PERCENT_RESOLUTION)
However nothing else is done with this amount.
For comparison function harvest() has this call to do something with the fees:

if (_feeAmount > 0) {
      token.safeTransfer(rewards, _feeAmount);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gpersoon I have updated the code. The fee will be sent to reward address now

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the number of xtoken send should also be subtracted with the _borrowFeeAmount
something like:
_amount = _amount.sub(__borrowFeeAmount);
Otherwise, who is paying for the fees?
(please doublecheck if my hunch is correct)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_amount should be the same.

Otherwise, who is paying for the fees?

The fee is paid by the user. The user needs to clear debt to get its collateral back

});

// describe('mint', () => {
// let depositAmt = parseEther('5000');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test for mint() is commented out.
While it is extra important because it includes the borrowFee now, so some additional test will be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is issue for setting whitelist in Transmuter. I cannot figure out why it is failing in the test

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do see you have an additional setWhitelist that isn't in the original.

// await token.connect(minter).approve(alchemist.address, depositAmt);
// await alUsd.connect(deployer).setWhitelist(alchemist.address, true);
// });

https://github.com/alchemix-finance/alchemix-protocol/blob/eec27cfd4a2540816ef9cbe3297fa1e0871237d1/test/contracts/Alchemist.spec.ts#L811-L812

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. I added it for debugging because setWhitelist is not working at line 811

// // await alUsd.connect(deployer).setWhitelist(alchemist.address, true);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants