Add control port GETINFO support for dumping the local consensus

We have GETINFO ns/all but that only dumps the routerstatus documents from the consensus and not the entire consensus. There is lots of good information in the consensus headers/footers that people are interested in and it should be exposed to the control port.

(This will also need a spec patch)

changed milestone to %Tor: 0.4.3.x-final

Trac:
Child Ticket(s): #31762 (moved)

added actualpoints::0.5 component::core tor/tor control-port easy extra-review milestone::Tor: 0.4.3.x-final points::1 priority::medium resolution::fixed reviewer::asn severity::normal status::closed type::enhancement labels

I think this is already implemented as GETINFO dir/status-vote/current/consensus?

Trac:
Status: new to needs_information

Whoops! dir/status-vote/current/consensus works for an NS consensus, but not for a microdesc consensus.

Trac:
Status: needs_information to new

https://github.com/torproject/tor/pull/1328

This adds GETINFO dir/status-vote/microdesc/consensus

Trac:
Username: ltbringer

Trac:
Username: ltbringer
Status: new to needs_review

Hi, thanks for this pull request!

Replying to ltbringer:

https://github.com/torproject/tor/pull/1328

This adds GETINFO dir/status-vote/microdesc/consensus

How did you come up with this GETINFO name? If we want the control spec to match the directory spec, it should be: GETINFO dir/status-vote/current/consensus-microdesc See https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n3895

This ticket creates a small reduction in code quality, because it turns a 300-line function into a 320-line function. If you would like to improve code quality, please add a commit that splits the two dir/status-vote/* GETINFOs into their own static function. Otherwise, our reviewer can do that for you, before they approve your pull request.

We also need to update the control-spec, I'll open a child ticket.

(We could do #28982 (moved) instead of this ticket, but that's a much more complex refactor.)

This ticket creates a small reduction in code quality, because it turns a 300-line function into a 320-line function. If you would like to improve code quality, please add a commit that splits the two dir/status-vote/* GETINFOs into their own static function. Otherwise, our reviewer can do that for you, before they approve your pull request.

I will create the static function. I feel the entire function could use some help as I see the if-else blocks having some degree of similarity (my commit creates almost identical blocks, which differ only by a string argument).

Also, I was unaware of the spec. I'll fix that too.

Trac:
Username: ltbringer

Yes, the whole function could be turned in to a lookup table.

Hopefully we'll do that in #28982 (moved).

https://github.com/torproject/tor/pull/1328 is updated with the function that switches between the two dir/status-vote/*

and the name follows the convention GETINFO dir/status-vote/current/microdesc/consensus to GETINFO dir/status-vote/current/consensus-microdesc

Trac:
Username: ltbringer

Thanks! We'll get someone to review this change in the next week or so.

Trac:
Reviewer: N/A to asn

OK I finished a code review here. And left a few comments on the PR. Marking as needs_revision for now!

Also can you tell me how you tested this new feature?

Thanks for all the code!

Trac:
Status: needs_review to needs_revision

You are welcome :) I'll work on the comments in the PR.

I used the python stem client to test this feature.

Trac:
Username: ltbringer

Trac:
Username: ltbringer
Status: needs_revision to needs_review

Closer! Some more work on the unittest would be great!

Trac:
Status: needs_review to needs_revision

I created a mock of networkstatus_map_cached_consensus and make it return a simulated tor_mmap_t object. Please review and let me know if that works.

Trac:
Username: ltbringer

Trac:
Username: ltbringer
Status: needs_revision to needs_review

Yes the unittest looks much better now! There are still a few things that need to be ironed out, and also the Appveyor CI fails. But we are close! :)

Trac:
Status: needs_review to needs_revision

Trac:
Username: ltbringer
Status: needs_revision to needs_review

Thanks for all the work. We are moving closer!

Trac:
Status: needs_review to needs_revision

1. function declaration and definition are not separated within tests.
2. unmocked networkstatus_get_cache_fname and we_want_to_fetch_flavor.

Trac:
Username: ltbringer
Status: needs_revision to needs_review

This seems pretty much ready, but you should use networkstatus_get_flavor_name() instead of making a new function for that.

Trac:
Status: needs_review to needs_revision

Trac:
Username: ltbringer
Status: needs_revision to needs_review

Thanks. Two more minor issues and we are done.

I also tested that this works fine with a stem script that looks like this:

 controller = Controller.from_port(port=9090)
 controller.authenticate()
 ns = controller.get_info("dir/status-vote/current/consensus-microdesc")
 print("microdesc: %s" % ns)
 ns = controller.get_info("dir/status-vote/current/consensus")
 print("ns: %s" % ns)

After you change these two minor issues please mark it as merge_ready.

Marking as extra review so that the merger checks that the issues have been fixed before merging (I will be away for two weeks)

Thanks again for the code!

Trac:
Keywords: control-port easy deleted, control-port easy extra-review added

Trac:
Status: needs_review to needs_revision

Trac:
Username: ltbringer
Status: needs_revision to merge_ready

Trac:
Keywords: control-port easy extra-review deleted, control-port easy extra-review nickm-merge dgoulet-merge added

I did some fixes:

• standard whitespace
• typos
• standard error handling
• remove unnecessary test code

Here is the branch with all the fixes: https://github.com/teor2345/tor/commits/ticket31684 There is a description of each fix in my review on PR 1328.

Here is the squashed PR for merging:

• master: https://github.com/torproject/tor/pull/1434 I squashed the whitespace fixes with the rest of the code, to minimise the diff.

Since these are obvious fixes on already reviewed code, any maintainer can merge after CI passes.

Trac:
Keywords: control-port easy extra-review nickm-merge dgoulet-merge deleted, control-port easy extra-review added
Actualpoints: N/A to 0.5
Type: task to enhancement

I also tested the new GETINFO with tor-prompt.

The torspec PR is https://github.com/torproject/torspec/pull/92 They should merge at the same time.

Merged to master.

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

closed

changed time estimate to 8h

added 4h of time spent

mentioned in issue #32171 (closed)

mentioned in issue #31762 (moved)

moved to tpo/core/tor#31684 (closed)