Opened 3 years ago

Closed 2 years ago

#24378 closed enhancement (implemented)

Prune the list of supported consensus methods

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop290, 034-triage-20180328, fast-fix
Cc: Actual Points:
Parent ID: Points: .5
Reviewer: teor Sponsor:


We currently have 13 supported consensus methods.

In 0.3.3, it is likely that prop282 will add 1 more, and prop283 will add 2 more.

Maybe we should prune this list eventually, because it will let us simplify our code, and make votes smaller, less expensive to calculate, and reduce authority RAM requirements (due to fewer microdescs).

It has almost no impact on consensus size.

Here's how we could work out what to prune:

By mandatory feature:

We are currently locked into using consensus method 16 or later in the public network, because 0.2.9 and later require ntor keys, and 0.2.9 clients use microdescriptors by default.

We may add more mandatory features in 0.3.3 and 0.3.4.

By supported tor version:

On May 1, 2018, we will stop supporting 0.2.5, and only support 0.2.9 and later. This means that all supported non-alpha versions will support consensus methods 25 and later. (Or, if we count 0.2.9 alpha versions, it's 22 and later.)

Child Tickets

Change History (12)

comment:1 Changed 2 years ago by teor

See #10163, for how we did this last time.

comment:2 Changed 2 years ago by nickm

Keywords: 034-triage-20180328 added

comment:3 Changed 2 years ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:4 Changed 2 years ago by nickm

Keywords: prop290 fast-fix added; needs-proposal 034-removed-20180328 removed
Owner: set to nickm
Points: 2.5
Status: newaccepted

comment:5 Changed 2 years ago by nickm

Status: acceptedneeds_review

See my branch remove_old_consensus_methods_2018, available for review at

To note:

  • There are a few places where I changed if (x) { to { because I felt that the block structure kept the code more organized.

comment:6 Changed 2 years ago by asn

Reviewer: isis

comment:7 Changed 2 years ago by teor

Reviewer: isisisis, teor

Looks good to me, I left a review on github with two nitpick changes:

  • fix some missing words in a commit message
  • please don't remove a unit test for a far future consensus method

We will also need a dir-spec change like this one:

[ As of, authorities no longer advertise or negotiate
          any consensus methods lower than 13. ]

I'm not sure if isis also wants to review this code.

Let's also make sure someone has run "make test-network-all" on this branch on an IPv6/mixed system, with tor-stable linked to 0.2.9.

comment:8 Changed 2 years ago by nickm

Added some fixup/squash commits to resolve your concerns above.

comment:9 Changed 2 years ago by nickm

Also see branch bug24378_spec in my torspec repo for the requested dir-spec changes.

comment:10 in reply to:  7 Changed 2 years ago by isis

Reviewer: isis, teorteor

Replying to teor:

I'm not sure if isis also wants to review this code.

Nope, your review looks good! Thanks, teor! I'll take review on #25818 in place of this.

comment:11 Changed 2 years ago by nickm

Status: needs_reviewmerge_ready

comment:12 Changed 2 years ago by nickm

Resolution: implemented
Status: merge_readyclosed

Both merged to 'master' and/or squashed as appropriate.

Note: See TracTickets for help on using tickets.