mirrors/arti.git/crates/tor-proto/src/lib.rs, branch arti-v0.5.0

Run maint/add_warning crates/*/src/{lib,main}.rs

2022-06-23T18:15:42Z

Update all lint blocks

channel padding: Rename ChannelsParams from ChannelsConfig

2022-06-21T18:19:28Z

As per https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/586#note_2814276 Change names and comments and docs everywhere.

channel padding: Plumb settings from chanmgr

2022-06-21T18:19:22Z

try to differentiate transient from nontransient error

2022-06-08T15:02:01Z

lints: Add lint block delimiters to every crate

2022-05-31T12:00:31Z

This was the result of: maint/add_warning crates/*/src/{lib,main}.rs and then manually curating the results.

clippy: Change a binding to (), and allow clippy::let_unit_value

2022-05-11T17:52:12Z

The type of ret.map_err(codec_err_to_chan)? is (). ISTM that writing `let () = ` makes it clear that there is nothing there, but the lint forbids this. This lint is warn by default and trips here for me on current nightly. It seems wrong to me. We should be able to make it clear to the reader that there is nothing here - note how this differs from the lines below where Ready contains msg. A let () binding is a good way to do that. I think the lint allow ought to be added everywhere, but that doesn't seem easy right now - see this issue about maint/add_warning: https://gitlab.torproject.org/tpo/core/arti/-/issues/469

tor-proto: add a backend to detect reported clock skew.

2022-03-23T12:24:36Z

NETINFO cells, which are sent in every handshake, may contain timestamps. This patch adds an accessor for the timestamp in the Netinfo messages, and teaches the tor-proto code how to compute the minimum clock skew in the code. The computation isn't terribly precise, but it doesn't need to be: Tor should work fine if your clock is accurate to within a few hours. This patch also notes a Y2038 problem in the protocol: see torspec#80. Part of #405.

Merge branch 'clippy-allow-arc-clone' into 'main'

2022-03-01T20:38:05Z

Disable clippy::clone_on_ref_ptr See merge request tpo/core/arti!352

Make coarsetime dependency and traffic-timestamping non-optional.

2022-02-25T15:31:01Z

Previously coarsetime and the traffic-timestamp feature were enabled, since they were only required for a small corner of the guardmgr algorithm. But in 1.0 and beyond we'll be adding a bunch of other features (eg, netflow padding, DoS prevention) that will need coarsetime all over the place. And since we're going to be doing coarsetime all over the place, the previous justification for making traffic-timestamping optional (the tiny performance hit) is no longer relevant.

Disable clippy::clone_on_ref_ptr

2022-02-24T18:15:44Z

This lint is IMO inherently ill-conceived. I have looked for the reasons why this might be thought to be a good idea and there were basically two (and they are sort of contradictory): I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact that only the pointer is being cloned, not the underlying data." This is the wording from https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr It is a bit terse; we are left to infer why it is a bad idea to obscure this fact. It seems to me that if it is bad to obscure some fact, that must be because the fact is a hazard. But why would it be a hazard to not copy the underlying data ? In other languages, faliing to copy the underlying data is a serious correctness hazard. There is a whose class of bugs where things were not copied, and then mutated and/or reused in multiple places in ways that were not what the programmer intended. In my experience, this is a very common bug when writing Python and Javascript. I'm told it's common in golang too. But in Rust this bug is much much harder to write. The data inside an Arc is immutable. To have this bug you'd have use interior mutability - ie mess around with Mutex or RefCell. That provides a good barrier to these kind of accidents. II. "The reason for writing Rc::clone and Arc::clone [is] to make it clear that only the pointer is being cloned, as opposed to the underlying data. The former is always fast, while the latter can be very expensive depending on what is being cloned." This is the reasoning found here https://github.com/rust-lang/rust-clippy/issues/2048 This is saying that *not* using Arc::clone is hazardous. Specifically, that a deep clone is a performance hazard. But for this argument, the lint is precisely backwards. It's linting the "good" case and asking for it to be written in a more explicit way; while the supposedly bad case can be written conveniently. Also, many objects (in our codebase, and in all the libraries we use) that are Clone are in fact simply handles. They contain Arc(s) (or similar) and are cheap to clone. Indeed, that is the usual case. It does not make sense to distinguish in the syntax we use to clone such a handle, whether the handle is a transparent Arc, or an opaque struct containing one or more other handles. Forcing Arc::clone to be written as such makes for code churn when a type is changed from Arc to Something: Clone, or vice versa.