Submitted by dirk_y
Severity: Medium
https://github.com/GenerationSoftware/pt-v5-claimer/blob/main/src/Claimer.sol#L76-L78
https://github.com/GenerationSoftware/pt-v5-claimer/blob/main/src/Claimer.sol#L136
https://github.com/GenerationSoftware/pt-v5-claimer/blob/main/src/Claimer.sol#L262-L264
https://github.com/GenerationSoftware/pt-v5-claimer/blob/main/src/Claimer.sol#L223-L250
https://github.com/GenerationSoftware/pt-v5-claimer/blob/main/src/Claimer.sol#L289
The V5 implementation delegates the task of claiming prizes to a network of claimers. The fees received by a claimer are calculated based on a dutch auction and limited based on the prize size of the highest tier (the smallest prize). As a result, it is possible that the gas price could exceed the fee received by claimers, leading to prizes not being claimed. If any high value prizes happen to be drawn during this period then they will go unclaimed.
The new implementation only computes the maxFee size based on the prize tier that is being claimed for. As a result, the fees received by claimers are now larger for larger prize tiers; thereby, incentivising lower tiers (i.e. those with higher prizes) to be claimed first and resulting in more fees paid to claimers.
Because all the tiers run on the same auction, each auction will now run at a completely different speed.
If the _maximumFee parameter specified in the constructor is relatively small, then the maxFee for the lower prize tiers (higher prizes) will never be reached anyway. If the _maximumFee is relatively large to give sufficient range for the auctions, the auctions for higher tiers (lower prizes) will ramp up very quickly to the max limit based on the prize tier. The real impact of this is that auctions are now running inefficiently, where fees are likely to be higher than they could be for the higher tiers (i.e. the bots are getting more fees than they would be willing to accept).
Based on the updated implementation, the maximum fee to be paid is now a function of the tier being claimed for, not the total number of active tiers:
The return value of this call is used as the first parameter for calls to _computeFeePerClaim and in turn the last parameter of _computeFeeForNextClaim:
As you can see, the fee is capped based on the maxFee for the prize tier being claimed for. This seems to be doing what was intended; however, there is only one auction for all the tiers, and thus the auction decay constant is consistent across all the tiers:
Whichever is the lowest of the auction _maximumFee and the tier maxFee is the max fee that could possibly be collected. In order to allow all prize tiers to be claimed it is likely that the _maximumFee in the constructor will be increased; however, this now means the decay constant is greater and therefore, the auctions ramp up faster. For tiers with a lower max fee, this means the maxFee is reached significantly faster, leading to inefficient auctions.
Potentially, there is another angle to resolve the issue reported in the original audit by having a mechanism that allows the maxFee to be increased if not enough prizes have been redeemed in the last draw, although this introduces additional complexity.
Alternatively there needs to be an auction for each tier with its own decay constant (i.e. min and max fees) to ensure that all the auctions are ramping up in a similar (but not necessarily identical) manner. In my opinion, this option makes the most sense and is the least complex and therefore, also the hardest to manipulate (if at all).
asselstine (PoolTogether) acknowledged
Note: For full discussion, see here.
