RFPSimpleStrategy: fail in distributing the upcoming milestone

Summary

RFPSimpleStrategy._distribute can revert if upcomingMilestone > 0 due to incorrect check at the line L#432.

Vulnerability Detail

RFPSimpleStrategy._distribute function distributes the upcoming milestone to acceptedRecipientId. There is a check to make sure the contract has enough funds.

431        // make sure has enough funds to distribute based on the proposal bid
432        if (recipient.proposalBid > poolAmount) revert NOT_ENOUGH_FUNDS();

435        uint256 amount = (recipient.proposalBid * milestone.amountPercentage) / 1e18;

438        poolAmount -= amount;

The recipient.proposalBid is the total amount which should be distributed. So due to this check the poolAmount should not be less than recipient.proposalBid at any milestone. But the poolAmount is decreased at each milestone and should be refilled or the _distribute will revert.

Impact

RFPSimpleStrategy._distribute reverts for upcomingMilestone > 0. The contract functionality will be broken.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/6430c8004017e96ae2f5aac365bdefd0b6eeea72/allo-v2/contracts/strategies/rfp-simple/RFPSimpleStrategy.sol#L431-L435

Tool used

Manual Review

Recommendation

Consider using (recipient.proposalBid * milestone.amountPercentage) / 1e18 > poolAmount instead of recipient.proposalBid > poolAmount at the line L#432.

Duplicate of #492

sherlock-audit / 2023-09-Gitcoin-judging

pontifex - RFPSimpleStrategy: fail in distributing the upcoming milestone #945