feat(nip-fi): add PostgreSQL-final admission authority - #7157
Conversation
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head 17edab6f7da4ad52cee2e577b509c060aef4799c against exact base cd0b9798f0b502aeb46be1e1ee32904cc7ba19cd.
Contract: Phase A kind-9 admission must combine independent assertion/proof, the exact current identity-to-key binding, current enrollment/operation policy, and live authority with the event in one transaction. Baseline non-NIP-FI traffic remains compatible. This is a source-only review: no checkout, build, tests, or PR-code execution.
Exposure qualification: AppState::new currently leaves nip_fi: None; the new verifier has no startup wiring. The findings below concern the new authority implementation and its attached-WebSocket path, not a claim that deployed traffic already reaches it. I am not requiring HTTP, optional profiles, or full deployment wiring in this staged PR. Staging does not make incorrect final-authority decisions safe to build on.
1. P1: Require the proven actor to match the assertion and existing binding
At admission.rs:865–911, binding lookup uses only community/issuer/subject and never reads event_author_pubkey. Revalidation only compares the fresh assertion’s asserted key with its earlier value, not with the proven actor. The POA then records the caller’s actor alongside the selected binding, and protected use compares that actor with the caller again; its binding read checks only state/revision/expiry. The schema FK identifies a binding, but does not enforce actor equality.
Source-derived reproduction: create an active binding (issuer, subject) → key A; present that identity’s still-valid assertion with an authenticated, otherwise channel-eligible key B and a fresh proof. Admission selects A’s binding and authorizes B. This also accepts an assertion explicitly naming A. Require asserted-key equality when present and exact binding ownership by the proven actor before any enrollment/authority writes, including the concurrent-enrollment recheck. See core NIP-FI PrepareDirect, lines 338–347.
2. P1: Enforce the enrollment mode before creating a binding
The policy read at admission.rs:773–805 omits enrollment_mode, and the missing-binding branch always calls enroll_binding. make_binding_proposal chooses provenance solely from asserted-key presence. Database constraints accept modes/provenances as closed values but do not enforce the selected mode’s admission semantics.
Source-derived reproduction: under a current mode-2 provisioned policy with no binding, ordinary first use creates one; under mode-1 attested-key policy, an assertion without nostr_pubkey creates a TOFU binding. Read and enforce the current mode in final admission: provisioned requires an existing binding, attested-key requires matching attestation, and TOFU creation requires explicit TOFU policy. Add negative cases proving no authority/event effects. Mode 1 is attested-key, not “open/all” as the fixture comment says.
3. P1: Do not authorize against transaction-start time after lock waits
mod.rs:225–245 samples transaction_timestamp() before admission acquires its locks and reuses it for protected use. PostgreSQL freezes that value for the whole transaction; moving the same SELECT after the lock would not fix it.
Source-derived reproduction: let a binding expire at T, start admission just before T, hold the community writer lock until just after T, then release it while proof/assertion remain live. The binding checks at admission.rs:906–909 and 1860–1863 both compare against the pre-T value, so the expired binding still authorizes the event. Policy expiry has the same problem. The POA issued_at < expires_at CHECK does not catch this: issued_at uses wall-clock time, but its expiry is only min(proof, upstream_deadline) (admission.rs:1156–1175), omitting the binding/policy bounds. Sample current DB time after waits at the final admission/use boundary and apply every applicable bound; test this ordered interleaving.
4. P1: Revalidate the current assertion dependency at the final authority boundary
mod.rs:182–189 performs the only JWS revalidation before opening the transaction. Its resulting assertion is then reused across pool acquisition, advisory-lock waits, and retries. Neither admission nor protected use rereads the key source/snapshot.
Source-derived reproduction: verification succeeds under snapshot S1; while admission waits, the authenticated source installs S2 removing the signing key; admission resumes and commits from the cached S1 result. The shared source explicitly exposes refreshed snapshots to all verifiers (buzz-auth/src/nip_fi/verifier.rs:173–195), but the database writer lock does not serialize it. Retain any needed prefetch outside the transaction, but revalidate/fence current dependency evidence at final authority acquisition and on retries. A controlled S1→S2 removal during the wait must deny without mutations, as required by core FI-INV-07 and final admission (NIP-FI.md:372–389).
5. P2: A connection’s second message is rejected as proof replay
admission.rs:1070–1082 inserts (community_id, proof_event_id) for each message. That ID is the connection’s one NIP-42 AUTH event, stored in OnceLock by auth.rs:285–306 and copied unchanged by event.rs:755–765. Further AUTH attempts are rejected as already authenticated.
Source-derived reproduction: with the verifier wired, authenticate once and send two distinct valid kind-9 messages on that socket. The first claims the AUTH ID; the second hits the replay primary key and fails. Reconnection is currently the only escape. Separate connection-proof/session admission from request-bound message effects, preserving replay/idempotence guarantees rather than substituting an unbound random operation ID. Add a real ingress test for two distinct messages and duplicate submission.
6. P2: Use the fixed denial contract instead of exposing private reasons and DB errors
ingest.rs:3092–3122 emits distinguishable replay/binding/conflict/resource messages and formats every unmatched error with {e:?}. Transient(String) includes raw sqlx diagnostics. These become IngestError::Rejected, which event.rs:798–805 sends unchanged; only Internal receives sanitization.
Source-derived reproduction: trigger a replay versus an inactive binding, or a database failure in final admission. The client receives private-state-specific text or the database error rather than the fixed public class. Map through the existing denial contract and nostr_text(), preserving unavailable versus denied classification, and keep diagnostics server-side. Assert byte-identical private denials through the actual WS response path (FI-INV-13).
7. P2: Replace the advertised atomicity witnesses with causal production-path tests
The factory at admission.rs:3196–3199 returns NipFiTestOrchestrator, a separate copy of the transaction body (mod.rs:401–555). Removing protected use or splitting the transaction only in production cannot affect these tests. Shared SQL helper coverage is not production-orchestrator coverage.
The failure witnesses also miss their target: the rollback case deletes the channel before admission, so ResourceStateDenied fires before any authority writes/event insert; the epoch zero-row case submits a nonexistent channel and accepts any error; the POA zero-row case accepts ordinary success (admission.rs:2840–2845). Guard removal therefore is not falsified by these oracles.
Use the actual production transaction body with controlled dependencies; inject event-insert failure after successful admission, and zero-row UPDATE results at the intended seam. Assert the exact error and rollback of all authority/event effects. Then select these tests and both seal drivers in CI: the current just test-unit/Backend Integration filters omit them, and all ten PG cases are ignored. Selected integration tests must fail, not silently return success, when their required DB setup fails. This follows the existing TESTING.md:25–32 standard, not a new full-conformance requirement.
Additional validation corrections
The all-targets warnings-as-errors command also compiles the ignored tests. committed at admission.rs:2609, ghost_ctx at 2738, ghost_community_id at 2769, and db_now2 at 2772 are unused and need correction for cargo clippy --workspace --all-targets -- -D warnings. This is source-derived, not a claimed local/CI run.
Exit criteria: fix the seven mechanisms above with production-bound negative/positive witnesses and actual CI selection. Keep startup/enforce rollout, HTTP coverage, optional lifecycle/delegation profiles, and unrelated hardening outside this repair. The shared transaction and deletion-fence structure are useful; the missing authorization predicates and non-causal tests are the blockers.
17edab6 to
fcc68a9
Compare
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head fcc68a94af7116b64ccc9d4381b11f0124e2b573 against exact base cd0b9798f0b502aeb46be1e1ee32904cc7ba19cd, with a true prior-head comparison against 17edab6f7da4ad52cee2e577b509c060aef4799c (not the rewritten branch’s merge-base diff).
Contract and scope: preserve baseline traffic in off mode; protected kind-9 admission requires independent assertion/proof, exact current identity-to-key binding, current enrollment policy, and atomic authority/event effects. This re-review covers the seven prior findings and changed paths. Startup now exposes enforce and deny-protected, so their actual behavior is newly in scope. Optional profiles and unrelated hardening remain excluded. Source-only: no checkout, build, tests, CI execution, or PR-code execution; test claims in the PR description are not independently verified runtime evidence.
Progress credited: production and tests now share commit_kind9_inner; the orchestrator rollback case reaches an AUTH-event insertion rejection instead of deleting the channel before admission; the four previously identified unused bindings are repaired in source. Finding 7 is therefore partially fixed, not unchanged. Findings 1–6 remain. Two new startup defects are listed first because they determine reachability.
8. P1: Enforce the configured mode rather than letting missing evidence select baseline admission
state.rs:1431–1448 recognizes deny-protected but returns without storing that mode or installing a denying gate. It has the same admission state as off mode. Under enforce, the only state change is installing a verifier, while the actual gate at ingest.rs:3047–3133 is conditional on client-supplied FI context.
Source-derived reproduction: configure either protected mode, authenticate an otherwise eligible NIP-42 member, omit the FI header, and send kind 9. router.rs:415–430 also maps repeated/malformed headers to None; connection.rs:223–224 accepts that as no assertion. The event then takes ordinary storage without NIP-FI admission. The existing enum explicitly defines Enforce as requiring valid assertion evidence on every protected ingress (buzz-auth/src/nip_fi/startup/mod.rs:18–25), and the new startup warning identifies kind 9 as protected. This violates the server-owned/no-downgrade contract (core NIP-FI lines 125–141), not merely an inferred preference for stricter defaults. Persist mode separately from verifier availability and enforce it at the protected-operation boundary. Unsupported protected ingress must deny rather than fall through; this does not require implementing full HTTP NIP-FI support in this PR. Test both modes with absent, malformed, and repeated evidence, plus off-mode compatibility.
9. P1: Give the production JWKS source a warm/refresh owner
state.rs:1515–1523 creates ProductionJwksSource and immediately moves it into FederatedAssertionVerifier, without fetching a snapshot or retaining a refresh handle. The constructor initializes empty state (buzz-auth/src/nip_fi/jwks/mod.rs:480–498); key_set():631–647 only reads the cache. Only get_snapshot() performs the asynchronous refresh.
Source-derived reproduction: start with valid enforce configuration and a reachable JWKS endpoint, then attach a correctly signed assertion. Verification reads no snapshot, returns KeySourceUnavailable, and the connection closes. The new initialization does not make successful FI traffic reachable. Wire bounded initial acquisition and ongoing refresh into the same source used by the verifier, with explicit fail-closed unavailability, and add a production-initialization success/refresh witness. A test orchestrator with a synthetic assertion cannot establish this.
Reachability qualification for the remaining findings: the cold-cache defect currently prevents successfully asserted traffic through this startup path. The defects below remain in the new final-authority implementation and become reachable when its source is correctly supplied; they are not claims that currently deployed production traffic has exercised these paths.
1. P1: Tie both the assertion and existing binding to the proven actor
admission.rs:755–802 still selects a binding only by community/issuer/subject, without reading its event_author_pubkey. Assertion equivalence compares the fresh asserted key with the prepared asserted key, not with the actor (:443–445). Protected use reads only binding state/revision/expiry (:1638–1677); its actor comparison checks the POA actor that admission itself just wrote. The schema FK identifies the binding but does not establish actor equality (schema/schema.sql:2891–2893).
Reproduction: identity I has active binding to key A; present I’s assertion with a valid otherwise-eligible proof from key B. Admission selects A’s binding and writes authority for B, including when the assertion explicitly names A. Require asserted_key == actor when present and the exact current binding(I, actor) before writes, including the enrollment recheck (:1205–1233). Test mismatch denial with no authority/event effects. Core NIP-FI lines 338–347 already require this.
2. P1: Read and enforce enrollment mode before creating a binding
admission.rs:663–697 still omits enrollment_mode; every absent binding calls enroll_binding (:804–820). make_binding_proposal infers provenance solely from asserted-key presence (mod.rs:344–356). Closed-value schema constraints do not implement the mode’s admission semantics.
Reproduction: ordinary first use under provisioned mode creates a binding; an un-attested assertion under attested-key mode creates TOFU. Require an existing binding for provisioned mode, matching attestation for attested-key enrollment, and explicit TOFU policy for TOFU creation. Add each negative case and prove no durable effects. This is the existing PrepareDirect contract, not a new rollout requirement.
3. P1: Stop checking expiry against transaction-start time after lock waits
mod.rs:241–260 still samples transaction_timestamp() before the community fence and blocking writer lock (admission.rs:335–346,597–628), then reuses it for protected use. PostgreSQL freezes this timestamp for the transaction; moving the same SELECT below the locks is insufficient.
Reproduction: start just before an existing binding or policy expires, hold the writer lock across that deadline, and release while proof/assertion remain live. Binding checks (:797–800,1671–1677) and policy checks (:690–697) still use the pre-expiry value, so the event can commit. The POA wall-clock issued_at < expires_at CHECK does not rescue this case: its expiry is only min(proof, upstream_deadline) (:1047–1064), excluding binding/policy bounds. Sample current authoritative DB time after relevant waits at final admission/use and apply every applicable bound. Retain the ordered expiry-interleaving regression requested previously.
4. P1: Revalidate current assertion dependencies at final authority acquisition
mod.rs:163–171 still performs the only JWS revalidation before the transaction. fresh_assertion is then reused across pool acquisition, lock waits, and retries (:233–291). Neither admission nor protected use consults the verifier/source again.
Reproduction with a supplied shared source: verification succeeds under snapshot S1; while admission waits, refresh commits S2 removing the signing key; admission resumes using S1 and commits. The DB writer lock cannot fence that source. Keep network prefetch outside the transaction if necessary, but validate/fence the current dependency at the final authority boundary and repeat on retries. S1→S2 key removal must deny without mutation, while a retained key may pass current equivalent revalidation. This remains FI-INV-07 and core final-admission lines 372–389, not an optional key-rotation enhancement.
5. P2: Do not consume the connection’s one AUTH proof again for every message
admission.rs:959–973 still inserts (community_id, proof_event_id) on every kind-9 admission. handlers/auth.rs:285–306 stores the connection’s AUTH ID once; event.rs:754–765 reuses it. Further AUTH attempts are rejected as already authenticated.
Reproduction once FI verification succeeds: authenticate once and send two distinct valid messages. The first commits, the second is ProofReplayed. Separate connection-proof/session admission from request-bound message effects while preserving replay/idempotence; do not substitute an unbound random operation ID. Exercise two distinct messages and duplicate submission through real ingress.
6. P2: Preserve the fixed denial contract and keep private/database errors off the wire
ingest.rs:3097–3127 still distinguishes replay, binding, resource, and conflict reasons, then formats unmatched errors with {e:?}. Transient(String) includes sqlx diagnostics; event.rs:798–805 passes Rejected text unchanged to the client.
Reproduction: trigger replay versus inactive binding, or a final-admission database error. The wire response exposes the private reason or DB diagnostic instead of the fixed class. Use the existing DenialClass/nostr_text() contract, preserve unavailable versus denied classification, and retain diagnostics server-side. Assert byte-identical private denials through the response path (FI-INV-13).
7. P2: Finish the causal test and CI repair
The shared transaction body and improved insertion trigger repair the corresponding portions of the previous finding. The remaining witnesses still cannot establish their advertised guarantees:
- Both zero-row tests roll admission back, then accept
NoActiveBinding(admission.rs:2431–2477,:2514–2555). That exits at the missing-POA SELECT (:1531), before UPDATE guards (:1842,1872). Removing those guards would not change these outcomes. The orchestrator “zero-row” case now asserts two ordinary successes (:3480–3552). Inject zero affected rows at each intended UPDATE seam with earlier checks passing, and assert the exact error and rollback. - The rollback case accepts any
Transient(:3105–3110), not the intended insert failure, and checks only a subset of authority tables. The new wired-ingest positive asserts success/event existence (ingest.rs:6035–6055), which ordinary insertion can also satisfy. Add an authority-effect/call witness and exact failure assertions so bypassing the configured verifier or failing early cannot pass. - The 13 PG witnesses remain ignored and unselected by the reviewed
Justfile/Backend Integration commands; startup and seal drivers are also absent from those selections. Setup errors still turn into success, including new connection/migration.ok()?atingest.rs:5787–5790followed byelse { return; }(:5841–5843,5931–5933). Explicitly select the witnesses and both seal drivers, and make required setup failures fail the selected tests.
These requests follow existing TESTING.md:25–32; they are not a demand for optional full conformance.
Stable exit criteria
Complete the remaining seven original mechanisms with production-bound positive/negative witnesses and actual test selection. For the newly introduced startup surface, either make protected-mode enforcement and JWKS ownership real, or keep that rollout explicitly unavailable/staged rather than advertising modes that bypass or cannot succeed. Preserve the shared atomic transaction and off-mode baseline. No new requirement to implement optional profiles or all HTTP authorization features.
wpfleger96
changed the title
|
🤖 This is now the sole canonical Phase A PR 4. #7117, #7148, and #7150 are superseded and closed; no fixes should land on those branches. I consolidated their still-applicable feedback with the current reviews here. Before this returns to review, the branch needs to satisfy this single acceptance list:
The current formal review is review 5074148645. Item 10 is carried from the superseded #7150 review thread; source inspection at the canonical head still shows FI events forcing downstream The branch also needs to be rebuilt on the current #7109 head before any readiness claim. PR 5 remains held. |
e9e812c to
c92ee0e
Compare
fcc68a9 to
844985a
Compare
844985a to
c9ce902
Compare
c9ce902 to
a103340
Compare
GitHub Actions
Error
a103340 to
b9af5da
Compare
GitHub Actions
Error
GitHub Actions
Error
…thority (Design C) Implements PostgreSQL as the final authority for NIP-FI kind-9 admission on the Design C (one READ COMMITTED transaction) path. This is PR 4 in the Phase A stack; it builds on PR 3 (buzz-auth production assertion runtime, merged as 70895b3). - Add `connection_id UUID NOT NULL` to `nip_fi_proof_replay_claims`. Records the WebSocket connection UUID that first claimed each proof event, enabling per-connection ownership checks at admission time. - `NS_ADMISSION_OP`: domain-separated namespace constant (SHA-256 of "buzz.nip-fi.admission-op.v1", first 16 bytes). - `deterministic_admission_op_id(community_id, proof_event_id, signed_event_id)`: derives the admission operation_id via UUID v5 (SHA-1 namespaced), replacing the previous `Uuid::new_v4()` call in `commit_kind9_inner`. Same (community, proof, event) triple always produces the same operation_id, enabling idempotent exact-replay detection via the receipt protocol. - `Kind9Params` no longer carries an `operation_id` field; callers no longer allocate or pass it. - `commit_admission_body` reads `events FOR SHARE` after acquiring the NIP-FI writer lock. If the exact (community_id, created_at, id) row already exists, returns `AdmissionError::DuplicateEvent` immediately, writing zero new authority rows. - Reads `nip_fi_proof_replay_claims FOR SHARE` before any write. Same `conn_id` → same-connection reuse, continues. Different `conn_id` → returns `ProofReplayed`. - Reads `authorization_operation_receipts FOR SHARE` before any write. Same `operation_id` + same `request_fingerprint` → `DuplicateEvent`. Same `operation_id` + different fingerprint → `Transient` (intent conflict). - Moved from old step 10 (before epoch/POA) to after the epoch/fence and POA upserts. Uses `ON CONFLICT (community_id, proof_event_id) DO NOTHING`. After a DO NOTHING (PK race), re-reads the winning row under FOR SHARE: same conn_id → same-connection race resolved; different → `ProofReplayed`. - Includes `connection_id` in the INSERT. - If `insert_event_with_thread_metadata_in_tx` returns `was_inserted=false` after the authority mutations succeed (race that bypasses the precheck), `commit_kind9_inner` explicitly rolls back before returning the result, preventing orphaned authority mutations. - `AdmissionError::DuplicateEvent` is intercepted before the error-mapping closure. Returns `IngestResult { accepted: true, message: "duplicate:" }` immediately — the same duplicate response as the non-NIP-FI path. - Deleted `crates/buzz-nip-fi-seal-test/` and `crates/buzz-nip-fi-inner-seal-test/`. - Removed both from workspace `Cargo.toml` members. - Removed `nip-fi-boundary-test = []` feature from `buzz-relay/Cargo.toml`. - Collapsed cfg-split `mod nip_fi` / `pub mod nip_fi` in `lib.rs` to single private `mod nip_fi`. - Collapsed cfg-split `mod context` / `pub mod context` in `nip_fi/mod.rs`. - Updated `context.rs` doc comment (no longer references deleted crate). Named per the #6730 `postgres_tests` CI convention so they are selected automatically once this branch lands on main: 1. `postgres_same_conn_proof_reuse_allowed` — step 3d same-conn path 2. `postgres_cross_conn_proof_replay_rejected` — step 3d cross-conn path 3. `postgres_duplicate_event_precheck_is_noop` — step 3c precheck 4. `postgres_concurrent_same_conn_pk_race_both_succeed` — step-13 DO NOTHING 5. `postgres_concurrent_cross_conn_pk_race_one_rejected` — step-13 cross-conn 6. `postgres_deterministic_op_id_exact_replay_is_noop` — op_id idempotence Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The CI postgres-test-discovery checker requires that PG-backed tests sit inside a module whose name ends with `postgres_tests`. The two test modules in admission.rs used names `pg_integration` and `pg_orchestrator_integration`, which the checker rejects. Rename: mod pg_integration → mod postgres_tests mod pg_orchestrator_integration → mod orchestrator_postgres_tests Update the cross-module import accordingly. No behavioural change. Verified: `scripts/test-postgres-test-discovery.sh` passes across all 401 Rust source files in the crates directory. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…tnesses
Three correctness fixes for the Phase A PR 4 pass-2 review findings.
## Fix 3: post-lock DB time sample
db_now is now sampled from clock_timestamp() inside commit_admission_body
and authorize_protected_use_body AFTER both advisory locks are acquired
(steps 3 and 3b), not before the transaction opens.
Previously transaction_timestamp() was sampled pre-lock in commit_kind9_inner
and passed through the call chain. A proof that was valid at tx-open but
expired before the lock was acquired could pass the deadline check on a slow
server. With clock_timestamp() post-lock, every deadline check (proof expiry,
policy dates, upstream assertion deadline, binding expiry) is made against a
time provably concurrent with the lock-held authoritative reads.
Removes db_now from commit_admission_in_tx, commit_admission_body,
authorize_protected_use_in_tx, and authorize_protected_use_body signatures.
## Fix 4: deterministic protected-use operation ID
Adds NS_PROTECTED_USE_OP namespace constant and
deterministic_protected_use_op_id(community_id, proof_event_id, signed_event_id)
to admission.rs. The re-fence receipt now uses this deterministic ID instead
of Uuid::new_v4(), making the receipt insert idempotent on retry.
Adds signed_event_id field to CommittedAuthorization so authorize_protected_use_body
can bind the use-operation ID to the exact signed event. Includes signed_event_id
in the use-request fingerprint hash for complete binding.
## Fix 8: three additional test witnesses
(a) orchestrator_pg_on_conflict_do_nothing_zero_row_same_conn: exercises the
step-13 ON CONFLICT DO NOTHING zero-row path explicitly. Same-conn reuse
with a pre-existing claim row → rows_affected() == 0 → function continues.
(b) orchestrator_pg_fresh_proof_duplicate_event_no_new_rows: fresh proof_event_id
with an already-persisted signed event → step 3c fires → DuplicateEvent.
Verifies receipt, admission-result, and replay-claim counts are all unchanged.
(c) orchestrator_pg_deterministic_op_id_mutation_evidence: shows that the
exact-replay-is-noop property (deterministic op_id idempotence) would break
under a UUID-v4 mutation by asserting the receipt count does not change on
replay. The assertion names the mutation explicitly so a red test identifies
the broken invariant.
Also fixes DbError → sqlx::Error conversion in the late-conflict reread async
block (begin_transaction() returns DbError, not sqlx::Error).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
main merged 0043_push_gateway_dogfood_profile.sql while this branch was in review, colliding with the NIP-FI 0043_nip_fi_proof_replay_claims.sql introduced by this PR. Rename: 0043_nip_fi_proof_replay_claims.sql → 0044_nip_fi_proof_replay_claims.sql 0044_nip_fi_proof_claim_owner.sql → 0045_nip_fi_proof_claim_owner.sql Update all test assertions and doc comments that referred to migration 0043 or 0044 by the old NIP-FI numbering. No behavioural change. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Four orchestrator_postgres_tests used make_kind9_event for both the
first and second calls. Nostr event IDs are deterministic over
(pubkey, created_at, kind, tags, content), so two calls with identical
inputs within the same second produce the same event ID. Step 3c
(event duplicate precheck) fires before the intended guard — lifecycle
guard, epoch guard, or proof-replay uniqueness — causing the tests to
assert the wrong error.
Fix: add make_kind9_event_msg(keys, channel_id, content) helper and
use distinct content strings for every second call that is not
deliberately testing step-3c behaviour.
orchestrator_pg_concurrent_enrollment_converges: format content with
task index so each spawned task produces a unique event.
orchestrator_pg_lifecycle_advance_rejects_final_use: second call uses
"lifecycle-advance-second-msg" → NoActiveBinding or BindingRetired
fires as intended.
orchestrator_pg_epoch_guard_catches_zero_row_update: second call uses
"epoch-guard-second-msg" → epoch UPDATE guard runs and passes.
orchestrator_pg_proof_replay_is_rejected: second call uses
"proof-replay-second-msg" → step-3d proof-owner mismatch fires
(ProofReplayed) instead of step-3c DuplicateEvent.
After this change all 22 NIP-FI ignored PG tests pass (15/15
orchestrator_postgres_tests, 7/7 postgres_tests).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Migration 0043 is now push_gateway_dogfood_profile (index 42). Migration 0044 is nip_fi_proof_replay_claims (index 43). Migration 0045 is nip_fi_proof_claim_owner (index 44). The embedded_migrator_contains_consolidated_initial_schema test had two off-by-one errors introduced during the renumbering commit: 1. replay_claims used migrations[42].sql (push_gateway) instead of migrations[43].sql (nip_fi_proof_replay_claims). 2. The dogfood_profile version guard used migrations[43].version == 44 (replay_claims) instead of migrations[42].version == 43. Fix both index references and update comments to reflect the final migration numbering with 0043_push_gateway inserted before the two NIP-FI PR4 migrations. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
2007037 to
180059b
Compare
GitHub Actions
Error
2 participants
What
Adds PostgreSQL-final NIP-FI authority for protected kind-9 channel messages.
One
READ COMMITTEDtransaction acquires the ordinary community deletion fence and a per-community NIP-FI writer lock before final admission, protected-use validation, authority mutations, and event/thread insertion. The transaction commits those effects atomically or rolls them all back.Scope
Optional profiles, broad HTTP enablement, and unrelated hardening remain outside this PR.
Changes
New migrations (Design C Phase A):
0044_nip_fi_proof_replay_claims.sql: replay-claim table withconnection_id UUID NOT NULLowner column + extendedcommunity_write_fence_excluded_tableto exclude it from fence drift0045_nip_fi_proof_claim_owner.sql: proof-claim owner index + supporting infrastructureAdmission path (Design C protocol):
db_nowviaclock_timestamp()sampled inside the advisory-locked body — deadline/expiry checks reflect actual wall time at lock acquisition, not transaction startNS_ADMISSION_OPnamespace, derived from community+proof+event triple)FOR SHAREread ofeventsbefore any writeFOR SHARE; same conn → pass, different conn → ProofReplayedON CONFLICT DO NOTHING+ post-race owner rereadwas_inserted == falseincommit_kind9_innerDuplicateEventhandled iningest.rsas no-op duplicate responseSeal crates removed: both
buzz-nip-fi-seal-testandbuzz-nip-fi-inner-seal-testdeleted;cfgvisibility forks collapsed to plainmod nip_fiTests (22 NIP-FI PG tests, all pass):
buzz-dbmigration.rs:embedded_migrator_contains_consolidated_initial_schemaupdated to reflect final migration numbering (0043 = push_gateway_dogfood_profile, 0044 = replay_claims, 0045 = proof_claim_owner)Current status
PR 3 (#7109) merged as
70895b355. This PR now targetsmaindirectly.Stack: #7109 (merged) → this PR → PR 5