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-02 ERC-20 Approval Issues #1920

Merged

Conversation

sparrowDom
Copy link
Member

Audit notes:

  • A single approval is used to grant the approval for an asset token. However, an asset
    token like USDT can be non-compliant and may revert if the approval is not reset before
    this approval call. This may block repeated execution of safeApproveAllTokens .
    Although the LST tokens used in the current strategy do not have this issue, if the code
    is extended to also use Balancer strategies for other assets (e.g., for the tokens backing
    OUSD), this may become an issue. Consider documenting this possibility or renaming
    the function to _approveERC20CompliantAsset . Alternatively, consider following the
    double approval pattern.
  • A double approval is used for the BPT. However, this is unnecessary as the BPTs are
    ERC-20-compliant tokens. Consider only doing one approve in this case.

@rafaelugolini rafaelugolini temporarily deployed to preview-ousd-sparrowdom-zkw2uz November 7, 2023 23:48 Inactive
Copy link

github-actions bot commented Nov 7, 2023

Warnings
⚠️ 👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against 1c71d64

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

❗ No coverage uploaded for pull request base (sparrowDom/balancer-composable-st-pool@4ac3998). Click here to learn what that means.
The diff coverage is n/a.

@@                            Coverage Diff                            @@
##             sparrowDom/balancer-composable-st-pool    #1920   +/-   ##
=========================================================================
  Coverage                                          ?   57.41%           
=========================================================================
  Files                                             ?       51           
  Lines                                             ?     2724           
  Branches                                          ?      703           
=========================================================================
  Hits                                              ?     1564           
  Misses                                            ?     1157           
  Partials                                          ?        3           

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@sparrowDom sparrowDom merged commit 0d45ea7 into sparrowDom/balancer-composable-st-pool Nov 8, 2023
14 of 18 checks passed
@sparrowDom sparrowDom deleted the sparrowDom/N-02-approval branch November 8, 2023 20:35
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