From fb00431b7c0b55fe2c9630aa07951975d453e0a3 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 15 Jul 2021 08:34:31 +0200 Subject: [PATCH 1/9] Merge bitcoin/bitcoin#22385: refactor: Use DeploymentEnabled to hide VB deployments fa5658ed077bfb02b6281d642dc649abdb99b6ee Use DeploymentEnabled to hide VB deployments (MarcoFalke) fa11fecf0dac44846a08e1b325547641f2eca957 doc: Move buried deployment doc to the enum that enumerates them (MarcoFalke) Pull request description: Plus a doc commit. ACKs for top commit: jnewbery: utACK fa5658ed077bfb02b6281d642dc649abdb99b6ee ajtowns: utACK fa5658ed077bfb02b6281d642dc649abdb99b6ee Tree-SHA512: 2aeceee0674feb603d76656eff40695b7d7305de309f837bbb6a8c1dbb1d0b962b741f06ab7b9a8b1dbd1964c9c0c9aa5dc9588fd8e6d896e620b69e08eedbaa --- src/consensus/params.h | 11 +++++++---- src/deploymentstatus.h | 2 +- src/rpc/blockchain.cpp | 9 +++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/consensus/params.h b/src/consensus/params.h index f025d8a540..47c8725a58 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -14,8 +14,12 @@ namespace Consensus { -enum BuriedDeployment : int16_t -{ +/** + * A buried deployment is one where the height of the activation has been hardcoded into + * the client implementation long after the consensus change has activated. See BIP 90. + */ +enum BuriedDeployment : int16_t { + // buried deployments get negative values to avoid overlap with DeploymentPos DEPLOYMENT_HEIGHTINCB = std::numeric_limits::min(), DEPLOYMENT_DERSIG, DEPLOYMENT_CLTV, @@ -31,8 +35,7 @@ enum BuriedDeployment : int16_t }; constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_V19; } -enum DeploymentPos : uint16_t -{ +enum DeploymentPos : uint16_t { DEPLOYMENT_TESTDUMMY, DEPLOYMENT_V20, // Deployment of EHF, LLMQ Randomness Beacon DEPLOYMENT_MN_RR, // Deployment of Masternode Reward Location Reallocation diff --git a/src/deploymentstatus.h b/src/deploymentstatus.h index 84ce46375e..608e4bda06 100644 --- a/src/deploymentstatus.h +++ b/src/deploymentstatus.h @@ -49,7 +49,7 @@ inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::Buried inline bool DeploymentEnabled(const Consensus::Params& params, Consensus::DeploymentPos dep) { assert(Consensus::ValidDeployment(dep)); - return params.vDeployments[dep].nTimeout != 0; + return params.vDeployments[dep].nStartTime != Consensus::BIP9Deployment::NEVER_ACTIVE; } /** this function is convenient helper for DIP0003 because 'active' and 'enforced' are different statuses for DIP0003 */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e6b7a49492..657068d70b 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1615,10 +1615,7 @@ static RPCHelpMan verifychain() static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& softforks, const Consensus::Params& params, Consensus::BuriedDeployment dep) { // For buried deployments. - // A buried deployment is one where the height of the activation has been hardcoded into - // the client implementation long after the consensus change has activated. See BIP 90. - // Buried deployments with activation height value of - // std::numeric_limits::max() are disabled and thus hidden. + if (!DeploymentEnabled(params, dep)) return; UniValue rv(UniValue::VOBJ); @@ -1633,8 +1630,8 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue& static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, const std::unordered_map& signals, UniValue& softforks, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) { // For BIP9 deployments. - // Deployments that are never active are hidden. - if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; + + if (!DeploymentEnabled(consensusParams, id)) return; UniValue bip9(UniValue::VOBJ); const ThresholdState thresholdState = g_versionbitscache.State(active_chain_tip, consensusParams, id); From 8928146bfa3d9139dace478c76b99a1642167ff5 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 28 Jul 2021 13:56:04 +0200 Subject: [PATCH 2/9] Merge bitcoin/bitcoin#22550: test: improve `test_signing_with_{csv,cltv}` subtests (speed, prevent timeout) 12f094ec215aacf30e4e380c0399f80d4e45c345 test: use constants for CSV/CLTV activation heights in rpc_signrawtransaction (Sebastian Falbesoner) 746f203f1950a7df50b9a7de87a361cc7354ffb4 test: introduce `generate_to_height` helper, use in rpc_signrawtransaction (Sebastian Falbesoner) e3237b1cd07a5099fbb0108218194eb653b6a9f3 test: check that CSV/CLTV are active in rpc_signrawtransaction (Sebastian Falbesoner) Pull request description: This PR primarily aims to solve the current RPC timeout problem for test rpc_signrawtransaction.py, as described in #22542. In the course of that the test is also improved in other ways (see https://github.com/bitcoin/bitcoin/pull/22542#pullrequestreview-714297804). Reviewers guideline: * In `test_signing_with_cltv()`, a comment is fixed -- it wrongly referred to CSV, it should be CLTV. * As preparation, assertions are added that ensure that CSV and CLTV have been really activated after generating blocks by checking the 'softforks' output of the getblockchaininfo() RPC. Right now in master, one could remove (or decrease, like in #22542) the generate calls and the test would still pass, when it shouldn't. * A helper `generate_to_height()` is introduced which improves the previous way of reaching a block height in two ways: - instead of blindly generating TH blocks to reach target block height >= TH, the current block height CH is taken into account, and only (TH - CH) are generated in total - to avoid potential RPC timeouts, the block generation is split up into multiple generatetoaddress RPC calls ([as suggested by laanwj](https://github.com/bitcoin/bitcoin/pull/22542#issuecomment-886237866)); here chunks of 200 blocks have been chosen * The helper is used in the affected sub-tests, which should both speed-up the test (from ~18s to ~12s on my machine) and avoid potential timeouts * Finally, the activation constants for CSV and CLTV are used instead of using magic numbers 500 and 1500 Open questions: * Any good naming alternatives for `generate_to_height()`? Not really happy with the name, happy to hear suggestions * Where to put the CSV and CLTV activation height constants in the test_framewor folder? I guess importing constants from other tests isn't really the desired way to go ACKs for top commit: laanwj: Code review and tested ACK 12f094ec215aacf30e4e380c0399f80d4e45c345 rajarshimaitra: reACK https://github.com/bitcoin/bitcoin/pull/22550/commits/12f094ec215aacf30e4e380c0399f80d4e45c345 Tree-SHA512: 14509f6d3e5a5a05d6a30a3145bb82cd96a29d9d8a589abf1944a8bf34291cde78ce711195f52e9426588dc822b3618ec9b455e057360021ae46152bb7613516 --- test/functional/feature_cltv.py | 3 +-- test/functional/feature_csv_activation.py | 2 +- test/functional/test_framework/blocktools.py | 4 ++++ test/functional/test_framework/util.py | 11 +++++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 64f7e44a35..5b019979e5 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -9,6 +9,7 @@ """ from test_framework.blocktools import ( + CLTV_HEIGHT, create_block, create_coinbase, ) @@ -34,8 +35,6 @@ MiniWalletMode, ) -CLTV_HEIGHT = 1351 - # Helper function to modify a transaction by # 1) prepending a given script to the scriptSig of vin 0 and diff --git a/test/functional/feature_csv_activation.py b/test/functional/feature_csv_activation.py index c5bf7ddeaa..2634b95c06 100755 --- a/test/functional/feature_csv_activation.py +++ b/test/functional/feature_csv_activation.py @@ -40,6 +40,7 @@ from itertools import product from test_framework.blocktools import ( + CSV_ACTIVATION_HEIGHT, create_block, create_coinbase, TIME_GENESIS_BLOCK, @@ -63,7 +64,6 @@ TESTING_TX_COUNT = 83 # Number of testing transactions: 1 BIP113 tx, 16 BIP68 txs, 66 BIP112 txs (see comments above) COINBASE_BLOCK_COUNT = TESTING_TX_COUNT # Number of coinbase blocks we need to generate as inputs for our txs BASE_RELATIVE_LOCKTIME = 10 -CSV_ACTIVATION_HEIGHT = 432 SEQ_DISABLE_FLAG = 1 << 31 SEQ_RANDOM_HIGH_BIT = 1 << 25 SEQ_TYPE_FLAG = 1 << 22 diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index b12a4749ef..423ffbeb49 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -35,6 +35,10 @@ # Coinbase transaction outputs can only be spent after this number of new blocks (network rule) COINBASE_MATURITY = 100 +# Soft-fork activation heights +CLTV_HEIGHT = 1351 +CSV_ACTIVATION_HEIGHT = 432 + NORMAL_GBT_REQUEST_PARAMS = {"rules": []} # type: ignore[var-annotated] def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl=None, txlist=None, dip4_activated=False, v20_activated=False): diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index ec3c6dcde1..9f0930e409 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -601,6 +601,17 @@ def mine_large_block(node, utxos=None): node.generate(1) +def generate_to_height(node, target_height): + """Generates blocks until a given target block height has been reached. + To prevent timeouts, only up to 200 blocks are generated per RPC call. + Can be used to activate certain soft-forks (e.g. CSV, CLTV).""" + current_height = node.getblockcount() + while current_height < target_height: + nblocks = min(200, target_height - current_height) + current_height += len(node.generate(nblocks)) + assert_equal(node.getblockcount(), target_height) + + def find_vout_for_address(node, txid, addr): """ Locate the vout index of the given transaction sending to the From cbd2be8e18cc41f3e27fd6ce131e4c5e7b60429a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 3 Aug 2021 13:54:20 +0200 Subject: [PATCH 3/9] Merge bitcoin/bitcoin#22597: consensus/params: simplify ValidDeployment check to avoid gcc warning 059171009b0138555f311cedc2553015ff618323 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns) Pull request description: Simplifies the ValidDeployment check to only check the upperbound, relying on the lower bound to be trivially true due to enum values starting at the minimum value of the underlying type (which is checked at compile time in deploymentstatus.cpp). Avoids a "comparison always true" warning in some versions of gcc. Fixes #22587 ACKs for top commit: MarcoFalke: review ACK 059171009b0138555f311cedc2553015ff618323 tryphe: retested ACK 059171009b0138555f311cedc2553015ff618323 Tree-SHA512: e53b5d478b46d35ec476d004e3c92803afb874c138dd6ef3848861f4281cc113fe88921bc4ac74fd4decbf318ed776d3f816c3a1185f99dc36a5cfecfff51f7c --- src/consensus/params.h | 4 ++-- src/deploymentstatus.cpp | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/consensus/params.h b/src/consensus/params.h index 47c8725a58..e5e5d6ff95 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -33,7 +33,7 @@ enum BuriedDeployment : int16_t { DEPLOYMENT_BRR, DEPLOYMENT_V19, }; -constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_V19; } +constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_V19; } enum DeploymentPos : uint16_t { DEPLOYMENT_TESTDUMMY, @@ -42,7 +42,7 @@ enum DeploymentPos : uint16_t { // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp MAX_VERSION_BITS_DEPLOYMENTS }; -constexpr bool ValidDeployment(DeploymentPos dep) { return DEPLOYMENT_TESTDUMMY <= dep && dep <= DEPLOYMENT_MN_RR; } +constexpr bool ValidDeployment(DeploymentPos dep) { return dep < MAX_VERSION_BITS_DEPLOYMENTS; } /** * Struct for each individual consensus rule change using BIP9. diff --git a/src/deploymentstatus.cpp b/src/deploymentstatus.cpp index 9007800421..bba86639a3 100644 --- a/src/deploymentstatus.cpp +++ b/src/deploymentstatus.cpp @@ -7,6 +7,8 @@ #include #include +#include + VersionBitsCache g_versionbitscache; /* Basic sanity checking for BuriedDeployment/DeploymentPos enums and @@ -15,3 +17,18 @@ VersionBitsCache g_versionbitscache; static_assert(ValidDeployment(Consensus::DEPLOYMENT_TESTDUMMY), "sanity check of DeploymentPos failed (TESTDUMMY not valid)"); static_assert(!ValidDeployment(Consensus::MAX_VERSION_BITS_DEPLOYMENTS), "sanity check of DeploymentPos failed (MAX value considered valid)"); static_assert(!ValidDeployment(static_cast(Consensus::DEPLOYMENT_TESTDUMMY)), "sanity check of BuriedDeployment failed (overlaps with DeploymentPos)"); + +/* ValidDeployment only checks upper bounds for ensuring validity. + * This checks that the lowest possible value or the type is also a + * (specific) valid deployment so that lower bounds don't need to be checked. + */ + +template +static constexpr bool is_minimum() +{ + using U = typename std::underlying_type::type; + return x == std::numeric_limits::min(); +} + +static_assert(is_minimum(), "heightincb is not minimum value for BuriedDeployment"); +static_assert(is_minimum(), "testdummy is not minimum value for DeploymentPos"); From fc25503cbc866cd027af3c88bc9f235aa827d532 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Tue, 10 Aug 2021 15:41:16 +0200 Subject: [PATCH 4/9] Merge bitcoin/bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up tests fafe896a0b870d85250927bd5374caf73d379468 test: Set regtest.BIP66Height = 102 to speed up tests (MarcoFalke) Pull request description: No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 66. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 66, which is enforced on mainnet for all new blocks. ACKs for top commit: GeneFerneau: Concept + code review ACK [fafe896](https://github.com/bitcoin/bitcoin/pull/22632/commits/fafe896a0b870d85250927bd5374caf73d379468) 0xB10C: crACK fafe896a0b870d85250927bd5374caf73d379468 laanwj: ACK fafe896a0b870d85250927bd5374caf73d379468 Zero-1729: tACK fafe896 kristapsk: ACK fafe896a0b870d85250927bd5374caf73d379468. Full functional test suite showed few second speed incrase on my laptop (although I didn't do proper benchmarking with multiple runs, just single `time ./test/functional/test_runner.py` on current master vs this PR). theStack: Tested ACK fafe896a0b870d85250927bd5374caf73d379468 hg333: tACK https://github.com/bitcoin/bitcoin/commit/fafe896a0b870d85250927bd5374caf73d379468 Tree-SHA512: 4bbee3c8587d612e74a59fde49b6439c1296f2fc27d3a7cf59a35e920f729fdd581c930290bd04def618f81412236676ddb99b4ceb4d80dfb9fd610b128a04b1 --- doc/release-notes-6189.md | 5 +++++ src/chainparams.cpp | 2 +- test/functional/feature_dersig.py | 7 ++++--- test/functional/rpc_blockchain.py | 3 ++- test/functional/test_framework/blocktools.py | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 doc/release-notes-6189.md diff --git a/doc/release-notes-6189.md b/doc/release-notes-6189.md new file mode 100644 index 0000000000..be8815c960 --- /dev/null +++ b/doc/release-notes-6189.md @@ -0,0 +1,5 @@ +Tests +----- + +- For the `regtest` network the BIP 66 (DERSIG) activation height was changed + from 1251 to 102. (dash#6189) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index d946b2d84b..90e652a839 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -797,7 +797,7 @@ class CRegTestParams : public CChainParams { consensus.BIP34Height = 500; // BIP34 activated on regtest (Used in functional tests) consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in functional tests) - consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in functional tests) + consensus.BIP66Height = 102; // BIP66 activated on regtest (Block at height 101 and earlier not enforced for testing purposes) consensus.BIP147Height = 432; // BIP147 activated on regtest (Used in functional tests) consensus.CSVHeight = 432; // CSV activated on regtest (Used in rpc activation tests) consensus.DIP0001Height = 2000; diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 6e3a49df3e..597bce5a7e 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -4,10 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test BIP66 (DER SIG). -Test that the DERSIG soft-fork activates at (regtest) height 1251. +Test the DERSIG soft-fork activation on regtest. """ from test_framework.blocktools import ( + DERSIG_HEIGHT, create_block, create_coinbase, ) @@ -24,8 +25,6 @@ MiniWalletMode, ) -DERSIG_HEIGHT = 1251 - # A canonical signature consists of: # <30> <02> <02> @@ -88,8 +87,10 @@ def run_test(self): block.rehash() block.solve() + assert_equal(self.nodes[0].getblockcount(), DERSIG_HEIGHT - 2) self.test_dersig_info(is_active=False) # Not active as of current tip and next block does not need to obey rules peer.send_and_ping(msg_block(block)) + assert_equal(self.nodes[0].getblockcount(), DERSIG_HEIGHT - 1) self.test_dersig_info(is_active=True) # Not active as of current tip, but next block must obey rules assert_equal(self.nodes[0].getbestblockhash(), block.hash) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 9c4eab3a37..1e6b23d354 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -27,6 +27,7 @@ from test_framework.address import ADDRESS_BCRT1_P2SH_OP_TRUE from test_framework.blocktools import ( + DERSIG_HEIGHT, create_block, create_coinbase, TIME_GENESIS_BLOCK, @@ -143,7 +144,7 @@ def _test_getblockchaininfo(self): assert_greater_than(res['size_on_disk'], 0) assert_equal(res['softforks'], { 'bip34': {'type': 'buried', 'active': False, 'height': 500}, - 'bip66': {'type': 'buried', 'active': False, 'height': 1251}, + 'bip66': {'type': 'buried', 'active': True, 'height': DERSIG_HEIGHT}, 'bip65': {'type': 'buried', 'active': False, 'height': 1351}, 'bip147': { 'type': 'buried', 'active': False, 'height': 432}, 'csv': {'type': 'buried', 'active': False, 'height': 432}, diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 423ffbeb49..ae909a9ee0 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -36,6 +36,7 @@ COINBASE_MATURITY = 100 # Soft-fork activation heights +DERSIG_HEIGHT = 102 # BIP 66 CLTV_HEIGHT = 1351 CSV_ACTIVATION_HEIGHT = 432 From 71af8816ef726d4da4b051b6a0c9f727e78648a5 Mon Sep 17 00:00:00 2001 From: merge-script Date: Fri, 10 Sep 2021 10:02:03 +0200 Subject: [PATCH 5/9] Merge bitcoin/bitcoin#22907: test: Avoid intermittent test failure in feature_csv_activation.py fa676dbac87919061de9f82bce65e373e8d85bd1 test: pep-8 whitespace (MarcoFalke) faed284eabb250a07331dfca22bb8f96a95c72ea test: Avoid intermittent test failure in feature_csv_activation.py (MarcoFalke) Pull request description: Otherwise there will be disconnects if the test runs longer than the default peertimeout (60s): ``` node0 2021-09-05T20:28:30.973116Z (mocktime: 2021-09-01T07:17:29Z) [net] [net.cpp:1323] [InactivityCheck] socket receive timeout: 393061s peer=0 ``` Fix that by skipping `InactivityCheck` via a large `-peertimeout`. ACKs for top commit: fanquake: ACK fa676dbac87919061de9f82bce65e373e8d85bd1 Tree-SHA512: 061c0585a805aa2f8e55c4beedd4b8498a2951f33d60aa3632dda0a284db3a627d14a23dbd57e8a66c69a1612f39418e3a755c8ca97f6ae1105c0d70f0d1a801 --- test/functional/feature_csv_activation.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_csv_activation.py b/test/functional/feature_csv_activation.py index 2634b95c06..5db5311c6b 100755 --- a/test/functional/feature_csv_activation.py +++ b/test/functional/feature_csv_activation.py @@ -69,6 +69,7 @@ SEQ_TYPE_FLAG = 1 << 22 SEQ_RANDOM_LOW_BIT = 1 << 18 + def relative_locktime(sdf, srhb, stf, srlb): """Returns a locktime with certain bits set.""" @@ -83,6 +84,7 @@ def relative_locktime(sdf, srhb, stf, srlb): locktime |= SEQ_RANDOM_LOW_BIT return locktime + def all_rlt_txs(txs): return [tx['tx'] for tx in txs] @@ -94,6 +96,7 @@ def set_test_params(self): # Must also set '-maxtipage=600100' to allow syncing from very old blocks # and '-dip3params=2000:2000' to create pre-dip3 blocks only self.extra_args = [[ + '-peertimeout=999999', # bump because mocktime might cause a disconnect otherwise '-whitelist=noban@127.0.0.1', '-maxtipage=600100', '-dip3params=2000:2000', '-par=1', # Use only one script thread to get the exact reject reason for testing @@ -149,13 +152,13 @@ def create_bip112txs(self, bip112inputs, varyOP_CSV, txversion, locktime_delta=0 for i, (sdf, srhb, stf, srlb) in enumerate(product(*[[True, False]] * 4)): locktime = relative_locktime(sdf, srhb, stf, srlb) tx = self.create_self_transfer_from_utxo(bip112inputs[i]) - if (varyOP_CSV): # if varying OP_CSV, nSequence is fixed + if varyOP_CSV: # if varying OP_CSV, nSequence is fixed tx.vin[0].nSequence = BASE_RELATIVE_LOCKTIME + locktime_delta else: # vary nSequence instead, OP_CSV is fixed tx.vin[0].nSequence = locktime + locktime_delta tx.nVersion = txversion self.miniwallet.sign_tx(tx) - if (varyOP_CSV): + if varyOP_CSV: tx.vin[0].scriptSig = CScript([locktime, OP_CHECKSEQUENCEVERIFY, OP_DROP] + list(CScript(tx.vin[0].scriptSig))) else: tx.vin[0].scriptSig = CScript([BASE_RELATIVE_LOCKTIME, OP_CHECKSEQUENCEVERIFY, OP_DROP] + list(CScript(tx.vin[0].scriptSig))) @@ -202,7 +205,7 @@ def run_test(self): self.tip = int(self.nodes[0].getbestblockhash(), 16) # Activation height is hardcoded - test_blocks = self.generate_blocks(CSV_ACTIVATION_HEIGHT-5 - COINBASE_BLOCK_COUNT) + test_blocks = self.generate_blocks(CSV_ACTIVATION_HEIGHT - 5 - COINBASE_BLOCK_COUNT) #test_blocks = self.generate_blocks(345) self.send_blocks(test_blocks) assert not softfork_active(self.nodes[0], 'csv') @@ -488,5 +491,6 @@ def run_test(self): self.send_blocks([self.create_test_block(time_txs)]) self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) + if __name__ == '__main__': BIP68_112_113Test().main() From 101a863399b7564fcd7083d81311b944759490f2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 3 Aug 2021 10:09:42 +0200 Subject: [PATCH 6/9] Merge bitcoin/bitcoin#16333: test: Set BIP34Height = 2 for regtest 222290f54388270937cb6c174195717e2214ec0d test: Set BIP34Height = 2 for regtest (MarcoFalke) fac90c55be478f0323eafa1d560ea2c56f04fb23 test: Create all blocks with version 4 or higher (MarcoFalke) Pull request description: BIP34 is active on the current tip of mainnet, so all miners must obey it. It would be nice if it also was active in fresh regtest instances from the earliest time possible. I changed the BIP34 height to `2`, so that the block at height=1 may be used to mine a duplicate coinbase. (Needed to test mainnet behaviour) This pull is done in two commits: * test: Create all blocks with version 4 or higher: Now that BIP34 is activated earlier, we need to create blocks with a higher version number. Just bump it to 4 instead of 2 to avoid having to bump it again later. * test: Set BIP34Height = 2 for regtest: This fixes the BIP34 implementation in the tests (to match the one of the Core codebase) and updates the tests where needed ACKs for top commit: ajtowns: ACK 222290f54388270937cb6c174195717e2214ec0d jonatack: ACK 222290f54388270937cb6c174195717e2214ec0d tested and reviewed rebased to current master 5e213822f86d theStack: Tested ACK 222290f54388270937cb6c174195717e2214ec0d Tree-SHA512: d69c637a62a64b8e87de8c7f0b305823d8f4d115c1852514b923625dbbcf9a4854b5bb3771ff41702ebf47c4c182a4442c6d7c0b9f282c95a34b83e56a73939b --- src/chainparams.cpp | 2 +- src/test/validation_block_tests.cpp | 14 +++++++------- test/functional/feature_block.py | 5 ++++- test/functional/rpc_blockchain.py | 2 +- test/functional/test_framework/blocktools.py | 4 +++- test/functional/test_framework/messages.py | 2 +- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 90e652a839..b9b1885eed 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -794,7 +794,7 @@ class CRegTestParams : public CChainParams { consensus.nGovernanceMinQuorum = 1; consensus.nGovernanceFilterElements = 100; consensus.nMasternodeMinimumConfirmations = 1; - consensus.BIP34Height = 500; // BIP34 activated on regtest (Used in functional tests) + consensus.BIP34Height = 2; // BIP34 activated on regtest (Block at height 1 not enforced for testing purposes) consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in functional tests) consensus.BIP66Height = 102; // BIP66 activated on regtest (Block at height 101 and earlier not enforced for testing purposes) diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 20ac5ab130..352421a815 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -79,6 +79,8 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) txCoinbase.vout[1].scriptPubKey = P2SH_OP_TRUE; txCoinbase.vout[1].nValue = txCoinbase.vout[0].nValue; txCoinbase.vout[0].nValue = 0; + // Always pad with OP_0 at the end to avoid bad-cb-length error + txCoinbase.vin[0].scriptSig = CScript{} << WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(prev_hash)->nHeight + 1) << OP_0; pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); return pblock; @@ -92,6 +94,11 @@ std::shared_ptr MinerTestingSetup::FinalizeBlock(std::shared_ptr ++(pblock->nNonce); } + // submit block header, so that miner can get the block height from the + // global state and the node has the topology of the chain + BlockValidationState ignored; + BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders({pblock->GetBlockHeader()}, ignored, Params())); + return pblock; } @@ -146,13 +153,6 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) } bool ignored; - BlockValidationState state; - std::vector headers; - std::transform(blocks.begin(), blocks.end(), std::back_inserter(headers), [](std::shared_ptr b) { return b->GetBlockHeader(); }); - - // Process all the headers so we understand the toplogy of the chain - BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlockHeaders(headers, state, Params())); - // Connect the genesis block and drain any outstanding events BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(Params(), std::make_shared(Params().GenesisBlock()), true, &ignored)); SyncWithValidationInterfaceQueue(); diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index f5e9bd5c04..e4bc12a46a 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -378,7 +378,9 @@ def run_test(self): # b30 has a max-sized coinbase scriptSig. self.move_tip(23) b30 = self.next_block(30) - b30.vtx[0].vin[0].scriptSig = b'\x00' * 100 + b30.vtx[0].vin[0].scriptSig = bytes(b30.vtx[0].vin[0].scriptSig) # Convert CScript to raw bytes + b30.vtx[0].vin[0].scriptSig += b'\x00' * (100 - len(b30.vtx[0].vin[0].scriptSig)) # Fill with 0s + assert_equal(len(b30.vtx[0].vin[0].scriptSig), 100) b30.vtx[0].rehash() b30 = self.update_block(30, []) self.send_blocks([b30], True) @@ -839,6 +841,7 @@ def run_test(self): b61.vtx[0].rehash() b61 = self.update_block(61, []) assert_equal(duplicate_tx.serialize(), b61.vtx[0].serialize()) + # BIP30 is always checked on regtest, regardless of the BIP34 activation height self.send_blocks([b61], success=False, reject_reason='bad-txns-BIP30', reconnect=True) # Test BIP30 (allow duplicate if spent) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 1e6b23d354..0358d2cb24 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -143,7 +143,7 @@ def _test_getblockchaininfo(self): assert_equal(res['prune_target_size'], 576716800) assert_greater_than(res['size_on_disk'], 0) assert_equal(res['softforks'], { - 'bip34': {'type': 'buried', 'active': False, 'height': 500}, + 'bip34': {'type': 'buried', 'active': True, 'height': 2}, 'bip66': {'type': 'buried', 'active': True, 'height': DERSIG_HEIGHT}, 'bip65': {'type': 'buried', 'active': False, 'height': 1351}, 'bip147': { 'type': 'buried', 'active': False, 'height': 432}, diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index ae909a9ee0..1491e23909 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -42,12 +42,14 @@ NORMAL_GBT_REQUEST_PARAMS = {"rules": []} # type: ignore[var-annotated] +VERSIONBITS_LAST_OLD_BLOCK_VERSION = 4 + def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl=None, txlist=None, dip4_activated=False, v20_activated=False): """Create a block (with regtest difficulty).""" block = CBlock() if tmpl is None: tmpl = {} - block.nVersion = version or tmpl.get('version') or 1 + block.nVersion = version or tmpl.get('version') or VERSIONBITS_LAST_OLD_BLOCK_VERSION block.nTime = ntime or tmpl.get('curtime') or int(time.time() + 600) block.hashPrevBlock = hashprev or int(tmpl['previousblockhash'], 0x10) if tmpl and not tmpl.get('bits') is None: diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index b98616df86..b634231253 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -564,7 +564,7 @@ def __init__(self, header=None): self.calc_sha256() def set_null(self): - self.nVersion = 1 + self.nVersion = 4 self.hashPrevBlock = 0 self.hashMerkleRoot = 0 self.nTime = 0 From adcf095ab9013fb3e1bdb01af0b2320b0e3e9ca8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 16 Aug 2021 21:06:48 +0200 Subject: [PATCH 7/9] Merge bitcoin/bitcoin#22718: doc: Add missing PR 16333 release note fa76ebd1a67384495dd3099116715e4267ca5b0c doc: Add missing PR 16333 release note (MarcoFalke) Pull request description: Missed in commit ad0fc453cc3cefde6c84f11f7288237e4190283c ACKs for top commit: laanwj: Documentation review ACK fa76ebd1a67384495dd3099116715e4267ca5b0c Zero-1729: ACK fa76ebd1a67384495dd3099116715e4267ca5b0c theStack: ACK fa76ebd1a67384495dd3099116715e4267ca5b0c Tree-SHA512: 5fc9e3f61d2f8534f537cc14c7518d09babd5ebbedd5f24ad10ca9aff69d499f3daa0c45eb548eec94cdbf25bf53c9f0158ec2ee7aa5c3f653f7670c68d1d24c --- doc/release-notes-6189.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/release-notes-6189.md b/doc/release-notes-6189.md index be8815c960..33729289ad 100644 --- a/doc/release-notes-6189.md +++ b/doc/release-notes-6189.md @@ -1,5 +1,7 @@ Tests ----- -- For the `regtest` network the BIP 66 (DERSIG) activation height was changed - from 1251 to 102. (dash#6189) +- For the `regtest` network the activation heights of several softforks were + changed. (dash#6189) + * BIP 34 (blockheight in coinbase) from 500 to 2 + * BIP 66 (DERSIG) from 1251 to 102 From 65b92fa09353293f90668b6acab74d61d519af75 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 27 Aug 2021 07:35:02 +0800 Subject: [PATCH 8/9] Merge bitcoin/bitcoin#21862: test: Set regtest.BIP65Height = 111 to speed up tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit faf7e485e901d6c72db5d969b526fa148060a003 Set regtest.BIP65Height = 111 to speed up tests (MarcoFalke) Pull request description: No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 65. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 65, which is enforced on mainnet for all new blocks. ACKs for top commit: theStack: re-ACK faf7e485e901d6c72db5d969b526fa148060a003 📍 Zero-1729: re-ACK faf7e485e901d6c72db5d969b526fa148060a003 kristapsk: ACK faf7e485e901d6c72db5d969b526fa148060a003 Tree-SHA512: 79a8263e7233838666b9b636b496a8b9eb12398c779f9434677e1d62816732c0a7c7b3e73965be1fb0038d35e05e5a90e665bd74e9610104127dfc4ea38169bf --- doc/release-notes-6189.md | 1 + src/chainparams.cpp | 2 +- test/functional/feature_cltv.py | 10 +++++----- test/functional/rpc_blockchain.py | 3 ++- test/functional/test_framework/blocktools.py | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/doc/release-notes-6189.md b/doc/release-notes-6189.md index 33729289ad..b88a030d82 100644 --- a/doc/release-notes-6189.md +++ b/doc/release-notes-6189.md @@ -5,3 +5,4 @@ Tests changed. (dash#6189) * BIP 34 (blockheight in coinbase) from 500 to 2 * BIP 66 (DERSIG) from 1251 to 102 + * BIP 65 (CLTV) from 1351 to 111 diff --git a/src/chainparams.cpp b/src/chainparams.cpp index b9b1885eed..c67d349e54 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -796,7 +796,7 @@ class CRegTestParams : public CChainParams { consensus.nMasternodeMinimumConfirmations = 1; consensus.BIP34Height = 2; // BIP34 activated on regtest (Block at height 1 not enforced for testing purposes) consensus.BIP34Hash = uint256(); - consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in functional tests) + consensus.BIP65Height = 111; // BIP65 activated on regtest (Block at height 110 and earlier not enforced for testing purposes) consensus.BIP66Height = 102; // BIP66 activated on regtest (Block at height 101 and earlier not enforced for testing purposes) consensus.BIP147Height = 432; // BIP147 activated on regtest (Used in functional tests) consensus.CSVHeight = 432; // CSV activated on regtest (Used in rpc activation tests) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 5b019979e5..60a0ec4538 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -4,8 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test BIP65 (CHECKLOCKTIMEVERIFY). -Test that the CHECKLOCKTIMEVERIFY soft-fork activates at (regtest) block height -1351. +Test that the CHECKLOCKTIMEVERIFY soft-fork activates. """ from test_framework.blocktools import ( @@ -65,9 +64,9 @@ def cltv_invalidate(tx, failure_reason): # +-------------------------------------------------+------------+--------------+ [[OP_CHECKLOCKTIMEVERIFY], None, None], [[OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP], None, None], - [[CScriptNum(1000), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 1296688602], # timestamp of genesis block - [[CScriptNum(1000), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 500], - [[CScriptNum(500), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0xffffffff, 500], + [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 1296688602], # timestamp of genesis block + [[CScriptNum(100), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 50], + [[CScriptNum(50), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0xffffffff, 50], ][failure_reason] cltv_modify_tx(tx, prepend_scriptsig=scheme[0], nsequence=scheme[1], nlocktime=scheme[2]) @@ -110,6 +109,7 @@ def run_test(self): self.log.info("Mining %d blocks", CLTV_HEIGHT - 2) wallet.generate(10) self.nodes[0].generate(CLTV_HEIGHT - 2 - 10) + assert_equal(self.nodes[0].getblockcount(), CLTV_HEIGHT - 2) self.log.info("Test that invalid-according-to-CLTV transactions can still appear in a block") diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 0358d2cb24..10c8d96136 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -27,6 +27,7 @@ from test_framework.address import ADDRESS_BCRT1_P2SH_OP_TRUE from test_framework.blocktools import ( + CLTV_HEIGHT, DERSIG_HEIGHT, create_block, create_coinbase, @@ -145,7 +146,7 @@ def _test_getblockchaininfo(self): assert_equal(res['softforks'], { 'bip34': {'type': 'buried', 'active': True, 'height': 2}, 'bip66': {'type': 'buried', 'active': True, 'height': DERSIG_HEIGHT}, - 'bip65': {'type': 'buried', 'active': False, 'height': 1351}, + 'bip65': {'type': 'buried', 'active': True, 'height': CLTV_HEIGHT}, 'bip147': { 'type': 'buried', 'active': False, 'height': 432}, 'csv': {'type': 'buried', 'active': False, 'height': 432}, 'dip0001': { 'type': 'buried', 'active': False, 'height': 2000}, diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 1491e23909..a77a11669c 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -37,7 +37,7 @@ # Soft-fork activation heights DERSIG_HEIGHT = 102 # BIP 66 -CLTV_HEIGHT = 1351 +CLTV_HEIGHT = 111 # BIP 65 CSV_ACTIVATION_HEIGHT = 432 NORMAL_GBT_REQUEST_PARAMS = {"rules": []} # type: ignore[var-annotated] From e4e7c440f48058dabdc571a0a20626b513efd26c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 9 Aug 2024 16:57:37 +0700 Subject: [PATCH 9/9] fix: use proper chain instead using ActiveChain for test framework --- src/miner.cpp | 4 ++-- src/test/util/setup_common.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index c9d47ab938..5e5269dda2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -64,9 +64,9 @@ BlockAssembler::Options::Options() } BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, const CTxMemPool& mempool, const CChainParams& params, const Options& options) : - m_blockman(Assert(node.chainman)->m_blockman), + m_blockman(chainstate.m_blockman), m_cpoolman(*Assert(node.cpoolman)), - m_chain_helper(*Assert(node.chain_helper)), + m_chain_helper(chainstate.ChainHelper()), m_chainstate(chainstate), m_dmnman(*Assert(node.dmnman)), m_evoDb(*Assert(node.evodb)), diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 7ab9ed9eb1..72b723645e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -428,10 +428,10 @@ CBlock TestChainSetup::CreateBlock( auto cbTx = GetTxPayload(*block.vtx[0]); BOOST_ASSERT(cbTx.has_value()); BlockValidationState state; - if (!CalcCbTxMerkleRootMNList(block, m_node.chainman->ActiveChain().Tip(), cbTx->merkleRootMNList, *m_node.dmnman, state, m_node.chainman->ActiveChainstate().CoinsTip())) { + if (!CalcCbTxMerkleRootMNList(block, chainstate.m_chain.Tip(), cbTx->merkleRootMNList, *m_node.dmnman, state, chainstate.CoinsTip())) { BOOST_ASSERT(false); } - if (!CalcCbTxMerkleRootQuorums(block, m_node.chainman->ActiveChain().Tip(), *m_node.llmq_ctx->quorum_block_processor, cbTx->merkleRootQuorums, state)) { + if (!CalcCbTxMerkleRootQuorums(block, chainstate.m_chain.Tip(), *m_node.llmq_ctx->quorum_block_processor, cbTx->merkleRootQuorums, state)) { BOOST_ASSERT(false); } CMutableTransaction tmpTx{*block.vtx[0]}; @@ -443,7 +443,7 @@ CBlock TestChainSetup::CreateBlock( { LOCK(cs_main); unsigned int extraNonce = 0; - IncrementExtraNonce(&block, m_node.chainman->ActiveChain().Tip(), extraNonce); + IncrementExtraNonce(&block, chainstate.m_chain.Tip(), extraNonce); } while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;