hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

0 stars 0 forks source link

`merklGaugeMiddleman` address could be different in factory and guage #22

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0x92d750511244e578f717387ba7dc6ca37b88548ac3fb7c0056bf68d50d6165ac Severity: low

Description: Description\

merklGaugeMiddleman is a contract acts as a middleman between Gauges and the DistributionCreator, facilitating the reward distribution process.

    address public override merklGaugeMiddleman;

In GaugeFactoryUpgradeable.sol, merklGaugeMiddleman is initialized in initialize().

There is also a setter function provided in contract in case to change it.

    function setMerklGaugeMiddleman(address _newMerklGaugeMiddleman) external onlyOwner {
        merklGaugeMiddleman = _newMerklGaugeMiddleman;
    }

The issue is, merklGaugeMiddleman is being used for gauges and the setter function is updating the merklGaugeMiddleman in factory contract which is not required.

Another issue is, GaugeUpgradeable.setMerklGaugeMiddleman() has this function to update the merklGaugeMiddleman address.

    function setMerklGaugeMiddleman(address _newMerklGaugeMiddleman) external onlyOwner {
        require(_newMerklGaugeMiddleman != address(0));
        merklGaugeMiddleman = _newMerklGaugeMiddleman;
    }

Using this function from guage interface would inline the address of merklGaugeMiddleman as same and would make less mistakes while setting it otherwise the address of merklGaugeMiddleman could be different in factory and guage.

This is similar approach has been done in GaugeFactoryUpgradeable.setDistribution().

Recommendations\

    function setMerklGaugeMiddleman(address _gauge, address _newMerklGaugeMiddleman) external onlyOwner {
+        IGauge(_gauge).setMerklGaugeMiddleman(_newMerklGaugeMiddleman);
        merklGaugeMiddleman = _newMerklGaugeMiddleman;
    }
BohdanHrytsak commented 4 months ago

Thank you for the submission.

This behaviour is expected, although it seems logical that gauge should take the current address of merklGaugeMiddleman from Factory, it is not. There can be more than one MerklGaugeMiddlan. The address set in Factory is only needed to configure newly created addresses. At the same time, the previous gauges will be updated if it is really necessary