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

N-01 Redundant Code #2320

Conversation

sparrowDom
Copy link
Member

Issue by Open Zeppelin team:
Multiple instances of redundant code were identified in the OUSD contract:

The initialize function cannot be executed since the contract was already initialized in previous versions. To avoid unnecessarily increasing code size, consider removing the function or commenting it out for future reference.
In the delegateYield function, checking both the yieldFrom/yieldTo mappings and the rebaseState mapping is redundant, as the first check will only pass if the second passes, and vice versa. Consider removing the first check, as it involves more storage reads than the second one.
In the undelegateYield function, there is no need to set the credit balance of the delegation source, as it was already set to their balance within delegateYield.
Consider removing these redundancies to enhance the clarity and efficiency of the codebase.

Origin comment:

  • We require the initialize function for any future fresh token contract deploys that would extend OUSD.sol
  • We recognize the redundancy of code in the delegateYield function and have intentionally written in that way in the spirit of code readability and defensive programming. The exaggeration of verbosity is in place to prevent any current or future changes that could introduce an error in this critical part.
  • Redundant storage slot write in undelegateYield has been removed

Copy link

github-actions bot commented Dec 4, 2024

Warnings
⚠️ 👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against ba0cd4f

@sparrowDom sparrowDom changed the title remove redundant setting of of yield delegation from account N-01 Redundant Code Dec 5, 2024
Copy link
Contributor

@clement-ux clement-ux left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@DanielVF
Copy link
Collaborator

DanielVF commented Dec 5, 2024

Perhaps we just make no changes on this one? Not sure it's worth it.

@sparrowDom sparrowDom closed this Dec 6, 2024
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.

3 participants