Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions lib/features/labels/adapters/labels_repository_adapter.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:bb_mobile/core/storage/storage.dart';
import 'package:bb_mobile/core/utils/logger.dart';
import 'package:bb_mobile/features/labels/adapters/label_mapper.dart';
import 'package:bb_mobile/features/labels/application/labels_repository_port.dart';
import 'package:bb_mobile/features/labels/domain/label_entity.dart';
Expand All @@ -11,6 +12,21 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort {

@override
Future<LabelEntity> store(NewLabel newLabel) async {
// Validate BEFORE writing: constructing a LabelEntity is what enforces
// its invariants (see LabelEntity._validateReference), and it must
// throw here — before the insert below — or a caller told the store
// failed has in fact already had its row persisted (the previous shape
// built the companion from the unvalidated newLabel directly and only
// constructed a LabelEntity afterwards, purely to shape the return
// value, by which point the row was already committed).
LabelEntity(
id: 0, // unknown before insert; only the validation side effect matters
type: newLabel.type,
label: newLabel.label,
reference: newLabel.reference,
origin: newLabel.origin,
);

final companion = LabelMapper.newLabelEntityToCompanion(newLabel);
final id = await _database
.into(_database.labels)
Expand All @@ -36,15 +52,15 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort {
final rows = await _database.managers.labels
.filter((l) => l.label(label))
.get();
return rows.map((row) => LabelMapper.toLabelEntity(row)).toList();
return _mapRowsTolerantly(rows);
}

@override
Future<List<LabelEntity>> fetchByReference(String reference) async {
final rows = await _database.managers.labels
.filter((l) => l.reference(reference))
.get();
return rows.map((row) => LabelMapper.toLabelEntity(row)).toList();
return _mapRowsTolerantly(rows);
}

@override
Expand All @@ -63,6 +79,29 @@ class DriftLabelsRepositoryAdapter implements LabelsRepositoryPort {
@override
Future<List<LabelEntity>> fetchAll() async {
final rows = await _database.managers.labels.get();
return rows.map((row) => LabelMapper.toLabelEntity(row)).toList();
return _mapRowsTolerantly(rows);
}

/// Maps each row independently and drops (with a log) any row that fails
/// [LabelEntity]'s validation, instead of letting `.map().toList()`
/// propagate the first bad row's exception and discard every valid label
/// in the same query. Every fetch method here feeds every label lookup in
/// the app (including the wallet transaction list's per-input/output
/// label enrichment), so one corrupt row used to silently blank out label
/// data everywhere it was read.
List<LabelEntity> _mapRowsTolerantly(List<LabelRow> rows) {
final entities = <LabelEntity>[];
for (final row in rows) {
try {
entities.add(LabelMapper.toLabelEntity(row));
} catch (e) {
log.warning(
'Skipping corrupt label row id=${row.id}: failed to map to a '
'LabelEntity',
error: e,
);
}
}
return entities;
}
}
10 changes: 8 additions & 2 deletions lib/features/labels/domain/label_entity.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,20 @@ class LabelEntity {
}

void _validateTxid(String input) {
if (reference.length != 64) {
// Validates the passed slice, not the full `reference` field: for
// LabelType.input/output/publicKey, reference is `txid:vout` and would
// never pass a 64-hex-char check on its own — every well-formed label
// of those three types was being rejected unconditionally before this
// fix (the txid slice was already split out by the caller into
// `input`, but validation looked at the untouched full field instead).
if (input.length != 64) {
throw LabelValidationException(
'Invalid transaction reference: must be 64 hex characters',
);
}

try {
hex.decode(reference);
hex.decode(input);
} catch (e) {
throw LabelValidationException(
'Invalid transaction reference: must be valid hex',
Expand Down
97 changes: 97 additions & 0 deletions test/features/labels/adapters/labels_repository_adapter_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import 'package:bb_mobile/core/storage/sqlite_database.dart';
import 'package:bb_mobile/core/storage/tables/labels_table.dart';
import 'package:bb_mobile/features/labels/adapters/labels_repository_adapter.dart';
import 'package:bb_mobile/features/labels/domain/label_entity.dart';
import 'package:bb_mobile/features/labels/domain/new_label.dart';
import 'package:bb_mobile/features/labels/domain/primitive/label_type.dart';
import 'package:drift/native.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
late SqliteDatabase db;
late DriftLabelsRepositoryAdapter adapter;

setUp(() {
db = SqliteDatabase(NativeDatabase.memory());
adapter = DriftLabelsRepositoryAdapter(database: db);
});

tearDown(() async => db.close());

group('store', () {
test('never persists a row when the label fails validation', () async {
// A transaction reference must be 64 hex chars — this one isn't.
await expectLater(
() => adapter.store(
NewLabel(
type: LabelType.transaction,
label: 'test',
reference: 'too-short',
),
),
throwsA(isA<LabelValidationException>()),
);

final rows = await db.select(db.labels).get();
expect(
rows,
isEmpty,
reason:
'a rejected store must never reach the DB — validating before '
'the insert is the whole point of this fix',
);
});

test('persists a valid label and returns it with its id', () async {
final stored = await adapter.store(
NewLabel(
type: LabelType.transaction,
label: 'payjoin',
reference: 'a' * 64,
),
);

expect(stored.id, greaterThan(0));
final rows = await db.select(db.labels).get();
expect(rows, hasLength(1));
});
});

group('fetchAll / fetchByReference tolerate a corrupt row', () {
test('a single corrupt row is skipped and logged, not letting it discard '
'every valid label in the same query', () async {
// Insert one valid row through the adapter (validated).
await adapter.store(
NewLabel(
type: LabelType.transaction,
label: 'payjoin',
reference: 'a' * 64,
),
);

// Insert a corrupt row directly at the DB level, bypassing
// LabelEntity's validation (simulates data that predates a
// validation fix, or any other source of a malformed row).
await db
.into(db.labels)
.insert(
LabelsCompanion.insert(
label: 'corrupt',
reference: 'not-a-valid-64-char-txid',
type: LabelTypeColumn.tx,
),
);

final all = await adapter.fetchAll();
expect(
all,
hasLength(1),
reason: 'the corrupt row must be dropped, not poison the whole batch',
);
expect(all.single.label, 'payjoin');

final byReference = await adapter.fetchByReference('a' * 64);
expect(byReference, hasLength(1));
});
});
}
95 changes: 95 additions & 0 deletions test/features/labels/domain/label_entity_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import 'package:bb_mobile/features/labels/domain/label_entity.dart';
import 'package:bb_mobile/features/labels/domain/primitive/label_type.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('LabelEntity reference validation', () {
final validTxid = 'a' * 64;

test('accepts a well-formed transaction reference', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.transaction,
label: 'test',
reference: validTxid,
),
returnsNormally,
);
});

test('accepts a well-formed input reference (txid:vout) — regression for '
'the bug where the full reference was validated instead of the txid '
'slice, rejecting every well-formed input/output/publicKey label', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.input,
label: 'test',
reference: '$validTxid:0',
),
returnsNormally,
);
});

test('accepts a well-formed output reference (txid:vout)', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.output,
label: 'test',
reference: '$validTxid:12',
),
returnsNormally,
);
});

test('accepts a well-formed publicKey reference (txid:vout)', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.publicKey,
label: 'test',
reference: '$validTxid:1',
),
returnsNormally,
);
});

test('rejects an input reference with a non-hex txid slice', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.input,
label: 'test',
reference: '${'z' * 64}:0',
),
throwsA(isA<LabelValidationException>()),
);
});

test('rejects an input reference with a negative vout', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.input,
label: 'test',
reference: '$validTxid:-1',
),
throwsA(isA<LabelValidationException>()),
);
});

test('rejects a transaction reference that is not 64 hex characters', () {
expect(
() => LabelEntity(
id: 1,
type: LabelType.transaction,
label: 'test',
reference: 'tooshort',
),
throwsA(isA<LabelValidationException>()),
);
});
});
}