rpc: Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test #13288

pull Empact wants to merge 1 commits into bitcoin:master from Empact:get-difficulty changing 4 files +10 −69

Empact commented at 9:17 PM on May 20, 2018: member
By dropping the chain argument to GetDifficulty. GetDifficulty was called in two ways:
- with a guaranteed non-null blockindex
- with no argument
Change the latter case to be provided chainActive.Tip() explicitly.

Introduced in: #11748
Empact commented at 9:18 PM on May 20, 2018: member

This conflicts with #13230. Will rebase when that goes in.
Empact renamed this:
~~Remove the need to include "rpc/blockchain.cpp' in order to put `GetDifficulty` under test~~
Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test
on May 20, 2018
Empact renamed this:
~~Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test~~
refactoring: Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test
on May 20, 2018
Empact commented at 10:48 PM on May 20, 2018: member

/cc #11748
fanquake added the label Refactoring on May 20, 2018

Drop the chain argument to GetDifficulty

This removes the need to include rpc/blockchain.cpp in order to put
GetDifficulty under test. GetDifficulty was called in two ways:
* with a guaranteed non-null blockindex
* with no argument

Change the latter case to be provided chainActive.Tip() explicitly.

ebec7317ca

Empact force-pushed on May 21, 2018
Empact commented at 5:23 AM on May 21, 2018: member

Dropped the now-unused chain.h include as well.

practicalswift commented at 7:14 AM on May 21, 2018: contributor

Concept ACK

What about adding this to lint-includes.sh?

diff --git a/contrib/devtools/lint-includes.sh b/contrib/devtools/lint-includes.sh
index 0f9fec4..97b037f 100755
--- a/contrib/devtools/lint-includes.sh
+++ b/contrib/devtools/lint-includes.sh
@@ -40,4 +40,12 @@ if [[ ${QUOTE_SYNTAX_INCLUDES} != "" ]]; then
     EXIT_CODE=1
 fi

+INCLUDED_CPP_FILES=$(git grep -E "^#include <[^>]*\.cpp>" -- "**/*.cpp" "**/*.h")
+if [[ ${INCLUDED_CPP_FILES} != "" ]]; then
+    echo "The following files #include .cpp files:"
+    echo "${INCLUDED_CPP_FILES}"
+    echo
+    EXIT_CODE=1
+fi
+
 exit ${EXIT_CODE}

Empact commented at 7:31 AM on May 21, 2018: member

@practicalswift I'm in favor - we'd need to merge this and #13291 to get something like that to pass, so how about as a follow-up?
MarcoFalke added the label RPC/REST/ZMQ on May 21, 2018
in src/rpc/blockchain.cpp:345 in ebec7317ca
```
 341 | @@ -352,7 +342,7 @@ static UniValue getdifficulty(const JSONRPCRequest& request)
 342 |          );
 343 |  
 344 |      LOCK(cs_main);
 345 | -    return GetDifficulty();
 346 | +    return GetDifficulty(chainActive.Tip());
```
MarcoFalke commented at 4:19 PM on May 21, 2018:

style nit: Personally I prefer ::chainActive for globals.

Empact commented at 3:16 AM on May 22, 2018:

I like this too but I'm going to leave this PR be. I have a follow-up I think will accommodate it better.
MarcoFalke approved
MarcoFalke commented at 4:19 PM on May 21, 2018: member

utACK ebec7317ca1acbc65afa7fb08fc219c315fc4527
kallewoof approved
kallewoof commented at 3:06 AM on May 22, 2018: member

utACK ebec7317ca1acbc65afa7fb08fc219c315fc4527
practicalswift commented at 1:16 PM on May 24, 2018: contributor

utACK ebec7317ca1acbc65afa7fb08fc219c315fc4527
MarcoFalke renamed this:
~~refactoring: Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test~~
rpc: Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test
on May 24, 2018
laanwj commented at 6:38 PM on June 5, 2018: member

utACK ebec7317ca1acbc65afa7fb08fc219c315fc4527
laanwj merged this on Jun 5, 2018
laanwj closed this on Jun 5, 2018
laanwj referenced this in commit a589f536b5 on Jun 5, 2018
Empact deleted the branch on Jun 6, 2018
fanquake referenced this in commit 68bfc0bce3 on Sep 3, 2018
laanwj referenced this in commit 788b8a290c on Sep 4, 2018
pstratem referenced this in commit 4b7e190382 on Sep 6, 2018
jfhk referenced this in commit 7b350687e3 on Nov 14, 2018
HashUnlimited referenced this in commit fb0269fd52 on Nov 26, 2018
kallewoof referenced this in commit e51ed82876 on Oct 4, 2019
PastaPastaPasta referenced this in commit 6cffc0839c on Jun 17, 2020
PastaPastaPasta referenced this in commit 43e187ea3f on Jul 2, 2020
PastaPastaPasta referenced this in commit 044aa7c5c4 on Jun 27, 2021
PastaPastaPasta referenced this in commit 15088bd95c on Jun 28, 2021
PastaPastaPasta referenced this in commit 5b47ba75e6 on Jun 29, 2021
PastaPastaPasta referenced this in commit aa5ae54d3c on Jun 29, 2021
PastaPastaPasta referenced this in commit ca7b75ecb5 on Jun 29, 2021
PastaPastaPasta referenced this in commit a128cd945b on Jun 29, 2021
PastaPastaPasta referenced this in commit 54c04f4bb5 on Jun 29, 2021
PastaPastaPasta referenced this in commit 1a8f904a0b on Jul 1, 2021
Munkybooty referenced this in commit 4f986a5290 on Jul 1, 2021
Munkybooty referenced this in commit dc526ae3c4 on Jul 2, 2021
Munkybooty referenced this in commit d18087026c on Jul 3, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Refactoring RPC/REST/ZMQ

Linked (view graph)

#11748 [Tests] Adding unit tests for GetDifficulty in blockchain.cpp.