Merge branch 'ticket14881-v3'

4 files changed, 258 insertions, 3 deletions

diff --git a/changes/14881 b/changes/14881
new file mode 100644
index 0000000000..e686c4d982
--- /dev/null
+++ b/changes/14881
@@ -0,0 +1,7 @@
+  o Minor bugfixes (consensus weight):
+    - Add new consensus method that initializes bw weights to 1 instead of 0. This
+      prevents a zero weight from making it all the way to the end (happens in
+      small testing networks) and causing an error. Fixes bug 14881.
+
+  o Minor features (unit tests):
+    - Add tests for networkstatus_compute_bw_weights_v10
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index fe664698bc..d44ed7f8c1 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -913,7 +913,7 @@ networkstatus_check_weights(int64_t Wgg, int64_t Wgd, int64_t Wmg,
  *
  * It returns true if weights could be computed, false otherwise.
  */
-static int
+int
 networkstatus_compute_bw_weights_v10(smartlist_t *chunks, int64_t G,
                                      int64_t M, int64_t E, int64_t D,
                                      int64_t T, int64_t weight_scale)
@@ -1335,7 +1335,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
   smartlist_t *flags;
   const char *flavor_name;
   uint32_t max_unmeasured_bw_kb = DEFAULT_MAX_UNMEASURED_BW_KB;
-  int64_t G=0, M=0, E=0, D=0, T=0; /* For bandwidth weights */
+  int64_t G, M, E, D, T; /* For bandwidth weights */
   const routerstatus_format_type_t rs_format =
     flavor == FLAV_NS ? NS_V3_CONSENSUS : NS_V3_CONSENSUS_MICRODESC;
   char *params = NULL;
@@ -1367,6 +1367,16 @@ networkstatus_compute_consensus(smartlist_t *votes,
     consensus_method = MAX_SUPPORTED_CONSENSUS_METHOD;
   }
 
+  if (consensus_method >= MIN_METHOD_FOR_INIT_BW_WEIGHTS_ONE) {
+    /* It's smarter to initialize these weights to 1, so that later on,
+     * we can't accidentally divide by zero. */
+    G = M = E = D = 1;
+    T = 4;
+  } else {
+    /* ...but originally, they were set to zero. */
+    G = M = E = D = T = 0;
+  }
+
   /* Compute medians of time-related things, and figure out how many
    * routers we might need to talk about. */
   {
diff --git a/src/or/dirvote.h b/src/or/dirvote.h
index efd233ef5f..ac7db69db2 100644
--- a/src/or/dirvote.h
+++ b/src/or/dirvote.h
@@ -55,7 +55,7 @@
 #define MIN_SUPPORTED_CONSENSUS_METHOD 13
 
 /** The highest consensus method that we currently support. */
-#define MAX_SUPPORTED_CONSENSUS_METHOD 25
+#define MAX_SUPPORTED_CONSENSUS_METHOD 26
 
 /** Lowest consensus method where microdesc consensuses omit any entry
  * with no microdesc. */
@@ -111,6 +111,10 @@
  * entries. */
 #define MIN_METHOD_FOR_RS_PROTOCOLS 25
 
+/** Lowest consensus method where authorities initialize bandwidth weights to 1
+ * instead of 0. See #14881 */
+#define MIN_METHOD_FOR_INIT_BW_WEIGHTS_ONE 26
+
 /** Default bandwidth to clip unmeasured bandwidths to using method >=
  * MIN_METHOD_TO_CLIP_UNMEASURED_BW.  (This is not a consensus method; do not
  * get confused with the above macros.) */
@@ -234,6 +238,10 @@ STATIC smartlist_t *dirvote_compute_params(smartlist_t *votes, int method,
                              int total_authorities);
 STATIC char *compute_consensus_package_lines(smartlist_t *votes);
 STATIC char *make_consensus_method_list(int low, int high, const char *sep);
+STATIC int
+networkstatus_compute_bw_weights_v10(smartlist_t *chunks, int64_t G,
+                                     int64_t M, int64_t E, int64_t D,
+                                     int64_t T, int64_t weight_scale);
 #endif
 
 #endif
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 419aa249a7..788ec617ff 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -1874,6 +1874,235 @@ test_routerstatus_for_v3ns(routerstatus_t *rs, time_t now)
   return;
 }
 
+static void
+test_dir_networkstatus_compute_bw_weights_v10(void *arg)
+{
+  (void) arg;
+  smartlist_t *chunks = smartlist_new();
+  int64_t G, M, E, D, T, weight_scale;
+  int ret;
+  weight_scale = 10000;
+
+  /* no case. one or more of the values is 0 */
+  G = M = E = D = 0;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(smartlist_len(chunks), OP_EQ, 0);
+
+  /* case 1 */
+  /* XXX dir-spec not followed? See #20272. If it isn't closed, then this is
+   * testing current behavior, not spec. */
+  G = E = 10;
+  M = D = 1;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_int_op(smartlist_len(chunks), OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=3333 "
+    "Wbe=3000 Wbg=3000 Wbm=10000 Wdb=10000 Web=10000 Wed=3333 Wee=7000 "
+    "Weg=3333 Wem=7000 Wgb=10000 Wgd=3333 Wgg=7000 Wgm=7000 Wmb=10000 "
+    "Wmd=3333 Wme=3000 Wmg=3000 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 2a E scarce */
+  M = 100;
+  G = 20;
+  E = D = 5;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
+    "Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 "
+    "Wem=10000 Wgb=10000 Wgd=0 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=0 Wme=0 "
+    "Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 2a G scarce */
+  M = 100;
+  E = 20;
+  G = D = 5;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
+    "Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=0 Wee=10000 Weg=0 Wem=10000 "
+    "Wgb=10000 Wgd=10000 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=0 Wme=0 Wmg=0 "
+    "Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 2b1 (Wgg=1, Wmd=Wgd) */
+  M = 10;
+  E = 30;
+  G = 10;
+  D = 100;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=4000 "
+    "Wbe=0 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=2000 Wee=10000 Weg=2000 "
+    "Wem=10000 Wgb=10000 Wgd=4000 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=4000 "
+    "Wme=0 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 2b2 */
+  M = 60;
+  E = 30;
+  G = 10;
+  D = 100;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=666 Wbe=0 "
+    "Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=3666 Wee=10000 Weg=3666 "
+    "Wem=10000 Wgb=10000 Wgd=5668 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=666 "
+    "Wme=0 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 2b3 */
+  /* XXX I can't get a combination of values that hits this case without error,
+   * so this just tests that it fails. See #20285. Also see #20284 as 2b3 does
+   * not follow dir-spec. */
+  /* (E < T/3 && G < T/3) && (E+D>=G || G+D>=E) && (M > T/3) */
+  M = 80;
+  E = 30;
+  G = 30;
+  D = 30;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 0);
+  smartlist_clear(chunks);
+
+  /* case 3a G scarce */
+  M = 10;
+  E = 30;
+  G = 10;
+  D = 5;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 "
+    "Wbe=3333 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=0 Wee=6667 Weg=0 "
+    "Wem=6667 Wgb=10000 Wgd=10000 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=0 "
+    "Wme=3333 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 3a E scarce */
+  M = 10;
+  E = 10;
+  G = 30;
+  D = 5;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
+    "Wbg=3333 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 "
+    "Wem=10000 Wgb=10000 Wgd=0 Wgg=6667 Wgm=6667 Wmb=10000 Wmd=0 Wme=0 "
+    "Wmg=3333 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 3bg */
+  M = 10;
+  E = 30;
+  G = 10;
+  D = 10;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 "
+    "Wbe=3334 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=0 Wee=6666 Weg=0 "
+    "Wem=6666 Wgb=10000 Wgd=10000 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=0 "
+    "Wme=3334 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case 3be */
+  M = 10;
+  E = 10;
+  G = 30;
+  D = 10;
+  T = G + M + E + D;
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_int_op(ret, OP_EQ, 1);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
+    "Wbg=3334 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 "
+    "Wem=10000 Wgb=10000 Wgd=0 Wgg=6666 Wgm=6666 Wmb=10000 Wmd=0 Wme=0 "
+    "Wmg=3334 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case from 21 Jul 2013 (3be) */
+  G = 5483409;
+  M = 1455379;
+  E = 980834;
+  D = 3385803;
+  T = 11305425;
+  tt_int_op(G+M+E+D, OP_EQ, T);
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=883 Wbe=0 "
+    "Wbg=3673 Wbm=10000 Wdb=10000 Web=10000 Wed=8233 Wee=10000 Weg=8233 "
+    "Wem=10000 Wgb=10000 Wgd=883 Wgg=6327 Wgm=6327 Wmb=10000 Wmd=883 Wme=0 "
+    "Wmg=3673 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case from 04 Oct 2016 (3a E scarce) */
+  G=29322240;
+  M=4721546;
+  E=1522058;
+  D=9273571;
+  T=44839415;
+  tt_int_op(G+M+E+D, OP_EQ, T);
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
+    "Wbg=4194 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 "
+    "Wem=10000 Wgb=10000 Wgd=0 Wgg=5806 Wgm=5806 Wmb=10000 Wmd=0 Wme=0 "
+    "Wmg=4194 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* case from 04 Sep 2013 (2b1) */
+  G=3091352;
+  M=1838837;
+  E=2109300;
+  D=2469369;
+  T=9508858;
+  tt_int_op(G+M+E+D, OP_EQ, T);
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=317 "
+    "Wbe=5938 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=9366 Wee=4061 "
+    "Weg=9366 Wem=4061 Wgb=10000 Wgd=317 Wgg=10000 Wgm=10000 Wmb=10000 "
+    "Wmd=317 Wme=5938 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+
+  /* explicitly test initializing weights to 1*/
+  G=1;
+  M=1;
+  E=1;
+  D=1;
+  T=4;
+  tt_int_op(G+M+E+D, OP_EQ, T);
+  ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
+                                             weight_scale);
+  tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=3333 "
+    "Wbe=0 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=3333 Wee=10000 Weg=3333 "
+    "Wem=10000 Wgb=10000 Wgd=3333 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=3333 "
+    "Wme=0 Wmg=0 Wmm=10000\n");
+  smartlist_clear(chunks);
+ done:
+  return;
+}
+
 static authority_cert_t *mock_cert;
 
 static authority_cert_t *
@@ -5555,6 +5784,7 @@ struct testcase_t dir_tests[] = {
   DIR_ARG(find_dl_schedule, TT_FORK, "cf"),
   DIR_ARG(find_dl_schedule, TT_FORK, "ca"),
   DIR(assumed_flags, 0),
+  DIR(networkstatus_compute_bw_weights_v10, 0),
   END_OF_TESTCASES
 };