Opened 3 months ago

Closed 6 weeks 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:


According to proposal 290, we remove some consensus methods every time an LTS release is no longer supported:

When 0.2.9 is no longer supported, the earliest stable LTS will be, which supports consensus methods 25-28. Therefore, we can remove all consensus methods less than 28:

Child Tickets

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

Change History (9)

comment:1 Changed 2 months ago by gaba

Keywords: network-team-roadmap-2020Q1 added

comment:2 Changed 7 weeks ago by nickm

Owner: set to nickm
Status: newaccepted

comment:3 Changed 7 weeks ago by nickm

Actual Points: .1
Status: acceptedneeds_review

See branch ticket32695 with PR at

comment:4 Changed 6 weeks ago by dgoulet

Reviewer: teor

comment:5 Changed 6 weeks 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 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 6 weeks ago by nickm

Actual Points: .1.2
Status: needs_revisionneeds_review

Suggested revisions made. Thanks for the review!

comment:7 Changed 6 weeks 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:

I'll review the extra changes soon :-)

comment:8 Changed 6 weeks 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 6 weeks ago by nickm

Actual Points: .2.5
Resolution: implemented
Status: merge_readyclosed

Squashed and merged!

Note: See TracTickets for help on using tickets.