state-res: Revert calculating the auth chain in ruma

In a previous commit I moved the auth chain calculation code to ruma
because I thought I could optimize it by only taking auth chains from
conflicted events instead of all events. It turned out that was wrong
and now I removed that algorithm again (the full auth chains are now
passed in as an argument to state_res::resolve again).
This commit is contained in:
Timo Kösters 2021-07-16 09:33:49 +02:00 committed by Jonas Platte
parent 63411165da
commit d970501c85
No known key found for this signature in database
GPG Key ID: CC154DE0E30B7C67
6 changed files with 70 additions and 56 deletions

View File

@ -2,7 +2,7 @@
Breaking changes: Breaking changes:
* state_res::resolve now doesn't take auth_events anymore and calculates it on its own instead * state_res::resolve auth_events type has been slightly changed and renamed to auth_chain_sets
# 0.2.0 # 0.2.0

View File

@ -66,6 +66,14 @@ fn resolution_shallow_auth_chain(c: &mut Criterion) {
&room_id(), &room_id(),
&RoomVersionId::Version6, &RoomVersionId::Version6,
&state_sets, &state_sets,
state_sets
.iter()
.map(|map| {
store
.auth_event_ids(&room_id(), &map.values().cloned().collect::<Vec<_>>())
.unwrap()
})
.collect(),
|id| ev_map.get(id).map(Arc::clone), |id| ev_map.get(id).map(Arc::clone),
) { ) {
Ok(state) => state, Ok(state) => state,
@ -81,7 +89,7 @@ fn resolve_deeper_event_set(c: &mut Criterion) {
let ban = BAN_STATE_SET(); let ban = BAN_STATE_SET();
inner.extend(ban); inner.extend(ban);
let _store = TestStore(inner.clone()); let store = TestStore(inner.clone());
let state_set_a = [ let state_set_a = [
inner.get(&event_id("CREATE")).unwrap(), inner.get(&event_id("CREATE")).unwrap(),
@ -115,6 +123,14 @@ fn resolve_deeper_event_set(c: &mut Criterion) {
&room_id(), &room_id(),
&RoomVersionId::Version6, &RoomVersionId::Version6,
&state_sets, &state_sets,
state_sets
.iter()
.map(|map| {
store
.auth_event_ids(&room_id(), &map.values().cloned().collect::<Vec<_>>())
.unwrap()
})
.collect(),
|id| inner.get(id).map(Arc::clone), |id| inner.get(id).map(Arc::clone),
) { ) {
Ok(state) => state, Ok(state) => state,
@ -159,8 +175,8 @@ impl<E: Event> TestStore<E> {
} }
/// Returns a Vec of the related auth events to the given `event`. /// Returns a Vec of the related auth events to the given `event`.
pub fn auth_event_ids(&self, room_id: &RoomId, event_ids: &[EventId]) -> Result<Vec<EventId>> { pub fn auth_event_ids(&self, room_id: &RoomId, event_ids: &[EventId]) -> Result<BTreeSet<EventId>> {
let mut result = vec![]; let mut result = BTreeSet::new();
let mut stack = event_ids.to_vec(); let mut stack = event_ids.to_vec();
// DFS for auth event chain // DFS for auth event chain
@ -170,7 +186,7 @@ impl<E: Event> TestStore<E> {
continue; continue;
} }
result.push(ev_id.clone()); result.insert(ev_id.clone());
let event = self.get_event(room_id, &ev_id)?; let event = self.get_event(room_id, &ev_id)?;

View File

@ -46,6 +46,9 @@ impl StateResolution {
/// * `state_sets` - The incoming state to resolve. Each `StateMap` represents a possible fork /// * `state_sets` - The incoming state to resolve. Each `StateMap` represents a possible fork
/// in the state of a room. /// in the state of a room.
/// ///
/// * `auth_chain_sets` - The full recursive set of `auth_events` for each event in the
/// `state_sets`.
///
/// * `fetch_event` - Any event not found in the `event_map` will defer to this closure to find /// * `fetch_event` - Any event not found in the `event_map` will defer to this closure to find
/// the event. /// the event.
/// ///
@ -58,6 +61,7 @@ impl StateResolution {
room_id: &RoomId, room_id: &RoomId,
room_version: &RoomVersionId, room_version: &RoomVersionId,
state_sets: &[StateMap<EventId>], state_sets: &[StateMap<EventId>],
auth_chain_sets: Vec<BTreeSet<EventId>>,
fetch_event: F, fetch_event: F,
) -> Result<StateMap<EventId>> ) -> Result<StateMap<EventId>>
where where
@ -98,8 +102,7 @@ impl StateResolution {
} }
// The set of auth events that are not common across server forks // The set of auth events that are not common across server forks
let mut auth_diff = let mut auth_diff = StateResolution::get_auth_chain_diff(room_id, auth_chain_sets)?;
StateResolution::get_auth_chain_diff(room_id, &conflicting_state_sets, &fetch_event)?;
// Add the auth_diff to conflicting now we have a full set of conflicting events // Add the auth_diff to conflicting now we have a full set of conflicting events
auth_diff.extend(conflicting.values().cloned().flatten().filter_map(|o| o)); auth_diff.extend(conflicting.values().cloned().flatten().filter_map(|o| o));
@ -219,52 +222,17 @@ impl StateResolution {
} }
/// Returns a Vec of deduped EventIds that appear in some chains but not others. /// Returns a Vec of deduped EventIds that appear in some chains but not others.
pub fn get_auth_chain_diff<E, F>( pub fn get_auth_chain_diff(
_room_id: &RoomId, _room_id: &RoomId,
conflicting_state_sets: &[BTreeSet<EventId>], auth_chain_sets: Vec<BTreeSet<EventId>>,
fetch_event: F, ) -> Result<BTreeSet<EventId>> {
) -> Result<BTreeSet<EventId>> if let Some(first) = auth_chain_sets.first().cloned() {
where let common = auth_chain_sets
E: Event,
F: Fn(&EventId) -> Option<Arc<E>>,
{
let mut chains = vec![];
// Conflicted state sets are just some top level state events. Now we fetch the complete
// auth chain of those events
for ids in conflicting_state_sets {
// TODO state store `auth_event_ids` returns self in the event ids list
// when an event returns `auth_event_ids` self is not contained
let mut todo = ids.iter().map(|e| e.clone()).collect::<BTreeSet<_>>();
let mut auth_chain_ids = ids.clone(); // we also return the events we started with
while let Some(event_id) = todo.iter().next().cloned() {
if let Some(pdu) = fetch_event(&event_id) {
todo.extend(
pdu.auth_events()
.clone()
.into_iter()
.collect::<BTreeSet<_>>()
.difference(&auth_chain_ids)
.cloned(),
);
auth_chain_ids.extend(pdu.auth_events().into_iter());
} else {
warn!("Could not find pdu mentioned in auth events.");
}
todo.remove(&event_id);
}
chains.push(auth_chain_ids);
}
if let Some(first) = chains.first().cloned() {
let common = chains
.iter() .iter()
.skip(1)
.fold(first, |a, b| a.intersection(&b).cloned().collect::<BTreeSet<EventId>>()); .fold(first, |a, b| a.intersection(&b).cloned().collect::<BTreeSet<EventId>>());
Ok(chains.into_iter().flatten().filter(|id| !common.contains(&id)).collect()) Ok(auth_chain_sets.into_iter().flatten().filter(|id| !common.contains(&id)).collect())
} else { } else {
Ok(btreeset![]) Ok(btreeset![])
} }

View File

@ -69,6 +69,14 @@ fn ban_with_auth_chains2() {
&room_id(), &room_id(),
&RoomVersionId::Version6, &RoomVersionId::Version6,
&state_sets, &state_sets,
state_sets
.iter()
.map(|map| {
store
.auth_event_ids(&room_id(), &map.values().cloned().collect::<Vec<_>>())
.unwrap()
})
.collect(),
|id| ev_map.get(id).map(Arc::clone), |id| ev_map.get(id).map(Arc::clone),
) { ) {
Ok(state) => state, Ok(state) => state,

View File

@ -258,6 +258,14 @@ fn test_event_map_none() {
&room_id(), &room_id(),
&RoomVersionId::Version2, &RoomVersionId::Version2,
&state_sets, &state_sets,
state_sets
.iter()
.map(|map| {
store
.auth_event_ids(&room_id(), &map.values().cloned().collect::<Vec<_>>())
.unwrap()
})
.collect(),
|id| ev_map.get(id).map(Arc::clone), |id| ev_map.get(id).map(Arc::clone),
) { ) {
Ok(state) => state, Ok(state) => state,

View File

@ -109,10 +109,20 @@ pub fn do_check(
.collect::<Vec<_>>() .collect::<Vec<_>>()
); );
let resolved = let resolved = StateResolution::resolve(
StateResolution::resolve(&room_id(), &RoomVersionId::Version6, &state_sets, |id| { &room_id(),
event_map.get(id).map(Arc::clone) &RoomVersionId::Version6,
}); &state_sets,
state_sets
.iter()
.map(|map| {
store
.auth_event_ids(&room_id(), &map.values().cloned().collect::<Vec<_>>())
.unwrap()
})
.collect(),
|id| event_map.get(id).map(Arc::clone),
);
match resolved { match resolved {
Ok(state) => state, Ok(state) => state,
Err(e) => panic!("resolution for {} failed: {}", node, e), Err(e) => panic!("resolution for {} failed: {}", node, e),
@ -217,8 +227,12 @@ impl<E: Event> TestStore<E> {
} }
/// Returns a Vec of the related auth events to the given `event`. /// Returns a Vec of the related auth events to the given `event`.
pub fn auth_event_ids(&self, room_id: &RoomId, event_ids: &[EventId]) -> Result<Vec<EventId>> { pub fn auth_event_ids(
let mut result = vec![]; &self,
room_id: &RoomId,
event_ids: &[EventId],
) -> Result<BTreeSet<EventId>> {
let mut result = BTreeSet::new();
let mut stack = event_ids.to_vec(); let mut stack = event_ids.to_vec();
// DFS for auth event chain // DFS for auth event chain
@ -228,7 +242,7 @@ impl<E: Event> TestStore<E> {
continue; continue;
} }
result.push(ev_id.clone()); result.insert(ev_id.clone());
let event = self.get_event(room_id, &ev_id)?; let event = self.get_event(room_id, &ev_id)?;