Refactor tor's circuit path node selection checks

In legacy/trac#33222 (moved), we added an extra "can extend over IPv6" check to tor's circuit path node selection code.

To make sure it's applied consistently, I did a refactor of that code, so all those checks are in one function.

changed milestone to %Tor: 0.4.5.x-final in legacy/trac

Trac:
Parent Ticket: legacy/trac#33221 (moved)

added actualpoints::1.5 in Legacy / Trac component::core tor/tor in Legacy / Trac extra-review in Legacy / Trac ipv6 in Legacy / Trac milestone::Tor: 0.4.5.x-final in Legacy / Trac owner::teor in Legacy / Trac parent::33221 in Legacy / Trac points::1 in Legacy / Trac priority::medium in Legacy / Trac prop311 in Legacy / Trac resolution::implemented in Legacy / Trac reviewer::nickm in Legacy / Trac severity::normal in Legacy / Trac sponsor::55-can in Legacy / Trac status::closed in Legacy / Trac technical-debt in Legacy / Trac type::defect in Legacy / Trac labels

See my PR, which is based on legacy/trac#33222 (moved):

• incomplete: https://github.com/torproject/tor/pull/1888

This PR still needs unit tests and changes files for the extra refactor on top of legacy/trac#33222 (moved).

The stem tests will fail due to:

• https://github.com/torproject/stem/issues/63 Reverting to the most recent stem release, or stem commit 13e401d, should resolve this issue.

Trac:
Cc: N/A to nickm
Status: assigned to needs_review

I have rebased this PR on top of legacy/trac#33222 (moved), and added a changes file:

• master: https://github.com/torproject/tor/pull/1888

To see the changes without legacy/trac#33222 (moved):

• https://github.com/teor2345/tor/pull/2

This PR makes some of tor's node selection code a lot simpler. It fixes a bunch of subtle bugs and edge cases. The code was almost impossible to unit test before the refactor. After the refactor, it's a lot more consistent and modular.

Here are the features and functions that don't have unit tests:

• node selection
  • choose_good_exit_server_general()
  • count_acceptable_nodes()
  • router_choose_random_node_helper()
  • router_choose_random_node()
  • router_can_choose_node()
  • router_add_running_nodes_to_smartlist()

Some of these functions are also modified in legacy/trac#33222 (moved). It should be easier to write the legacy/trac#33222 (moved) node selection unit tests on top of this refactor.

This code also runs in chutney, as part of Tor's CI. But chutney doesn't fully test node selection.

I suggest that we manually review the refactoring in this PR. We should also write unit tests for these functions (or open another ticket for the unit tests).

Trac:
Actualpoints: 1 to 1.5
Keywords: N/A deleted, extra-review added
Reviewer: N/A to nickm

Yes, this looks fine. On our current timeline it makes sense to take this (and the rest of the remaining S55 code) in 0.4.5.

Trac:
Milestone: Tor: 0.4.4.x-final to Tor: 0.4.5.x-final
Status: needs_review to merge_ready

(Merged to master)

Trac:
Status: merge_ready to closed
Resolution: N/A to implemented

closed

changed time estimate to 8h

added 12h of time spent

mentioned in issue legacy/trac#34412 (moved)

mentioned in issue legacy/trac#33221 (moved)

moved from legacy/trac#34200 (moved)

mentioned in issue #33221 (closed)

added Bug label and removed 1 deleted label

added 1 deleted label

added IPv6 + 1 deleted label and removed 1 deleted label

added BugSmashFund label

added Technical Debt label and removed 1 deleted label

mentioned in issue #40640