diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 76a449d..6fba8e9 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -2503,8 +2503,19 @@ class MeshCoreConnector extends ChangeNotifier { _maybeStartInitialChannelSync(); } + /// Keep [BlockService]'s notion of "me" in sync with the connected node, so + /// it can refuse a self-block and self-heal one that already exists — the + /// union pull can land before self-info arrives, so healing matters (#250). + void _applySelfKeyToBlockService() { + final service = _blockService; + if (service == null) return; + final key = _selfPublicKey; + unawaited(service.setSelfKey(key == null ? null : pubKeyToHex(key))); + } + void _resetConnectionHandshakeState() { _selfPublicKey = null; + _applySelfKeyToBlockService(); _selfName = null; _selfLatitude = null; _selfLongitude = null; @@ -2671,6 +2682,7 @@ class MeshCoreConnector extends ChangeNotifier { _conversations.clear(); _loadedConversationKeys.clear(); _selfPublicKey = null; + _applySelfKeyToBlockService(); _selfName = null; _selfLatitude = null; _selfLongitude = null; @@ -4489,6 +4501,7 @@ class MeshCoreConnector extends ChangeNotifier { _currentTxPower = reader.readInt8(); _maxTxPower = reader.readInt8(); _selfPublicKey = reader.readBytes(pubKeySize); + _applySelfKeyToBlockService(); _selfLatitude = reader.readInt32LE() / 1000000.0; _selfLongitude = reader.readInt32LE() / 1000000.0; _multiAcks = reader.readByte(); diff --git a/lib/screens/contacts_screen.dart b/lib/screens/contacts_screen.dart index 2b39a4e..4d178e0 100644 --- a/lib/screens/contacts_screen.dart +++ b/lib/screens/contacts_screen.dart @@ -1261,6 +1261,7 @@ class _ContactsScreenState extends State final isFavorite = contact.isFavorite; final blockService = context.read(); final isBlocked = blockService.isBlocked(contact.publicKeyHex); + final isSelf = blockService.isSelf(contact.publicKeyHex); showModalBottomSheet( context: context, @@ -1268,25 +1269,27 @@ class _ContactsScreenState extends State child: Column( mainAxisSize: MainAxisSize.min, children: [ - ListTile( - leading: Icon( - isBlocked ? Icons.check_circle_outline : Icons.block, - color: isBlocked ? null : Colors.red.shade700, - ), - title: Text( - isBlocked - ? context.l10n.block_unblock - : context.l10n.block_block, + // Blocking your own node is never meaningful (#250). + if (!isSelf) + ListTile( + leading: Icon( + isBlocked ? Icons.check_circle_outline : Icons.block, + color: isBlocked ? null : Colors.red.shade700, + ), + title: Text( + isBlocked + ? context.l10n.block_unblock + : context.l10n.block_block, + ), + onTap: () async { + Navigator.pop(sheetContext); + if (isBlocked) { + await blockService.unblock(contact.publicKeyHex); + } else { + await blockService.block(contact.publicKeyHex); + } + }, ), - onTap: () async { - Navigator.pop(sheetContext); - if (isBlocked) { - await blockService.unblock(contact.publicKeyHex); - } else { - await blockService.block(contact.publicKeyHex); - } - }, - ), if (isRepeater) ...[ ListTile( leading: const Icon(Icons.radar, color: Colors.green), diff --git a/lib/screens/discovery_screen.dart b/lib/screens/discovery_screen.dart index bb49969..d97f16f 100644 --- a/lib/screens/discovery_screen.dart +++ b/lib/screens/discovery_screen.dart @@ -220,6 +220,7 @@ class _DiscoveryScreenState extends State { ) async { final blockService = context.read(); final isBlocked = blockService.isBlocked(contact.publicKeyHex); + final isSelf = blockService.isSelf(contact.publicKeyHex); final action = await showModalBottomSheet( context: context, showDragHandle: true, @@ -239,16 +240,20 @@ class _DiscoveryScreenState extends State { title: Text(l10n.discoveredContacts_copyContact), onTap: () => Navigator.of(sheetContext).pop('copy_contact'), ), - ListTile( - leading: Icon( - isBlocked ? Icons.check_circle_outline : Icons.block, - color: isBlocked ? null : Colors.red.shade700, + // Blocking your own node is never meaningful (#250). + if (!isSelf) + ListTile( + leading: Icon( + isBlocked ? Icons.check_circle_outline : Icons.block, + color: isBlocked ? null : Colors.red.shade700, + ), + title: Text( + isBlocked ? l10n.block_unblock : l10n.block_block, + ), + onTap: () => Navigator.of( + sheetContext, + ).pop(isBlocked ? 'unblock' : 'block'), ), - title: Text(isBlocked ? l10n.block_unblock : l10n.block_block), - onTap: () => Navigator.of( - sheetContext, - ).pop(isBlocked ? 'unblock' : 'block'), - ), ListTile( leading: const Icon(Icons.delete), title: Text(l10n.discoveredContacts_deleteContact), diff --git a/lib/services/block_service.dart b/lib/services/block_service.dart index 07d7403..0b27747 100644 --- a/lib/services/block_service.dart +++ b/lib/services/block_service.dart @@ -22,6 +22,37 @@ class BlockService extends ChangeNotifier { /// (the pull side), so a synced key never echoes back to the radio. void Function(String keyHex, bool blocked)? firmwareSync; + String? _selfKeyHex; + + /// The connected node's own public key, injected by the connector once it is + /// learned and cleared on disconnect. Blocking yourself silently hides your + /// own traffic with no way to see why, so every add path refuses it (#250). + String? get selfKeyHex => _selfKeyHex; + + bool isSelf(String publicKeyHex) { + final self = _selfKeyHex; + return self != null && publicKeyHex.toLowerCase() == self; + } + + /// Called by the connector when the self key is learned (or cleared). If the + /// self key is already stored — blocked before this guard existed, or pulled + /// in from the radio's list — drop it here and tell the radio to remove it. + Future setSelfKey(String? publicKeyHex) async { + final key = publicKeyHex?.toLowerCase(); + final normalized = (key == null || key.isEmpty) ? null : key; + final changed = normalized != _selfKeyHex; + _selfKeyHex = normalized; + // Heal even when the key is unchanged — a self-block can appear after the + // key is already known (a union pull racing self-info, or a stale store), + // and an unchanged-key early return would strand it. + final healed = normalized != null && _blockedKeys.remove(normalized); + if (healed) firmwareSync?.call(normalized, false); + if (changed || healed) notifyListeners(); + // Everything above is synchronous, so a caller that doesn't await this + // still observes consistent state immediately; only the write is deferred. + if (healed) await _store.saveKeys(_blockedKeys); + } + /// Load persisted state. Call once during app startup. Future load() async { _blockedKeys @@ -45,6 +76,7 @@ class BlockService extends ChangeNotifier { Future block(String publicKeyHex) async { final key = publicKeyHex.toLowerCase(); + if (isSelf(key)) return; if (!_blockedKeys.add(key)) return; await _store.saveKeys(_blockedKeys); firmwareSync?.call(key, true); @@ -65,7 +97,11 @@ class BlockService extends ChangeNotifier { Future importKeys(Iterable keysHex) async { var changed = false; for (final k in keysHex) { - if (_blockedKeys.add(k.toLowerCase())) changed = true; + final key = k.toLowerCase(); + // A self-block already pushed to the radio must not come back via the + // union pull (#250). + if (isSelf(key)) continue; + if (_blockedKeys.add(key)) changed = true; } if (!changed) return; await _store.saveKeys(_blockedKeys); @@ -97,6 +133,13 @@ class BlockService extends ChangeNotifier { if (n.isEmpty || !_blockedNames.containsKey(n)) return; final key = publicKeyHex.toLowerCase(); _blockedNames.remove(n); + // Your own name resolving to your own key must not promote into a + // self-block — drop the name block and stop (#250). + if (isSelf(key)) { + await _store.saveNames(_blockedNames); + notifyListeners(); + return; + } final added = _blockedKeys.add(key); await _store.saveNames(_blockedNames); await _store.saveKeys(_blockedKeys); diff --git a/test/services/block_service_test.dart b/test/services/block_service_test.dart new file mode 100644 index 0000000..a65e770 --- /dev/null +++ b/test/services/block_service_test.dart @@ -0,0 +1,154 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/services/block_service.dart'; +import 'package:meshcore_open/storage/block_store.dart'; + +/// In-memory stand-in so the service can be exercised without SharedPreferences. +class _FakeBlockStore implements BlockStore { + Set keys = {}; + Map names = {}; + + @override + Future> loadKeys() async => {...keys}; + + @override + Future saveKeys(Set value) async => keys = {...value}; + + @override + Future> loadNames() async => {...names}; + + @override + Future saveNames(Map value) async => names = {...value}; +} + +void main() { + const selfKey = 'aa11bb22cc33dd44'; + const otherKey = '99ff88ee77dd66cc'; + + late _FakeBlockStore store; + late BlockService service; + late List<({String key, bool blocked})> pushes; + + setUp(() { + store = _FakeBlockStore(); + service = BlockService(store: store); + pushes = []; + service.firmwareSync = (key, blocked) => + pushes.add((key: key, blocked: blocked)); + }); + + group('self-block guard (#250)', () { + test('block() refuses the self key and pushes nothing', () async { + await service.setSelfKey(selfKey); + pushes.clear(); + + await service.block(selfKey); + + expect(service.isBlocked(selfKey), isFalse); + expect(store.keys, isNot(contains(selfKey))); + expect(pushes, isEmpty); + }); + + test('block() still blocks a normal key', () async { + await service.setSelfKey(selfKey); + + await service.block(otherKey); + + expect(service.isBlocked(otherKey), isTrue); + expect(pushes, contains((key: otherKey, blocked: true))); + }); + + test('importKeys() skips self so the radio cannot re-import it', () async { + await service.setSelfKey(selfKey); + + await service.importKeys([selfKey, otherKey]); + + expect(service.isBlocked(selfKey), isFalse); + expect(service.isBlocked(otherKey), isTrue); + }); + + test('setSelfKey() heals an existing self-block and sends REMOVE', () async { + // Blocked before the guard existed (or pulled in before self-info landed). + await service.block(selfKey); + expect(service.isBlocked(selfKey), isTrue); + pushes.clear(); + + await service.setSelfKey(selfKey); + + expect(service.isBlocked(selfKey), isFalse); + expect(store.keys, isNot(contains(selfKey))); + expect(pushes, contains((key: selfKey, blocked: false))); + }); + + test('maybePromote() will not promote a name into a self-block', () async { + await service.setSelfKey(selfKey); + await service.blockName('me'); + pushes.clear(); + + await service.maybePromote('me', selfKey); + + expect(service.isBlocked(selfKey), isFalse); + expect(service.isNameBlocked('me'), isFalse); + expect(pushes, isEmpty); + }); + + test('heals a self-block even when the self key is unchanged', () async { + await service.setSelfKey(selfKey); + + // Stale persisted state reloaded while the self key is already known — + // load() does not filter, so this lands a self-block behind the guards. + store.keys = {selfKey, otherKey}; + await service.load(); + expect(service.isBlocked(selfKey), isTrue); + pushes.clear(); + + // Same key as before, so an unchanged-key early return would strand it. + await service.setSelfKey(selfKey); + + expect(service.isBlocked(selfKey), isFalse); + expect( + service.isBlocked(otherKey), + isTrue, + reason: 'only self is healed', + ); + expect(pushes, contains((key: selfKey, blocked: false))); + }); + + test('repeat setSelfKey with nothing to heal pushes nothing', () async { + await service.setSelfKey(selfKey); + pushes.clear(); + + await service.setSelfKey(selfKey); + + expect(pushes, isEmpty); + expect(service.isSelf(selfKey), isTrue); + }); + + test('state is consistent synchronously when not awaited', () async { + await service.setSelfKey(selfKey); + await service.setSelfKey(null); + await service.block(selfKey); + + // Fire-and-forget, as the connector does via unawaited(). + final future = service.setSelfKey(selfKey); + + expect(service.isSelf(selfKey), isTrue); + expect(service.isBlocked(selfKey), isFalse); + await future; + expect(store.keys, isNot(contains(selfKey))); + }); + + test('isSelf() is false when no self key is known', () { + expect(service.isSelf(selfKey), isFalse); + }); + + test('clearing the self key stops treating the old key as self', () async { + await service.setSelfKey(selfKey); + await service.setSelfKey(null); + + expect(service.isSelf(selfKey), isFalse); + + await service.block(selfKey); + expect(service.isBlocked(selfKey), isTrue); + }); + }); +}