Update diagnostics to fix bug in clock test

Also improves clock test to not use the ntp call if enough
connections are present to use the GetTimeOffset().

src/qt/diagnosticsdialog.cpp

@@ -232,15 +232,14 @@ double DiagnosticsDialog::VerifyETTSReasonable()
     return result;
 }
 
-int DiagnosticsDialog::VerifyCountSeedNodes()
+int DiagnosticsDialog::VerifyCountOutboundNodes()
 {
     LOCK(cs_vNodes);
 
     int count = 0;
 
     for (auto const& vnodes : vNodes)
-        if (!vnodes->fInbound)
-            count++;
+        if (!vnodes->fInbound) count++;
 
     return count;
 }
@@ -396,32 +395,26 @@ void DiagnosticsDialog::on_testButton_clicked()
         UpdateTestStatus("verifyWalletIsSynced", ui->verifyWalletIsSyncedResultLabel, completed, failed);
     }
 
-    // clock
-    UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, pending, NA);
-    this->repaint();
-
-    VerifyClock();
-
-    // seed nodes
-    UpdateTestStatus("verifySeedNodes", ui->verifySeedNodesResultLabel, pending, NA);
+    // outbound nodes
+    UpdateTestStatus("verifyOutboundNodes", ui->verifyOutboundNodesResultLabel, pending, NA);
     this->repaint();
 
-    unsigned int seed_node_connections = VerifyCountSeedNodes();
+    unsigned int outbound_node_connections = VerifyCountOutboundNodes();
 
-    if (seed_node_connections >= 1 && seed_node_connections < 3)
+    if (outbound_node_connections >= 1 && outbound_node_connections < 3)
     {
-        UpdateTestStatus("verifySeedNodes", ui->verifySeedNodesResultLabel, completed, warning,
-                         tr("Warning: Count = %1 (Pass = 3+)").arg(QString::number(seed_node_connections)));
+        UpdateTestStatus("verifyOutboundNodes", ui->verifyOutboundNodesResultLabel, completed, warning,
+                         tr("Warning: Count = %1 (Pass = 3+)").arg(QString::number(outbound_node_connections)));
     }
-    else if(seed_node_connections >= 3)
+    else if (outbound_node_connections >= 3)
     {
-        UpdateTestStatus("verifySeedNodes", ui->verifySeedNodesResultLabel, completed, passed,
-                         tr("Passed: Count = %1").arg(QString::number(seed_node_connections)));
+        UpdateTestStatus("verifyOutboundNodes", ui->verifyOutboundNodesResultLabel, completed, passed,
+                         tr("Passed: Count = %1").arg(QString::number(outbound_node_connections)));
     }
     else
     {
-        UpdateTestStatus("verifySeedNodes", ui->verifySeedNodesResultLabel, completed, failed,
-                         tr("Failed: Count = %1").arg(QString::number(seed_node_connections)));
+        UpdateTestStatus("verifyOutboundNodes", ui->verifyOutboundNodesResultLabel, completed, failed,
+                         tr("Failed: Count = %1").arg(QString::number(outbound_node_connections)));
     }
 
     // connection count
@@ -446,6 +439,12 @@ void DiagnosticsDialog::on_testButton_clicked()
                          tr("Failed: Count = %1").arg(QString::number(connections)));
     }
 
+    // clock
+    UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, pending, NA);
+    this->repaint();
+
+    VerifyClock(connections);
+
     // tcp port
     UpdateTestStatus("verifyTCPPort", ui->verifyTCPPortResultLabel, pending, NA);
     this->repaint();
@@ -470,8 +469,28 @@ void DiagnosticsDialog::on_testButton_clicked()
     DisplayOverallDiagnosticResult();
 }
 
-void DiagnosticsDialog::VerifyClock()
+void DiagnosticsDialog::VerifyClock(unsigned int connections)
 {
+
+    // This is aligned to the minimum sample size in AddTimeData to compute a nTimeOffset from the sampling of
+    // connected nodes. If a sufficient sample exists, use the existing time offset from the node rather than
+    // going through the ntp connection code. The ntp connection code is retained to help people resolve no connection
+    // situations where their node is so far off they are banned and disconnected from the network.
+    if (connections >= 5)
+    {
+        int64_t time_offset = 0;
+
+        {
+            LOCK(cs_main);
+
+            time_offset = GetTimeOffset();
+        }
+
+        clkReportResults(time_offset);
+
+        return;
+    }
+
     QTimer *timerVerifyClock = new QTimer();
 
     // Set up a timeout clock of 10 seconds as a fail-safe.
@@ -481,8 +500,10 @@ void DiagnosticsDialog::VerifyClock()
     QHostInfo NTPHost = QHostInfo::fromName("pool.ntp.org");
     udpSocket = new QUdpSocket(this);
 
-    connect(udpSocket, SIGNAL(stateChanged(QAbstractSocket::SocketState)), this, SLOT(clkStateChanged(QAbstractSocket::SocketState)));
-    connect(udpSocket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(clkSocketError()));
+    connect(udpSocket, SIGNAL(stateChanged(QAbstractSocket::SocketState)), this,
+            SLOT(clkStateChanged(QAbstractSocket::SocketState)));
+    connect(udpSocket, SIGNAL(error(QAbstractSocket::SocketError)), this,
+            SLOT(clkSocketError()));
 
     udpSocket->connectToHost(QHostAddress(NTPHost.addresses().first()), 123, QIODevice::ReadWrite);
 }
@@ -525,29 +546,19 @@ void DiagnosticsDialog::clkFinished()
             if (BufferSocket.size() == 48)
             {
                 int nNTPCount = 40;
-                unsigned long DateTimeIn = uchar(BufferSocket.at(nNTPCount))
+                uint32_t DateTimeIn = uchar(BufferSocket.at(nNTPCount))
                         + (uchar(BufferSocket.at(nNTPCount + 1)) << 8)
                         + (uchar(BufferSocket.at(nNTPCount + 2)) << 16)
                         + (uchar(BufferSocket.at(nNTPCount + 3)) << 24);
-                long tmit = ntohl((time_t)DateTimeIn);
-                tmit -= 2208988800U;
+                time_t tmit = (time_t) (ntohl(DateTimeIn) - 2208988800U);
 
                 udpSocket->close();
 
                 boost::posix_time::ptime localTime = boost::posix_time::microsec_clock::universal_time();
                 boost::posix_time::ptime networkTime = boost::posix_time::from_time_t(tmit);
                 boost::posix_time::time_duration timeDiff = networkTime - localTime;
 
-                if (timeDiff.minutes() < 3)
-                {
-                    UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, passed);
-                }
-                else
-                {
-                    UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, failed);
-                }
-
-                DisplayOverallDiagnosticResult();
+                clkReportResults(timeDiff.total_seconds());
 
                 return;
             }
@@ -559,16 +570,41 @@ void DiagnosticsDialog::clkFinished()
         // above, then when the timer calls clkFinished again, it will hit this conditional and be a no-op.
         if (GetTestStatus("verifyClockResult") != completed)
         {
-            UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel,completed, warning,
-                             tr("Warning: Cannot connect to NTP server"));
-
-            DisplayOverallDiagnosticResult();
+            clkReportResults(0, true);
         }
 
         return;
     }
 }
 
+void DiagnosticsDialog::clkReportResults(const int64_t& time_offset, const bool& timeout_during_check)
+{
+    if (!timeout_during_check)
+    {
+        if (abs64(time_offset) < 3 * 60)
+        {
+            UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, passed);
+        }
+        else if (abs64(time_offset) < 5 * 60)
+        {
+            UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, warning,
+                             tr("Warning: Clock skew is between 3 and 5 minutes. Please check your clock settings."));
+        }
+        else
+        {
+            UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, failed,
+                             tr("Error: Clock skew is 5 minutes or greater. Please check your clock settings."));
+        }
+    }
+    else
+    {
+        UpdateTestStatus("verifyClockResult", ui->verifyClockResultLabel, completed, warning,
+                         tr("Warning: Cannot connect to NTP server"));
+    }
+
+    DisplayOverallDiagnosticResult();
+}
+
 void DiagnosticsDialog::VerifyTCPPort()
 {
     tcpSocket = new QTcpSocket(this);

src/qt/diagnosticsdialog.h

@@ -48,15 +48,15 @@ class DiagnosticsDialog : public QDialog
 private:
     Ui::DiagnosticsDialog *ui;
     void GetData();
-    void VerifyClock();
+    void VerifyClock(unsigned int connections);
     void VerifyTCPPort();
     bool VerifyBoincPath();
     bool VerifyCPIDIsEligible();
     bool VerifyWalletIsSynced();
     bool VerifyIsCPIDValid();
     bool VerifyCPIDHasRAC();
     double VerifyETTSReasonable();
-    int VerifyCountSeedNodes();
+    int VerifyCountOutboundNodes();
     int VerifyCountConnections();
     double GetTotalCPIDRAC(std::string cpid);
     double GetUserRAC(std::string cpid, int *projects);
@@ -105,6 +105,7 @@ private slots:
     void clkFinished();
     void clkStateChanged(QAbstractSocket::SocketState state);
     void clkSocketError();
+    void clkReportResults(const int64_t& time_offset, const bool& timeout_during_check = false);
     void TCPFinished();
     void TCPFailed(QAbstractSocket::SocketError socketError);
 };

src/qt/forms/diagnosticsdialog.ui

@@ -94,7 +94,7 @@
       </widget>
      </item>
      <item row="8" column="1">
-      <widget class="QLabel" name="verifySeedNodesResultLabel">
+      <widget class="QLabel" name="verifyOutboundNodesResultLabel">
        <property name="sizePolicy">
         <sizepolicy hsizetype="MinimumExpanding" vsizetype="Preferred">
          <horstretch>0</horstretch>
@@ -372,9 +372,9 @@
       </widget>
      </item>
      <item row="8" column="0">
-      <widget class="QLabel" name="verifySeedNodesLabel">
+      <widget class="QLabel" name="verifyOutboundNodesLabel">
        <property name="text">
-        <string>Verify connections to seeds</string>
+        <string>Verify outbound connections</string>
        </property>
       </widget>
      </item>