The dummy module is going to need an error type just like this but
with only the disabled variant. To avoid that dummy enum getting out
of step with the nontrivial one, we're going to make them the same.
So as a first step, break this out into its own file.
That the *de*serialisation works as expected will be tested properly
in just a moment, because when we plumb this all the way through, it
will be what parses the bridge lines in the example config file.
This struct is going to be the principal "dictionary-style" serde
representation for a bridge, and the builder, making this all in
keeping with our usual approach.
In this commit:
* Introduce the struct (defining the serialisation)
* Provide the setters (defining the Rust API)
* Add success test cases (not all of the data in which is used yet)
We previously had a trace message on _every_ change. That's fine,
but we also want to log more important changes where the user
can see them. Namely:
* If we go from any other status to Reachable, we want to tell
the user. (We don't want to spam them if it was already
reachable.)
* If we go from Untried or Reachable to Unreachable, we want to
tell the user. (We don't tell them about changes from Retriable
to Unreachable, since that just means that a retry attempt
was not successful.)
Closes#627.
There are two cases here, and we will want to log them
differently.
(By removing the "Unknown" variant entirely, we ensure that we
didn't miss any code that formerly checked for Unknown.)
This lets us avoid calling `Instant::now()`, when `Runtime::now()`
is what we want.
Unfortunately, there are a bunch of functions that called `update()`
that needed to change. Fortunately, none of the changes were very
complicated.
Fixes a `TODO pt-client` comment.
If we have a bridge guard that is using Direct connection and it
knows multiple addresses, our code to match it with a BridgeConfig
is wrong, because the BridgeConfig has only one address, and our
code looks for an exact match.
Fixes#642.
This is the only way I could find in which parameter interpretation
differs between bridge guards and relay guards; with it documented,
I can remove a TODO about identifying such ways.
Previously we would call extend_sample_as_needed in only two places,
one of which called the other unconditionally. That's obviously not
necessary.
I've selected just one of them (`update_guardset_internal`) since it
fits better with the theme if that function. I've added comments
explaining what is going on.
This commit also introduces a yes/no enum for "were any guards added
while extending this set". Formerly we had a boolean, but it got
passed around so many times that I think its intent became obscure.
What this function actually does is return the number of primary
guards whose presence (by identity) is ambiguous in a current
universe. The new name and documentation should help avoid
confusion.
The method's old name had led me astray when identifying whether it
should apply to bridges in one case. This commit also removes the
corresponding `TODO pt-client`.
Also, add a bunch of reminders around these implementations that
`HasAddrs` returns all the address associated with you for GeoIp or
family purposes, even if they are _not_ ones that we should actually
contact you at.
This explanation is slightly complicated by the fact that I think
that one of the calls to update_guardset_internal() is possibly
unnecessary, and that one of the calls that it makes is potentially
ill-advised.
I'm not going to make those changes right now, however, because they
are potentially a little destabilizing.
Now it is an Option, and is set to None if bridges aren't enabled.
This simplifies `replace_bridge_config` a bit, and forces us to
check for `None` in a few more places.
We do this by checking the FirstHops we're about to return, and when
they correspond to bridges, looking up an appropriate BridgeRelay
in the current BridgeSet (if we can).
The `GuardMgr` code has functionality to tell the DirMgr "Hey,
don't switch to the new NetDir yet: we still need more guard
information!" But we never want to do that if we're selecting
bridges, since they don't come from the NetDir.
Instead of duplicating the logic about which guard sample uses which
universe, we explicitly ask it, and then use that universe. This
will avoid trouble if/when we introduce more samples.
I'm using an Arc<[]> here though I think that there's a chance
that a simple Vec<> would suffice. Since it's an internal type,
nothing will break if we change it later.
Also, we now switch into and out of the Bridges guard sample
as needed. However, that selection is not (yet) built from the
list of bridges. That will come soon.
The first part changes which guard set is active based on based on
the parameters, which always come from a NetDir; the second changes
the contents of the active guard set, based on a Universe.
These arguments were used only for legacy (testing) purposes; the
tests now use `TestNetDirProvider`. This lets us simplify our
internal logic for passing a `NetDir` to our samples, and prepare
for having a `BridgeSet` to pass there instead.
This is a breaking change to `guardmgr` and `circmgr`.
0.99.[012] have a bug https://github.com/JelteF/derive_more/issues/114
which makes the Deref derive for bridgedesc::StateGuard not work
and therefore breaks minimal-versions CI.
It seems simpler to require the newer version everywhere.
Previously we always set `dir_info_missing` to `false` for new
guards, since new guards could only be taken from ones that were
present in the NetDir. But for bridges, we don't download their
info until _after_ we have chosen them as guards.
In this and the upcoming commits I'll be changing how guards related
to `NetDir` and to `Relay`. Previously, a guard could only come
from (or be updated from) a `Relay` in a `NetDir`. Soon it will be
able to be built from a bridge as well.
To do this, I'm defining a `Universe` trait (name negotiable) that
represents a set of things that may be guards. I'm going to
continue extending its functionality until there are no more
methods in guard.rs or sample.rs that take `NetDir`.
This commit removes most of the usage of `NetDir` and `Relay` in
`guard.rs`.
This is necessary for the (somewhat undesirable) lookup_ids function
to return an ID that the dirmgr can actually use to report successes
and failures.
As noted, lookup_ids will create problems down the road when we
implement relays. We should refactor it out before then.
This required a number of changes, which I've tried to document.
I've taken a conservative approach to modification, and I'm not
using any of the by_*_mut() functions (yet). For cases which
potentially modify the whole set, I'm using into_values() and
collect() to ensure that it's re-indexed correctly, even though the
identities don't change.
I introduce some "TODO pt-client" comments here which I will resolve
in the next commit(s).
Now it contains either an `OwnedChanTarget` or an `OwnedCircTarget`,
which will let `GuardMgr` return bridges that can be used to make
circuits.
As part of this change, it was necessary to revise some
address-modification functions that applied to filters and
`OwnedChanTarget`. Now they do the smart thing, and remove only the
address that are in the `ChanMethod`. This means that the addresses
from HasAddrs are still accurate about which addresses the relay
"has".
The most important part of this commit is to make sure that each
`FirstHopId` includes the `GuardSetSelector` from which the guard
was selected. Doing this lets us be certain that when we report
that a guard has succeeded or failed, we're reporting it in the
right context.
Additionally, this commit uses strum to make an iterator over the
samples, so that we can make sure that our "for each sample" code is
robust against future changes, and we don't miss the bridge sample.
Now keyed by Arc<BridgeConfig>, and the values can be errors.
Currently there is no implementation so there can't be any errors,
but the error enum will become nonempty.
The feature we want is `#[doc = include_str!("README.md")]`, which is
stable since 1.54 and our MSRV is now 1.56.
This commit is precisely the result of the following Perl rune:
perl -i~ -0777 -pe 's{(^//!(?!.*\@\@).*\n)+}{#![doc = include_str!("../README.md")]\n}m' crates/*/src/lib.rs
HasAddr used to mean "Here are addresses that I have, at which I can
be contacted." But "Where (and how) can I be contacted?" is now a
question for HasChannelMethod to answer.
(We still need to have "HasAddr", though, so we can answer things
like "what country is this relay in" and "are these relays in the
same /8?")
So this commit introduces:
* A new trait for adding an implementation of HasChannelMethod in
terms of HasAddr.
* A requirement on ChanTarget that it needs to implement
HasChannelMethod.
There is some temporary breakage here, marked with "TODO pt-client",
that I'll fix later in this branch.
Also add a BridgeRelayWithDesc type (name tbd) to guarantee that
a bridge relay really does have a known descriptor before you
try to build a circuit with it.
This is the one we'll actually use to connect to bridges. It
has a `Bridge` line, and an optional `BridgeDesc`.
Maybe this will turn into a `BridgeRelay<'a>` by analogy to `Relay`
some time; I'm not sure.
BridgeDesc is a separate type to make sure that we do not confuse
bridges' descriptors with the descriptors from other routers down
the road. (Bridges' descriptors need to be used differently, and
treated as more private.)
With this code, BridgerDescList is now just an alias for
`ByRelayIds<BridgeDesc>`, which is pretty keen.
Nightly rust gives a warning about this "pub use", but the warning
is a false positive. Since it doesn't seem to be going away in a
hurry, let's suppress it for now.
Include the offending word in all the applicable errors.
Always print it with {word:?}.
As a consequence, there are no From impls any more and error
generation/conversion is by hand in all cases.
Clarify InvalidPtOrAddr vs InvalidIAddrorPt, and don't make the
attempted parse be a source error for those.
Where we still have source errors, don't print them in Display.
(Since the APIs for the `Schedule::sleep*` functions changed, this
is a breaking change in tor-rtcompat. Therefore, the Runtime trait
in tor-rtcompat is now a different trait. Therefore, anything that
uses the Runtime trait in its APIs has also broken.)
Ian Jackson
Nick Mathewson
Ian Jackson
Dimitris Apostolou
trinity-1686a