From fa2197c8b3178787d99e2acb5c3c717df14ddabf Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 30 Apr 2021 19:53:31 +0200 Subject: [PATCH 1/3] test: Use loop to register RPCs The same loop is used by the server, so no need for the tests to do this differently. --- src/qt/test/rpcnestedtests.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 9e6e98b274d..df3b85ea71b 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -35,14 +35,16 @@ static RPCHelpMan rpcNestedTest_rpc() } static const CRPCCommand vRPCCommands[] = { - {"test", &rpcNestedTest_rpc}, + {"rpcNestedTest", &rpcNestedTest_rpc}, }; void RPCNestedTests::rpcNestedTests() { // do some test setup // could be moved to a more generic place when we add more tests on QT level - tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); + for (const auto& c : vRPCCommands) { + tableRPC.appendCommand(c.name, &c); + } TestingSetup test; m_node.setContext(&test.m_node); From 000098f9647cd2e21660603b7d7a8f623f70f673 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 30 Apr 2021 18:55:06 +0200 Subject: [PATCH 2/3] test: Use throwing variant accessor It does not matter if the tests fail due to a BOOST_CHECK failure or due to a thrown exception. Prefer the exception because it is less code. Example fail with the throwing accessor: unknown location(0): fatal error: in "script_standard_tests/script_standard_ExtractDestinations": std::bad_variant_access: std::get: wrong index for variant test/script_standard_tests.cpp(314): last checkpoint *** 1 failure is detected in the test module "Bitcoin Core Test Suite" --- src/test/script_standard_tests.cpp | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 4dc0dd5f511..103a971d982 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -199,23 +199,20 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) s.clear(); s << ToByteVector(pubkey) << OP_CHECKSIG; BOOST_CHECK(ExtractDestination(s, address)); - BOOST_CHECK(std::get_if(&address) && - *std::get_if(&address) == PKHash(pubkey)); + BOOST_CHECK(std::get(address) == PKHash(pubkey)); // TxoutType::PUBKEYHASH s.clear(); s << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; BOOST_CHECK(ExtractDestination(s, address)); - BOOST_CHECK(std::get_if(&address) && - *std::get_if(&address) == PKHash(pubkey)); + BOOST_CHECK(std::get(address) == PKHash(pubkey)); // TxoutType::SCRIPTHASH CScript redeemScript(s); // initialize with leftover P2PKH script s.clear(); s << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL; BOOST_CHECK(ExtractDestination(s, address)); - BOOST_CHECK(std::get_if(&address) && - *std::get_if(&address) == ScriptHash(redeemScript)); + BOOST_CHECK(std::get(address) == ScriptHash(redeemScript)); // TxoutType::MULTISIG s.clear(); @@ -233,7 +230,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) BOOST_CHECK(ExtractDestination(s, address)); WitnessV0KeyHash keyhash; CHash160().Write(pubkey).Finalize(keyhash); - BOOST_CHECK(std::get_if(&address) && *std::get_if(&address) == keyhash); + BOOST_CHECK(std::get(address) == keyhash); // TxoutType::WITNESS_V0_SCRIPTHASH s.clear(); @@ -241,7 +238,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) CSHA256().Write(redeemScript.data(), redeemScript.size()).Finalize(scripthash.begin()); s << OP_0 << ToByteVector(scripthash); BOOST_CHECK(ExtractDestination(s, address)); - BOOST_CHECK(std::get_if(&address) && *std::get_if(&address) == scripthash); + BOOST_CHECK(std::get(address) == scripthash); // TxoutType::WITNESS_UNKNOWN with unknown version s.clear(); @@ -251,7 +248,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) unk.length = 33; unk.version = 1; std::copy(pubkey.begin(), pubkey.end(), unk.program); - BOOST_CHECK(std::get_if(&address) && *std::get_if(&address) == unk); + BOOST_CHECK(std::get(address) == unk); } BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) @@ -275,8 +272,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEY); BOOST_CHECK_EQUAL(addresses.size(), 1U); BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get_if(&addresses[0]) && - *std::get_if(&addresses[0]) == PKHash(pubkeys[0])); + BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); // TxoutType::PUBKEYHASH s.clear(); @@ -285,8 +281,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) BOOST_CHECK_EQUAL(whichType, TxoutType::PUBKEYHASH); BOOST_CHECK_EQUAL(addresses.size(), 1U); BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get_if(&addresses[0]) && - *std::get_if(&addresses[0]) == PKHash(pubkeys[0])); + BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); // TxoutType::SCRIPTHASH CScript redeemScript(s); // initialize with leftover P2PKH script @@ -296,8 +291,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) BOOST_CHECK_EQUAL(whichType, TxoutType::SCRIPTHASH); BOOST_CHECK_EQUAL(addresses.size(), 1U); BOOST_CHECK_EQUAL(nRequired, 1); - BOOST_CHECK(std::get_if(&addresses[0]) && - *std::get_if(&addresses[0]) == ScriptHash(redeemScript)); + BOOST_CHECK(std::get(addresses[0]) == ScriptHash(redeemScript)); // TxoutType::MULTISIG s.clear(); @@ -309,10 +303,8 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestinations) BOOST_CHECK_EQUAL(whichType, TxoutType::MULTISIG); BOOST_CHECK_EQUAL(addresses.size(), 2U); BOOST_CHECK_EQUAL(nRequired, 2); - BOOST_CHECK(std::get_if(&addresses[0]) && - *std::get_if(&addresses[0]) == PKHash(pubkeys[0])); - BOOST_CHECK(std::get_if(&addresses[1]) && - *std::get_if(&addresses[1]) == PKHash(pubkeys[1])); + BOOST_CHECK(std::get(addresses[0]) == PKHash(pubkeys[0])); + BOOST_CHECK(std::get(addresses[1]) == PKHash(pubkeys[1])); // TxoutType::NULL_DATA s.clear(); From fa8a88849c08c810a82338bf0e70738eb6748906 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 30 Apr 2021 17:47:31 +0200 Subject: [PATCH 3/3] bench: Remove duplicate constants --- src/bench/block_assemble.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 67ab02a5b33..aa72981cb4f 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -18,23 +19,17 @@ static void AssembleBlock(benchmark::Bench& bench) { const auto test_setup = MakeNoLogFileContext(); - const std::vector op_true{OP_TRUE}; CScriptWitness witness; - witness.stack.push_back(op_true); - - uint256 witness_program; - CSHA256().Write(&op_true[0], op_true.size()).Finalize(witness_program.begin()); - - const CScript SCRIPT_PUB{CScript(OP_0) << std::vector{witness_program.begin(), witness_program.end()}}; + witness.stack.push_back(WITNESS_STACK_ELEM_OP_TRUE); // Collect some loose transactions that spend the coinbases of our mined blocks constexpr size_t NUM_BLOCKS{200}; std::array txs; for (size_t b{0}; b < NUM_BLOCKS; ++b) { CMutableTransaction tx; - tx.vin.push_back(MineBlock(test_setup->m_node, SCRIPT_PUB)); + tx.vin.push_back(MineBlock(test_setup->m_node, P2WSH_OP_TRUE)); tx.vin.back().scriptWitness = witness; - tx.vout.emplace_back(1337, SCRIPT_PUB); + tx.vout.emplace_back(1337, P2WSH_OP_TRUE); if (NUM_BLOCKS - b >= COINBASE_MATURITY) txs.at(b) = MakeTransactionRef(tx); } @@ -48,7 +43,7 @@ static void AssembleBlock(benchmark::Bench& bench) } bench.run([&] { - PrepareBlock(test_setup->m_node, SCRIPT_PUB); + PrepareBlock(test_setup->m_node, P2WSH_OP_TRUE); }); }