Opened 4 weeks ago

Last modified 3 weeks ago

#32088 needs_revision enhancement

Proposal 271 - straightforward improvements

Reported by: Jaym Owned by:
Priority: High Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec prop271
Cc: Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Child Tickets

Attachments (1)

0001-Makes-selection-of-filtered-guards-and-primary-guard.patch (7.0 KB) - added by nickm 4 weeks ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 weeks ago by arma

Component: Core TorCore Tor/Tor
Type: defectenhancement

comment:2 Changed 4 weeks ago by nickm

Milestone: Tor: 0.4.3.x-final
Status: newneeds_review

comment:3 Changed 4 weeks ago by dgoulet

Keywords: tor-spec prop271 added
Reviewer: nickm

comment:4 Changed 4 weeks ago by nickm

Priority: MediumHigh
Status: needs_reviewneeds_revision

Here are some initial comments based on reading the patch and the email, based on the assumption that the email is right, and that the patch does what the email says it should do.

Should next_sampled_idx persist across runs or be recalculated when we load guards? Right now it seems to start at 0 every time Tor starts. Maybe we should also re-calculate the sampled_idx values to be a dense array when we save/load the state, so that we aren't leaking more information than we intend to.

The patch will also need:

  • a short proposal. It can mostly be based on Florentin's email.
  • a patch to guard-spec.txt.
  • tests for the changed behavior.
  • a "changes" file (see doc/HACKING/)
  • editing so that "make check-spaces" passes.
  • Updates to the documentation and names of all the functions whose behavior has changed. (For example, sample_reachable_filtered_entry_guards` no longer does that the function's name implies or the documentation says.)

How much of that do the original authors want to do, and how much should we pick up?

comment:5 Changed 3 weeks ago by Jaym

woups, yes currently it would start at 0 every time, which is bad. Recalculation based on the last sampled_idx from the state file + 1 should be good.

I would recommend the short proposal to include the required change over the confirmed list (i.e., this list needs sorting by sampled_idx instead of confirmed_idx). So this patch would need to handle that as well.

I am happy to help on writing a small proposal from my email, and I can pick up whatever is left from your bullet list starting from January (I am bit busy for these next two months :/).

Could the proposal includes a kind of "further work" suggestion? Describing the issue that Roger wrote in the email thread.

comment:6 Changed 3 weeks ago by nickm

Could the proposal includes a kind of "further work" suggestion? Describing the issue that Roger wrote in the email thread.

Yes, that's fine.

Note: See TracTickets for help on using tickets.