diff options
Diffstat (limited to 'src')
32 files changed, 282 insertions, 633 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 7387eb1eaa..c2e0c7b5b8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -320,7 +320,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/spanparsing.h \ util/string.h \ - util/subprocess.hpp \ + util/subprocess.h \ util/syserror.h \ util/task_runner.h \ util/thread.h \ diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 830171f63a..8901d10ef6 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -549,7 +549,7 @@ public: peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, - peer.transport_protocol_type.starts_with('v') ? peer.transport_protocol_type[1] : ' ', + (peer.transport_protocol_type.size() == 2 && peer.transport_protocol_type[0] == 'v') ? peer.transport_protocol_type[1] : ' ', PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), peer.last_send ? ToString(time_now - peer.last_send) : "", diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index e5356490ef..347b486095 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,7 +12,7 @@ #include <univalue.h> #ifdef ENABLE_EXTERNAL_SIGNER -#include <util/subprocess.hpp> +#include <util/subprocess.h> #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) diff --git a/src/core_read.cpp b/src/core_read.cpp index e32e46d1b9..5956d9df5f 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -256,6 +256,6 @@ util::Result<int> SighashFromStr(const std::string& sighash) if (it != map_sighash_values.end()) { return it->second; } else { - return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")}; + return util::Error{Untranslated("'" + sighash + "' is not a valid sighash parameter.")}; } } diff --git a/src/init.cpp b/src/init.cpp index e8e6391af7..c19d596c7f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1301,30 +1301,33 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - for (const std::string port_option : { - "-i2psam", - "-onion", - "-proxy", - "-rpcbind", - "-torcontrol", - "-whitebind", - "-zmqpubhashblock", - "-zmqpubhashtx", - "-zmqpubrawblock", - "-zmqpubrawtx", - "-zmqpubsequence", + for (const auto &port_option : std::vector<std::pair<std::string, bool>>{ + // arg name UNIX socket support + {"-i2psam", false}, + {"-onion", true}, + {"-proxy", true}, + {"-rpcbind", false}, + {"-torcontrol", false}, + {"-whitebind", false}, + {"-zmqpubhashblock", true}, + {"-zmqpubhashtx", true}, + {"-zmqpubrawblock", true}, + {"-zmqpubrawtx", true}, + {"-zmqpubsequence", true} }) { - for (const std::string& socket_addr : args.GetArgs(port_option)) { + const std::string arg{port_option.first}; + const bool unix{port_option.second}; + for (const std::string& socket_addr : args.GetArgs(arg)) { std::string host_out; uint16_t port_out{0}; if (!SplitHostPort(socket_addr, port_out, host_out)) { #if HAVE_SOCKADDR_UN - // Allow unix domain sockets for -proxy and -onion e.g. unix:/some/file/path - if ((port_option != "-proxy" && port_option != "-onion") || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + // Allow unix domain sockets for some options e.g. unix:/some/file/path + if (!unix || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { + return InitError(InvalidPortErrMsg(arg, socket_addr)); } #else - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + return InitError(InvalidPortErrMsg(arg, socket_addr)); #endif } } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 6114236623..c41f35829d 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -127,7 +127,7 @@ public: virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0; //! Display address on external signer - virtual bool displayAddress(const CTxDestination& dest) = 0; + virtual util::Result<void> displayAddress(const CTxDestination& dest) = 0; //! Lock coin. virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; diff --git a/src/net.cpp b/src/net.cpp index e388f05b03..3e959c187c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2256,7 +2256,11 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + // Limit number of IPs learned from a single DNS seed. This limit exists to prevent the results from + // one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is + // bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be + // returned. + unsigned int nMaxIPs = 32; const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { diff --git a/src/netbase.cpp b/src/netbase.cpp index 3ca1a5227a..f0fa298378 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -632,10 +632,7 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti std::unique_ptr<Sock> Proxy::Connect() const { - if (!IsValid()) { - LogPrintf("Cannot connect to invalid Proxy\n"); - return {}; - } + if (!IsValid()) return {}; if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true); @@ -656,7 +653,6 @@ std::unique_ptr<Sock> Proxy::Connect() const socklen_t len = sizeof(addrun); if(!ConnectToSocket(*sock, (struct sockaddr*)&addrun, len, path, /*manual_connection=*/true)) { - LogPrintf("Cannot connect to socket for %s\n", path); return {}; } diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 84c3352b9d..2ad79b6f99 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -191,13 +191,13 @@ std::optional<std::pair<DiagramCheckError, std::string>> ImprovesFeerateDiagram( int64_t replacement_vsize) { // Require that the replacement strictly improves the mempool's feerate diagram. - const auto diagram_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + const auto chunk_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; - if (!diagram_results.has_value()) { - return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(diagram_results).original); + if (!chunk_results.has_value()) { + return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(chunk_results).original); } - if (!std::is_gt(CompareFeerateDiagram(diagram_results.value().second, diagram_results.value().first))) { + if (!std::is_gt(CompareChunks(chunk_results.value().second, chunk_results.value().first))) { return std::make_pair(DiagramCheckError::FAILURE, "insufficient feerate: does not improve feerate diagram"); } return std::nullopt; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 1bdf94d3b5..fe000bcbb8 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -569,16 +569,17 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } -bool WalletModel::displayAddress(std::string sAddress) const +void WalletModel::displayAddress(std::string sAddress) const { CTxDestination dest = DecodeDestination(sAddress); - bool res = false; try { - res = m_wallet->displayAddress(dest); + util::Result<void> result = m_wallet->displayAddress(dest); + if (!result) { + QMessageBox::warning(nullptr, tr("Signer error"), QString::fromStdString(util::ErrorString(result).translated)); + } } catch (const std::runtime_error& e) { QMessageBox::critical(nullptr, tr("Can't display address"), e.what()); } - return res; } bool WalletModel::isWalletEnabled() diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 503ee16823..ab2096c1fe 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -130,7 +130,7 @@ public: UnlockContext requestUnlock(); bool bumpFee(uint256 hash, uint256& new_hash); - bool displayAddress(std::string sAddress) const; + void displayAddress(std::string sAddress) const; static bool isWalletEnabled(); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index be4b357568..22ac062a63 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -295,7 +295,7 @@ struct TapSatisfier: Satisfier<XOnlyPubKey> { //! Conversion from a raw xonly public key. template <typename I> std::optional<XOnlyPubKey> FromPKBytes(I first, I last) const { - CHECK_NONFATAL(last - first == 32); + if (last - first != 32) return {}; XOnlyPubKey pubkey; std::copy(first, last, pubkey.begin()); return pubkey; diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 2e015b382e..5af3c3d7ed 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -82,43 +82,4 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) } -BOOST_AUTO_TEST_CASE(build_diagram_test) -{ - FeeFrac p1{1000, 100}; - FeeFrac empty{0, 0}; - FeeFrac zero_fee{0, 1}; - FeeFrac oversized_1{4611686000000, 4000000}; - FeeFrac oversized_2{184467440000000, 100000}; - - // Diagram-building will reorder chunks - std::vector<FeeFrac> chunks; - chunks.push_back(p1); - chunks.push_back(zero_fee); - chunks.push_back(empty); - chunks.push_back(oversized_1); - chunks.push_back(oversized_2); - - // Caller in charge of sorting chunks in case chunk size limit - // differs from cluster size limit - std::sort(chunks.begin(), chunks.end(), [](const FeeFrac& a, const FeeFrac& b) { return a > b; }); - - // Chunks are now sorted in reverse order (largest first) - BOOST_CHECK(chunks[0] == empty); // empty is considered "highest" fee - BOOST_CHECK(chunks[1] == oversized_2); - BOOST_CHECK(chunks[2] == oversized_1); - BOOST_CHECK(chunks[3] == p1); - BOOST_CHECK(chunks[4] == zero_fee); - - std::vector<FeeFrac> generated_diagram{BuildDiagramFromChunks(chunks)}; - BOOST_CHECK(generated_diagram.size() == 1 + chunks.size()); - - // Prepended with an empty, then the chunks summed in order as above - BOOST_CHECK(generated_diagram[0] == empty); - BOOST_CHECK(generated_diagram[1] == empty); - BOOST_CHECK(generated_diagram[2] == oversized_2); - BOOST_CHECK(generated_diagram[3] == oversized_2 + oversized_1); - BOOST_CHECK(generated_diagram[4] == oversized_2 + oversized_1 + p1); - BOOST_CHECK(generated_diagram[5] == oversized_2 + oversized_1 + p1 + zero_fee); -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/feeratediagram.cpp b/src/test/fuzz/feeratediagram.cpp index 6d710093cb..1a9c5ee946 100644 --- a/src/test/fuzz/feeratediagram.cpp +++ b/src/test/fuzz/feeratediagram.cpp @@ -16,6 +16,20 @@ namespace { +/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */ +std::vector<FeeFrac> BuildDiagramFromChunks(const Span<const FeeFrac> chunks) +{ + std::vector<FeeFrac> diagram; + diagram.reserve(chunks.size() + 1); + + diagram.emplace_back(0, 0); + for (auto& chunk : chunks) { + diagram.emplace_back(diagram.back() + chunk); + } + return diagram; +} + + /** Evaluate a diagram at a specific size, returning the fee as a fraction. * * Fees in diagram cannot exceed 2^32, as the returned evaluation could overflow @@ -103,7 +117,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram) assert(diagram1.front() == empty); assert(diagram2.front() == empty); - auto real = CompareFeerateDiagram(diagram1, diagram2); + auto real = CompareChunks(chunks1, chunks2); auto sim = CompareDiagrams(diagram1, diagram2); assert(real == sim); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 754aff4e70..64785948f6 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -82,17 +82,6 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) } } -void CheckDiagramConcave(std::vector<FeeFrac>& diagram) -{ - // Diagrams are in monotonically-decreasing feerate order. - FeeFrac last_chunk = diagram.front(); - for (size_t i = 1; i<diagram.size(); ++i) { - FeeFrac next_chunk = diagram[i] - diagram[i-1]; - assert(next_chunk <= last_chunk); - last_chunk = next_chunk; - } -} - FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); @@ -107,6 +96,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) std::vector<CTransaction> mempool_txs; size_t iter{0}; + int32_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(1, 1000000); + + // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow + // Add replacement_vsize since this is added to new diagram during RBF check + int64_t running_vsize_total{replacement_vsize}; + LOCK2(cs_main, pool.cs); LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) @@ -114,19 +109,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) // Make sure txns only have one input, and that a unique input is given to avoid circular references std::optional<CMutableTransaction> parent = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS); if (!parent) { - continue; + return; } assert(iter <= g_outpoints.size()); parent->vin.resize(1); parent->vin[0].prevout = g_outpoints[iter++]; mempool_txs.emplace_back(*parent); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto parent_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += parent_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits<int32_t>::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(parent_entry); if (fuzzed_data_provider.ConsumeBool() && !child->vin.empty()) { child->vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0}; } mempool_txs.emplace_back(*child); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto child_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += child_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits<int32_t>::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(child_entry); if (fuzzed_data_provider.ConsumeBool()) { pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000)); @@ -147,33 +156,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) pool.CalculateDescendants(txiter, all_conflicts); } - // Calculate the feerate diagrams for a replacement. + // Calculate the chunks for a replacement. CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider); - int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(1, 1000000); - auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + auto calc_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; if (calc_results.has_value()) { - // Sanity checks on the diagrams. + // Sanity checks on the chunks. - // Diagrams start at 0. - assert(calc_results->first.front().size == 0); - assert(calc_results->first.front().fee == 0); - assert(calc_results->second.front().size == 0); - assert(calc_results->second.front().fee == 0); - - CheckDiagramConcave(calc_results->first); - CheckDiagramConcave(calc_results->second); + // Feerates are monotonically decreasing. + FeeFrac first_sum; + for (size_t i = 0; i < calc_results->first.size(); ++i) { + first_sum += calc_results->first[i]; + if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i])); + } + FeeFrac second_sum; + for (size_t i = 0; i < calc_results->second.size(); ++i) { + second_sum += calc_results->second[i]; + if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i])); + } - CAmount replaced_fee{0}; - int64_t replaced_size{0}; + FeeFrac replaced; for (auto txiter : all_conflicts) { - replaced_fee += txiter->GetModifiedFee(); - replaced_size += txiter->GetTxSize(); + replaced.fee += txiter->GetModifiedFee(); + replaced.size += txiter->GetTxSize(); } - // The total fee of the new diagram should be the total fee of the old - // diagram - replaced_fee + replacement_fees - assert(calc_results->first.back().fee - replaced_fee + replacement_fees == calc_results->second.back().fee); - assert(calc_results->first.back().size - replaced_size + replacement_vsize == calc_results->second.back().size); + // The total fee & size of the new diagram minus replaced fee & size should be the total + // fee & size of the old diagram minus replacement fee & size. + assert((first_sum - replaced) == (second_sum - FeeFrac{replacement_fees, replacement_vsize})); } // If internals report error, wrapper should too @@ -182,10 +191,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE); } else { // Diagram check succeeded + auto old_sum = std::accumulate(calc_results->first.begin(), calc_results->first.end(), FeeFrac{}); + auto new_sum = std::accumulate(calc_results->second.begin(), calc_results->second.end(), FeeFrac{}); if (!err_tuple.has_value()) { // New diagram's final fee should always match or exceed old diagram's - assert(calc_results->first.back().fee <= calc_results->second.back().fee); - } else if (calc_results->first.back().fee > calc_results->second.back().fee) { + assert(old_sum.fee <= new_sum.fee); + } else if (old_sum.fee > new_sum.fee) { // Or it failed, and if old diagram had higher fees, it should be a failure assert(err_tuple.value().first == DiagramCheckError::FAILURE); } diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index ed33969710..19e45c550a 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -426,21 +426,21 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Replacement of size 1 { - const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; + const auto replace_one{pool.CalculateChunksForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; - BOOST_CHECK(replace_one->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(0, 1)}; - BOOST_CHECK(replace_one->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee, low_size}}; + BOOST_CHECK(replace_one->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{0, 1}}; + BOOST_CHECK(replace_one->second == expected_new_chunks); } // Non-zero replacement fee/size { - const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; + const auto replace_one_fee{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one_fee.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; + std::vector<FeeFrac> expected_old_diagram{{low_fee, low_size}}; BOOST_CHECK(replace_one_fee->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; + std::vector<FeeFrac> expected_new_diagram{{high_fee, low_size}}; BOOST_CHECK(replace_one_fee->second == expected_new_diagram); } @@ -451,22 +451,22 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto high_size = entry_high->GetTxSize(); { - const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; + const auto replace_single_chunk{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; BOOST_CHECK(replace_single_chunk.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_single_chunk->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; - BOOST_CHECK(replace_single_chunk->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_single_chunk->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}}; + BOOST_CHECK(replace_single_chunk->second == expected_new_chunks); } // Conflict with the 2nd tx, resulting in new diagram with three entries { - const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; + const auto replace_cpfp_child{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; BOOST_CHECK(replace_cpfp_child.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_cpfp_child->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size), FeeFrac(low_fee + high_fee, low_size + low_size)}; - BOOST_CHECK(replace_cpfp_child->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_cpfp_child->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}, {low_fee, low_size}}; + BOOST_CHECK(replace_cpfp_child->second == expected_new_chunks); } // third transaction causes the topology check to fail @@ -476,7 +476,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto normal_size = entry_normal->GetTxSize(); { - const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; + const auto replace_too_large{pool.CalculateChunksForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; BOOST_CHECK(!replace_too_large.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex())); } @@ -493,12 +493,12 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto low_size_2 = entry_low_2->GetTxSize(); { - const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; + const auto replace_two_chunks_single_cluster{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; BOOST_CHECK(replace_two_chunks_single_cluster.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(high_fee, high_size_2), FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{high_fee, high_size_2}, {low_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_chunks); } // You can have more than two direct conflicts if the there are multiple affected clusters, all of size 2 or less @@ -515,10 +515,10 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value(); { - const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; + const auto replace_multiple_clusters{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; BOOST_CHECK(replace_multiple_clusters.has_value()); - BOOST_CHECK(replace_multiple_clusters->first.size() == 4); - BOOST_CHECK(replace_multiple_clusters->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters->first.size() == 3); + BOOST_CHECK(replace_multiple_clusters->second.size() == 1); } // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate @@ -527,11 +527,11 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; + const auto replace_multiple_clusters_2{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; BOOST_CHECK(replace_multiple_clusters_2.has_value()); - BOOST_CHECK(replace_multiple_clusters_2->first.size() == 5); - BOOST_CHECK(replace_multiple_clusters_2->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters_2->first.size() == 4); + BOOST_CHECK(replace_multiple_clusters_2->second.size() == 1); } // Add another descendant to conflict_1, making the cluster size > 2 should fail at this point. @@ -540,86 +540,86 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_grand_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; + const auto replace_cluster_size_3{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; BOOST_CHECK(!replace_cluster_size_3.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); } } -BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) +BOOST_AUTO_TEST_CASE(feerate_chunks_utilities) { - // Sanity check the correctness of the feerate diagram comparison. + // Sanity check the correctness of the feerate chunks comparison. // A strictly better case. - std::vector<FeeFrac> old_diagram{{FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}}; - std::vector<FeeFrac> new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}}; + std::vector<FeeFrac> old_chunks{{{950, 300}, {100, 100}}}; + std::vector<FeeFrac> new_chunks{{{1000, 300}, {50, 100}}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Incomparable diagrams - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1000, 300}, {0, 100}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // Strictly better but smaller size. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 300}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // New diagram is strictly better due to the first chunk, even though // second chunk contributes no fees - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 100}, {0, 100}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Feerate of first new chunk is better with, but second chunk is worse - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {249, 250}, {151, 650}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // If we make the second chunk slightly better, the new diagram now wins. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {250, 250}, {150, 150}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Identical diagrams, cannot be strictly better - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{950, 300}, {100, 100}}; - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_eq(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_eq(CompareChunks(new_chunks, old_chunks))); // Same aggregate fee, but different total size (trigger single tail fee check step) - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}}; // No change in evaluation when tail check needed. - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Trigger multiple tail fee check steps - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}}; - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Multiple tail fee check steps, unordered result - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}, {1, 1}}; + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 0af2fdce08..e4142e203c 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1254,6 +1254,30 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) BOOST_CHECK(combined.scriptSig == partial3c); } +/** + * Reproduction of an exception incorrectly raised when parsing a public key inside a TapMiniscript. + */ +BOOST_AUTO_TEST_CASE(sign_invalid_miniscript) +{ + FillableSigningProvider keystore; + SignatureData sig_data; + CMutableTransaction prev, curr; + + // Create a Taproot output which contains a leaf in which a non-32 bytes push is used where a public key is expected + // by the Miniscript parser. This offending Script was found by the RPC fuzzer. + const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")}; + TaprootBuilder builder; + builder.Add(0, {invalid_pubkey}, 0xc0); + XOnlyPubKey nums{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0")}; + builder.Finalize(nums); + prev.vout.emplace_back(0, GetScriptForDestination(builder.GetOutput())); + curr.vin.emplace_back(COutPoint{prev.GetHash(), 0}); + sig_data.tr_spenddata = builder.GetSpendData(); + + // SignSignature can fail but it shouldn't raise an exception (nor crash). + BOOST_CHECK(!SignSignature(keystore, CTransaction(prev), curr, 0, SIGHASH_ALL, sig_data)); +} + BOOST_AUTO_TEST_CASE(script_standard_push) { ScriptError err; diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 8aab2b565c..2de147deea 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -11,7 +11,7 @@ #include <univalue.h> #ifdef ENABLE_EXTERNAL_SIGNER -#include <util/subprocess.hpp> +#include <util/subprocess.h> #endif // ENABLE_EXTERNAL_SIGNER #include <boost/test/unit_test.hpp> diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 82eec6241f..06066e38b2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1280,7 +1280,7 @@ std::optional<std::string> CTxMemPool::CheckConflictTopology(const setEntries& d return std::nullopt; } -util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) +util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) { Assume(replacement_vsize > 0); @@ -1335,7 +1335,6 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(old_chunks.begin(), old_chunks.end(), std::greater()); - std::vector<FeeFrac> old_diagram = BuildDiagramFromChunks(old_chunks); std::vector<FeeFrac> new_chunks; @@ -1365,6 +1364,5 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(new_chunks.begin(), new_chunks.end(), std::greater()); - std::vector<FeeFrac> new_diagram = BuildDiagramFromChunks(new_chunks); - return std::make_pair(old_diagram, new_diagram); + return std::make_pair(old_chunks, new_chunks); } diff --git a/src/txmempool.h b/src/txmempool.h index 9dd4d56da7..52a3dc2d7d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -738,7 +738,7 @@ public: } /** - * Calculate the old and new mempool feerate diagrams relating to the + * Calculate the sorted chunks for the old and new mempool relating to the * clusters that would be affected by a potential replacement transaction. * (replacement_fees, replacement_vsize) values are gathered from a * proposed set of replacement transactions that are considered as a single @@ -752,7 +752,7 @@ public: * @param[in] all_conflicts All transactions that would be removed * @return old and new diagram pair respectively, or an error string if the conflicts don't match a calculable topology */ - util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); /* Check that all direct conflicts are in a cluster size of two or less. Each * direct conflict may be in a separate cluster. diff --git a/src/util/feefrac.cpp b/src/util/feefrac.cpp index 68fb836936..5b6173835c 100644 --- a/src/util/feefrac.cpp +++ b/src/util/feefrac.cpp @@ -7,39 +7,26 @@ #include <array> #include <vector> -std::vector<FeeFrac> BuildDiagramFromChunks(const Span<const FeeFrac> chunks) -{ - std::vector<FeeFrac> diagram; - diagram.reserve(chunks.size() + 1); - - diagram.emplace_back(0, 0); - for (auto& chunk : chunks) { - diagram.emplace_back(diagram.back() + chunk); - } - return diagram; -} - -std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const FeeFrac> dia1) +std::partial_ordering CompareChunks(Span<const FeeFrac> chunks0, Span<const FeeFrac> chunks1) { /** Array to allow indexed access to input diagrams. */ - const std::array<Span<const FeeFrac>, 2> dias = {dia0, dia1}; + const std::array<Span<const FeeFrac>, 2> chunk = {chunks0, chunks1}; /** How many elements we have processed in each input. */ - size_t next_index[2] = {1, 1}; + size_t next_index[2] = {0, 0}; + /** Accumulated fee/sizes in diagrams, up to next_index[i] - 1. */ + FeeFrac accum[2]; /** Whether the corresponding input is strictly better than the other at least in one place. */ bool better_somewhere[2] = {false, false}; /** Get the first unprocessed point in diagram number dia. */ - const auto next_point = [&](int dia) { return dias[dia][next_index[dia]]; }; + const auto next_point = [&](int dia) { return chunk[dia][next_index[dia]] + accum[dia]; }; /** Get the last processed point in diagram number dia. */ - const auto prev_point = [&](int dia) { return dias[dia][next_index[dia] - 1]; }; - - // Diagrams should be non-empty, and first elements zero in size and fee - Assert(!dia0.empty() && !dia1.empty()); - Assert(prev_point(0).IsEmpty()); - Assert(prev_point(1).IsEmpty()); + const auto prev_point = [&](int dia) { return accum[dia]; }; + /** Move to the next point in diagram number dia. */ + const auto advance = [&](int dia) { accum[dia] += chunk[dia][next_index[dia]++]; }; do { - bool done_0 = next_index[0] == dias[0].size(); - bool done_1 = next_index[1] == dias[1].size(); + bool done_0 = next_index[0] == chunk[0].size(); + bool done_1 = next_index[1] == chunk[1].size(); if (done_0 && done_1) break; // Determine which diagram has the first unprocessed point. If a single side is finished, use the @@ -69,17 +56,16 @@ std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const // If B and P have the same size, B can be marked as processed (in addition to P, see // below), as we've already performed a comparison at this size. - if (point_b.size == point_p.size) ++next_index[!unproc_side]; + if (point_b.size == point_p.size) advance(!unproc_side); } // If P lies above AB, unproc_side is better in P. If P lies below AB, then !unproc_side is // better in P. if (std::is_gt(cmp)) better_somewhere[unproc_side] = true; if (std::is_lt(cmp)) better_somewhere[!unproc_side] = true; - ++next_index[unproc_side]; + advance(unproc_side); // If both diagrams are better somewhere, they are incomparable. if (better_somewhere[0] && better_somewhere[1]) return std::partial_ordering::unordered; - } while(true); // Otherwise compare the better_somewhere values. diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 7102f85f88..9772162010 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -146,15 +146,14 @@ struct FeeFrac } }; -/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */ -std::vector<FeeFrac> BuildDiagramFromChunks(Span<const FeeFrac> chunks); - -/** Compares two feerate diagrams. The shorter one is implicitly - * extended with a horizontal straight line. +/** Compare the feerate diagrams implied by the provided sorted chunks data. + * + * The implied diagram for each starts at (0, 0), then contains for each chunk the cumulative fee + * and size up to that chunk, and then extends infinitely to the right with a horizontal line. * - * A feerate diagram consists of a list of (fee, size) points with the property that size - * is strictly increasing and that the first entry is (0, 0). + * The caller must guarantee that the sum of the FeeFracs in either of the chunks' data set do not + * overflow (so sum fees < 2^63, and sum sizes < 2^31). */ -std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const FeeFrac> dia1); +std::partial_ordering CompareChunks(Span<const FeeFrac> chunks0, Span<const FeeFrac> chunks1); #endif // BITCOIN_UTIL_FEEFRAC_H diff --git a/src/util/subprocess.hpp b/src/util/subprocess.h index e660aa143d..4acfa8ff83 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.h @@ -33,8 +33,10 @@ Documentation for C++ subprocessing library. @version 1.0.0 */ -#ifndef SUBPROCESS_HPP -#define SUBPROCESS_HPP +#ifndef BITCOIN_UTIL_SUBPROCESS_H +#define BITCOIN_UTIL_SUBPROCESS_H + +#include <util/syserror.h> #include <algorithm> #include <cassert> @@ -150,24 +152,11 @@ class OSError: public std::runtime_error { public: OSError(const std::string& err_msg, int err_code): - std::runtime_error( err_msg + ": " + std::strerror(err_code) ) + std::runtime_error(err_msg + ": " + SysErrorString(err_code)) {} }; //-------------------------------------------------------------------- - -//Environment Variable types -#ifndef _MSC_VER - using env_string_t = std::string; - using env_char_t = char; -#else - using env_string_t = std::wstring; - using env_char_t = wchar_t; -#endif -using env_map_t = std::map<env_string_t, env_string_t>; -using env_vector_t = std::vector<env_char_t>; - -//-------------------------------------------------------------------- namespace util { template <typename R> @@ -305,100 +294,6 @@ namespace util if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0)) throw OSError("SetHandleInformation", 0); } - - // env_map_t MapFromWindowsEnvironment() - // * Imports current Environment in a C-string table - // * Parses the strings by splitting on the first "=" per line - // * Creates a map of the variables - // * Returns the map - inline env_map_t MapFromWindowsEnvironment(){ - wchar_t *variable_strings_ptr; - wchar_t *environment_strings_ptr; - std::wstring delimiter(L"="); - int del_len = delimiter.length(); - env_map_t mapped_environment; - - // Get a pointer to the environment block. - environment_strings_ptr = GetEnvironmentStringsW(); - // If the returned pointer is NULL, exit. - if (environment_strings_ptr == NULL) - { - throw OSError("GetEnvironmentStringsW", 0); - } - - // Variable strings are separated by NULL byte, and the block is - // terminated by a NULL byte. - - variable_strings_ptr = (wchar_t *) environment_strings_ptr; - - //Since the environment map ends with a null, we can loop until we find it. - while (*variable_strings_ptr) - { - // Create a string from Variable String - env_string_t current_line(variable_strings_ptr); - // Find the first "equals" sign. - auto pos = current_line.find(delimiter); - // Assuming it's not missing ... - if(pos!=std::wstring::npos){ - // ... parse the key and value. - env_string_t key = current_line.substr(0, pos); - env_string_t value = current_line.substr(pos + del_len); - // Map the entry. - mapped_environment[key] = value; - } - // Jump to next line in the environment map. - variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1; - } - // We're done with the old environment map buffer. - FreeEnvironmentStringsW(environment_strings_ptr); - - // Return the map. - return mapped_environment; - } - - // env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - // * Creates a vector buffer for the new environment string table - // * Copies in the mapped variables - // * Returns the vector - inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - { - // Make a new environment map buffer. - env_vector_t environment_map_buffer; - // Give it some space. - environment_map_buffer.reserve(4096); - - // And fill'er up. - for(auto kv: source_map){ - // Create the line - env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; - // Add the line to the buffer. - std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); - // Append a null - environment_map_buffer.push_back(0); - } - // Append one last null because of how Windows does it's environment maps. - environment_map_buffer.push_back(0); - - return environment_map_buffer; - } - - // env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map) - // * Merges host environment with new mapped variables - // * Creates and returns string vector based on map - inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){ - // Import the environment map - env_map_t environment_map = MapFromWindowsEnvironment(); - // Merge in the changes with overwrite - for(auto& it: changes_map) - { - environment_map[it.first] = it.second; - } - // Create a Windows-usable Environment Map Buffer - env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map); - - return environment_map_strings_vector; - } - #endif /*! @@ -431,26 +326,6 @@ namespace util } - /*! - * Function: join - * Parameters: - * [in] vec : Vector of strings which needs to be joined to form - * a single string with words separated by a separator char. - * [in] sep : String used to separate 2 words in the joined string. - * Default constructed to ' ' (space). - * [out] string: Joined string. - */ - static inline - std::string join(const std::vector<std::string>& vec, - const std::string& sep = " ") - { - std::string res; - for (auto& elem : vec) res.append(elem + sep); - res.erase(--res.end()); - return res; - } - - #ifndef __USING_WINDOWS__ /*! * Function: set_clo_on_exec @@ -652,56 +527,6 @@ namespace util */ /*! - * The buffer size of the stdin/stdout/stderr - * streams of the child process. - * Default value is 0. - */ -struct bufsize { - explicit bufsize(int sz): bufsiz(sz) {} - int bufsiz = 0; -}; - -/*! - * Option to defer spawning of the child process - * till `Popen::start_process` API is called. - * Default value is false. - */ -struct defer_spawn { - explicit defer_spawn(bool d): defer(d) {} - bool defer = false; -}; - -/*! - * Option to close all file descriptors - * when the child process is spawned. - * The close fd list does not include - * input/output/error if they are explicitly - * set as part of the Popen arguments. - * - * Default value is false. - */ -struct close_fds { - explicit close_fds(bool c): close_all(c) {} - bool close_all = false; -}; - -/*! - * Option to make the child process as the - * session leader and thus the process - * group leader. - * Default value is false. - */ -struct session_leader { - explicit session_leader(bool sl): leader_(sl) {} - bool leader_ = false; -}; - -struct shell { - explicit shell(bool s): shell_(s) {} - bool shell_ = false; -}; - -/*! * Base class for all arguments involving string value. */ struct string_arg @@ -727,34 +552,6 @@ struct executable: string_arg }; /*! - * Option to set the current working directory - * of the spawned process. - * - * Eg: cwd{"/some/path/x"} - */ -struct cwd: string_arg -{ - template <typename T> - cwd(T&& arg): string_arg(std::forward<T>(arg)) {} -}; - -/*! - * Option to specify environment variables required by - * the spawned process. - * - * Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }} - */ -struct environment -{ - environment(env_map_t&& env): - env_(std::move(env)) {} - explicit environment(const env_map_t& env): - env_(env) {} - env_map_t env_; -}; - - -/*! * Used for redirecting input/output/error */ enum IOTYPE { @@ -870,44 +667,6 @@ struct error int wr_ch_ = -1; }; -// Impoverished, meager, needy, truly needy -// version of type erasure to store function pointers -// needed to provide the functionality of preexec_func -// ATTN: Can be used only to execute functions with no -// arguments and returning void. -// Could have used more efficient methods, ofcourse, but -// that won't yield me the consistent syntax which I am -// aiming for. If you know, then please do let me know. - -class preexec_func -{ -public: - preexec_func() {} - - template <typename Func> - explicit preexec_func(Func f): holder_(new FuncHolder<Func>(std::move(f))) - {} - - void operator()() { - (*holder_)(); - } - -private: - struct HolderBase { - virtual void operator()() const = 0; - virtual ~HolderBase(){}; - }; - template <typename T> - struct FuncHolder: HolderBase { - FuncHolder(T func): func_(std::move(func)) {} - void operator()() const override { func_(); } - // The function pointer/reference - T func_; - }; - - std::unique_ptr<HolderBase> holder_ = nullptr; -}; - // ~~~~ End Popen Args ~~~~ @@ -1007,17 +766,9 @@ struct ArgumentDeducer ArgumentDeducer(Popen* p): popen_(p) {} void set_option(executable&& exe); - void set_option(cwd&& cwdir); - void set_option(bufsize&& bsiz); - void set_option(environment&& env); - void set_option(defer_spawn&& defer); - void set_option(shell&& sh); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); - void set_option(close_fds&& cfds); - void set_option(preexec_func&& prefunc); - void set_option(session_leader&& sleader); private: Popen* popen_ = nullptr; @@ -1168,9 +919,6 @@ public:// Yes they are public HANDLE g_hChildStd_ERR_Wr = nullptr; #endif - // Buffer size for the IO streams - int bufsiz_ = 0; - // Pipes for communicating with child // Emulates stdin @@ -1200,9 +948,9 @@ private: * interface to the client. * * API's provided by the class: - * 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....) + * 1. Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....) + * 2. Popen("cmd arg1"m output{..}, error{..}, ....) * Command provided in a single string. * 3. wait() - Wait for the child to exit. * 4. retcode() - The return code of the exited child. @@ -1218,8 +966,6 @@ private: in case of redirection. See piping examples. *12. error() - Get the error channel/File pointer. Usually used in case of redirection. - *13. start_process() - Start the child process. Only to be used when - * `defer_spawn` option was provided in Popen constructor. */ class Popen { @@ -1237,7 +983,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template <typename... Args> @@ -1249,7 +995,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template <typename... Args> @@ -1260,7 +1006,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } /* @@ -1272,8 +1018,6 @@ public: } */ - void start_process() noexcept(false); - int pid() const noexcept { return child_pid_; } int retcode() const noexcept { return retcode_; } @@ -1347,16 +1091,7 @@ private: std::future<void> cleanup_future_; #endif - bool defer_process_start_ = false; - bool close_fds_ = false; - bool has_preexec_fn_ = false; - bool shell_ = false; - bool session_leader_ = false; - std::string exe_name_; - std::string cwd_; - env_map_t env_; - preexec_func preexec_fn_; // Command in string format std::string args_; @@ -1391,20 +1126,6 @@ inline void Popen::populate_c_argv() cargv_.push_back(nullptr); } -inline void Popen::start_process() noexcept(false) -{ - // The process was started/tried to be started - // in the constructor itself. - // For explicitly calling this API to start the - // process, 'defer_spawn' argument must be set to - // true in the constructor. - if (!defer_process_start_) { - assert (0); - return; - } - execute_process(); -} - inline int Popen::wait() noexcept(false) { #ifdef __USING_WINDOWS__ @@ -1483,8 +1204,7 @@ inline void Popen::kill(int sig_num) throw OSError("TerminateProcess", 0); } #else - if (session_leader_) killpg(child_pid_, sig_num); - else ::kill(child_pid_, sig_num); + ::kill(child_pid_, sig_num); #endif } @@ -1492,17 +1212,6 @@ inline void Popen::kill(int sig_num) inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ - if (this->shell_) { - throw OSError("shell not currently supported on windows", 0); - } - - void* environment_string_table_ptr = nullptr; - env_vector_t environment_string_vector; - if(this->env_.size()){ - environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_); - environment_string_table_ptr = (void*)environment_string_vector.data(); - } - if (exe_name_.length()) { this->vargs_.insert(this->vargs_.begin(), this->exe_name_); this->populate_c_argv(); @@ -1549,7 +1258,7 @@ inline void Popen::execute_process() noexcept(false) NULL, // primary thread security attributes TRUE, // handles are inherited creation_flags, // creation flags - environment_string_table_ptr, // use provided environment + NULL, // use parent's environment NULL, // use parent's current directory &siStartInfo, // STARTUPINFOW pointer &piProcInfo); // receives PROCESS_INFORMATION @@ -1588,14 +1297,6 @@ inline void Popen::execute_process() noexcept(false) int err_rd_pipe, err_wr_pipe; std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); - if (shell_) { - auto new_cmd = util::join(vargs_); - vargs_.clear(); - vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"}); - vargs_.push_back(new_cmd); - populate_c_argv(); - } - if (exe_name_.length()) { vargs_.insert(vargs_.begin(), exe_name_); populate_c_argv(); @@ -1662,30 +1363,6 @@ namespace detail { popen_->exe_name_ = std::move(exe.arg_value); } - inline void ArgumentDeducer::set_option(cwd&& cwdir) { - popen_->cwd_ = std::move(cwdir.arg_value); - } - - inline void ArgumentDeducer::set_option(bufsize&& bsiz) { - popen_->stream_.bufsiz_ = bsiz.bufsiz; - } - - inline void ArgumentDeducer::set_option(environment&& env) { - popen_->env_ = std::move(env.env_); - } - - inline void ArgumentDeducer::set_option(defer_spawn&& defer) { - popen_->defer_process_start_ = defer.defer; - } - - inline void ArgumentDeducer::set_option(shell&& sh) { - popen_->shell_ = sh.shell_; - } - - inline void ArgumentDeducer::set_option(session_leader&& sleader) { - popen_->session_leader_ = sleader.leader_; - } - inline void ArgumentDeducer::set_option(input&& inp) { if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; @@ -1708,15 +1385,6 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } - inline void ArgumentDeducer::set_option(close_fds&& cfds) { - popen_->close_fds_ = cfds.close_all; - } - - inline void ArgumentDeducer::set_option(preexec_func&& prefunc) { - popen_->preexec_fn_ = std::move(prefunc); - popen_->has_preexec_fn_ = true; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1763,41 +1431,8 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); - // Close all the inherited fd's except the error write pipe - if (parent_->close_fds_) { - int max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) throw OSError("sysconf failed", errno); - - for (int i = 3; i < max_fd; i++) { - if (i == err_wr_pipe_) continue; - close(i); - } - } - - // Change the working directory if provided - if (parent_->cwd_.length()) { - sys_ret = chdir(parent_->cwd_.c_str()); - if (sys_ret == -1) throw OSError("chdir failed", errno); - } - - if (parent_->has_preexec_fn_) { - parent_->preexec_fn_(); - } - - if (parent_->session_leader_) { - sys_ret = setsid(); - if (sys_ret == -1) throw OSError("setsid failed", errno); - } - // Replace the current image with the executable - if (parent_->env_.size()) { - for (auto& kv : parent_->env_) { - setenv(kv.first.c_str(), kv.second.c_str(), 1); - } - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } else { - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); if (sys_ret == -1) throw OSError("execve failed", errno); @@ -1840,16 +1475,7 @@ namespace detail { for (auto& h : handles) { if (h == nullptr) continue; - switch (bufsiz_) { - case 0: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - case 1: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - default: - setvbuf(h, nullptr, _IOFBF, bufsiz_); - }; + setvbuf(h, nullptr, _IONBF, BUFSIZ); } #endif } @@ -1985,4 +1611,4 @@ namespace detail { } -#endif // SUBPROCESS_HPP +#endif // BITCOIN_UTIL_SUBPROCESS_H diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index a71f8f9fbc..b5703fa54a 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -9,9 +9,11 @@ #include <wallet/external_signer_scriptpubkeyman.h> #include <iostream> +#include <key_io.h> #include <memory> #include <stdexcept> #include <string> +#include <univalue.h> #include <utility> #include <vector> @@ -51,15 +53,26 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { return signers[0]; } -bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const +util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const { // TODO: avoid the need to infer a descriptor from inside a descriptor wallet + const CScript& scriptPubKey = GetScriptForDestination(dest); auto provider = GetSolvingProvider(scriptPubKey); auto descriptor = InferDescriptor(scriptPubKey, *provider); - signer.DisplayAddress(descriptor->ToString()); - // TODO inspect result - return true; + const UniValue& result = signer.DisplayAddress(descriptor->ToString()); + + const UniValue& error = result.find_value("error"); + if (error.isStr()) return util::Error{strprintf(_("Signer returned error: %s"), error.getValStr())}; + + const UniValue& ret_address = result.find_value("address"); + if (!ret_address.isStr()) return util::Error{_("Signer did not echo address")}; + + if (ret_address.getValStr() != EncodeDestination(dest)) { + return util::Error{strprintf(_("Signer echoed unexpected address %s"), ret_address.getValStr())}; + } + + return util::Result<void>(); } // If sign is true, transaction must previously have been filled diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index c052ce6129..44286456b6 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -9,6 +9,8 @@ #include <memory> +struct bilingual_str; + namespace wallet { class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan { @@ -27,7 +29,11 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan static ExternalSigner GetExternalSigner(); - bool DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const; + /** + * Display address on the device and verify that the returned value matches. + * @returns nothing or an error message + */ + util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index a866666f64..0c1cae7253 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -247,7 +247,7 @@ public: return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id) : m_wallet->SetAddressReceiveRequest(batch, dest, id, value); } - bool displayAddress(const CTxDestination& dest) override + util::Result<void> displayAddress(const CTxDestination& dest) override { LOCK(m_wallet->cs_wallet); return m_wallet->DisplayAddress(dest); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7f068c05ef..bed9ec029a 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -789,9 +789,8 @@ RPCHelpMan walletdisplayaddress() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); } - if (!pwallet->DisplayAddress(dest)) { - throw JSONRPCError(RPC_MISC_ERROR, "Failed to display address"); - } + util::Result<void> res = pwallet->DisplayAddress(dest); + if (!res) throw JSONRPCError(RPC_MISC_ERROR, util::ErrorString(res).original); UniValue result(UniValue::VOBJ); result.pushKV("address", request.params[0].get_str()); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4575881d96..2c1ab8d44a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -27,7 +27,6 @@ #include <unordered_map> enum class OutputType; -struct bilingual_str; namespace wallet { struct MigrationData; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 96c4397504..8f4171eb15 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2667,7 +2667,7 @@ void ReserveDestination::ReturnDestination() address = CNoDestination(); } -bool CWallet::DisplayAddress(const CTxDestination& dest) +util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest) { CScript scriptPubKey = GetScriptForDestination(dest); for (const auto& spk_man : GetScriptPubKeyMans(scriptPubKey)) { @@ -2676,9 +2676,9 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) continue; } ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); - return signer_spk_man->DisplayAddress(scriptPubKey, signer); + return signer_spk_man->DisplayAddress(dest, signer); } - return false; + return util::Error{_("There is no ScriptPubKeyManager for this address")}; } bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b49b5a7d0d..6a998fa398 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,8 +537,8 @@ public: bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Display address on an external signer. Returns false if external signer support is not compiled */ - bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Display address on an external signer. */ + util::Result<void> DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index d10db046f5..536e471053 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -8,6 +8,7 @@ #include <kernel/chain.h> #include <kernel/mempool_entry.h> #include <logging.h> +#include <netbase.h> #include <primitives/block.h> #include <primitives/transaction.h> #include <validationinterface.h> @@ -57,7 +58,12 @@ std::unique_ptr<CZMQNotificationInterface> CZMQNotificationInterface::Create(std { std::string arg("-zmq" + entry.first); const auto& factory = entry.second; - for (const std::string& address : gArgs.GetArgs(arg)) { + for (std::string& address : gArgs.GetArgs(arg)) { + // libzmq uses prefix "ipc://" for UNIX domain sockets + if (address.substr(0, ADDR_PREFIX_UNIX.length()) == ADDR_PREFIX_UNIX) { + address.replace(0, ADDR_PREFIX_UNIX.length(), ADDR_PREFIX_IPC); + } + std::unique_ptr<CZMQAbstractNotifier> notifier = factory(); notifier->SetType(entry.first); notifier->SetAddress(address); diff --git a/src/zmq/zmqutil.h b/src/zmq/zmqutil.h index 334b51aa91..bec48c0a56 100644 --- a/src/zmq/zmqutil.h +++ b/src/zmq/zmqutil.h @@ -9,4 +9,7 @@ void zmqError(const std::string& str); +/** Prefix for unix domain socket addresses (which are local filesystem paths) */ +const std::string ADDR_PREFIX_IPC = "ipc://"; // used by libzmq, example "ipc:///root/path/to/file" + #endif // BITCOIN_ZMQ_ZMQUTIL_H |