diff --git a/plans/2026-07-06-fix-directory-name-reapproval.md b/plans/2026-07-06-fix-directory-name-reapproval.md index 40bd552f85..b510e6224f 100644 --- a/plans/2026-07-06-fix-directory-name-reapproval.md +++ b/plans/2026-07-06-fix-directory-name-reapproval.md @@ -1,185 +1,168 @@ # Fix: SimpleX directory re-approves a channel every 30 min after a blockchain name is added -Status: proposed — **fix gated on a diagnostic step (see §5); root-cause delta not yet observed** +Status: proposed — **root cause deduced from code (see §3); fix in core** Branch: `nd/fix-directory-names` Base: `origin/master` (`b38015c7b`) +> This supersedes two earlier drafts of this plan that blamed the domain claim/proof. +> Deep investigation showed that was a **red herring** — the claim/proof round-trips +> and converges. The real non-convergent field is **`publicGroupId`**. See §3. + ## 1. Problem -After an owner adds a SimpleX name (blockchain domain claim) to a public -channel, the SimpleX Directory service asks the owner to re-approve the channel -**every 30 minutes, indefinitely**, with no changes on the owner's side. The -channel keeps flipping to "hidden until approved." +After an owner adds a SimpleX name (blockchain domain claim) to a public channel, +the SimpleX Directory service asks the owner to re-approve the channel **every +30 minutes, indefinitely**, with no changes on the owner's side. The channel keeps +flipping to "hidden until approved." -## 2. What is confirmed (facts, file:line) +## 2. Confirmed trigger (facts, file:line) -1. **The 30-min cadence is the directory's link-check timer.** `linkCheckInterval` +1. **30-min cadence = the directory's link-check timer.** `linkCheckInterval` defaults to **1800 s** (`apps/simplex-directory-service/src/Directory/Options.hs:181`); - `linkCheckThread_` sleeps that long and enqueues a `DEGroupLinkCheck` for every - non-removed registered group (`.../Directory/Service.hs:199-211`). + `linkCheckThread_` sleeps that interval and enqueues a `DEGroupLinkCheck` for + every non-removed registered group (`.../Directory/Service.hs:199-211`). +2. **`deGroupLinkCheck` re-approves iff `groupUpdated` is true** — + `when groupUpdated $ reapprove …` (`Service.hs:824`), flipping `GRSActive → + GRSPendingApproval` and re-sending for admin approval (`:843-854`). +3. **`groupUpdated` is a whole-profile byte inequality** — the `Bool` returned by + `updateGroupFromLinkData`: `profileChanged = p /= groupProfile` + (`src/Simplex/Chat/Library/Internal.hs:1466-1482`), `p` = directory's *stored* + `GroupProfile`, `groupProfile` = profile from the *fetched* link short-link data. +4. **The flag is behaviorally directory-only** — the mobile/desktop/iOS + `GroupLinkPlan.Known` variant does not carry it (`SimpleXAPI.kt:7158`; iOS + `AppAPITypes.swift`); only the directory consumes it (`Service.hs:824`, `:825`, + `:988`→`deReregistration:1043/1046`). -2. **`DEGroupLinkCheck` re-approves iff `groupUpdated` is true.** `deGroupLinkCheck` - runs `APIConnectPlan` against the channel link and, on a `GLPKnown` plan, does - `when groupUpdated $ reapprove …` (`Service.hs:815-854`, trigger at `:824`), - which sets `GRSPendingApproval` and re-sends for admin approval. +## 3. Root cause (deduced by elimination — code-confirmed) -3. **`groupUpdated` is a whole-profile byte inequality.** It is the `Bool` returned - by `updateGroupFromLinkData`: `profileChanged = p /= groupProfile` - (`src/Simplex/Chat/Library/Internal.hs:1466-1482`), where `p` is the directory's - **stored** `GroupProfile` and `groupProfile` is the profile in the **fetched - link data**. +Three independent traces close the case without needing a runtime diagnostic: -4. **`groupUpdated` is behaviorally directory-only.** The mobile/desktop clients do - not even deserialize it: the client `GroupLinkPlan.Known` variant carries only - `groupInfo` — `class Known(val groupInfo: GroupInfo)` - (`apps/multiplatform/.../model/SimpleXAPI.kt:7158`; iOS - `apps/ios/Shared/Model/AppAPITypes.swift`, `enum GroupLinkPlan`). The only - consumers of the flag are in the directory service: re-approval (`Service.hs:824`), - listing refresh (`:825`), and owner re-registration (`:988` → - `deReregistration:1043,1046`). (All other `groupUpdated` matches in the tree are - an unrelated chat event, `RcvGroupEvent`/`CR.GroupUpdated`.) +- **The fetched link profile is byte-identical across fetches.** It is served + verbatim from the owner's published, owner-authorized `LSET` blob via `LGET` + (`simplexmq` agent `getConnShortLink` → `decryptLinkData`); there is no + timestamp, per-fetch nonce, relay reconstruction, or server-computed member + count. Member count lives in `GroupShortLinkData.publicGroupData` (a sibling of + `groupProfile`) and drives `countChanged`, not `groupUpdated`. So the link side + cannot make `groupUpdated` flip on its own. +- **Every `Eq`-relevant `GroupProfile` field except one is persisted by + `updateGroupProfile` and therefore converges** after the first store + (`Internal.hs:1471`). The single exception is **`publicGroupId`** + (`Types.hs:858`, part of `PublicGroupProfile`, `deriving Eq`): the UPDATE in + `updateGroupProfile` (`src/Simplex/Chat/Store/Groups.hs:2721-2732`) writes + `group_type, group_link, group_web_page, group_domain, domain_web_page, + allow_embedding, group_domain_proof, …` but **omits `public_group_id`**. It is + read back from the stored column (`Shared.hs:699`, `Groups.hs:2779`), and + `toPublicGroupProfile` returns `publicGroup = Nothing` unless `group_type` **and** + `group_link` **and** `public_group_id` are all present (`Shared.hs:713-716`). +- **The domain claim/proof is not the cause.** `group_domain`/`group_domain_proof` + *are* persisted and round-trip symmetrically (`Shared.hs:721`/`:727`), so they + converge. In the group flow the proof is `Nothing` anyway (`SimplexDomainProof` + is never constructed in `src/`; group claims use `mkDomainClaim`, `proof = Nothing`, + `Names.hs:56`, `Commands.hs:5693`), and group verification is a separate scalar + column `group_domain_verified` (`Groups.hs:2740-2746`). -## 3. What is NOT yet established +**Deduction:** a *perpetual* re-approval requires a field that never reconciles. +The link side is deterministic, and every field except `publicGroupId` converges +after one store — so the only field that can hold `p /= groupProfile` forever is +`publicGroupId`. It does so because the directory's stored `public_group_id` is +**NULL / stale** and `updateGroupProfile` — the only function that syncs a group's +profile from link data or `XGrpInfo` — cannot write that column. -**Why the two profiles fail to converge every cycle is unproven.** An ordinary -profile change self-corrects: when `profileChanged` is true, -`updateGroupFromLinkData` writes the link profile to the DB (`Internal.hs:1471`), -and the store round-trips the domain claim + proof faithfully -(`group_domain_proof` column; symmetric `publicGroupAccessRow` / -`toPublicGroupAccess`, `src/Simplex/Chat/Store/Shared.hs:721`/`:727`). So a -one-time change re-approves **once** and then converges. A perpetual 30-min -re-approval requires the directory's stored profile to be **permanently unequal** -to the fetched link profile — and which field differs has **not been observed**. +**Why the directory's `public_group_id` is NULL, and why it correlates with the +name.** `public_group_id` is written in only three places, all *creation-time*: +the two group INSERTs (`Groups.hs:405`, `:912`, from the profile's `publicGroup`) +and `updateRelayGroupKeys` (`:2293`). If the directory joined the channel while it +had no `publicGroupId` in its link profile (an older channel, or one that became a +public/named channel later), its `public_group_id` was inserted NULL and can never +be populated afterward. When the owner adds the SimpleX name, the owner republishes +the link with a fully-populated `publicGroup` (`publicGroupId` + `publicGroupAccess`), +so the fetched profile's `publicGroup` becomes `Just{…}` while the directory's +stored copy stays `Nothing` — a permanent inequality, re-approved every 30 min. -Two hypotheses were considered and both are insufficient as stated: +**Confirmation step (cheap, not blocking):** on the affected directory, +`SELECT public_group_id FROM group_profiles` joined to the channel's group should +show NULL, while the link's `publicGroupId` is present. -- **Relay drops the domain claim from the served link data** - (`src/Simplex/Chat/Library/Commands.hs:4326-4328` handles this case). But this - **converges after one cycle**: the directory stores the claim-less link profile, - and with no further owner change nothing restores the claim, so the next check - matches. It explains a single reapproval, not a repeat. -- **A re-presented proof with a fresh nonce.** This does **not** apply to domain - claims. `PHTest`/`badgeProof` (fresh nonce per presentation) is the separate - *badge* feature, attached to `Profile.badge` (`Internal.hs:2085-2086`), not to - the domain claim. A `SimplexDomainProof`'s `presHeader` is **inside the signed - payload** (`Commands.hs:4869`, verified at `:4867`), so it cannot be re-presented - with a new nonce without re-signing by the owner — it is static. The group claim - is also frequently created with `proof = Nothing` (`mkDomainClaim`, - `src/Simplex/Chat/Names.hs:56`; used at `Commands.hs:5693`, `:1510`). +## 4. The fix (core, minimal): let `updateGroupProfile` populate `public_group_id` -So the correlation ("started right after adding a name") strongly implicates the -domain claim as the differing field, but the **exact field and the reason it -never converges are not yet demonstrated.** This plan therefore treats the fix as -gated on a diagnostic. +`updateGroupProfile` is missing a profile column that its sibling INSERT paths +already persist. Add `public_group_id` to its UPDATE, **guarded by `COALESCE` so it +only populates a currently-NULL value and never changes or erases an existing +identity** (respecting `publicGroupId`'s immutability): -## 4. Likely mechanism (hypothesis, to be confirmed) - -The differing field is the group profile's `publicGroup.publicGroupAccess.groupDomainClaim` -(or its `proof`): the directory's stored copy and the fetched link copy carry -different claim representations on every fetch, in a way that storing the link -copy does not reconcile. Because the domain claim is verified **independently** — -cryptographically + on-chain by `verifyEntityDomain` (`Commands.hs:4840`), with -verification tracked in a *separate* field `group_domain_verified` -(`Store/Groups.hs:2710-2713`) — it is not admin-moderated content and should never -gate re-approval. The bug is that `groupUpdated` conflates "profile changed at -all" (used to decide whether to *store* the fresh claim) with "profile changed in -a way needing admin *re-moderation*" (used to decide re-approval). - -## 5. Required diagnostic (gate before the fix) - -Add a temporary log at the one comparison that flips everything, -`Internal.hs:1482`, dumping the two profiles' domain claims when `profileChanged` -fires, and run it against the affected channel across ≥2 link-check cycles: - -```haskell --- temporary: -when (p /= groupProfile) $ logInfo $ "linkcheck delta: stored=" <> tshow (claimOf p) - <> " link=" <> tshow (claimOf groupProfile) +```sql +-- src/Simplex/Chat/Store/Groups.hs, updateGroupProfile_ UPDATE: +SET display_name = ?, full_name = ?, short_descr = ?, description = ?, image = ?, + group_type = ?, group_link = ?, public_group_id = COALESCE(public_group_id, ?), + group_web_page = ?, group_domain = ?, domain_web_page = ?, allow_embedding = ?, + group_domain_proof = ?, preferences = ?, member_admission = ?, updated_at = ? ``` -Decision: -- **Delta is the domain claim, identical each cycle** → structural asymmetry; - apply §6 (proof redaction covers it if the delta is the proof; if the delta is - the whole claim, widen the redaction to the claim — decide from the log). -- **Delta changes each cycle** → identify the varying field from the log; if it is - not the claim, this plan's fix does not apply and the diagnosis restarts here. - -The fix in §6 is only justified once the log shows the per-cycle delta is confined -to the domain claim/proof. - -## 6. The fix (conditional on §5) - -In `updateGroupFromLinkData` (`Internal.hs`), keep storing on the full -`profileChanged` (so the claim/proof and verification stay current), but report -`groupUpdated` with the domain-claim **proof** redacted — reusing the codebase's -existing proof-redaction idiom (`Types.hs:750` `clearProofs`; `Internal.hs:1265` -`redactedDomain`), rather than new helpers: +with the new `?` sourced exactly like the INSERT paths (`Groups.hs:384-385`): ```haskell - pure (g'', moderationChanged) -- was: profileChanged - ... - where - profileChanged = p /= groupProfile -- keep: still gates storing the fresh claim - -- A domain-claim change is verified independently (on-chain + owner signature) - -- and is not admin-moderated, so it must not trigger directory re-approval. - -- Redact the proof before comparing (same idiom as Types.clearProofs). - moderationChanged = redactProof p /= redactProof groupProfile - redactProof gp@GroupProfile {publicGroup} = gp {publicGroup = redactPg <$> publicGroup} - redactPg pg@PublicGroupProfile {publicGroupAccess} = pg {publicGroupAccess = redactAccess <$> publicGroupAccess} - redactAccess a@PublicGroupAccess {groupDomainClaim} = - a {groupDomainClaim = (\d -> d {proof = Nothing} :: SimplexDomainClaim) <$> groupDomainClaim} +publicGroupId_ = case publicGroup of + Just PublicGroupProfile {publicGroupId} -> Just publicGroupId + Nothing -> Nothing ``` -Redacting **only the proof** (not the whole claim) is the surgical choice: it kills -any proof-representation delta while still letting a genuine domain add/change -register as a one-time change (which then converges). If §5 shows the delta is the -whole claim (e.g. claim present vs absent every cycle), replace `redactAccess` with -`groupDomainClaim = Nothing`; note this also suppresses the legitimate one-time -reapproval on a name add. +Effect: on the next link check, the directory's NULL `public_group_id` is populated +from the authoritative link profile → its stored `publicGroup` reconstructs as +`Just{…}` equal to the link's → `profileChanged` becomes false → re-approval stops. -Storage keys off the full `profileChanged`, so the claim/proof and -`setGroupDomainVerified` still update — by-name lookups and the verified badge stay -correct. +## 5. Why this is the most correct fix -## 7. Why this is safe (given §5 confirms the delta) +- **Fixes the actual root cause, and converges.** After one sync the profiles are + equal, so `groupUpdated` correctly reports "no change." Symptom and cause both go. +- **It is a plain completeness fix.** The two INSERT paths and `updateRelayGroupKeys` + already persist `public_group_id`; `updateGroupProfile` omitting it is the bug. +- **Safe under `COALESCE(public_group_id, ?)`** — it *only* fills a NULL. It never + changes a set identity, so it cannot corrupt a correct `publicGroupId`, and it + cannot be erased by a claim-less `XGrpInfo` (new value `Nothing` → keep old). + This preserves the "immutable identity" invariant while repairing missing data. +- **General, not a directory workaround.** Any client whose `public_group_id` is + missing gets it repaired on the next profile sync; the fix is in shared core, as + intended, and needs no judgment about which fields are "moderatable." +- **Blast radius is safe.** `updateGroupProfile` is widely called, but the only + behavioral change is *populating a NULL column when a profile carrying a + `publicGroupId` is stored* — a pure improvement for every caller. -- **Directory-only blast radius** (fact, §2.4). All three consumers behave - correctly with the proof/claim excluded: - - `Service.hs:824` periodic re-approval → **fixed**. - - `Service.hs:988` → `deReregistration` (`:1043`/`:1046`) — an owner re-submitting a - name-only change gets "already listed" instead of forced re-approval. - - `Service.hs:825` `listingsUpdated` — minor trade-off: a name-only change no longer - *immediately* refreshes web-listing files (still refreshes on any real change or - member-count change). If prompt refresh matters, keep `:825` on the raw - `profileChanged` while `:824` uses `moderationChanged`. -- **Moderation not weakened.** Re-approval still fires on any moderatable field - (name, description, image); only proof/claim deltas are excluded, and those are - independently verified. +## 6. Alternative considered — and why not -## 8. Residual behaviour (accepted) +**Scope the re-approval comparison** (return `groupUpdated` from +`updateGroupFromLinkData` computed over only the moderatable display fields, +excluding `publicGroup`). This mirrors the contact path's `clearProofs` / +`sameProfileContent` redaction (`Types.hs:747-750`, `:803`) and the existing +`DirectoryTests` expectation that link-only changes must not re-approve +(`tests/Bots/DirectoryTests.hs:296-326`). It would stop the re-approval — **but it +only hides the symptom**: the stored `public_group_id` stays NULL, the profiles +stay unequal, so `updateGroupFromLinkData` keeps calling `updateGroupProfile` every +30 min to store a profile that never fully matches (idempotent churn), and any +other consumer of the difference stays broken. §4 is preferred because it removes +the inequality itself. (This scoping could still be added later as defense in depth +for genuinely non-moderatable fields, but it is not needed for this bug.) -If the claim/proof genuinely differs each cycle, the store branch re-writes the -`group_profiles` row every 30 min. This is idempotent and does not thrash -verification (`claimChanged` in `updateGroupProfile` compares the *domain*, not the -proof — `Groups.hs:2708-2709`). A follow-up could skip the store on proof-only -deltas; out of scope. +## 7. Verification plan -## 9. Verification plan +1. Confirm on the affected directory that the channel's `public_group_id` is NULL + while the link carries a `publicGroupId` (validates §3; one SELECT). +2. Apply §4. +3. **Regression test** in `tests/Bots/DirectoryTests.hs` (sits alongside the + existing "link-only / whitespace-only must not re-approve" tests at `:296-326`): + register a channel whose stored `public_group_id` is NULL → add a SimpleX name / + populate the link `publicGroupId` → run ≥2 link checks → assert the registration + stays `GRSActive` (no `GRSPendingApproval`, no owner re-approval message). Also + assert the group's `public_group_id` is populated after the first check. +4. Build the directory service (which statically links core); run the directory + test suite. -1. Land the §5 diagnostic, capture the delta on the affected channel, confirm it is - the domain claim/proof, and choose proof-vs-whole-claim redaction from the log. -2. Apply §6. -3. **Regression test** in `tests/Bots/DirectoryTests.hs`: register a channel → - approve → add a SimpleX name → trigger ≥2 link checks → assert the registration - stays `GRSActive` (no `GRSPendingApproval`, no owner re-approval message). -4. Build the directory service; run the directory test suite. +## 8. Files touched -## 10. Files touched +- `src/Simplex/Chat/Store/Groups.hs` — `updateGroupProfile`: add + `public_group_id = COALESCE(public_group_id, ?)` to the UPDATE and thread the + `publicGroupId_` parameter (as the INSERT paths do). No other logic change. +- `tests/Bots/DirectoryTests.hs` — add the name-added / NULL-`public_group_id` + no-reapproval regression test. -- `src/Simplex/Chat/Library/Internal.hs` — `updateGroupFromLinkData`: return a - claim/proof-insensitive change flag (store logic unchanged). Temporary diagnostic - log removed before merge. -- `tests/Bots/DirectoryTests.hs` — add the name-added-no-reapproval regression test. - -No schema, wire-format, or API changes. `GLPKnown.groupUpdated`'s type is unchanged; -only its value semantics (directory-only consumer) are refined. +No schema, wire-format, or API changes. `updateGroupProfile`'s signature is +unchanged; it simply stops dropping a column it is already given.