hats-finance / Smooth-0x64bc275b37e62eec81a00ecaecd2b9567058f990

Dappnode's MEV Smoothing Pool
0 stars 2 forks source link

The `initialize` not emit the `AcceptGovernance` event #20

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @Rotcivegaf Submission hash (on-chain): 0x6c6df446b8fc2037770878f4aaf14ed6604763b123073196a56460b30825d5b8 Severity: low

Description:

Description

The initialize function not emit the AcceptGovernance event when change the governance slot

Also not emit a event when the slot deploymentBlockNumber change

This would affect the offchain reading of events

Recommendation

Add the events:

@@ -172,6 +172,8 @@ contract DappnodeSmoothingPool is OwnableUpgradeable {
      */
     event AcceptGovernance(address newGovernance);

+    event UpdateDeploymentBlockNumber(uint256 newDeploymentBlockNumber);
+
     /**
      * @param _governance Governance address
      * @param _subscriptionCollateral Subscription collateral
@@ -213,10 +215,12 @@ contract DappnodeSmoothingPool is OwnableUpgradeable {
         __Ownable_init();

         // Emit events
+        emit AcceptGovernance(_governance);
         emit UpdatePoolFee(_poolFee);
         emit UpdatePoolFeeRecipient(_poolFeeRecipient);
         emit UpdateCheckpointSlotSize(_checkpointSlotSize);
         emit UpdateQuorum(_quorum);
+        emit UpdateDeploymentBlockNumber(block.number);
     }
invocamanman commented 1 year ago

The updateDeploymentBlockNumber does not exist because that variable cannot change.

Since the admin is not a variable that's needed to synch in the oracle we do not emit an event for it. The AcceptGovernance event could be useful just to check maybe how many times the governance has been transferred, and also seems weird that automatically a governance was "accepted" without any action by that address. So the system was designed this way.

Also i don't think this issue should be labeled as a bug, but more as an "informational/recommendation" issue.