[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order
#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@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs