Rename currency settler util contract #799
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Forge code coverage:
|
snreynolds
requested changes
Jul 18, 2024
| @@ -9,7 +9,9 @@ import {IPoolManager} from "../../src/interfaces/IPoolManager.sol"; | |||
| /// To settle a positive delta (a credit to the user), a user may take or mint. | |||
| /// To settle a negative delta (a debt on the user), a user make transfer or burn to pay off a debt. | |||
| /// @dev Note that sync() is called before any erc-20 transfer in `settle`. | |||
| library CurrencySettler { | |||
| /// @dev THIS CONTRACT IS NOT SECURE, IT IS FOR TESTING PURPOSES, DO NOT USE IT IN PROD | |||
There was a problem hiding this comment.
I feel like this warning is a bit overkill? It just doesn't use safe transfer from and we don't do this in a lot of our test contracts which should never be used in prod
There was a problem hiding this comment.
I think just renaming it and putting it in the test/utils file is sufficient
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue
Which issue does this pull request resolve?
This contract is not secure and is just a test util. Lets name it as such, and put a better version in periphery for real contracts.