search

sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

WATCHPUG - Gas `multiplier` should also apply to the `buffer` #152

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

WATCHPUG

medium

Gas multiplier should also apply to the buffer

Summary

Vulnerability Detail

multiplier in the keep modifier is designed to be a premium applied to the gas used as an incentive for the keeper, and buffer is an amount to fix for the difference between the gas used tracked and the gas spent on the part that was untracked.

multiplier should be applied to the gas with the fix, otherwise it may end up with a suboptimal or even undesirable keeperFee amount -- think of a case that the buffer is much larger than the gas tracked (gasUsed), which means that buffer actually takes a large portion of the actual gas cost. If there is no premium on that part, the entire keeperFee may not be able to compensate for the actual gas cost.

Impact

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/attribute/Kept.sol#L40-L57

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L359-L367

Tool used

Manual Review

Recommendation

UFixed18 keeperFee = UFixed18Lib.from(gasUsed)
    .add(UFixed18Lib.from(buffer))
    .mul(multiplier)
    .mul(_etherPrice())
    .mul(UFixed18.wrap(block.basefee));
sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

l

darkart commented:

LOW/Invalid