[Escrow] Payment not found panic in PaymentWithdraw when heightDelta is zero

[KentaMori115]

Labels: bug, escrow, panic, critical

Description

Bug Summary

The PaymentWithdraw function panics when attempting to withdraw from a StateOpen payment if the account's heightDelta is zero. This occurs because accountSettle only queries StateOverdrawn payments when heightDelta == 0, excluding StateOpen payments from the results.

Location

x/escrow/keeper/keeper.go:600-601

Code Snippet

PaymentWithdraw function:

func (k *keeper) PaymentWithdraw(ctx sdk.Context, id escrowid.Payment) error {
    pmnt, err := k.getPayment(ctx, id)
    if err != nil {
        return err
    }

    if pmnt.State.State != etypes.StateOpen {
        return module.ErrPaymentClosed
    }

    payments, od, err := k.accountSettle(ctx, acc)
    if err != nil {
        return err
    }

    if !od {
        pmnt = nil

        for i, p := range payments {
            if p.ID.Key() == id.Key() {
                pmnt = &payments[i]
            }
        }

        if pmnt == nil {
            panic(fmt.Sprintf("couldn't find payment %s", id.String()))  // ❌ PANIC HERE
        }

        err = k.paymentWithdraw(ctx, pmnt)
        // ...
    }
}

accountSettle function (lines 467-473):

pStates := []etypes.State{
    etypes.StateOverdrawn,
}

if !heightDelta.IsZero() {
    pStates = append(pStates, etypes.StateOpen)  // Only included if heightDelta > 0
}

Root Cause

The accountSettle function conditionally includes StateOpen payments in its query based on heightDelta:

• When heightDelta == 0: Only queries [StateOverdrawn] payments
• When heightDelta > 0: Queries [StateOverdrawn, StateOpen] payments

heightDelta is zero when:

1. Account state is StateOverdrawn (line 462: if acc.State.State != etypes.StateOverdrawn is false)
2. OR ctx.BlockHeight() == acc.State.SettledAt (settlement happens in the same block)

When heightDelta == 0, StateOpen payments are excluded from the query, causing the payment lookup to fail and trigger a panic.

Impact

• CRITICAL: Application panic
• Transaction failure
• Users cannot withdraw from payments when:
  • Account is StateOverdrawn, OR
  • Settlement happens in the same block (heightDelta == 0)

Steps to Reproduce

1. Create an escrow account with a payment in StateOpen
2. Make account StateOverdrawn (or trigger settlement in same block where heightDelta == 0)
3. Attempt to withdraw from the StateOpen payment
4. Observed: Panic at line 601 - "couldn't find payment"
5. Expected: Payment should be found and processed correctly

Expected Behavior

accountSettle should always include StateOpen payments in its query, regardless of heightDelta value, OR PaymentWithdraw should handle the case where payment is not found in settlement results gracefully.

Proposed Fix

Option 1: Always include StateOpen payments (Recommended)

// In accountSettle (line 467-473)
pStates := []etypes.State{
    etypes.StateOverdrawn,
    etypes.StateOpen,  // Always include, not conditional
}

// Remove the conditional check:
// if !heightDelta.IsZero() {
//     pStates = append(pStates, etypes.StateOpen)
// }

Option 2: Handle payment not found gracefully

// In PaymentWithdraw (line 600-601)
if pmnt == nil {
    // Re-fetch payment to verify it's still Open
    pmnt, err = k.getPayment(ctx, id)
    if err != nil {
        return err
    }
    if pmnt.State.State != etypes.StateOpen {
        return module.ErrPaymentClosed
    }
    // Add to payments array for processing
    payments = append(payments, *pmnt)
    pmnt = &payments[len(payments)-1]
}

Additional Context

• This bug can occur when account becomes overdrawn
• Can also occur if settlement happens in the same block (heightDelta == 0)
• Payment is verified as StateOpen before settlement, but not found after
• Critical bug that causes application panic
• Related to payment settlement logic in escrow module