Informational - Change call sequence for reentrancy mitigation

[kitty-the-kat]

_withdraw(), redeem(), and report() should change the order of transferring assets and updating vaultAssets to follow checks-effects-interactions and get one step closer to not needing the nonReentrant modifiers.

Technical Details

In _withdraw(), funds are transferred out of the GVault before the vaultAssets state variable is updated. According to checks-effects-interactions, the external interactions should happen last, meaning the vaultAssets variable should be updated before funds are transferred. This is what the solmate ERC4626 implementation does by calling the internal _burn() first to update the totalSupply value before transferring funds, which is the opposite of when the transfer happens relative to _mint() in mint(). The same change can be applied to redeem() and report() (1, 2).

Impact

Informational.

Recommendation

Consider changing the order of operations so the subtraction from vaultAssets happens before assets are transferred outside of the GVault. This may allow the nonReentrant modifier to be removed in the future.

[kitty-the-kat]

acknowledged - Wont fix, reentrancy guard deals with this for a minor gas cost