-
Notifications
You must be signed in to change notification settings - Fork 25
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
BaseVaultTest refactoring #1196
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a big diff I might have missed something, but looks good. Could perhaps have done the lifecycle stuff separately as a small PR, but I guess it's fine
@@ -75,19 +75,19 @@ contract BigPoolDataTest is BaseVaultTest { | |||
vm.stopPrank(); | |||
} | |||
|
|||
function _approveForSender() internal { | |||
function _approveNewPoolForSender() internal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several similar functions here that are identical except for the target of the loop. Consider factoring it out into BaseVaultTest
(maybe with an argument for the array). For instance, BigWeightedPool.t.sol's _approvePoolTokensForSender
/ _approvePoolTokensForPool
; and here _approveNewPoolForSender
/ _approveForNewPool
uint256 internal DEFAULT_AMOUNT_ROUND_DOWN = DEFAULT_AMOUNT - 2; | ||
// Default amount of BPT to use in tests for user operations. | ||
uint256 internal DEFAULT_BPT_AMOUNT = 2e3 * 1e18; | ||
// Default amount of BPT round down. | ||
uint256 internal DEFAULT_BPT_AMOUNT_ROUND_DOWN = DEFAULT_BPT_AMOUNT - 2; | ||
// Default rate for the rate provider mock. | ||
uint256 internal DEFAULT_MOCK_RATE = 2e18; | ||
|
||
// Default swap fee percentage. | ||
uint256 internal DEFAULT_SWAP_FEE_PERCENTAGE = 1e16; // 1% | ||
// Default protocol swap fee percentage. | ||
uint64 internal DEFAULT_PROTOCOL_SWAP_FEE_PERCENTAGE = 50e16; // 50% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these can all be constant.
uint256 internal DEFAULT_AMOUNT_ROUND_DOWN = DEFAULT_AMOUNT - 2; | |
// Default amount of BPT to use in tests for user operations. | |
uint256 internal DEFAULT_BPT_AMOUNT = 2e3 * 1e18; | |
// Default amount of BPT round down. | |
uint256 internal DEFAULT_BPT_AMOUNT_ROUND_DOWN = DEFAULT_BPT_AMOUNT - 2; | |
// Default rate for the rate provider mock. | |
uint256 internal DEFAULT_MOCK_RATE = 2e18; | |
// Default swap fee percentage. | |
uint256 internal DEFAULT_SWAP_FEE_PERCENTAGE = 1e16; // 1% | |
// Default protocol swap fee percentage. | |
uint64 internal DEFAULT_PROTOCOL_SWAP_FEE_PERCENTAGE = 50e16; // 50% | |
uint256 internal constant DEFAULT_AMOUNT_ROUND_DOWN = DEFAULT_AMOUNT - 2; | |
// Default amount of BPT to use in tests for user operations. | |
uint256 internal constant DEFAULT_BPT_AMOUNT = 2e3 * 1e18; | |
// Default amount of BPT round down. | |
uint256 internal constant DEFAULT_BPT_AMOUNT_ROUND_DOWN = DEFAULT_BPT_AMOUNT - 2; | |
// Default rate for the rate provider mock. | |
uint256 internal constant DEFAULT_MOCK_RATE = 2e18; | |
// Default swap fee percentage. | |
uint256 internal constant DEFAULT_SWAP_FEE_PERCENTAGE = 1e16; // 1% | |
// Default protocol swap fee percentage. | |
uint64 internal constant DEFAULT_PROTOCOL_SWAP_FEE_PERCENTAGE = 50e16; // 50% |
Description
This PR makes variables that are used as constants actually constant and also adds some hooks for more flexible lifecycle customization.
Type of change
Checklist:
main
, or there's a description of how to mergeIssue Resolution