Skip to content

Retire the 0032 roster trigger in migration 0081 #8696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
loganj wants to merge 1 commit into
base: main
Choose a base branch
from
+108 −571
Open

Retire the 0032 roster trigger in migration 0081 #8696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 16 additions & 26 deletions crates/buzz-db/src/runtime/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,14 +777,27 @@ mod postgres_tests {
let mut migrations: Vec<_> = MIGRATOR.iter().collect();
migrations.sort_by_key(|migration| migration.version);

assert_eq!(migrations.len(), 80);
assert_eq!(migrations.len(), 81);
for (index, migration) in migrations.iter().enumerate() {
assert_eq!(migration.version, (index + 1) as i64);
}
assert!(migrations[79]
.sql
.as_str()
.contains("ADD COLUMN auto_join BOOLEAN NOT NULL DEFAULT false"));
assert_eq!(migrations[80].version, 81);
let retire_roster_fence = strip_sql_comments(migrations[80].sql.as_str());
for statement in [
"SET LOCAL lock_timeout = '1s';",
"SET LOCAL statement_timeout = '5s';",
"DROP TRIGGER trg_events_guard_channel_roster_snapshot ON events;",
"DROP FUNCTION guard_channel_roster_snapshot();",
] {
assert!(
retire_roster_fence.contains(statement),
"0081 must run {statement}"
);
}
assert!(migrations[70]
.sql
.as_str()
Expand Down Expand Up @@ -1336,31 +1349,8 @@ mod postgres_tests {
assert!(roster_fence.contains("snapshot_members IS DISTINCT FROM canonical_members"));
assert!(roster_fence.contains("ERRCODE = '23514'"));

// Fresh desired-state bootstrap must install the identical executable
// fence as migration 0032. CI and isolated relay startup use schema.sql
// without running migrations, so drift reopens rolling-deploy races.
// The desired state keeps the function and its trigger in separate
// files, so compare each statement rather than one contiguous span.
fn extract_span<'a>(sql: &'a str, start: &str, end: &str) -> &'a str {
let begin = sql.find(start).expect("span start");
let relative_end = sql[begin..].find(end).expect("span end");
&sql[begin..begin + relative_end + end.len()]
}
for (start, end) in [
(
"CREATE OR REPLACE FUNCTION guard_channel_roster_snapshot()",
"$$ LANGUAGE plpgsql;",
),
(
"CREATE TRIGGER trg_events_guard_channel_roster_snapshot",
" FOR EACH ROW EXECUTE FUNCTION guard_channel_roster_snapshot();",
),
] {
assert_eq!(
extract_span(roster_fence, start, end),
extract_span(&desired_schema, start, end)
);
}
// Migration 0081 retires this fence; the desired state never installs it.
assert!(!desired_schema.contains("guard_channel_roster_snapshot"));

// The single-row heartbeat table is updated continuously. Prevent
// autovacuum from truncating its heap so standby queries are not
Expand Down
363 changes: 71 additions & 292 deletions crates/buzz-db/src/store/channel_members.rs
View file
Open in desktop

Large diffs are not rendered by default.

167 changes: 0 additions & 167 deletions crates/buzz-db/src/store/replaceable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,173 +784,6 @@ mod postgres_tests {
assert_eq!(new_rows, 0, "new roster must roll back with its index");
}

#[tokio::test]
#[ignore = "requires Postgres"]
async fn stale_legacy_roster_cannot_replace_new_locked_snapshot() {
use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp};

let (setup_pool, scratch) =
ScratchDb::migrated(&admin_url().await, "mixed_roster_writer").await;
let pool = PgPoolOptions::new()
.max_connections(1)
.acquire_timeout(Duration::from_secs(1))
.connect(scratch.url())
.await
.expect("connect one-connection scratch pool");
setup_pool.close().await;
let db = Db::from_pool(pool.clone());
let community_uuid = Uuid::new_v4();
let community = CommunityId::from_uuid(community_uuid);
let channel = Uuid::new_v4();
let relay_keys = Keys::generate();
let owner_keys = Keys::generate();
let owner = owner_keys.public_key().to_bytes();
seed_community_channel(&pool, community_uuid, channel, &owner_keys).await;

// This is the old pod's unlocked capture A. It remains in process memory
// while a role-only canonical mutation advances and the new pod publishes B.
let base = Timestamp::now().as_secs();
let roster = |members: &[(&[u8], &str)], timestamp| {
let tags =
std::iter::once(Tag::parse(["d", channel.to_string().as_str()]).expect("d tag"))
.chain(members.iter().map(|(member, role)| {
Tag::parse(["p", hex::encode(member).as_str(), "", *role]).expect("p tag")
}))
.collect::<Vec<_>>();
EventBuilder::new(Kind::Custom(39002), "")
.tags(tags)
.custom_created_at(Timestamp::from(timestamp))
.sign_with_keys(&relay_keys)
.expect("sign roster")
};

let newcomer = Keys::generate().public_key().to_bytes();
sqlx::query(
"INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) \
VALUES ($1, $2, $3, 'member', $4)",
)
.bind(community_uuid)
.bind(channel)
.bind(newcomer.as_slice())
.bind(owner.as_slice())
.execute(&pool)
.await
.expect("seed member before legacy capture");
let stale_a = roster(
&[(owner.as_slice(), "owner"), (newcomer.as_slice(), "member")],
base + 2,
);

sqlx::query(
"UPDATE channel_members SET role = 'admin' \
WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3",
)
.bind(community_uuid)
.bind(channel)
.bind(newcomer.as_slice())
.execute(&pool)
.await
.expect("commit newer canonical role");

let relay_pubkey = relay_keys.public_key().to_bytes();
let mut snapshot = db
.lock_member_snapshot(community, channel, &relay_pubkey)
.await
.expect("new writer captures locked roster B");
let fresh_b = roster(
&[(owner.as_slice(), "owner"), (newcomer.as_slice(), "admin")],
base + 1,
);
assert!(
snapshot
.replace_member_event(&fresh_b)
.await
.expect("new writer publishes B")
.1
);
snapshot
.release()
.await
.expect("commit B and release locks");

// Reproduce the old binary on one connection: replacement key ->
// old-head UPDATE -> raw INSERT. Do not use the app API, whose guard
// rejects before the retained trigger can exercise this legacy fence.
// A size-one pool and bounded attempt also catch self-deadlock.
let error = tokio::time::timeout(Duration::from_secs(3), async {
let mut tx = pool.begin().await.expect("begin legacy replacement");
let relay_pubkey = relay_keys.public_key().to_bytes();
sqlx::query("SELECT pg_advisory_xact_lock($1)")
.bind(crate::store::replaceable::event_replacement_lock_key(
community,
39002,
&relay_pubkey,
Some(channel.as_bytes().as_slice()),
))
.execute(&mut *tx)
.await
.expect("legacy replacement key");
let deleted = sqlx::query(
"UPDATE events SET deleted_at = NOW() \
WHERE community_id=$1 AND kind=39002 AND pubkey=$2 \
AND channel_id=$3 AND deleted_at IS NULL",
)
.bind(community_uuid)
.bind(relay_pubkey.as_slice())
.bind(channel)
.execute(&mut *tx)
.await
.expect("legacy soft-delete of current head");
assert_eq!(deleted.rows_affected(), 1, "legacy path must delete the live head");
let result = sqlx::query(
"INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig, received_at, channel_id, d_tag) \
VALUES ($1, $2, $3, $4, 39002, $5, $6, $7, NOW(), $8, $9)",
)
.bind(community_uuid)
.bind(stale_a.id.as_bytes().as_slice())
.bind(relay_pubkey.as_slice())
.bind(chrono::DateTime::from_timestamp(stale_a.created_at.as_secs() as i64, 0).expect("timestamp"))
.bind(serde_json::to_value(&stale_a.tags).expect("tags"))
.bind(&stale_a.content)
.bind(stale_a.sig.serialize().as_slice())
.bind(channel)
.bind(channel.to_string())
.execute(&mut *tx)
.await;
tx.rollback().await.expect("restore live head after legacy attempt");
result
})
.await
.expect("legacy replacement must not deadlock")
.expect_err("stale captured roster A must be rejected");
assert!(
matches!(error, sqlx::Error::Database(ref db_error)
if db_error.code().as_deref() == Some("23514")
&& db_error.message() == "kind 39002 roster does not match canonical channel membership"),
"expected trigger roster fence check violation, got {error:?}"
);

let live_ids: Vec<Vec<u8>> = sqlx::query_scalar(
"SELECT id FROM events WHERE community_id=$1 AND channel_id=$2 \
AND kind=39002 AND pubkey=$3 AND deleted_at IS NULL",
)
.bind(community_uuid)
.bind(channel)
.bind(relay_pubkey.as_slice())
.fetch_all(&pool)
.await
.expect("load live roster heads");
assert_eq!(live_ids, vec![fresh_b.id.as_bytes().to_vec()]);
let stale_rows: i64 =
sqlx::query_scalar("SELECT count(*) FROM events WHERE community_id=$1 AND id=$2")
.bind(community_uuid)
.bind(stale_a.id.as_bytes().as_slice())
.fetch_one(&pool)
.await
.expect("count rejected stale roster");
assert_eq!(stale_rows, 0, "stale roster insert must roll back");
}

#[tokio::test]
#[ignore = "requires Postgres"]
async fn nip_rs_replacement_hard_deletes_payload_and_watermark_rejects_replay() {
Expand Down
2 changes: 1 addition & 1 deletion crates/buzz-relay/src/handlers/side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,7 @@ async fn store_group_members_event(
// relay's moderation-DM key participates in the {relay, recipient} DM used
// for moderation notices) MUST retain the relay's own `p` tag. nostr's
// default `build_with_ctx` strips any `p` tag matching the signer, which
// would drop the relay from the snapshot and fail migration 0032's roster
// would drop the relay from the snapshot and fail the roster
// fence against the canonical two-member DM. `allow_self_tagging` keeps the
// snapshot faithful to `channel_members`.
let event = EventBuilder::new(Kind::Custom(KIND_NIP29_GROUP_MEMBERS as u16), "")
Expand Down
20 changes: 20 additions & 0 deletions migrations/0081_retire_channel_roster_trigger.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-- Every kind:39002 roster writer checks the roster against canonical channel
-- membership in the application (validate_channel_roster_snapshot, from
-- #8316), and since #8611 no relay requires this trigger at startup. Retire
-- the trigger that repeats that check: it compares roster labels with the
-- stored role exactly, so it would refuse rosters that show the computed
-- `member`/`bot` label. Every relay and rollback target must run #8611 or
-- later before this migration runs; older relays refuse to start without the
-- trigger. Published migrations remain checksum-frozen.
--
-- DROP TRIGGER takes ACCESS EXCLUSIVE on each partition of events in turn,
-- and keeps each lock while it waits for the next. While the locks are queued
-- or held, event reads and writes both block.
-- lock_timeout bounds each lock wait and statement_timeout bounds each whole
-- statement, as in 0049. Either timeout rolls the migration back with no
-- change, and it can be retried.
SET LOCAL lock_timeout = '1s';
SET LOCAL statement_timeout = '5s';

DROP TRIGGER trg_events_guard_channel_roster_snapshot ON events;
DROP FUNCTION guard_channel_roster_snapshot();
72 changes: 0 additions & 72 deletions schema/functions/guard_channel_roster_snapshot.sql
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion schema/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto;

-- functions
\i functions/channels_community_id_immutable.sql
\i functions/guard_channel_roster_snapshot.sql
\i functions/events_created_at_floor_guard.sql
\i functions/prevent_community_deletion_request_retargeting.sql
\i functions/prevent_community_deletion_approval_removal.sql
Expand Down
4 changes: 0 additions & 4 deletions schema/triggers/events.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
CREATE TRIGGER trg_events_guard_channel_roster_snapshot
BEFORE INSERT ON events
FOR EACH ROW EXECUTE FUNCTION guard_channel_roster_snapshot();

-- INSERT OR UPDATE OF: an UPDATE can move a previously exempt row into the
-- guarded set (channel_id NULL -> NOT NULL) or move a channel row's
-- created_at below the fence, so both mutation paths re-run the guard on the
Expand Down
Loading
Loading