Opened 7 weeks ago

Closed 7 days ago

#32695 closed task (implemented)

Remove consensus methods 25-27

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 043-should, teor-backlog network-team-roadmap-2020Q1
Cc: Actual Points: .5
Parent ID: Points: 0.5
Reviewer: teor Sponsor:

Description

According to proposal 290, we remove some consensus methods every time an LTS release is no longer supported:
https://gitweb.torproject.org/torspec.git/tree/proposals/290-deprecate-consensus-methods.txt

When 0.2.9 is no longer supported, the earliest stable LTS will be 0.3.5.7, which supports consensus methods 25-28. Therefore, we can remove all consensus methods less than 28:
https://gitweb.torproject.org/tor.git/tree/src/feature/dirauth/dirvote.h?h=tor-0.3.5.7#n60

Child Tickets

TicketStatusOwnerSummaryComponent
#32955closedteorUpdate dir-spec for removing consensus methods 25-27Core Tor/Tor

Change History (9)

comment:1 Changed 4 weeks ago by gaba

Keywords: network-team-roadmap-2020Q1 added

comment:2 Changed 2 weeks ago by nickm

Owner: set to nickm
Status: newaccepted

comment:3 Changed 2 weeks ago by nickm

Actual Points: .1
Status: acceptedneeds_review

See branch ticket32695 with PR at https://github.com/torproject/tor/pull/1646

comment:4 Changed 9 days ago by dgoulet

Reviewer: teor

comment:5 Changed 9 days ago by teor

Status: needs_reviewneeds_revision

Looks good as-is, except for one typo.

If you'd like, we could also remove more code:

  • remove the consensus_method argument to routerstatus_format_entry()
  • delete ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD
  • delete node_awaiting_ipv6(), because it is false for all supported consensus methods
    • it was only ever true when microdescriptors had IPv6 addresses, and we were missing a microdescriptor for the node
  • delete networkstatus_consensus_has_ipv6(), because it's only used in node_awaiting_ipv6()
  • delete the unit tests for networkstatus_consensus_has_ipv6()

comment:6 Changed 8 days ago by nickm

Actual Points: .1.2
Status: needs_revisionneeds_review

Suggested revisions made. Thanks for the review!

comment:7 Changed 8 days ago by teor

macOS CI hung, probably an instance of #32804. I restarted it.

Stem failed, but I've seen the failure before, and logged an issue here:
https://github.com/torproject/stem/issues/52

I'll review the extra changes soon :-)

comment:8 Changed 8 days ago by teor

Status: needs_reviewmerge_ready

Looks good to me!

We removed a bug warning in fascist_firewall_choose_address_node(), but I don't think that bug can trigger any more, because:

  • when the client has a valid consensus during bootstrap, all nodes can have IPv6 addresses available
  • before and during bootstrap, there aren't any nodes

comment:9 Changed 7 days ago by nickm

Actual Points: .2.5
Resolution: implemented
Status: merge_readyclosed

Squashed and merged!

Note: See TracTickets for help on using tickets.