Use solmate owned #520
Use solmate owned #520
Conversation
| 103987 |
There was a problem hiding this comment.
it just makes no sense 😭
There was a problem hiding this comment.
wonder if running forge test --isolate before and after this change will still introduce a diff
There was a problem hiding this comment.
good q i'll try it
There was a problem hiding this comment.
nope sadly they all still change - different values ofc but they still all change by like <200 ish. Could be that thats an expected result but its definitely weird
There was a problem hiding this comment.
We don't currently have a test for this in main but since we're changing this code I think we may as well add one now and it will check the onlyOwner code in the dependency... But we should really be testing that setProtocolFeeController reverts if called not by the owner
| @@ -78,6 +77,16 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { | |||
| assertEq(address(manager.protocolFeeController()), address(feeController)); | |||
| } | |||
|
|
|||
| function test_setProtocolFeeController_failsIfNotOwner() public { | |||
There was a problem hiding this comment.
technically i think this will be moved out of this file (like a lot of other tests tbh) but since we haven't migrated the tests out of here I think its fine for this PR. and whoever is assigned with ProtocolFees.t.sol will move it (I think its me so I can do it when I pick that up).
Related Issue
#230 to meet EIP, and also good to rely on a battletested implementation
Description of changes