diff options
Diffstat (limited to 'docs/explanation/purgatory-design.md')
| -rw-r--r-- | docs/explanation/purgatory-design.md | 112 |
1 files changed, 112 insertions, 0 deletions
diff --git a/docs/explanation/purgatory-design.md b/docs/explanation/purgatory-design.md index bf867ad..5ee4e06 100644 --- a/docs/explanation/purgatory-design.md +++ b/docs/explanation/purgatory-design.md | |||
| @@ -1,5 +1,10 @@ | |||
| 1 | # Purgatory Implementation Design | 1 | # Purgatory Implementation Design |
| 2 | 2 | ||
| 3 | **Status**: ✅ Implemented (2025-12-23) | ||
| 4 | **Implementation**: Phases 1-7 complete | ||
| 5 | **Source Code**: [`src/purgatory/`](../../src/purgatory/) | ||
| 6 | **Related**: [`docs/explanation/architecture.md`](architecture.md) - System architecture overview | ||
| 7 | |||
| 3 | ## Overview | 8 | ## Overview |
| 4 | 9 | ||
| 5 | Purgatory is an in-memory holding area for nostr events that depend on git data that hasn't arrived yet, **and** for git data that arrived before its corresponding nostr event. Events/placeholders are held until the other half arrives, at which point they are processed and saved to the database. | 10 | Purgatory is an in-memory holding area for nostr events that depend on git data that hasn't arrived yet, **and** for git data that arrived before its corresponding nostr event. Events/placeholders are held until the other half arrives, at which point they are processed and saved to the database. |
| @@ -899,3 +904,110 @@ WritePolicyResult::Accept | |||
| 899 | 2. **Same event submitted twice** - Deduplicated by event ID | 904 | 2. **Same event submitted twice** - Deduplicated by event ID |
| 900 | 3. **Push timeout during processing** - Entry expiry extended to 15 min minimum | 905 | 3. **Push timeout during processing** - Entry expiry extended to 15 min minimum |
| 901 | 4. **Race between event and git push** - Whichever completes the pair triggers release | 906 | 4. **Race between event and git push** - Whichever completes the pair triggers release |
| 907 | |||
| 908 | |||
| 909 | ## Purgatory Authorization Fix (2025-12-24) | ||
| 910 | |||
| 911 | **Critical Implementation Note**: The original purgatory design placed purgatory checking AFTER git push execution. This created a deadlock where pushes were rejected because the authorizing state event was in purgatory, not the database. | ||
| 912 | |||
| 913 | ### The Deadlock Problem | ||
| 914 | |||
| 915 | **Original broken flow:** | ||
| 916 | 1. State event arrives → No git data exists → Event stored in PURGATORY (not database) | ||
| 917 | 2. Git push arrives → Authorization checks DATABASE only → No state found → **PUSH REJECTED** ❌ | ||
| 918 | 3. Purgatory check runs → But push already failed, so this never helps | ||
| 919 | |||
| 920 | ### The Fix: Authorization-Time Purgatory Check | ||
| 921 | |||
| 922 | **Correct flow (implemented):** | ||
| 923 | 1. State event arrives → No git data exists → Event stored in purgatory | ||
| 924 | 2. Git push arrives → Authorization checks **DATABASE + PURGATORY** → State found in purgatory → **PUSH AUTHORIZED** ✅ | ||
| 925 | 3. After successful push → Save purgatory event to database → Remove from purgatory | ||
| 926 | |||
| 927 | ### Implementation Details | ||
| 928 | |||
| 929 | #### 1. Modified [`AuthorizationResult`](../../src/git/authorization.rs:397) | ||
| 930 | |||
| 931 | Added `from_purgatory: bool` field to track whether the authorizing state came from purgatory: | ||
| 932 | |||
| 933 | ```rust | ||
| 934 | pub struct AuthorizationResult { | ||
| 935 | pub authorized: bool, | ||
| 936 | pub reason: String, | ||
| 937 | pub state: Option<RepositoryState>, | ||
| 938 | pub maintainers: Vec<String>, | ||
| 939 | pub from_purgatory: bool, // NEW: Track event source | ||
| 940 | } | ||
| 941 | ``` | ||
| 942 | |||
| 943 | #### 2. Enhanced [`get_authorization_for_owner()`](../../src/git/authorization.rs:342) | ||
| 944 | |||
| 945 | Added purgatory checking when no state found in database: | ||
| 946 | |||
| 947 | ```rust | ||
| 948 | pub async fn get_authorization_for_owner( | ||
| 949 | database: &SharedDatabase, | ||
| 950 | identifier: &str, | ||
| 951 | owner_pubkey: &str, | ||
| 952 | purgatory: Option<&Arc<Purgatory>>, | ||
| 953 | pushed_refs: &[(String, String, String)], | ||
| 954 | repo_path: &Path, | ||
| 955 | ) -> Result<AuthorizationResult> | ||
| 956 | ``` | ||
| 957 | |||
| 958 | **Logic**: | ||
| 959 | 1. Check database for state events (existing behavior) | ||
| 960 | 2. If no state in database AND purgatory available: | ||
| 961 | - Parse pushed refs to RefPairs | ||
| 962 | - Get local refs from repository | ||
| 963 | - Call [`find_matching_states()`](../../src/purgatory/mod.rs:203) | ||
| 964 | - Filter to latest event from authorized authors | ||
| 965 | - Return authorization with `from_purgatory: true` | ||
| 966 | |||
| 967 | #### 3. Post-Push Purgatory Event Save | ||
| 968 | |||
| 969 | In [`handle_receive_pack()`](../../src/git/handlers.rs:187), after successful push: | ||
| 970 | |||
| 971 | ```rust | ||
| 972 | if from_purgatory { | ||
| 973 | if let (Some(db), Some(purg)) = (&database, &purgatory) { | ||
| 974 | // Save state event to database | ||
| 975 | db.save_event(&state.event).await?; | ||
| 976 | |||
| 977 | // Remove from purgatory | ||
| 978 | purg.remove_state_event(identifier, &state.event.id); | ||
| 979 | } | ||
| 980 | } | ||
| 981 | ``` | ||
| 982 | |||
| 983 | ### Files Modified | ||
| 984 | |||
| 985 | 1. **[`src/git/authorization.rs`](../../src/git/authorization.rs)** | ||
| 986 | - Added `from_purgatory` field to `AuthorizationResult` | ||
| 987 | - Modified `get_authorization_for_owner()` signature and logic | ||
| 988 | - Added purgatory checking when database has no state | ||
| 989 | |||
| 990 | 2. **[`src/git/handlers.rs`](../../src/git/handlers.rs)** | ||
| 991 | - Modified `authorize_push()` to accept purgatory and repo_path parameters | ||
| 992 | - Added tracking of `from_purgatory` flag | ||
| 993 | - Added post-push database save for purgatory events | ||
| 994 | |||
| 995 | ### Why This Order Matters | ||
| 996 | |||
| 997 | Checking purgatory DURING authorization (before push execution) is critical: | ||
| 998 | |||
| 999 | - **Prevents deadlock**: Push is authorized by purgatory state before execution | ||
| 1000 | - **Maintains atomicity**: Only saves to database after successful push | ||
| 1001 | - **Race condition safe**: First successful push claims the purgatory event | ||
| 1002 | |||
| 1003 | The alternative (checking purgatory after push) creates an insurmountable deadlock where valid pushes are rejected because their authorizing state is in purgatory instead of the database. | ||
| 1004 | |||
| 1005 | ### Testing | ||
| 1006 | |||
| 1007 | The fix enables the `test_push_authorized_by_owner_state` integration test scenario where: | ||
| 1008 | 1. State event is sent to relay (goes to purgatory - no git data yet) | ||
| 1009 | 2. Git push is sent (uses purgatory state for authorization) | ||
| 1010 | 3. State event is released from purgatory to database | ||
| 1011 | |||
| 1012 | --- | ||
| 1013 | |||