Merge branch 'ticket28563_rebased_ticket28565'

Conflicts:
	sbws/lib/v3bwfile.py
	tests/unit/lib/test_v3bwfile.py

sbws/lib/v3bwfile.py

@@ -121,6 +121,15 @@ BW_KEYVALUES_BASIC = ['node_id', 'bw']
 BW_KEYVALUES_FILE = BW_KEYVALUES_BASIC + \
                     ['master_key_ed25519', 'nick', 'rtt', 'time',
                      'success', 'error_stream', 'error_circ', 'error_misc',
+                     # `vote=0` is used for the relays that were excluded to
+                     # be reported in the bandwidth file and now they are
+                     # reported.
+                     # It tells Tor to do not vote on the relay.
+                     # `unmeasured=1` is used for the same relays and it is
+                     # added in case Tor would vote on them in future versions.
+                     # Maybe these keys should not be included for the relays
+                     # in which vote=1 and unmeasured=0.
+                     'vote', 'unmeasured',
                      # Added in #292951
                      'error_second_relay', 'error_destination']
 BW_KEYVALUES_EXTRA_BWS = ['bw_median', 'bw_mean', 'desc_bw_avg', 'desc_bw_bur',
@@ -168,8 +177,11 @@ BANDWIDTH_LINE_KEY_VALUES_MONITOR = [
 ]
 BW_KEYVALUES_EXTRA = BW_KEYVALUES_FILE + BW_KEYVALUES_EXTRA_BWS \
                + BANDWIDTH_LINE_KEY_VALUES_MONITOR
+# NOTE: tech-debt: assign boolean type to vote and unmeasured,
+# when the attributes are defined with a type, as stem does.
 BW_KEYVALUES_INT = ['bw', 'rtt', 'success', 'error_stream',
-                    'error_circ', 'error_misc'] + BW_KEYVALUES_EXTRA_BWS \
+                    'error_circ', 'error_misc', 'vote', 'unmeasured'] \
+                   + BW_KEYVALUES_EXTRA_BWS \
                    + BANDWIDTH_LINE_KEY_VALUES_MONITOR
 BW_KEYVALUES = BW_KEYVALUES_BASIC + BW_KEYVALUES_EXTRA
 
@@ -522,22 +534,16 @@ class V3BWLine(object):
     """
     Create a Bandwidth List line following the spec version 1.X.X.
 
-    :param str node_id:
-    :param int bw:
-    :param dict kwargs: extra headers. Currently supported:
+    :param str node_id: the relay fingerprint
+    :param int bw: the bandwidth value that directory authorities will include
+        in their votes.
+    :param dict kwargs: extra headers.
 
-        - nickname, str
-        - master_key_ed25519, str
-        - rtt, int
-        - time, str
-        - sucess, int
-        - error_stream, int
-        - error_circ, int
-        - error_misc, int
+    .. note:: tech-debt: move node_id and bw to kwargs and just ensure that
+       the required values are in **kwargs
     """
     def __init__(self, node_id, bw, **kwargs):
         assert isinstance(node_id, str)
-        assert isinstance(bw, int)
         assert node_id.startswith('$')
         self.node_id = node_id
         self.bw = bw
@@ -601,38 +607,69 @@ class V3BWLine(object):
         # In #28563 we report these relays, but make Tor ignore them.
         # This might confirm #28042.
 
+        # If the relay is non-`eligible`:
+        # Create a bandwidth line with the relay, but set ``vote=0`` so that
+        # Tor versions with patch #29806 does not vote on the relay.
+        # Set ``bw=1`` so that Tor versions without the patch,
+        # will give the relay low bandwidth.
+        # Include ``unmeasured=1`` in case Tor would vote on unmeasured relays
+        # in future versions.
+        # And return because there are not bandwidth values.
+        # NOTE: the bandwidth values could still be obtained if:
+        # 1. ``ResultError`` will store them
+        # 2. assign ``results_recent = results`` when there is a ``exclusion
+        # reason.
+        # This could be done in a better way as part of a refactor #28684.
+
+        kwargs['vote'] = '0'
+        kwargs['unmeasured'] = '1'
+
+        exclusion_reason = None
+
         number_excluded_error = len(results) - len(success_results)
         if number_excluded_error > 0:
             # then the number of error results is the number of results
             kwargs['relay_recent_measurements_excluded_error_count'] = \
                 number_excluded_error
         if not success_results:
-            return None, 'recent_measurements_excluded_error_count'
+            exclusion_reason = 'recent_measurements_excluded_error_count'
+            return (cls(node_id, 1, **kwargs), exclusion_reason)
 
         results_away = \
             cls.results_away_each_other(success_results, secs_away)
         number_excluded_near = len(success_results) - len(results_away)
         if number_excluded_near > 0:
             kwargs['relay_recent_measurements_excluded_near_count'] = \
-                len(success_results) - len(results_away)
+                number_excluded_near
         if not results_away:
-            return None, 'recent_measurements_excluded_near_count'
-
+            exclusion_reason = \
+                'recent_measurements_excluded_near_count'
+            return (cls(node_id, 1, **kwargs), exclusion_reason)
         # log.debug("Results away from each other: %s",
         #           [unixts_to_isodt_str(r.time) for r in results_away])
+
         results_recent = cls.results_recent_than(results_away, secs_recent)
         number_excluded_old = len(results_away) - len(results_recent)
         if number_excluded_old > 0:
             kwargs['relay_recent_measurements_excluded_old_count'] = \
                 number_excluded_old
         if not results_recent:
-            return None, 'recent_measurements_excluded_old_count'
+            exclusion_reason = \
+                'recent_measurements_excluded_old_count'
+            return (cls(node_id, 1, **kwargs), exclusion_reason)
 
         if not len(results_recent) >= min_num:
             kwargs['relay_recent_measurements_excluded_few_count'] = \
                 len(results_recent)
             # log.debug('The number of results is less than %s', min_num)
-            return None, 'recent_measurements_excluded_few_count'
+            exclusion_reason = \
+                'recent_measurements_excluded_few_count'
+            return (cls(node_id, 1, **kwargs), exclusion_reason)
+
+        # For any line not excluded, do not include vote and unmeasured
+        # KeyValues
+        del kwargs['vote']
+        del kwargs['unmeasured']
 
         rtt = cls.rtt_from_results(results_recent)
         if rtt:
@@ -859,6 +896,7 @@ class V3BWFile(object):
         header = V3BWHeader.from_results(results, scanner_country,
                                          destinations_countries, state_fpath)
         bw_lines_raw = []
+        bw_lines_excluded = []
         number_consensus_relays = cls.read_number_consensus_relays(
             consensus_path)
         state = State(state_fpath)
@@ -875,9 +913,13 @@ class V3BWFile(object):
             # log.debug("Relay fp %s", fp)
             line, reason = V3BWLine.from_results(values, secs_recent,
                                                  secs_away, min_num)
-            if line is not None:
+            # If there is no reason it means the line will not be excluded.
+            if not reason:
                 bw_lines_raw.append(line)
             else:
+                # Store the excluded lines to include them in the bandwidth
+                # file.
+                bw_lines_excluded.append(line)
                 exclusion_dict[reason] = exclusion_dict.get(reason, 0) + 1
         # Add the headers with the number of excluded relays by reason
         header.add_relays_excluded_counters(exclusion_dict)
@@ -887,7 +929,9 @@ class V3BWFile(object):
                      "there is not any. Scaling can not be applied.")
             cls.update_progress(
                 cls, bw_lines_raw, header, number_consensus_relays, state)
-            return cls(header, [])
+            # Create the bandwidth file with the excluded lines that does not
+            # have ``bw`` attribute
+            return cls(header, bw_lines_excluded)
         if scaling_method == SBWS_SCALING:
             bw_lines = cls.bw_sbws_scale(bw_lines_raw, scale_constant)
             cls.warn_if_not_accurate_enough(bw_lines, scale_constant)
@@ -904,7 +948,7 @@ class V3BWFile(object):
         # Not using the result for now, just warning
         cls.is_max_bw_diff_perc_reached(bw_lines, max_bw_diff_perc)
         header.add_time_report_half_network()
-        f = cls(header, bw_lines)
+        f = cls(header, bw_lines + bw_lines_excluded)
         return f
 
     @classmethod
@@ -1266,7 +1310,7 @@ class V3BWFile(object):
 
     @property
     def sum_bw(self):
-        return sum([l.bw for l in self.bw_lines])
+        return sum([l.bw for l in self.bw_lines if hasattr(l, 'bw')])
 
     @property
     def num(self):
@@ -1274,19 +1318,19 @@ class V3BWFile(object):
 
     @property
     def mean_bw(self):
-        return mean([l.bw for l in self.bw_lines])
+        return mean([l.bw for l in self.bw_lines if hasattr(l, 'bw')])
 
     @property
     def median_bw(self):
-        return median([l.bw for l in self.bw_lines])
+        return median([l.bw for l in self.bw_lines if hasattr(l, 'bw')])
 
     @property
     def max_bw(self):
-        return max([l.bw for l in self.bw_lines])
+        return max([l.bw for l in self.bw_lines if hasattr(l, 'bw')])
 
     @property
     def min_bw(self):
-        return min([l.bw for l in self.bw_lines])
+        return min([l.bw for l in self.bw_lines if hasattr(l, 'bw')])
 
     @property
     def info_stats(self):

tests/unit/lib/test_v3bwfile.py

@@ -56,6 +56,9 @@ header_extra_ls = [timestamp_l, version_l,
 header_extra_str = LINE_SEP.join(header_extra_ls) + LINE_SEP
 
 # Line produced without any scaling.
+# unmeasured and vote are not congruent with the exclusion,
+# but `from_data` is only used in the test and doesn't include the
+# arg `min_num`
 raw_bwl_str = "bw=56 bw_mean=61423 bw_median=55656 "\
     "consensus_bandwidth=600000 consensus_bandwidth_is_unmeasured=False "\
     "desc_bw_avg=1000000000 desc_bw_bur=123456 desc_bw_obs_last=524288 "\
@@ -323,6 +326,8 @@ def test_results_away_each_other(datadir):
     bwl, reason = V3BWLine.from_results(values, secs_away=secs_away, min_num=2)
     assert bwl.relay_recent_measurements_excluded_error_count == 1
     assert reason is None
+    assert not hasattr(bwl, "vote")
+    assert not hasattr(bwl, "unmeasured")
 
     success_results = [r for r in values if isinstance(r, ResultSuccess)]
     assert len(success_results) >= min_num
@@ -335,28 +340,32 @@ def test_results_away_each_other(datadir):
     # Two measurements are excluded and there were only 2,
     # the relay is excluded
     bwl, reason = V3BWLine.from_results(values, secs_away=secs_away, min_num=2)
-    # TODO ticket28563: uncomment
-    # assert bwl.relay_recent_measurements_excluded_near_count == 2
+    assert bwl.relay_recent_measurements_excluded_near_count == 2
     assert reason == 'recent_measurements_excluded_near_count'
+    assert bwl.vote == '0'
+    assert bwl.unmeasured == '1'
 
     success_results = [r for r in values if isinstance(r, ResultSuccess)]
     assert len(success_results) >= min_num
     results_away = V3BWLine.results_away_each_other(success_results, secs_away)
     assert not results_away
+
     secs_away = 43200  # 12h
     values = results["BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"]
     success_results = [r for r in values if isinstance(r, ResultSuccess)]
     assert len(success_results) >= min_num
     results_away = V3BWLine.results_away_each_other(success_results, secs_away)
     assert len(results_away) == 2
+
     # C has 1 result
     values = results["CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC"]
 
     # There is only 1 result, the relay is excluded
     bwl, reason = V3BWLine.from_results(values, min_num=2)
-    # TODO ticket28563: uncomment
-    # assert bwl.recent_measurements_excluded_few_count == 1
+    assert bwl.relay_recent_measurements_excluded_few_count == 1
     assert reason == 'recent_measurements_excluded_few_count'
+    assert bwl.vote == '0'
+    assert bwl.unmeasured == '1'
 
     success_results = [r for r in values if isinstance(r, ResultSuccess)]
     assert len(success_results) < min_num