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

Update exchange rate in crank #100

Merged
merged 3 commits into from
Nov 28, 2022
Merged

Update exchange rate in crank #100

merged 3 commits into from
Nov 28, 2022

Conversation

ams9198
Copy link
Contributor

@ams9198 ams9198 commented Nov 26, 2022

I extended the tests in scenarios 2 and 3 to actually perform the withdraw at the end to confirm that the lenders received what they were told they were eligible for -- and there was a very slight mismatch, leading to an underflow when draining the pool.

This was due to the crank using an exchange rate of total assets available to be cranked as opposed to total assets earmarked by crank due to the withdrawal gate. Since the former is larger, I believe the exchange rate had more precision / was larger, which led to the eligible amounts having dust that wasn't actually there.

The amounts are small, but I think this should guarantee that a lender is always able to claim what is being advertised.

// Calculate the redeemable rate for each lender
uint256 redeemableRateRay = redeemableShares.mul(PoolLib.RAY).div(
globalState.eligibleShares
);

// Calculate the exchange rate for the snapshotted funds
uint256 fxExchangeRate = availableAssets.mul(PoolLib.RAY).div(
availableShares
uint256 fxExchangeRate = withdrawableAssets.mul(PoolLib.RAY).div(
Copy link
Contributor Author

@ams9198 ams9198 Nov 26, 2022

Choose a reason for hiding this comment

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

This was the bug fix

venables
venables previously approved these changes Nov 28, 2022
@ams9198 ams9198 merged commit 6e3e4ef into circlefin:master Nov 28, 2022
@ams9198 ams9198 deleted the roundingchecks branch November 28, 2022 19:57
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