| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
| |
Closes #839
|
| |\
| |
| |
| |
| |
| |
| | |
DirMgr: Detect that directory state is ready if it begins with all MDs
Closes #802
See merge request tpo/core/arti!1126
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, we'd only call PendingNetDir::upgrade_if_necesessary
when adding a microdescriptor. But if it began already having all
of its descriptors (because we found them in the cache), we wouldn't
actually upgrade it to a PendingNetDir::Yielding, which would make
it unusable, and would make us schedule its reset time too far
in the future.
Fixes #802.
|
| |/
|
|
|
|
| |
Now that we require a version of Rust that allows
`b.then_some(v)`, clippy complains about our use of
`b.then(|| v)`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Remove the return value, which was not used anywhere.
Also remove the code to calculate the return value.
* Take an Arc<NetDir> rather than a reference. We are going to want
this for HS support. This has no overall effect on the lifetime of
the4 Arc, which was owned at the one call site and then imediately
dropped.
* Change the documentation to explain what the function's role is in
the netdir API, rather than the fiddly details of what it actually
does internally. Relegate the latter to a code comment.
(When we have HS, this will do more, or, at least, make further
arrangements.)
|
| |
|
|
| |
Split off for ease of review and possible rebase.
|
| | |
|
| |
|
|
|
| |
This panics on error, and we're fine with a panic on misbehavior in
tests.
|
| |
|
|
|
|
|
|
|
| |
This is in lieu of upgrading to the latest base64 crate, which has
a different API from the old one. Since we have to migrate either
way, we might as well use base64ct everywhere.
I don't think that most of these cases _require_ constant-time
base64, but it won't hurt.
|
| |
|
|
|
|
|
| |
This is the hunks from running the rune in maint/adhoc-add-lint-blocks
but which require some subsequent manual fixup: usually, deleting
now-superfluous outer allows, but in some cases manually putting back
lints that the adhoc script deleted.
|
| |
|
|
|
|
| |
This name is more accurate because we aren't only dealing with
clock skew here: we're also trying to tolerate the case where the
authorities fail to reach consensus for a while.
|
| |
|
|
| |
This is all crate-internal APIs, fortunately.
|
| |
|
|
|
|
| |
This is about to become only a _part_ of what defines a DirStatus: a
DirStatus will also include a reset count, and some kind of info
about how long we've gone without progress.
|
| |
|
|
|
|
|
|
| |
Now that the relevant functions now report changed/not-changed
status via a boolean out-parameter (see !527), there's no reason to
have a separate NoChanged error case.
Closes #484.
|
| |\
| |
| |
| |
| |
| |
| | |
DirMgr: Stop load-from-cache process when there is no change.
Closes #482
See merge request tpo/core/arti!527
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously in !511 I had introduced a bug where, if there was an
error more serious than "no change", that error would keep us from
noticing that we had no change, and we'd loop until the safety
counter ran out. Then we'd panic.
This commit fixes the bug by reintroducing the `changed` boolean --
this time as an outparam for the add_from_* methods.
Fixes #482.
|
| |/
|
|
|
| |
This is an approximately minimal revision to get Builder in place;
subsequent commits will clean up the API.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We no longer have separate return paths for recoverable and fatal
errors; instead, they are merged, and distinguished based on
recovery actions.
Since it is now possible for download() to give an error that should
_not_ destroy the previous state, it takes `&mut Box<dyn DirState>`.
This change unfortunately means that we can no longer call `state =
state.advance()`, but instead have to do some mem::swap junk with
poisoned values. Any better solution would be a good thing.
Additionally, the reset() and advance() methods can no longer fail.
There is still a separate return path for reset-triggering errors;
I'm about to fix that.
|
| | |
|
| |
|
|
|
|
| |
This commit adds a couple of new error types that we will soon want
to distinguish, and a new way of classifying errors. These are not
yet all used.
|
| |
|
|
|
| |
This should be sufficient to detect several kinds of nefariousness
that we'd previously overlooked.
|
| |
|
|
| |
This should have gone in when we fixed #412
|
| |
|
|
|
|
|
|
|
|
|
| |
Previously, we did this in `advance()`, but that wasn't so great: it
meant that we could fail in the advance() code, whereas the calls to
`advance()` treated errors as fatal.
This treats failed verification as a blocking error that requires a
reset.
Fixes one aspect of #439.
|
| |
|
|
|
| |
(A blocking error is one that means that the current bootstrap
attempt has failed, and must be restarted.)
|
| | |
|
| |
|
|
|
|
|
| |
Previously all errors were treated as non-fatal.
The add_from_* implementations don't yet behave properly;
I'll fix them in subsequent commits.
|
| |
|
|
|
|
|
|
|
| |
Previously DocSource would tell you whether the document was from
a local store or a cache server, but it wouldn't tell you _which_
server it came from.
This change required adding DocSource as an argument to
DirState::add_from_download.
|
| |
|
|
|
|
| |
In our status reporting code, we consider an
expired-but-still-usable directory still bootstrapped, but not 100%
bootstrapped.
|
| |
|
|
|
|
|
|
|
|
|
| |
Since we want to be willing to use older consensuses, we don't
necessarily want to reset a download just because the consensus is
expired.
This new behavior isn't ideal either; I've added a TODO that relates
to #433.
Related of #412
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This new section describes how much variance we accept when it comes
to expired and not-yet-valid directory documents. (Currently, the
only ones where this matters for are consensus documents and
authority certificates.) A document that is invalid by no more than
these tolerances is not _live_, but it can still be used.
These tolerances serve two purposes:
* First, they allow clients to run with a little more clock skew
than they would tolerate otherwise.
* Second, they allow clients to survive the situation where the
authorities are unable to reach a consensus for a day or two.
Compare with Tor's REASONABLY_LIVE_TIME and NETWORKSTATUS_ALLOW_SKEW
constants; also compare with proposal 212.
Closes #412.
|
| |
|
|
|
|
|
| |
- Some FIXMEs got removed or amended.
- AddMicrodescs now yields a mutable reference, so we can use .drain()
and reuse the allocation.
- Some panics were downgraded to debug_asserts.
|
| |
|
|
|
|
|
|
|
| |
- Taking a previous netdir directly and keeping it around before we need
it is a bit of a waste of memory, and also doesn't mesh well with how
SharedMutArc works.
- To remedy this, introduce a new trait `PreviousNetDir` and have the
state machines take that instead. (I was a bit tempted to just pass in
the SharedMutArc directly. Maybe I should've done that.)
|
| |
|
|
| |
- (Also fixes up some dirfilter stuff, whoops.)
|
| |
|
|
|
| |
- The only purpose of WriteNetDir was to provide a filter, which isn't
necessary any more. Refactor to provide the filter directly.
|
| |
|
|
|
|
|
|
|
|
|
| |
- GetMicrodescsState now uses the NetDirChange API to propagate netdir
changes, instead of modifying the netdir directly.
- PendingNetDir was refactored in order to support this use case.
- As a result, the netdir-related methods in WriteNetDir can be removed,
leaving only the DirFilter for now.
- add_from_cache() no longer takes a store, because nothing uses it.
- (bodge: apply_netdir_changes() was put in a few places missed
previously)
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
- The new DirState::get_netdir_change() API lets the state machine
export a NetDirChange: a request to either replace the current netdir,
or add microdescs to it.
- bootstrap.rs now consumes this new API, even though nothing implements
it yet.
- This will let us implement GetMicrodescsState without having to
directly mutate the netdir. The calling code also handles checking the
netdir against the circmgr for sufficiency, and updating the consensus
metadata in the store, meaning the revised GetMicrodescsState will not
have to perform these tasks.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
- The additional parameters passed to GetConsensusState are now passed
through all the states, and used as well.
- WriteNetDir doesn't have a now() or config() method any more, since
the states now get this from the runtime or the config parameters.
- This required modifying the tests to make a mocked runtime and custom
config directly, instead of using DirRcv for this purpose.
- Additionally, because we don't have to upgrade a weak reference for
DirState::dl_config(), that function no longer wraps its return value
in Result.
- (A bunch of the FIXMEs from the previous commit that introduced the
additional parameters have now been rectified as a result.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
- GetConsensusState::new now takes a set of parameters matching what it
actually needs, instead of just taking a writedir. (It still *does*
take a writedir, and indeed still uses it for basically everything,
but that will eventually go away.)
- Its call sites were updated.
- Some tests now need to take a runtime, and got indented a lot as a
result.
- Resetting was made non-functional, because we need to thread through
the parameters passed to GetConsensusState to all of the other
states, too. This will happen in a later commit.
|
| |
|
|
|
|
| |
- Given that this is effectively an implementation detail, it doesn't
really make sense to have it be in the crate root...
- (also, we're going to change it a bunch now)
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The new API is (roughly) as discussed in
https://gitlab.torproject.org/tpo/core/arti/-/issues/451
This is quite a large commit and it is not convenient to split it up.
It contains the following changes:
* Redo the list builder and accessor macros implemnetation,
including docs and tests.
* Change uses of define_list_config_builder. In each case:
- Move the docs about the default value to the containing field.
- Remove the other docs (which were just recapitulations, and
are now not needed since the ListBuilder is no longer public).
- Rewmove or replace `pub` in the define_list_builder_helper call,
so that the builder is no longer public.
- Change the main macro call site to use define_list_builder_helper.
- Add a call to define_list_builder_accessors.
* Make the module `list_builder` pub so that we have somewhere to
put the overview documentation.
* Consequential changes:
- Change `outer.inner().replace(X)` to `outer.set_inner(X)`
- Consequential changes to imports (`use` statements).
|
| |\
| |
| |
| |
| | |
DownloadSchedule: Introduce Builder
See merge request tpo/core/arti!473
|
| | |
| |
| |
| |
| |
| |
| | |
These fields are pub.
Retain two convenience accessor functions that access sub-fields of
network.
|
| | |
| |
| |
| | |
This entire struct is config. This is otiose.
|
| | |
| |
| |
| | |
We can just make the fields pub(crate).
|
| |\|
| |
| |
| |
| |
| | |
# Conflicts:
# crates/tor-config/Cargo.toml
# crates/tor-dirmgr/src/state.rs
# doc/semver_status.md
|
| | |
| |
| |
| |
| | |
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/471#note_2798024
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
NetworkConfigBuilder needs to not contain any validated structs, so
that its serde does not expose the validated details.
AuthorityListBuilder is what ought to go here - and it contains
Vec<AuthorityBuilder>, not Vec<Authority>. As a consequence, many
places now deal with AuthorityBuilder, rather than Authority.
|