sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

[I-1] - Magic Numbers in FeeManager.sol #456

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 2 months ago

[I-1] - Magic Numbers in FeeManager.sol

Low/Info issue submitted by BugTrippin

Summary

The totalAllocation value in FeeManager.sol is a hardcoded literal in multiple places instead of a constant.

Vulnerability Detail

The totalAllocation value is used in more than one place in FeeManager.sol using a literal value instead of a constant variable.

Hardcoding values in-line without context as so called "magic numbers" makes the code prone to future errors, less readable and harder to maintain.

Impact

While this currently has no impact, it's possible that in case this value needs to change in the future, the fact that it needs to be modified in multiple places might cause it to be overlooked. Even a single place where this change was overlooked can cause hard to detect business logic issues in the protocol down the line.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L148

receiver = Target({
    target: splitFactory.createSplit(
        SplitV2Lib.Split({
            recipients: targets,
            allocations: revshares,
            totalAllocation: 1e6,
            distributionIncentive: 0
        }),
        address(this),
        creator.target
        ),
    chainId: creator.chainId
});

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L483

uint32 creatorShare = 1e6 - (attributionRevShare * attributionShares);

Tool used

Manual Review, Aderyn

Recommendation

Introduce a constant variable defining the value for totalAllocation in FeeManager.sol.

...

+ uint256 public constant TOTAL_ALLOCATION = 1e6;

...

-  uint32 creatorShare = 1e6 - (attributionRevShare * attributionShares);
+ uint32 creatorShare = TOTAL_ALLOCATION - (attributionRevShare * attributionShares);

...

receiver = Target({
    target: splitFactory.createSplit(
        SplitV2Lib.Split({
            recipients: targets,
            allocations: revshares,
-           totalAllocation: 1e6,
+          totalAllocation: TOTAL_ALLOCATION,
            distributionIncentive: 0
        }),
        address(this),
        creator.target
        ),
    chainId: creator.chainId
});