While reviewing the mutation PR, I wondered if we might be missing any modifiers, or if any might be wrong. So I made a list of all functions with modifiers in the vault package, checking things like "all external functions in VaultExtension/VaultAdmin should be onlyVaultDelegateCall, etc.
There were a few anomalies. A couple were public that could be external; some were unnecessary (e.g., nonReentrant when there are no external calls), and some inconsistent (e.g., external functions that called the same internal functions had different modifiers, such as withRegisteredPool, when it really should be withInitializedPool).
There were also a couple cosmetic/style preference changes (e.g., reordering adjacent functions for a better flow).
It saves a small amount of bytecode in the Router and VaultAdmin.
Description
While reviewing the mutation PR, I wondered if we might be missing any modifiers, or if any might be wrong. So I made a list of all functions with modifiers in the vault package, checking things like "all external functions in VaultExtension/VaultAdmin should be
onlyVaultDelegateCall, etc.There were a few anomalies. A couple were public that could be external; some were unnecessary (e.g., nonReentrant when there are no external calls), and some inconsistent (e.g., external functions that called the same internal functions had different modifiers, such as withRegisteredPool, when it really should be withInitializedPool).
There were also a couple cosmetic/style preference changes (e.g., reordering adjacent functions for a better flow).
It saves a small amount of bytecode in the Router and VaultAdmin.
See this spreadsheet for the current state.
Type of change
Checklist:
main, or there's a description of how to mergeIssue Resolution