BREAKING CHANGE: refactor labels with the new architecture

[ethicnology]

Schema Migration (v11 → v12)

Labels table changes:

• Add id autoincrement primary key
• Rename ref → reference
• Remove spendable column
• Add unique constraint on (label, reference)

Key Changes

• Domain: LabelEntity with reference validators (tx, pubkey, input/output, xpub)
• Facade: Public API returning primitive types, uses StoreLabelEnvelope for input
• Ports/Adapters: Clean separation - usecases depend on abstract ports, not implementations
• i18n: Simplified export success message (removed singular/plural)

Files

• schema_11_to_12.dart - Migration preserving existing data
• label_entity.dart - Domain entity with validators
• labels_facade.dart - Public API
• *_port.dart / *_adapter.dart - Hexagonal boundaries

[kumulynja]

You should not directly import a Facade here, as this tightly couples the "what" (adding a label) with the "how" (through the labels feature). A usecase should only get Ports injected, not concrete implementations, so you should create an ExchangeLabelsPort and implement that Port with the LabelsFacade. You could make the interface of the port something like addLabelsForOrder and then just pass the order to it, so in the concrete implementation of the ExchangeLabelsPort you can check the order type etc and decide which labels to add based on that. I would also move the system labels check and everything to it, as those are implementation details, the usecase just wants to make sure the order is labeled, it shouldn't know how exactly this is done.

[ethicnology]

This usecase is out of the scope as it belongs to the exchange feature. This PR focus on refactoring the labels folder

[ethicnology]

This usecase is out of the scope as it belongs to the exchange feature. This PR focus on refactoring the labels folder

[kumulynja]

Oh ok, just wrote another comment saying the same and only now see you responded haha.
Will focus only on the labels feature code itself.

[ethicnology]

We will refactor this usecase in when we attack the exchange feature

[kumulynja]

I think this is not a good usecase, this labeling should be part of the usecase that creates the orders.
But I don't know if we should refactor that when we apply the new architecture guidelines to the exchange features and we should focus on the labels feature here.

[ethicnology]

The labeling is already part of the usecase that creates orders. This usecase is executed once the first time the user connect to the exchange to ensure it is labeling the orders that might have been created before the app was supporting the exchange.

The day we attack the exchange folder, let's rename it better!

LabelExistingExchangeOrdersUsecase ?

[kumulynja]

I was thinking some more about this and I actually don't even think this should be done from the exchange feature. In the end it is the labels feature that needs a migration of the labels, or a seeding in some sense of missed values. So I was thinking that we should actually remove this usecase and let the labels feature use the core BBX api client directly to fetch the orders and seed the labels in a real db migration/seeding in the labels feature. It is a pure migration (framework) thing from the labels, no usecase needed and certainly not in exchange features.

[kumulynja]

This can just be moved to the domain folder. Primitives are part of the domain. No separate primitives folder needed.

[kumulynja]

This can just be moved to the domain folder. Primitives are part of the domain. No separate primitives folder needed.

[kumulynja]

The LabelEntity is too linked to bip329 specs still, it is also unclear if it is a system label or not. This is something crucial for our app, since it is the business that wants us to label things automatically. So this should reflect in the domain and be more explicit instead of needing to parse the label itself (which isn’t even clear from looking at the domain if that’s the way to know). Introducing the Provenance concept would help. I will add a general comment at the end or in another issue as to how we could model the domain better.

[kumulynja]

id should not be optional, one of the main characteristics of an entity is that it has an identity.

[kumulynja]

I can not comment on a folder here, but the whole usecases folder should move to the application layer, since usecases are application logic, not domain logic.

[kumulynja]

abel_error mixes UI build context with domain errors. This should be done in the UI layer. The domain errors should stay strictly domain related and shouldn’t even be the same errors exposed to the UI.

[kumulynja]

Add page.dart into UI folder outside of presentation folder. We might create other screens and label related widgets that could be put in UI too then and pure UI is something that can be embedded in other features without problems, while the presentation folder should be more about the controllers (BLoC/Cubit) which I think shouldn't be used from other features.

[kumulynja]

Also, the id is now added here, but it is not used anywhere, so this addition doesn't add what it should be adding at the moment, everything is still by reference. We need the facade and use cases to use id, not the bip329 reference, otherwise there is still no way to edit a label cleanly and easily

[kumulynja]

This shouldn’t have a toModelbehavior since this is supposed to be a plain data object used by other features that never need this. Better to define a mapper class for it or if you really want to have a function like this somewhere and avoid an extra class, then I think it is better on the ApplicationLabel class since it is more internal at least. Otherwise you are leaking application related behaviour in the public object.

[kumulynja]

Origin shouldn’t be required here since it is an optional field.

[kumulynja]

As a general comment, the domain is not modelled well, which leaks in the usecases and public api in the end too with things like it being impossible to edit an existing label. With a lot of unclarity and string manipulations for the system labels as well. This can be solved by better modelling of the domain by introducing custom types/classes that have meaning in our domain instead of using just native types. It helps with type safety and it makes polymorphism easier later as well since you can extend/implement these concepts.

Here is an example of how the LabelEntity could be modeled by using value objects that give clear meanings to the fields of the entity independent of any external label specs:

import 'dart:collection';

/// Labels domain definitions in a Framework-agnostic way: no JSON, no persistence, no label specs (bip329, etc).

/// ------------
/// Entities
/// ------------

/// Label entity representing a label attached to a target independent of storage or transport format.
///
/// Immutable:
/// - All fields are final
/// - Update operations return a new Label instance
/// - No clock usage in the domain: caller supplies timestamps
final class Label {
  final LabelId id;
  final LabelContent content;
  final LabelTarget target;
  final LabelProvenance provenance;
  final LabelScope? scope;
  final DateTime createdAt;
  final DateTime updatedAt;

  Label({
    required this.id,
    required this.target,
    required this.content,
    required this.provenance,
    this.scope,
    required this.createdAt,
    required this.updatedAt,
  }) {
    _validateTimestamps(createdAt: createdAt, updatedAt: updatedAt);
  }

  /// Returns a new Label with updated content and caller-supplied updatedAt.
  Label withContent(LabelContent newContent, {required DateTime updatedAt}) {
    _validateTimestamps(createdAt: createdAt, updatedAt: updatedAt);
    return Label(
      id: id,
      target: target,
      content: newContent,
      provenance: provenance,
      scope: scope,
      createdAt: createdAt,
      updatedAt: updatedAt,
    );
  }

  /// Convenience: update just the text/tags/note within content immutably.
  Label updateContent({
    LabelText? newText,
    LabelTags? newTags,
    LabelNote? Function()? newNoteSupplier,
    required DateTime updatedAt,
  }) {
    final newContent = content.update(
      newText: newText,
      newTags: newTags,
      newNoteSupplier: newNoteSupplier,
    );
    return withContent(newContent, updatedAt: updatedAt);
  }

  static void _validateTimestamps({
    required DateTime createdAt,
    required DateTime updatedAt,
  }) {
    // You can enforce UTC if you want:
    // if (!createdAt.isUtc || !updatedAt.isUtc) throw ArgumentError('Use UTC');
    if (updatedAt.isBefore(createdAt)) {
      throw ArgumentError('updatedAt cannot be before createdAt');
    }
  }

  @override
  bool operator ==(Object other) => other is Label && other.id == id;

  @override
  int get hashCode => id.hashCode;
}

/// ------------
/// Value Objects
/// ------------

/// ------------
/// LabelId
/// ------------
/// LabelId instead of directly using int for type safety.
final class LabelId {
  final int value;
  const LabelId(this.value) {
    if (value <= 0) throw ArgumentError('LabelId must be positive');
  }

  @override
  String toString() => value.toString();

  @override
  bool operator ==(Object other) => other is LabelId && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// ------------
/// Content
/// ------------
/// Bundle of different elements a label can have as (extended) content.
///
/// Immutable:
/// - fields are final
/// - update(...) returns a new instance
final class LabelContent {
  final LabelText text;
  final LabelTags tags;
  final LabelNote? note;

  const LabelContent({
    required this.text,
    this.tags = const LabelTags([]),
    this.note,
  });

  LabelContent update({
    LabelText? newText,
    LabelTags? newTags,
    /// Nullable update semantics for note:
    /// - omit `newNoteSupplier` to keep current note
    /// - `() => someNote` to set
    /// - `() => null` to clear
    LabelNote? Function()? newNoteSupplier,
  }) {
    return LabelContent(
      text: newText ?? text,
      tags: newTags ?? tags,
      note: newNoteSupplier != null ? newNoteSupplier() : note,
    );
  }

  @override
  bool operator ==(Object other) =>
      other is LabelContent &&
      other.text == text &&
      other.tags == tags &&
      other.note == note;

  @override
  int get hashCode => Object.hash(text, tags, note);
}

final class LabelText {
  final String value;
  final int maxLength;

  LabelText(String input, {this.maxLength = 120}) : value = input.trim() {
    if (value.isEmpty) throw ArgumentError('LabelText cannot be empty');
    if (value.length > maxLength) {
      throw ArgumentError('LabelText too long (max $maxLength)');
    }
  }

  @override
  String toString() => value;

  @override
  bool operator ==(Object other) => other is LabelText && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// Optional tags (kept small and normalized).
final class Tag {
  final String value;
  Tag(String input) : value = _normalize(input) {
    if (value.isEmpty) throw ArgumentError('Tag cannot be empty');
    if (value.length > 32) throw ArgumentError('Tag too long (max 32)');
  }

  static String _normalize(String s) => s.trim().toLowerCase();

  @override
  String toString() => value;

  @override
  bool operator ==(Object other) => other is Tag && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// A set-like VO for tags.
///
/// Immutable:
/// - stores an UnmodifiableSetView
/// - "add/remove" return new instances
final class LabelTags {
  final UnmodifiableSetView<Tag> values;

  const LabelTags._(this.values);

  factory LabelTags(Iterable<Tag> tags) {
    final set = <Tag>{...tags};
    return LabelTags._(UnmodifiableSetView<Tag>(set));
  }

  bool get isEmpty => values.isEmpty;

  LabelTags add(Tag tag) => LabelTags([...values, tag]);

  LabelTags remove(Tag tag) => LabelTags(values.where((t) => t != tag));

  LabelTags addAll(Iterable<Tag> tags) => LabelTags([...values, ...tags]);

  LabelTags removeAll(Iterable<Tag> tags) {
    final toRemove = tags.toSet();
    return LabelTags(values.where((t) => !toRemove.contains(t)));
  }

  @override
  String toString() => values.map((t) => t.value).join(',');

  @override
  bool operator ==(Object other) =>
      other is LabelTags && _setEquals(other.values, values);

  @override
  int get hashCode => Object.hashAll(values);

  static bool _setEquals(Set<Tag> a, Set<Tag> b) {
    if (a.length != b.length) return false;
    for (final e in a) {
      if (!b.contains(e)) return false;
    }
    return true;
  }
}

/// Optional longer note.
final class LabelNote {
  final String value;
  final int maxLength;

  LabelNote(String input, {this.maxLength = 2000}) : value = input.trim() {
    if (value.isEmpty) throw ArgumentError('LabelNote cannot be empty');
    if (value.length > maxLength) {
      throw ArgumentError('LabelNote too long (max $maxLength)');
    }
  }

  @override
  String toString() => value;

  @override
  bool operator ==(Object other) => other is LabelNote && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// ------------
/// Targets
/// ------------
sealed class LabelTarget {
  const LabelTarget();

  /// A stable key for deduping / indexing labels by target.
  String get key;
}

/// Label attached to a transaction.
final class TxTarget extends LabelTarget {
  final TxId txId;
  const TxTarget(this.txId);

  @override
  String get key => 'tx:${txId.hex}';

  @override
  bool operator ==(Object other) => other is TxTarget && other.txId == txId;

  @override
  int get hashCode => Object.hash('tx', txId);
}

/// Label attached to a specific UTXO (outpoint).
final class UtxoTarget extends LabelTarget {
  final OutPoint outPoint;
  const UtxoTarget(this.outPoint);

  @override
  String get key => 'utxo:${outPoint.txId.hex}:${outPoint.vout}';

  @override
  bool operator ==(Object other) =>
      other is UtxoTarget && other.outPoint == outPoint;

  @override
  int get hashCode => Object.hash('utxo', outPoint);
}

/// Label attached to an address.
final class AddressTarget extends LabelTarget {
  final BitcoinAddress address;
  const AddressTarget(this.address);

  @override
  String get key => 'addr:${address.value}';

  @override
  bool operator ==(Object other) =>
      other is AddressTarget && other.address == address;

  @override
  int get hashCode => Object.hash('addr', address);
}

/// Unknown target to store import references that aren't recognized/supported by the app yet.
final class UnknownTarget extends LabelTarget {
  final TargetRef ref;
  const UnknownTarget(this.ref);

  @override
  String get key => 'unknown:${ref.value}';

  @override
  bool operator ==(Object other) =>
      other is UnknownTarget && other.ref == ref;

  @override
  int get hashCode => Object.hash('unknown', ref);
}

final class TargetRef {
  final String value;
  TargetRef(String input) : value = input.trim() {
    if (value.isEmpty) throw ArgumentError('TargetRef cannot be empty');
    if (value.length > 1024) throw ArgumentError('TargetRef too long');
  }

  @override
  String toString() => value;

  @override
  bool operator ==(Object other) => other is TargetRef && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// Bitcoin transaction id (hex string).
final class TxId {
  final String hex;
  TxId(String input) : hex = input.trim().toLowerCase() {
    if (hex.length != 64) {
      throw ArgumentError('TxId must be 64 hex chars');
    }
    final isHex = RegExp(r'^[0-9a-f]{64}$').hasMatch(hex);
    if (!isHex) {
      throw ArgumentError('TxId must be hex');
    }
  }

  @override
  String toString() => hex;

  @override
  bool operator ==(Object other) => other is TxId && other.hex == hex;

  @override
  int get hashCode => hex.hashCode;
}

/// A transaction outpoint: txid + vout.
final class OutPoint {
  final TxId txId;
  final int vout;

  OutPoint({required this.txId, required this.vout}) {
    if (vout < 0) throw ArgumentError('vout must be >= 0');
  }

  @override
  String toString() => '${txId.hex}:$vout';

  @override
  bool operator ==(Object other) =>
      other is OutPoint && other.txId == txId && other.vout == vout;

  @override
  int get hashCode => Object.hash(txId, vout);
}

/// A Bitcoin address as a raw string.
final class BitcoinAddress {
  final String value;
  BitcoinAddress(String input) : value = input.trim() {
    if (value.isEmpty) {
      throw ArgumentError('BitcoinAddress cannot be empty');
    }
    // More validation could be added here based on address formats and supported networks.
  }

  @override
  String toString() => value;

  @override
  bool operator ==(Object other) =>
      other is BitcoinAddress && other.value == value;

  @override
  int get hashCode => value.hashCode;
}

/// ------------
/// Provenance
/// ------------
sealed class LabelProvenance {
  const LabelProvenance();
}

final class CreatedByUser extends LabelProvenance {
  const CreatedByUser();
}

final class CreatedBySystem extends LabelProvenance {
  const CreatedBySystem();
}

final class Imported extends LabelProvenance {
  final LabelFormat format;
  final String? source; // e.g. filename, app name, etc.
  const Imported({required this.format, this.source});
}

enum LabelFormat {
  bip329,
  customJson,
  unknown,
}

/// ------------
/// Scope
/// ------------
sealed class LabelScope {
  const LabelScope();
}

final class WalletScoped extends LabelScope {
  final int walletId;
  const WalletScoped(this.walletId);
}