Opened 4 years ago

Closed 4 years ago

#8165 closed defect (fixed)

torspec: Document the new "flag-thresholds" item in votes

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: torspec
Cc: Actual Points:
Parent ID: #8151 Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (10)

comment:1 Changed 4 years ago by asn

  • Status changed from new to needs_review

See branch bug8165 in https://git.torproject.org/user/asn/torspec.git.

comment:3 Changed 4 years ago by atagar

Please add a formal description of the Thresholds attribute. At present there's not enough information to know what the line has. My first thought was 'oh, they're probably just space separated flags', then 'hm, maybe they're key=<int> mappings?' - both were wrong so I was lucky that Karsten had an example on #8164.

Something like the following might work...

"flag-thresholds" SP THRESHOLDS NL

[At most once for votes; does not occur in consensuses.]

THRESHOLDS = THRESHOLD | THRESHOLD SP THRESHOLDS
THRESHOLD = THRESHOLD_KEY "=" THRESHOLD_VALUE
THRESHOLD_KEY = [A-Za-z0-9]+
THRESHOLD_VALUE = [A-Za-z0-9%]+

I suspect that what we really want for THRESHOLD_VALUE is 'ArgumentChar except WS' though I'm not entirely sure how we'd define that.

Also, can there be duplicate THRESHOLD_KEYs? If so then what does it mean?

comment:4 Changed 4 years ago by atagar

Correction, THRESHOLD_KEY should probably be 'KeywordChar+'.

comment:5 Changed 4 years ago by atagar

Even better, can we ensure numeric values with an optional '%'?

THRESHOLD_VALUE = [0-9]+[%]?

If so then libraries like stem and metrics-lib can provide a nicer API to users.

comment:6 Changed 4 years ago by arma

Seems like Tor should stop including the %. It doesn't help and seems to hurt.

comment:7 Changed 4 years ago by karsten

Stem and metrics-lib could simply divide threshold values with a trailing % by 100. No need to change the line format, IMO. In fact, the format with % is already in a released version, so please don't change it.

comment:8 Changed 4 years ago by arma

To be clear, I'm not suggesting changing where the decimal place goes. Just dropping the extraneous characters that no parsers are using anyway.

comment:9 Changed 4 years ago by karsten

Still, you'd change the format by doing so. Parsers have to support all existing formats, with and without the % sign, because they have to handle archived votes, too.

comment:10 Changed 4 years ago by nickm

  • Milestone set to Tor: 0.2.4.x-final
  • Resolution set to fixed
  • Status changed from needs_review to closed

Okay, I've merged and tweaked it a little. If it's wrong somehow, more patches are welcome.

Note: See TracTickets for help on using tickets.