Opened 2 years ago

Closed 2 years ago

#22148 closed defect (implemented)

prop140: conformance to proposal, unhandled corner cases

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: ahf Actual Points: .7
Parent ID: #13339 Points: .5
Reviewer: ahf Sponsor: Sponsor4

Description

There are some remaining issues in my prop140_complete branch.

We should allocated a protover for directories that support these new requests.

Right now, we treat failures to apply a consensus diff as if the consensus download had failed. Is this what we should be doing?

The proposal specifies a method for downloading diffs without using the X-Or-Diff-From-Consensus syntax. We should implement that on the server side.

Our proposal specifies networkstatus parameters, but the code doesn't implement them.

The proposal says that sha3 digests may be truncated in requests: I propose we do not implement that, and revise the proposal to match.

Child Tickets

Change History (11)

comment:1 Changed 2 years ago by nickm

Status: newaccepted

comment:2 Changed 2 years ago by ahf

Cc: ahf added

comment:3 Changed 2 years ago by nickm

note to self: prop140_aftermath_url is done but needs more testing.

comment:4 Changed 2 years ago by nickm

note to self: prop140_aftermath_cfg is done

comment:5 in reply to:  description Changed 2 years ago by nickm

Status: acceptedneeds_review

Replying to nickm:

There are some remaining issues in my prop140_complete branch.

We should [allocate] a protover for directories that support these new requests.

Done in prop140_aftermath_url, which now I've tested. :)

Right now, we treat failures to apply a consensus diff as if the consensus download had failed. Is this what we should be doing?

Still remains. I've made this into #22172 since it will take more thinking.

The proposal specifies a method for downloading diffs without using the X-Or-Diff-From-Consensus syntax. We should implement that on the server side.

Done in prop140_aftermath_url.

Our proposal specifies networkstatus parameters, but the code doesn't implement them.

Done in prop140_aftermath_cfg.

The proposal says that sha3 digests may be truncated in requests: I propose we do not implement that, and revise the proposal to match.

Done in torspec in 28816242f9eaa5509dc400a48ade1e7c4a591717 and 522d75dd3969a4e220a113d38cf2c4497d264ab4.

comment:6 Changed 2 years ago by nickm

Actual Points: .5

comment:7 Changed 2 years ago by ahf

Reviewer: ahf

comment:8 Changed 2 years ago by ahf

Status: needs_reviewmerge_ready

Both prop140_aftermath_cfg and prop140_aftermath_url looks good. It also fixes one of my questions on #22149.

This looks like it will cause some conflicts for the changes in #21667, but that should be easy to handle.

comment:9 Changed 2 years ago by nickm

ok; I'll aim to merge these and resolve conflicts after the #21667 merge.

comment:10 Changed 2 years ago by nickm

prop140_aftermath_cfg merged cleanly and pushed.

comment:11 Changed 2 years ago by nickm

Actual Points: .5.7
Resolution: implemented
Status: merge_readyclosed

prop140_aftermath_url_v3 created, reviewed, merged, and fixed. :)

Note: See TracTickets for help on using tickets.