Move BaseHook to new utils directory

[gretzke]

Related Issue

Which issue does this pull request resolve?

Description of changes

[saucepoint]

in favor of the change,

BUT in this tidy-up PR, it makes sense to change BaseHook to inherit ImmutableState instead

#436 (comment)

[gretzke]

Only SafeCallback relied on the pool manager variable / modifier, we can remove ImmutableState entirely

[saucepoint]

@gretzke we should decorate the hook functions with a onlyPoolManager modifier, so we should inherit ImmutableState i think

as much as I hate repeating code, the modifier lives in SafeCallback which we shouldnt change; so we'll need to redefine the modifier in BaseHook?

[gretzke]

If someone overrides the hook function they also override the function modifier, because the functions are reverting anyways right now we could not have it there. If we want to ensure the modifier is not overwritten accidentally, we should split the functions into two:

-function beforeInitialize(address, PoolKey calldata, uint160) external virtual returns (bytes4) {
-    revert HookNotImplemented();
-}

+function beforeInitialize(address sender, PoolKey calldata key, uint160 sqrtPriceX96) external returns (bytes4) {
+    return _beforeInitialize(sender, key, sqrtPriceX96);
+}
+
+function _beforeInitialize(address sender, PoolKey calldata key, uint160 sqrtPriceX96) internal virtual returns (bytes4)
+    revert HookNotImplemented();
+}

This ensures that the external function cannot be overwritten and is always protected

[gretzke]

do we still need the selfOnly and onlyValidPools modifiers?

[hensha256]

I'm pro moving the modifier to ImmutableState... 👀 @saucepoint interested in your reason that we cant? just because of code freeze?

And yeah I think I'm pro adding the modifier and then adding internal functions to baseHook.sol. We follow that same pattern in SafeCallback too.

[saucepoint]

just because of code freeze?

yeah, if were not careful -- we might end up deploying a version of posm with different bytecode than whats currently in the wild

[gretzke]

bytecode for positionmanager did not change after my implementation, feel free to double check

[dianakocsis]

not only thrown when calling unlockCallBack since its also being called elsewhere

[gretzke]

pushed a change here, in the future, could you suggest a change instead? e.g.,

Suggested change

	/// @notice Thrown when calling unlockCallback where the caller is not PoolManager
	/// @notice Thrown when the caller is not PoolManager

[dianakocsis]

ah forgot again! my bad!

[arora-anmol]

@hensha256 @dianakocsis @gretzke with this change, might need to update the V4 hook docs here -- https://docs.uniswap.org/contracts/v4/guides/hooks/your-first-hook as it uses base/hooks/BaseHook for import.

Also if you guys could tag the commit that works with v4-core@v4.0.0, that could help keep track of dependencies

[Hugoo]

Note, the README.md was not updated to reflect the directory change, here is a fix: #450