Donation strategy allocate() & distribute() can be called in the same block

In DonationVotingMerkleDistributionBaseStrategy.sol the functions allocate() and distribute() can be called in the same block contrary to developers assumptions. This could lead to issues with integration of the strategy by other smart contracts or off-chain components

Vulnerability Detail

The implementations of the mentioned modifiers - _checkOnlyActiveAllocation & _checkOnlyAfterAllocation does not work according to the developer's assumptions when block.timestamp = allocationEndTime. The issue is that _checkOnlyAfterAllocation() won't revert since block.timestamp isn't < allocationEndTime (they are equal), therefore at the allocationEndTime block both allocate() and distribute() can be executed in the same block which shouldn't be allowed.

Impact

This could lead to integration issues with smart contracts / off-chain systems

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/donation-voting-merkle-base/DonationVotingMerkleDistributionBaseStrategy.sol#L219-L231 https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/donation-voting-merkle-base/DonationVotingMerkleDistributionBaseStrategy.sol#L465-L478

Tool used

Manual Review

Recommendation

Rework the after allocation modifier to such

function _checkOnlyAfterAllocation() internal view virtual {
        if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED();
    }

sherlock-audit / 2023-09-Gitcoin-judging