Skip to content

Order lacks value equality, so OrderState/Dispute comparisons always report a change #652

Description

@grunch

Context

Found while writing tests for OrderState (PR #651).

lib/data/models/order.dart declares no operator == and no hashCode, so Order falls back to identity comparison. Every sibling payload model does define value equality — PaymentRequest (payment_request.dart:93), Dispute (dispute.dart:298), Peer, CantDo, Amount, RatingUser, TextMessage, RangeAmount, Currency.

Why it matters

OrderState.== and OrderState.hashCode both include order:

// lib/features/order/models/order_state.dart
other is OrderState &&
    other.status == status &&
    other.action == action &&
    other.order == order &&   // <- identity comparison
    ...

OrderState is Riverpod state. Riverpod skips notifying listeners when the new state == the old one. Because two structurally identical Order instances are never equal, any OrderState carrying an order compares unequal to itself, so every rebuild of that state notifies every listener and re-renders the trade UI even when nothing changed.

Dispute.== and Dispute.hashCode have the same problem: both include order, so two otherwise-identical disputes never compare equal once an order is attached.

Reproduction

Order build() => const Order(
      id: 'order-1',
      kind: OrderType.sell,
      status: Status.pending,
      amount: 50000,
      fiatCode: 'USD',
      fiatAmount: 100,
      paymentMethod: 'Wire transfer',
    );

final a = OrderState(status: Status.pending, action: Action.newOrder, order: build());
final b = OrderState(status: Status.pending, action: Action.newOrder, order: build());

print(a == b); // false — expected true

The current behaviour is pinned in test/features/order/models/order_state_test.dart:

test('states holding equal-but-distinct orders compare unequal today', () {
  expect(baseState(), isNot(baseState()));
});

How to fix

Add value equality to Order, matching the style already used by PaymentRequest and Dispute:

@override
bool operator ==(Object other) {
  if (identical(this, other)) return true;
  return other is Order &&
      other.id == id &&
      other.kind == kind &&
      other.status == status &&
      other.amount == amount &&
      other.fiatCode == fiatCode &&
      other.minAmount == minAmount &&
      other.maxAmount == maxAmount &&
      other.fiatAmount == fiatAmount &&
      other.paymentMethod == paymentMethod &&
      other.premium == premium &&
      other.masterBuyerPubkey == masterBuyerPubkey &&
      other.masterSellerPubkey == masterSellerPubkey &&
      other.buyerTradePubkey == buyerTradePubkey &&
      other.sellerTradePubkey == sellerTradePubkey &&
      other.buyerInvoice == buyerInvoice &&
      other.createdAt == createdAt &&
      other.expiresAt == expiresAt;
}

@override
int get hashCode => Object.hashAll([
      id, kind, status, amount, fiatCode, minAmount, maxAmount,
      fiatAmount, paymentMethod, premium, masterBuyerPubkey,
      masterSellerPubkey, buyerTradePubkey, sellerTradePubkey,
      buyerInvoice, createdAt, expiresAt,
    ]);

Order is already immutable with a const constructor, so this is safe.

Then flip the pinned test in order_state_test.dart from isNot to an equality assertion, and re-check the OrderState/Dispute equality tests.

Watch out for

Anything that relies on the current identity semantics to force a rebuild — grep for orderNotifierProvider, ref.listen<OrderState> and state = in lib/features/order/notifiers/. After the fix a genuinely unchanged state stops notifying, which is the intent, but code that mutated something outside the compared fields and relied on the notification would need to be updated.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions