feat(relay): add atomic community ownership transfer#1845
Merged
Conversation
Add POST /operator/communities/transfer operator endpoint that atomically transfers community ownership to a new pubkey. The previous owner is demoted to member (not admin), per product decision. - transfer_ownership() in relay_members.rs: single atomic txn that upserts new owner and demotes all other owners to member - TransferResult enum: Transferred/AlreadyOwner/NoOwner edge cases - Db::transfer_ownership() wrapper in lib.rs - TransferCommunityRequest/Response structs in community_provisioning.rs - transfer_community() handler in operator.rs with NIP-98 operator auth, UUID/hex validation, and best-effort NIP-43 snapshot publication - Route wired in router.rs - 5 DB-level tests + 3 HTTP-level tests (all #[ignore] requires Postgres) Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
High 1 — Stale owner race: transfer_ownership() now accepts expected_owner_pubkey. The current owner row is locked FOR UPDATE and verified against the expected value inside the same transaction. Returns OwnerConflict (HTTP 409) when the current owner no longer matches — a delayed/retried request can no longer overwrite a completed transfer. High 2 — Racy 3-community limit: the per-recipient ownership count is now enforced at the relay layer inside the transfer transaction, protected by a transaction-scoped advisory lock on the transferee pubkey. The same lock key is used by create_community_with_owner, so transfer-vs-create races to the same recipient are serialized. The kgoose preflight check remains as a fast fail for user experience but is no longer the authoritative enforcement. High 3 — NIP-43 snapshot staleness: added replace_addressable_event_forced() which unconditionally soft-deletes prior events for the same (kind, pubkey, channel_id) without created_at/event-id ordering. The relay is the authoritative source for membership snapshots — the latest snapshot always wins, regardless of same-second random event ID races. The publish_nip43_membership_list path now uses the forced variant. Medium 1 — Missing audit trail: BuzzCommunityService.transfer() now records audit entries for both success and failure, matching the create path. Medium 2 — npub validation: decodeNpub() now requires exactly 32 decoded bytes. A valid-bech32 npub with wrong payload length is correctly rejected as invalid input instead of misclassified as 'transferee not registered'. Tests: 2 new DB tests (OwnerConflict, LimitReached), 1 new create limit test. All existing tests updated for the new expected_owner_pubkey parameter. cargo check + cargo test --lib pass (non-ignored tests). Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
High (NIP-43 snapshot race): The previous fix acquired the advisory lock only inside replace_addressable_event_forced, after the membership query and event construction had already completed outside the lock. A concurrent publication could read stale members, then overwrite a newer snapshot by arrival order. Fix: Added publish_nip43_membership_locked() which acquires the per-community snapshot lock, reads members, builds the event, and replaces the prior snapshot — all inside one transaction on one database connection. The lock serializes the entire read-build-write cycle. publish_nip43_membership_list now delegates to this method instead of querying members separately. Medium (bootstrap_owner): Added explicit deployment-root authority exception doc comment. bootstrap_owner is called only by startup and legacy operator provisioning, not end-user paths. The per-owner limit is documented as an end-user invariant enforced by create_community_with_owner and transfer_ownership; deployment-root operations may exceed it by design. Validation: cargo check clean, cargo test --lib -p buzz-db 79 passed, cargo test --lib -p buzz-relay 563 passed + 14 ignored. Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Remove the unused forced replacement path, consolidate advisory lock calculation and test fixtures, and distinguish owner quota failures during provisioning. Co-authored-by: npub122y0pqkertljmedu303rl0aqrj3w8pvu43t6jxm6875lzg6f2pwqegc3xc <5288f082d91aff2de5bc8be23fbfa01ca2e3859cac57a91b7a3fa9f12349505c@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub122y0pqkertljmedu303rl0aqrj3w8pvu43t6jxm6875lzg6f2pwqegc3xc <5288f082d91aff2de5bc8be23fbfa01ca2e3859cac57a91b7a3fa9f12349505c@sprout-oss.stage.blox.sqprod.co>
wesbillman
approved these changes
Jul 14, 2026
wesbillman
left a comment
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
Approve. Independent review focused on the concurrency design, traced end-to-end at the source level rather than diff-only.
What holds up:
transfer_ownershiptxn ordering is sound: advisory lock on the transferee →FOR UPDATEon the community's owner rows →expected_owner_pubkeycheck → limit count → upsert/demote, all in one transaction. Sharingowner_count_advisory_lock_keywithcreate_community_with_ownermeans transfer-vs-create races to the same recipient serialize instead of both passing the 3-community limit check.- The
FOR UPDATE+ expected-owner guard correctly rejects stale/retried transfer requests after a concurrent transfer has changed hands (OwnerConflict, non-retryable by design). publish_nip43_membership_lockedis a genuine race fix, not just a refactor: the previous path read members outside the replacement lock, so a stale read could win by arrival order. Holding the per-community lock across the whole read-build-write cycle closes that.- No deadlock cycles across the lock pairs; FNV keyspace collisions can only cause extra serialization, as documented.
- All five
TransferResultarms map to sensible HTTP statuses; NIP-98 operator auth runs before body parsing; snapshot publication stays best-effort, matching the provision path.
One non-blocking operational gotcha (runbook line or startup guard as a follow-up):
bootstrap_owner at startup re-promotes RELAY_OWNER_PUBKEY and demotes other owners to admin. So transferring ownership of the deployment community reverts on the next relay restart unless the env var is updated — and the transferee lands as admin, not member, diverging from this PR's demotion semantics. Worth documenting so an operator isn't surprised.
Base is 6 commits behind main, but none touch crates/ — no rebase needed. CI fully green including relay e2e.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
memberand reject stale requests usingexpected_owner_pubkeyValidation
flutteris unavailable locally