Some low severity issues which must be fixed

[hats-bug-reporter[bot]]

Github username: @0xRizwan Submission hash (on-chain): 0xdf90f41d0ea2fd60a123dc646dd1e33b82f1c52a0ff35bb512eb5d1a1f09638f Severity: low

Description: Description\

Issue 1

In ProtocolRevenueManager.sol, There is wrong consideration of vesting period for stakers which is being calculated in number of days. Per the Natspec, the vesting period should be 6 months, However, the calculated vesting period is not 6 months and its approximately 5.5 months

        DEPRECATED_auctionFeeVestingPeriodForStakersInDays = 6 * 7 * 4; // 6 months

Therefore, with current implementation, the vesting period will come early than the expected design of 6 months. It will come early by 12 to 14 days which should not be desired. It should be noted that solidity allows to use times in days type too. This can be checked here

Affected code link

Recommendation to fix

On average, 30 days per month would be 180 days for 6 months should be considered.

-        DEPRECATED_auctionFeeVestingPeriodForStakersInDays = 6 * 7 * 4; // 6 months
+        DEPRECATED_auctionFeeVestingPeriodForStakersInDays = 180 days; // 6 months

Issue 2

Auction fee is being transferred to membershipManagerContractAddress but the Natspec states membership NFT contract. This is misleading for users/readers/reviewers of contract and must be corrected. The affected code at L-204 can be checked here

Recommendation to fix:

-    /// @notice Transfer the auction fee received from the node operator to the membership NFT contract when above the threshold
+    /// @notice Transfer the auction fee received from the node operator to the membership Manager contract when above the threshold
    /// @dev Called by registerValidator() in StakingManager.sol
    /// @param _bidId the ID of the validator
    function processAuctionFeeTransfer(
        uint256 _bidId
    ) external onlyStakingManagerContract {

           // some code

            (bool sent, ) = membershipManagerContractAddress.call{value: newAccumulatedRevenue}("");
            require(sent, "Failed to send Ether");
        } else {
            accumulatedRevenue = uint128(newAccumulatedRevenue);
        }
    }

Another misleading comment at L-327 which can be checked here

Recommendation to fix:

-    /// @notice Updates the accumulated revenue threshold that will trigger a transfer to MembershipNFT contract
+    /// @notice Updates the accumulated revenue threshold that will trigger a transfer to membership Manager contract
    /// @param _newThreshold the new threshold to set
    function setAccumulatedRevenueThreshold(uint128 _newThreshold) external onlyAdmin {
        accumulatedRevenueThreshold = _newThreshold;
    }

The issues are considered as low severity based on contest rules which mentions,

Issues where the behavior of the contracts differs from the intended behavior (as described in the docs and by common sense), but no funds are at risk.

Therefore, the issue should be considered and fixed by protocol team.

[seongyun-ko]

PRM contract is deprecated

[0xRizwan]

@seongyun-ko

PRM contract is in scope and it is confirmed by protocol team in discord by @chuck_dao

Please check src folder here

I politely request for the consideration of this issue.