Looking into #463 and trying to squeeze some bytes out of the main Vault, I realized that we had very little tests for vault registration in general.
We have coverage for hook / pool flags, pause / swap manager, etc, but the basics were not there. In the process I also found a small bug: you could register a pool with fees outside the boundaries that the pool should be enforcing, and you could even set percentages slightly larger than FP(1) which makes no sense at all.
On the other hand, the library was enforcing that percentages fit in 24 bits. While that's fine in terms of storage, it doesn't make sense to store percentages larger than FP(1), which given the right constants to encode should fit in 24 bits or less. To enforce this, I made a small test that checks that the constants are OK, and I made the percentage setters at the library more strict (instead of checking whether the encoded value fits, check the input directly).
EDIT: after some back and forth I've decided to keep _setSwapFeePercentage in VaultCommon to keep the library consistent. In any case the library shall not be used elsewhere in production code. Testing was improved to cover edge cases around registration.
Type of change
[x] Bug fix
[ ] New feature
[ ] Breaking change
[ ] Dependency changes
[ ] Code refactor / cleanup
[ ] Documentation or wording changes
[ ] Other
Checklist:
[x] The diff is legible and has no extraneous changes
[x] Complex code has been commented, including external interfaces
N/A Tests have 100% code coverage
[x] The base branch is either main, or there's a description of how to merge
Description
Looking into #463 and trying to squeeze some bytes out of the main Vault, I realized that we had very little tests for vault registration in general.
We have coverage for hook / pool flags, pause / swap manager, etc, but the basics were not there. In the process I also found a small bug: you could register a pool with fees outside the boundaries that the pool should be enforcing, and you could even set percentages slightly larger than FP(1) which makes no sense at all.
On the other hand, the library was enforcing that percentages fit in 24 bits. While that's fine in terms of storage, it doesn't make sense to store percentages larger than FP(1), which given the right constants to encode should fit in 24 bits or less. To enforce this, I made a small test that checks that the constants are OK, and I made the percentage setters at the library more strict (instead of checking whether the encoded value fits, check the input directly).
EDIT: after some back and forth I've decided to keep
_setSwapFeePercentageinVaultCommonto keep the library consistent. In any case the library shall not be used elsewhere in production code. Testing was improved to cover edge cases around registration.Type of change
Checklist:
main, or there's a description of how to mergeIssue Resolution
Closes #463