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/skipRevert-documentation-issue-19 #319

Closed
wants to merge 2 commits into from

Conversation

Jean-Grimal
Copy link
Contributor

@@ -15,7 +15,7 @@ import {BaseBundler} from "./BaseBundler.sol";
abstract contract UrdBundler is BaseBundler {
/// @notice Claims `amount` of `reward` on behalf of `account` on the given rewards distributor, using `proof`.
/// @dev Assumes the given distributor implements IUniversalRewardsDistributor.
/// @dev Pass `skipRevert = true` to avoid reverting the whole bundle in case the proof expired.
/// @dev Pass `skipRevert = true` to avoid reverting the call in case the proof expired or frontrunned.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// @dev Pass `skipRevert = true` to avoid reverting the call in case the proof expired or frontrunned.
/// @dev Pass `skipRevert = true` to avoid reverting the call in case the proof is frontrunned.

Copy link
Contributor

Choose a reason for hiding this comment

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

expired is important I think too

@MerlinEgalite
Copy link
Contributor

I think this PR should be done on top of the fix of https://github.com/cantinasec/review-morpho-blue-1/issues/28, or the opposite. How do you plan to do it @Jean-Grimal ?

@Jean-Grimal
Copy link
Contributor Author

I think this PR should be done on top of the fix of cantinasec/review-morpho-blue-1#28, or the opposite. How do you plan to do it @Jean-Grimal ?

Yes you are right, I am working on the fix of this issue right now, I'll handle it

@Rubilmax
Copy link
Collaborator

Rubilmax commented Oct 25, 2023

This PR is also included in #302, because the order of natspecs is changed and params are added so it's clearer to have this in front of the skipRevert param spec
Should we have the corresponding issue linked to #302 instead?

@MerlinEgalite
Copy link
Contributor

This PR is also included in #322, because the order of natspecs is changed and params are added so it's clearer to have this in front of the skipRevert param spec Should we have the corresponding issue linked to #322 instead?

I'd say yes

@Rubilmax Rubilmax closed this Oct 25, 2023
@Rubilmax
Copy link
Collaborator

Rubilmax commented Oct 25, 2023

Moved to #302

@MerlinEgalite MerlinEgalite deleted the fix/issue-19 branch November 9, 2023 14:23
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