#32088: Proposal 310 - choose guards in sampled order --------------------------------------+------------------------------------ Reporter: Jaym | Owner: (none) Type: enhancement | Status: needs_review Priority: High | Milestone: Tor: 0.4.4.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: tor-spec prop271 prop310 | Actual Points: Parent ID: | Points: Reviewer: nickm, asn | Sponsor: --------------------------------------+------------------------------------
Comment (by asn): Hey thanks for the super fast revisions here! I left a comment on the new PR. Furthermore, I'd be more confidence if we had some more testing here. The test coverage of the branch in terms of LoC is great, but I would also like a test that tests the entire logic of this new feature. In particular, given that the changes are far from trivial, and change various behaviors in the code, I would like a test that tests most of that. In particular, I would be looking for something like: Load some guards to the sampled set (without using a state file), make sure that the sampled set has the sampled idxs correctly set, then create the confirmed set and make sure that's well ordered too, then make the primary guards list and make sure that's well ordered, finally ask for a guard and make sure that's what you would expect. Feel free to reuse code from other unittests of course... So, before this branch gets merged we would need to do the following two things: - Get that unittest in. - Answer the comment on the new PR. - Get the torspec patch for `guard-spec.txt` merged. - Squash the PR into more organized commits. Ideally this PR would be squashed down to max 3-4 logical commits. Something like the following commit structure would work, but feel free to improvise: - Commit 1: Use helper functions parse_from_state_set_vals/parse_from_state_handle_time - Commit 2: Implement prop310" - Commit 3: Fix existing unittests - Commit 4: Add unittests Thanks a lot for your work! :) -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/32088#comment:26> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs