From b2d889380c246bd59e02eb97fd37e78ab1ebd5ce Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 29 Apr 2021 20:44:21 +0200 Subject: [PATCH 01/11] Merge bitcoin/bitcoin#21792: test: Fix intermittent issue in p2p_segwit.py fad6269916dbf8adc14d757a18f19c74e95cf659 test: Assert that exit code indicates failure (MarcoFalke) faecb72c3ca744f1adb77bd910c643cedec3b445 test: Fix intermittent issue in p2p_segwit.py (MarcoFalke) Pull request description: Calling `start_node` might call `wait_for_rpc_connection`, which will fail. https://cirrus-ci.com/task/5669555591708672?logs=ci#L3504 ``` File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_segwit.py", line 1974, in test_upgrade_after_activation self.start_node(2, extra_args=["-reindex", f"-segwitheight={SEGWIT_HEIGHT}"]) File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 508, in start_node node.wait_for_rpc_connection() File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_node.py", line 224, in wait_for_rpc_connection raise FailedToStartError(self._node_msg( test_framework.test_node.FailedToStartError: [node 2] bitcoind exited with status 1 during initialization ACKs for top commit: jnewbery: ACK fad6269916dbf8adc14d757a18f19c74e95cf659 dhruv: ACK fad6269 Tree-SHA512: 4c5e39ce25e135717ea433258518f93f09d1c528c4538a8627d3da13bc0c0ba4b45911703c26392ff0f5e0cb7831a6c7cc53e6e29102d3da9c8cfce7cef333cc --- test/functional/test_framework/test_node.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index e72cf3ea79..5c9c084de6 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -510,6 +510,7 @@ class TestNode(): self.start(extra_args, stdout=log_stdout, stderr=log_stderr, *args, **kwargs) ret = self.process.wait(timeout=self.rpc_timeout) self.log.debug(self._node_msg(f'dashd exited with status {ret} during initialization')) + assert ret != 0 # Exit code must indicate failure self.running = False self.process = None # Check stderr for expected message From a63f9c31cc5459a08c490b69145903d1fa05acb7 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 25 Apr 2021 17:00:58 +0300 Subject: [PATCH 02/11] Merge bitcoin-core/gui#284: refactor: Simplify SendCoinsDialog::updateCoinControlState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5f438d66c1fbc0e524d12fef233f2ed2952e6f17 refactor, qt: Simplify SendCoinsDialog::updateCoinControlState (João Barbosa) Pull request description: This PR doesn't change behaviour, removes the coin control argument from `updateCoinControlState` since it's a class member. ACKs for top commit: hebasto: ACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17, I have reviewed the code and it looks OK, I agree it can be merged. jonatack: Code review ACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17 kristapsk: utACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17. Code looks correct. Tree-SHA512: 14abaa3d561f8c8854fed989b6aca886dcca42135880bac76070043f61c0042ec8967f2b83e50bbbb82050ef0f074209e97fa300cb4dc51ee182316e0846506d --- src/qt/sendcoinsdialog.cpp | 16 ++++++++-------- src/qt/sendcoinsdialog.h | 3 +-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index f52b45b17e..529267df0d 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -316,7 +316,7 @@ bool SendCoinsDialog::send(const QList& recipients, QString& m_current_transaction = std::make_unique(recipients); WalletModel::SendCoinsReturn prepareStatus; - updateCoinControlState(*m_coin_control); + updateCoinControlState(); prepareStatus = model->prepareTransaction(*m_current_transaction, *m_coin_control); @@ -829,18 +829,18 @@ void SendCoinsDialog::updateFeeMinimizedLabel() } } -void SendCoinsDialog::updateCoinControlState(CCoinControl& ctrl) +void SendCoinsDialog::updateCoinControlState() { if (ui->radioCustomFee->isChecked()) { - ctrl.m_feerate = CFeeRate(ui->customFee->value()); + m_coin_control->m_feerate = CFeeRate(ui->customFee->value()); } else { - ctrl.m_feerate.reset(); + m_coin_control->m_feerate.reset(); } // Avoid using global defaults when sending money from the GUI // Either custom fee will be used or if not selected, the confirmation target from dropdown box - ctrl.m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); + m_coin_control->m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); // Include watch-only for wallets without private key - ctrl.fAllowWatchOnly = model->wallet().privateKeysDisabled(); + m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled(); } void SendCoinsDialog::updateNumberOfBlocks(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool header, SynchronizationState sync_state) { @@ -853,7 +853,7 @@ void SendCoinsDialog::updateSmartFeeLabel() { if(!model || !model->getOptionsModel()) return; - updateCoinControlState(*m_coin_control); + updateCoinControlState(); m_coin_control->m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels int returned_target; FeeReason reason; @@ -1013,7 +1013,7 @@ void SendCoinsDialog::coinControlUpdateLabels() if (!model || !model->getOptionsModel()) return; - updateCoinControlState(*m_coin_control); + updateCoinControlState(); // set pay amounts CoinControlDialog::payAmounts.clear(); diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index b41e16a635..6f64142731 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -79,8 +79,7 @@ private: // Format confirmation message bool PrepareSendText(QString& question_string, QString& informative_text, QString& detailed_text); void updateFeeMinimizedLabel(); - // Update the passed in CCoinControl with state from the GUI - void updateCoinControlState(CCoinControl& ctrl); + void updateCoinControlState(); private Q_SLOTS: void sendButtonClicked(bool checked); From de4d2a839d1f029e67ade9e68e3aa69845ea0445 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 26 Apr 2021 09:20:39 +0200 Subject: [PATCH 03/11] Merge bitcoin/bitcoin#21714: refactor: Drop CCoinControl::SetNull MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit c5a470eee1ac864b7c02b6a1669327b68411d806 refactor: Drop CCoinControl::SetNull (João Barbosa) Pull request description: The only external call to `SetNull` is changed as follow ```diff - m_coin_control->SetNull(); + m_coin_control = std::make_unique(); ``` ACKs for top commit: theStack: Code-Review ACK c5a470eee1ac864b7c02b6a1669327b68411d806 MarcoFalke: review ACK c5a470eee1ac864b7c02b6a1669327b68411d806 🍤 Tree-SHA512: 2588828720cdcf405fc4fb06f2aa17ad4eef2a645e12d820311006127e732258dd084993f17f23742f8e7f782e18289a6199dcec3690efc9b92458f90b816a8f --- src/qt/sendcoinsdialog.cpp | 2 +- src/wallet/coincontrol.cpp | 20 ++------------------ src/wallet/coincontrol.h | 28 ++++++++++++---------------- src/wallet/test/wallet_tests.cpp | 14 +++++++------- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 529267df0d..1570567e0f 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -924,7 +924,7 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) ui->frameCoinControl->setVisible(checked); if (!checked && model) { // coin control features disabled - m_coin_control->SetNull(false); + m_coin_control = std::make_unique(m_coin_control->nCoinType); } coinControlUpdateLabels(); diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp index f21ff8d3bf..279c2d0b8d 100644 --- a/src/wallet/coincontrol.cpp +++ b/src/wallet/coincontrol.cpp @@ -6,24 +6,8 @@ #include -void CCoinControl::SetNull(bool fResetCoinType) +CCoinControl::CCoinControl(CoinType coinType) +: nCoinType(coinType) { - destChange = CNoDestination(); - m_add_inputs = true; - fAllowOtherInputs = false; - fAllowWatchOnly = false; m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS); - m_avoid_address_reuse = false; - setSelected.clear(); - m_feerate.reset(); - fOverrideFeeRate = false; - m_confirm_target.reset(); - m_fee_mode = FeeEstimateMode::UNSET; - fRequireAllInputs = true; - m_discard_feerate.reset(); - m_min_depth = DEFAULT_MIN_DEPTH; - m_max_depth = DEFAULT_MAX_DEPTH; - if (fResetCoinType) { - nCoinType = CoinType::ALL_COINS; - } } diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 920e9d606b..aa8e2440ed 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -36,17 +36,18 @@ const int DEFAULT_MAX_DEPTH = 9999999; class CCoinControl { public: - CTxDestination destChange; + //! Custom change destination, if not set an address is generated + CTxDestination destChange = CNoDestination(); //! If false, only selected inputs are used - bool m_add_inputs; + bool m_add_inputs = true; //! If false, allows unselected inputs, but requires all selected inputs be used if fAllowOtherInputs is true (default) - bool fAllowOtherInputs; + bool fAllowOtherInputs = false; //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with fAllowOtherInputs - bool fRequireAllInputs; + bool fRequireAllInputs = true; //! Includes watch only addresses which are solvable - bool fAllowWatchOnly; + bool fAllowWatchOnly = false; //! Override automatic min/max checks on fee, m_feerate must be set if true - bool fOverrideFeeRate; + bool fOverrideFeeRate = false; //! Override the wallet's m_pay_tx_fee if set std::optional m_feerate; //! Override the discard feerate estimation with m_discard_feerate in CreateTransaction if set @@ -54,24 +55,19 @@ public: //! Override the default confirmation target if set std::optional m_confirm_target; //! Avoid partial use of funds sent to a given address - bool m_avoid_partial_spends; + bool m_avoid_partial_spends = DEFAULT_AVOIDPARTIALSPENDS; //! Forbids inclusion of dirty (previously used) addresses - bool m_avoid_address_reuse; + bool m_avoid_address_reuse = false; //! Fee estimation mode to control arguments to estimateSmartFee - FeeEstimateMode m_fee_mode; + FeeEstimateMode m_fee_mode = FeeEstimateMode::UNSET; //! Minimum chain depth value for coin availability int m_min_depth = DEFAULT_MIN_DEPTH; //! Maximum chain depth value for coin availability int m_max_depth = DEFAULT_MAX_DEPTH; //! Controls which types of coins are allowed to be used (default: ALL_COINS) - CoinType nCoinType; + CoinType nCoinType = CoinType::ALL_COINS; - CCoinControl() - { - SetNull(); - } - - void SetNull(bool fResetCoinType = true); + CCoinControl(CoinType coin_type = CoinType::ALL_COINS); bool HasSelected() const { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 86f28641d8..0115d90503 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -869,7 +869,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) // First run the tests with only one input containing 100k duffs { - coinControl.SetNull(); + coinControl = CCoinControl(); coinControl.Select(GetCoins({{100000, false}})[0]); // Start with fallback feerate @@ -909,7 +909,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) } // Now use 4 different inputs with a total of 100k duff { - coinControl.SetNull(); + coinControl = CCoinControl(); auto setCoins = GetCoins({{1000, false}, {5000, false}, {10000, false}, {84000, false}}); for (auto coin : setCoins) { coinControl.Select(coin); @@ -953,7 +953,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) // Last use 10 equal inputs with a total of 100k duff { - coinControl.SetNull(); + coinControl = CCoinControl(); auto setCoins = GetCoins({{10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}}); @@ -999,7 +999,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) // Some tests without selected coins in coinControl, let the wallet decide // which inputs to use { - coinControl.SetNull(); + coinControl = CCoinControl(); coinControl.m_feerate = CFeeRate(fallbackFee); auto setCoins = GetCoins({{1000, false}, {1000, false}, {1000, false}, {1000, false}, {1000, false}, {1100, false}, {1200, false}, {1300, false}, {1400, false}, {1500, false}, @@ -1048,7 +1048,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) } // Test if the change output ends up at the requested position { - coinControl.SetNull(); + coinControl = CCoinControl(); coinControl.m_feerate = CFeeRate(fallbackFee); coinControl.Select(GetCoins({{100000, false}})[0]); @@ -1059,7 +1059,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) } // Test error cases { - coinControl.SetNull(); + coinControl = CCoinControl(); coinControl.m_feerate = CFeeRate(fallbackFee); // First try to send something without any coins available { @@ -1096,7 +1096,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup) } }; - coinControl.SetNull(); + coinControl = CCoinControl(); coinControl.m_feerate = CFeeRate(fallbackFee); coinControl.Select(GetCoins({{100 * COIN, false}})[0]); From 71f23d6e33e87aaaa3bad055fa1ac67b96297add Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 5 May 2021 16:46:19 +0200 Subject: [PATCH 04/11] Merge bitcoin/bitcoin#21814: test: Fix feature_config_args.py intermittent issue fab1eb65b196d62466fdc2ed319ffa19d3560a0c test: Fix feature_config_args.py intermittent issue (MarcoFalke) Pull request description: Fix #21448 ACKs for top commit: laanwj: Code review ACK fab1eb65b196d62466fdc2ed319ffa19d3560a0c Tree-SHA512: cad9f684f43aa801d0c1cb5f1684ffa624df1216be225cea46b1389ba2b67cbd6159ffb786fe144bf1ca865623dd9a10289d4293cfabb678bdb243d4ea00734d --- test/functional/feature_config_args.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 72b6801b8a..8447ecb496 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -165,6 +165,7 @@ class ConfArgsTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(expected_msgs=[ "Loaded 0 addresses from peers.dat", "0 addresses found from DNS seeds", + "opencon thread start", # Ensure ThreadOpenConnections::start time is properly set ]): self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}']) with self.nodes[0].assert_debug_log(expected_msgs=[ @@ -206,6 +207,7 @@ class ConfArgsTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(expected_msgs=[ "Loaded 0 addresses from peers.dat", "DNS seeding disabled", + "opencon thread start", # Ensure ThreadOpenConnections::start time is properly set ]): self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1', '-addnode=fakenodeaddr', f'-mocktime={start}']) with self.nodes[0].assert_debug_log(expected_msgs=[ From a02a2c03222343862f12cdeba012153ea2d43e81 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 5 May 2021 18:31:48 +0200 Subject: [PATCH 05/11] Merge bitcoin/bitcoin#21681: validation: fix ActivateSnapshot to use hardcoded nChainTx 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672 validation: remove nchaintx from assumeutxo metadata (James O'Beirne) 931684b24a89aba884cb18c13fa67ccca339ee8c validation: fix ActivateSnapshot to use hardcoded nChainTx (James O'Beirne) Pull request description: This fixes an oversight from the move of nChainTx from the user-supplied snapshot metadata into the hardcoded assumeutxo chainparams. Since the nChainTx is now unused in the metadata, it should be removed in a future commit. See: https://github.com/bitcoin/bitcoin/pull/19806#discussion_r612165410 ACKs for top commit: Sjors: utACK 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672 ryanofsky: Code review ACK 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672. No change to previous commit, just new commit removing now unused utxo snapshot field and updating tests. Tree-SHA512: 445bdd738faf007451f40bbcf360dd1fb4675e17a4c96546e6818c12e33dd336dadd95cf8d4b5f8df1d6ccfbc4bf5496864bb5528e416cea894857b6b732140c --- src/node/utxo_snapshot.h | 9 ++------- src/test/validation_chainstatemanager_tests.cpp | 6 ++++++ src/validation.cpp | 2 +- test/functional/rpc_dumptxoutset.py | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index fe78cb46bd..61292cdcc5 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -22,20 +22,15 @@ public: //! during snapshot load to estimate progress of UTXO set reconstruction. uint64_t m_coins_count = 0; - //! Necessary to "fake" the base nChainTx so that we can estimate progress during - //! initial block download for the assumeutxo chainstate. - unsigned int m_nchaintx = 0; - SnapshotMetadata() { } SnapshotMetadata( const uint256& base_blockhash, uint64_t coins_count, unsigned int nchaintx) : m_base_blockhash(base_blockhash), - m_coins_count(coins_count), - m_nchaintx(nchaintx) { } + m_coins_count(coins_count) { } - SERIALIZE_METHODS(SnapshotMetadata, obj) { READWRITE(obj.m_base_blockhash, obj.m_coins_count, obj.m_nchaintx); } + SERIALIZE_METHODS(SnapshotMetadata, obj) { READWRITE(obj.m_base_blockhash, obj.m_coins_count); } }; #endif // BITCOIN_NODE_UTXO_SNAPSHOT_H diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 1ca640d952..e4d6c5d62d 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -247,6 +247,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can // be found. + constexpr int snapshot_height = 110; mineBlocks(10); initial_size += 10; initial_total_coins += 10; @@ -292,6 +293,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup) *chainman.ActiveChainstate().m_from_snapshot_blockhash, *chainman.SnapshotBlockhash()); + const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params()); + const CBlockIndex* tip = chainman.ActiveTip(); + + BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx); + // To be checked against later when we try loading a subsequent snapshot. uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()}; diff --git a/src/validation.cpp b/src/validation.cpp index 4a584e51d1..4704580ee8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -6013,7 +6013,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( } assert(index); - index->nChainTx = metadata.m_nchaintx; + index->nChainTx = au_data.nChainTx; snapshot_chainstate.setBlockIndexCandidates.insert(snapshot_start_block); LogPrintf("[snapshot] validated snapshot (%.2f MB)\n", diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 7b2d02c3a6..ecd93593c1 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -43,7 +43,7 @@ class DumptxoutsetTest(BitcoinTestFramework): digest = hashlib.sha256(f.read()).hexdigest() # UTXO snapshot hash should be deterministic based on mocked time. assert_equal( - digest, '83ec62f0b9d9f2cdfe4514e8996d5ba0a6aa4cf74517988670d912db83bc0318') + digest, '32f1d4b7f643c97e88c540f431e8277fdd9332c3dea260b046c93787745e35b0') # Specifying a path to an existing file will fail. assert_raises_rpc_error( From 51633d70eaefbfdd4039c3b5521697b7630c4ba5 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 7 May 2021 15:44:16 +0200 Subject: [PATCH 06/11] Merge bitcoin/bitcoin#21874: fuzz: Add WRITE_ALL_FUZZ_TARGETS_AND_ABORT fa5cb6b268554fe0f272833794a98106872ac6a5 fuzz: Add WRITE_ALL_FUZZ_TARGETS_AND_ABORT (MarcoFalke) Pull request description: This is needed when stdout is polluted by the fuzz engine. stderr can't be used instead because it is polluted by aborting the program. Top commit has no ACKs. Tree-SHA512: bf0a2a6bcd964ff1f0f3ef6e7e297b4c780430c4d6312332ed99ace0e1c58243c1483fd387e39405837d39b36072dfeb9ae03d2a7aa728ad6955159754fd5766 --- src/test/fuzz/fuzz.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 3c83691a07..4b03d88d01 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -46,13 +46,24 @@ void initialize() return WrappedGetAddrInfo(name, false); }; + bool should_abort{false}; if (std::getenv("PRINT_ALL_FUZZ_TARGETS_AND_ABORT")) { for (const auto& t : FuzzTargets()) { if (std::get<2>(t.second)) continue; std::cout << t.first << std::endl; } - Assert(false); + should_abort = true; } + if (const char* out_path = std::getenv("WRITE_ALL_FUZZ_TARGETS_AND_ABORT")) { + std::cout << "Writing all fuzz target names to '" << out_path << "'." << std::endl; + std::ofstream out_stream(out_path, std::ios::binary); + for (const auto& t : FuzzTargets()) { + if (std::get<2>(t.second)) continue; + out_stream << t.first << std::endl; + } + should_abort = true; + } + Assert(!should_abort); std::string_view fuzz_target{Assert(std::getenv("FUZZ"))}; const auto it = FuzzTargets().find(fuzz_target); Assert(it != FuzzTargets().end()); From eeec2f279990f4a46dd89e71106cb25df7e71321 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sun, 9 May 2021 11:25:18 +0800 Subject: [PATCH 07/11] Merge bitcoin/bitcoin#21884: fuzz: Remove unused --enable-danger-fuzz-link-all option fa27d6d3ac065684a1219e9a948514d27929cf7c fuzz: Remove unused --enable-danger-fuzz-link-all option (MarcoFalke) Pull request description: Remove the unused build option, which was *dangerous* (as the name implies). Also remove the fuzzbuzz config, which was never used as part of this repo and seems redundant now that we integrate with oss-fuzz. ACKs for top commit: practicalswift: cr ACK fa27d6d3ac065684a1219e9a948514d27929cf7c: patch looks correct and rationale makes sense hebasto: ACK fa27d6d3ac065684a1219e9a948514d27929cf7c, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 9bd65ed6a76d13d8d9c7a88aaae30f701215d5d0619693a3115d5ec350808aaf6a1aa4737466a5b96f3948513ec4d063808fe16219818366720e247880a15177 --- .fuzzbuzz.yml | 16 ---------------- configure.ac | 7 ------- src/Makefile.test.include | 5 ----- src/test/fuzz/danger_link_all.sh | 28 ---------------------------- 4 files changed, 56 deletions(-) delete mode 100644 .fuzzbuzz.yml delete mode 100755 src/test/fuzz/danger_link_all.sh diff --git a/.fuzzbuzz.yml b/.fuzzbuzz.yml deleted file mode 100644 index feace80d60..0000000000 --- a/.fuzzbuzz.yml +++ /dev/null @@ -1,16 +0,0 @@ -base: ubuntu:focal -language: c++ -engine: libFuzzer -environment: - - CXXFLAGS=-fcoverage-mapping -fno-omit-frame-pointer -fprofile-instr-generate -gline-tables-only -O1 -setup: - - sudo apt-get update - - sudo apt-get install -y autoconf bsdmainutils clang git libboost-system-dev libboost-filesystem-dev libboost-test-dev libc++1 libc++abi1 libc++abi-dev libc++-dev libclang1 libclang-dev libdb5.3++ libevent-dev libllvm-ocaml-dev libomp5 libomp-dev libqt5core5a libqt5dbus5 libqt5gui5 libtool llvm llvm-dev llvm-runtime pkg-config qttools5-dev qttools5-dev-tools software-properties-common - - ./autogen.sh - - CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined --enable-danger-fuzz-link-all - - make - - git clone https://github.com/bitcoin-core/qa-assets -auto_targets: - find_targets_command: find src/test/fuzz/ -executable -type f ! -name "*.cpp" ! -name "*.h" - base_corpus_dir: qa-assets/fuzz_seed_corpus/ - memory_limit: none diff --git a/configure.ac b/configure.ac index 76133de1de..7249fe4fc0 100644 --- a/configure.ac +++ b/configure.ac @@ -185,12 +185,6 @@ AC_ARG_ENABLE([fuzz-binary], [enable_fuzz_binary=$enableval], [enable_fuzz_binary=yes]) -AC_ARG_ENABLE([danger_fuzz_link_all], - AS_HELP_STRING([--enable-danger-fuzz-link-all], - [Danger! Modifies source code. Needs git and gnu sed installed. Link each fuzz target (default no).]), - [enable_danger_fuzz_link_all=$enableval], - [enable_danger_fuzz_link_all=no]) - AC_ARG_WITH([qrencode], [AS_HELP_STRING([--with-qrencode], [enable QR code support (default is yes if qt is enabled and libqrencode is found)])], @@ -1818,7 +1812,6 @@ AM_CONDITIONAL([ENABLE_TRACING],[test x$have_sdt = xyes]) AM_CONDITIONAL([ENABLE_TESTS],[test x$BUILD_TEST = xyes]) AM_CONDITIONAL([ENABLE_FUZZ],[test x$enable_fuzz = xyes]) AM_CONDITIONAL([ENABLE_FUZZ_BINARY],[test x$enable_fuzz_binary = xyes]) -AM_CONDITIONAL([ENABLE_FUZZ_LINK_ALL],[test x$enable_danger_fuzz_link_all = xyes]) AM_CONDITIONAL([ENABLE_QT],[test x$bitcoin_enable_qt = xyes]) AM_CONDITIONAL([ENABLE_QT_TESTS],[test x$BUILD_TEST_QT = xyes]) AM_CONDITIONAL([ENABLE_BENCH],[test x$use_bench = xyes]) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 4f54e3276e..3f71e6112f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -398,11 +398,6 @@ univalue_test_object_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) univalue_test_object_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) endif -if ENABLE_FUZZ_LINK_ALL -all-local: $(FUZZ_BINARY) - bash ./test/fuzz/danger_link_all.sh -endif - %.cpp.test: %.cpp @echo Running tests: $$(\ cat $< | \ diff --git a/src/test/fuzz/danger_link_all.sh b/src/test/fuzz/danger_link_all.sh deleted file mode 100755 index 2ddd00c658..0000000000 --- a/src/test/fuzz/danger_link_all.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env bash -# Copyright (c) 2020 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -export LC_ALL=C.UTF-8 - -set -e - -ROOT_DIR="$(git rev-parse --show-toplevel)" - -# Run only once (break make recursion) -if [ -d "${ROOT_DIR}/lock_fuzz_link_all" ]; then - exit -fi -mkdir "${ROOT_DIR}/lock_fuzz_link_all" - -echo "Linking each fuzz target separately." -for FUZZING_HARNESS in $(PRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 "${ROOT_DIR}/src/test/fuzz/fuzz" | sort -u); do - echo "Building src/test/fuzz/${FUZZING_HARNESS} ..." - git checkout -- "${ROOT_DIR}/src/test/fuzz/fuzz.cpp" - sed -i "s/std::getenv(\"FUZZ\")/\"${FUZZING_HARNESS}\"/g" "${ROOT_DIR}/src/test/fuzz/fuzz.cpp" - make - mv "${ROOT_DIR}/src/test/fuzz/fuzz" "${ROOT_DIR}/src/test/fuzz/${FUZZING_HARNESS}" -done -git checkout -- "${ROOT_DIR}/src/test/fuzz/fuzz.cpp" -rmdir "${ROOT_DIR}/lock_fuzz_link_all" -echo "Successfully built all fuzz targets." From ecade9bc39cd3f0e87c6439e95dbc70249faf7f1 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Mon, 10 May 2021 13:49:29 +0200 Subject: [PATCH 08/11] Merge bitcoin/bitcoin#21749: test: Bump shellcheck version 08f3dbb1b0cd5ca01d87e488a2fa905adf7df057 test: Bump shellcheck version (Hennadii Stepanov) Pull request description: The changelog for v0.7.2 is available [here](https://github.com/koalaman/shellcheck/blob/v0.7.2/CHANGELOG.md). Only [SC2268](https://github.com/koalaman/shellcheck/wiki/SC2268) requires to update our code. ACKs for top commit: jarolrod: ACK 08f3dbb1b0cd5ca01d87e488a2fa905adf7df057 Tree-SHA512: 4585cd1f4d9def2fbaafe5a2a57761288d432781eb8c6c6d37064727d7ca8fc3f35c552e6a2ffdf0820d753d4bde2c8e43e5f3f57d242f5f57591a9b1b03558d --- ci/lint/04_install.sh | 2 +- test/README.md | 2 +- test/lint/commit-script-check.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index c936c3089b..f2c8d411d3 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -17,6 +17,6 @@ ${CI_RETRY_EXE} pip3 install vulture==2.3 ${CI_RETRY_EXE} pip3 install yq ${CI_RETRY_EXE} pip3 install mypy==0.781 -SHELLCHECK_VERSION=v0.7.1 +SHELLCHECK_VERSION=v0.7.2 curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/ export PATH="/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}" diff --git a/test/README.md b/test/README.md index 0836aef391..c2ec80f07d 100644 --- a/test/README.md +++ b/test/README.md @@ -308,7 +308,7 @@ Use the `-v` option for verbose output. |-----------|:----------:|:-------------------------------------------:|-------------- | [`lint-python.sh`](lint/lint-python.sh) | [flake8](https://gitlab.com/pycqa/flake8) | [3.8.3](https://github.com/bitcoin/bitcoin/pull/19348) | `pip3 install flake8==3.8.3` | [`lint-python.sh`](lint/lint-python.sh) | [mypy](https://github.com/python/mypy) | [0.781](https://github.com/bitcoin/bitcoin/pull/19348) | `pip3 install mypy==0.781` -| [`lint-shell.sh`](lint/lint-shell.sh) | [ShellCheck](https://github.com/koalaman/shellcheck) | [0.7.1](https://github.com/bitcoin/bitcoin/pull/19348) | [details...](https://github.com/koalaman/shellcheck#installing) +| [`lint-shell.sh`](lint/lint-shell.sh) | [ShellCheck](https://github.com/koalaman/shellcheck) | [0.7.2](https://github.com/bitcoin/bitcoin/pull/21749) | [details...](https://github.com/koalaman/shellcheck#installing) | [`lint-shell.sh`](lint/lint-shell.sh) | [yq](https://github.com/kislyuk/yq) | default | `pip3 install yq` | [`lint-spelling.sh`](lint/lint-spelling.sh) | [codespell](https://github.com/codespell-project/codespell) | [2.0.0](https://github.com/bitcoin/bitcoin/pull/20817) | `pip3 install codespell==2.0.0` diff --git a/test/lint/commit-script-check.sh b/test/lint/commit-script-check.sh index 766606a720..ecf8039839 100755 --- a/test/lint/commit-script-check.sh +++ b/test/lint/commit-script-check.sh @@ -12,7 +12,7 @@ # one. Any remaining diff signals an error. export LC_ALL=C -if test -z "$1"; then +if test -z $1; then echo "Usage: $0 ..." exit 1 fi From 51911388f24965c2bf537062bcaa43f1007406bb Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 15 Apr 2021 09:33:47 +0800 Subject: [PATCH 09/11] Merge #21377: Speedy trial support for versionbits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ffe33dfbd4c3b11e3475b022b6c1dd077613de79 chainparams: drop versionbits threshold to 90% for mainnnet and signet (Anthony Towns) f054f6bcd2c2ce5fea84cf8681013f85a444e7ea versionbits: simplify state transitions (Anthony Towns) 55ac5f568a3b73d6f1ef4654617fb76e8bcbccdf versionbits: Add explicit NEVER_ACTIVE deployments (Anthony Towns) dd07e6da48040dc7eae46bc7941db48d98a669fd fuzz: test versionbits delayed activation (Anthony Towns) dd85d5411c1702c8ae259610fe55050ba212e21e tests: test versionbits delayed activation (Anthony Towns) 73d4a706393e6dbd6b6d6b6428f8d3233ac0a2d8 versionbits: Add support for delayed activation (Anthony Towns) 9e6b65f6fa205eee5c3b99343988adcb8d320460 tests: clean up versionbits test (Anthony Towns) 593274445004506c921d5d851361aefb3434d744 tests: test ComputeBlockVersion for all deployments (Anthony Towns) 63879f0a4760c0c0f784029849cb5d21ee088abb tests: pull ComputeBlockVersion test into its own function (Anthony Towns) Pull request description: BIP9-based implementation of "speedy trial" activation specification, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-March/018583.html Edge cases are tested by fuzzing added in #21380. ACKs for top commit: instagibbs: tACK https://github.com/bitcoin/bitcoin/pull/21377/commits/ffe33dfbd4c3b11e3475b022b6c1dd077613de79 jnewbery: utACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 MarcoFalke: review ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 💈 achow101: re-ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 gmaxwell: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 benthecarman: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 Sjors: ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 jonatack: Initial approach ACK ffe33dfbd4c3b11e3475b022b6c1dd077613de79 after a first pass of review, building and testing each commit, mostly looking at the changes and diffs. Will do a more high-level review iteration. A few minor comments follow to pick/choose/ignore. ariard: Code Review ACK ffe33df Tree-SHA512: f79a7146b2450057ee92155cbbbcec12cd64334236d9239c6bd7d31b32eec145a9781c320f178da7b44ababdb8808b84d9d22a40e0851e229ba6d224e3be747c --- src/chainparams.cpp | 63 ++-- src/chainparamsbase.cpp | 4 +- src/consensus/params.h | 10 + src/evo/mnhftx.cpp | 1 + src/rpc/blockchain.cpp | 8 +- src/test/block_reward_reallocation_tests.cpp | 2 +- .../dynamic_activation_thresholds_tests.cpp | 2 +- src/test/fuzz/versionbits.cpp | 63 ++-- src/test/versionbits_tests.cpp | 309 +++++++++++++----- src/versionbits.cpp | 25 +- src/versionbits.h | 3 +- test/functional/feature_cltv.py | 2 +- test/functional/feature_dersig.py | 2 +- test/functional/feature_governance.py | 2 +- test/functional/feature_llmq_data_recovery.py | 2 +- test/functional/feature_mnehf.py | 4 +- .../feature_new_quorum_type_activation.py | 2 +- test/functional/rpc_blockchain.py | 3 + 18 files changed, 333 insertions(+), 174 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index a0043c0139..9f98fd16c6 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -200,11 +200,12 @@ public: consensus.fPowNoRetargeting = false; consensus.nPowKGWHeight = 15200; consensus.nPowDGWHeight = 34140; - consensus.nRuleChangeActivationThreshold = 1916; // 95% of 2016 + consensus.nRuleChangeActivationThreshold = 1815; // 90% of 2016 consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28; - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008 - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008 + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay consensus.vDeployments[Consensus::DEPLOYMENT_V20].bit = 9; consensus.vDeployments[Consensus::DEPLOYMENT_V20].nStartTime = 1700006400; // November 15, 2023 @@ -403,8 +404,9 @@ public: consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28; - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008 - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008 + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay consensus.vDeployments[Consensus::DEPLOYMENT_V20].bit = 9; consensus.vDeployments[Consensus::DEPLOYMENT_V20].nStartTime = 1693526400; // Friday, September 1, 2023 0:00:00 @@ -577,8 +579,9 @@ public: consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28; - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008 - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008 + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay consensus.vDeployments[Consensus::DEPLOYMENT_V20].bit = 9; consensus.vDeployments[Consensus::DEPLOYMENT_V20].nStartTime = 1661990400; // Sep 1st, 2022 @@ -815,9 +818,11 @@ public: consensus.nPowDGWHeight = 34140; // same as mainnet consensus.nRuleChangeActivationThreshold = 108; // 75% for testchains consensus.nMinerConfirmationWindow = 144; // Faster than normal for regtest (144 instead of 2016) + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28; consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 0; consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay consensus.vDeployments[Consensus::DEPLOYMENT_V20].bit = 9; consensus.vDeployments[Consensus::DEPLOYMENT_V20].nStartTime = 0; @@ -939,10 +944,11 @@ public: /** * Allows modifying the Version Bits regtest parameters. */ - void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout, int64_t nWindowSize, int64_t nThresholdStart, int64_t nThresholdMin, int64_t nFalloffCoeff, int64_t nUseEHF) + void UpdateVersionBitsParameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout, int min_activation_height, int64_t nWindowSize, int64_t nThresholdStart, int64_t nThresholdMin, int64_t nFalloffCoeff, int64_t nUseEHF) { consensus.vDeployments[d].nStartTime = nStartTime; consensus.vDeployments[d].nTimeout = nTimeout; + consensus.vDeployments[d].min_activation_height = min_activation_height; if (nWindowSize != -1) { consensus.vDeployments[d].nWindowSize = nWindowSize; } @@ -1022,45 +1028,50 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) for (const std::string& strDeployment : args.GetArgs("-vbparams")) { std::vector vDeploymentParams = SplitString(strDeployment, ':'); - if (vDeploymentParams.size() != 3 && vDeploymentParams.size() != 5 && vDeploymentParams.size() != 8) { + if (vDeploymentParams.size() != 3 && vDeploymentParams.size() != 4 && vDeploymentParams.size() != 6 && vDeploymentParams.size() != 9) { throw std::runtime_error("Version bits parameters malformed, expecting " ":: or " - ":::: or " - ":::::::"); + "::: or " + "::::: or " + "::::::::"); } int64_t nStartTime, nTimeout, nWindowSize = -1, nThresholdStart = -1, nThresholdMin = -1, nFalloffCoeff = -1, nUseEHF = -1; + int min_activation_height = 0; if (!ParseInt64(vDeploymentParams[1], &nStartTime)) { throw std::runtime_error(strprintf("Invalid nStartTime (%s)", vDeploymentParams[1])); } if (!ParseInt64(vDeploymentParams[2], &nTimeout)) { throw std::runtime_error(strprintf("Invalid nTimeout (%s)", vDeploymentParams[2])); } - if (vDeploymentParams.size() >= 5) { - if (!ParseInt64(vDeploymentParams[3], &nWindowSize)) { - throw std::runtime_error(strprintf("Invalid nWindowSize (%s)", vDeploymentParams[3])); + if (vDeploymentParams.size() >= 4 && !ParseInt32(vDeploymentParams[3], &min_activation_height)) { + throw std::runtime_error(strprintf("Invalid min_activation_height (%s)", vDeploymentParams[3])); + } + if (vDeploymentParams.size() >= 6) { + if (!ParseInt64(vDeploymentParams[4], &nWindowSize)) { + throw std::runtime_error(strprintf("Invalid nWindowSize (%s)", vDeploymentParams[4])); } - if (!ParseInt64(vDeploymentParams[4], &nThresholdStart)) { - throw std::runtime_error(strprintf("Invalid nThresholdStart (%s)", vDeploymentParams[4])); + if (!ParseInt64(vDeploymentParams[5], &nThresholdStart)) { + throw std::runtime_error(strprintf("Invalid nThresholdStart (%s)", vDeploymentParams[5])); } } - if (vDeploymentParams.size() == 8) { - if (!ParseInt64(vDeploymentParams[5], &nThresholdMin)) { - throw std::runtime_error(strprintf("Invalid nThresholdMin (%s)", vDeploymentParams[5])); + if (vDeploymentParams.size() == 9) { + if (!ParseInt64(vDeploymentParams[6], &nThresholdMin)) { + throw std::runtime_error(strprintf("Invalid nThresholdMin (%s)", vDeploymentParams[6])); } - if (!ParseInt64(vDeploymentParams[6], &nFalloffCoeff)) { - throw std::runtime_error(strprintf("Invalid nFalloffCoeff (%s)", vDeploymentParams[6])); + if (!ParseInt64(vDeploymentParams[7], &nFalloffCoeff)) { + throw std::runtime_error(strprintf("Invalid nFalloffCoeff (%s)", vDeploymentParams[7])); } - if (!ParseInt64(vDeploymentParams[7], &nUseEHF)) { - throw std::runtime_error(strprintf("Invalid nUseEHF (%s)", vDeploymentParams[7])); + if (!ParseInt64(vDeploymentParams[8], &nUseEHF)) { + throw std::runtime_error(strprintf("Invalid nUseEHF (%s)", vDeploymentParams[8])); } } bool found = false; for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) { - UpdateVersionBitsParameters(Consensus::DeploymentPos(j), nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); + UpdateVersionBitsParameters(Consensus::DeploymentPos(j), nStartTime, nTimeout, min_activation_height, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); found = true; - LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld, window=%ld, thresholdstart=%ld, thresholdmin=%ld, falloffcoeff=%ld, useehf=%ld\n", - vDeploymentParams[0], nStartTime, nTimeout, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); + LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld, min_activation_height=%ld, window=%ld, thresholdstart=%ld, thresholdmin=%ld, falloffcoeff=%ld, useehf=%ld\n", + vDeploymentParams[0], nStartTime, nTimeout, min_activation_height, nWindowSize, nThresholdStart, nThresholdMin, nFalloffCoeff, nUseEHF); break; } } diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index e80d02e84e..af4310de08 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -37,8 +37,8 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-regtest", "Enter regression test mode, which uses a special chain in which blocks can be solved instantly. " "This is intended for regression testing tools and app development. Equivalent to -chain=regtest", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-testnet", "Use the test chain. Equivalent to -chain=test", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-vbparams=::(::(:::))", - "Use given start/end times for specified version bits deployment (regtest-only). " + argsman.AddArg("-vbparams=::(:min_activation_height(::(:::)))", + "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only). " "Specifying window, threshold/thresholdstart, thresholdmin, falloffcoeff and mnactivation is optional.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); } diff --git a/src/consensus/params.h b/src/consensus/params.h index a0e185ee1f..7d6e9772c2 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -51,6 +51,11 @@ struct BIP9Deployment { int64_t nStartTime; /** Timeout/expiry MedianTime for the deployment attempt. */ int64_t nTimeout; + /** If lock in occurs, delay activation until at least this block + * height. Note that activation will only occur on a retarget + * boundary. + */ + int min_activation_height{0}; /** The number of past blocks (including the block under consideration) to be taken into account for locking in a fork. */ int64_t nWindowSize{0}; /** A starting number of blocks, in the range of 1..nWindowSize, which must signal for a fork in order to lock it in. */ @@ -73,6 +78,11 @@ struct BIP9Deployment { * process (which takes at least 3 BIP9 intervals). Only tests that specifically test the * behaviour during activation cannot use this. */ static constexpr int64_t ALWAYS_ACTIVE = -1; + + /** Special value for nStartTime indicating that the deployment is never active. + * This is useful for integrating the code changes for a new feature + * prior to deploying it on some or all networks. */ + static constexpr int64_t NEVER_ACTIVE = -2; }; /** diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 8919456360..003f753e1e 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -55,6 +55,7 @@ CMNHFManager::~CMNHFManager() CMNHFManager::Signals CMNHFManager::GetSignalsStage(const CBlockIndex* const pindexPrev) { Signals signals = GetForBlock(pindexPrev); + if (pindexPrev == nullptr) return {}; const int height = pindexPrev->nHeight + 1; for (auto it = signals.begin(); it != signals.end(); ) { bool found{false}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 50ec6f51db..3d996a8fe4 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1636,10 +1636,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 (e.g. testdummy) with timeout value before Jan 1, 2009 are hidden. - // A timeout value of 0 guarantees a softfork will never be activated. - // This is used when merging logic to implement a proposed softfork without a specified deployment schedule. - if (consensusParams.vDeployments[id].nTimeout <= 1230768000) return; + // Deployments that are never active are hidden. + if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; UniValue bip9(UniValue::VOBJ); const ThresholdState thresholdState = g_versionbitscache.State(active_chain_tip, consensusParams, id); @@ -1676,6 +1674,7 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, const std: else if (ThresholdState::LOCKED_IN == thresholdState) { bip9.pushKV("activation_height", since_height + static_cast(consensusParams.vDeployments[id].nWindowSize)); } + bip9.pushKV("min_activation_height", consensusParams.vDeployments[id].min_activation_height); UniValue rv(UniValue::VOBJ); rv.pushKV("type", "bip9"); @@ -1725,6 +1724,7 @@ RPCHelpMan getblockchaininfo() {RPCResult::Type::NUM, "ehf_height", /* optional */ true, "the minimum height when miner's signals for the deployment matter. Below this height miner signaling cannot trigger hard fork lock-in. Not specified for non-EHF forks"}, {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"}, {RPCResult::Type::NUM, "activation_height", "expected activation height for this softfork (only for \"locked_in\" status)"}, + {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"}, {RPCResult::Type::OBJ, "statistics", "numeric statistics about BIP9 signalling for a softfork", { {RPCResult::Type::NUM, "period", "the length in blocks of the BIP9 signalling period"}, diff --git a/src/test/block_reward_reallocation_tests.cpp b/src/test/block_reward_reallocation_tests.cpp index 6feb162ac9..ad74c36960 100644 --- a/src/test/block_reward_reallocation_tests.cpp +++ b/src/test/block_reward_reallocation_tests.cpp @@ -41,7 +41,7 @@ using SimpleUTXOMap = std::map>; struct TestChainBRRBeforeActivationSetup : public TestChainSetup { // Force fast DIP3 activation - TestChainBRRBeforeActivationSetup() : TestChainSetup(497, {"-dip3params=30:50", "-vbparams=mn_rr:0:999999999999:20:16:12:5:1"}) {} + TestChainBRRBeforeActivationSetup() : TestChainSetup(497, {"-dip3params=30:50", "-vbparams=mn_rr:0:999999999999:0:20:16:12:5:1"}) {} }; static SimpleUTXOMap BuildSimpleUtxoMap(const std::vector& txs) diff --git a/src/test/dynamic_activation_thresholds_tests.cpp b/src/test/dynamic_activation_thresholds_tests.cpp index 6938b9e1c9..c5c06fdf91 100644 --- a/src/test/dynamic_activation_thresholds_tests.cpp +++ b/src/test/dynamic_activation_thresholds_tests.cpp @@ -36,7 +36,7 @@ static constexpr int threshold(int attempt) struct TestChainDATSetup : public TestChainSetup { - TestChainDATSetup() : TestChainSetup(window - 2, {"-vbparams=testdummy:0:999999999999:100:80:60:5:0"}) {} + TestChainDATSetup() : TestChainSetup(window - 2, {"-vbparams=testdummy:0:999999999999:0:100:80:60:5:0"}) {} void signal(int num_blocks, bool expected_lockin) { diff --git a/src/test/fuzz/versionbits.cpp b/src/test/fuzz/versionbits.cpp index a543bb06a7..5de130dab2 100644 --- a/src/test/fuzz/versionbits.cpp +++ b/src/test/fuzz/versionbits.cpp @@ -30,14 +30,16 @@ public: const int64_t m_end; const int m_period; const int m_threshold; + const int m_min_activation_height; const int m_bit; - TestConditionChecker(int64_t begin, int64_t end, int period, int threshold, int bit) - : m_begin{begin}, m_end{end}, m_period{period}, m_threshold{threshold}, m_bit{bit} + TestConditionChecker(int64_t begin, int64_t end, int period, int threshold, int min_activation_height, int bit) + : m_begin{begin}, m_end{end}, m_period{period}, m_threshold{threshold}, m_min_activation_height{min_activation_height}, m_bit{bit} { assert(m_period > 0); assert(0 <= m_threshold && m_threshold <= m_period); assert(0 <= m_bit && m_bit < 32 && m_bit < VERSIONBITS_NUM_BITS); + assert(0 <= m_min_activation_height); } bool Condition(const CBlockIndex* pindex, const Consensus::Params& params) const override { return Condition(pindex->nVersion); } @@ -46,6 +48,7 @@ public: int64_t EndTime(const Consensus::Params& params) const override { return m_end; } int Period(const Consensus::Params& params) const override { return m_period; } int Threshold(const Consensus::Params& params, int nAttempt) const override { return m_threshold; } + int MinActivationHeight(const Consensus::Params& params) const override { return m_min_activation_height; } ThresholdState GetStateFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateFor(pindexPrev, dummy_params, m_cache); } int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, dummy_params, m_cache); } @@ -146,32 +149,27 @@ FUZZ_TARGET_INIT(versionbits, initialize_versionbits) // pick the timestamp to switch based on a block // note states will change *after* these blocks because mediantime lags int start_block = fuzzed_data_provider.ConsumeIntegralInRange(0, period * (max_periods - 3)); - int end_block = fuzzed_data_provider.ConsumeIntegralInRange(start_block, period * (max_periods - 3)); + int end_block = fuzzed_data_provider.ConsumeIntegralInRange(0, period * (max_periods - 3)); start_time = block_start_time + start_block * interval; timeout = block_start_time + end_block * interval; - assert(start_time <= timeout); - // allow for times to not exactly match a block if (fuzzed_data_provider.ConsumeBool()) start_time += interval / 2; if (fuzzed_data_provider.ConsumeBool()) timeout += interval / 2; - - // this may make timeout too early; if so, don't run the test - if (start_time > timeout) return; } else { if (fuzzed_data_provider.ConsumeBool()) { start_time = Consensus::BIP9Deployment::ALWAYS_ACTIVE; - timeout = Consensus::BIP9Deployment::NO_TIMEOUT; always_active_test = true; } else { - start_time = 1199145601; // January 1, 2008 - timeout = 1230767999; // December 31, 2008 + start_time = Consensus::BIP9Deployment::NEVER_ACTIVE; never_active_test = true; } + timeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral(); } + int min_activation = fuzzed_data_provider.ConsumeIntegralInRange(0, period * max_periods); - TestConditionChecker checker(start_time, timeout, period, threshold, bit); + TestConditionChecker checker(start_time, timeout, period, threshold, min_activation, bit); // Early exit if the versions don't signal sensibly for the deployment if (!checker.Condition(ver_signal)) return; @@ -296,28 +294,35 @@ FUZZ_TARGET_INIT(versionbits, initialize_versionbits) assert(since == 0); assert(exp_state == ThresholdState::DEFINED); assert(current_block->GetMedianTimePast() < checker.m_begin); - assert(current_block->GetMedianTimePast() < checker.m_end); break; case ThresholdState::STARTED: assert(current_block->GetMedianTimePast() >= checker.m_begin); - assert(current_block->GetMedianTimePast() < checker.m_end); if (exp_state == ThresholdState::STARTED) { assert(blocks_sig < threshold); + assert(current_block->GetMedianTimePast() < checker.m_end); } else { assert(exp_state == ThresholdState::DEFINED); } break; case ThresholdState::LOCKED_IN: - assert(exp_state == ThresholdState::STARTED); - assert(current_block->GetMedianTimePast() < checker.m_end); - assert(blocks_sig >= threshold); + if (exp_state == ThresholdState::LOCKED_IN) { + assert(current_block->nHeight + 1 < min_activation); + } else { + assert(exp_state == ThresholdState::STARTED); + assert(blocks_sig >= threshold); + } break; case ThresholdState::ACTIVE: + assert(always_active_test || min_activation <= current_block->nHeight + 1); assert(exp_state == ThresholdState::ACTIVE || exp_state == ThresholdState::LOCKED_IN); break; case ThresholdState::FAILED: - assert(current_block->GetMedianTimePast() >= checker.m_end); - assert(exp_state != ThresholdState::LOCKED_IN && exp_state != ThresholdState::ACTIVE); + assert(never_active_test || current_block->GetMedianTimePast() >= checker.m_end); + if (exp_state == ThresholdState::STARTED) { + assert(blocks_sig < threshold); + } else { + assert(exp_state == ThresholdState::FAILED); + } break; default: assert(false); @@ -328,26 +333,20 @@ FUZZ_TARGET_INIT(versionbits, initialize_versionbits) assert(state == ThresholdState::ACTIVE || state == ThresholdState::FAILED); } - // "always active" has additional restrictions if (always_active_test) { + // "always active" has additional restrictions assert(state == ThresholdState::ACTIVE); assert(exp_state == ThresholdState::ACTIVE); assert(since == 0); + } else if (never_active_test) { + // "never active" does too + assert(state == ThresholdState::FAILED); + assert(exp_state == ThresholdState::FAILED); + assert(since == 0); } else { - // except for always active, the initial state is always DEFINED + // for signalled deployments, the initial state is always DEFINED assert(since > 0 || state == ThresholdState::DEFINED); assert(exp_since > 0 || exp_state == ThresholdState::DEFINED); } - - // "never active" does too - if (never_active_test) { - assert(state == ThresholdState::FAILED); - assert(since == period); - if (exp_since == 0) { - assert(exp_state == ThresholdState::DEFINED); - } else { - assert(exp_state == ThresholdState::FAILED); - } - } } } // namespace diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index fe341d10c2..104b55b138 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -46,6 +46,12 @@ public: int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, paramsDummy, cache); } }; +class TestDelayedActivationConditionChecker : public TestConditionChecker +{ +public: + int MinActivationHeight(const Consensus::Params& params) const override { return 15000; } +}; + class TestAlwaysActiveConditionChecker : public TestConditionChecker { public: @@ -55,8 +61,7 @@ public: class TestNeverActiveConditionChecker : public TestConditionChecker { public: - int64_t BeginTime(const Consensus::Params& params) const override { return 0; } - int64_t EndTime(const Consensus::Params& params) const override { return 1230768000; } + int64_t BeginTime(const Consensus::Params& params) const override { return Consensus::BIP9Deployment::NEVER_ACTIVE; } }; #define CHECKERS 6 @@ -70,6 +75,8 @@ class VersionBitsTester // The first one performs all checks, the second only 50%, the third only 25%, etc... // This is to test whether lack of cached information leads to the same results. TestConditionChecker checker[CHECKERS]; + // Another 6 that assume delayed activation + TestDelayedActivationConditionChecker checker_delayed[CHECKERS]; // Another 6 that assume always active activation TestAlwaysActiveConditionChecker checker_always[CHECKERS]; // Another 6 that assume never active activation @@ -79,14 +86,18 @@ class VersionBitsTester int num; public: - VersionBitsTester() : num(0) {} + VersionBitsTester() : num(1000) {} VersionBitsTester& Reset() { + // Have each group of tests be counted by the 1000s part, starting at 1000 + num = num - (num % 1000) + 1000; + for (unsigned int i = 0; i < vpblock.size(); i++) { delete vpblock[i]; } for (unsigned int i = 0; i < CHECKERS; i++) { checker[i] = TestConditionChecker(); + checker_delayed[i] = TestDelayedActivationConditionChecker(); checker_always[i] = TestAlwaysActiveConditionChecker(); checker_never[i] = TestNeverActiveConditionChecker(); } @@ -102,7 +113,7 @@ public: while (vpblock.size() < height) { CBlockIndex* pindex = new CBlockIndex(); pindex->nHeight = vpblock.size(); - pindex->pprev = vpblock.size() > 0 ? vpblock.back() : nullptr; + pindex->pprev = Tip(); pindex->nTime = nTime; pindex->nVersion = nVersion; pindex->BuildSkip(); @@ -111,34 +122,53 @@ public: return *this; } - VersionBitsTester& TestStateSinceHeight(int height) { + VersionBitsTester& TestStateSinceHeight(int height) + { + return TestStateSinceHeight(height, height); + } + + VersionBitsTester& TestStateSinceHeight(int height, int height_delayed) + { + const CBlockIndex* tip = Tip(); for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { - BOOST_CHECK_MESSAGE(checker[i].GetStateSinceHeightFor(vpblock.empty() ? nullptr : vpblock.back()) == height, strprintf("Test %i for StateSinceHeight", num)); - BOOST_CHECK_MESSAGE(checker_always[i].GetStateSinceHeightFor(vpblock.empty() ? nullptr : vpblock.back()) == 0, strprintf("Test %i for StateSinceHeight (always active)", num)); - - // never active may go from DEFINED -> FAILED at the first period - const auto never_height = checker_never[i].GetStateSinceHeightFor(vpblock.empty() ? nullptr : vpblock.back()); - BOOST_CHECK_MESSAGE(never_height == 0 || never_height == checker_never[i].Period(paramsDummy), strprintf("Test %i for StateSinceHeight (never active)", num)); + BOOST_CHECK_MESSAGE(checker[i].GetStateSinceHeightFor(tip) == height, strprintf("Test %i for StateSinceHeight", num)); + BOOST_CHECK_MESSAGE(checker_delayed[i].GetStateSinceHeightFor(tip) == height_delayed, strprintf("Test %i for StateSinceHeight (delayed)", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (always active)", num)); + BOOST_CHECK_MESSAGE(checker_never[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (never active)", num)); } } num++; return *this; } - VersionBitsTester& TestState(ThresholdState exp) { + VersionBitsTester& TestState(ThresholdState exp) + { + return TestState(exp, exp); + } + + VersionBitsTester& TestState(ThresholdState exp, ThresholdState exp_delayed) + { + if (exp != exp_delayed) { + // only expected differences are that delayed stays in locked_in longer + BOOST_CHECK_EQUAL(exp, ThresholdState::ACTIVE); + BOOST_CHECK_EQUAL(exp_delayed, ThresholdState::LOCKED_IN); + } + + const CBlockIndex* pindex = Tip(); for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { - const CBlockIndex* pindex = vpblock.empty() ? nullptr : vpblock.back(); ThresholdState got = checker[i].GetStateFor(pindex); + ThresholdState got_delayed = checker_delayed[i].GetStateFor(pindex); ThresholdState got_always = checker_always[i].GetStateFor(pindex); ThresholdState got_never = checker_never[i].GetStateFor(pindex); // nHeight of the next block. If vpblock is empty, the next (ie first) // block should be the genesis block with nHeight == 0. int height = pindex == nullptr ? 0 : pindex->nHeight + 1; BOOST_CHECK_MESSAGE(got == exp, strprintf("Test %i for %s height %d (got %s)", num, StateName(exp), height, StateName(got))); + BOOST_CHECK_MESSAGE(got_delayed == exp_delayed, strprintf("Test %i for %s height %d (got %s; delayed case)", num, StateName(exp_delayed), height, StateName(got_delayed))); BOOST_CHECK_MESSAGE(got_always == ThresholdState::ACTIVE, strprintf("Test %i for ACTIVE height %d (got %s; always active case)", num, height, StateName(got_always))); - BOOST_CHECK_MESSAGE(got_never == ThresholdState::DEFINED|| got_never == ThresholdState::FAILED, strprintf("Test %i for DEFINED/FAILED height %d (got %s; never active case)", num, height, StateName(got_never))); + BOOST_CHECK_MESSAGE(got_never == ThresholdState::FAILED, strprintf("Test %i for FAILED height %d (got %s; never active case)", num, height, StateName(got_never))); } } num++; @@ -151,7 +181,10 @@ public: VersionBitsTester& TestActive() { return TestState(ThresholdState::ACTIVE); } VersionBitsTester& TestFailed() { return TestState(ThresholdState::FAILED); } - CBlockIndex * Tip() { return vpblock.size() ? vpblock.back() : nullptr; } + // non-delayed should be active; delayed should still be locked in + VersionBitsTester& TestActiveDelayed() { return TestState(ThresholdState::ACTIVE, ThresholdState::LOCKED_IN); } + + CBlockIndex* Tip() { return vpblock.empty() ? nullptr : vpblock.back(); } }; BOOST_FIXTURE_TEST_SUITE(versionbits_tests, TestingSetup) @@ -159,18 +192,19 @@ BOOST_FIXTURE_TEST_SUITE(versionbits_tests, TestingSetup) BOOST_AUTO_TEST_CASE(versionbits_test) { for (int i = 0; i < 64; i++) { - // DEFINED -> FAILED + // DEFINED -> STARTED after timeout reached -> FAILED VersionBitsTester().TestDefined().TestStateSinceHeight(0) .Mine(1, TestTime(1), 0x100).TestDefined().TestStateSinceHeight(0) .Mine(11, TestTime(11), 0x100).TestDefined().TestStateSinceHeight(0) .Mine(989, TestTime(989), 0x100).TestDefined().TestStateSinceHeight(0) - .Mine(999, TestTime(20000), 0x100).TestDefined().TestStateSinceHeight(0) - .Mine(1000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(1000) - .Mine(1999, TestTime(30001), 0x100).TestFailed().TestStateSinceHeight(1000) - .Mine(2000, TestTime(30002), 0x100).TestFailed().TestStateSinceHeight(1000) - .Mine(2001, TestTime(30003), 0x100).TestFailed().TestStateSinceHeight(1000) - .Mine(2999, TestTime(30004), 0x100).TestFailed().TestStateSinceHeight(1000) - .Mine(3000, TestTime(30005), 0x100).TestFailed().TestStateSinceHeight(1000) + .Mine(999, TestTime(20000), 0x100).TestDefined().TestStateSinceHeight(0) // Timeout and start time reached simultaneously + .Mine(1000, TestTime(20000), 0).TestStarted().TestStateSinceHeight(1000) // Hit started, stop signalling + .Mine(1999, TestTime(30001), 0).TestStarted().TestStateSinceHeight(1000) + .Mine(2000, TestTime(30002), 0x100).TestFailed().TestStateSinceHeight(2000) // Hit failed, start signalling again + .Mine(2001, TestTime(30003), 0x100).TestFailed().TestStateSinceHeight(2000) + .Mine(2999, TestTime(30004), 0x100).TestFailed().TestStateSinceHeight(2000) + .Mine(3000, TestTime(30005), 0x100).TestFailed().TestStateSinceHeight(2000) + .Mine(4000, TestTime(30006), 0x100).TestFailed().TestStateSinceHeight(2000) // DEFINED -> STARTED -> FAILED .Reset().TestDefined().TestStateSinceHeight(0) @@ -182,19 +216,19 @@ BOOST_AUTO_TEST_CASE(versionbits_test) .Mine(3000, TestTime(20000), 0).TestFailed().TestStateSinceHeight(3000) // 50 old blocks (so 899 out of the past 1000) .Mine(4000, TestTime(20010), 0x100).TestFailed().TestStateSinceHeight(3000) - // DEFINED -> STARTED -> FAILED while threshold reached + // DEFINED -> STARTED -> LOCKEDIN after timeout reached -> ACTIVE .Reset().TestDefined().TestStateSinceHeight(0) .Mine(1, TestTime(1), 0).TestDefined().TestStateSinceHeight(0) .Mine(1000, TestTime(10000) - 1, 0x101).TestDefined().TestStateSinceHeight(0) // One second more and it would be defined .Mine(2000, TestTime(10000), 0x101).TestStarted().TestStateSinceHeight(2000) // So that's what happens the next period .Mine(2999, TestTime(30000), 0x100).TestStarted().TestStateSinceHeight(2000) // 999 new blocks - .Mine(3000, TestTime(30000), 0x100).TestFailed().TestStateSinceHeight(3000) // 1 new block (so 1000 out of the past 1000 are new) - .Mine(3999, TestTime(30001), 0).TestFailed().TestStateSinceHeight(3000) - .Mine(4000, TestTime(30002), 0).TestFailed().TestStateSinceHeight(3000) - .Mine(14333, TestTime(30003), 0).TestFailed().TestStateSinceHeight(3000) - .Mine(24000, TestTime(40000), 0).TestFailed().TestStateSinceHeight(3000) + .Mine(3000, TestTime(30000), 0x100).TestLockedIn().TestStateSinceHeight(3000) // 1 new block (so 1000 out of the past 1000 are new) + .Mine(3999, TestTime(30001), 0).TestLockedIn().TestStateSinceHeight(3000) + .Mine(4000, TestTime(30002), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000) + .Mine(14333, TestTime(30003), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000) + .Mine(24000, TestTime(40000), 0).TestActive().TestStateSinceHeight(4000, 15000) - // DEFINED -> STARTED -> LOCKEDIN at the last minute -> ACTIVE + // DEFINED -> STARTED -> LOCKEDIN before timeout -> ACTIVE .Reset().TestDefined() .Mine(1, TestTime(1), 0).TestDefined().TestStateSinceHeight(0) .Mine(1000, TestTime(10000) - 1, 0x101).TestDefined().TestStateSinceHeight(0) // One second more and it would be defined @@ -204,9 +238,10 @@ BOOST_AUTO_TEST_CASE(versionbits_test) .Mine(2999, TestTime(19999), 0x200).TestStarted().TestStateSinceHeight(2000) // 49 old blocks .Mine(3000, TestTime(29999), 0x200).TestLockedIn().TestStateSinceHeight(3000) // 1 old block (so 900 out of the past 1000) .Mine(3999, TestTime(30001), 0).TestLockedIn().TestStateSinceHeight(3000) - .Mine(4000, TestTime(30002), 0).TestActive().TestStateSinceHeight(4000) - .Mine(14333, TestTime(30003), 0).TestActive().TestStateSinceHeight(4000) - .Mine(24000, TestTime(40000), 0).TestActive().TestStateSinceHeight(4000) + .Mine(4000, TestTime(30002), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000) // delayed will not become active until height=15000 + .Mine(14333, TestTime(30003), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000) + .Mine(15000, TestTime(40000), 0).TestActive().TestStateSinceHeight(4000, 15000) + .Mine(24000, TestTime(40000), 0).TestActive().TestStateSinceHeight(4000, 15000) // DEFINED multiple periods -> STARTED multiple periods -> FAILED .Reset().TestDefined().TestStateSinceHeight(0) @@ -216,10 +251,16 @@ BOOST_AUTO_TEST_CASE(versionbits_test) .Mine(3000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000) .Mine(4000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000) .Mine(5000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000) + .Mine(5999, TestTime(20000), 0).TestStarted().TestStateSinceHeight(3000) .Mine(6000, TestTime(20000), 0).TestFailed().TestStateSinceHeight(6000) - .Mine(7000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(6000); + .Mine(7000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(6000) + .Mine(24000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(6000) // stay in FAILED no matter how much we signal + ; } +} +BOOST_AUTO_TEST_CASE(versionbits_sanity) +{ // Sanity checks of version bit deployments const auto chainParams = CreateChainParams(*m_node.args, CBaseChainParams::MAIN); const Consensus::Params &mainnetParams = chainParams->GetConsensus(); @@ -228,6 +269,13 @@ BOOST_AUTO_TEST_CASE(versionbits_test) // Make sure that no deployment tries to set an invalid bit. BOOST_CHECK_EQUAL(bitmask & ~(uint32_t)VERSIONBITS_TOP_MASK, bitmask); + // Check min_activation_height is on a retarget boundary + BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height % mainnetParams.nMinerConfirmationWindow, 0); + // Check min_activation_height is 0 for ALWAYS_ACTIVE and never active deployments + if (mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) { + BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height, 0); + } + // Verify that the deployment windows of different deployment using the // same bit are disjoint. // This test may need modification at such time as a new deployment @@ -244,81 +292,124 @@ BOOST_AUTO_TEST_CASE(versionbits_test) } } -BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) +/** Check that ComputeBlockVersion will set the appropriate bit correctly */ +static void check_computeblockversion(const Consensus::Params& params, Consensus::DeploymentPos dep) { - // Check that ComputeBlockVersion will set the appropriate bit correctly - // on mainnet. - const auto chainParams = CreateChainParams(*m_node.args, CBaseChainParams::MAIN); - const Consensus::Params &mainnetParams = chainParams->GetConsensus(); + // This implicitly uses versionbitscache, so clear it every time + g_versionbitscache.Clear(); - // Use the TESTDUMMY deployment for testing purposes. - int64_t bit = mainnetParams.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit; - int64_t nStartTime = mainnetParams.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime; - int64_t nTimeout = mainnetParams.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout; + int64_t bit = params.vDeployments[dep].bit; + int64_t nStartTime = params.vDeployments[dep].nStartTime; + int64_t nTimeout = params.vDeployments[dep].nTimeout; + int min_activation_height = params.vDeployments[dep].min_activation_height; + uint32_t window_size = params.vDeployments[dep].nWindowSize; + if (window_size == 0) window_size = params.nMinerConfirmationWindow; - assert(nStartTime < nTimeout); + // TODO: make these unit tests to works with EHF too + bool useEHF = params.vDeployments[dep].useEHF; + if (useEHF) return; + + // should not be any signalling for first block + BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); + + // always/never active deployments shouldn't need to be tested further + if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE) return; + if (nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; + + BOOST_REQUIRE(nStartTime < nTimeout); + BOOST_REQUIRE(nStartTime >= 0); + BOOST_REQUIRE(nTimeout <= std::numeric_limits::max() || nTimeout == Consensus::BIP9Deployment::NO_TIMEOUT); + BOOST_REQUIRE(0 <= bit && bit < 32); + BOOST_REQUIRE(((1 << bit) & VERSIONBITS_TOP_MASK) == 0); + BOOST_REQUIRE(min_activation_height >= 0); + BOOST_REQUIRE_EQUAL(min_activation_height % window_size, 0); // In the first chain, test that the bit is set by CBV until it has failed. // In the second chain, test the bit is set by CBV while STARTED and // LOCKED-IN, and then no longer set while ACTIVE. VersionBitsTester firstChain, secondChain; - // Start generating blocks before nStartTime - int64_t nTime = nStartTime - 1; + int64_t nTime = nStartTime; + + const CBlockIndex *lastBlock = nullptr; // Before MedianTimePast of the chain has crossed nStartTime, the bit // should not be set. - CBlockIndex *lastBlock = nullptr; - lastBlock = firstChain.Mine(mainnetParams.nMinerConfirmationWindow, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, mainnetParams) & (1< 0) { lastBlock = firstChain.Mine(nHeight+1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); - BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, mainnetParams) & (1<nHeight + 1 < min_activation_height) { + // check signalling continues while min_activation_height is not reached + lastBlock = secondChain.Mine(min_activation_height - 1, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); + BOOST_CHECK((g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1 << bit)) != 0); + // then reach min_activation_height, which was already REQUIRE'd to start a new period + lastBlock = secondChain.Mine(min_activation_height, nTime, VERSIONBITS_LAST_OLD_BLOCK_VERSION).Tip(); + } + + // Check that we don't signal after activation + BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(lastBlock, params) & (1<GetConsensus(), static_cast(i)); + } + } + + { + // Use regtest/testdummy to ensure we always exercise some + // deployment that's not always/never active + ArgsManager args; + args.ForceSetArg("-vbparams", "testdummy:1199145601:1230767999"); // January 1, 2008 - December 31, 2008 + const auto chainParams = CreateChainParams(args, CBaseChainParams::REGTEST); + check_computeblockversion(chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); + } + + { + // Use regtest/testdummy to ensure we always exercise the + // min_activation_height test, even if we're not using that in a + // live deployment + ArgsManager args; + args.ForceSetArg("-vbparams", "testdummy:1199145601:1230767999:403200"); // January 1, 2008 - December 31, 2008, min act height 403200 + const auto chainParams = CreateChainParams(args, CBaseChainParams::REGTEST); + check_computeblockversion(chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY); + } +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/versionbits.cpp b/src/versionbits.cpp index c4e0af68c0..939cbb825a 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -32,6 +32,7 @@ static int calculateStartHeight(const CBlockIndex* pindexPrev, ThresholdState st ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const { int nPeriod = Period(params); + int min_activation_height = MinActivationHeight(params); int64_t nTimeStart = BeginTime(params); int masternodeStartHeight = SignalHeight(pindexPrev, params); int64_t nTimeTimeout = EndTime(params); @@ -41,6 +42,11 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* return ThresholdState::ACTIVE; } + // Check if this deployment is never active. + if (nTimeStart == Consensus::BIP9Deployment::NEVER_ACTIVE) { + return ThresholdState::FAILED; + } + // A block's state is always the same as that of the first of its period, so it is computed based on a pindexPrev whose height equals a multiple of nPeriod - 1. if (pindexPrev != nullptr) { pindexPrev = pindexPrev->GetAncestor(pindexPrev->nHeight - ((pindexPrev->nHeight + 1) % nPeriod)); @@ -77,19 +83,13 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* switch (state) { case ThresholdState::DEFINED: { - if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) { - stateNext = ThresholdState::FAILED; - } else if (pindexPrev->GetMedianTimePast() >= nTimeStart && pindexPrev->nHeight >= masternodeStartHeight) { + if (pindexPrev->GetMedianTimePast() >= nTimeStart && pindexPrev->nHeight >= masternodeStartHeight) { stateNext = ThresholdState::STARTED; nStartHeight = pindexPrev->nHeight + 1; } break; } case ThresholdState::STARTED: { - if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) { - stateNext = ThresholdState::FAILED; - break; - } // We need to count const CBlockIndex* pindexCount = pindexPrev; int count = 0; @@ -103,12 +103,16 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* int nAttempt = (pindexCount->nHeight + 1 - nStartHeight) / nPeriod; if (count >= Threshold(params, nAttempt)) { stateNext = ThresholdState::LOCKED_IN; + } else if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) { + stateNext = ThresholdState::FAILED; } break; } case ThresholdState::LOCKED_IN: { - // Always progresses into ACTIVE. - stateNext = ThresholdState::ACTIVE; + // Progresses into ACTIVE provided activation height will have been reached. + if (pindexPrev->nHeight + 1 >= min_activation_height) { + stateNext = ThresholdState::ACTIVE; + } break; } case ThresholdState::FAILED: @@ -164,7 +168,7 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const { int64_t start_time = BeginTime(params); - if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE) { + if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE || start_time == Consensus::BIP9Deployment::NEVER_ACTIVE) { return 0; } @@ -223,6 +227,7 @@ protected: return it->second; } int64_t EndTime(const Consensus::Params& params) const override { return params.vDeployments[id].nTimeout; } + int MinActivationHeight(const Consensus::Params& params) const override { return params.vDeployments[id].min_activation_height; } int Period(const Consensus::Params& params) const override { return params.vDeployments[id].nWindowSize ? params.vDeployments[id].nWindowSize : params.nMinerConfirmationWindow; } int Threshold(const Consensus::Params& params, int nAttempt) const override { diff --git a/src/versionbits.h b/src/versionbits.h index c1880e856b..c02769809e 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -28,7 +28,7 @@ static const int32_t VERSIONBITS_NUM_BITS = 29; enum class ThresholdState { DEFINED, // First state that each softfork starts out as. The genesis block is by definition in this state for each deployment. STARTED, // For blocks past the starttime. - LOCKED_IN, // For one retarget period after the first retarget period with STARTED blocks of which at least threshold have the associated bit set in nVersion. + LOCKED_IN, // For at least one retarget period after the first retarget period with STARTED blocks of which at least threshold have the associated bit set in nVersion, until min_activation_height is reached. ACTIVE, // For all blocks after the LOCKED_IN retarget period (final state) FAILED, // For all blocks once the first retarget period after the timeout time is hit, if LOCKED_IN wasn't already reached (final state) }; @@ -61,6 +61,7 @@ protected: virtual int64_t BeginTime(const Consensus::Params& params) const =0; virtual int SignalHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params) const = 0; virtual int64_t EndTime(const Consensus::Params& params) const =0; + virtual int MinActivationHeight(const Consensus::Params& params) const { return 0; } virtual int Period(const Consensus::Params& params) const =0; virtual int Threshold(const Consensus::Params& params, int nAttempt) const =0; diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 0b677b653d..934ff4b6b8 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -60,7 +60,7 @@ class BIP65Test(BitcoinTestFramework): '-dip3params=9000:9000', '-par=1', # Use only one script thread to get the exact reject reason for testing '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard - '-vbparams=v20:0:999999999999:480:384:288:5:0' # Delay v20 for this test as we don't need it + '-vbparams=v20:0:999999999999:0:480:384:288:5:0' # Delay v20 for this test as we don't need it ]] self.setup_clean_chain = True self.rpc_timeout = 480 diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 7637c0cb78..919f2d6ee5 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -40,7 +40,7 @@ def unDERify(tx): class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1', '-vbparams=v20:0:999999999999:480:384:288:5:0']] # Use only one script thread to get the exact reject reason for testing + self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1', '-vbparams=v20:0:999999999999:0:480:384:288:5:0']] # Use only one script thread to get the exact reject reason for testing self.setup_clean_chain = True self.rpc_timeout = 240 diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 65309a0391..420419cbed 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -14,7 +14,7 @@ class DashGovernanceTest (DashTestFramework): def set_test_params(self): self.v20_start_time = 1417713500 # using adjusted v20 deployment params to test an edge case where superblock maturity window is equal to deployment window size - self.set_dash_test_params(6, 5, [["-budgetparams=10:10:10", f"-vbparams=v20:{self.v20_start_time}:999999999999:10:8:6:5:0"]] * 6, fast_dip3_enforcement=True) + self.set_dash_test_params(6, 5, [["-budgetparams=10:10:10", f"-vbparams=v20:{self.v20_start_time}:999999999999:0:10:8:6:5:0"]] * 6, fast_dip3_enforcement=True) def prepare_object(self, object_type, parent_hash, creation_time, revision, name, amount, payment_address): proposal_rev = revision diff --git a/test/functional/feature_llmq_data_recovery.py b/test/functional/feature_llmq_data_recovery.py index 0a939fbfa2..2ebdbec1ae 100755 --- a/test/functional/feature_llmq_data_recovery.py +++ b/test/functional/feature_llmq_data_recovery.py @@ -24,7 +24,7 @@ llmq_type_strings = {llmq_test: 'llmq_test', llmq_test_v17: 'llmq_test_v17'} class QuorumDataRecoveryTest(DashTestFramework): def set_test_params(self): - extra_args = [["-vbparams=testdummy:0:999999999999:10:8:6:5:-1"] for _ in range(9)] + extra_args = [["-vbparams=testdummy:0:999999999999:0:10:8:6:5:-1"] for _ in range(9)] self.set_dash_test_params(9, 7, fast_dip3_enforcement=True, extra_args=extra_args) self.set_dash_llmq_test_params(4, 3) diff --git a/test/functional/feature_mnehf.py b/test/functional/feature_mnehf.py index 283e19e15a..a7406e7197 100755 --- a/test/functional/feature_mnehf.py +++ b/test/functional/feature_mnehf.py @@ -25,7 +25,7 @@ from test_framework.util import ( class MnehfTest(DashTestFramework): def set_test_params(self): - extra_args = [["-vbparams=testdummy:0:999999999999:12:12:12:5:1", "-persistmempool=0"] for _ in range(4)] + extra_args = [["-vbparams=testdummy:0:999999999999:0:12:12:12:5:1", "-persistmempool=0"] for _ in range(4)] self.set_dash_test_params(4, 3, fast_dip3_enforcement=True, extra_args=extra_args) def skip_test_if_missing_module(self): @@ -35,7 +35,7 @@ class MnehfTest(DashTestFramework): for inode in range(self.num_nodes): self.log.info(f"Restart node {inode} with {self.extra_args[inode]}") if params is not None: - self.extra_args[inode][0] = f"-vbparams=testdummy:{params[0]}:{params[1]}:12:12:12:5:1" + self.extra_args[inode][0] = f"-vbparams=testdummy:{params[0]}:{params[1]}:0:12:12:12:5:1" self.log.info(f"Actual restart options: {self.extra_args[inode]}") self.restart_node(0) diff --git a/test/functional/feature_new_quorum_type_activation.py b/test/functional/feature_new_quorum_type_activation.py index de47d81460..dbbcd89218 100755 --- a/test/functional/feature_new_quorum_type_activation.py +++ b/test/functional/feature_new_quorum_type_activation.py @@ -17,7 +17,7 @@ class NewQuorumTypeActivationTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-vbparams=testdummy:0:999999999999:10:8:6:5:0"]] + self.extra_args = [["-vbparams=testdummy:0:999999999999:0:10:8:6:5:0"]] def run_test(self): self.log.info(get_bip9_details(self.nodes[0], 'testdummy')) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 2a6bf9a450..f1391def37 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -152,6 +152,7 @@ class BlockchainTest(BitcoinTestFramework): 'start_time': 0, 'timeout': 9223372036854775807, 'since': 0, + 'min_activation_height': 0, 'ehf': False, }, 'active': False}, 'mn_rr': { @@ -161,6 +162,7 @@ class BlockchainTest(BitcoinTestFramework): 'start_time': 0, 'timeout': 9223372036854775807, 'since': 0, + 'min_activation_height': 0, 'ehf': True, }, 'active': False}, @@ -179,6 +181,7 @@ class BlockchainTest(BitcoinTestFramework): 'count': 57, 'possible': True, }, + 'min_activation_height': 0, 'ehf': False, }, 'active': False}, From a933a60b1a9332e03047c7b226b654cd05a1fe66 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 17 Apr 2024 13:58:27 +0700 Subject: [PATCH 10/11] feat: new command line argument -bip147height for bitcoin#21373 This command line argument is substitute for -segwitheight so far as dash does not have a segwit feature --- src/chainparams.cpp | 19 +++++++++++++++++++ src/chainparamsbase.cpp | 1 + 2 files changed, 20 insertions(+) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 9f98fd16c6..29c0e20e03 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -861,6 +861,7 @@ public: UpdateActivationParametersFromArgs(args); UpdateDIP3ParametersFromArgs(args); UpdateDIP8ParametersFromArgs(args); + UpdateBIP147ParametersFromArgs(args); UpdateBudgetParametersFromArgs(args); genesis = CreateGenesisBlock(1417713337, 1096447, 0x207fffff, 1, 50 * COIN); @@ -986,6 +987,12 @@ public: } void UpdateDIP8ParametersFromArgs(const ArgsManager& args); + void UpdateBIP147Parameters(int nActivationHeight) + { + consensus.BIP147Height = nActivationHeight; + } + void UpdateBIP147ParametersFromArgs(const ArgsManager& args); + /** * Allows modifying the budget regtest parameters. */ @@ -1118,6 +1125,18 @@ void CRegTestParams::UpdateDIP8ParametersFromArgs(const ArgsManager& args) UpdateDIP8Parameters(nDIP8ActivationHeight); } +void CRegTestParams::UpdateBIP147ParametersFromArgs(const ArgsManager& args) +{ + if (!args.IsArgSet("-bip147height")) return; + int nBIP147Height; + const std::string strParams = args.GetArg("-bip147height", ""); + if (!ParseInt32(strParams, &nBIP147Height)) { + throw std::runtime_error(strprintf("Invalid activation height (%s)", strParams)); + } + LogPrintf("Setting BIP147 parameters to activation=%lld\n", nBIP147Height); + UpdateBIP147Parameters(nBIP147Height); +} + void CRegTestParams::UpdateBudgetParametersFromArgs(const ArgsManager& args) { if (!args.IsArgSet("-budgetparams")) return; diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index af4310de08..0ed763e735 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -21,6 +21,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-devnet=", "Use devnet chain with provided name", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-dip3params=:", "Override DIP3 activation and enforcement heights (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-dip8params=", "Override DIP8 activation height (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); + argsman.AddArg("-bip147height=", "Override BIP147 activation height (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-highsubsidyblocks=", "The number of blocks with a higher than normal subsidy to mine at the start of a chain. Block after that height will have fixed subsidy base. (default: 0, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-highsubsidyfactor=", "The factor to multiply the normal block subsidy by while in the highsubsidyblocks window of a chain (default: 1, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-llmqchainlocks=", "Override the default LLMQ type used for ChainLocks. Allows using ChainLocks with smaller LLMQs. (default: llmq_devnet, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); From 7cc77f3a30cd79fec39f9340d3fea0333500beca Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 1 Apr 2021 13:17:07 +0200 Subject: [PATCH 11/11] Merge #21373: test: generate fewer blocks in feature_nulldummy to fix timeouts, speed up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ccd976dd3dbb8f991dc1203ada2043f1736be5a4 test: use 327 fewer blocks in feature_nulldummy (Jon Atack) 68c280f19732fb96bc29113ce9c8007d0101868c test, refactor: abstract the feature_nulldummy blockheight values (Jon Atack) Pull request description: The resolved timeout issue seen in the CI can be reproduced locally by running `test/functional/feature_nulldummy.py --valgrind --loglevel=debug` Speeds up the normal test runtime for me from 3.8 to 2.2 seconds (debug build). Thanks to Marco Falke for the approach suggestion. ACKs for top commit: AnthonyRonning: reACK ccd976dd3dbb8f991dc1203ada2043f1736be5a4 - ran a few times with the rest of the tests and still passing for me with just the fewer block change. MarcoFalke: review ACK ccd976dd3dbb8f991dc1203ada2043f1736be5a4 🏝 Tree-SHA512: 38339dca4276d1972e3a5a5ee436da64e9e58fd3b50b186e34b07ade9523ac4c03f6c3869c5f2a59c23b43c44f87e712f8297a01a8d83202049c081e3eeb4445 --- test/functional/feature_nulldummy.py | 39 ++++++++++++++++------------ 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 47c2515c70..247a9617fd 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -6,27 +6,31 @@ Connect to a single node. Generate 2 blocks (save the coinbases for later). -Generate 427 more blocks. -[Policy/Consensus] Check that NULLDUMMY compliant transactions are accepted in the 430th block. +Generate COINBASE_MATURITY (CB) more blocks to ensure the coinbases are mature. +[Policy/Consensus] Check that NULLDUMMY compliant transactions are accepted in block CB + 3. [Policy] Check that non-NULLDUMMY transactions are rejected before activation. -[Consensus] Check that the new NULLDUMMY rules are not enforced on the 431st block. -[Policy/Consensus] Check that the new NULLDUMMY rules are enforced on the 432nd block. +[Consensus] Check that the new NULLDUMMY rules are not enforced on block CB + 4. +[Policy/Consensus] Check that the new NULLDUMMY rules are enforced on block CB + 5. """ -from test_framework.blocktools import NORMAL_GBT_REQUEST_PARAMS, create_block, create_transaction +from test_framework.blocktools import ( + COINBASE_MATURITY, + NORMAL_GBT_REQUEST_PARAMS, + create_block, + create_transaction, +) from test_framework.messages import CTransaction from test_framework.script import CScript from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error - NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)" def trueDummy(tx): scriptSig = CScript(tx.vin[0].scriptSig) newscript = [] for i in scriptSig: - if (len(newscript) == 0): + if len(newscript) == 0: assert len(i) == 0 newscript.append(b'\x51') else: @@ -37,10 +41,10 @@ def trueDummy(tx): class NULLDUMMYTest(BitcoinTestFramework): def set_test_params(self): - # Need two nodes only so GBT doesn't complain that it's not connected + # Need two nodes so GBT (getblocktemplate) doesn't complain that it's not connected. self.num_nodes = 2 self.setup_clean_chain = True - self.extra_args = [['-whitelist=127.0.0.1']] * 2 + self.extra_args = [['-whitelist=127.0.0.1', '-dip3params=105:105', '-bip147height=105']] * 2 def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -56,16 +60,16 @@ class NULLDUMMYTest(BitcoinTestFramework): # Legacy wallets need to import these so that they are watched by the wallet. This is unnecssary (and does not need to be tested) for descriptor wallets wmulti.importaddress(self.ms_address) - self.coinbase_blocks = self.nodes[0].generate(2) # Block 2 + self.coinbase_blocks = self.nodes[0].generate(2) # block height = 2 coinbase_txid = [] for i in self.coinbase_blocks: coinbase_txid.append(self.nodes[0].getblock(i)['tx'][0]) - self.nodes[0].generate(427) # Block 429 + self.nodes[0].generate(COINBASE_MATURITY) # block height = COINBASE_MATURITY + 2 self.lastblockhash = self.nodes[0].getbestblockhash() - self.lastblockheight = 429 - self.lastblocktime = self.mocktime + 429 + self.lastblockheight = COINBASE_MATURITY + 2 + self.lastblocktime = self.mocktime + self.lastblockheight - self.log.info("Test 1: NULLDUMMY compliant base transactions should be accepted to mempool and mined before activation [430]") + self.log.info(f"Test 1: NULLDUMMY compliant base transactions should be accepted to mempool and mined before activation [{COINBASE_MATURITY + 3}]") test1txs = [create_transaction(self.nodes[0], coinbase_txid[0], self.ms_address, amount=49)] txid1 = self.nodes[0].sendrawtransaction(test1txs[0].serialize().hex(), 0) test1txs.append(create_transaction(self.nodes[0], txid1, self.ms_address, amount=48)) @@ -77,7 +81,7 @@ class NULLDUMMYTest(BitcoinTestFramework): trueDummy(test2tx) assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test2tx.serialize().hex(), 0) - self.log.info("Test 3: Non-NULLDUMMY base transactions should be accepted in a block before activation [431]") + self.log.info(f"Test 3: Non-NULLDUMMY base transactions should be accepted in a block before activation [{COINBASE_MATURITY + 4}]") self.block_submit(self.nodes[0], [test2tx], True) self.log.info("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation") @@ -87,14 +91,14 @@ class NULLDUMMYTest(BitcoinTestFramework): assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test4tx.serialize().hex(), 0) self.block_submit(self.nodes[0], [test4tx]) - self.log.info("Test 6: NULLDUMMY compliant transactions should be accepted to mempool and in block after activation [432]") + self.log.info(f"Test 6: NULLDUMMY compliant base/witness transactions should be accepted to mempool and in block after activation [{COINBASE_MATURITY + 5}]") for i in test6txs: self.nodes[0].sendrawtransaction(i.serialize().hex(), 0) self.block_submit(self.nodes[0], test6txs, True) def block_submit(self, node, txs, accept = False): - dip4_activated = self.lastblockheight + 1 >= 432 + dip4_activated = self.lastblockheight + 1 >= COINBASE_MATURITY + 5 tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) assert_equal(tmpl['previousblockhash'], self.lastblockhash) assert_equal(tmpl['height'], self.lastblockheight + 1) @@ -114,5 +118,6 @@ class NULLDUMMYTest(BitcoinTestFramework): else: assert_equal(node.getbestblockhash(), self.lastblockhash) + if __name__ == '__main__': NULLDUMMYTest().main()