Re: [tor-bugs] #19877 [Core Tor/Tor]: Implement new guard selection algorithm of prop 271

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#19877: Implement new guard selection algorithm of prop 271
-------------------------------------------------+-------------------------
 Reporter:  andrea                               |          Owner:  nickm
     Type:  task                                 |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.0.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  unspecified
 Severity:  Normal                               |     Resolution:
 Keywords:  isaremoved, nickwants029, tor-       |  Actual Points:
  guards-revamp, TorCoreTeam201611, review-      |
  group-13, actual-review-points=2               |
Parent ID:                                       |         Points:
 Reviewer:  chelseakomlo, asn                    |        Sponsor:
                                                 |  SponsorU-must
-------------------------------------------------+-------------------------

Comment (by chelseakomlo):

 Hey Nick,

 Here are some general thoughts, feel free to take/leave what is useful. I
 added notes to the gitlab review as well.

 - Naming conventions are a bit confusing, as we use entrynodes,
 entryguards, and guards. If this naming signifies pre and post prop271,
 maybe we can add in legacy namespacing to make it more clear (see below).
 Also, the bridge-specific module looks really great- if there is guard-
 specific code that is distinct from entrynodes, maybe this belongs in a
 separate guard module.

 - Making a clear distinction between pre and post prop271 code would
 definitely make reviewing easier, as well as eventually migrating away
 from legacy code... I liked asn's recommendation of namespacing, we could
 also move legacy code to it's own module, etc.


 - Some functions are quite large, such as
 sampled_guards_update_from_consensus and
 entry_guards_upgrade_waiting_circuits. I see some todos about splitting
 these up, which is great.

 - Would it make sense to put parsing code into a parser.c?

 - I didn't see a lot of tests here:
 https://gitlab.com/nickm_tor/tor/merge_requests/11 - is there another set
 of commits for these?

 - bridges.c doesn't have test coverage :(

 - It looks like there are several remaining todos that need to be
 completed. Which of these should be completed as part of this patch? For
 example, in entrynodes.c, update_guard_selection_choice has a comment
 about needing to expire existing circuits (line 653).

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19877#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs