diff --git a/CHANGELOG.md b/CHANGELOG.md index f57b0f345..1a3a60888 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -345,6 +345,59 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **React Native holds an interest declared before `start()` and applies it + before the engine starts.** (#472) The engine's start-up exchange offers + every held space with the interest in force at that moment, and a narrowing + never deletes what a wider exchange pulled in, so interest has to be + declared after storage opens and before the engine starts. React Native's + `start()` does both steps, and the JSDoc said to call `setInterest` before + it, where both native modules rejected it with `DataStore not initialized`. + An app that moved the call after `start()` had its first exchange ask for + everything. `DataStore.setInterest` now holds a call made before `start()` + and resolves it, and `start()` applies what is held after MLS + initialization and before the native start. A pattern refused then rejects + `start()` with an error naming the space and carrying the native code, and + the engine is not started, because starting would replicate the space + whole; when the data layer is off the held call is dropped with a warning + and `start()` proceeds. `destroy()` discards what is held. The behaviour + change for a deployed app: an invalid pattern declared before `start()` + used to reject that call and now rejects `start()`; a call after `start()` + is unchanged. Both native `destroy()` paths now also release the data + store, which holds a strong reference to the engine: left set, every data + call after `destroy()` reached the stopped engine instead of rejecting, and + on Android the engine stayed alive until the collector reached it. Python's + `ProtocolManager` initializes MLS inside its own `start()` too; its first + exchange still runs under the default interest, now stated in + `docs/bridges/python.md`. + +- **`DiagnosticEvent.level` declares `debug`, which both native platforms + emit, and Android no longer emits `warn`.** (#471) The iOS and Android + managers emit diagnostics at `debug` at about seventy sites, while the + TypeScript union said `'info' | 'warning' | 'error'`. Android's GATT server + also emitted four diagnostics as `warn`, a spelling nothing declared; they + are `warning` now. The union is `'debug' | 'info' | 'warning' | 'error'`, and + `react_native_diagnostic_levels_match_every_bridge` reads every level + literal the bridges hand to a diagnostic sink and fails when a level is + emitted but not declared, or declared but never emitted. This widens a + public type to match what already arrives at runtime. It is not + automatically source-compatible: a consumer with an exhaustive `switch` or + a `Record` keyed by the level needs a `debug` entry to typecheck, and a + filter that compared against `'warn'` for the four GATT server diagnostics + should compare against `'warning'`. + +- **Python holds a pinned copy of the relay-answer prefixes, and decides + attribution in one place.** (#368) The relay's group answers reach the core + only when they carry no transport peer identity, so an answer injected with + a `sender_id` is refused as unsigned while the inject reports success. The + Swift and Kotlin bridges each hold the list and force those answers + unattributed centrally; Python held no copy, and every call site had to get + it right on its own. 0.27.0 already corrected the prefix names and the + attribution at each site (#453). Now `relay_answer_prefixes.py` holds the + list, `_inject_group_frame` drops the actor for any answer whatever the + caller passed, and `test_relay_answer_prefixes.py` pins the six literals, + so the next registry change fails the Python suite rather than a relay + feature in the field. No behaviour change for a correct caller. + - **A key package the application marks synced keeps its record, so its private key is still destroyed when it expires.** `mls_mark_key_package_synced` deleted the record and left the init key in diff --git a/bindings/python/offline_protocol_sdk/internet_manager.py b/bindings/python/offline_protocol_sdk/internet_manager.py index e0088af65..e0af8fcd2 100644 --- a/bindings/python/offline_protocol_sdk/internet_manager.py +++ b/bindings/python/offline_protocol_sdk/internet_manager.py @@ -20,6 +20,7 @@ from websockets.asyncio.client import ClientConnection from . import address_declaration +from .relay_answer_prefixes import attributable_actor from .transport_manager import TransportError, TransportManager, TransportState logger = logging.getLogger(__name__) @@ -1098,7 +1099,8 @@ def _handle_group_message(self, msg_type: str, msg: dict[str, Any]) -> None: is data plane (MLS authenticates it afterwards) and the attribution is the reachability signal for the relayed sender. - The others are relay answers (the core's ``RELAY_ANSWER_PREFIXES``) and + The others are relay answers (``relay_answer_prefixes``, mirroring the + core's ``RELAY_ANSWER_PREFIXES``) and must reach the core unattributed: no peer sent them, so nothing can sign them, and the core's exemption from the control-frame signature gate only recognizes a frame with no transport peer identity. An @@ -1157,8 +1159,11 @@ def _inject_group_frame( ``actor`` is both the FFI ``sender_id`` and the frame's ``sender``; ``None`` selects unattributed ingest, with the "relay" placeholder as - the frame sender because the core rejects an empty one. + the frame sender because the core rejects an empty one. A relay + answer is forced unattributed here, whatever the caller passed: see + ``relay_answer_prefixes``, which holds the pinned list. """ + actor = attributable_actor(prefix, actor) content = prefix + json.dumps(payload) data_bytes = json.dumps( self._build_internal_message(actor or "relay", content) diff --git a/bindings/python/offline_protocol_sdk/relay_answer_prefixes.py b/bindings/python/offline_protocol_sdk/relay_answer_prefixes.py new file mode 100644 index 000000000..71e89148c --- /dev/null +++ b/bindings/python/offline_protocol_sdk/relay_answer_prefixes.py @@ -0,0 +1,61 @@ +"""Which synthesized relay frames must reach the core unattributed. + +A port of ``RelayAnswerPrefixes.swift`` and ``RelayAnswerPrefixes.kt``. + +These are the prefixes the **relay server** originates. The bridge +synthesizes a frame from a WebSocket answer rather than receiving one from a +peer, so no key exists to sign it. The core exempts exactly these from its +control-frame signature gate (``RELAY_ANSWER_PREFIXES`` in +``crates/offline-protocol/src/protocol/prefixes.rs``), and the four copies +must agree: the core, the Swift bridge, the Kotlin bridge and this module. + +Why attribution breaks them: the core's exemption is narrower than the +prefix. It also requires the frame to carry **no transport peer identity**, +which is what a locally synthesized answer looks like. Passing a non-empty +``sender_id`` to ``internet_message_received`` sets that identity, so the +frame stops looking synthesized and is dropped as unsigned. The caller sees a +successful inject and the answer never takes effect. That narrowness is doing +real work and must not be widened to close this: without it, any peer able to +address us through the relay could inject unsigned group state. + +``__GROUP_MSG__`` is **not** here. It is a data-plane prefix, never +signature-gated (MLS authenticates it afterwards), so it keeps its +attribution and remains the reachability signal for a relayed sender. + +``test_relay_answer_prefixes.py`` pins the set as literals (contract C5 in +``docs/bridges/README.md``). A test that recomputed it from this constant +would agree with any edit, which is the failure it exists to catch. +""" + +from __future__ import annotations + +RELAY_ANSWER_PREFIXES: frozenset[str] = frozenset( + { + "__GROUP_CREATED__", + "__GROUP_MEMBER_ADDED__", + "__GROUP_MEMBER_REMOVED__", + "__GROUP_INFO__", + "__USER_GROUPS__", + "__GROUP_ERROR__", + } +) + + +def is_relay_answer(prefix: str) -> bool: + """Whether ``prefix`` names a relay answer that must be injected unattributed. + + Matched whole, not as a prefix of a prefix: whether a content string that + starts with an exempt prefix is admitted is decided in the core, against + the frame's transport and attribution. + """ + return prefix in RELAY_ANSWER_PREFIXES + + +def attributable_actor(prefix: str, actor: str | None) -> str | None: + """The actor a synthesized frame may carry: ``None`` for a relay answer. + + Enforced here rather than trusted to each call site. The rule comes from + a constant in the Rust core, and a new answer injected with an actor is + dropped as unsigned with no error anywhere. + """ + return None if is_relay_answer(prefix) else actor diff --git a/bindings/python/tests/test_internet_manager.py b/bindings/python/tests/test_internet_manager.py index e3c4c8af4..f41e913e2 100644 --- a/bindings/python/tests/test_internet_manager.py +++ b/bindings/python/tests/test_internet_manager.py @@ -249,6 +249,22 @@ def test_relay_answers_are_injected_unattributed( assert frame["recipient"] == self.ADDRESS assert frame["content"] == prefix + json.dumps(payload) + def test_a_relay_answer_is_unattributed_whatever_the_caller_passes( + self, mock_protocol: MagicMock + ) -> None: + # The rule lives in one place, not at each call site: a future arm that + # hands an actor to a relay answer must still reach the core with no + # transport identity, or the gate drops it as unsigned. + mgr = self._manager(mock_protocol) + mgr._inject_group_frame("off1someone", "__USER_GROUPS__", {"groups": []}) + sender_id, frame = self._injected(mock_protocol) + assert sender_id == "" + assert frame["sender"] == "relay" + + mgr._inject_group_frame("off1someone", "__GROUP_MSG__", {"group_id": "g"}) + sender_id, frame = self._injected(mock_protocol) + assert sender_id == "off1someone" + def test_group_delivery_report_stays_off_the_message_plane( self, mock_protocol: MagicMock ) -> None: diff --git a/bindings/python/tests/test_relay_answer_prefixes.py b/bindings/python/tests/test_relay_answer_prefixes.py new file mode 100644 index 000000000..55abb3d05 --- /dev/null +++ b/bindings/python/tests/test_relay_answer_prefixes.py @@ -0,0 +1,53 @@ +"""Pins the "a relay answer reaches the core unattributed" rule. + +Mirrors ``RelayAnswerPrefixesTests.swift`` and ``RelayAnswerPrefixesTest.kt`` +against the core's ``RELAY_ANSWER_PREFIXES``. Python used to hold no copy at +all, and the injection used prefixes the registry did not contain (#368): a +frame under an unregistered prefix is refused as unsigned, invisibly. +""" + +from __future__ import annotations + +from offline_protocol_sdk.relay_answer_prefixes import ( + RELAY_ANSWER_PREFIXES, + attributable_actor, + is_relay_answer, +) + + +def test_the_set_matches_the_core_constant() -> None: + # Written out, not derived: a prefix here the core does not exempt is + # dropped as unsigned, and one the core exempts that is missing here is + # attributed and dropped the same way. + assert RELAY_ANSWER_PREFIXES == { + "__GROUP_CREATED__", + "__GROUP_MEMBER_ADDED__", + "__GROUP_MEMBER_REMOVED__", + "__GROUP_INFO__", + "__USER_GROUPS__", + "__GROUP_ERROR__", + } + + +def test_membership_answers_are_never_attributed() -> None: + for prefix in ("__GROUP_MEMBER_ADDED__", "__GROUP_MEMBER_REMOVED__"): + assert attributable_actor(prefix, "alice") is None + + +def test_every_relay_answer_drops_its_actor() -> None: + for prefix in RELAY_ANSWER_PREFIXES: + assert attributable_actor(prefix, "alice") is None + assert attributable_actor(prefix, None) is None + + +def test_data_plane_and_peer_prefixes_keep_their_actor() -> None: + # Scoped, not blanket: `__GROUP_MSG__` is data plane and its attribution + # is the reachability signal for a relayed sender. + for prefix in ("__GROUP_MSG__", "__CONN_REQ__", "__MLS_ENC__"): + assert attributable_actor(prefix, "alice") == "alice" + + +def test_is_relay_answer_discriminates() -> None: + assert is_relay_answer("__GROUP_CREATED__") + assert not is_relay_answer("__GROUP_MSG__") + assert not is_relay_answer("__GROUP_CREATED__extra") diff --git a/bindings/react-native/README.md b/bindings/react-native/README.md index a4889486d..fbf786d97 100644 --- a/bindings/react-native/README.md +++ b/bindings/react-native/README.md @@ -1085,7 +1085,7 @@ interface FileReceivedEvent { ```typescript interface DiagnosticEvent { type: 'diagnostic'; - level: 'info' | 'warning' | 'error'; + level: 'debug' | 'info' | 'warning' | 'error'; message: string; context?: Record; } diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/OfflineProtocolModule.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/OfflineProtocolModule.kt index ea01bcbef..9934a6722 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/OfflineProtocolModule.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/OfflineProtocolModule.kt @@ -2368,6 +2368,18 @@ class OfflineProtocolModule(reactContext: ReactApplicationContext) : // destroyed handle throws; publishing the null first means the // widest that race can be is one already-in-flight call, which // [notifyAppStateQuietly] absorbs. + // The data store holds a strong reference to the engine, so it is + // released first and explicitly: left set, every data call after + // destroy() reached the stopped engine instead of rejecting with + // "DataStore not initialized", and its reference kept the engine + // alive until the collector got to it. + val store = dataStore + dataStore = null + try { + store?.destroy() + } catch (e: Exception) { + android.util.Log.w(NAME, "Releasing the data store handle failed", e) + } val handle = protocol protocol = null try { diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/RelayAnswerPrefixes.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/RelayAnswerPrefixes.kt index 8a6e91873..ad3fabb41 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/RelayAnswerPrefixes.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/RelayAnswerPrefixes.kt @@ -6,7 +6,8 @@ package com.offlineprotocol * * Mirrors `RELAY_ANSWER_PREFIXES` in * `crates/offline-protocol/src/protocol/prefixes.rs`, and iOS's - * RelayAnswerPrefixes.swift — keep all three in sync. The lists must agree: the + * RelayAnswerPrefixes.swift and Python's relay_answer_prefixes.py. Keep all + * four in sync. The lists must agree: the * core exempts exactly these from its unconditional control-frame signature * gate, because no peer sent them so no key exists to sign them. * diff --git a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/PeripheralGattServer.kt b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/PeripheralGattServer.kt index baf4584c8..5aa2528df 100644 --- a/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/PeripheralGattServer.kt +++ b/bindings/react-native/android/src/main/java/com/offlineprotocol/ble/PeripheralGattServer.kt @@ -453,7 +453,7 @@ class PeripheralGattServer( if (!isReady) { Log.w(TAG, "GATT service ready timeout (attempt=$setupAttempts)") diagnosticEmitter( - "warn", + "warning", "gatt_service_ready_timeout", mapOf("attempt" to setupAttempts), ) @@ -700,13 +700,13 @@ class PeripheralGattServer( if (cached != null) { identityReadSnapshots.remove(address) diagnosticEmitter( - "warn", + "warning", "identity_long_read_snapshot_stale", mapOf("address" to address, "offset" to offset), ) } else { diagnosticEmitter( - "warn", + "warning", "identity_long_read_missing_snapshot", mapOf("address" to address, "offset" to offset), ) @@ -744,7 +744,7 @@ class PeripheralGattServer( // the payload to the upstream fragment assembler. if (value.size > MAX_INBOUND_WRITE_BYTES) { diagnosticEmitter( - "warn", + "warning", "gatt_inbound_write_oversize", mapOf( "address" to device.address, diff --git a/bindings/react-native/ios/OfflineProtocolModule.swift b/bindings/react-native/ios/OfflineProtocolModule.swift index 716cd3942..83afde155 100644 --- a/bindings/react-native/ios/OfflineProtocolModule.swift +++ b/bindings/react-native/ios/OfflineProtocolModule.swift @@ -2648,6 +2648,10 @@ class OfflineProtocolModule: RCTEventEmitter { } protocolInstance = nil meshServicesInstance = nil + // The store holds a strong reference to the engine. Left set, every + // data call after destroy() reached the stopped engine instead of + // rejecting with "DataStore not initialized", and kept it alive. + dataStoreInstance = nil currentConfig = nil // The app tore the SDK down itself; a message held for this account // must not reach whatever it constructs next. diff --git a/bindings/react-native/ios/RelayAnswerPrefixes.swift b/bindings/react-native/ios/RelayAnswerPrefixes.swift index 087935f1d..16e4c87e6 100644 --- a/bindings/react-native/ios/RelayAnswerPrefixes.swift +++ b/bindings/react-native/ios/RelayAnswerPrefixes.swift @@ -2,7 +2,8 @@ // RelayAnswerPrefixes.swift // // Which synthesized relay frames must reach the core unattributed. -// Mirrors android's RelayAnswerPrefixes.kt — keep in sync. +// Mirrors android's RelayAnswerPrefixes.kt and Python's +// relay_answer_prefixes.py. Keep all four copies in sync. // import Foundation diff --git a/bindings/react-native/js-ci-harness/README.md b/bindings/react-native/js-ci-harness/README.md index 568f5a105..45c4a487b 100644 --- a/bindings/react-native/js-ci-harness/README.md +++ b/bindings/react-native/js-ci-harness/README.md @@ -73,3 +73,4 @@ Two traps, both of which produce a test that passes while proving nothing: | `rich-send-app-id.test.js` | The per-send `appId` on `sendMessage` and `sendMedia` (`src/index.ts`): an appId-only call must take the rich native method, because only it carries `app_id`, and the plain path would send under the configured id without a word; a rich call without one sends `null`. | | `relay-config.test.js` | The relay and DORS config payloads this layer hands to native: the whole `relay` section crossing at create time (not just `relayPriority`), the legacy `low`/`medium`/`high` spelling mapping to the engine vocabulary, and a runtime update naming only the fields it was given — which is what makes the native-side merge a partial update rather than a full overwrite. Its other half is the Rust guard `react_native_bridges_merge_dors_updates_from_the_live_config`. | | `key-package-mapping.test.js` | The JS shape of a key package record (`src/index.ts`, `toMlsKeyPackage`): `createdAt`, `expiresAt` and `isSynced` read from the `createdAtMs`, `expiresAtMs` and `synced` both native bridges send. The wrappers read other names, so every caller got `undefined` for both, and neither the typecheck nor a text guard can see a key that is sent but never read. | +| `data-interest-order.test.js` | When an interest declared with `DataStore.setInterest` reaches native (`src/index.ts`, `pendingInterest`): held before `start()`, applied between MLS initialization and the native start so the engine's start-up exchange carries it (#472), straight through afterwards, a refusal rejecting `start()` without starting the engine, a layer that is off dropping it with a warning, and `destroy()` discarding it. Its other half is the Rust guard `react_native_start_applies_held_interest_before_the_engine_starts`. | diff --git a/bindings/react-native/js-ci-harness/data-interest-order.test.js b/bindings/react-native/js-ci-harness/data-interest-order.test.js new file mode 100644 index 000000000..93c43e780 --- /dev/null +++ b/bindings/react-native/js-ci-harness/data-interest-order.test.js @@ -0,0 +1,300 @@ +#!/usr/bin/env node +/** + * Behavioral tests for the order in which a React Native interest + * declaration reaches the native data store (`DataStore.setInterest` and + * `OfflineProtocol.start` in `src/index.ts`). + * + * The engine's order is: open storage, declare interest, start. Its start-up + * exchange offers every held space with the interest in force at that + * instant, and a narrowing never deletes what a wider offer already pulled + * in. React Native folds storage and start into one `start()` call, so the + * SDK holds a declaration made before it and applies it between MLS + * initialization and the native start (#472). Each failure here is silent on + * a device: a declaration applied after the native start compiles, resolves, + * and replicates the whole space first. + * + * The sibling Rust guard `react_native_start_applies_held_interest_before_the_engine_starts` + * pins the same order as text. See README.md for why the package has no + * other JS test setup. + */ +'use strict'; + +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const Module = require('node:module'); +const os = require('node:os'); +const path = require('node:path'); + +const PACKAGE_DIR = path.resolve(__dirname, '..'); + +function compileSdk() { + const tsc = path.join(PACKAGE_DIR, 'node_modules', 'typescript', 'bin', 'tsc'); + if (!fs.existsSync(tsc)) { + throw new Error(`TypeScript not found at ${tsc} — run \`npm ci\` in ${PACKAGE_DIR} first.`); + } + const outDir = fs.mkdtempSync(path.join(os.tmpdir(), 'op-rn-interest-')); + execFileSync( + process.execPath, + [tsc, '--outDir', outDir, '--declaration', 'false', '--declarationMap', 'false'], + { cwd: PACKAGE_DIR, stdio: 'inherit' } + ); + return outDir; +} + +let nativeOverrides = {}; +let nativeCalls = []; + +const nativeModule = new Proxy( + {}, + { + get(_target, method) { + if (typeof method !== 'string') return undefined; + return (...args) => { + nativeCalls.push({ method, args }); + const override = nativeOverrides[method]; + return override ? override(...args) : Promise.resolve(); + }; + }, + } +); + +class StubNativeEventEmitter { + addListener() { + return { remove: () => {} }; + } +} + +const realLoad = Module._load; +Module._load = function loadWithReactNativeStub(request) { + if (request === 'react-native') { + return { + NativeModules: { OfflineProtocolModule: nativeModule }, + NativeEventEmitter: StubNativeEventEmitter, + }; + } + return realLoad.apply(this, arguments); +}; + +const realConsole = { log: console.log, warn: console.warn, error: console.error }; +let captured = { warn: [], error: [] }; + +function captureConsole() { + captured = { warn: [], error: [] }; + console.log = () => {}; + console.warn = (...args) => captured.warn.push(args.join(' ')); + console.error = (...args) => captured.error.push(args.join(' ')); +} + +function releaseConsole() { + Object.assign(console, realConsole); +} + +const tests = []; +const test = (name, fn) => tests.push({ name, fn }); + +let OfflineProtocol; +let DataStore; + +const newSdk = (config = {}) => + new OfflineProtocol({ appId: 'harness', profile: 'harness-profile', ...config }); + +const methods = () => nativeCalls.map((c) => c.method); +const callsTo = (method) => nativeCalls.filter((c) => c.method === method); + +/** + * Clears the module-level hold left by the previous test. Only an instance + * that created an engine resets it, so this starts one and destroys it. + */ +async function resetModuleState() { + const overrides = nativeOverrides; + nativeOverrides = {}; + const sdk = newSdk(); + await sdk.start(); + await sdk.destroy(); + nativeOverrides = overrides; + nativeCalls = []; +} + +function rejectWith(code, message) { + const error = new Error(message); + error.code = code; + return Promise.reject(error); +} + +// --------------------------------------------------------------------------- + +test('a declaration before start() is held, and reaches native nowhere', async () => { + await new DataStore().setInterest('space-1', ['inbox*']); + assert.deepEqual(callsTo('dataSetInterest'), []); +}); + +test('start() applies it after MLS initialization and before the engine starts', async () => { + await new DataStore().setInterest('space-1', ['inbox*', 'profile']); + await newSdk().start(); + + const order = methods().filter((m) => + ['create', 'initializeMlsWithSecureStorage', 'dataSetInterest', 'start'].includes(m) + ); + assert.deepEqual(order, ['create', 'initializeMlsWithSecureStorage', 'dataSetInterest', 'start']); + assert.deepEqual(callsTo('dataSetInterest')[0].args, ['space-1', ['inbox*', 'profile']]); +}); + +test('after start() a declaration goes straight to native', async () => { + await newSdk().start(); + nativeCalls = []; + await new DataStore().setInterest('space-2', ['notes*']); + assert.deepEqual(callsTo('dataSetInterest').map((c) => c.args), [['space-2', ['notes*']]]); +}); + +test('the last declaration for a space wins, and each space is applied once', async () => { + const store = new DataStore(); + await store.setInterest('space-1', ['a*']); + await store.setInterest('space-2', ['b*']); + await store.setInterest('space-1', ['c*']); + await newSdk().start(); + + assert.deepEqual(callsTo('dataSetInterest').map((c) => c.args), [ + ['space-1', ['c*']], + ['space-2', ['b*']], + ]); +}); + +test('a held declaration is a copy of the caller\'s array', async () => { + const patterns = ['a*']; + await new DataStore().setInterest('space-1', patterns); + patterns.push('everything-else*'); + await newSdk().start(); + assert.deepEqual(callsTo('dataSetInterest')[0].args[1], ['a*']); +}); + +test('a refused declaration rejects start(), names the space, and never starts the engine', async () => { + nativeOverrides.dataSetInterest = () => rejectWith('InvalidArgument', 'bad pattern'); + await new DataStore().setInterest('space-1', ['a*b']); + + let refusal; + try { + await newSdk().start(); + } catch (error) { + refusal = error; + } + assert.ok(refusal, 'start() must reject when a held interest is refused'); + assert.match(refusal.message, /space-1/); + assert.equal(refusal.code, 'InvalidArgument'); + assert.equal(callsTo('start').length, 0, 'the engine started under the default interest'); +}); + +test('the refused declaration stays held until the app corrects it', async () => { + nativeOverrides.dataSetInterest = (_space, patterns) => + patterns.includes('a*b') ? rejectWith('InvalidArgument', 'bad pattern') : Promise.resolve(); + const store = new DataStore(); + await store.setInterest('space-1', ['a*b']); + const sdk = newSdk(); + await assert.rejects(sdk.start()); + await assert.rejects(sdk.start(), 'a retry without a correction must fail the same way'); + assert.equal(callsTo('start').length, 0); + + await store.setInterest('space-1', ['a*']); + await sdk.start(); + assert.deepEqual(callsTo('dataSetInterest').at(-1).args, ['space-1', ['a*']]); + assert.equal(callsTo('start').length, 1); +}); + +test('with the data layer off, a held declaration is dropped with a warning and start() proceeds', async () => { + for (const code of ['DataDisabled', 'DataStorageUnavailable']) { + await resetModuleState(); + nativeOverrides.dataSetInterest = () => rejectWith(code, 'off'); + const store = new DataStore(); + await store.setInterest('space-1', ['a*']); + await store.setInterest('space-2', ['b*']); + captured.warn = []; + await newSdk().start(); + + assert.equal(callsTo('start').length, 1, `${code}: the engine did not start`); + assert.equal(callsTo('dataSetInterest').length, 1, `${code}: kept applying after the layer said off`); + assert.ok(captured.warn.some((w) => w.includes(code)), `${code}: no warning`); + } +}); + +test('destroy() discards a held declaration and holds the next one again', async () => { + const sdk = newSdk(); + await sdk.emitTestEvent(); // creates the engine without starting it + await new DataStore().setInterest('space-old', ['a*']); + await sdk.destroy(); + nativeCalls = []; + + await new DataStore().setInterest('space-new', ['b*']); + assert.deepEqual(callsTo('dataSetInterest'), [], 'a declaration after destroy() must be held'); + await newSdk().start(); + assert.deepEqual(callsTo('dataSetInterest').map((c) => c.args), [['space-new', ['b*']]]); +}); + +test('after a started engine is destroyed, the next declaration is held again', async () => { + const sdk = newSdk(); + await sdk.start(); + await sdk.destroy(); + nativeCalls = []; + await new DataStore().setInterest('space-1', ['a*']); + assert.deepEqual(callsTo('dataSetInterest'), []); +}); + +test('destroying an instance that never created an engine leaves a running store alone', async () => { + const running = newSdk(); + await running.start(); + const stale = newSdk(); + await stale.destroy(); + nativeCalls = []; + + await new DataStore().setInterest('space-1', ['a*']); + assert.deepEqual( + callsTo('dataSetInterest').map((c) => c.args), + [['space-1', ['a*']]], + 'a stale instance turned a live declaration into a held one nothing will apply' + ); +}); + +test('a refused space name on a layer that is off does not hold the engine back', async () => { + // The engine validates the name before it checks the layer, so the refusal + // says InvalidArgument; the probe is what reveals the layer is off. + nativeOverrides.dataSetInterest = () => rejectWith('InvalidArgument', 'bad space'); + nativeOverrides.dataListSpaces = () => rejectWith('DataDisabled', 'off'); + await new DataStore().setInterest('bad/space', ['a*']); + await newSdk().start(); + assert.equal(callsTo('start').length, 1); + assert.ok(captured.warn.some((w) => w.includes('DataDisabled'))); +}); + +// --------------------------------------------------------------------------- +// Runner +// --------------------------------------------------------------------------- + +(async () => { + const outDir = compileSdk(); + try { + ({ OfflineProtocol, DataStore } = require(path.join(outDir, 'index.js'))); + + let failed = 0; + for (const { name, fn } of tests) { + nativeOverrides = {}; + captureConsole(); + try { + await resetModuleState(); + await fn(); + releaseConsole(); + realConsole.log(` ✓ ${name}`); + } catch (error) { + failed += 1; + releaseConsole(); + realConsole.log(` ✗ ${name}\n ${error.message}`); + } + } + + realConsole.log( + failed === 0 ? `\n${tests.length} passed.` : `\n${failed} of ${tests.length} FAILED.` + ); + process.exitCode = failed === 0 ? 0 : 1; + } finally { + releaseConsole(); + fs.rmSync(outDir, { recursive: true, force: true }); + } +})(); diff --git a/bindings/react-native/package.json b/bindings/react-native/package.json index 55a5788e2..9586e8e16 100644 --- a/bindings/react-native/package.json +++ b/bindings/react-native/package.json @@ -29,7 +29,7 @@ "scripts": { "prepare": "tsc", "build": "tsc", - "test:js": "node js-ci-harness/one-shot-hold.test.js && node js-ci-harness/local-address.test.js && node js-ci-harness/forward-priority.test.js && node js-ci-harness/rich-send-app-id.test.js && node js-ci-harness/relay-config.test.js && node js-ci-harness/data-config.test.js && node js-ci-harness/security-config.test.js && node js-ci-harness/telemetry-config.test.js && node js-ci-harness/custody-config.test.js && node js-ci-harness/key-package-mapping.test.js", + "test:js": "node js-ci-harness/one-shot-hold.test.js && node js-ci-harness/local-address.test.js && node js-ci-harness/forward-priority.test.js && node js-ci-harness/rich-send-app-id.test.js && node js-ci-harness/relay-config.test.js && node js-ci-harness/data-config.test.js && node js-ci-harness/data-interest-order.test.js && node js-ci-harness/security-config.test.js && node js-ci-harness/telemetry-config.test.js && node js-ci-harness/custody-config.test.js && node js-ci-harness/key-package-mapping.test.js", "build:ios": "bash scripts/build-ios.sh", "build:android": "bash scripts/build-android.sh", "build:all": "bash scripts/build-all.sh", diff --git a/bindings/react-native/src/index.ts b/bindings/react-native/src/index.ts index 77da7ba57..984e828cc 100644 --- a/bindings/react-native/src/index.ts +++ b/bindings/react-native/src/index.ts @@ -310,6 +310,109 @@ function toMlsKeyPackage(raw: any): MlsKeyPackage { * await protocol.stop(); * ``` */ +/** + * Interest declared before the native data store could take it, by space. + * + * The engine's order is: open storage, declare interest, start. Its start-up + * exchange offers every held space with the interest in force at that + * instant, and a narrowing never deletes what a wider offer already pulled + * in. React Native folds the first and last steps into the one + * {@link OfflineProtocol.start} call, so there is no moment outside it at + * which the native store can accept a declaration: before `create` the + * native module has no store, and before MLS initialization the store has no + * storage. A declaration made then is held here, and `start()` applies it + * after storage opens and before the engine starts (#472). + * + * Module-level, like the native module it stands in front of: there is one + * native store per process, and `DataStore` instances are stateless. + */ +const pendingInterest: Map = new Map(); + +/** + * Whether the native store has taken the held interest, so a declaration can + * go straight to it. Set by `start()` once the held interest is applied, and + * cleared by `destroy()`, after which a declaration is held again. + */ +let dataStoreReady = false; + +/** + * The two data-layer answers that mean the layer stores and replicates + * nothing at all. A held interest that meets one of them cannot be violated + * by starting, so `start()` warns rather than refusing. + */ +const DATA_LAYER_OFF_CODES: ReadonlySet = new Set([ + "DataDisabled", + "DataStorageUnavailable", +]); + +/** + * Hands every held interest to the native store, in declaration order. + * + * Re-reads the map on every step, so a declaration made while one is in + * flight is applied too, and an entry is removed only if it was not replaced + * meanwhile. A refusal leaves the refused entry held: retrying `start()` + * without correcting it fails the same way, which is the point, since + * starting would offer the space whole. + */ +async function applyPendingInterest(): Promise { + // Resolves the code that says the data layer is off, or null. The engine + // validates the space name before it checks whether the layer is on, so a + // malformed name on a layer that is off is refused as an invalid argument; + // a cheap read answers the real question, so that refusal cannot hold the + // engine back when nothing would be replicated anyway. + const dataLayerOffCode = async (error: unknown): Promise => { + const code = (error as { code?: unknown } | null)?.code; + if (typeof code === "string" && DATA_LAYER_OFF_CODES.has(code)) { + return code; + } + try { + await OfflineProtocolNativeModule.dataListSpaces(); + return null; + } catch (probe) { + const probeCode = (probe as { code?: unknown } | null)?.code; + return typeof probeCode === "string" && DATA_LAYER_OFF_CODES.has(probeCode) + ? probeCode + : null; + } + }; + + for (;;) { + const next = pendingInterest.entries().next(); + if (next.done) { + return; + } + const [spaceId, patterns] = next.value; + try { + await OfflineProtocolNativeModule.dataSetInterest(spaceId, patterns); + } catch (error) { + const offCode = await dataLayerOffCode(error); + if (offCode !== null) { + console.warn( + `[OfflineProtocol] The data layer is unavailable (${offCode}), so the interest ` + + "declared before start() was dropped; nothing is replicated while it is off.", + error + ); + pendingInterest.clear(); + return; + } + const code = (error as { code?: unknown } | null)?.code; + const message = error instanceof Error ? error.message : String(error); + const refusal = new Error( + `setInterest for space "${spaceId}" was refused, so the protocol was not ` + + `started: starting would replicate the whole space. ${message}` + ) as Error & { code?: string; cause?: unknown }; + if (typeof code === "string") { + refusal.code = code; + } + refusal.cause = error; + throw refusal; + } + if (pendingInterest.get(spaceId) === patterns) { + pendingInterest.delete(spaceId); + } + } +} + export class OfflineProtocol { private eventEmitter: NativeEventEmitter; private eventSubscription: EmitterSubscription | null = null; @@ -1126,6 +1229,13 @@ export class OfflineProtocol { } } + // After storage opened and before the engine starts: the engine's + // start-up exchange builds its offers from the interest in force now, so + // a declaration made before start() has to land here or the first + // exchange asks for every document. See `pendingInterest`. + await applyPendingInterest(); + dataStoreReady = true; + await OfflineProtocolNativeModule.start(); // A session is starting, so nothing held from before it can still be @@ -3557,6 +3667,15 @@ export class OfflineProtocol { // Destroy native protocol instance if (this.isCreated) { + // The store goes with the instance, and interest is policy for one + // launch of one account: a declaration from before this point must not + // reach whatever is created next. Reset before the native call, so a + // declaration racing it is held rather than sent to a store being torn + // down. Only here, inside the branch: the state is module-wide, and an + // instance that never created an engine must not mark another + // instance's running store as not ready. + dataStoreReady = false; + pendingInterest.clear(); await OfflineProtocolNativeModule.destroy(); this.isCreated = false; } @@ -3978,7 +4097,8 @@ export type DataValue = * Requires `initializeMlsWithSecureStorage()` to have run — documents are * sealed at rest with the same per-install key as every other protocol * record, and that key is minted there — and `data.enabled` set in the - * config. Every method answers `DataDisabled` until it is. + * config. Every method answers `DataDisabled` until it is, except + * {@link setInterest}, which is held until `start()` can apply it. * * Edits change the open document in memory only. Nothing is stored or * replicated to peers until {@link flush} or {@link flushAll} runs (the SDK @@ -4079,11 +4199,35 @@ export class DataStore { * which is what makes a narrowing mean something against a peer on an * older build. * - * Not persisted: declare it at launch, before `start()`. Narrowing does - * not delete what is already held; widening asks the peers for what it - * adds. + * Declare it before {@link OfflineProtocol.start}. The native store does + * not exist until `start()` opens it, so a call made earlier is held and + * resolves at once, and `start()` applies it after storage opens and + * before the engine starts. That ordering matters: the engine's start-up + * exchange offers every held space with the interest in force at that + * moment, and narrowing afterwards does not delete what the wider + * exchange pulled in. A later call for the same space replaces a held one. + * + * Validation of a held declaration therefore happens in `start()`: a + * refused pattern rejects `start()` with an error naming the space, and + * the engine is not started, because starting would replicate the space + * whole. Correct it with another `setInterest` and call `start()` again. + * When the data layer is off (`DataDisabled`, `DataStorageUnavailable`) a + * held declaration is dropped with a warning instead, since nothing is + * replicated. + * + * After `start()` resolves, a call applies immediately, and widening asks + * the peers for what it adds. Narrowing never deletes what is already held. + * + * Not persisted: declare it at every launch, and again after + * {@link OfflineProtocol.destroy}, which discards anything held. */ async setInterest(spaceId: string, patterns: string[]): Promise { + if (!dataStoreReady) { + // A copy, so a caller mutating its array later cannot change what is + // applied, and so `applyPendingInterest` can tell a replacement apart. + pendingInterest.set(spaceId, [...patterns]); + return; + } await OfflineProtocolNativeModule.dataSetInterest(spaceId, patterns); } diff --git a/bindings/react-native/src/types.ts b/bindings/react-native/src/types.ts index 45fab5c6b..4fae8d07a 100644 --- a/bindings/react-native/src/types.ts +++ b/bindings/react-native/src/types.ts @@ -1533,7 +1533,15 @@ export interface FragmentAssemblyEvictedEvent extends BaseEvent { */ export interface DiagnosticEvent extends BaseEvent { type: 'diagnostic'; - level: 'info' | 'warning' | 'error'; + /** + * The four levels the iOS and Android managers emit. `debug` is the + * high-volume one: filter it rather than surface it. + * + * Pinned from the Rust side by `react_native_diagnostic_levels_match_every_bridge`, + * which reads every native emitter: a level emitted but not declared here + * arrives at runtime as a value this type says is impossible. + */ + level: 'debug' | 'info' | 'warning' | 'error'; message: string; context?: Record; } diff --git a/crates/offline-protocol-uniffi/src/lib.rs b/crates/offline-protocol-uniffi/src/lib.rs index 897ab5faf..37be25851 100644 --- a/crates/offline-protocol-uniffi/src/lib.rs +++ b/crates/offline-protocol-uniffi/src/lib.rs @@ -17687,6 +17687,231 @@ mod tests { } } + /// The diagnostic levels TypeScript declares are exactly the ones the + /// bridges emit. + /// + /// Every bridge passes the level as a plain string, so only the + /// TypeScript union types it, and nothing compiles the two together. + /// `debug` was emitted on both platforms for several releases while the + /// union said `'info' | 'warning' | 'error'` (#471), and Android's GATT + /// server emitted a fifth spelling, `warn`, that no declaration had. A + /// consumer's exhaustive switch reads either one as impossible. + /// + /// Both directions: an emitted level the union lacks, and a declared level + /// nothing emits. Every sink that takes a level literal is read: the + /// managers' `emitDiagnostic`, the Android BLE `diagnosticEmitter`, and + /// the peer-stream host callbacks. Python's managers are read too, since + /// they share the vocabulary even though their level never reaches + /// TypeScript. The floors on files and literals keep a scan that stopped + /// matching from passing on nothing. + #[test] + fn react_native_diagnostic_levels_match_every_bridge() { + use std::collections::BTreeSet; + + let repo = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + + let types_ts = std::fs::read_to_string(repo.join("bindings/react-native/src/types.ts")) + .expect("read types.ts"); + let interface = types_ts + .split_once("export interface DiagnosticEvent extends BaseEvent {") + .expect("types.ts must declare DiagnosticEvent") + .1 + .split_once("\n}") + .expect("unterminated DiagnosticEvent") + .0; + let level_line = interface + .lines() + .map(str::trim) + .find(|l| l.starts_with("level:")) + .expect("DiagnosticEvent must declare `level`"); + let declared: BTreeSet = level_line + .trim_start_matches("level:") + .trim_end_matches(';') + .split('|') + .map(|member| member.trim().trim_matches('\'').to_string()) + .collect(); + + // Comment lines dropped, then flattened, so a call the formatter + // wrapped after its parenthesis still reads as one call. + fn flatten(source: &str, python: bool) -> String { + source + .lines() + .map(str::trim) + .filter(|l| { + if python { + !l.starts_with('#') + } else { + !l.starts_with("//") && !l.starts_with('*') && !l.starts_with("/*") + } + }) + .collect::>() + .join(" ") + .split_whitespace() + .collect::>() + .join(" ") + } + + fn walk(dir: &std::path::Path, ext: &str, out: &mut Vec) { + let entries = std::fs::read_dir(dir) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", dir.display())); + for entry in entries { + let path = entry.expect("dir entry").path(); + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + if path.is_dir() { + // Build output, test doubles, generated bindings and the + // local API (which emits no diagnostics) are not emitters. + if ![".build", "tests", "Generated", "__pycache__", "local_api"].contains(&name) + { + walk(&path, ext, out); + } + } else if name.ends_with(ext) && name != "offline_protocol.py" { + out.push(path); + } + } + } + + const SINKS: [&str; 5] = [ + "emitDiagnostic(", + "diagnosticEmitter(", + "diagnostic(", + "peerStreamDiagnostic(", + "_emit_diagnostic(", + ]; + + let mut emitted: BTreeSet = BTreeSet::new(); + let mut sites: Vec<(String, String)> = Vec::new(); + let mut files_with_sites = 0usize; + for (root, ext, python) in [ + ("bindings/react-native/ios", ".swift", false), + ("bindings/react-native/android/src/main/java", ".kt", false), + ("bindings/python/offline_protocol_sdk", ".py", true), + ] { + let mut files = Vec::new(); + walk(&repo.join(root), ext, &mut files); + for file in files { + let code = flatten( + &std::fs::read_to_string(&file).expect("read source"), + python, + ); + let before = sites.len(); + for sink in SINKS { + for (at, _) in code.match_indices(sink) { + // `diagnostic(` also matches inside the longer sink + // names; only a call whose name starts at a word + // boundary counts, so each site is read once. + let preceding = code[..at].chars().next_back(); + if preceding.is_some_and(|c| c.is_ascii_alphanumeric() || c == '_') { + continue; + } + let rest = code[at + sink.len()..].trim_start(); + // Swift's `level:` label and Kotlin's `level =` named + // argument both precede the literal. + let rest = rest + .strip_prefix("level:") + .or_else(|| rest.strip_prefix("level =")) + .map_or(rest, str::trim_start); + let Some(literal) = rest.strip_prefix('"') else { + continue; // a forwarded variable, not a level + }; + let level: String = literal.chars().take_while(|c| *c != '"').collect(); + emitted.insert(level.clone()); + sites.push((file.display().to_string(), level)); + } + } + if sites.len() > before { + files_with_sites += 1; + } + } + } + + assert!( + files_with_sites >= 15 && sites.len() >= 500, + "the diagnostic scan found {} level literals in {files_with_sites} files, so it is \ + no longer reading the emitters and would pass against anything", + sites.len() + ); + + let undeclared: Vec<&(String, String)> = sites + .iter() + .filter(|(_, level)| !declared.contains(level)) + .collect(); + assert!( + undeclared.is_empty(), + "these diagnostics emit a level DiagnosticEvent.level in types.ts does not declare \ + ({declared:?}); declare it there and in the README, or use a declared one: \ + {undeclared:?}" + ); + let unused: Vec<&String> = declared.difference(&emitted).collect(); + assert!( + unused.is_empty(), + "DiagnosticEvent.level declares {unused:?}, which no bridge emits" + ); + } + + /// React Native applies an interest declared before `start()` after + /// storage opens and before the engine starts. + /// + /// The engine's start-up exchange offers every held space with the + /// interest in force at that instant, and narrowing later never deletes + /// what the wider offer pulled in. React Native folds MLS initialization + /// and the engine start into one `start()`, so the SDK holds a declaration + /// made earlier and applies it in between (#472). Moved after the native + /// start, everything still compiles and resolves, and the first exchange + /// asks for every document. `js-ci-harness/data-interest-order.test.js` + /// is the behavioural pin; this one reads the order as text so a reorder + /// fails the Rust suite too. + #[test] + fn react_native_start_applies_held_interest_before_the_engine_starts() { + let index = rn_source_code_only("src/index.ts"); + let start = index + .split_once("async start(): Promise {") + .expect("index.ts must define start()") + .1 + .split_once("async stop(): Promise {") + .expect("stop() must follow start() in index.ts") + .0; + let at = |needle: &str| { + start + .find(needle) + .unwrap_or_else(|| panic!("start() in index.ts must call `{needle}`")) + }; + let mls = at("OfflineProtocolNativeModule.initializeMlsWithSecureStorage()"); + let apply = at("await applyPendingInterest();"); + let ready = at("dataStoreReady = true;"); + let engine = at("await OfflineProtocolNativeModule.start();"); + assert!( + mls < apply && apply < ready && ready < engine, + "start() must initialize MLS, apply the held interest, mark the store ready, and \ + only then start the engine: the engine's start-up exchange offers each space with \ + the interest in force when it starts" + ); + + let set_interest = index + .split_once("async setInterest(spaceId: string, patterns: string[]): Promise {") + .expect("DataStore must define setInterest") + .1 + .split_once("async listDocs(") + .expect("listDocs must follow setInterest") + .0; + assert!( + set_interest.contains("if (!dataStoreReady) {") + && set_interest.contains("pendingInterest.set(spaceId, [...patterns]);"), + "setInterest must hold a declaration made before the store is ready; sent to \ + native then, it is refused (no store before create, no storage before MLS)" + ); + + let destroy = index + .split_once("async destroy(): Promise {") + .expect("index.ts must define destroy()") + .1; + assert!( + destroy.contains("dataStoreReady = false;") + && destroy.contains("pendingInterest.clear();"), + "destroy() must discard held interest and hold the next declaration again; a \ + surviving flag sends it to a store that no longer exists" + ); + } + /// Frames a bridge synthesizes for the core carry the configured app id. /// /// Every relay answer, relay group frame and legacy plain-text DM is diff --git a/crates/offline-protocol/src/protocol/prefixes.rs b/crates/offline-protocol/src/protocol/prefixes.rs index 02722a742..1d1ee5bd9 100644 --- a/crates/offline-protocol/src/protocol/prefixes.rs +++ b/crates/offline-protocol/src/protocol/prefixes.rs @@ -211,13 +211,14 @@ pub(crate) const DATA_PLANE_PREFIXES: &[&str] = /// `internet_group_report_received` already handles the group delivery report; /// that is deliberately out of scope here and left as the follow-up. /// -/// **Maintenance note:** this list is mirrored, by hand, in three places that +/// **Maintenance note:** this list is mirrored, by hand, in four places that /// no single compiler ever sees together — here, `RelayAnswerPrefixes.swift`, -/// and `RelayAnswerPrefixes.kt`. Each bridge pins its own copy against the same -/// literals ([`relay_answer_prefixes_are_pinned`] does it for this one), because +/// `RelayAnswerPrefixes.kt` and the Python `relay_answer_prefixes.py`. Each +/// bridge pins its own copy against the same literals +/// ([`relay_answer_prefixes_are_pinned`] does it for this one), because /// a prefix present in one list and absent from another fails **silently**: the /// bridge injects the answer unattributed, this list declines to exempt it, and -/// the frame is dropped as unsigned with no peer at fault. Edit all three. +/// the frame is dropped as unsigned with no peer at fault. Edit all four. pub(crate) const RELAY_ANSWER_PREFIXES: &[&str] = &[ internal_prefixes::GROUP_CREATED, internal_prefixes::GROUP_MEMBER_ADDED, @@ -234,11 +235,11 @@ mod tests { /// The membership of [`RELAY_ANSWER_PREFIXES`], pinned to literals. /// /// Written out rather than derived, for the same reason the bridges write - /// theirs out: this list is one of three hand-maintained copies, and a test + /// theirs out: this list is one of four hand-maintained copies, and a test /// that recomputed it from the constant would agree with any edit — which /// is precisely the failure mode. The literals here are the contract the - /// two bridge lists are also pinned against, so a divergence in any one of - /// the three now fails a test in its own language. + /// three bridge lists are also pinned against, so a divergence in any one + /// of the four now fails a test in its own language. /// /// Dropping an entry is the dangerous direction and the reason this test /// exists: the bridge would keep injecting that answer unattributed, the @@ -258,9 +259,9 @@ mod tests { "__USER_GROUPS__", "__GROUP_ERROR__", ], - "the relay-answer exemption list changed — update RelayAnswerPrefixes.swift \ - and RelayAnswerPrefixes.kt to match, or the bridges and the gate will \ - disagree silently" + "the relay-answer exemption list changed: update RelayAnswerPrefixes.swift, \ + RelayAnswerPrefixes.kt and relay_answer_prefixes.py to match, or the \ + bridges and the gate will disagree silently" ); } diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 7c1505d53..d5856732f 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -2249,7 +2249,11 @@ is a document name, optionally ending in `*` to match a prefix; `["*"]` is everything and is the default, `[]` is nothing, and a space takes at most 32. **Set it before `start()`.** It is not persisted, on purpose: it is your -policy for this launch, not a fact about the store. `wipeAll()` clears it +policy for this launch, not a fact about the store. On React Native the +store only opens inside `start()`, and from 0.28.0 the SDK holds a call made +before it and applies it there, before the engine starts; on 0.27.0 the same +call rejected with `DataStore not initialized`, so an app that worked around +that by calling it after `start()` can move the call back. `wipeAll()` clears it along with the documents it scoped, so re-declare it if you wipe while the engine is running. diff --git a/docs/adr/0004-control-plane-signature-gate.md b/docs/adr/0004-control-plane-signature-gate.md index ab8ff5757..7a238a744 100644 --- a/docs/adr/0004-control-plane-signature-gate.md +++ b/docs/adr/0004-control-plane-signature-gate.md @@ -73,8 +73,9 @@ the group delivery report already works. See ## Maintenance hazard -The relay-answer list exists in three places no single compiler sees together: -the core and each native bridge. A prefix present in one copy and absent from +The relay-answer list exists in four places no single compiler sees together: +the core, the Swift and Kotlin bridges, and the Python binding (which had no +copy until #368). A prefix present in one copy and absent from another fails **silently**: the bridge injects the answer unattributed, the gate declines to exempt it, and the frame is dropped as unsigned with no peer at fault. diff --git a/docs/api-reference.md b/docs/api-reference.md index 75a2329b1..271cc4cb7 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1057,7 +1057,7 @@ await store.flush('space-1', 'profile'); | `deleteDoc(space, doc)` | `void` | Drops this device's copy. Records nothing about the name, so a replica that still holds the document refills it | | `removeDoc(space, doc)` | `void` | Removes the document from every replica of the space. An edit made concurrently with the removal wins and brings the document back whole. Reads and writes on the name throw `InvalidState` until `createDoc` | | `removeSpace(space)` | `void` | Removes every document the space holds, from every replica | -| `setInterest(space, patterns)` | `void` | The documents this device wants from its peers. Names or `name*` prefixes, at most 32. `["*"]` is the default, `[]` is nothing. Not persisted: set it before `start()` | +| `setInterest(space, patterns)` | `void` | The documents this device wants from its peers. Names or `name*` prefixes, at most 32. `["*"]` is the default, `[]` is nothing. Not persisted: set it before `start()`, which React Native holds and applies inside `start()` before the engine starts ([details](data.md#replicating-part-of-a-space)) | | `listDocs(space)` | `string[]` | | | `listSpaces()` | `string[]` | Spaces this device holds documents or removal records for | | `mapSet(space, doc, collection, key, value)` | `void` | | diff --git a/docs/bridges/README.md b/docs/bridges/README.md index 327a22270..21612f719 100644 --- a/docs/bridges/README.md +++ b/docs/bridges/README.md @@ -122,7 +122,10 @@ Fourteen sets do today, and they are pinned by **two different** mechanisms, so knowing which one you are touching matters. **The relay-answer prefix exemption list** is the canonical example: the core, -the Swift bridge, and the Kotlin bridge each hold a copy. A prefix present in one +the Swift bridge, the Kotlin bridge and the Python binding +(`relay_answer_prefixes.py`) each hold a copy. Python had none until #368, and +its injection had drifted to prefix names the registry never held, which is +this failure exactly. A prefix present in one copy and absent from another **fails silently**: the bridge injects the answer unattributed, the core's gate declines to exempt it, and the frame is dropped as unsigned with no peer at fault. The visible symptom is a relay feature quietly @@ -315,6 +318,7 @@ alone. | Reticulum available reported **only after** the gateway binds the session | An unbound session may submit and be told a verdict, and is never a recipient, so offering it to the selector offers a transport that can only refuse | | Relay capabilities cleared **on** internet drop | Otherwise a stale capability keeps the broadcast gate open | | Per-peer end-to-end capabilities restored **before** queued sends flush | Otherwise the startup flush emits downgraded envelopes to every established peer | +| Interest applied **after** MLS initialization and **before** the engine starts | The start-up exchange offers every held space with the interest in force then, and a narrowing never deletes what a wider offer pulled in. React Native's `start()` does both steps, so it holds a declaration made before it and applies it in between | | `close_file_stores()` **after** `stop()`, and nothing after it | A running engine writes to its stores, so the call is refused until the protocol is stopped; afterwards the engine holds closed stores, so `start()`, `enable_telemetry()` and both `initialize_mls` entry points are refused | ## C8. The identifier the bridge reports must match the namespace it is asked for diff --git a/docs/bridges/kotlin.md b/docs/bridges/kotlin.md index 36c3de697..75823feae 100644 --- a/docs/bridges/kotlin.md +++ b/docs/bridges/kotlin.md @@ -99,7 +99,7 @@ subscription, not on what it means. See ## K7. Pinned constant lists -`RelayAnswerPrefixes.kt` holds one of the three copies of the relay-answer +`RelayAnswerPrefixes.kt` holds one of the four copies of the relay-answer exemption list, pinned in `RelayAnswerPrefixesTest.kt`. See [C5](README.md#c5-hand-mirrored-constants-must-be-pinned-in-every-language). diff --git a/docs/bridges/python.md b/docs/bridges/python.md index 30b1ff787..cfaad3014 100644 --- a/docs/bridges/python.md +++ b/docs/bridges/python.md @@ -96,6 +96,14 @@ removals, so on a running engine with live sessions it is undone by the peer's next version offer, which recreates and refills every document. `DataStore.remove_space()` is what removes documents from the other replicas. +Interest (`DataStore.set_interest`) belongs after `initialize_mls` and before +`start()`, because the engine's start-up exchange offers each space with the +interest in force when it starts. `ProtocolManager.start()` does both, and the +local API server opens its store after it, so a store reached through either +runs its first exchange under the default interest and narrows from the next +one. An application that needs the first exchange narrowed drives +`OfflineProtocol` itself in the engine's order. + ## P7. The internet send loop is adaptive here, and fixed elsewhere The core's internet outbox is poll-only across the FFI in every binding. The @@ -339,6 +347,23 @@ pins the attach order, the correlation, the flags and what a dead connection owes. No daemon has been run against it; a conforming daemon is a deployment this repository does not ship. +## P12. A relay answer reaches the core unattributed, and the rule lives in one place + +The relay's group answers (`__GROUP_CREATED__`, the membership pair, +`__GROUP_INFO__`, `__USER_GROUPS__`, `__GROUP_ERROR__`) are frames this +binding synthesizes, so nothing can sign them. The core exempts them from the +control-frame signature gate only when they arrive on the Internet transport +with no transport peer identity, so an answer injected with a `sender_id` is +dropped as unsigned, and the caller sees a successful inject. + +`relay_answer_prefixes.py` holds the list, and `_inject_group_frame` passes +every actor through its `attributable_actor`, as the Swift and Kotlin bridges +do. A new arm cannot attribute an answer by mistake. The list is one of the +four hand-mirrored copies under [C5](README.md#c5-hand-mirrored-constants-must-be-pinned-in-every-language), +and `test_relay_answer_prefixes.py` pins it as literals. Before #368 there was +no Python copy, and the injection had drifted to `__GRP_*` names the registry +never held. + ## Testing ```bash diff --git a/docs/bridges/swift.md b/docs/bridges/swift.md index 4c614041f..a46ae16a0 100644 --- a/docs/bridges/swift.md +++ b/docs/bridges/swift.md @@ -198,7 +198,7 @@ languages, Python included. See ## S7. Pinned constant lists -`RelayAnswerPrefixes.swift` holds one of the three copies of the relay-answer +`RelayAnswerPrefixes.swift` holds one of the four copies of the relay-answer exemption list. It is pinned against literals in `RelayAnswerPrefixesTests.swift`. See [C5](README.md#c5-hand-mirrored-constants-must-be-pinned-in-every-language). diff --git a/docs/bridges/typescript.md b/docs/bridges/typescript.md index b3c1cd63d..abde9a6a9 100644 --- a/docs/bridges/typescript.md +++ b/docs/bridges/typescript.md @@ -35,6 +35,14 @@ field set inside it. The tests that pin event field shapes assert against Rust literals and never read `types.ts`. So a renamed or removed event **field** passes every guard in the repository and arrives mistyped at runtime. +One field is the exception: `DiagnosticEvent.level`. The bridges emit +diagnostics as JSON they build themselves, with the level as a plain string at +several hundred call sites, so the union is the only place the +vocabulary is typed. `react_native_diagnostic_levels_match_every_bridge` reads +the union and every level literal handed to a diagnostic sink in the Swift, +Kotlin and Python sources, and fails in both directions. It exists because +`debug` was emitted for several releases while the union omitted it (#471). + Adding an event field means updating `types.ts` in the same change, and nothing will remind you. When `types.ts` lags, nothing fails loudly: the event simply arrives untyped. diff --git a/docs/data.md b/docs/data.md index bbf928191..c828d870e 100644 --- a/docs/data.md +++ b/docs/data.md @@ -294,12 +294,24 @@ It does two things, and they are worth telling apart. whoever sends it. That half depends on nothing, so a narrowed space stays narrow even against a peer that ignores the request. -**Set it before `start()`.** It is not persisted, on purpose: it is your -application's policy for this launch rather than a fact about the store, and -a durable copy would be a second thing to reconcile against an app that has -changed its mind. Declaring it costs nothing there: the exchange `start()` -makes already carries it, and declaring the same patterns twice sends -nothing. +**Set it before `start()`.** The engine's start-up exchange offers every +space with the interest in force at that moment, and narrowing afterwards +does not delete what that wider exchange already pulled in. On React Native +the store does not exist until `start()` opens it, so the SDK holds a call +made earlier, resolves it at once, and applies it after storage opens and +before the engine starts. A pattern it refuses then rejects `start()` with an +error naming the space, and the engine is not started. On the Swift package, +the Android library and Python, call it after `initializeMls` and before +`start()`, the order the engine documents; Python's `ProtocolManager` +initializes MLS inside its own `start()`, so a store opened through it runs +its first exchange under the default interest. + +It is not persisted, on purpose: it is your application's policy for this +launch rather than a fact about the store, and a durable copy would be a +second thing to reconcile against an app that has changed its mind. Declare +it at every launch, and again after `destroy()`, which discards a held one. +Declaring it costs nothing: the exchange `start()` makes already carries it, +and declaring the same patterns twice sends nothing. **`wipeAll()` clears it too**, along with the documents it was scoping, because nothing may distinguish a wiped space from one this device has never diff --git a/docs/react-native-integration.md b/docs/react-native-integration.md index 1b5bf4c12..c1b4302e2 100644 --- a/docs/react-native-integration.md +++ b/docs/react-native-integration.md @@ -585,7 +585,7 @@ await store.flush('space-1', 'profile'); | **deleteDoc** | `deleteDoc(spaceId, docId): Promise` | Drops this device's copy. Records nothing about the name, so a replica that still holds the document refills it. | | **removeDoc** | `removeDoc(spaceId, docId): Promise` | Removes the document from every replica of the space. An edit made concurrently with the removal wins and brings the document back whole. | | **removeSpace** | `removeSpace(spaceId): Promise` | Removes every document the space holds, from every replica. | -| **setInterest** | `setInterest(spaceId, patterns): Promise` | The documents this device wants from its peers: names or `name*` prefixes, at most 32. Not persisted, so set it before `start()`. | +| **setInterest** | `setInterest(spaceId, patterns): Promise` | The documents this device wants from its peers: names or `name*` prefixes, at most 32. Call it before `start()` on every launch: the SDK holds it and applies it inside `start()`, before the engine starts, so the first exchange carries it. A refused pattern rejects `start()`. Not persisted, and discarded by `destroy()`. | | **listDocs** | `listDocs(spaceId): Promise` | Documents in a space. | | **listSpaces** | `listSpaces(): Promise` | Spaces this device holds documents or removal records for. | | **mapSet** | `mapSet(spaceId, docId, collection, key, value: DataValue): Promise` | Sets a key in a map collection. | diff --git a/docs/spec/control-messages.md b/docs/spec/control-messages.md index 40cca800b..12dc2ab71 100644 --- a/docs/spec/control-messages.md +++ b/docs/spec/control-messages.md @@ -412,8 +412,9 @@ assert disjointness in a test. ### Hand-mirrored lists -The relay-answer exemption list exists in three places that no single compiler -sees together: the protocol core and each native bridge. A prefix present in one +The relay-answer exemption list exists in four places that no single compiler +sees together: the protocol core, the Swift and Kotlin bridges, and the Python +binding. A prefix present in one copy and absent from another fails **silently**: the bridge injects the answer unattributed, the gate declines to exempt it, and the frame is dropped as unsigned with no peer at fault.