search

Uniswap / v4-core

🦄 🦄 🦄 🦄 Core smart contracts of Uniswap v4
https://blog.uniswap.org/uniswap-v4
Other
1.97k stars 936 forks source link

Consider having Lock.sol check invariants #708

Open marktoda opened 4 months ago

marktoda commented 4 months ago

Component

No response

Describe the suggested feature and problem it solves.

Lock.unlock() should only work when the pool is currently not yet unlocked (i.e. no double unlocking) Lock.lock() should only work when the pool is currently unlocked (i.e. no need to double lock)

These invariants are currently checked in PoolManager.sol.unlock() but IMO would be cleaner to colocate with the operations in the lock library

https://github.com/Uniswap/v4-core/blob/bf06c56f8a876e26a857c49b195bd74cba2604b9/src/PoolManager.sol#L140

Describe the desired implementation.

No response

Describe alternatives.

No response

Additional context.

No response

linear[bot] commented 4 months ago

PROTO-245 Consider having Lock.sol check invariants

snreynolds commented 4 months ago

So you would rather the Lock library check the NonZeroDelta count storage than the main contract? That seems a bit odd to me to have the library itself check the contracts storage, like the library itself is not really standalone. And every other library similar to NonzerDeltaCount really only interact with the slot it defines at the top and no other one.

See diff here.. not inclined to merge but lmk thoughts https://github.com/Uniswap/v4-core/compare/move-lock-logic?expand=1