state-res: Make comments consistent and remove nested ifs

crates/ruma-state-res/src/event_auth.rs

@@ -173,7 +173,7 @@ pub fn auth_check<E: Event>(
 
     // [synapse] checks for federation here
 
-    // 4. if type is m.room.aliases
+    // 4. If type is m.room.aliases
     if incoming_event.kind() == EventType::RoomAliases && room_version.special_case_aliases_auth {
         info!("starting m.room.aliases check");
 
@@ -531,7 +531,7 @@ pub fn check_power_levels<E: Event>(
     let current_content =
         serde_json::from_value::<PowerLevelsEventContent>(current_state.content()).unwrap();
 
-    // validation of users is done in Ruma, synapse for loops validating user_ids and integers here
+    // Validation of users is done in Ruma, synapse for loops validating user_ids and integers here
     info!("validation of power event finished");
 
     let user_level = get_user_power_level(power_event.sender(), auth_events);
@@ -739,21 +739,13 @@ pub fn get_user_power_level<E: Event>(user_id: &UserId, auth_events: &StateMap<A
             0 // TODO if this fails DB error?
         }
     } else {
-        // if no power level event found the creator gets 100 everyone else gets 0
+        // If no power level event found the creator gets 100 everyone else gets 0
         let key = (EventType::RoomCreate, "".into());
-        if let Some(create) = auth_events.get(&key) {
+        auth_events
-            if let Ok(c) = serde_json::from_value::<CreateEventContent>(create.content()) {
+            .get(&key)
-                if &c.creator == user_id {
+            .and_then(|create| serde_json::from_value::<CreateEventContent>(create.content()).ok())
-                    100
+            .and_then(|create| (create.creator == *user_id).then(|| 100))
-                } else {
+            .unwrap_or_default()
-                    0
-                }
-            } else {
-                0
-            }
-        } else {
-            0
-        }
     }
 }
 
@@ -808,9 +800,10 @@ pub fn verify_third_party_invite<E: Event>(
     tp_id: &ThirdPartyInvite,
     current_third_party_invite: Option<Arc<E>>,
 ) -> bool {
-    // 1. check for user being banned happens before this is called
+    // 1. Check for user being banned happens before this is called
     // checking for mxid and token keys is done by ruma when deserializing
 
+    // The state key must match the invitee
     if user_state_key != Some(tp_id.signed.mxid.as_str()) {
         return false;
     }

crates/ruma-state-res/src/lib.rs

@@ -58,7 +58,7 @@ impl StateResolution {
     ) -> Result<StateMap<EventId>> {
         info!("State resolution starting");
 
-        // split non-conflicting and conflicting state
+        // Split non-conflicting and conflicting state
         let (clean, conflicting) = StateResolution::separate(state_sets);
 
         info!("non conflicting {:?}", clean.len());
@@ -70,34 +70,34 @@ impl StateResolution {
 
         info!("{} conflicting events", conflicting.len());
 
-        // 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 = StateResolution::get_auth_chain_diff(room_id, &auth_events)?;
 
         debug!("auth diff size {:?}", auth_diff);
 
-        // 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());
         let mut all_conflicted =
             auth_diff.into_iter().collect::<BTreeSet<_>>().into_iter().collect::<Vec<_>>();
 
         info!("full conflicted set is {} events", all_conflicted.len());
 
-        // we used to check that all events are events from the correct room
+        // We used to check that all events are events from the correct room
         // this is now a check the caller of `resolve` must make.
 
-        // synapse says `full_set = {eid for eid in full_conflicted_set if eid in event_map}`
+        // Synapse says `full_set = {eid for eid in full_conflicted_set if eid in event_map}`
         //
-        // don't honor events we cannot "verify"
+        // Don't honor events we cannot "verify"
         all_conflicted.retain(|id| event_map.contains_key(id));
 
-        // get only the control events with a state_key: "" or ban/kick event (sender != state_key)
+        // Get only the control events with a state_key: "" or ban/kick event (sender != state_key)
         let control_events = all_conflicted
             .iter()
             .filter(|id| is_power_event_id(id, event_map))
             .cloned()
             .collect::<Vec<_>>();
 
-        // sort the control events based on power_level/clock/event_id and outgoing/incoming edges
+        // Sort the control events based on power_level/clock/event_id and outgoing/incoming edges
         let mut sorted_control_levels = StateResolution::reverse_topological_power_sort(
             room_id,
             &control_events,
@@ -108,7 +108,7 @@ impl StateResolution {
         debug!("SRTD {:?}", sorted_control_levels);
 
         let room_version = RoomVersion::new(room_version)?;
-        // sequentially auth check each control event.
+        // Sequentially auth check each control event.
         let resolved_control = StateResolution::iterative_auth_check(
             room_id,
             &room_version,
@@ -158,7 +158,7 @@ impl StateResolution {
             event_map,
         )?;
 
-        // add unconflicted state to the resolved state
+        // Add unconflicted state to the resolved state
         // We priorities the unconflicting state
         resolved_state.extend(clean);
         Ok(resolved_state)
@@ -253,7 +253,7 @@ impl StateResolution {
             // tasks can make progress
         }
 
-        // this is used in the `key_fn` passed to the lexico_topo_sort fn
+        // This is used in the `key_fn` passed to the lexico_topo_sort fn
         let mut event_to_pl = BTreeMap::new();
         for event_id in graph.keys() {
             let pl = StateResolution::get_power_level_for_sender(room_id, event_id, event_map);
@@ -267,7 +267,6 @@ impl StateResolution {
         }
 
         StateResolution::lexicographical_topological_sort(&graph, |event_id| {
-            // debug!("{:?}", event_map.get(event_id).unwrap().origin_server_ts());
             let ev = event_map.get(event_id).unwrap();
             let pl = event_to_pl.get(event_id).unwrap();
 
@@ -312,7 +311,7 @@ impl StateResolution {
 
         for (node, edges) in graph.iter() {
             if edges.is_empty() {
-                // the `Reverse` is because rusts `BinaryHeap` sorts largest -> smallest we need
+                // The `Reverse` is because rusts `BinaryHeap` sorts largest -> smallest we need
                 // smallest -> largest
                 zero_outdegree.push(Reverse((key_fn(node), node)));
             }
@@ -325,16 +324,16 @@ impl StateResolution {
 
         let mut heap = BinaryHeap::from(zero_outdegree);
 
-        // we remove the oldest node (most incoming edges) and check against all other
+        // We remove the oldest node (most incoming edges) and check against all other
         let mut sorted = vec![];
-        // destructure the `Reverse` and take the smallest `node` each time
+        // Destructure the `Reverse` and take the smallest `node` each time
         while let Some(Reverse((_, node))) = heap.pop() {
             let node: &EventId = node;
             for parent in reverse_graph.get(node).unwrap() {
-                // the number of outgoing edges this node has
+                // The number of outgoing edges this node has
                 let out = outdegree_map.get_mut(parent).unwrap();
 
-                // only push on the heap once older events have been cleared
+                // Only push on the heap once older events have been cleared
                 out.remove(node);
                 if out.is_empty() {
                     heap.push(Reverse((key_fn(parent), parent)));
@@ -566,7 +565,7 @@ impl StateResolution {
             // tasks can make progress
         }
 
-        // sort the event_ids by their depth, timestamp and EventId
+        // Sort the event_ids by their depth, timestamp and EventId
         // unwrap is OK order map and sort_event_ids are from to_sort (the same Vec)
         let mut sort_event_ids = order_map.keys().map(|&k| k.clone()).collect::<Vec<_>>();
         sort_event_ids.sort_by_key(|sort_id| order_map.get(sort_id).unwrap());
@@ -589,11 +588,9 @@ impl StateResolution {
                 return Ok(*depth);
             }
 
-            // dbg!(&sort_ev);
             let auth_events = &sort_ev.auth_events();
             event = None;
             for aid in auth_events {
-                // dbg!(&aid);
                 let aev = StateResolution::get_or_load_event(room_id, aid, event_map)?;
                 if is_type_and_key(&aev, EventType::RoomPowerLevels, "") {
                     event = Some(aev);
@@ -614,11 +611,11 @@ impl StateResolution {
     ) {
         let mut state = vec![event_id.clone()];
         while !state.is_empty() {
-            // we just checked if it was empty so unwrap is fine
+            // We just checked if it was empty so unwrap is fine
             let eid = state.pop().unwrap();
             graph.entry(eid.clone()).or_insert_with(Vec::new);
-            // prefer the store to event as the store filters dedups the events
+            // Prefer the store to event as the store filters dedups the events
-            // otherwise it seems we can loop forever
+            // Otherwise it seems we can loop forever
             for aid in
                 &StateResolution::get_or_load_event(room_id, &eid, event_map).unwrap().auth_events()
             {
@@ -627,7 +624,7 @@ impl StateResolution {
                         state.push(aid.clone());
                     }
 
-                    // we just inserted this at the start of the while loop
+                    // We just inserted this at the start of the while loop
                     graph.get_mut(&eid).unwrap().push(aid.clone());
                 }
             }

crates/ruma-state-res/src/room_version.rs

@@ -35,7 +35,7 @@ pub struct RoomVersion {
     pub event_format: EventFormatVersion,
     /// Which state resolution algorithm is used.
     pub state_res: StateResolutionVersion,
-    /// not sure
+    // FIXME: not sure what this one means?
     pub enforce_key_validity: bool,
 
     /// `m.room.aliases` had special auth rules and redaction rules
@@ -43,7 +43,7 @@ pub struct RoomVersion {
     ///
     /// before MSC2261/MSC2432,
     pub special_case_aliases_auth: bool,
-    /// Strictly enforce canonicaljson, do not allow:
+    /// Strictly enforce canonical json, do not allow:
     /// * Integers outside the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]
     /// * Floats
     /// * NaN, Infinity, -Infinity

crates/ruma-state-res/src/state_event.rs

@@ -46,7 +46,10 @@ pub trait Event {
     /// The `unsigned` content of this event.
     fn unsigned(&self) -> &BTreeMap<String, JsonValue>;
 
+    /// The content hash of this PDU.
     fn hashes(&self) -> &EventHash;
 
+    /// A map of server names to another map consisting of the signing key id and finally the
+    /// signature.
     fn signatures(&self) -> BTreeMap<Box<ServerName>, BTreeMap<ServerSigningKeyId, String>>;
 }

crates/ruma-state-res/tests/utils.rs

@@ -49,10 +49,10 @@ pub fn do_check(
 
     // This will be lexi_topo_sorted for resolution
     let mut graph = BTreeMap::new();
-    // this is the same as in `resolve` event_id -> StateEvent
+    // This is the same as in `resolve` event_id -> StateEvent
     let mut fake_event_map = BTreeMap::new();
 
-    // create the DB of events that led up to this point
+    // Create the DB of events that led up to this point
     // TODO maybe clean up some of these clones it is just tests but...
     for ev in init_events.values().chain(events) {
         graph.insert(ev.event_id().clone(), vec![]);
@@ -78,7 +78,7 @@ pub fn do_check(
     // event_id -> StateMap<EventId>
     let mut state_at_event: BTreeMap<EventId, StateMap<EventId>> = BTreeMap::new();
 
-    // resolve the current state and add it to the state_at_event map then continue
+    // Resolve the current state and add it to the state_at_event map then continue
     // on in "time"
     for node in StateResolution::lexicographical_topological_sort(&graph, |id| {
         (0, MilliSecondsSinceUnixEpoch(uint!(0)), id.clone())
@@ -164,7 +164,7 @@ pub fn do_check(
             prev_events,
         );
 
-        // we have to update our store, an actual user of this lib would
+        // We have to update our store, an actual user of this lib would
         // be giving us state from a DB.
         store.0.insert(ev_id.clone(), event.clone());
 
@@ -197,7 +197,7 @@ pub fn do_check(
                 // Filter out the dummy messages events.
                 // These act as points in time where there should be a known state to
                 // test against.
-                && k != &&(EventType::RoomMessage, "dummy".to_string())
+                && **k != (EventType::RoomMessage, "dummy".to_string())
         })
         .map(|(k, v)| (k.clone(), v.clone()))
         .collect::<StateMap<EventId>>();