Remove the known digest check from channel_check_for_duplicates()

[teor]

This is in arma's commit f5ff9f23 in his bug24898-more branch.

This comment is wrong, the check is already implemented in channel_add_to_digest_map():

+    /* XXX we might want to replace this logic with a call to
+     * channel_is_client() instead. Or maybe we can change
+     * channel_identity_mapso it refuses to add any channels that are
+     * client channels? */

So we can remove the check entirely.

[arma]

Replying to teor:

the check is already implemented in channel_add_to_digest_map()

Really? Where in channel_add_to_digest_map does it check if channel_is_client() and abort?

I see a check for chan->identity_digest, but it's not obvious to me that that's the same thing, especially wrt, say, handshake-proto=2 connections. It would sure be simpler if we could decide to make them the same thing though.

[teor]

Ok, let's make channel_add_to_digest_map check for channel_is_client().

[arma]

Replying to teor:

Ok, let's make channel_add_to_digest_map check for channel_is_client().

Actually, for whatever version we decide to backport the more-accurate channel_is_client() to, I think we would be wise to work towards having an invariant where, when channel_is_client is true, conn->identity_digest is zero. I think we're not too far away from having that be true now, and it would sure simplify things for us rather than forcing us to sprinkle in tiny confusing exception clauses everywhere.

[teor]

I think this would help - should we even be storing v1 and v2 client certificate fingerprints anyway?

But there are still two separate questions we can ask, and we should keep them separate:

• channel_is_client(): does the remote end act like a client (use v1 or v2, or not authenticate and use v3 or v4)
• …is_known_relay(): was the remote end authenticated and in a recent consensus?

There is a small gap between these two cases, filled with unpublished relays.

[nickm]

Marking all tickets reached by current round of 033 triage.

[nickm]

Mark all not-already-included tickets as pending review for removal from 0.3.3 milestone.

[nickm]

These tickets were marked as removed, and nobody has said that they can fix them. Let's remember to look at 033-removed-20180320 as we re-evaluate our triage process, to see whether we're triaging out unnecessarily, and to evaluate whether we're deferring anything unnecessarily. But for now, we can't do these: we need to fix the 033-must stuff now.