[BUG]: Batch Invoice Payment Race Condition #59
Description
Problem
The payInvoicesBatch function has a critical bug that can cause users to lose funds.
Currently, it marks all invoices as "paid" before actually transferring the money.
If any transfer fails halfway through, some invoices will show as paid even though the seller never received payment.
// ❌ Problem: State changes happen before transfers
for (uint256 i = 0; i < n; i++) {
invoices[invoiceIds[i]].isPaid = true; // Marked paid too early!
}
accumulatedFees += totalNativeFee;
// If this part fails, above changes stay corrupted
for (uint256 i = 0; i < n; i++) {
// ... transfer logic
}
Proposed Solution Approach
Following the Checks-Effects-Interactions pattern:
Checks - Validate all invoices first (already done ✅)
Interactions - Execute all transfers first
Effects - Update state only after all transfers succeed
Alternatively, we could use a two-phase approach:
Phase 1: Validate everything and execute transfers
Phase 2: Update state atomically only if all transfers succeeded
Impact
Severity: Critical - Direct fund loss possible
Affected: Anyone using batch payments
Note: Not providing the full solution yet - want to discuss the best approach with maintainers first.
Happy to implement once we agree on the path forward!
@Zahnentferner Please assign this issue to me!