We have two conflicting notions of channel_is_client()
In channel_tls_process_netinfo_cell() we have a section that says ``` if (!(chan->conn->handshake_state->authenticated)) { tor_assert(tor_digest_is_zero( (const char*)(chan->conn->handshake_state-> authenticated_rsa_peer_id))); tor_assert(tor_mem_is_zero( (const char*)(chan->conn->handshake_state-> authenticated_ed25519_peer_id.pubkey), 32)); /* If the client never authenticated, it's a tor client or bridge * relay, and we must not use it for EXTEND requests (nor could we, as * there are no authenticated peer IDs) */ channel_mark_client(TLS_CHAN_TO_BASE(chan)); ``` That's great and awesome -- it's a clear solid definition of client: a client is somebody who did the client handshake with us, not the relay handshake. But then in command.c we have this weird clause: ``` if (connection_or_digest_is_known_relay(chan->identity_digest)) { rep_hist_note_circuit_handshake_requested(create_cell->handshake_type); // Needed for chutney: Sometimes relays aren't in the consensus yet, and // get marked as clients. This resets their channels once they appear. // Probably useful for normal operation wrt relay flapping, too. channel_clear_client(chan); } else { channel_mark_client(chan); } ``` This definition of client is much murkier: a client is anybody who, at this moment of receiving a create cell, had earlier presented an identity digest that isn't in our current consensus and that we don't have a cached descriptor for. I think we should standardize on one definition, and I think we should pick the first one: it's clean and not full of false positives.
issue