Opened 9 months ago

Closed 8 months ago

#29959 closed defect (fixed)

Actually include the bandwidth file digest in the vote

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version: Tor: 0.4.0.2-alpha
Severity: Normal Keywords: 040-must, 040-backport, fast-fix, nickm-merge
Cc: Actual Points: 0.2
Parent ID: #29947 Points: 0.2
Reviewer: asn Sponsor:

Description

The original code in #26698 had a logic error, so the bandwidth file digest was never included in the vote.

Child Tickets

TicketStatusOwnerSummaryComponent
#29961closedteorUpdate the Tor version for bandwidth-file-digest in torspecCore Tor/Tor

Change History (9)

comment:1 Changed 9 months ago by teor

Status: assignedneeds_review

comment:2 Changed 9 months ago by teor

(The unit tests pass because we only test the function that creates the digest, not the function that adds it to the votes.)

comment:3 Changed 9 months ago by teor

Parent ID: #29947

comment:4 Changed 9 months ago by teor

Actual Points: 0.30.2
Keywords: fast-fix added
Points: 0.30.2

comment:5 Changed 9 months ago by asn

Reviewer: asn

comment:6 in reply to:  1 Changed 8 months ago by asn

Status: needs_reviewneeds_revision

Replying to teor:

See my pull requests:

Tiny improvement by removing the nesting altogether, since no retval checking is needed:
https://github.com/torproject/tor/pull/888
I like removing nesting.

Feel free to put to merge_ready if you like it.

comment:7 Changed 8 months ago by teor

Keywords: nickm-merge added
Status: needs_revisionmerge_ready

My original fix in #29960 added a failure return value to digest256_to_base64().
But Nick recommended we just crash instead, because failure is unlikely, and the resulting code is much simpler.

So https://github.com/torproject/tor/pull/888 can be merged by Nick, because we've swapped reviews here.

comment:8 Changed 8 months ago by nickm

Squashed and merged to 0.4.0 and forward.

comment:9 Changed 8 months ago by nickm

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.