fix(chequebook): handle nil cumulative payout in cheque processing

pkg/settlement/swap/chequebook/cashout.go

@@ -134,12 +134,6 @@ func (s *cashoutService) CashCheque(ctx context.Context, chequebook, recipient c
 		return common.Hash{}, err
 	}
 
-	// a corrupt statestore entry unmarshals into a non-nil cheque with a nil
-	// cumulativePayout; guard against packing it below.
-	if cheque.CumulativePayout == nil {
-		return common.Hash{}, fmt.Errorf("nil cumulative payout on cheque loaded from statestore for chequebook %x: %w", chequebook, ErrNoCheque)
-	}
-
 	callData, err := chequebookABI.Pack("cashChequeBeneficiary", recipient, cheque.CumulativePayout, cheque.Signature)
 	if err != nil {
 		return common.Hash{}, err
@@ -176,12 +170,6 @@ func (s *cashoutService) CashoutStatus(ctx context.Context, chequebookAddress co
 		return nil, err
 	}
 
-	// a corrupt statestore entry unmarshals into a non-nil cheque with a nil
-	// cumulativePayout; guard against the arithmetic below.
-	if cheque.CumulativePayout == nil {
-		return nil, fmt.Errorf("nil cumulative payout on cheque loaded from statestore for chequebook %x: %w", chequebookAddress, ErrNoCheque)
-	}
-
 	var action cashoutAction
 	err = s.store.Get(cashoutActionKey(chequebookAddress), &action)
 	if err != nil {

pkg/settlement/swap/chequebook/chequebook.go

@@ -201,10 +201,6 @@ func (s *service) Issue(ctx context.Context, beneficiary common.Address, amount
 			return nil, err
 		}
 		cumulativePayout = big.NewInt(0)
-	} else if lastCheque.CumulativePayout == nil {
-		// a corrupt statestore entry unmarshals into a non-nil cheque with a
-		// nil cumulativePayout; guard against the arithmetic below.
-		return nil, fmt.Errorf("nil cumulative payout on cheque loaded from statestore for beneficiary %x: %w", beneficiary, ErrNoCheque)
 	} else {
 		cumulativePayout = lastCheque.CumulativePayout
 	}
@@ -280,6 +276,10 @@ func (s *service) LastCheque(beneficiary common.Address) (*SignedCheque, error)
 		return nil, fmt.Errorf("nil cheque loaded from statestore for beneficiary %x: %w", beneficiary, ErrNoCheque)
 	}
 
+	if lastCheque.CumulativePayout == nil {
+		return nil, fmt.Errorf("nil cumulative payout on cheque loaded from statestore for beneficiary %x: %w", beneficiary, ErrNilCumulativePayout)
+	}
+
 	return lastCheque, nil
 }
 

pkg/settlement/swap/chequebook/chequestore.go

@@ -26,6 +26,9 @@ const (
 var (
 	// ErrNoCheque is the error returned if there is no prior cheque for a chequebook or beneficiary.
 	ErrNoCheque = errors.New("no cheque")
+	// ErrNilCumulativePayout is returned when a stored cheque has no cumulative payout.
+	// It does not wrap ErrNoCheque: callers treat that as "no cheque" and may start a new payout at zero.
+	ErrNilCumulativePayout = errors.New("nil cumulative payout")
 	// ErrChequeNotIncreasing is the error returned if the cheque amount is the same or lower.
 	ErrChequeNotIncreasing = errors.New("cheque cumulativePayout is not increasing")
 	// ErrChequeInvalid is the error returned if the cheque itself is invalid.
@@ -103,6 +106,10 @@ func (s *chequeStore) LastCheque(chequebook common.Address) (*SignedCheque, erro
 		return nil, fmt.Errorf("nil cheque loaded from statestore for chequebook %x: %w", chequebook, ErrNoCheque)
 	}
 
+	if cheque.CumulativePayout == nil {
+		return nil, fmt.Errorf("nil cumulative payout on cheque loaded from statestore for chequebook %x: %w", chequebook, ErrNilCumulativePayout)
+	}
+
 	return cheque, nil
 }
 

pkg/settlement/swap/chequebook/nilcheque_test.go

@@ -104,6 +104,58 @@ func TestServiceLastChequeNilEntry(t *testing.T) {
 	}
 }
 
+// TestServiceLastChequeNilCumulativePayout covers a last-issued-cheque entry of
+// `{}`: it unmarshals into a non-nil cheque whose CumulativePayout is nil.
+// LastCheque must reject it with its own error, not ErrNoCheque, so Issue does
+// not treat the row as a first cheque and start the payout at zero.
+func TestServiceLastChequeNilCumulativePayout(t *testing.T) {
+	t.Parallel()
+
+	store := newRawStore()
+	beneficiary := common.HexToAddress("0xbe")
+	store.setRaw(chequebook.LastIssuedChequeKey(beneficiary), []byte(`{}`))
+
+	svc, err := chequebook.New(nil, common.Address{}, common.Address{}, store, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cheque, err := svc.LastCheque(beneficiary)
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
+	}
+	if cheque != nil {
+		t.Fatalf("expected nil cheque, got %v", cheque)
+	}
+}
+
+// TestChequeStoreLastChequeNilCumulativePayout covers a last-received-cheque
+// entry of `{}`. Readers such as settlements add CumulativePayout, so LastCheque
+// must reject the blank amount before it is returned.
+func TestChequeStoreLastChequeNilCumulativePayout(t *testing.T) {
+	t.Parallel()
+
+	store := newRawStore()
+	chequebookAddr := common.HexToAddress("0xcb")
+	store.setRaw(chequebook.LastReceivedChequeKey(chequebookAddr), []byte(`{}`))
+
+	cs := chequebook.NewChequeStore(store, nil, 1, common.HexToAddress("0xbe"), nil, nil)
+
+	cheque, err := cs.LastCheque(chequebookAddr)
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
+	}
+	if cheque != nil {
+		t.Fatalf("expected nil cheque, got %v", cheque)
+	}
+}
+
 // TestChequeStoreReceiveChequeNilCumulativePayout is a regression test for a
 // peer-supplied cheque that simply omits cumulativePayout: it unmarshals into a
 // non-nil cheque whose CumulativePayout is a nil *big.Int, which big.Int.Sub
@@ -148,8 +200,14 @@ func TestChequeStoreEmptyObjectLastReceived(t *testing.T) {
 	cs := chequebook.NewChequeStore(store, nil, 1, beneficiary, nil, nil)
 
 	lastCheque, err := cs.LastCheque(chequebookAddr)
-	if err == nil && lastCheque == nil {
-		t.Fatal("LastCheque returned a nil cheque with a nil error")
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
+	}
+	if lastCheque != nil {
+		t.Fatalf("expected nil cheque, got %v", lastCheque)
 	}
 
 	cheque := &chequebook.SignedCheque{

pkg/settlement/swap/chequebook/nilcumulativepayout_test.go

@@ -45,12 +45,9 @@ func pendingBackend() transaction.Backend {
 	)
 }
 
-// TestCashoutStatusNilCumulativePayoutLastCheque covers the guard in
-// CashoutStatus that follows LastCheque. A last-received-cheque entry of `{}`
-// unmarshals into a non-nil *SignedCheque whose CumulativePayout is a nil
-// *big.Int; every CashoutStatus branch does arithmetic on it, so without the
-// guard the arithmetic against the recorded cashout action panics. Reachable
-// from GET /chequebook/cashout/{peer}.
+// TestCashoutStatusNilCumulativePayoutLastCheque covers a last-received-cheque
+// entry of `{}`. LastCheque rejects the nil payout before CashoutStatus does
+// arithmetic on it. Reachable from GET /chequebook/cashout/{peer}.
 func TestCashoutStatusNilCumulativePayoutLastCheque(t *testing.T) {
 	t.Parallel()
 
@@ -87,8 +84,11 @@ func TestCashoutStatusNilCumulativePayoutLastCheque(t *testing.T) {
 	)
 
 	_, err = cashoutService.CashoutStatus(context.Background(), chequebookAddress)
-	if !errors.Is(err, chequebook.ErrNoCheque) {
-		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNoCheque, err)
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
 	}
 }
 
@@ -133,8 +133,8 @@ func TestCashoutStatusNilCumulativePayoutAction(t *testing.T) {
 	}
 }
 
-// TestCashChequeNilCumulativePayout covers the guard in CashCheque: the nil
-// *big.Int would otherwise be dereferenced by the ABI packer.
+// TestCashChequeNilCumulativePayout covers a last-received cheque with a nil
+// payout. LastCheque rejects it before CashCheque packs the amount for the chain.
 func TestCashChequeNilCumulativePayout(t *testing.T) {
 	t.Parallel()
 
@@ -153,14 +153,17 @@ func TestCashChequeNilCumulativePayout(t *testing.T) {
 	)
 
 	_, err := cashoutService.CashCheque(context.Background(), chequebookAddress, recipientAddress)
-	if !errors.Is(err, chequebook.ErrNoCheque) {
-		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNoCheque, err)
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
 	}
 }
 
-// TestChequebookIssueNilCumulativePayout covers the sending-side mirror in
-// Issue: a corrupt last-issued-cheque entry yields a non-nil cheque with a nil
-// CumulativePayout, which Issue would otherwise add the amount to.
+// TestChequebookIssueNilCumulativePayout covers a corrupt last-issued cheque.
+// LastCheque rejects the nil payout with its own error, so Issue must not send
+// a new cheque and must not treat the row as a first cheque starting at zero.
 func TestChequebookIssueNilCumulativePayout(t *testing.T) {
 	t.Parallel()
 
@@ -192,7 +195,10 @@ func TestChequebookIssueNilCumulativePayout(t *testing.T) {
 		t.Fatal("cheque must not be sent")
 		return nil
 	})
-	if !errors.Is(err, chequebook.ErrNoCheque) {
-		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNoCheque, err)
+	if !errors.Is(err, chequebook.ErrNilCumulativePayout) {
+		t.Fatalf("expected error wrapping %v, got %v", chequebook.ErrNilCumulativePayout, err)
+	}
+	if errors.Is(err, chequebook.ErrNoCheque) {
+		t.Fatal("corrupt payout must not be reported as no cheque")
 	}
 }

pkg/settlement/swap/chequebook/statestore_fuzz_test.go

@@ -85,10 +85,10 @@ func FuzzChequeStoreStatestoreCorruption(f *testing.F) {
 			func(*chequebook.SignedCheque, int64) (common.Address, error) { return common.Address{}, nil },
 		)
 
-		// 1. LastCheque must never panic
+		// 1. LastCheque must never panic, and a success must carry a payout.
 		lastCheque, err := cs.LastCheque(chequebookAddr)
-		if err == nil && lastCheque == nil {
-			t.Fatal("NIL-02: LastCheque returned (nil, nil)")
+		if err == nil && (lastCheque == nil || lastCheque.CumulativePayout == nil) {
+			t.Fatal("LastCheque returned a cheque with no cumulative payout")
 		}
 
 		// 2. ReceiveCheque must never panic when reading corrupted last-received cheque
@@ -106,8 +106,8 @@ func FuzzChequeStoreStatestoreCorruption(f *testing.F) {
 		svc, err := chequebook.New(nil, common.Address{}, common.Address{}, store, nil, nil)
 		if err == nil {
 			issuedCheque, err := svc.LastCheque(beneficiary)
-			if err == nil && issuedCheque == nil {
-				t.Fatal("NIL-04: svc.LastCheque returned (nil, nil)")
+			if err == nil && (issuedCheque == nil || issuedCheque.CumulativePayout == nil) {
+				t.Fatal("svc.LastCheque returned a cheque with no cumulative payout")
 			}
 		}
 	})