[TheSentinel454]

What

This is phase 1 of 2 for retiring the events_created_at_floor deferred constraint trigger and its events_created_at_floor_guard() function (migration 0021). The invariant now holds in the app writers. The trigger, function and buzz.created_at_floor GUC arming all stay as the backstop while pods run mixed versions. A later PR drops them.

One helper, replica_fence::guard_event_created_at_floor(&mut AdmittedTx, created_at, channel_id), runs on the admitted write connection. If created_at < clock_timestamp() - CREATED_AT_FLOOR_SECS, it rejects the write with the typed, non-retryable DbError::CreatedAtFloorRejected. It uses the same 960 s constant as the GUC and the fence wall. Channel-less rows skip it and cost no query.

Why checking at insert time is enough: the fence wall is min(oldest_xact_start, S) - floor - 5s. A writer transaction that is open during the probe's activity scan is bounded by its own xact_start. A transaction that starts after the scan samples a clock later than S. So any clock_timestamp() sample taken inside the writing transaction is as strong as the trigger's commit-time sample. This needs no commit hook, no candidate-row tracking and no ordering against the TTL refresh, and AdmittedTx::commit is unchanged.

Site	Check placement
store/event.rs insert_event_on	after the ON CONFLICT DO NOTHING INSERT, only when a row was inserted, before follow-ups
store/event.rs insert_event_with_thread_metadata_tx	after the INSERT, only when a row was inserted, before follow-ups and thread metadata
store/replaceable.rs parameterized replacement	after the ON CONFLICT DO NOTHING INSERT, only when a row was inserted, inside the supersede savepoint (a rejection rolls back to it and restores the old head)
store/replaceable.rs addressable replacement	after the ON CONFLICT DO NOTHING INSERT, only when a row was inserted, before follow-ups (the caller's rollback restores the old head)
store/channel_members.rs LockedMemberSnapshot::replace_member_event	before the old head is superseded

Writer inventory: there are 7 production INSERT INTO events functions. The other two are structurally channel_id = NULL: push.rs accept_lease_event (kind 30350) and relay_members.rs publish_nip43_membership_locked (kind 13534). Both write a literal NULL for channel_id in their INSERT SQL, and the policy exempts them only while that NULL sits at the channel_id position. No production UPDATE writes events.created_at or events.channel_id, and the events ON CONFLICT clauses are DO NOTHING.

Client-visible outcome is unchanged. The trigger's SQLSTATE 23514 surfaced as a generic DB error, which the relay maps to IngestError::Internal (WS OK false / HTTP 500). The new variant follows the same path. Relay mappings are untouched.

scripts/seed-admin-dashboard.sh writes a channel event with created_at = 1790000000 through raw psql, and it accepted any PGHOST. It now refuses non-local hosts (localhost, 127.0.0.1, ::1 and Unix socket paths are still allowed), a non-empty PGHOSTADDR or PGSERVICE, a multi-host PGHOST, and any PGDATABASE that is not a plain name (psql treats URI and key=value -d values as connection strings that override -h). Both psql paths pass -X, so a psqlrc can't \connect elsewhere after the guard.

Tests

There are eleven new PG tests in store::event_floor_postgres_tests. Each runs in its own per-test database with ALTER TABLE events DISABLE TRIGGER events_created_at_floor, and a pg_partition_tree check asserts that every partition clone is disabled, so the trigger can't hide a missing app check.

• insert_event_floor_rejects_old_and_accepts_fresh, thread_metadata_event_floor_rejects_old_and_accepts_fresh, parameterized_event_floor_rejects_old_and_accepts_fresh, addressable_event_floor_rejects_old_and_accepts_fresh, member_snapshot_event_floor_rejects_old_and_accepts_fresh: each rejects a too-old event with the typed error, confirms no row was written, then accepts a fresh event.
• insert_event_floor_accepts_expired_duplicate, thread_metadata_event_floor_accepts_expired_duplicate: replaying an already-stored event whose created_at is now below the floor returns Duplicate (inserted = false), leaves one row and runs no thread follow-ups.
• parameterized_replay_of_soft_deleted_aged_event_is_a_duplicate, addressable_replay_of_soft_deleted_aged_event_is_a_duplicate: replaying a replacement whose stored row is soft-deleted and has aged below the floor returns a duplicate, not a floor rejection, as the trigger did (it never fired on ON CONFLICT).
• cluster_global_event_floor_held_transaction_cannot_write_below_probe_wall: runs a real heartbeat probe while a writer transaction is open. It asserts that the wall is bounded by that writer's xact_start, that a below-wall event is rejected, and that a near-floor event sampled after the probe lands above the wall.
• event_floor_samples_statement_clock_after_transaction_start: waits on the database until a boundary timestamp that was valid at BEGIN expires (normally ≤1.1 s), so now() would pass where clock_timestamp() fails.

Source policy (observability_source.rs): every channel-bearing production events INSERT in any crate's src must call the helper, no production UPDATE may assign created_at or channel_id (including ONLY, schema-qualified, aliased and row-constructor forms; quoted identifiers are normalized), the helper must use clock_timestamp() and the shared constant, and the seed script must refuse a remote host. Negative fixtures cover each rule.

Each check was confirmed to fail when its fix is removed. All of these mutants failed on assertions:

• removing the helper from all five writers, which failed all seven PG tests
• switching clock_timestamp() to now(), which failed both the clock PG test and the clock policy
• removing one call
• adding a created_at UPDATE
• disabling the policy self-test
• removing the seed guard
• (review round 2) restoring the pre-INSERT check placement, which failed both duplicate-replay tests; the round-1 UPDATE matcher, quoted-identifier normalization, buzz-db-only scan, NIP-43 bind exemption, process-wide canvas base and each seed override were each restored and failed their tests
• (review round 3) restoring the round-2 seed script, which failed the extended seed test on its URI and key/value PGDATABASE cases; restoring the script without -X, which failed the control's -X assertion

Fixture changes: about 40 existing buzz-db PG tests seeded historical rows (fixed old timestamps) through the serving writers. The trigger never fired on them because the test pool is unarmed. They now seed through a #[cfg(test)] raw-SQL helper, test_support::insert_historical_event (and insert_historical_event_with_thread_metadata for thread fixtures), which keeps admission, follow-ups and mentions. The canvas CAS fixtures keep their logical seconds and ties but each test passes its own fresh base. The existing trigger backstop tests still exercise raw armed INSERT/UPDATE plus COMMIT returning 23514.

The new PG tests are selected by the postgres-ci filter (test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/). The cluster_global_ test also joins the existing serial group.

Behavior notes

• Prepared transactions (2PC) are covered only by the retained trigger, which runs at PREPARE. The insert-time app guard runs earlier, and no app writer issues PREPARE TRANSACTION. The comment in replica_fence.rs now says so, so the trigger drop has to keep this case covered.

• One extra round trip for each newly inserted channel event. Duplicate replays add none.

• Ordinary and thread event writes check the floor only for newly inserted rows, before follow-ups, mentions or thread metadata. Replaying an already-stored id stays Duplicate even when its timestamp has aged below the floor, as with the trigger. (Internal writers such as the admin outbox worker do replay relay-signed events with their original created_at.) A too-old new row returns the non-retryable CreatedAtFloorRejected, and the owning transaction rolls back; every current caller propagates the error and drops the transaction. The transaction-composition seam documents that contract.

• Replacement writers check after their INSERT, only when it inserted a row, so a replay of any stored id (live or soft-deleted) stays a duplicate. The locked roster writer checks before the old head is superseded.

• The trigger is stricter for a row that ages below the floor between the app check and COMMIT. The proof above doesn't need that strictness, and phase 2 gives it up.

What got simpler

The NIP-43 insert drops a bind for a literal NULL, the floor and follow-up policies share one workspace walk, and the canvas fixtures lose their process-wide OnceLock. No commit-time machinery was added: no candidate tracking, commit hook, cleanup protocol or new lock. Test-only historical writes now go through one helper instead of being scattered through the serving APIs. Nothing could be deleted in this phase because mixed-version pods still rely on the trigger. Phase 2 drops the trigger and function, the GUC arming, verify_floor_guard_catalog / verify_floor_guard_behavior and the schema reconciliation entries. Its deploy gate: the whole fleet on this binary, no rollback expected, and the replica breaker held closed through the drop.

Identified debt (pre-existing, outside this diff): three buzz-relay handlers::group_state_postgres_tests::* tests fail with ensure community: Sqlx(PoolTimedOut) on the Blox host. They fail the same way on a clean checkout of the base (state.rs test pool, 100 ms acquire timeout). writer_pool_rejects_non_read_committed_database_default and writer_pool_metrics_stop_after_session_timeout_setup_failure hang on that host at base too.

Verification

Head b046df76d (after the placement fix and main's newer test fixtures): on Blox, fmt, clippy and buzz-db unit (241/241) pass, and a filtered buzz-db PG lane over the floor tests and every converted fixture passes 88/88. CI is running at this head.

Earlier full gate, at tree 67c4fe5e1 + this diff, before the placement fix:

Gate	Result
cargo fmt --all -- --check	pass
cargo clippy --workspace --all-targets -- -D warnings	pass
cargo nextest run -p buzz-db --lib	167/167
cargo nextest run -p buzz-db --test observability_source	22/22
scripts/test-postgres-test-discovery.sh	pass
scripts/postgres-test-run.sh -p buzz-db (excluding the two host hangs)	517/517
scripts/postgres-test-run.sh -p buzz-admin / audit / deletion / push-gateway / search / workflow	all rc 0 (4, 7, 11, 11, 20, 2)
scripts/postgres-test-run.sh -p buzz-relay	402 pass, 3 fail (the pre-existing PoolTimedOut failures above)

Local gates used a reverted rustls lock bump, because the build mirror lacks 0.23.46. The committed Cargo.lock is unchanged, and CI uses the real lock.

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: e2dc54e22fc113f298889d76c3acf674cb46db4b...b046df76d361e65575a7ba1a8c5f30e6d0403b55
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability regressions found in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Review used read-only source and history inspection. Builds, tests, and repository scripts were not executed, as requested.

---

_{Generated by Codex Security Review |
Requested by: @TheSentinel454 |
Workflow run}

[wpfleger96]

🤖 Blocking at 91b507a66. This is a comment, not a request for changes. The issue below needs a fix before merge.

The core change looks right. For the fence wall, checking created_at against clock_timestamp() on the admitted transaction at insert time is enough. A transaction that's open when the probe scans activity keeps its original xact_start, including across a savepoint rollback, so a row it accepted early is still bounded even if it commits late. A transaction that starts after the scan reads the clock after S. All five channel writers call the guard on the admitted transaction. The ordinary and thread inserts check only when a row was actually inserted and before follow-ups. The three replacements check after the replay/dominated exits and before touching the old head. Every production caller propagates the error before commit, including both roster-snapshot publishers, command persistence, artifact/reaction/audio composition, and the Db wrappers. buzz-admin doesn't pick up any new rejections either: it uses the armed writer pool, its channel events are signed at the current time, and the one custom-timestamp event (kind 13534) has no channel.

Blocking: a connection string in PGDATABASE bypasses the new local-only seed guard (scripts/seed-admin-dashboard.sh:23-43)

The guard checks PGHOST, PGHOSTADDR, PGSERVICE and multi-host values, but run_psql passes PGDATABASE straight to psql -d (both the local and Docker branches). psql treats a -d value containing = or starting with postgresql:// as a full connection string, and its docs say those parameters override conflicting command-line options. So PGHOST=localhost PGDATABASE="host=shared-db.example.invalid dbname=buzz" passes the guard and connects to the remote host, and a URI does the same. If that database has a matching local-community row, the raw unarmed fixture INSERT INTO events at line 326 runs against it, which is exactly what this guard is meant to prevent.

Fix: accept only a plain database name before either psql call, and reject both URI and key/value forms. Then extend admin_seed_refuses_remote_database_before_invoking_psql (observability_source.rs:2194) with a URI case and a key/value case, including embedded hostaddr= and service=.

Optional:

• The source policy in observability_source.rs catches SQL literals inside function bodies, but it won't see SQL built from pieces. With sqlx::query(concat!("UPDATE ", "events SET created_at=...")), normalized_sql_source (line 1631) keeps the comma between the fragments, so there's no update events to match. A split INSERT behaves the same way. A module-level SQL constant before the first function, or SQL loaded with include_str!, falls outside function_slices (line 959). The minimum-five count wouldn't notice an extra writer that's invisible to the scan. The trigger still backs this up in phase 1, so this isn't blocking. Before phase 2 drops the trigger, I'd add negative fixtures for these forms and either resolve them or ban them at event-write seams.
• The prepared-transaction comment in replica_fence.rs:638-642 says prepared transactions are bounded by pg_prepared_xacts.prepared because "their deferred floor guard already ran at PREPARE". With the trigger still in place that's true. The app guard, though, runs at insert time, which is earlier than PREPARE, so after phase 2 nothing would cover this case. Nothing in crates/ issues PREPARE TRANSACTION. A sentence saying that only the trigger covers this case (and app writers never prepare) would keep phase 2 from losing it.
• The routing and personal-read tests now seed history through insert_historical_event instead of going through the serving insert. That still fits what they test (reads and routing), and the helper is #[cfg(test)]-gated, but those tests no longer cover the writers end to end. The new event_floor_postgres_tests and the existing writer tests cover the writers.

[wpfleger96]

🤖 Clear at 2f337aae9. The blocker from the last review is fixed.

• scripts/seed-admin-dashboard.sh:36 now refuses any PGDATABASE that isn't a plain name, and it runs before either psql function is defined, so a URI or host=/hostaddr=/service= connection string can no longer override -h. Both psql calls go through run_psql, and both now pass -X, so a psqlrc can't \connect somewhere else after the guard.
• admin_seed_refuses_remote_database_before_invoking_psql passes, and it runs by name in the Unit Tests job. Deleting the PGDATABASE check makes the URI and key/value cases fail, and dropping -X makes the control assertion fail. All nine event_floor_postgres_tests and the existing floor tests in runtime::postgres_tests run by name in the PostgreSQL job and pass.
• The rebase kept all five floor checks next to the roster guard from #8316. In event.rs, the two checks (:512 and :1819) still run only inside if was_inserted. The member snapshot check (channel_members.rs:491) and the two in replaceable.rs (:254 and :498) still run before the old head is touched. production_event_writer_source_files() has the same body as the PR's old helper, so the floor source-policy test scans the same files.
• The new replica_fence.rs comment covers the prepared-transaction case from the last review.

Optional:

• The Docker fallback in seed-admin-dashboard.sh:47-50 isn't covered by the guard. docker exec goes to whatever daemon DOCKER_HOST, DOCKER_CONTEXT or the saved Docker context selects, and .env is sourced with allexport. The container-side psql also has no explicit -h, so it uses the container's libpq settings instead of the ones the guard checked. Without host psql, a remote context with a buzz-postgres container would get the unarmed fixture writes. This branch was already on main and the fixes only added -X, so I'm not blocking on it. Failing closed when host psql is missing is the smallest fix. Otherwise, pin docker --context default, pass -h explicitly, and add a fallback case to the refusal test.
• The PR description doesn't have buzz-review-completed yet.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account.

One P2 finding inline: preserve expired duplicate replay in both replacement writers. No additional actionable findings. Reviewed head 2f337aae90e5d7e2e372cc95745bbfabc7cdf8c1 against base/merge-base 97e1f6a0828d97d0031e2a31402554b679bf1146.

Checked all five channel-bearing writer guards, duplicate/replacement exits, caller-owned rollback paths, clock sampling against the replica-fence handshake, NULL-channel exemptions, the retained deferred trigger, test fixtures/source policies, and the seed-script changes. The public PR description has no image attachments; no actionable disclosure finding. Prepared transactions still depend on the retained trigger, as documented. The pre-existing Docker-fallback and source-scanner limitations discussed in the earlier review remain outside this change.

Independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/error/replay flows) lanes, integrated by Star Lord. Source inspection only: no PR code, tests, builds, applications or database operations executed. One GitHub check snapshot showed 56 successful and 28 skipped checks; PostgreSQL timing/concurrency, deployment behavior and added query latency were not independently exercised. This COMMENT review is not approval or merge authorization.

[wesbillman]

[P2] Preserve expired replay of stored, soft-deleted replacement IDs

The early duplicate/dominated checks only inspect undeleted heads (plus the NIP-RS watermark), so they miss an already-stored, soft-deleted ID when its coordinate has no live head. For a channel-bearing event older than 960 seconds, this new check now returns CreatedAtFloorRejected before INSERT ... ON CONFLICT DO NOTHING can identify the duplicate. At the base revision, that INSERT affected zero rows and the savepoint rollback returned ParameterizedReplaceStatus::Duplicate; the deferred AFTER INSERT trigger did not run for the conflict. The addressable replacement has the same regression at line 498, before its duplicate/false rollback branch. Callers replaying these already-persisted IDs now receive an error instead of the previous idempotent no-op.

Preserve stored-ID duplicate detection before rejecting age, while retaining the existing coordinate/precondition semantics and protection against superseding the old head on rejection. Add expired, channel-bearing soft-deleted replay cases for both writers; the existing parameterized_duplicate_restores_live_head_inside_caller_transaction test uses channel_id=None, which bypasses this guard.

[loganj]

🤖 Larry and Auto: an independent adversarial review of 9512e8c165. Source review only, plus local Postgres 17 runs and mutation tests. Approving, with one must-fix: Wes's P2 on the replacement writers is still open at this head. Auto and I each found it independently before reading the PR comments, and we reproduced it.

Must-fix (error): replaying an expired, soft-deleted replacement id is rejected, not reported as Duplicate.

• replaceable.rs:254 (parameterized) and :483 (addressable) still check the floor before INSERT … ON CONFLICT DO NOTHING. The early duplicate/dominated exits only look at undeleted heads. So a channel event that was stored and later soft-deleted (for example by NIP-09), replayed more than 960 s after its created_at, now returns CreatedAtFloorRejected, which the relay reports as an internal error. Before this change the INSERT hit the conflict, the trigger never fired, and the result was (event, false).
• Repro: insert an aged row with insert_historical_event, run UPDATE events SET deleted_at = now(), then replay it through replace_parameterized_event (kind 30078) or replace_addressable_event (kind 39000). Both return Err(CreatedAtFloorRejected) at this head.
• Fix (validated locally by both of us): move the check to just after each writer's if insert_result.rows_affected() == 0 { return Ok(false); }. For the parameterized writer that is inside the savepoint block, so the existing error arm's rollback to the savepoint restores the old head. For the addressable writer it is before after_admitted_insert. With that change, both repro tests pass, and all existing event_floor tests still pass, including *_rejects_old_and_accepts_fresh (an aged new row is still rejected and leaves no row). Please add the two repro tests as regression tests, and update the placement comment on guard_event_created_at_floor and the placement table and behavior notes in the PR body. replace_member_event can stay as is: a conflict there already errors.

What we checked

• The rebase builds now. 55917a3631 didn't compile (a lost Uuid import and a test_support call with the old arity). 9512e8c165 fixes both.
• The fence proof holds. The check runs on the admitted transaction with clock_timestamp(), which is never earlier than xact_start. A transaction open during the probe scan is bounded by its xact_start, and one that starts later samples a clock after S. Rows aging between the check and COMMIT don't break the proof. The cases only the retained trigger covers (UPDATEs of created_at/channel_id, raw-SQL writers, 2PC) are named in the PR, and the source policy covers the first two.
• Completeness. There are 7 production INSERT INTO events sites. The five channel-bearing ones call the check (event.rs ×2, replaceable.rs ×2, channel_members.rs). push.rs and relay_members.rs write a literal NULL channel_id. That matches the PR's inventory, and main up to e2dc54e22 adds no new writer.
• Each check is tested. Auto removed each of the five checks one at a time. Each removal failed its own *_rejects_old_and_accepts_fresh Postgres test and the floor source-policy test. The only behavior no test covers is the soft-deleted replay above.
• We read the earlier reviews before posting. The seed-script blockers are fixed, and we don't repeat the Docker-fallback note.

Optional

• (advice) The trigger fired only on sessions that set the buzz.created_at_floor GUC (the writer pool), but the app check runs on every pool. That's why the historical fixtures moved to insert_historical_event. We found no production writer outside the armed pool, but the PR body should state that difference so phase 2 doesn't rely on the old scope.
• (preference) The two fix(seed) commits harden scripts/seed-admin-dashboard.sh, which is separate from moving the floor check into the app. They could go in their own PR.
• (advice) The check adds one database round trip per newly inserted channel event. If write latency matters later, the clock comparison could go into a statement the writer already runs, such as the follow-up in after_admitted_insert.