Based on #772, which ensures modifiers on hook examples, this provides a way to meaningfully test BaseHooks.
This PR:
1) Moves HookFlags from inside the IHooks interface to VaultTypes. I can see both sides here, but I don't think it's best practice generally to define structs inside interfaces (especially if widely used), and putting it next to HooksConfig makes it really clear that the HooksConfig = HookFlags + hook contract address, and we can visually inspect to ensure they don't start diverging.
2) Leaves BaseHooks abstract, but changes the visibility of the hook functions from external to public, so that derived contracts can call them.
3) Creates a BaseHooksMock concrete contract that overrides all the hook functions and calls the base class. The corresponding test ensures that BaseHooks functions all return false. We're checking here that if a misconfigured hook contract doesn't define a given hook, and the Vault calls it, it will fall back on BaseHooks and return false/revert.
It doesn't really make sense to test onlyVault constraints here, as real hook contracts will not be calling the base contract functions, so the modifiers would never be executed. onlyVault constraints have to be tested at the most derived level (e.g., in the tests for the example hook contracts).
Type of change
[ ] Bug fix
[ ] New feature
[ ] Breaking change
[ ] Dependency changes
[ ] Code refactor / cleanup
[ ] Optimization: [ ] gas / [ ] bytecode
[ ] Documentation or wording changes
[ ] Other
Checklist:
[ ] The diff is legible and has no extraneous changes
[ ] Complex code has been commented, including external interfaces
[ ] Tests have 100% code coverage
[ ] The base branch is either main, or there's a description of how to merge
Description
Based on #772, which ensures modifiers on hook examples, this provides a way to meaningfully test BaseHooks.
This PR:
1) Moves HookFlags from inside the IHooks interface to VaultTypes. I can see both sides here, but I don't think it's best practice generally to define structs inside interfaces (especially if widely used), and putting it next to HooksConfig makes it really clear that the HooksConfig = HookFlags + hook contract address, and we can visually inspect to ensure they don't start diverging.
2) Leaves BaseHooks abstract, but changes the visibility of the hook functions from external to public, so that derived contracts can call them.
3) Creates a BaseHooksMock concrete contract that overrides all the hook functions and calls the base class. The corresponding test ensures that BaseHooks functions all return false. We're checking here that if a misconfigured hook contract doesn't define a given hook, and the Vault calls it, it will fall back on BaseHooks and return false/revert.
It doesn't really make sense to test
onlyVaultconstraints here, as real hook contracts will not be calling the base contract functions, so the modifiers would never be executed.onlyVaultconstraints have to be tested at the most derived level (e.g., in the tests for the example hook contracts).Type of change
Checklist:
main, or there's a description of how to mergeIssue Resolution