Opened 3 years ago

Closed 2 years ago

#21367 closed enhancement (fixed)

mark platform string red (or in some other obvious way) if recommended_version field is not true

Reported by: cypherpunks Owned by: RaBe
Priority: Medium Milestone:
Component: Metrics/Relay Search Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Goal: make it obvious to tor relay ops that they run an outdated version

'recommended_version' boolean is provided by onionoo (details document).

Child Tickets

Attachments (1)

0001-Highlight-not-recommended-Tor-versions-with-labels.patch (4.3 KB) - added by cypherpunks 2 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 3 years ago by karsten

Good idea!

comment:2 Changed 3 years ago by cypherpunks

Please flag not recommended relays in the search results table as red entries + tooltip text.

Use case:
"show me my outdated relays"

https://lists.torproject.org/pipermail/tor-relays/2017-February/011921.html

Last edited 3 years ago by cypherpunks (previous) (diff)

comment:3 Changed 2 years ago by RaBe

Status: newneeds_review

Relays running a not recommended software version are now flagged, and in the detail view, also the software version is colored red:

https://github.com/RaphaelBergmann/atlas/commit/3844568c5e279cbd8fabed26f4fb9b407468f348

Last edited 2 years ago by RaBe (previous) (diff)

comment:4 Changed 2 years ago by cypherpunks

Coloring the software version red is redundant as there is already a bright red icon in the flags. Also the grammar of the tooltip could be improved (see the inline patch for my suggestion).

diff --git a/js/models/relay.js b/js/models/relay.js
index 2a4fe17..f4ea7c6 100644
--- a/js/models/relay.js
+++ b/js/models/relay.js
@@ -56,7 +56,7 @@ define([
                     output.push([flag,"cloud_download_"+size[0], "This relay is more useful for building general-purpose exit circuits than for relay circuits."]);
                 }
                 if (flag == "Not Recommended") {
-                    output.push([flag,"not_recommended_"+size[0], "This relay is running a not recommended software version."]);
+                    output.push([flag,"not_recommended_"+size[0], "This relay is running a software version that is not recommended by the directory authorities."]);
                 }
             });
             return output;

comment:6 Changed 2 years ago by cypherpunks

LGTM

comment:7 Changed 2 years ago by irl

Resolution: fixed
Status: needs_reviewclosed

Merged, thanks. (:

comment:8 Changed 2 years ago by cypherpunks

Resolution: fixed
Status: closedreopened

Thanks for implementing this feature request.

Some minor suggestions:
is:
"This relay is running a software version that is not recommended by the directory authorities."

better:
"This relay is running a tor version that is not recommended by the directory authorities. If you are the operator of this relay please update tor."

comment:9 Changed 2 years ago by cypherpunks

Not sure if URLs are possible but giving the (potential) operator specific information on how to get rid of this red warning, like "What version is recommended?" is certainly good.

https://consensus-health.torproject.org/#recommendedversions

comment:10 Changed 2 years ago by irl

As the flag description in the tooltip, I think I do like "This relay is running a tor version that is not recommended by the directory authorities." but I don't think this is the place to give an extended recommendation to the operator.

I've played with the idea of a "Help" page before: https://irl.github.io/atlas/#/help

Do you think this could be a good place for recommendations for relay operators if we make sure it's prominent enough? This would probably be related to #6787.

comment:11 Changed 2 years ago by cypherpunks

Ok lets change it to (as you suggested)
"This relay is running a tor version that is not recommended by the directory authorities."

"...software version..." is a bit to generic I think.

On the relay page I would change
"Not Recommended"
to
"Not Recommended Version"
(hoping that this is not to long)

"Not recommended" might suggest that people blacklist this relay in their client torrc - which is not what we are trying to say.

On the relay page:
Red sign would fit better next to the "Platform" string, since this is technically not a flag, that would make it also more obvious to understand what is wrong with this relay.

Last edited 2 years ago by cypherpunks (previous) (diff)

comment:12 Changed 2 years ago by irl

might suggest that people blacklist this relay in their client torrc

This is a scary enough thought to consider changing this. What about "NoRecVer"? It's a little cryptic but the tooltip is still there to explain and it's then a similar length to the others. I don't think this is any worse than V2Dir or HSDir.

comment:13 in reply to:  12 Changed 2 years ago by cypherpunks

Replying to irl:

might suggest that people blacklist this relay in their client torrc

This is a scary enough thought to consider changing this. What about "NoRecVer"? It's a little cryptic but the tooltip is still there to explain and it's then a similar length to the others. I don't think this is any worse than V2Dir or HSDir.

Please do not make this more cryptic.

Moving the sign to the platform string and changing it from "not recommended"
to
"runs not recommended version"
should be fine.

Unlike V2Dir and HSDir this is not a flag and this information is actual actionable information to relay ops.

Last edited 2 years ago by cypherpunks (previous) (diff)

comment:14 Changed 2 years ago by irl

Status: reopenedneeds_review

Ok, how about this?

https://gitweb.torproject.org/user/irl/atlas.git/patch/?id=d2ff11c76cde78c6aea2b6c132a818ee5e465529

Removing the psuedoflag and instead adding the warning next to the platform string.

comment:15 in reply to:  14 ; Changed 2 years ago by teor

Replying to irl:

Ok, how about this?

https://gitweb.torproject.org/user/irl/atlas.git/patch/?id=d2ff11c76cde78c6aea2b6c132a818ee5e465529

Removing the psuedoflag and instead adding the warning next to the platform string.

I actually found the pseudoflag really useful, because it shows up in the relay table, so I can search all my relays at once and check if any are out of date.

comment:16 in reply to:  15 ; Changed 2 years ago by cypherpunks

Replying to teor:

Replying to irl:

Ok, how about this?

https://gitweb.torproject.org/user/irl/atlas.git/patch/?id=d2ff11c76cde78c6aea2b6c132a818ee5e465529

Removing the psuedoflag and instead adding the warning next to the platform string.

I actually found the pseudoflag really useful, because it shows up in the relay table, so I can search all my relays at once and check if any are out of date.

Maybe we can append the flag to the nickname in the search results?

Also I'm in favor of replacing the flag icon with a label. Labels are text with some styling which scale better than images and prevents the introduction of another instance of #19538.

Lastly, i noticed Onionoo doesn't add the recommended_version field to bridge details documents. Should i open another ticket for this or is this intentional?

comment:17 Changed 2 years ago by cypherpunks

I've uploaded a patch which reflects what i said in comment:16.

comment:18 in reply to:  16 Changed 2 years ago by karsten

Replying to cypherpunks:

Lastly, i noticed Onionoo doesn't add the recommended_version field to bridge details documents. Should i open another ticket for this or is this intentional?

The reason is that the bridge authority does not recommend versions, so that we'd have to take version recommendations from relay network status consensuses and apply that to bridges. There may be other issues. But I see how this would be potentially useful. Please open an Onionoo ticket for this if you want this feature to be added to Onionoo.

comment:19 Changed 2 years ago by teor

I think the icon should remain red (like the "down" dot), rather than being black.

comment:20 Changed 2 years ago by RaBe

By moving this icon to its own column in #21635, I think it's quite noticeable even in black :) We don't want the red = bad = blacklisting thing we talked about starting in comment:11...

comment:21 in reply to:  20 Changed 2 years ago by teor

Replying to RaBe:

By moving this icon to its own column in #21635, I think it's quite noticeable even in black :) We don't want the red = bad = blacklisting thing we talked about starting in comment:11...

OK, let's see how it goes :-)

comment:22 Changed 2 years ago by RaBe

Owner: changed from irl to RaBe
Status: needs_reviewassigned

comment:23 Changed 2 years ago by cypherpunks

With the merge of #21635 to master commit 251ba71de9ce116f8380f61fdcf3937038acabb6 is now live thus this ticket can be closed as fixed.

comment:24 in reply to:  23 Changed 2 years ago by irl

Resolution: fixed
Status: assignedclosed

Replying to cypherpunks:

With the merge of #21635 to master commit 251ba71de9ce116f8380f61fdcf3937038acabb6 is now live thus this ticket can be closed as fixed.

Agreed.

Note: See TracTickets for help on using tickets.