[metrics-bugs] #20994 [Metrics/Onionoo]: invalid first_seen timestamp on bridges

[Tor Bug Tracker & Wiki]

#20994: invalid first_seen timestamp on bridges
-----------------------------+------------------------------
 Reporter:  cypherpunks      |          Owner:  metrics-team
     Type:  defect           |         Status:  merge_ready
 Priority:  Medium           |      Milestone:
Component:  Metrics/Onionoo  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:                   |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------
Changes (by iwakeh):

 * status:  needs_review => merge_ready


Comment:

 Replying to [comment:5 karsten]:
 > Replying to [comment:4 iwakeh]:
 > > The reasoning in comment:3 makes sense; the code seems to do what is
 suggested, but test(s) missing.
 >
 > Cool, thanks for looking!  There are indeed no tests for this class.
 But I'm afraid it's a small project to start writing tests for this class
 now, and that shouldn't block the bugfix.

 Hmm, I prefer having a coded proof of the fix.  It doesn't seem such a
 huge task from merely looking at the class.

 Well, I'll add a ticket for writing this test.

 >
 > > Couldn't there also be relays affected, b/c of the cached descriptor
 imports?
 >
 > Good question.  The answer is that relays are not affected, because
 `processRelayServerDescriptor()` did ''not'' contain a line
 `this.updatedNodes.add(fingerprint);` as it was contained in
 `processBridgeServerDescriptor()`.  We only put relay fingerprints into
 that set that we saw in consensuses, and that's how they all got valid
 `first_seen` timestamps.
 >
 > So, good to merge?

 Given that you're currently system-testing this fix (by re-importing) ok.
 Ready for merge.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20994#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online