Add XIP-81: App Data Updates#136
Conversation
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This XIP specification document is owned by @jhaaaa, not the PR author. Additionally, there are two unresolved high-severity review comments identifying potential specification inconsistencies that should be addressed by the designated owner. You can customize Macroscope's approvability policy. Learn more. |
* Align XIP-81 with shipped libxmtp implementation * xip-81: pre-commit permission evaluation + correctness pass - Add explicit pre-commit evaluation rule (Section 2.4) and a matching security-considerations subsection. Permissions for every AppDataUpdate resolve against the pre-commit registry / admin lists; a commit cannot grant a permission and exercise it in the same commit. Bootstrap is the only exception and routes through a dedicated validator. - Update Test Cases to use shipped ComponentIds (GroupName at 0x8004, SuperAdminList at 0x8001, ConversationType at 0xBFFF, reserved 0xFF50) and add tests for the pre-commit rule, unknown-component tolerance, steady-state floor bumps, and Step A pause behavior. Move BatchProposal and KeyValueDelta tests to a "deferred" list paired with their feature. - Rationale: replace "Why range-based defaults with an override map" with "Why a per-component registry". Note BatchProposal deferral in the atomic-batching rationale. - Security Considerations: drop the "ComponentId range model" framing, reference the registry directly, note pre-commit evaluation, drop the BatchProposal sub-operation paragraph. - Backward Compatibility: BatchProposal subsection now notes deferral. - Section 1.4 table: drop the BatchProposal exception (deferred); reword AppDataUpdate row from "proposed" to "shipped". - Section 3.3 reference fix: "steps 2a-2d" -> "Step B". * xip-81: second review pass - §2.2.1: "register-then-write" was wrongly said to fit in one commit; pre-commit evaluation forces split commits (or bootstrap path). - §2.2.1: clarify that SUPER_ADMIN_LIST permission is enforced in code, not from the registry — registry just covers it for type dispatch. - Rationale "Why one-time migration": drop the "only happens when all members support" claim; the shipped version-bump path pauses non-supporting peers instead of blocking the migration on them. - Rationale "Application ComponentId registration alternatives": on-chain registry is deferred; v1 ships per-group explicit registration with duplicate-id rejection at register time. - Test 8: spell out the two-proposer scenario so the pre-commit evaluation property is testable rather than ambiguous. * xip-81: note pre-d14n migrations are shipped + sketch 32-byte component handle - Move pre-d14n migration support out of "Optional future work" and into the existing two-step bootstrap line — that mechanism is exactly what makes pre-d14n migrations safe (lower-version peers pause on Step A). - §2.2.2 + Open Questions §5: add a brief note on a 32-byte stable identifier per application-range component, resolved by the SDK to the per-group 16-bit ComponentId. Lets multiple apps ship components without coordinating on the 16-bit namespace.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
|
||
| **Wire shape: standalone proposal + commit.** Migrated sender intents publish two messages per metadata update: a standalone `AppDataUpdate` proposal first, then a commit that consumes the proposal store. This is the same MLS proposal-by-reference shape used elsewhere — it is *not* the inline-bundled GCE-proposal-plus-commit pattern from the legacy path. | ||
|
|
||
| **Lazy batching.** The commit issued by `stage_app_data_propose_and_commit` has no precondition on the pending proposal store. It sweeps every existing pending proposal — concurrent `AppDataUpdate`s, leaf-node `Update`s, `Add`/`Remove`/`SelfRemove`, PSK, etc. — into the same commit body. Producers of those pending proposals already accepted "ride into the next commit" by leaving them pending; MLS freezes message-send while proposals are pending regardless of which commit ultimately consumes them. The dictionary-update step in receivers iterates only the `AppDataUpdate` proposals; non-AppData proposals ride natively through OpenMLS. |
There was a problem hiding this comment.
🟠 High XIPs/xip-81-app-data-updates.md:388
The lazy batching rule in stage_app_data_propose_and_commit can merge a pending ComponentRegistry permission change with a dependent AppDataUpdate in the same commit. Section 2.4 requires permission changes and dependent writes to land in separate commits because authorization is evaluated against the pre-commit registry state. When these proposals are batched together, the dependent AppDataUpdate is evaluated against the old registry and rejected. Consider filtering the pending proposal store to exclude registry/admin-list mutations before sweeping remaining proposals into the commit, or document that callers must ensure no permission-changing proposals are pending.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @XIPs/xip-81-app-data-updates.md around line 388:
The `lazy batching` rule in `stage_app_data_propose_and_commit` can merge a pending `ComponentRegistry` permission change with a dependent `AppDataUpdate` in the same commit. Section 2.4 requires permission changes and dependent writes to land in separate commits because authorization is evaluated against the pre-commit registry state. When these proposals are batched together, the dependent `AppDataUpdate` is evaluated against the old registry and rejected. Consider filtering the pending proposal store to exclude registry/admin-list mutations before sweeping remaining proposals into the commit, or document that callers must ensure no permission-changing proposals are pending.
|
|
||
| ### 1.2 Capability Negotiation | ||
|
|
||
| Before enabling proposals on a group, the initiator MUST verify that all current members support the proposal extension: |
There was a problem hiding this comment.
🟠 High XIPs/xip-81-app-data-updates.md:49
Section 1.2 only checks for PROPOSAL_SUPPORT_EXTENSION_ID capability before enabling proposals, but the bootstrap commit in Section 3.2 Step B adds AppDataDictionary and uses AppDataUpdate proposals. A peer can advertise standalone-proposal support without advertising AppDataDictionary in its MLS capabilities, so the check passes for members who cannot actually process the migration. The result is a group that may execute Step A (version-floor bump), pause older clients, then fail Step B because some members lack the required MLS extension capability. Consider also requiring AppDataDictionary in the capability check before allowing migration.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @XIPs/xip-81-app-data-updates.md around line 49:
`Section 1.2` only checks for `PROPOSAL_SUPPORT_EXTENSION_ID` capability before enabling proposals, but the bootstrap commit in `Section 3.2 Step B` adds `AppDataDictionary` and uses `AppDataUpdate` proposals. A peer can advertise standalone-proposal support without advertising `AppDataDictionary` in its MLS capabilities, so the check passes for members who cannot actually process the migration. The result is a group that may execute Step A (version-floor bump), pause older clients, then fail Step B because some members lack the required MLS extension capability. Consider also requiring `AppDataDictionary` in the capability check before allowing migration.
2 participants
Original AppData/ComponentId design — replaces GroupContextExtensions with
AppDataUpdateproposals for delta-based group state.This is the pre-XIP draft as written. A follow-up PR (stacked on this branch) shows where the shipped implementation diverged.
Note
Add XIP-81 specification for App Data Updates
Adds xip-81-app-data-updates.md, a new protocol specification proposing to replace
GroupContextExtensionswithAppDataUpdateproposals for managing mutable group state.AppDataDictionarywith per-component permissions andComponentIdrangesBatchProposaltypeChanges since #136 opened
0x8000as single source of truth for per-component policies [b0e8475]📊 Macroscope summarized b0e8475. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.