Merge #10189: devtools/net: add a verifier for scriptable changes. Use it to make CNode::id private.

0f3471f net: make CNode's id private (Cory Fields)
9ff0a51 scripted-diff: net: Use accessor rather than node's id directly (Cory Fields)
e50c33e devtools: add script to verify scriptable changes (Cory Fields)

Tree-SHA512: a0ff50f4e1d38a2b63109b4996546c91b3e02e00d92c0bf04f48792948f78b1f6d9227a15d25c823fd4723a0277fc6a32c2c1287c7abbb7e50fd82ffb0f8d994

Author: laanwj

.travis.yml

@@ -43,6 +43,7 @@ install:
     - if [ -n "$PACKAGES" ]; then travis_retry sudo apt-get update; fi
     - if [ -n "$PACKAGES" ]; then travis_retry sudo apt-get install --no-install-recommends --no-upgrade -qq $PACKAGES; fi
 before_script:
+    - if [ "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/commit-script-check.sh $TRAVIS_COMMIT_RANGE; fi
     - unset CC; unset CXX
     - if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/check-doc.py; fi
     - mkdir -p depends/SDKs depends/sdk-sources

contrib/devtools/commit-script-check.sh

@@ -0,0 +1,39 @@
+#!/bin/sh
+# Copyright (c) 2017 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+# This simple script checks for commits beginning with: scripted-diff:
+# If found, looks for a script between the lines -BEGIN VERIFY SCRIPT- and
+# -END VERIFY SCRIPT-. If no ending is found, it reads until the end of the
+# commit message.
+
+# The resulting script should exactly transform the previous commit into the current
+# one. Any remaining diff signals an error.
+
+if test "x$1" = "x"; then
+    echo "Usage: $0 <commit>..."
+    exit 1
+fi
+
+RET=0
+PREV_BRANCH=`git name-rev --name-only HEAD`
+PREV_HEAD=`git rev-parse HEAD`
+for i in `git rev-list --reverse $1`; do
+    git rev-list -n 1 --pretty="%s" $i | grep -q "^scripted-diff:" || continue
+    git checkout --quiet $i^ || exit
+    SCRIPT="`git rev-list --format=%b -n1 $i | sed '/^-BEGIN VERIFY SCRIPT-$/,/^-END VERIFY SCRIPT-$/{//!b};d'`"
+    if test "x$SCRIPT" = "x"; then
+        echo "Error: missing script for: $i"
+        echo "Failed"
+        RET=1
+    else
+        echo "Running script for: $i"
+        echo "$SCRIPT"
+        eval "$SCRIPT"
+        git --no-pager diff --exit-code $i && echo "OK" || (echo "Failed"; false) || RET=1
+    fi
+    git reset --quiet --hard HEAD
+done
+git checkout --quiet $PREV_BRANCH 2>/dev/null || git checkout --quiet $PREV_HEAD
+exit $RET

src/net.cpp

@@ -948,7 +948,7 @@ bool CConnman::AttemptToEvictConnection()
                 continue;
             if (node->fDisconnect)
                 continue;
-            NodeEvictionCandidate candidate = {node->id, node->nTimeConnected, node->nMinPingUsecTime,
+            NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime,
                                                node->nLastBlockTime, node->nLastTXTime,
                                                (node->nServices & nRelevantServices) == nRelevantServices,
                                                node->fRelayTxes, node->pfilter != NULL, node->addr, node->nKeyedNetGroup};
@@ -1374,7 +1374,7 @@ void CConnman::ThreadSocketHandler()
             {
                 if (pnode->nLastRecv == 0 || pnode->nLastSend == 0)
                 {
-                    LogPrint(BCLog::NET, "socket no message in first 60 seconds, %d %d from %d\n", pnode->nLastRecv != 0, pnode->nLastSend != 0, pnode->id);
+                    LogPrint(BCLog::NET, "socket no message in first 60 seconds, %d %d from %d\n", pnode->nLastRecv != 0, pnode->nLastSend != 0, pnode->GetId());
                     pnode->fDisconnect = true;
                 }
                 else if (nTime - pnode->nLastSend > TIMEOUT_INTERVAL)
@@ -1394,7 +1394,7 @@ void CConnman::ThreadSocketHandler()
                 }
                 else if (!pnode->fSuccessfullyConnected)
                 {
-                    LogPrintf("version handshake timeout from %d\n", pnode->id);
+                    LogPrintf("version handshake timeout from %d\n", pnode->GetId());
                     pnode->fDisconnect = true;
                 }
             }
@@ -2487,7 +2487,7 @@ bool CConnman::DisconnectNode(NodeId id)
 {
     LOCK(cs_vNodes);
     for(CNode* pnode : vNodes) {
-        if (id == pnode->id) {
+        if (id == pnode->GetId()) {
             pnode->fDisconnect = true;
             return true;
         }
@@ -2625,10 +2625,10 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     nTimeConnected(GetSystemTimeInSeconds()),
     addr(addrIn),
     fInbound(fInboundIn),
-    id(idIn),
     nKeyedNetGroup(nKeyedNetGroupIn),
     addrKnown(5000, 0.001),
     filterInventoryKnown(50000, 0.000001),
+    id(idIn),
     nLocalHostNonce(nLocalHostNonceIn),
     nLocalServices(nLocalServicesIn),
     nMyStartingHeight(nMyStartingHeightIn),
@@ -2744,7 +2744,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
 {
     size_t nMessageSize = msg.data.size();
     size_t nTotalSize = nMessageSize + CMessageHeader::HEADER_SIZE;
-    LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n",  SanitizeString(msg.command.c_str()), nMessageSize, pnode->id);
+    LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n",  SanitizeString(msg.command.c_str()), nMessageSize, pnode->GetId());
 
     std::vector<unsigned char> serializedHeader;
     serializedHeader.reserve(CMessageHeader::HEADER_SIZE);
@@ -2782,7 +2782,7 @@ bool CConnman::ForNode(NodeId id, std::function<bool(CNode* pnode)> func)
     CNode* found = nullptr;
     LOCK(cs_vNodes);
     for (auto&& pnode : vNodes) {
-        if(pnode->id == id) {
+        if(pnode->GetId() == id) {
             found = pnode;
             break;
         }

src/net.h

@@ -611,7 +611,6 @@ class CNode
     CCriticalSection cs_filter;
     CBloomFilter* pfilter;
     std::atomic<int> nRefCount;
-    const NodeId id;
 
     const uint64_t nKeyedNetGroup;
     std::atomic_bool fPauseRecv;
@@ -682,6 +681,7 @@ class CNode
 private:
     CNode(const CNode&);
     void operator=(const CNode&);
+    const NodeId id;
 
 
     const uint64_t nLocalHostNonce;