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

Fix remaining dust on oToken when burning all #1672

Closed
wants to merge 1 commit into from

Conversation

DanielVF
Copy link
Collaborator

@DanielVF DanielVF commented Jun 29, 2023

Makes OUSD/OETH not leave dust when the entire balance of an account is redeemed. Closes #1495

To detect when we were burning the entire funds for an account, the old code did this:

currentCredits == creditAmount || currentCredits - 1 == creditAmount

This would not detect the problem if the creditAmount rounding error differed by more than 1.

Changed the code to the much more direct:

amount == balanceOf(account)

Maybe 1K gas more, but we only burn on redeems (super rare), and some AMO allocations.

Also:

  • Added some helpful comments around the rounding behavior in the function
  • Removed safe math from this function

Contract change checklist:

  • Code reviewed by 2 reviewers.
  • Copy & paste code review security checklist below this checklist.
  • Unit tests pass
  • Slither tests pass with no warning
  • Echidna tests pass if PR includes changes to OUSD contract (not automated, run manually on local)

@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-danielvf-b-0jvvnc June 29, 2023 21:32 Inactive
@DanielVF DanielVF added the contracts Works related to contracts label Jun 30, 2023
@DanielVF
Copy link
Collaborator Author

DanielVF commented Jul 4, 2023

Note that I've got an another, possibly better fix version coming.

No need to review this one.

@DanielVF
Copy link
Collaborator Author

DanielVF commented Jul 4, 2023

I'm just going to close this since I like #1680 so much better.

@DanielVF DanielVF closed this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Works related to contracts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix rounding issue in OUSD redeemAll unit test
2 participants