prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements

Signed-off-by: David Goulet <dgoulet@torproject.org>

src/or/dirvote.c

@@ -1341,8 +1341,16 @@ networkstatus_compute_consensus(smartlist_t *votes,
   }
 
   if (consensus_method >= MIN_METHOD_FOR_SHARED_RANDOM) {
+    int num_dirauth = get_n_authorities(V3_DIRINFO);
+    /* Default value of this is 2/3 of the total number of authorities. For
+     * instance, if we have 9 dirauth, the default value is 6. The following
+     * calculation will round it down. */
+    int32_t num_srv_agreements =
+      dirvote_get_intermediate_param_value(param_list,
+                                           "AuthDirNumSRVAgreements",
+                                           (num_dirauth * 2) / 3);
     /* Add the shared random value. */
-    char *srv_lines = sr_get_string_for_consensus(votes);
+    char *srv_lines = sr_get_string_for_consensus(votes, num_srv_agreements);
     if (srv_lines != NULL) {
       smartlist_add(chunks, srv_lines);
     }

src/or/shared_random.c

@@ -105,6 +105,12 @@ static const char current_srv_str[] = "shared-rand-current-value";
 static const char commit_ns_str[] = "shared-rand-commit";
 static const char sr_flag_ns_str[] = "shared-rand-participate";
 
+/* The value of the consensus param AuthDirNumSRVAgreements found in the
+ * vote. This is set once the consensus creation subsystem requests the
+ * SRV(s) that should be put in the consensus. We use this value to decide
+ * if we keep or not an SRV. */
+static int32_t num_srv_agreements_from_vote;
+
 /* Return a heap allocated copy of the SRV <b>orig</b>. */
 STATIC sr_srv_t *
 srv_dup(const sr_srv_t *orig)
@@ -737,18 +743,6 @@ save_commit_to_state(sr_commit_t *commit)
   }
 }
 
-/* Return the number of required participants of the SR protocol. This is based
- * on a consensus params. */
-static int
-get_n_voters_for_srv_agreement(void)
-{
-  int num_dirauths = get_n_authorities(V3_DIRINFO);
-  /* If the params is not found, default value should always be the maximum
-   * number of trusted authorities. Let's not take any chances. */
-  return networkstatus_get_param(NULL, "AuthDirNumSRVAgreements",
-                                 num_dirauths, 1, num_dirauths);
-}
-
 /* Return 1 if we should we keep an SRV voted by <b>n_agreements</b> auths.
  * Return 0 if we should ignore it. */
 static int
@@ -769,11 +763,9 @@ should_keep_srv(int n_agreements)
    * to keep it. */
   if (sr_state_srv_is_fresh()) {
     /* Check if we have super majority for this new SRV value. */
-    int num_required_agreements = get_n_voters_for_srv_agreement();
-
-    if (n_agreements < num_required_agreements) {
+    if (n_agreements < num_srv_agreements_from_vote) {
       log_notice(LD_DIR, "SR: New SRV didn't reach agreement [%d/%d]!",
-                 n_agreements, num_required_agreements);
+                 n_agreements, num_srv_agreements_from_vote);
       return 0;
     }
   }
@@ -1253,9 +1245,14 @@ sr_get_string_for_vote(void)
  *
  * This is called when a consensus (any flavor) is bring created thus it
  * should NEVER change the state nor the state should be changed in between
- * consensus creation. */
+ * consensus creation.
+ *
+ * <b>num_srv_agreements</b> is taken from the votes thus the voted value
+ * that should be used.
+ * */
 char *
-sr_get_string_for_consensus(const smartlist_t *votes)
+sr_get_string_for_consensus(const smartlist_t *votes,
+                            int32_t num_srv_agreements)
 {
   char *srv_str;
   const or_options_t *options = get_options();
@@ -1269,6 +1266,9 @@ sr_get_string_for_consensus(const smartlist_t *votes)
     goto end;
   }
 
+  /* Set the global value of AuthDirNumSRVAgreements found in the votes. */
+  num_srv_agreements_from_vote = num_srv_agreements;
+
   /* Check the votes and figure out if SRVs should be included in the final
    * consensus. */
   sr_srv_t *prev_srv = get_majority_srv_from_votes(votes, 0);
@@ -1340,3 +1340,15 @@ sr_save_and_cleanup(void)
   sr_state_save();
   sr_cleanup();
 }
+
+#ifdef TOR_UNIT_TESTS
+
+/* Set the global value of number of SRV agreements so the test can play
+ * along by calling specific functions that don't parse the votes prior for
+ * the AuthDirNumSRVAgreements value. */
+void set_num_srv_agreements(int32_t value)
+{
+  num_srv_agreements_from_vote = value;
+}
+
+#endif /* TOR_UNIT_TESTS */

src/or/shared_random.h

@@ -112,7 +112,8 @@ void sr_handle_received_commits(smartlist_t *commits,
 sr_commit_t *sr_parse_commit(const smartlist_t *args);
 sr_srv_t *sr_parse_srv(const smartlist_t *args);
 char *sr_get_string_for_vote(void);
-char *sr_get_string_for_consensus(const smartlist_t *votes);
+char *sr_get_string_for_consensus(const smartlist_t *votes,
+                                  int32_t num_srv_agreements);
 void sr_commit_free(sr_commit_t *commit);
 void sr_srv_encode(char *dst, size_t dst_len, const sr_srv_t *srv);
 
@@ -156,4 +157,10 @@ STATIC void save_commit_during_reveal_phase(const sr_commit_t *commit);
 
 #endif /* SHARED_RANDOM_PRIVATE */
 
+#ifdef TOR_UNIT_TESTS
+
+void set_num_srv_agreements(int32_t value);
+
+#endif /* TOR_UNIT_TESTS */
+
 #endif /* TOR_SHARED_RANDOM_H */

src/test/test_shared_random.c

@@ -830,14 +830,6 @@ get_test_vote_with_curr_srv(const char *srv)
   return vote;
 }
 
-/* Mock function to return the value located in the options instead of the
- * consensus so we can modify it at will. */
-static networkstatus_t *
-mock_networkstatus_get_latest_consensus(void)
-{
-  return mock_consensus;
-}
-
 /* Test the function that picks the right SRV given a bunch of votes. Make sure
  * that the function returns an SRV iff the majority/agreement requirements are
  * met. */
@@ -894,31 +886,13 @@ test_sr_get_majority_srv_from_votes(void *arg)
 
   /* Now we achieve majority for SRV_1, but not the AuthDirNumSRVAgreements
      requirement. So still not picking an SRV. */
+  set_num_srv_agreements(8);
   chosen_srv = get_majority_srv_from_votes(votes, 1);
   tt_assert(!chosen_srv);
 
   /* We will now lower the AuthDirNumSRVAgreements requirement by tweaking the
-   * consensus parameter and we will try again.  This time it should work. */
-  {
-    char *my_net_params;
-    /* Set a dummy consensus parameter in every vote */
-    SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, vote) {
-      vote->net_params = smartlist_new();
-      smartlist_split_string(vote->net_params,
-                             "AuthDirNumSRVAgreements=7", NULL, 0, 0);
-    } SMARTLIST_FOREACH_END(vote);
-
-    /* Pretend you are making a consensus out of the votes */
-    mock_consensus = tor_malloc_zero(sizeof(networkstatus_t));
-    mock_consensus->net_params = smartlist_new();
-    my_net_params = dirvote_compute_params(votes, 66, smartlist_len(votes));
-    smartlist_split_string(mock_consensus->net_params, my_net_params, " ",
-                           SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  }
-
-  /* Use our fake consensus for finding consensus parameters. */
-  MOCK(networkstatus_get_latest_consensus,
-       mock_networkstatus_get_latest_consensus);
+   * consensus parameter and we will try again. This time it should work. */
+  set_num_srv_agreements(7);
   chosen_srv = get_majority_srv_from_votes(votes, 1);
   tt_assert(chosen_srv);
   tt_int_op(chosen_srv->num_reveals, ==, 42);
@@ -928,8 +902,6 @@ test_sr_get_majority_srv_from_votes(void *arg)
   SMARTLIST_FOREACH(votes, networkstatus_t *, vote,
                     networkstatus_vote_free(vote));
   smartlist_free(votes);
-  networkstatus_vote_free(mock_consensus);
-  UNMOCK(networkstatus_get_latest_consensus);
 }
 
 static void