prop140: Update dir-spec with prop140 protocols

changed milestone to %Tor: 0.3.1.x-final

added TorCoreTeam201705 actualpoints::.3 component::core tor/tor milestone::Tor: 0.3.1.x-final owner::nickm parent::13339 points::.3 priority::medium resolution::implemented severity::normal sponsor::4 status::closed type::defect labels

Trac:
Status: new to accepted

See my dirspec branch prop140_completed

Trac:
Actualpoints: N/A to .3
Status: accepted to needs_review
Keywords: N/A deleted, TorCoreTeam201705 added

Looks reasonable to me at first glance. I haven't exhaustively checked how faithfully the new dir-spec.txt reflects the prop140 content. I just noticed that the following diff chunk seems like a possible typo, though.

@@ -216,7 +217,7 @@ Status: Accepted
 
   The following parameters govern how relays and clients use this protocol.
 
-     min-consensuses-age-to-cache-for-diff
+b     min-consensuses-age-to-cache-for-diff
        (min 0, max 744, default 6)
      max-consensuses-age-to-cache-for-diff
        (min 0, max 8192, default 72)

Generally looks good. Two questions:

1. This patch marks prop140 as closed, but do we support the /tor/status-vote/current/consensus/diff/<HASH>/<FPRLIST>.z paths already?

2. This is in the nitpick category. Do we want to change the delimiter in X-Or-Diff-From-Consensus to use "," and optionally " " like in the rest of HTTP?

Catalyst also commented on a possible typo above.

Trac:
Status: needs_review to merge_ready

Replying to ahf:

Generally looks good. Two questions:

1. This patch marks prop140 as closed, but do we support the /tor/status-vote/current/consensus/diff/<HASH>/<FPRLIST>.z paths already?

Not until we merge one of the branches in #22149 (moved) ; I'm not planning to merge this branch until #22149 (moved) is done

2. This is in the nitpick category. Do we want to change the delimiter in X-Or-Diff-From-Consensus to use "," and optionally " " like in the rest of HTTP?

Huh. I think right now it allows either in the code; I guess we should change the spec to match.

Catalyst also commented on a possible typo above.

Agreed, will fix before merge.

The code does:

3661   smartlist_split_string(hex_digests, hdr, " ",
3662                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1);

So no ",".

ok. IMO it's fine to have it start accepting commas, since currently nothing sends more than one X-Or-Diff-From-Consensus value anyway.

branch updated; I should do a corresponding update for the comma issue once the other prop#278 and prop#140 stuff is merged

squashed and merged

Trac:
Resolution: N/A to implemented
Status: merge_ready to closed

closed

changed time estimate to 2h 24m

added 2h 24m of time spent

moved to tpo/core/tor#22149 (closed)