sujithsomraaj / lifi-emergency-pause-facet-v1.2.0

1 day review - 1 / 09
0 stars 0 forks source link

`OnlyPauserWalletOrOwner` modifier could be optimized #3

Open sujithsomraaj opened 2 months ago

sujithsomraaj commented 2 months ago

Context: EmergencyPauseFacet.sol#L37

Description: The OnlyPauserWalletOrOwner modifier validates the msg.sender. This modifier accepts the msg.sender as an argument; however, msg.sender is directly available to the modifier by default.

Hence, optimizing the function could result in a more clear and slightly optimized code.

Recommendation: Consider updating the OnlyPauserWalletOrOwner as below:

...
- modifier OnlyPauserWalletOrOwner(address msgSender) {
-     if (
-        msgSender != pauserWallet &&
-        msgSender != LibDiamond.contractOwner()
-    ) revert UnAuthorized();
-    _;
- }
+ modifier OnlyPauserWalletOrOwner {
+    if (
+        msg.sender != pauserWallet &&
+        msg.sender != LibDiamond.contractOwner()
+    ) revert UnAuthorized();
+    _;
+ }
...
function removeFacet(
        address _facetAddress
-    ) external OnlyPauserWalletOrOwner(msg.sender) {
+    ) external OnlyPauserWalletOrOwner {
...

LI.FI: Fixed in e6cbae6ef39e1fb13937aaaea384d0f93397c591

Researcher: Confirmed.

0xDEnYO commented 2 months ago

Absolutely correct. Thanks for pointing it out:

Fix: https://github.com/lifinance/contracts/commit/e6cbae6ef39e1fb13937aaaea384d0f93397c591