Make FetchUselessDescriptors fetch all consensus flavours

FetchUselessDescriptors 1 used to imply UseMicrodescriptors 0, but due to #6769 (moved), it doesn't in 0.3.0 and later.

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

We should document this in the man page and probably the changelog summary, as it is a breaking change for many configs that used to obtain a full consensus.

(Alternately, we could fix this bug by FetchUselessDescriptors download a full consensus, even on clients, and document that behaviour.)

Discovered when running a test bandwidth authority - see #20621 (moved).

changed milestone to %Tor: 0.3.0.x-final

added actualpoints::0.5 component::core tor/tor easy intro milestone::Tor: 0.3.0.x-final points::0.5 priority::medium resolution::fixed review-group-13 reviewer::nickm severity::normal status::closed type::defect version::tor 0.3.0.0-alpha-dev labels

I think we should make FetchUselessDescriptors fetch all the consensus flavors and all the descriptor flavors. That's what most people seem to use it for at least.

(The comments and man page entry talk about how it will make you fetch descriptors for non-running relays too. But that concept is now obsolete, since non-runnings relays are never even mentioned anymore.)

Replying to arma:

I think we should make FetchUselessDescriptors fetch all the consensus flavors and all the descriptor flavors. That's what most people seem to use it for at least.

I don't think it has done this for quite some time, at least on clients - unless the download schedule for the useless descriptors is minutes long.

I tested with 0.2.7, 0.2.8, and 0.2.9, and FetchUselessDescriptors only fetched the full consensus and descriptors, not microdescriptors

And I tested with 0.3.0, and FetchUselessDescriptors only fetched the micro consensus and microdescriptors.

Since this is not a regression in 0.2.9, I'm keeping it in 0.3.0, and I've retitled the bug and edited the description.

Trac:
Description: FetchUselessDescriptors 1 used to imply UseMicrodescriptors 0, but due to #6769 (moved), it doesn't in 0.3.0 and later.

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

We should document this in the man page and probably the changelog summary, as it is a breaking change for many configs that used to obtain a full consensus.

(Alternately, we could fix this bug by FetchUselessDescriptors download a full consensus, even on clients, and document that behaviour.)

Discovered when running a test bandwidth authority - see #20621 (moved).

to

FetchUselessDescriptors 1 used to imply UseMicrodescriptors 0, but due to #6769 (moved), it doesn't in 0.3.0 and later.

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

We should document this in the man page and probably the changelog summary, as it is a breaking change for many configs that used to obtain a full consensus.

(Alternately, we could fix this bug by FetchUselessDescriptors download a full consensus, even on clients, and document that behaviour.)

Discovered when running a test bandwidth authority - see #20621 (moved).
Points: 0.1 to 0.5
Summary: Clarify FetchUselessDescriptors and UseMicrodescriptors in 0.3.0 to Make FetchUselessDescriptors actually fetch all flavours on clients
Keywords: doc deleted, intro, easy added

Replying to teor:

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

Another possibility is that we treat #20621 (moved) as a bug in the bwauthority documentation / configuration ("if you wanted the microdesc-flavored consensus, you should have asked for it explicitly"), and then we close out the fetchuselessdescriptors config option as deprecated and eventually obsolete, since its original reason for existing ("fetch info about relays that you know you won't use") no longer makes sense since we took those relays out of the consensus.

Replying to arma:

Replying to teor:

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

Another possibility is that we treat #20621 (moved) as a bug in the bwauthority documentation / configuration ("if you wanted the microdesc-flavored consensus, you should have asked for it explicitly"), and then we close out the fetchuselessdescriptors config option as deprecated and eventually obsolete, since its original reason for existing ("fetch info about relays that you know you won't use") no longer makes sense since we took those relays out of the consensus.

Ok, that sounds sensible. We could even deprecate it with a message that says: "Set UseMicrodescriptors 0 to download the full consensus on a client", or something perhaps worded a bit better.

Do we need an option that makes a client fetch both full descriptors and microdescriptors? If not, this sounds like a sensible solution.

Trac:
Summary: Make FetchUselessDescriptors actually fetch all flavours on clients to Deprecate FetchUselessDescriptors, mentioning UseMicrodescriptors 0 in the message

Replying to teor:

Do we need an option that makes a client fetch both full descriptors and microdescriptors? If not, this sounds like a sensible solution.

Once upon a time, that option was called DirPort.

Now I kind of assumed it would be what DirCache does, but looking at the code, maybe it isn't.

What is the thing that makes relays fetch all the things? We should let people turn that on.

directory_caches_dir_info() looks promising, which in turn calls dir_server_mode(), which in turn looks at options->DirPort_set.

Replying to arma:

Replying to teor:

Do we need an option that makes a client fetch both full descriptors and microdescriptors? If not, this sounds like a sensible solution.

Once upon a time, that option was called DirPort.

Now I kind of assumed it would be what DirCache does, but looking at the code, maybe it isn't.

What is the thing that makes relays fetch all the things? We should let people turn that on.

Nothing, currently - they are all conditional. (Except for perhaps DirPort 127.0.0.1:9999, which is a sleazy hack.)

I think we need a new option, which makes the following functions return true: directory_caches_unknown_auth_certs directory_caches_dir_info

But doesn't change: dir_server_mode Because DirPort, BridgeRelay, and (usually) DirCache do that.

Nor: directory_fetches_from_authorities Because FetchDirInfoEarly does that.

And then we might want to review every use of dir_server_mode to see if we really meant to say one or the other of: directory_caches_unknown_auth_certs directory_caches_dir_info

(OK, fixing the title for the last time. Sorry!)

Trac:
Summary: Deprecate FetchUselessDescriptors, mentioning UseMicrodescriptors 0 in the message to Make FetchUselessDescriptors fetch all consensus flavours

getinfo_helper_dir checks directory_caches_dir_info before providing the NS consensus. Maybe this is ok if we fix directory_caches_dir_info. But maybe we want to use we_want_to_fetch_flavor instead.

networkstatus_set_current_consensus drops the flavor we're not using, unless directory_caches_dir_info is true. Maybe we want to use we_want_to_fetch_flavor here instead. Or fix directory_caches_dir_info.

update_consensus_router_descriptor_downloads does the same thing.

authority_certs_fetch_missing and trusted_dirs_load_certs_from_string do a similar thing with directory_caches_unknown_auth_certs.

This is fixed in my github branch bug20667, this branch includes #20839 (moved).

Trac:
Status: new to needs_review
Actualpoints: N/A to 0.5

Trac:
Keywords: N/A deleted, review-group-13 added

Trac:
Reviewer: N/A to nickm

Looks lovely to me. Merging!

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

closed

changed time estimate to 4h

added 4h of time spent

mentioned in issue #20839 (moved)

mentioned in issue #20909 (moved)

mentioned in issue #27247 (moved)

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