diff options
25 files changed, 258 insertions, 105 deletions
diff --git a/contrib/depends/packages/openssl.mk b/contrib/depends/packages/openssl.mk index 77d4b2d11..926b3b180 100644 --- a/contrib/depends/packages/openssl.mk +++ b/contrib/depends/packages/openssl.mk @@ -1,8 +1,8 @@ package=openssl -$(package)_version=3.0.19 -$(package)_download_path=https://www.openssl.org/source +$(package)_version=3.0.21 +$(package)_download_path=https://github.com/openssl/openssl/releases/download/openssl-$($(package)_version) $(package)_file_name=$(package)-$($(package)_version).tar.gz -$(package)_sha256_hash=fa5a4143b8aae18be53ef2f3caf29a2e0747430b8bc74d32d88335b94ab63072 +$(package)_sha256_hash=617e29af8e421f46649484a4937e48c685e47f46488167c982f88bc4ec1d522f define $(package)_set_vars $(package)_config_env=AR="$($(package)_ar)" ARFLAGS=$($(package)_arflags) RANLIB="$($(package)_ranlib)" CC="$($(package)_cc)" diff --git a/contrib/epee/include/net/abstract_tcp_server2.h b/contrib/epee/include/net/abstract_tcp_server2.h index fe201f155..75a26cbbb 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.h +++ b/contrib/epee/include/net/abstract_tcp_server2.h @@ -320,7 +320,7 @@ namespace net_utils bool speed_limit_is_enabled() const; ///< tells us should we be sleeping here (e.g. do not sleep on RPC connections) - bool cancel(); + bool cancel(bool wait_for_shutdown = false); private: //----------------- i_service_endpoint --------------------- @@ -378,8 +378,21 @@ namespace net_utils /// wait for service workers stop bool timed_wait_server_stop(uint64_t wait_mseconds); + /// Mark the server as stopping without closing connections or stopping the io_context. + bool mark_stop_signal_sent(); + + /// Close boosted_tcp_server-owned connections, including ones not yet registered with the protocol handler. + void close_server_connections(); + + /// Stop the server io_context. + void stop_io_context(); + /// Stop the server. - void send_stop_signal(std::function<void()> close_all_connections = [](){}); + /// + /// Warning: Do NOT call this if the io_context is shared for connections + /// managed outside the boosted_tcp_server. See p2p net_node shutdown for + /// the correct staged shutdown in that case. + void send_stop_signal(); bool is_stop_signal_sent() const noexcept { return m_stop_signal_sent; }; diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index 56d3d7de2..f8651fae9 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -1119,9 +1119,9 @@ namespace net_utils } template<typename T> - bool connection<T>::cancel() + bool connection<T>::cancel(const bool wait_for_shutdown) { - return close(false); + return close(wait_for_shutdown); } template<typename T> @@ -1140,7 +1140,9 @@ namespace net_utils bool connection<T>::close(const bool wait_for_shutdown) { std::lock_guard<std::mutex> guard(m_state.lock); - if (m_state.status != status_t::RUNNING) + if (m_state.status == status_t::TERMINATED || m_state.status == status_t::WASTED) + return true; + if (!wait_for_shutdown && m_state.status != status_t::RUNNING) return false; terminate_async(); @@ -1286,7 +1288,7 @@ namespace net_utils template<class t_protocol_handler> boosted_tcp_server<t_protocol_handler>::~boosted_tcp_server() { - this->send_stop_signal(); + send_stop_signal(); timed_wait_server_stop(10000); } //--------------------------------------------------------------------------------- @@ -1577,26 +1579,55 @@ namespace net_utils } //--------------------------------------------------------------------------------- template<class t_protocol_handler> - void boosted_tcp_server<t_protocol_handler>::send_stop_signal(std::function<void()> close_all_connections) + bool boosted_tcp_server<t_protocol_handler>::mark_stop_signal_sent() { - m_stop_signal_sent = true; + if (m_stop_signal_sent.exchange(true)) + { + MDEBUG("Stop signal already sent"); + return false; + } typename connection<t_protocol_handler>::shared_state *state = static_cast<typename connection<t_protocol_handler>::shared_state*>(m_state.get()); state->stop_signal_sent = true; - TRY_ENTRY(); - connections_mutex.lock(); - for (auto &c: connections_) + return true; + } + //--------------------------------------------------------------------------------- + template<class t_protocol_handler> + void boosted_tcp_server<t_protocol_handler>::close_server_connections() + { + decltype(connections_) connections; { - c->cancel(); + boost::unique_lock<boost::mutex> lock(connections_mutex); + connections.swap(connections_); } - connections_.clear(); - connections_mutex.unlock(); - // Since we shut down connections in the strand, we want to make sure to complete the shutdown sequence before - // stopping the io_context. We let the caller handle closing because the caller is the one keeping track of all - // connections (connections_ is only a subset of all connections). - close_all_connections(); + for (auto &c: connections) + { + c->cancel(true/*wait_for_shutdown*/); + } + } + //--------------------------------------------------------------------------------- + template<class t_protocol_handler> + void boosted_tcp_server<t_protocol_handler>::stop_io_context() + { + { + boost::unique_lock<boost::mutex> lock(connections_mutex); + if (!connections_.empty()) + { + MERROR("Stopping io_context with " << connections_.size() << " server-owned connections still open"); + } + } + MDEBUG("Stopping io_context"); io_context_.stop(); - MDEBUG("Done with send_stop_signal"); + } + //--------------------------------------------------------------------------------- + template<class t_protocol_handler> + void boosted_tcp_server<t_protocol_handler>::send_stop_signal() + { + TRY_ENTRY(); + if (!mark_stop_signal_sent()) + return; + close_server_connections(); + stop_io_context(); CATCH_ENTRY_L0("boosted_tcp_server<t_protocol_handler>::send_stop_signal()", void()); } //--------------------------------------------------------------------------------- diff --git a/contrib/epee/src/parserse_base_utils.cpp b/contrib/epee/src/parserse_base_utils.cpp index e154a75f8..923dde424 100644 --- a/contrib/epee/src/parserse_base_utils.cpp +++ b/contrib/epee/src/parserse_base_utils.cpp @@ -129,7 +129,7 @@ namespace misc_utils case '/': //Slash character val.push_back('/');break; case 'u': //Unicode code point - if (buf_end - it < 4) + if (buf_end - it < 5) { ASSERT_MES_AND_THROW("Invalid Unicode escape sequence"); } diff --git a/contrib/gitian/dockrun.sh b/contrib/gitian/dockrun.sh index 396db126b..63aa6eef6 100755 --- a/contrib/gitian/dockrun.sh +++ b/contrib/gitian/dockrun.sh @@ -9,12 +9,18 @@ VERSION=$1 DOCKER=`command -v docker` CACHER=`command -v apt-cacher-ng` -if [ -z "$DOCKER" -o -z "$CACHER" ]; then - echo "$0: you must first install docker.io and apt-cacher-ng" - echo " e.g. sudo apt-get install docker.io apt-cacher-ng" +if [ -z "$DOCKER" ]; then + echo "$0: you must first install docker.io" + echo " e.g. sudo apt-get install docker.io" exit 1 fi +# only use APT cacher if package is present +DOCKER_CACHE_LINE="" +if [ "$CACHER" ]; then + DOCKER_CACHE_LINE="RUN echo 'Acquire::http { Proxy \"http://172.17.0.1:3142\"; };' > /etc/apt/apt.conf.d/50cacher" +fi + GH_USER=${GH_USER-$USER} TAG=gitrun-bionic @@ -33,7 +39,7 @@ cat <<EOF > ${TAG}.Dockerfile FROM ubuntu:bionic ENV DEBIAN_FRONTEND=noninteractive -RUN echo 'Acquire::http { Proxy "http://172.17.0.1:3142"; };' > /etc/apt/apt.conf.d/50cacher +$DOCKER_CACHE_LINE RUN echo "$GID" >> /etc/group RUN apt-get update && apt-get --no-install-recommends -y install lsb-release ruby git make wget docker.io python3 curl @@ -66,7 +72,7 @@ cat <<EOF > ${TAG2}.Dockerfile FROM ubuntu:bionic ENV DEBIAN_FRONTEND=noninteractive -RUN echo 'Acquire::http { Proxy "http://172.17.0.1:3142"; };' > /etc/apt/apt.conf.d/50cacher +$DOCKER_CACHE_LINE RUN apt-get update && apt-get --no-install-recommends -y install build-essential git language-pack-en \ wget lsb-release curl gcc-7 g++-7 gcc g++ binutils-gold pkg-config autoconf libtool automake faketime \ bsdmainutils ca-certificates python cmake gperf diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 97fb8b8e3..f20bd1e26 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2214,9 +2214,7 @@ bool Blockchain::handle_get_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, NO //pack block e.block = std::move(bl.first); - e.block_weight = 0; - if (arg.prune && m_db->block_exists(arg.blocks[i])) - e.block_weight = m_db->get_block_weight(m_db->get_block_height(arg.blocks[i])); + e.block_weight = arg.prune ? m_db->get_block_weight(get_block_height(bl.second)) : 0; } return true; diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index f50c9ad3f..4d7a454b2 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1548,9 +1548,9 @@ namespace cryptonote return m_mempool.get_transactions_and_spent_keys_info(tx_infos, key_image_infos, include_sensitive_data); } //----------------------------------------------------------------------------------------------- - bool core::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const + bool core::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const { - return m_mempool.get_pool_for_rpc(tx_infos, key_image_infos); + return m_mempool.get_pool_for_rpc(tx_infos, key_image_infos, include_sensitive); } //----------------------------------------------------------------------------------------------- bool core::get_short_chain_history(std::list<crypto::hash>& ids, uint64_t& current_height) const diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 777de3319..19d9c6e65 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -556,10 +556,11 @@ namespace cryptonote /** * @copydoc tx_memory_pool::get_pool_for_rpc + * @param include_sensitive include node-private fields (timing) * * @note see tx_memory_pool::get_pool_for_rpc */ - bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const; + bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const; /** * @copydoc tx_memory_pool::get_transactions_count diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp index c350e24c4..7aea5cf50 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.cpp +++ b/src/cryptonote_core/cryptonote_tx_utils.cpp @@ -453,11 +453,12 @@ namespace cryptonote crypto::public_key out_eph_public_key; crypto::view_tag view_tag; - hwdev.generate_output_ephemeral_keys(tx.version,sender_account_keys, txkey_pub, tx_key, + const bool r = hwdev.generate_output_ephemeral_keys(tx.version,sender_account_keys, txkey_pub, tx_key, dst_entr, change_addr, output_index, need_additional_txkeys, additional_tx_keys, additional_tx_public_keys, amount_keys, out_eph_public_key, use_view_tags, view_tag); + CHECK_AND_ASSERT_MES(r, false, "Failed to generate output ephemeral keys"); tx_out out; cryptonote::set_tx_out(dst_entr.amount, out_eph_public_key, use_view_tags, view_tag, out); diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index badcde361..8af654185 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -416,7 +416,7 @@ namespace cryptonote break; try { - const crypto::hash &txid = it->second; + const crypto::hash txid = it->second; txpool_tx_meta_t meta; if (!m_blockchain.get_txpool_tx_meta(txid, meta)) { @@ -1241,13 +1241,13 @@ namespace cryptonote return true; } //--------------------------------------------------------------------------------- - bool tx_memory_pool::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const + bool tx_memory_pool::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const { CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); tx_infos.reserve(m_blockchain.get_txpool_tx_count()); key_image_infos.reserve(m_blockchain.get_txpool_tx_count()); - m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){ + m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos, include_sensitive](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){ cryptonote::rpc::tx_in_pool txi; txi.tx_hash = txid; if (!(meta.pruned ? parse_and_validate_tx_base_from_blob(*bd, txi.tx) : parse_and_validate_tx_from_blob(*bd, txi.tx))) @@ -1265,9 +1265,11 @@ namespace cryptonote txi.max_used_block_hash = meta.max_used_block_id; txi.last_failed_block_height = meta.last_failed_height; txi.last_failed_block_hash = meta.last_failed_id; - txi.receive_time = meta.receive_time; + // In restricted mode we do not include this data: + txi.receive_time = include_sensitive ? meta.receive_time : 0; txi.relayed = meta.relayed; - txi.last_relayed_time = meta.dandelionpp_stem ? 0 : meta.last_relayed_time; + // In restricted mode we do not include this data: + txi.last_relayed_time = (include_sensitive && !meta.dandelionpp_stem) ? meta.last_relayed_time : 0; txi.do_not_relay = meta.do_not_relay; txi.double_spend_seen = meta.double_spend_seen; tx_infos.push_back(txi); diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index f3630368b..f84983f05 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -323,10 +323,11 @@ namespace cryptonote * * @param tx_infos [out] the transactions' information * @param key_image_infos [out] the spent key images' information + * @param include_sensitive include fields that are sensitive to node privacy * * @return true */ - bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const; + bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const; /** * @brief check for presence of key images in the pool diff --git a/src/daemonizer/posix_fork.cpp b/src/daemonizer/posix_fork.cpp index 16758215d..39e9f7b5f 100644 --- a/src/daemonizer/posix_fork.cpp +++ b/src/daemonizer/posix_fork.cpp @@ -5,13 +5,19 @@ // #include "daemonizer/posix_fork.h" +#include "misc_language.h" #include "misc_log_ex.h" +#include <cerrno> #include <cstdlib> +#include <cstring> #include <fcntl.h> +#include <fstream> #include <unistd.h> #include <stdexcept> #include <string> +#include <sys/stat.h> +#include <sys/types.h> #ifndef TMPDIR #define TMPDIR "/tmp" @@ -35,26 +41,64 @@ void fork(const std::string & pidfile) // processes. // Only in the final child process do we write the PID to the // file (and close it). - std::ofstream pidofs; + int pid_fd = -1; + auto close_pid_fd = [&pid_fd]() + { + if (pid_fd >= 0) + { + close(pid_fd); + pid_fd = -1; + } + }; + epee::misc_utils::auto_scope_leave_caller pid_fd_guard = + epee::misc_utils::create_scope_leave_handler(close_pid_fd); if (! pidfile.empty ()) { - int oldpid; - std::ifstream pidrifs; - pidrifs.open(pidfile, std::fstream::in); - if (! pidrifs.fail()) + struct stat st; + if (lstat(pidfile.c_str(), &st) == 0) { - // Read the PID and send signal 0 to see if the process exists. - if (pidrifs >> oldpid && oldpid > 1 && kill(oldpid, 0) == 0) + if (S_ISLNK(st.st_mode)) { - quit("PID file " + pidfile + " already exists and the PID therein is valid"); - } - pidrifs.close(); - } + quit("PID file path is a symlink, refusing: " + pidfile); + } + if (!S_ISREG(st.st_mode)) + { + quit("PID file path exists and is not a regular file: " + pidfile); + } - pidofs.open(pidfile, std::fstream::out | std::fstream::trunc); - if (pidofs.fail()) + int oldpid = 0; + std::ifstream pidrifs; + pidrifs.open(pidfile, std::fstream::in); + if (!pidrifs.fail()) + { + // Read the PID and send signal 0 to see if the process exists. + errno = 0; + if (pidrifs >> oldpid && oldpid > 1 && (kill(oldpid, 0) == 0 || errno == EPERM)) + { + quit("PID file " + pidfile + " already exists and the PID therein is valid"); + } + pidrifs.close(); + } + + if (unlink(pidfile.c_str()) != 0) + { + quit("Failed to remove stale PID file: " + pidfile + ": " + std::strerror(errno)); + } + } + else if (errno != ENOENT) { - quit("Failed to open specified PID file for writing"); + quit("Failed to inspect PID file path: " + pidfile + ": " + std::strerror(errno)); + } + +#ifdef O_NOFOLLOW + const int flags = O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW; +#else + const int flags = O_WRONLY | O_CREAT | O_EXCL; +#endif + pid_fd = open(pidfile.c_str(), flags, 0644); + if (pid_fd < 0) + { + quit("Failed to create PID file: " + pidfile + ": " + std::strerror(errno)); } } // Fork the process and have the parent exit. If the process was started @@ -65,7 +109,7 @@ void fork(const std::string & pidfile) if (pid > 0) { // We're in the parent process and need to exit. - pidofs.close(); + close_pid_fd(); // When the exit() function is used, the program terminates without // invoking local variables' destructors. Only global variables are // destroyed. @@ -86,7 +130,7 @@ void fork(const std::string & pidfile) { if (pid > 0) { - pidofs.close(); + close_pid_fd(); exit(0); } else @@ -95,11 +139,18 @@ void fork(const std::string & pidfile) } } - if (! pidofs.fail()) + if (pid_fd >= 0) { - int pid = ::getpid(); - pidofs << pid << std::endl; - pidofs.close(); + const std::string pid = std::to_string(::getpid()) + "\n"; + const ssize_t written = write(pid_fd, pid.data(), pid.size()); + if (written < 0) + { + quit("Failed to write PID file: " + pidfile + ": " + std::strerror(errno)); + } + if (static_cast<size_t>(written) != pid.size()) + { + quit("Failed to write complete PID file: " + pidfile); + } } // Close the standard streams. This decouples the daemon from the terminal diff --git a/src/multisig/multisig_tx_builder_ringct.cpp b/src/multisig/multisig_tx_builder_ringct.cpp index 33c0396dc..3220246ec 100644 --- a/src/multisig/multisig_tx_builder_ringct.cpp +++ b/src/multisig/multisig_tx_builder_ringct.cpp @@ -254,7 +254,7 @@ static void make_tx_secret_key_seed(const crypto::secret_key& tx_secret_key_entr rct::keyV hash_context; hash_context.reserve(2 + sources.size()); auto hash_context_wiper = epee::misc_utils::create_scope_leave_handler([&]{ - memwipe(hash_context.data(), hash_context.size()); + memwipe(hash_context.data(), hash_context.size() * sizeof(rct::key)); }); hash_context.emplace_back(); rct::cn_fast_hash(hash_context.back(), domain_separator.data(), domain_separator.size()); //domain sep @@ -282,7 +282,7 @@ static void make_tx_secret_keys(const crypto::secret_key& tx_secret_key_seed, rct::keyV hash_context; hash_context.resize(2); auto hash_context_wiper = epee::misc_utils::create_scope_leave_handler([&]{ - memwipe(hash_context.data(), hash_context.size()); + memwipe(hash_context.data(), hash_context.size() * sizeof(rct::key)); }); hash_context[0] = rct::sk2rct(tx_secret_key_seed); rct::cn_fast_hash(hash_context[1], domain_separator.data(), domain_separator.size()); diff --git a/src/net/i2p_address.cpp b/src/net/i2p_address.cpp index e793048c0..e24e7da01 100644 --- a/src/net/i2p_address.cpp +++ b/src/net/i2p_address.cpp @@ -117,11 +117,15 @@ namespace net bool i2p_address::_load(epee::serialization::portable_storage& src, epee::serialization::section* hparent) { i2p_serialized in{}; - if (in._load(src, hparent) && in.host.size() < sizeof(host_) && (in.host == unknown_host || !host_check(in.host).has_error())) + if (in._load(src, hparent) && in.host.size() < sizeof(host_)) { - std::memcpy(host_, in.host.data(), in.host.size()); - std::memset(host_ + in.host.size(), 0, sizeof(host_) - in.host.size()); - return true; + net::canonicalize_host(in.host); + if (in.host == unknown_host || !host_check(in.host).has_error()) + { + std::memcpy(host_, in.host.data(), in.host.size()); + std::memset(host_ + in.host.size(), 0, sizeof(host_) - in.host.size()); + return true; + } } static_assert(sizeof(unknown_host) <= sizeof(host_), "bad buffer size"); std::memcpy(host_, unknown_host, sizeof(unknown_host)); // include null terminator diff --git a/src/net/tor_address.cpp b/src/net/tor_address.cpp index 35bd8e9a2..25f9fde66 100644 --- a/src/net/tor_address.cpp +++ b/src/net/tor_address.cpp @@ -129,12 +129,16 @@ namespace net bool tor_address::_load(epee::serialization::portable_storage& src, epee::serialization::section* hparent) { tor_serialized in{}; - if (in._load(src, hparent) && in.host.size() < sizeof(host_) && (in.host == unknown_host || !host_check(in.host).has_error())) + if (in._load(src, hparent) && in.host.size() < sizeof(host_)) { - std::memcpy(host_, in.host.data(), in.host.size()); - std::memset(host_ + in.host.size(), 0, sizeof(host_) - in.host.size()); - port_ = in.port; - return true; + net::canonicalize_host(in.host); + if (in.host == unknown_host || !host_check(in.host).has_error()) + { + std::memcpy(host_, in.host.data(), in.host.size()); + std::memset(host_ + in.host.size(), 0, sizeof(host_) - in.host.size()); + port_ = in.port; + return true; + } } static_assert(sizeof(unknown_host) <= sizeof(host_), "bad buffer size"); std::memcpy(host_, unknown_host, sizeof(unknown_host)); // include null terminator diff --git a/src/p2p/net_node.h b/src/p2p/net_node.h index 8e3312f29..d7cbaa567 100644 --- a/src/p2p/net_node.h +++ b/src/p2p/net_node.h @@ -451,6 +451,7 @@ namespace nodetool bool m_use_ipv6; bool m_require_ipv4; std::atomic<bool> is_closing; + std::atomic<bool> m_stop_signal_sent_once{false}; std::unique_ptr<boost::thread> mPeersLoggerThread; //critical_section m_connections_lock; //connections_indexed_container m_connections; diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index d1f7e7cdd..91a54f43e 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -1107,28 +1107,44 @@ namespace nodetool template<class t_payload_net_handler> bool node_server<t_payload_net_handler>::send_stop_signal() { + if (m_stop_signal_sent_once.exchange(true)) + { + MDEBUG("[node] Stop signal already sent"); + return true; + } MDEBUG("[node] stopping server payload handler"); m_payload_handler.stop(); - MDEBUG("[node] sending stop signal"); + + MDEBUG("[node] marking net servers as stopping"); for (auto& zone : m_network_zones) { - const auto close_all_connections = [&, this]() + zone.second.m_net_server.mark_stop_signal_sent(); + } + + MDEBUG("[node] closing connections"); + for (auto& zone : m_network_zones) + { + zone.second.m_net_server.close_server_connections(); + + std::list<boost::uuids::uuid> connection_ids; + zone.second.m_net_server.get_config_object().foreach_connection([&](const p2p_connection_context& cntxt) { - std::list<boost::uuids::uuid> connection_ids; - zone.second.m_net_server.get_config_object().foreach_connection([&](const p2p_connection_context& cntxt) { - connection_ids.push_back(cntxt.m_connection_id); - return true; - }); - for (const auto &connection_id: connection_ids) - { - MDEBUG("Closing connection " << connection_id); - // We need to wait for every connection's shutdown sequence to complete before stopping the io_context. - zone.second.m_net_server.get_config_object().close(connection_id, true/*wait_for_shutdown*/); - MDEBUG("Closed connection " << connection_id); - } - }; + connection_ids.push_back(cntxt.m_connection_id); + return true; + }); + for (const auto &connection_id: connection_ids) + { + MDEBUG("Closing connection " << connection_id); + // All zone connections must finish shutting down before any shared io_context is stopped. + zone.second.m_net_server.get_config_object().close(connection_id, true/*wait_for_shutdown*/); + MDEBUG("Closed connection " << connection_id); + } + } - zone.second.m_net_server.send_stop_signal(close_all_connections); + MDEBUG("[node] stopping net server io_contexts"); + for (auto& zone : m_network_zones) + { + zone.second.m_net_server.stop_io_context(); } MDEBUG("[node] Stop signal sent"); return true; diff --git a/src/rpc/daemon_handler.cpp b/src/rpc/daemon_handler.cpp index 10b20282e..add8bca8f 100644 --- a/src/rpc/daemon_handler.cpp +++ b/src/rpc/daemon_handler.cpp @@ -756,7 +756,7 @@ namespace rpc void DaemonHandler::handle(const GetTransactionPool::Request& req, GetTransactionPool::Response& res) { - bool r = m_core.get_pool_for_rpc(res.transactions, res.key_images); + bool r = m_core.get_pool_for_rpc(res.transactions, res.key_images, !m_restricted); if (!r) res.status = Message::STATUS_FAILED; else res.status = Message::STATUS_OK; diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 4c8d54f61..61f49481e 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -9116,7 +9116,6 @@ void wallet2::light_wallet_get_outs(std::vector<std::vector<tools::wallet2::get_ { const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex}; bool r = epee::net_utils::invoke_http_json("/get_random_outs", oreq, ores, *m_http_client, rpc_timeout, "POST"); - m_daemon_rpc_mutex.unlock(); THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "get_random_outs"); THROW_WALLET_EXCEPTION_IF(ores.amount_outs.empty() , error::wallet_internal_error, "No outputs received from light wallet node. Error: " + ores.Error); } diff --git a/src/wallet/wallet_errors.h b/src/wallet/wallet_errors.h index c54cd3499..e0a767159 100644 --- a/src/wallet/wallet_errors.h +++ b/src/wallet/wallet_errors.h @@ -440,7 +440,7 @@ namespace tools struct out_of_hashchain_bounds_error : public refresh_error { explicit out_of_hashchain_bounds_error(std::string&& loc) - : refresh_error(std::move(loc), "Index out of bounds of of hashchain") + : refresh_error(std::move(loc), "Index out of bounds of hashchain") { } diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 634bdae76..6a3e92b0b 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -3336,14 +3336,14 @@ namespace tools } entry.m_address = info.address; entry.m_is_subaddress = info.is_subaddress; - if (info.has_payment_id) - entry.m_payment_id = info.payment_id; + entry.m_has_payment_id = info.has_payment_id; + entry.m_payment_id = info.has_payment_id ? info.payment_id : crypto::null_hash8; } if (req.set_description) entry.m_description = req.description; - if (!m_wallet->set_address_book_row(req.index, entry.m_address, req.set_address && entry.m_has_payment_id ? &entry.m_payment_id : NULL, entry.m_description, entry.m_is_subaddress)) + if (!m_wallet->set_address_book_row(req.index, entry.m_address, entry.m_has_payment_id ? &entry.m_payment_id : NULL, entry.m_description, entry.m_is_subaddress)) { er.code = WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR; er.message = "Failed to edit address book entry"; diff --git a/tests/core_tests/tx_pool.cpp b/tests/core_tests/tx_pool.cpp index fab40a972..9ec89a936 100644 --- a/tests/core_tests/tx_pool.cpp +++ b/tests/core_tests/tx_pool.cpp @@ -455,7 +455,7 @@ bool txpool_double_spend_base::check_changed(cryptonote::core& c, const size_t e { std::vector<cryptonote::rpc::tx_in_pool> infos{}; cryptonote::rpc::key_images_with_tx_hashes key_images{}; - if (!c.get_pool_for_rpc(infos, key_images) || infos.size() != m_broadcasted_hashes.size() || key_images.size() != m_broadcasted_hashes.size()) + if (!c.get_pool_for_rpc(infos, key_images, true) || infos.size() != m_broadcasted_hashes.size() || key_images.size() != m_broadcasted_hashes.size()) { MERROR("Expected broadcasted rpc data to return " << m_broadcasted_hashes.size() << " but got " << infos.size() << " infos and " << key_images.size() << "key images"); return false; diff --git a/tests/functional_tests/address_book.py b/tests/functional_tests/address_book.py index 396ce505e..a315a1cdc 100755 --- a/tests/functional_tests/address_book.py +++ b/tests/functional_tests/address_book.py @@ -98,15 +98,15 @@ class AddressBookTest(): # request (partially) out of range ok = False - try: res = wallet.get_address_book[4, 2] + try: res = wallet.get_address_book([4, 2]) except: ok = True assert ok ok = False - try: res = wallet.get_address_book[0, 2] + try: res = wallet.get_address_book([0, 2]) except: ok = True assert ok ok = False - try: res = wallet.get_address_book[2, 0] + try: res = wallet.get_address_book([2, 0]) except: ok = True assert ok diff --git a/tests/unit_tests/epee_boosted_tcp_server.cpp b/tests/unit_tests/epee_boosted_tcp_server.cpp index 6052f7a50..586cc25a5 100644 --- a/tests/unit_tests/epee_boosted_tcp_server.cpp +++ b/tests/unit_tests/epee_boosted_tcp_server.cpp @@ -801,14 +801,11 @@ TEST(boosted_tcp_server, shutdown) server.get_config_object().handshake_received.wait(); } - // Now stop the server, providing the callback necessary to wait for all connections to shutdown - const auto close_all_connections = [&]() - { - server.get_config_object().close(context.m_connection_id, true/*wait_for_shutdown*/); - }; - MINFO("Stopping the server"); - server.send_stop_signal(close_all_connections); + server.mark_stop_signal_sent(); + server.close_server_connections(); + server.get_config_object().close(context.m_connection_id, true/*wait_for_shutdown*/); + server.stop_io_context(); running_server.join(); MINFO("Waiting for handshake to cancel"); diff --git a/tests/unit_tests/net.cpp b/tests/unit_tests/net.cpp index 9633f50e5..291f1e6ab 100644 --- a/tests/unit_tests/net.cpp +++ b/tests/unit_tests/net.cpp @@ -343,6 +343,20 @@ TEST(tor_address, epee_serializev_v3) EXPECT_STREQ(v3_onion, command.tor.host_str()); EXPECT_EQ(10u, command.tor.port()); + // make sure tor_address::_load canonicalizes incoming hosts + { + epee::serialization::portable_storage stg{}; + stg.load_from_binary(epee::to_span(buffer)); + + EXPECT_TRUE(stg.set_value("host", std::string{v3_onion_upper}, stg.open_section("tor", nullptr, false))); + EXPECT_TRUE(command.load(stg)); + } + + EXPECT_FALSE(command.tor.is_unknown()); + EXPECT_NE(net::tor_address{}, command.tor); + EXPECT_STREQ(v3_onion, command.tor.host_str()); + EXPECT_EQ(10u, command.tor.port()); + // make sure that exceeding max buffer doesn't destroy tor_address::_load { epee::serialization::portable_storage stg{}; @@ -751,6 +765,20 @@ TEST(i2p_address, epee_serializev_b32) EXPECT_STREQ(b32_i2p, command.i2p.host_str()); EXPECT_EQ(1u, command.i2p.port()); + // make sure i2p_address::_load canonicalizes incoming hosts + { + epee::serialization::portable_storage stg{}; + stg.load_from_binary(epee::to_span(buffer)); + + EXPECT_TRUE(stg.set_value("host", std::string{b32_i2p_upper}, stg.open_section("i2p", nullptr, false))); + EXPECT_TRUE(command.load(stg)); + } + + EXPECT_FALSE(command.i2p.is_unknown()); + EXPECT_NE(net::i2p_address{}, command.i2p); + EXPECT_STREQ(b32_i2p, command.i2p.host_str()); + EXPECT_EQ(1u, command.i2p.port()); + // make sure that exceeding max buffer doesn't destroy i2p_address::_load { epee::serialization::portable_storage stg{}; |