mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
fix: apply NIP-01 state ordering in purgatory
This commit is contained in:
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
- Added four configuration options for bounded retention of NIP-09 deletion requests and NIP-62 request-to-vanish events, together with cleanup telemetry for operators.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Purgatory promotion now applies NIP-01's lowest-event-ID tie-break for same-second repository state replacements.
|
||||
|
||||
### Changed
|
||||
|
||||
- Addressed a production storage imbalance where roughly 50k of 60k stored events were deletion requests. Deletion requests now have a bounded lifecycle, so requests that are no longer relevant are reconciled and retired while requests that may still affect valid event handling are preserved.
|
||||
|
||||
+29
-17
@@ -1055,10 +1055,15 @@ async fn process_purgatory_state_events(
|
||||
return result;
|
||||
}
|
||||
|
||||
// Sort by created_at (oldest first) so we process events in chronological order.
|
||||
// This ensures that when multiple state events are in purgatory, older ones
|
||||
// get processed first, allowing newer ones to correctly supersede them.
|
||||
purgatory_states.sort_by_key(|entry| entry.event.created_at);
|
||||
// Process oldest states first. NIP-01 breaks equal-timestamp replaceable
|
||||
// event ties by retaining the lexicographically lowest event ID, so include
|
||||
// that ordering here instead of relying on insertion order.
|
||||
purgatory_states.sort_by(|a, b| {
|
||||
a.event
|
||||
.created_at
|
||||
.cmp(&b.event.created_at)
|
||||
.then_with(|| a.event.id.cmp(&b.event.id))
|
||||
});
|
||||
|
||||
debug!(
|
||||
identifier = %identifier,
|
||||
@@ -1161,23 +1166,26 @@ async fn process_purgatory_state_events(
|
||||
result.refs_deleted += process_result.refs_deleted;
|
||||
result.errors.extend(process_result.errors);
|
||||
|
||||
// Check if there's a newer state from the same author in the database
|
||||
let has_newer_from_same_author = db_repo_data.states.iter().any(|s| {
|
||||
// Check whether the database already has the NIP-01-preferred state
|
||||
// for this author and coordinate. For equal timestamps, the lower
|
||||
// event ID wins.
|
||||
let has_preferred_state_from_same_author = db_repo_data.states.iter().any(|s| {
|
||||
s.event.pubkey == state.event.pubkey
|
||||
&& (s.event.created_at > state.event.created_at
|
||||
|| (s.event.created_at == state.event.created_at
|
||||
&& s.event.id > state.event.id))
|
||||
&& s.event.id < state.event.id))
|
||||
});
|
||||
|
||||
if has_newer_from_same_author {
|
||||
// Just remove from purgatory without saving - a newer event from same author exists
|
||||
if has_preferred_state_from_same_author {
|
||||
// Just remove from purgatory without saving: a newer or NIP-01
|
||||
// tie-break-preferred state from the same author already exists.
|
||||
purgatory.remove_state_event(identifier, &entry.event.id);
|
||||
result.states_released += 1;
|
||||
|
||||
debug!(
|
||||
identifier = %identifier,
|
||||
event_id = %entry.event.id,
|
||||
"Removed older state event from purgatory - newer event from same author exists in DB"
|
||||
"Removed superseded state event from purgatory - preferred event from same author exists in DB"
|
||||
);
|
||||
} else {
|
||||
// Save to database
|
||||
@@ -1330,25 +1338,28 @@ fn is_latest_authorized_state(
|
||||
maintainers: &[String],
|
||||
db_states: &[RepositoryState],
|
||||
) -> bool {
|
||||
// Find the latest authorized state from database
|
||||
// Find the NIP-01-preferred authorized state from the database: newest
|
||||
// timestamp wins, and the lowest event ID wins an equal-timestamp tie.
|
||||
let latest_db_state = db_states
|
||||
.iter()
|
||||
.filter(|s| maintainers.contains(&s.event.pubkey.to_hex()))
|
||||
.max_by(|a, b| {
|
||||
// Compare by created_at, then by event id for tie-breaking
|
||||
// Compare by created_at, then reverse event-ID order so `max_by`
|
||||
// selects the lexicographically lowest ID on equal timestamps.
|
||||
a.event
|
||||
.created_at
|
||||
.cmp(&b.event.created_at)
|
||||
.then_with(|| a.event.id.cmp(&b.event.id))
|
||||
.then_with(|| b.event.id.cmp(&a.event.id))
|
||||
});
|
||||
|
||||
match latest_db_state {
|
||||
None => true, // No other states exist in DB, this is the latest
|
||||
Some(latest) => {
|
||||
// This state is latest if it's newer, or if equal timestamp with larger event id
|
||||
// This state is preferred if it's newer, or if an equal timestamp
|
||||
// has a lower event ID under NIP-01's replaceable-event tie-break.
|
||||
state.event.created_at > latest.event.created_at
|
||||
|| (state.event.created_at == latest.event.created_at
|
||||
&& state.event.id >= latest.event.id)
|
||||
&& state.event.id <= latest.event.id)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2157,8 +2168,9 @@ mod tests {
|
||||
|
||||
let maintainers = vec![keys.public_key().to_hex()];
|
||||
|
||||
// The one with larger event ID should be considered latest
|
||||
let (latest, older) = if state1.event.id > state2.event.id {
|
||||
// NIP-01 retains the lexicographically lower event ID on an
|
||||
// equal-timestamp replaceable-event tie.
|
||||
let (latest, older) = if state1.event.id < state2.event.id {
|
||||
(state1, state2)
|
||||
} else {
|
||||
(state2, state1)
|
||||
|
||||
@@ -37,6 +37,38 @@ use common::{
|
||||
use nostr_sdk::prelude::*;
|
||||
use std::time::Duration;
|
||||
|
||||
/// Build a state event with a nonce that changes its ID without changing its
|
||||
/// replaceable coordinate or declared repository state.
|
||||
fn build_grinded_state_event(
|
||||
keys: &Keys,
|
||||
identifier: &str,
|
||||
commit_hash: &str,
|
||||
clone_url: &str,
|
||||
relay_url: &str,
|
||||
created_at: Timestamp,
|
||||
nonce: u64,
|
||||
) -> Event {
|
||||
EventBuilder::new(Kind::RepoState, "")
|
||||
.tags(vec![
|
||||
Tag::identifier(identifier),
|
||||
Tag::custom("clone", vec![clone_url.to_string()]),
|
||||
Tag::custom("relays", vec![relay_url.to_string()]),
|
||||
Tag::custom("refs/heads/main", vec![commit_hash.to_string()]),
|
||||
Tag::custom("HEAD", vec!["refs/heads/main".to_string()]),
|
||||
Tag::custom(
|
||||
"nonce",
|
||||
vec![
|
||||
nonce.to_string(),
|
||||
"0".to_string(),
|
||||
"ngit-created-at-tiebreak".to_string(),
|
||||
],
|
||||
),
|
||||
])
|
||||
.custom_created_at(created_at)
|
||||
.finalize(keys)
|
||||
.expect("build grinded state event")
|
||||
}
|
||||
|
||||
/// Test that a git push triggers `process_newly_available_git_data` and
|
||||
/// releases state events from purgatory.
|
||||
///
|
||||
@@ -123,6 +155,91 @@ async fn test_push_triggers_unified_processing() {
|
||||
relay.stop().await;
|
||||
}
|
||||
|
||||
/// Equal-timestamp state events from one author share a replaceable
|
||||
/// coordinate. Both must be held while their commit is missing, then only the
|
||||
/// NIP-01 winner (the lower event ID) may be promoted after the push.
|
||||
#[tokio::test]
|
||||
async fn same_second_same_author_states_in_purgatory_keep_nip01_lowest_id() {
|
||||
let relay = TestRelay::start().await;
|
||||
let keys = Keys::generate();
|
||||
let identifier = "same-second-purgatory-state";
|
||||
let temp_dir = tempfile::tempdir().expect("create test repository directory");
|
||||
let commit_hash = create_test_repo_with_commit(temp_dir.path(), CommitVariant::StateTest)
|
||||
.expect("create test repository");
|
||||
let announcement = create_repo_announcement(&keys, &[&relay.domain()], identifier);
|
||||
|
||||
let client = Client::builder()
|
||||
.authenticator(SignerAuthenticator::new(keys.clone()))
|
||||
.build();
|
||||
client.add_relay(relay.url()).await.expect("add relay");
|
||||
client.connect().await;
|
||||
client
|
||||
.send_event(&announcement)
|
||||
.await
|
||||
.expect("send announcement");
|
||||
|
||||
let npub = keys.public_key().to_bech32().expect("encode public key");
|
||||
let clone_url = format!("http://{}/{}/{}.git", relay.domain(), npub, identifier);
|
||||
let created_at = Timestamp::from_secs(Timestamp::now().as_secs() + 1);
|
||||
let losing_state = build_grinded_state_event(
|
||||
&keys,
|
||||
identifier,
|
||||
&commit_hash,
|
||||
&clone_url,
|
||||
relay.url(),
|
||||
created_at,
|
||||
0,
|
||||
);
|
||||
let winning_state = (1..100_000)
|
||||
.map(|nonce| {
|
||||
build_grinded_state_event(
|
||||
&keys,
|
||||
identifier,
|
||||
&commit_hash,
|
||||
&clone_url,
|
||||
relay.url(),
|
||||
created_at,
|
||||
nonce,
|
||||
)
|
||||
})
|
||||
.find(|candidate| candidate.id < losing_state.id)
|
||||
.expect("nonce grinding must produce a lower NIP-01 event ID");
|
||||
|
||||
assert_eq!(losing_state.created_at, winning_state.created_at);
|
||||
assert!(winning_state.id < losing_state.id);
|
||||
|
||||
// Submit the losing state first to prove promotion does not depend on
|
||||
// arrival order when purgatory releases an equal-timestamp pair.
|
||||
client
|
||||
.send_event(&losing_state)
|
||||
.await
|
||||
.expect("send higher-ID state event");
|
||||
client
|
||||
.send_event(&winning_state)
|
||||
.await
|
||||
.expect("send lower-ID state event");
|
||||
|
||||
for event_id in [losing_state.id, winning_state.id] {
|
||||
verify_event_not_served(relay.url(), &event_id, Duration::from_secs(1))
|
||||
.await
|
||||
.expect("both state events must remain in purgatory before the push");
|
||||
}
|
||||
|
||||
push_to_relay(temp_dir.path(), &relay.domain(), &npub, identifier)
|
||||
.expect("push state commit to relay");
|
||||
|
||||
let promoted = wait_for_event_served(relay.url(), &winning_state.id, Duration::from_secs(5))
|
||||
.await
|
||||
.expect("lower-ID state event must be promoted");
|
||||
assert_eq!(promoted.id, winning_state.id);
|
||||
verify_event_not_served(relay.url(), &losing_state.id, Duration::from_secs(1))
|
||||
.await
|
||||
.expect("higher-ID state event must not be promoted");
|
||||
|
||||
client.disconnect().await;
|
||||
relay.stop().await;
|
||||
}
|
||||
|
||||
/// Test that a state event entering purgatory triggers remote git fetch
|
||||
/// and is released once the git data is available.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user