MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

Manager Cut Calculation Uses Confusing Logic

muzammilmohammed678

Root + Impact


Description:

The manager cut calculation uses division by `managerCutPercent` (which is 10) to calculate a 10% cut. While mathematically correct (dividing by 10 gives 10%), this is confusing and error-prone. If someone changes `managerCutPercent` to represent an actual percentage (like 10 for 10%), the calculation would break. The variable name suggests it's a percentage, but it's actually a divisor.

The current implementation works correctly for the value 10, but the logic is non-standard and could lead to errors if modified.

```solidity
// Root cause in the codebase
20| uint256 private constant managerCutPercent = 10;
54| uint256 managerCut = remainingRewards / managerCutPercent;
```

Risk

Likelihood:

  • * This occurs when `closePot()` is called after the 90-day period

    * The issue manifests if someone attempts to change the manager cut percentage without understanding the current logic

Impact:

  • * Confusing code that is difficult to maintain

    * High risk of introducing bugs if the percentage is changed

    * If `managerCutPercent` is changed to represent actual percentage (e.g., 10 for 10%), the calculation becomes incorrect

    * Non-standard percentage calculation makes code review more difficult

Proof of Concept

```solidity
// Current implementation:
// managerCutPercent = 10
// remainingRewards = 100
// managerCut = 100 / 10 = 10 (correctly 10%)
​
// If someone changes to percentage-based:
// managerCutPercent = 10 (intending 10%)
// managerCut = 100 / 10 = 10 (still works by accident)
​
// If someone changes to 20%:
// managerCutPercent = 20 (intending 20%)
// managerCut = 100 / 20 = 5 (incorrect! Should be 20, not 5)
```

Recommended Mitigation

```diff
- uint256 private constant managerCutPercent = 10;
+ uint256 private constant MANAGER_CUT_BASIS_POINTS = 1000; // 10% = 1000 basis points
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
- uint256 managerCut = remainingRewards / managerCutPercent;
+ uint256 managerCut = (remainingRewards * MANAGER_CUT_BASIS_POINTS) / 10000;
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
```
​
Alternatively, use a simpler percentage calculation:
​
```diff
- uint256 private constant managerCutPercent = 10;
+ uint256 private constant MANAGER_CUT_PERCENT = 10; // 10%
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
- uint256 managerCut = remainingRewards / managerCutPercent;
+ uint256 managerCut = (remainingRewards * MANAGER_CUT_PERCENT) / 100;
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!