Merge #21042: doc, test: Improve setup_clean_chain documentation

590bda79e876d9b959083105b8c7c41dd87706eb scripted-diff: Remove setup_clean_chain if default is not changed (Fabian Jahr)
98892f39e3d079c73bff7f2a5d5420fa95270497 doc: Improve setup_clean_chain documentation (Fabian Jahr)

Pull request description:

  The first commit improves documentation on setup_clean_chain which is misunderstood quite frequently. Most importantly it fixes the TestShell docs which are simply incorrect.

  The second commit removes the instances of `setup_clean_clain` in functional tests where it is not changing the default.

  This used to be part of #19168 which also sought to rename`setup_clean_chain`.

ACKs for top commit:
  jonatack:
    ACK 590bda79e876d9b959083105b8c7c41dd87706eb

Tree-SHA512: a7881186e65d31160b8f84107fb185973b37c6e50f190a85c6e2906a13a7472bb4efa9440bd37fe0a9ac5cd2d1e8559870a7e4380632d9a249eca8980b945f3e

18 files changed, 11 insertions, 20 deletions

diff --git a/test/functional/README.md b/test/functional/README.md
index 2d04413eb2..d830ba0334 100644
--- a/test/functional/README.md
+++ b/test/functional/README.md
@@ -63,10 +63,13 @@ don't have test cases for.
 - Avoid stop-starting the nodes multiple times during the test if possible. A
   stop-start takes several seconds, so doing it several times blows up the
   runtime of the test.
-- Set the `self.setup_clean_chain` variable in `set_test_params()` to control whether
-  or not to use the cached data directories. The cached data directories
-  contain a 200-block pre-mined blockchain and wallets for four nodes. Each node
-  has 25 mature blocks (25x50=1250 BTC) in its wallet.
+- Set the `self.setup_clean_chain` variable in `set_test_params()` to `True` to
+  initialize an empty blockchain and start from the Genesis block, rather than
+  load a premined blockchain from cache with the default value of `False`. The
+  cached data directories contain a 200-block pre-mined blockchain with the
+  spendable mining rewards being split between four nodes. Each node has 25
+  mature block subsidies (25x50=1250 BTC) in its wallet. Using them is much more
+  efficient than mining blocks in your test.
 - When calling RPCs with lots of arguments, consider using named keyword
   arguments instead of positional arguments to make the intent of the call
   clear to readers.
diff --git a/test/functional/example_test.py b/test/functional/example_test.py
index 97f24e1b6e..a0eb213a78 100755
--- a/test/functional/example_test.py
+++ b/test/functional/example_test.py
@@ -76,6 +76,9 @@ class ExampleTest(BitcoinTestFramework):
         """Override test parameters for your individual test.
 
         This method must be overridden and num_nodes must be explicitly set."""
+        # By default every test loads a pre-mined chain of 200 blocks from cache.
+        # Set setup_clean_chain to True to skip this and start from the Genesis
+        # block.
         self.setup_clean_chain = True
         self.num_nodes = 3
         # Use self.extra_args to change command-line arguments for the nodes
diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py
index 2c6553fbe2..5fcecb4882 100755
--- a/test/functional/feature_asmap.py
+++ b/test/functional/feature_asmap.py
@@ -36,7 +36,6 @@ def expected_messages(filename):
 
 class AsmapTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def test_without_asmap_arg(self):
diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py
index f9ece244fb..2b56bc78f5 100755
--- a/test/functional/feature_dbcrash.py
+++ b/test/functional/feature_dbcrash.py
@@ -49,7 +49,6 @@ from test_framework.util import (
 class ChainstateWriteCrashTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 4
-        self.setup_clean_chain = False
         self.rpc_timeout = 480
         self.supports_cli = False
 
diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
index 6f1a0cd348..f22b7f266a 100755
--- a/test/functional/feature_includeconf.py
+++ b/test/functional/feature_includeconf.py
@@ -20,7 +20,6 @@ from test_framework.test_framework import BitcoinTestFramework
 
 class IncludeConfTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def setup_chain(self):
diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py
index a63c3a3287..a04ba5db2d 100755
--- a/test/functional/p2p_add_connections.py
+++ b/test/functional/p2p_add_connections.py
@@ -17,7 +17,6 @@ def check_node_connections(*, node, num_in, num_out):
 
 class P2PAddConnections(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 2
 
     def setup_network(self):
diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py
index 91fbd722cf..69821763bd 100755
--- a/test/functional/p2p_addr_relay.py
+++ b/test/functional/p2p_addr_relay.py
@@ -46,7 +46,6 @@ class AddrReceiver(P2PInterface):
 
 class AddrTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def run_test(self):
diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py
index c592ab52b1..6584efae79 100755
--- a/test/functional/p2p_blocksonly.py
+++ b/test/functional/p2p_blocksonly.py
@@ -15,7 +15,6 @@ from test_framework.util import assert_equal
 
 class P2PBlocksOnly(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
         self.extra_args = [["-blocksonly"]]
 
diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py
index 642a217047..458e5235b6 100755
--- a/test/functional/p2p_filter.py
+++ b/test/functional/p2p_filter.py
@@ -81,7 +81,6 @@ class P2PBloomFilter(P2PInterface):
 
 class FilterTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
         self.extra_args = [[
             '-peerbloomfilters',
diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py
index 2b75ad5175..d375af6fe1 100755
--- a/test/functional/p2p_getaddr_caching.py
+++ b/test/functional/p2p_getaddr_caching.py
@@ -41,7 +41,6 @@ class AddrReceiver(P2PInterface):
 
 class AddrTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def run_test(self):
diff --git a/test/functional/p2p_invalid_locator.py b/test/functional/p2p_invalid_locator.py
index e4fc9fd178..f884cf90ff 100755
--- a/test/functional/p2p_invalid_locator.py
+++ b/test/functional/p2p_invalid_locator.py
@@ -13,7 +13,6 @@ from test_framework.test_framework import BitcoinTestFramework
 class InvalidLocatorTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        self.setup_clean_chain = False
 
     def run_test(self):
         node = self.nodes[0]  # convenience reference to the node
diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py
index 8a751c6b54..4bf96cb0e6 100755
--- a/test/functional/p2p_tx_download.py
+++ b/test/functional/p2p_tx_download.py
@@ -56,7 +56,6 @@ MAX_GETDATA_INBOUND_WAIT = GETDATA_TX_INTERVAL + INBOUND_PEER_TX_DELAY + TXID_RE
 
 class TxDownloadTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 2
 
     def test_tx_requests(self):
diff --git a/test/functional/rpc_estimatefee.py b/test/functional/rpc_estimatefee.py
index 81862ac69e..51b7efb4c3 100755
--- a/test/functional/rpc_estimatefee.py
+++ b/test/functional/rpc_estimatefee.py
@@ -14,7 +14,6 @@ from test_framework.util import assert_raises_rpc_error
 
 class EstimateFeeTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def run_test(self):
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index b364077a9a..ed6abaed78 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -24,7 +24,6 @@ MAX_BIP125_RBF_SEQUENCE = 0xfffffffd
 class PSBTTest(BitcoinTestFramework):
 
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 3
         self.extra_args = [
             ["-walletrbf=1"],
diff --git a/test/functional/test-shell.md b/test/functional/test-shell.md
index f6ea9ef682..b8e899d675 100644
--- a/test/functional/test-shell.md
+++ b/test/functional/test-shell.md
@@ -178,7 +178,7 @@ can be called after the TestShell is shut down.
 | `num_nodes` | `1` | Sets the number of initialized bitcoind processes. |
 | `perf` | False | Profiles running nodes with `perf` for the duration of the test if set to `True`. |
 | `rpc_timeout` | `60` | Sets the RPC server timeout for the underlying bitcoind processes. |
-| `setup_clean_chain` | `False` | Initializes an empty blockchain by default. A 199-block-long chain is initialized if set to `True`. |
+| `setup_clean_chain` | `False` | A 200-block-long chain is initialized from cache by default. Instead, `setup_clean_chain` initializes an empty blockchain if set to `True`. |
 | `randomseed` | Random Integer | `TestShell.options.randomseed` is a member of `TestShell` which can be accessed during a test to seed a random generator. User can override default with a constant value for reproducible test runs. |
 | `supports_cli` | `False` | Whether the bitcoin-cli utility is compiled and available for the test. |
 | `tmpdir` | `"/var/folders/.../"` | Sets directory for test logs. Will be deleted upon a successful test run unless `nocleanup` is set to `True` |
diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py
index 229c134a4b..bc4fa90e83 100755
--- a/test/functional/wallet_avoidreuse.py
+++ b/test/functional/wallet_avoidreuse.py
@@ -65,7 +65,6 @@ def assert_balances(node, mine):
 class AvoidReuseTest(BitcoinTestFramework):
 
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 2
         # This test isn't testing txn relay/timing, so set whitelist on the
         # peers for instant txn relay. This speeds up the test run time 2-3x.
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index cf3317121f..16a0a50b07 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -17,7 +17,6 @@ from test_framework.wallet_util import bytes_to_wif, generate_wif_key
 
 class CreateWalletTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def skip_test_if_missing_module(self):
diff --git a/test/functional/wallet_watchonly.py b/test/functional/wallet_watchonly.py
index b0c41b2738..24799fe5f2 100755
--- a/test/functional/wallet_watchonly.py
+++ b/test/functional/wallet_watchonly.py
@@ -14,7 +14,6 @@ from test_framework.util import (
 
 class CreateWalletWatchonlyTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = False
         self.num_nodes = 1
 
     def skip_test_if_missing_module(self):