Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove old consensus methods 2018 #45

Conversation

Labels
None yet
Projects
None yet
2 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Apr 9, 2018

No description provided.

nmathewson added 9 commits Apr 9, 2018
Consensus method 25 is the oldest one supported by any stable
version of 0.2.9, which is our current most-recent LTS.  Thus, by
proposal 290, they should be removed.

This commit does not actually remove the code to implement these
methods: it only makes it so authorities will no longer support
them.  I'll remove the backend code for them in later commits.
Also, in networkstatus.c, remove client code for recognizing pre-
MIN_METHOD_FOR_A_LINES consensuses, and corresponding unit tests in
test_dir.c.
Also remove a unit test for pre-MIN_METHOD_FOR_NTOR_KEY consensuses.
Also remove a rest for pre-19 microdesc versions.
This also lets us remove the old rsa-based routerstatus collator.
Also remove client detection for pre-EXCLUDING_INVALID_NODES
consensuses, and a test for that detection.
(Remove support for running without this method.)
@@ -3014,7 +3016,7 @@ gen_routerstatus_for_umbw(int idx, time_t now)
if (vrs) {
vrs->microdesc = tor_malloc_zero(sizeof(vote_microdesc_hash_t));
tor_asprintf(&vrs->microdesc->microdesc_hash_line,
"m 9,10,11,12,13,14,15,16,17 "
"m 13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28 "
Copy link
Contributor

@teor2345 teor2345 Apr 16, 2018

Should this list be 25-28?
21 is not a supported method, should it be added to the list?
After this branch, 13-24 aren't supported, should they be in the list?

Copy link
Contributor Author

@nmathewson nmathewson Apr 16, 2018

8420898 will fix this

@@ -3040,7 +3042,7 @@ vote_tweaks_for_umbw(networkstatus_t *v, int voter, time_t now)
smartlist_clear(v->supported_methods);
/* Method 17 is MIN_METHOD_TO_CLIP_UNMEASURED_BW_KB */
smartlist_split_string(v->supported_methods,
"1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17",
"13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28",
Copy link
Contributor

@teor2345 teor2345 Apr 16, 2018

Should this list also be 25-28?
(See my previous comment)

Copy link
Contributor Author

@nmathewson nmathewson Apr 16, 2018

8420898 will fix this too.

Copy link
Contributor

@teor2345 teor2345 left a comment

The commit title "Remove MIN_METHOD_FOR and MIN_METHOD_FOR_A_LINES" should probably read MIN_METHOD_FOR_{MANDATORY_MICRODESC,A_LINES}

@@ -1528,7 +1528,7 @@ networkstatus_consensus_has_ipv6(const or_options_t* options)
return
cons->consensus_method >= MIN_METHOD_FOR_A_LINES_IN_MICRODESC_CONSENSUS;
} else {
return cons->consensus_method >= MIN_METHOD_FOR_A_LINES;
return 1;
Copy link
Contributor

@teor2345 teor2345 Apr 16, 2018

I think it's ok for us to modify this client code. As of 1 May 2018, we will only support 0.2.9, which creates consensuses with method 25 and later.

And this is edge-case code. It is only useful:

  • on networks with consensus methods below 13 (no IPv6 addresses in the full consensus),
  • on clients using full descriptors (IPv6 addresses in descriptors),
  • with IPv6-supporting relays,
  • and IPv6-supporting fallbacks,
  • with an IPv6-only client.

has_ipv6 = networkstatus_consensus_has_ipv6(get_options());
tt_assert(has_ipv6);

mock_networkstatus->consensus_method = MIN_METHOD_FOR_A_LINES + 20;
Copy link
Contributor

@teor2345 teor2345 Apr 16, 2018

Please don't delete this one test.
Instead, replace MIN_METHOD_FOR_A_LINES + 20 with MIN_SUPPORTED_CONSENSUS_METHOD + 20.

Copy link
Contributor Author

@nmathewson nmathewson Apr 16, 2018

I think MAX_SUPPORTED_CONSENSUS_METHOD (for a far-future-method) would be even better?

Going with that in 427732a

nmathewson added 3 commits Apr 16, 2018
In tests, don't claim supported methods include 13...24.
SQUASH NOTE: Change title to:

Remove MIN_METHOD_FOR{MANDATORY_MICRODESC,A_LINES}.
Restore test for far-future consensus method.
@nmathewson
Copy link
Contributor Author

@nmathewson nmathewson commented May 7, 2018

This was squashed and merged as 0e8ae82

@nmathewson nmathewson closed this May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment