Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#22050 closed defect (fixed)

Consensus health is concerned about missing reveal values but it shouldn't

Reported by: asn Owned by: tom
Priority: Medium Milestone:
Component: Metrics/Consensus Health Version:
Severity: Normal Keywords: tor-hs consensus health tor-srv
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hello,

Sebastian the other day let me know that consensus health marks missing SRV reveal values as red, even when we are in commit phase and there shouldn't be any reveal values there.

e.g.:

[V:1 A:sha3-256 C:AAAAAFj9QADRCL8xJjqrEGCYLDD8rirah1ekzwwFtzwI1rbNUbr35g==(Empty)], 
[V:1 A:sha3-256 C:AAAAAFj9QACpdT+YSqMfmqydd0pifsVDh6RXjFdUHjWFxDrJFOfKGQ==(Empty)], 
[V:1 A:sha3-256 C:AAAAAFj9QACfd6bmG5brC5zEZT0OxA/q9b0pnTlWAkO45i0oaRah0Q==(Empty)], 
[V:1 A:sha3-256 C:AAAAAFj9QAA9PtA2l5UnnKvp+t9NA0FQlLPe/RobysQU8Is6Zskb6w==(Empty)], 
[V:1 A:sha3-256 C:AAAAAFj9QAB9yqvzx3kBe9Pm3dfOPDLsRQVhgLFvfFZ6FWil7gAPoQ==(Empty)], 

It's perfectly normal that there are no reveal values, because from 00:00 UTC to 12:00 UTC we have the commit phase where there are no reveals at all. From 12:00 UTC to 00:00 UTC there should be reveal values.

I can see two fixes here:
a) Easy fix: Stop having (Empty) with red letters to reduce reader anxiety.
b) Involved fix: Check whether the vote is in commit phase or reveal phase (using valid-after) and only put (Empty) there when we are in reveal phase and there should be a reveal value there.

Child Tickets

Change History (5)

comment:1 Changed 2 years ago by tom

I can fix this. I'd prefer to do Option 2, but....

The consensus generation times are all controlled by variables. This lets test networks generate consensuses every 10 minutes or so forth. Are the commit-reveal times also controlled by those variables? Or is it safe to hardcode the 00-12 and 12-00 timeframes?

comment:2 Changed 2 years ago by asn

Hmm I guess we want consensus-health to work on networks with all sorts of voting intervals, eh?

In this case, if we wanted consensus-health to derive the current SRV phase it would probably have to copy the little-t-tor logic:
https://gitweb.torproject.org/tor.git/tree/src/or/shared_random_state.c#n190

comment:3 Changed 2 years ago by tom

Right on, that works. I will fix this... probably sometime in the next month =)

comment:5 Changed 2 years ago by asn

I think that works out! Thanks!

Note: See TracTickets for help on using tickets.