Opus finding
DopplerDN404 pool starts unlocked but `lockPool` falsely advertises it as locking
highbughigh
- src/dn404/DopplerDN404.sol:36-39
- src/dn404/DopplerDN404.sol:90-95
- src/dn404/DopplerDN404.sol:142-150
The transfer guard reverts only when `to == pool && !isPoolUnlocked`. After deployment `pool` is `address(0)` and `isPoolUnlocked` is `false` (the default). Until `lockPool` is called, the guard is effectively inactive (since `to` is usually not `address(0)` for normal transfers, and any transfer to `address(0)` would revert in DN404 anyway). More critically, calling `lockPool` only sets `pool = pool_` and leaves `isPoolUnlocked = false` — semantically identical to construction (no behavior change except recording the address). The semantic invariant the contract appears to want is `default = locked, must be explicitly unlocked`, but as named the boolean is `isPoolUnlocked` and starts `false`, so the pool is locked-by-name but `pool` is unset, so no transfers are actually rejected pre-lock. This is contradictory and confusing relative to the sibling `DopplerERC20V1` which uses `isPoolLocked` and requires `lockPool`/`unlockPool` to be paired and state-changing. The DN404 version permits calling `unlockPool()` before `lockPool()` was ever called, and `lockPool` is essentially only an address setter (does not change `isPoolUnlocked` from any meaningful prior state).
Recommendation
Mirror the DopplerERC20V1 pattern: track `isPoolLocked` (default false), require `!isPoolLocked` in `lockPool` and `isPoolLocked` in `unlockPool`, and emit events. Also clarify the docstring on `lockPool`: it does not actually set the locked flag; the flag was already false. Either toggle the boolean inside `lockPool` or rename/redocument.