DrawManager::canStartDraw() does not take into account failed requests, returning false when it should return true and harms bots
Summary
The DrawManager rewards users based on a Parabolic Fractional Dutch Auction, in which time is critical and few seconds of delay may cost bots rewards. There is a bug in DrawManager::canStartDraw() as it returns false whenever block.timestamp - prizePool.drawClosesAt(drawId) is bigger than auctionDuration, not considering failed requests.
Vulnerability Detail
Draws are handled by the DrawManager, which rewards users based on a Parabolic Fractional Dutch Auction, which increases rewards over time based on the curve in the docs. Since DrawManager::startDraw() is permissionless, it is critically to ensure bots can get accurate information to call it and earn rewards, otherwise the auction is unfair and some bots profit unfairly. From the docs:
The draw auctions outlined in these docs leverage an incentivization mechanism to encourage competition among third parties to complete the draws in a timely manner while maximizing cost efficiency.
The duration of an auction is limited to auctionDuration only when the first RNG request succeeds. When the first one fails, the auction may be extended again by a duration of auctionDuration, since the time of the last draw request.
However, DrawManager::canStartDraw() does not take into account failed requests, and always computes the duration of the auction as block.timestamp - prizePool.drawClosesAt(drawId) and reverts if it is bigger than auctionDuration. This is incorrect as if there is any failed request, the actual duration limit is block.timestamp - lastRequest.closedAt, so DrawManager::startDraw() may still be called, but DrawManager::canStartDraw() returns false.
DrawManager::startDrawReward() is called by bots to return the expected reward when starting the draw, but it will return 0 when it should return an amount different than 0 if there was a failed RNG request. Thus, most bots, if not all will be affected by this and will not call DrawManager::startDraw() when they should, leading to an incorrect increase of the reward that should not happen.
Impact
The Parabolic Fractional Dutch Auction is flawed as it is not rendering the correct information to bots, tricking them into not starting a draw and increasing the reward fraction, which harms the reserve of the PrizePool.
0x73696d616f
medium
DrawManager::canStartDraw()does not take into account failed requests, returning false when it should return true and harms botsSummary
The
DrawManagerrewards users based on a Parabolic Fractional Dutch Auction, in which time is critical and few seconds of delay may cost bots rewards. There is a bug inDrawManager::canStartDraw()as it returns false wheneverblock.timestamp - prizePool.drawClosesAt(drawId)is bigger thanauctionDuration, not considering failed requests.Vulnerability Detail
Draws are handled by the
DrawManager, which rewards users based on a Parabolic Fractional Dutch Auction, which increases rewards over time based on the curve in the docs. SinceDrawManager::startDraw()is permissionless, it is critically to ensure bots can get accurate information to call it and earn rewards, otherwise the auction is unfair and some bots profit unfairly. From the docs:The duration of an auction is limited to
auctionDurationonly when the first RNG request succeeds. When the first one fails, the auction may be extended again by a duration ofauctionDuration, since the time of the last draw request.However,
DrawManager::canStartDraw()does not take into account failed requests, and always computes the duration of the auction asblock.timestamp - prizePool.drawClosesAt(drawId)and reverts if it is bigger thanauctionDuration. This is incorrect as if there is any failed request, the actual duration limit isblock.timestamp - lastRequest.closedAt, soDrawManager::startDraw()may still be called, butDrawManager::canStartDraw()returns false.DrawManager::startDrawReward()is called by bots to return the expected reward when starting the draw, but it will return 0 when it should return an amount different than 0 if there was a failed RNG request. Thus, most bots, if not all will be affected by this and will not callDrawManager::startDraw()when they should, leading to an incorrect increase of the reward that should not happen.Impact
The Parabolic Fractional Dutch Auction is flawed as it is not rendering the correct information to bots, tricking them into not starting a draw and increasing the reward fraction, which harms the reserve of the
PrizePool.Code Snippet
https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-draw-manager/src/DrawManager.sol#L289 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-draw-manager/src/DrawManager.sol#L246 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-draw-manager/src/DrawManager.sol#L296-L298
Tool used
Manual Review
Vscode
Recommendation
If the last request has failed, the correct check is
if (block.timestamp - lastRequest.closedAt > auctionDuration) revert AuctionExpired();Duplicate of #129