Cleanup TCP throttling code (performance) + move connection checks

89ad8ac epee: string_tools: keep full path in cut_off_extension (tobtoht)
c51ca53 epee: string_tools: remove dot from get_extension (tobtoht)

.github/workflows/build.yml

@@ -68,12 +68,7 @@ jobs:
     - uses: msys2/setup-msys2@v2
       with:
         update: true
-        install: mingw-w64-x86_64-toolchain make mingw-w64-x86_64-cmake mingw-w64-x86_64-ccache mingw-w64-x86_64-boost mingw-w64-x86_64-openssl mingw-w64-x86_64-zeromq mingw-w64-x86_64-libsodium mingw-w64-x86_64-hidapi mingw-w64-x86_64-libusb mingw-w64-x86_64-unbound git
-    - shell: msys2 {0}
-      run: |
-           curl -O https://repo.msys2.org/mingw/mingw64/mingw-w64-x86_64-protobuf-c-1.4.1-1-any.pkg.tar.zst
-           curl -O https://repo.msys2.org/mingw/mingw64/mingw-w64-x86_64-protobuf-21.9-1-any.pkg.tar.zst
-           pacman --noconfirm -U mingw-w64-x86_64-protobuf-c-1.4.1-1-any.pkg.tar.zst mingw-w64-x86_64-protobuf-21.9-1-any.pkg.tar.zst
+        install: mingw-w64-x86_64-toolchain make mingw-w64-x86_64-cmake mingw-w64-x86_64-ccache mingw-w64-x86_64-boost mingw-w64-x86_64-openssl mingw-w64-x86_64-zeromq mingw-w64-x86_64-libsodium mingw-w64-x86_64-hidapi mingw-w64-x86_64-protobuf mingw-w64-x86_64-libusb mingw-w64-x86_64-unbound git pkg-config
     - name: build
       run: |
         ${{env.CCACHE_SETTINGS}}

CMakeLists.txt

@@ -1084,31 +1084,40 @@ if(STATIC)
   set(Boost_USE_STATIC_RUNTIME ON)
 endif()
 
-set(BOOST_COMPONENTS system filesystem thread date_time chrono regex serialization program_options)
-if (WIN32)
+# Find Boost headers
+set(BOOST_MIN_VER 1.62)
+find_package(Boost ${BOOST_MIN_VER} QUIET REQUIRED)
+
+if(NOT Boost_FOUND)
+  die("Could not find Boost libraries, please make sure you have installed Boost or libboost-all-dev (>=${BOOST_MIN_VER}) or the equivalent")
+elseif(Boost_FOUND)
+  message(STATUS "Found Boost Version: ${Boost_VERSION_STRING}")
+
+  set(BOOST_COMPONENTS filesystem thread date_time chrono serialization program_options)
+  if (WIN32)
     list(APPEND BOOST_COMPONENTS locale)
+  endif()
+
+  # Boost System is header-only since 1.69
+  if (Boost_VERSION_STRING VERSION_LESS 1.69.0)
+    list(APPEND BOOST_COMPONENTS system)
+  endif()
+
+  # Boost Regex is header-only since 1.77
+  if (Boost_VERSION_STRING VERSION_LESS 1.77.0)
+    list(APPEND BOOST_COMPONENTS regex)
+  endif()
+
+  message(STATUS "Boost components: ${BOOST_COMPONENTS}")
+
+  # Find required Boost libraries
+  find_package(Boost ${BOOST_MIN_VER} QUIET REQUIRED COMPONENTS ${BOOST_COMPONENTS})
+  set(CMAKE_FIND_LIBRARY_SUFFIXES ${OLD_LIB_SUFFIXES})
 endif()
-find_package(Boost 1.58 QUIET REQUIRED COMPONENTS ${BOOST_COMPONENTS})
+
 add_definitions(-DBOOST_ASIO_ENABLE_SEQUENTIAL_STRAND_ALLOCATION)
 add_definitions(-DBOOST_NO_AUTO_PTR)
-
-set(CMAKE_FIND_LIBRARY_SUFFIXES ${OLD_LIB_SUFFIXES})
-if(NOT Boost_FOUND)
-  die("Could not find Boost libraries, please make sure you have installed Boost or libboost-all-dev (>=1.58) or the equivalent")
-elseif(Boost_FOUND)
-  message(STATUS "Found Boost Version: ${Boost_VERSION}")
-  if (Boost_VERSION VERSION_LESS 10 AND Boost_VERSION VERSION_LESS 1.62.0 AND NOT (OPENSSL_VERSION VERSION_LESS 1.1))
-    set(BOOST_BEFORE_1_62 true)
-  endif()
-  if (NOT Boost_VERSION VERSION_LESS 10 AND Boost_VERSION VERSION_LESS 106200 AND NOT (OPENSSL_VERSION VERSION_LESS 1.1))
-    set(BOOST_BEFORE_1_62 true)
-  endif()
-  if (BOOST_BEFORE_1_62)
-      message(FATAL_ERROR "Boost ${Boost_VERSION} (older than 1.62) is too old to link with OpenSSL ${OPENSSL_VERSION} (1.1 or newer) found at ${OPENSSL_INCLUDE_DIR} and ${OPENSSL_LIBRARIES}. "
-                          "Update Boost or install OpenSSL 1.0 and set path to it when running cmake: "
-                          "cmake -DOPENSSL_ROOT_DIR='/usr/include/openssl-1.0'")
-  endif()
-endif()
+add_definitions(-DBOOST_UUID_DISABLE_ALIGNMENT) # This restores UUID's std::has_unique_object_representations property
 
 include_directories(SYSTEM ${Boost_INCLUDE_DIRS})
 if(MINGW)

README.md

@@ -167,7 +167,7 @@ library archives (`.a`).
 | GCC          | 7             | NO       | `build-essential`    | `base-devel` | `base-devel`       | `gcc`               | NO       |                 |
 | CMake        | 3.5           | NO       | `cmake`              | `cmake`      | `cmake`            | `cmake`             | NO       |                 |
 | pkg-config   | any           | NO       | `pkg-config`         | `base-devel` | `base-devel`       | `pkgconf`           | NO       |                 |
-| Boost        | 1.58          | NO       | `libboost-all-dev`   | `boost`      | `boost-devel`      | `boost-devel`       | NO       | C++ libraries   |
+| Boost        | 1.62          | NO       | `libboost-all-dev`   | `boost`      | `boost-devel`      | `boost-devel`       | NO       | C++ libraries   |
 | OpenSSL      | basically any | NO       | `libssl-dev`         | `openssl`    | `openssl-devel`    | `openssl-devel`     | NO       | sha256 sum      |
 | libzmq       | 4.2.0         | NO       | `libzmq3-dev`        | `zeromq`     | `zeromq-devel`     | `zeromq-devel`      | NO       | ZeroMQ library  |
 | OpenPGM      | ?             | NO       | `libpgm-dev`         | `libpgm`     |                    | `openpgm-devel`     | NO       | For ZeroMQ      |

cmake/CheckTrezor.cmake

@@ -23,32 +23,6 @@ OPTION(USE_DEVICE_TREZOR_UDP_RELEASE "Trezor UdpTransport in release mode" $ENV{
 OPTION(USE_DEVICE_TREZOR_DEBUG "Trezor Debugging enabled" $ENV{USE_DEVICE_TREZOR_DEBUG})
 OPTION(TREZOR_DEBUG "Main Trezor debugging switch" $ENV{TREZOR_DEBUG})
 
-# Helper function to fix cmake < 3.6.0 FindProtobuf variables
-function(_trezor_protobuf_fix_vars)
-    if(${CMAKE_VERSION} VERSION_LESS "3.6.0")
-        foreach(UPPER
-                PROTOBUF_SRC_ROOT_FOLDER
-                PROTOBUF_IMPORT_DIRS
-                PROTOBUF_DEBUG
-                PROTOBUF_LIBRARY
-                PROTOBUF_PROTOC_LIBRARY
-                PROTOBUF_INCLUDE_DIR
-                PROTOBUF_PROTOC_EXECUTABLE
-                PROTOBUF_LIBRARY_DEBUG
-                PROTOBUF_PROTOC_LIBRARY_DEBUG
-                PROTOBUF_LITE_LIBRARY
-                PROTOBUF_LITE_LIBRARY_DEBUG
-                )
-            if (DEFINED ${UPPER})
-                string(REPLACE "PROTOBUF_" "Protobuf_" Camel ${UPPER})
-                if (NOT DEFINED ${Camel})
-                    set(${Camel} ${${UPPER}} PARENT_SCOPE)
-                endif()
-            endif()
-        endforeach()
-    endif()
-endfunction()
-
 macro(trezor_fatal_msg msg)
     if ($ENV{USE_DEVICE_TREZOR_MANDATORY})
         message(FATAL_ERROR
@@ -72,40 +46,21 @@ endmacro()
 
 # Use Trezor master switch
 if (USE_DEVICE_TREZOR)
-    # Protobuf is required to build protobuf messages for Trezor
-    include(FindProtobuf OPTIONAL)
+    # Look for protobuf-config.cmake, provided by Protobuf
+    find_package(Protobuf CONFIG)
 
-    # PkgConfig works better with new Protobuf
-    find_package(PkgConfig QUIET)
-    pkg_check_modules(PROTOBUF protobuf)
-
-    if (NOT Protobuf_FOUND)
-        FIND_PACKAGE(Protobuf CONFIG)
+    if (Protobuf_FOUND)
+        # https://github.com/protocolbuffers/protobuf/issues/14576
+        find_program(Protobuf_PROTOC_EXECUTABLE protoc REQUIRED)
+        set(Protobuf_LIBRARY protobuf::libprotobuf) # Compatibility with FindProtobuf.cmake
+    else()
+        # Look for FindProtobuf.cmake, provided by CMake
+        find_package(Protobuf)
     endif()
-    if (NOT Protobuf_FOUND)
-        FIND_PACKAGE(Protobuf)
-    endif()
-
-    _trezor_protobuf_fix_vars()
 
     # Early fail for optional Trezor support
-    if(NOT Protobuf_FOUND AND NOT Protobuf_LIBRARY AND NOT Protobuf_PROTOC_EXECUTABLE AND NOT Protobuf_INCLUDE_DIR)
-        trezor_fatal_msg("Trezor: Could not find Protobuf")
-    elseif(${CMAKE_CXX_STANDARD} LESS 17 AND ${Protobuf_VERSION} GREATER 21)
-        trezor_fatal_msg("Trezor: Unsupported Protobuf version ${Protobuf_VERSION} with C++ ${CMAKE_CXX_STANDARD}. Please, use Protobuf v21.")
-    elseif(NOT Protobuf_LIBRARY)
-        trezor_fatal_msg("Trezor: Protobuf library not found: ${Protobuf_LIBRARY}")
-        unset(Protobuf_FOUND)
-    elseif(NOT Protobuf_PROTOC_EXECUTABLE OR NOT EXISTS "${Protobuf_PROTOC_EXECUTABLE}")
-        trezor_fatal_msg("Trezor: Protobuf executable not found: ${Protobuf_PROTOC_EXECUTABLE}")
-        unset(Protobuf_FOUND)
-    elseif(NOT Protobuf_INCLUDE_DIR OR NOT EXISTS "${Protobuf_INCLUDE_DIR}")
-        trezor_fatal_msg("Trezor: Protobuf include dir not found: ${Protobuf_INCLUDE_DIR}")
-        unset(Protobuf_FOUND)
-    else()
-        message(STATUS "Trezor: Protobuf lib: ${Protobuf_LIBRARY}, inc: ${Protobuf_INCLUDE_DIR}, protoc: ${Protobuf_PROTOC_EXECUTABLE}")
-        set(Protobuf_INCLUDE_DIRS ${Protobuf_INCLUDE_DIR})
-        set(Protobuf_FOUND 1)  # override found if all required info was provided by variables
+    if (NOT Protobuf_FOUND)
+        trezor_fatal_msg("Trezor: protobuf library not found")
     endif()
 
     if(TREZOR_DEBUG)

contrib/epee/include/net/abstract_tcp_server2.inl

@@ -328,7 +328,7 @@ namespace net_utils
       return;
     }
     auto self = connection<T>::shared_from_this();
-    if (m_connection_type != e_connection_type_RPC) {
+    if (speed_limit_is_enabled()) {
       auto calc_duration = []{
         CRITICAL_REGION_LOCAL(
           network_throttle_manager_t::m_lock_get_global_throttle_in
@@ -382,7 +382,7 @@ namespace net_utils
             m_conn_context.m_max_speed_down,
             speed
           );
-          {
+          if (speed_limit_is_enabled()) {
             CRITICAL_REGION_LOCAL(
               network_throttle_manager_t::m_lock_get_global_throttle_in
             );
@@ -454,7 +454,7 @@ namespace net_utils
       return;
     }
     auto self = connection<T>::shared_from_this();
-    if (m_connection_type != e_connection_type_RPC) {
+    if (speed_limit_is_enabled()) {
       auto calc_duration = [this]{
         CRITICAL_REGION_LOCAL(
           network_throttle_manager_t::m_lock_get_global_throttle_out
@@ -513,7 +513,7 @@ namespace net_utils
             m_conn_context.m_max_speed_down,
             speed
           );
-          {
+          if (speed_limit_is_enabled()) {
             CRITICAL_REGION_LOCAL(
               network_throttle_manager_t::m_lock_get_global_throttle_out
             );
@@ -1022,7 +1022,7 @@ namespace net_utils
   template<typename T>
   bool connection<T>::speed_limit_is_enabled() const
   {
-    return m_connection_type != e_connection_type_RPC;
+    return m_connection_type == e_connection_type_P2P;
   }
 
   template<typename T>

contrib/epee/include/net/network_throttle-detail.hpp

@@ -46,13 +46,13 @@ namespace net_utils
 
 
 class network_throttle : public i_network_throttle {
-	private:
+	public:
 		struct packet_info {
 			size_t m_size; // octets sent. Summary for given small-window (e.g. for all packaged in 1 second)
 			packet_info();
 		};
 
-
+	private:
 		network_speed_bps m_target_speed;
 		size_t m_network_add_cost; // estimated add cost of headers 
 		size_t m_network_minimal_segment; // estimated minimal cost of sending 1 byte to round up to

contrib/epee/include/serialization/keyvalue_serialization.h

@@ -98,16 +98,18 @@ public: \
 #define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name) \
   epee::serialization::selector<is_store>::serialize_t_val_as_blob(this_ref.varialble, stg, hparent_section, val_name); 
 
-#define KV_SERIALIZE_VAL_POD_AS_BLOB_N(varialble, val_name) \
-  static_assert(std::is_pod<decltype(this_ref.varialble)>::value, "t_type must be a POD type."); \
-  KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name)
+#define KV_SERIALIZE_VAL_POD_AS_BLOB_N(variable, val_name) \
+  static_assert(std::is_trivially_copyable_v<decltype(this_ref.variable)>, "t_type must be a trivially copyable type."); \
+  static_assert(std::is_standard_layout_v<decltype(this_ref.variable)>, "t_type must be a standard layout type."); \
+  KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(variable, val_name)
 
-#define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(varialble, val_name, default_value) \
+#define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(variable, val_name, default_value) \
   do { \
-    static_assert(std::is_pod<decltype(this_ref.varialble)>::value, "t_type must be a POD type."); \
-    bool ret = KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, val_name) \
+    static_assert(std::is_trivially_copyable_v<decltype(this_ref.variable)>, "t_type must be a trivially copyable type."); \
+    static_assert(std::is_standard_layout_v<decltype(this_ref.variable)>, "t_type must be a standard layout type."); \
+    bool ret = KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(variable, val_name) \
     if (!ret) \
-      epee::serialize_default(this_ref.varialble, default_value); \
+      epee::serialize_default(this_ref.variable, default_value); \
   } while(0);
 
 #define KV_SERIALIZE_CONTAINER_POD_AS_BLOB_N(varialble, val_name) \
@@ -118,7 +120,7 @@ public: \
 #define KV_SERIALIZE(varialble)                           KV_SERIALIZE_N(varialble, #varialble)
 #define KV_SERIALIZE_VAL_POD_AS_BLOB(varialble)           KV_SERIALIZE_VAL_POD_AS_BLOB_N(varialble, #varialble)
 #define KV_SERIALIZE_VAL_POD_AS_BLOB_OPT(varialble, def)  KV_SERIALIZE_VAL_POD_AS_BLOB_OPT_N(varialble, #varialble, def)
-#define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE(varialble)     KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, #varialble) //skip is_pod compile time check
+#define KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE(varialble)     KV_SERIALIZE_VAL_POD_AS_BLOB_FORCE_N(varialble, #varialble) //skip is_trivially_copyable and is_standard_layout compile time check
 #define KV_SERIALIZE_CONTAINER_POD_AS_BLOB(varialble)     KV_SERIALIZE_CONTAINER_POD_AS_BLOB_N(varialble, #varialble)
 #define KV_SERIALIZE_OPT(variable,default_value)          KV_SERIALIZE_OPT_N(variable, #variable, default_value)
 

contrib/epee/include/span.h

@@ -133,17 +133,13 @@ namespace epee
     return {src.data(), src.size()};
   }
 
-  template<typename T>
-  constexpr bool has_padding() noexcept
-  {
-    return !std::is_standard_layout<T>() || alignof(T) != 1;
-  }
-
   //! \return Cast data from `src` as `span<const std::uint8_t>`.
   template<typename T>
   span<const std::uint8_t> to_byte_span(const span<const T> src) noexcept
   {
-    static_assert(!has_padding<T>(), "source type may have padding");
+    static_assert(!std::is_empty<T>(), "empty value types will not work -> sizeof == 1");
+    static_assert(std::is_standard_layout_v<T>, "type must have standard layout");
+    static_assert(std::has_unique_object_representations_v<T>, "type must be trivially copyable with no padding");
     return {reinterpret_cast<const std::uint8_t*>(src.data()), src.size_bytes()}; 
   }
 
@@ -153,7 +149,8 @@ namespace epee
   {
     using value_type = typename T::value_type;
     static_assert(!std::is_empty<value_type>(), "empty value types will not work -> sizeof == 1");
-    static_assert(!has_padding<value_type>(), "source value type may have padding");
+    static_assert(std::is_standard_layout_v<value_type>, "value type must have standard layout");
+    static_assert(std::has_unique_object_representations_v<value_type>, "value type must be trivially copyable with no padding");
     return {reinterpret_cast<std::uint8_t*>(src.data()), src.size() * sizeof(value_type)};
   }
 
@@ -162,7 +159,8 @@ namespace epee
   span<const std::uint8_t> as_byte_span(const T& src) noexcept
   {
     static_assert(!std::is_empty<T>(), "empty types will not work -> sizeof == 1");
-    static_assert(!has_padding<T>(), "source type may have padding");
+    static_assert(std::is_standard_layout_v<T>, "type must have standard layout");
+    static_assert(std::has_unique_object_representations_v<T>, "type must be trivially copyable with no padding");
     return {reinterpret_cast<const std::uint8_t*>(std::addressof(src)), sizeof(T)};
   }
 
@@ -171,7 +169,8 @@ namespace epee
   span<std::uint8_t> as_mut_byte_span(T& src) noexcept
   {
     static_assert(!std::is_empty<T>(), "empty types will not work -> sizeof == 1");
-    static_assert(!has_padding<T>(), "source type may have padding");
+    static_assert(std::is_standard_layout_v<T>, "type must have standard layout");
+    static_assert(std::has_unique_object_representations_v<T>, "type must be trivially copyable with no padding");
     return {reinterpret_cast<std::uint8_t*>(std::addressof(src)), sizeof(T)};
   }
 

contrib/epee/include/string_tools.h

@@ -89,6 +89,7 @@ namespace string_tools
   std::string pod_to_hex(const t_pod_type& s)
   {
     static_assert(std::is_standard_layout<t_pod_type>(), "expected standard layout type");
+    static_assert(std::has_unique_object_representations_v<t_pod_type>, "type may have padding");
     return to_hex::string(as_byte_span(s));
   }
   //----------------------------------------------------------------------------
@@ -96,6 +97,7 @@ namespace string_tools
   bool hex_to_pod(const boost::string_ref hex_str, t_pod_type& s)
   {
     static_assert(std::is_standard_layout<t_pod_type>(), "expected standard layout type");
+    static_assert(std::has_unique_object_representations_v<t_pod_type>, "type may have padding");
     return from_hex::to_buffer(as_mut_byte_span(s), hex_str);
   }
   //----------------------------------------------------------------------------

contrib/epee/src/network_throttle-detail.cpp

@@ -46,7 +46,7 @@
 #include "misc_log_ex.h" 
 #include <boost/chrono.hpp>
 #include "misc_language.h"
-#include <sstream>
+#include <fstream>
 #include <iomanip>
 #include <algorithm>
 
@@ -186,6 +186,23 @@ void network_throttle::handle_trafic_exact(size_t packet_size)
 	_handle_trafic_exact(packet_size, packet_size);
 }
 
+namespace
+{
+	struct output_history
+	{
+		const boost::circular_buffer< network_throttle::packet_info >& history;
+	};
+
+	std::ostream& operator<<(std::ostream& out, const output_history& source)
+	{
+		out << '[';
+		for (auto sample: source.history)
+			out << sample.m_size << ' ';
+		out << ']';
+		return out;
+	}
+}
+
 void network_throttle::_handle_trafic_exact(size_t packet_size, size_t orginal_size)
 {
 	tick();
@@ -196,14 +213,11 @@ void network_throttle::_handle_trafic_exact(size_t packet_size, size_t orginal_s
 	m_total_packets++;
 	m_total_bytes += packet_size;
 
-	std::ostringstream oss; oss << "["; 	for (auto sample: m_history) oss << sample.m_size << " ";	 oss << "]" << std::ends;
-	std::string history_str = oss.str();
-
 	MTRACE("Throttle " << m_name << ": packet of ~"<<packet_size<<"b " << " (from "<<orginal_size<<" b)"
         << " Speed AVG=" << std::setw(4) <<  ((long int)(cts .average/1024)) <<"[w="<<cts .window<<"]"
         <<           " " << std::setw(4) <<  ((long int)(cts2.average/1024)) <<"[w="<<cts2.window<<"]"
 				<<" / " << " Limit="<< ((long int)(m_target_speed/1024)) <<" KiB/sec "
-				<< " " << history_str
+				<< " " << output_history{m_history}
 		);
 }
 
@@ -289,8 +303,6 @@ void network_throttle::calculate_times(size_t packet_size, calculate_times_struc
     }
 
 	if (dbg) {
-		std::ostringstream oss; oss << "["; 	for (auto sample: m_history) oss << sample.m_size << " ";	 oss << "]" << std::ends;
-		std::string history_str = oss.str();
 		MTRACE((cts.delay > 0 ? "SLEEP" : "")
 			<< "dbg " << m_name << ": " 
 			<< "speed is A=" << std::setw(8) <<cts.average<<" vs "
@@ -300,7 +312,7 @@ void network_throttle::calculate_times(size_t packet_size, calculate_times_struc
 			<< "E="<< std::setw(8) << E << " (Enow="<<std::setw(8)<<Enow<<") "
             << "M=" << std::setw(8) << M <<" W="<< std::setw(8) << cts.window << " "
             << "R=" << std::setw(8) << cts.recomendetDataSize << " Wgood" << std::setw(8) << Wgood << " "
-			<< "History: " << std::setw(8) << history_str << " "
+			<< "History: " << std::setw(8) << output_history{m_history} << " "
 			<< "m_last_sample_time=" << std::setw(8) << m_last_sample_time
 		);
 

contrib/epee/src/string_tools.cpp

@@ -178,13 +178,18 @@ namespace string_tools
   
   std::string get_extension(const std::string& str)
   {
-    return boost::filesystem::path(str).extension().string();
+    std::string ext_with_dot = boost::filesystem::path(str).extension().string();
+
+    if (ext_with_dot.empty())
+      return {};
+
+    return ext_with_dot.erase(0, 1);
   }
 
 	//----------------------------------------------------------------------------
   std::string cut_off_extension(const std::string& str)
   {
-    return boost::filesystem::path(str).stem().string();
+    return boost::filesystem::path(str).replace_extension("").string();
   }
 
 #ifdef _WIN32

src/crypto/crypto.h

@@ -171,7 +171,9 @@ namespace crypto {
   /* Generate a value filled with random bytes.
    */
   template<typename T>
-  typename std::enable_if<std::is_pod<T>::value, T>::type rand() {
+  T rand() {
+    static_assert(std::is_standard_layout_v<T>, "cannot write random bytes into non-standard layout type");
+    static_assert(std::is_trivially_copyable_v<T>, "cannot write random bytes into non-trivially copyable type");
     typename std::remove_cv<T>::type res;
     generate_random_bytes_thread_safe(sizeof(T), (uint8_t*)&res);
     return res;
@@ -314,8 +316,14 @@ namespace crypto {
   inline std::ostream &operator <<(std::ostream &o, const crypto::public_key &v) {
     epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;
   }
-  inline std::ostream &operator <<(std::ostream &o, const crypto::secret_key &v) {
-    epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;
+  /* Do NOT overload the << operator for crypto::secret_key here. Use secret_key_explicit_print_ref
+   * instead to prevent accidental implicit dumping of secret key material to the logs (which has
+   * happened before). For the same reason, do not overload it for crypto::ec_scalar either since
+   * crypto::secret_key is a subclass. I'm not sorry that it's obtuse; that's the point, bozo.
+   */
+  struct secret_key_explicit_print_ref { const crypto::secret_key &sk; };
+  inline std::ostream &operator <<(std::ostream &o, const secret_key_explicit_print_ref v) {
+    epee::to_hex::formatted(o, epee::as_byte_span(unwrap(unwrap(v.sk)))); return o;
   }
   inline std::ostream &operator <<(std::ostream &o, const crypto::key_derivation &v) {
     epee::to_hex::formatted(o, epee::as_byte_span(v)); return o;

src/cryptonote_basic/cryptonote_format_utils.cpp

@@ -292,7 +292,7 @@ namespace cryptonote
     bool r = hwdev.generate_key_derivation(tx_public_key, ack.m_view_secret_key, recv_derivation);
     if (!r)
     {
-      MWARNING("key image helper: failed to generate_key_derivation(" << tx_public_key << ", " << ack.m_view_secret_key << ")");
+      MWARNING("key image helper: failed to generate_key_derivation(" << tx_public_key << ", <viewkey>)");
       memcpy(&recv_derivation, rct::identity().bytes, sizeof(recv_derivation));
     }
 
@@ -303,7 +303,7 @@ namespace cryptonote
       r = hwdev.generate_key_derivation(additional_tx_public_keys[i], ack.m_view_secret_key, additional_recv_derivation);
       if (!r)
       {
-        MWARNING("key image helper: failed to generate_key_derivation(" << additional_tx_public_keys[i] << ", " << ack.m_view_secret_key << ")");
+        MWARNING("key image helper: failed to generate_key_derivation(" << additional_tx_public_keys[i] << ", <viewkey>)");
       }
       else
       {

src/cryptonote_core/cryptonote_tx_utils.cpp

@@ -144,7 +144,7 @@ namespace cryptonote
       crypto::key_derivation derivation = AUTO_VAL_INIT(derivation);
       crypto::public_key out_eph_public_key = AUTO_VAL_INIT(out_eph_public_key);
       bool r = crypto::generate_key_derivation(miner_address.m_view_public_key, txkey.sec, derivation);
-      CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to generate_key_derivation(" << miner_address.m_view_public_key << ", " << txkey.sec << ")");
+      CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to generate_key_derivation(" << miner_address.m_view_public_key << ", " << crypto::secret_key_explicit_print_ref{txkey.sec} << ")");
 
       r = crypto::derive_public_key(derivation, no, miner_address.m_spend_public_key, out_eph_public_key);
       CHECK_AND_ASSERT_MES(r, false, "while creating outs: failed to derive_public_key(" << derivation << ", " << no << ", "<< miner_address.m_spend_public_key << ")");
@@ -484,7 +484,7 @@ namespace cryptonote
           crypto::generate_ring_signature(tx_prefix_hash, boost::get<txin_to_key>(tx.vin[i]).k_image, keys_ptrs, in_contexts[i].in_ephemeral.sec, src_entr.real_output, sigs.data());
         ss_ring_s << "signatures:" << ENDL;
         std::for_each(sigs.begin(), sigs.end(), [&](const crypto::signature& s){ss_ring_s << s << ENDL;});
-        ss_ring_s << "prefix_hash:" << tx_prefix_hash << ENDL << "in_ephemeral_key: " << in_contexts[i].in_ephemeral.sec << ENDL << "real_output: " << src_entr.real_output << ENDL;
+        ss_ring_s << "prefix_hash:" << tx_prefix_hash << ENDL << "in_ephemeral_key: " << crypto::secret_key_explicit_print_ref{in_contexts[i].in_ephemeral.sec} << ENDL << "real_output: " << src_entr.real_output << ENDL;
         i++;
       }
 

src/cryptonote_protocol/block_queue.cpp

@@ -40,15 +40,6 @@
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "cn.block_queue"
 
-namespace std {
-  static_assert(sizeof(size_t) <= sizeof(boost::uuids::uuid), "boost::uuids::uuid too small");
-  template<> struct hash<boost::uuids::uuid> {
-    std::size_t operator()(const boost::uuids::uuid &_v) const {
-      return reinterpret_cast<const std::size_t &>(_v);
-    }
-  };
-}
-
 namespace cryptonote
 {
 
@@ -472,7 +463,7 @@ bool block_queue::has_spans(const boost::uuids::uuid &connection_id) const
 float block_queue::get_speed(const boost::uuids::uuid &connection_id) const
 {
   boost::unique_lock<boost::recursive_mutex> lock(mutex);
-  std::unordered_map<boost::uuids::uuid, float> speeds;
+  std::unordered_map<boost::uuids::uuid, float, boost::hash<boost::uuids::uuid>> speeds;
   for (const auto &span: blocks)
   {
     if (span.blocks.empty())
@@ -480,7 +471,7 @@ float block_queue::get_speed(const boost::uuids::uuid &connection_id) const
     // note that the average below does not average over the whole set, but over the
     // previous pseudo average and the latest rate: this gives much more importance
     // to the latest measurements, which is fine here
-    std::unordered_map<boost::uuids::uuid, float>::iterator i = speeds.find(span.connection_id);
+    const auto i = speeds.find(span.connection_id);
     if (i == speeds.end())
       speeds.insert(std::make_pair(span.connection_id, span.rate));
     else

src/device/device_default.cpp

@@ -317,13 +317,15 @@ namespace hw {
             {
             // sending change to yourself; derivation = a*R
                 r = generate_key_derivation(txkey_pub, sender_account_keys.m_view_secret_key, derivation);
-                CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << txkey_pub << ", " << sender_account_keys.m_view_secret_key << ")");
+                CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << txkey_pub << ", <viewkey>)");
             }
             else
             {
             // sending to the recipient; derivation = r*A (or s*C in the subaddress scheme)
-                r = generate_key_derivation(dst_entr.addr.m_view_public_key, dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key, derivation);
-                CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation(" << dst_entr.addr.m_view_public_key << ", " << (dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key) << ")");
+                const crypto::secret_key &tx_privkey{dst_entr.is_subaddress && need_additional_txkeys ? additional_txkey.sec : tx_key};
+                r = generate_key_derivation(dst_entr.addr.m_view_public_key, tx_privkey, derivation);
+                CHECK_AND_ASSERT_MES(r, false, "at creation outs: failed to generate_key_derivation("
+                    << dst_entr.addr.m_view_public_key << ", " << crypto::secret_key_explicit_print_ref{tx_privkey} << ")");
             }
 
             if (need_additional_txkeys)

src/lmdb/util.h

@@ -127,7 +127,7 @@ namespace lmdb
     /*!
         A LMDB comparison function that uses `std::memcmp`.
 
-        \toaram T is `!epee::has_padding`
+        \toaram T has standard layout and an alignment of 1
         \tparam offset to `T` within the value.
 
         \return The result of `std::memcmp` over the value.
@@ -135,7 +135,7 @@ namespace lmdb
     template<typename T, std::size_t offset = 0>
     inline int compare(MDB_val const* left, MDB_val const* right) noexcept
     {
-        static_assert(!epee::has_padding<T>(), "memcmp will not work");
+        static_assert(std::is_standard_layout_v<T> && alignof(T) == 1, "memcmp will not work");
         if (!left || !right || left->mv_size < sizeof(T) + offset || right->mv_size < sizeof(T) + offset)
         {
             assert("invalid use of custom comparison" == 0);

src/p2p/net_node.inl

@@ -174,6 +174,19 @@ namespace nodetool
   template<class t_payload_net_handler>
   bool node_server<t_payload_net_handler>::is_remote_host_allowed(const epee::net_utils::network_address &address, time_t *t)
   {
+    const network_zone& zone = m_network_zones.at(address.get_zone());
+    if (zone.m_current_number_of_in_peers >= zone.m_config.m_net_config.max_in_connection_count) // in peers limit
+    {
+      MWARNING("Exceeded max incoming connections, so dropping this one.");
+      return false;
+    }
+
+    if(has_too_many_connections(address))
+    {
+      MWARNING("CONNECTION FROM " << address.host_str() << " REFUSED, too many connections from the same address");
+      return false;
+    }
+
     CRITICAL_REGION_LOCAL(m_blocked_hosts_lock);
 
     const time_t now = time(nullptr);
@@ -2543,13 +2556,6 @@ namespace nodetool
       return 1;
     }
 
-    if (zone.m_current_number_of_in_peers >= zone.m_config.m_net_config.max_in_connection_count) // in peers limit
-    {
-      LOG_WARNING_CC(context, "COMMAND_HANDSHAKE came, but already have max incoming connections, so dropping this one.");
-      drop_connection(context);
-      return 1;
-    }
-
     if(!m_payload_handler.process_payload_sync_data(arg.payload_data, context, true))
     {
       LOG_WARNING_CC(context, "COMMAND_HANDSHAKE came, but process_payload_sync_data returned false, dropping connection.");
@@ -2559,13 +2565,6 @@ namespace nodetool
 
     zone.m_notifier.on_handshake_complete(context.m_connection_id, context.m_is_income);
 
-    if(has_too_many_connections(context.m_remote_address))
-    {
-      LOG_PRINT_CCONTEXT_L1("CONNECTION FROM " << context.m_remote_address.host_str() << " REFUSED, too many connections from the same address");
-      drop_connection(context);
-      return 1;
-    }
-
     //associate peer_id with this connection
     context.peer_id = arg.node_data.peer_id;
     context.m_in_timedsync = false;
@@ -2885,15 +2884,16 @@ namespace nodetool
       if (cntxt.m_is_income && cntxt.m_remote_address.is_same_host(address)) {
         count++;
 
-        if (count > max_connections) {
+        // the only call location happens BEFORE foreach_connection list is updated
+        if (count >= max_connections) {
           return false;
         }
       }
 
       return true;
     });
-
-    return count > max_connections;
+    // the only call location happens BEFORE foreach_connection list is updated
+    return count >= max_connections;
   }
 
   template<class t_payload_net_handler>

src/simplewallet/simplewallet.cpp

@@ -7872,9 +7872,9 @@ bool simple_wallet::submit_transfer(const std::vector<std::string> &args_)
 std::string get_tx_key_stream(crypto::secret_key tx_key, std::vector<crypto::secret_key> additional_tx_keys)
 {
   ostringstream oss;
-  oss << epee::string_tools::pod_to_hex(tx_key);
+  oss << epee::string_tools::pod_to_hex(unwrap(unwrap(tx_key)));
   for (size_t i = 0; i < additional_tx_keys.size(); ++i)
-    oss << epee::string_tools::pod_to_hex(additional_tx_keys[i]);
+    oss << epee::string_tools::pod_to_hex(unwrap(unwrap(additional_tx_keys[i])));
   return oss.str();
 }
 

src/wallet/api/wallet.cpp

@@ -895,7 +895,7 @@ std::string WalletImpl::integratedAddress(const std::string &payment_id) const
 
 std::string WalletImpl::secretViewKey() const
 {
-    return epee::string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_view_secret_key);
+    return epee::string_tools::pod_to_hex(unwrap(unwrap(m_wallet->get_account().get_keys().m_view_secret_key)));
 }
 
 std::string WalletImpl::publicViewKey() const
@@ -905,7 +905,7 @@ std::string WalletImpl::publicViewKey() const
 
 std::string WalletImpl::secretSpendKey() const
 {
-    return epee::string_tools::pod_to_hex(m_wallet->get_account().get_keys().m_spend_secret_key);
+    return epee::string_tools::pod_to_hex(unwrap(unwrap(m_wallet->get_account().get_keys().m_spend_secret_key)));
 }
 
 std::string WalletImpl::publicSpendKey() const
@@ -1999,9 +1999,9 @@ std::string WalletImpl::getTxKey(const std::string &txid_str) const
         {
             clearStatus();
             std::ostringstream oss;
-            oss << epee::string_tools::pod_to_hex(tx_key);
+            oss << epee::string_tools::pod_to_hex(unwrap(unwrap(tx_key)));
             for (size_t i = 0; i < additional_tx_keys.size(); ++i)
-                oss << epee::string_tools::pod_to_hex(additional_tx_keys[i]);
+                oss << epee::string_tools::pod_to_hex(unwrap(unwrap(additional_tx_keys[i])));
             return oss.str();
         }
         else

src/wallet/wallet2.cpp

@@ -4721,7 +4721,7 @@ boost::optional<wallet2::keys_file_data> wallet2::get_keys_file_data(const crypt
     original_address = get_account_address_as_str(m_nettype, false, m_original_address);
     value.SetString(original_address.c_str(), original_address.length());
     json.AddMember("original_address", value, json.GetAllocator());
-    original_view_secret_key = epee::string_tools::pod_to_hex(m_original_view_secret_key);
+    original_view_secret_key = epee::string_tools::pod_to_hex(unwrap(unwrap(m_original_view_secret_key)));
     value.SetString(original_view_secret_key.c_str(), original_view_secret_key.length());
     json.AddMember("original_view_secret_key", value, json.GetAllocator());
   }

src/wallet/wallet_rpc_server.cpp

@@ -1311,9 +1311,9 @@ namespace tools
       res.tx_hash_list.push_back(epee::string_tools::pod_to_hex(cryptonote::get_transaction_hash(ptx.tx)));
       if (req.get_tx_keys)
       {
-        res.tx_key_list.push_back(epee::string_tools::pod_to_hex(ptx.tx_key));
+        res.tx_key_list.push_back(epee::string_tools::pod_to_hex(unwrap(unwrap(ptx.tx_key))));
         for (const crypto::secret_key& additional_tx_key : ptx.additional_tx_keys)
-          res.tx_key_list.back() += epee::string_tools::pod_to_hex(additional_tx_key);
+          res.tx_key_list.back() += epee::string_tools::pod_to_hex(unwrap(unwrap(additional_tx_key)));
       }
     }
 

tests/benchmark.cpp

@@ -109,7 +109,7 @@ namespace
     template<typename T>
     bool compare(const T& lhs, const T& rhs) noexcept
     {
-        static_assert(!epee::has_padding<T>(), "type might have padding");
+        static_assert(std::is_standard_layout_v<T> && alignof(T) == 1, "type might have padding");
         return std::memcmp(std::addressof(lhs), std::addressof(rhs), sizeof(T)) == 0;
     }
 

tests/core_tests/multisig.cpp

@@ -227,13 +227,13 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector<test_event_entry
         CHECK_AND_ASSERT_MES(r, false, "Failed to generate multisig export key image");
       }
       MDEBUG("Party " << msidx << ":");
-      MDEBUG("spend: sec " << miner_account[msidx].get_keys().m_spend_secret_key << ", pub " << miner_account[msidx].get_keys().m_account_address.m_spend_public_key);
-      MDEBUG("view: sec " << miner_account[msidx].get_keys().m_view_secret_key << ", pub " << miner_account[msidx].get_keys().m_account_address.m_view_public_key);
+      MDEBUG("spend: sec " << crypto::secret_key_explicit_print_ref{miner_account[msidx].get_keys().m_spend_secret_key} << ", pub " << miner_account[msidx].get_keys().m_account_address.m_spend_public_key);
+      MDEBUG("view: sec " << crypto::secret_key_explicit_print_ref{miner_account[msidx].get_keys().m_view_secret_key} << ", pub " << miner_account[msidx].get_keys().m_account_address.m_view_public_key);
       for (const auto &k: miner_account[msidx].get_multisig_keys())
-        MDEBUG("msk: " << k);
+        MDEBUG("msk: " << crypto::secret_key_explicit_print_ref{k});
       for (size_t n = 0; n < account_k[msidx][tdidx].size(); ++n)
       {
-        MDEBUG("k: " << account_k[msidx][tdidx][n]);
+        MDEBUG("k: " << crypto::secret_key_explicit_print_ref{account_k[msidx][tdidx][n]});
         MDEBUG("L: " << account_L[msidx][tdidx][n]);
         MDEBUG("R: " << account_R[msidx][tdidx][n]);
       }
@@ -406,7 +406,7 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector<test_event_entry
       MDEBUG("signing with k " << k.back()[n]);
     MDEBUG("signing with sk " << skey);
     for (const auto &sk: used_keys)
-      MDEBUG("  created with sk " << sk);
+      MDEBUG("  created with sk " << crypto::secret_key_explicit_print_ref{sk});
     CHECK_AND_ASSERT_MES(signer_tx_builder.next_partial_sign(sig.total_alpha_G, sig.total_alpha_H, k, skey, sig.c_0, sig.s), false, "error: multisig::signing::tx_builder_ringct_t::next_partial_sign");
 
     // in round-robin signing, the last signer finalizes the tx

tests/functional_tests/make_test_signature.cc

@@ -48,7 +48,7 @@ int main(int argc, const char **argv)
     crypto::public_key pkey;
     crypto::random32_unbiased((unsigned char*)skey.data);
     crypto::secret_key_to_public_key(skey, pkey);
-    printf("%s %s\n", epee::string_tools::pod_to_hex(skey).c_str(), epee::string_tools::pod_to_hex(pkey).c_str());
+    printf("%s %s\n", epee::to_hex::string({to_bytes(skey), 32}).c_str(), epee::string_tools::pod_to_hex(pkey).c_str());
     return 0;
   }
 

tests/unit_tests/crypto.cpp

@@ -79,10 +79,10 @@ TEST(Crypto, Ostream)
   EXPECT_TRUE(is_formatted<crypto::hash8>());
   EXPECT_TRUE(is_formatted<crypto::hash>());
   EXPECT_TRUE(is_formatted<crypto::public_key>());
-  EXPECT_TRUE(is_formatted<crypto::secret_key>());
   EXPECT_TRUE(is_formatted<crypto::signature>());
   EXPECT_TRUE(is_formatted<crypto::key_derivation>());
   EXPECT_TRUE(is_formatted<crypto::key_image>());
+  EXPECT_TRUE(is_formatted<rct::key>());
 }
 
 TEST(Crypto, null_keys)

tests/unit_tests/epee_utils.cpp

@@ -1443,6 +1443,21 @@ TEST(StringTools, GetIpInt32)
   EXPECT_EQ(htonl(0xff0aff00), ip);
 }
 
+TEST(StringTools, GetExtension)
+{
+  EXPECT_EQ(std::string{}, epee::string_tools::get_extension(""));
+  EXPECT_EQ(std::string{}, epee::string_tools::get_extension("."));
+  EXPECT_EQ(std::string{"keys"}, epee::string_tools::get_extension("wallet.keys"));
+  EXPECT_EQ(std::string{"3"}, epee::string_tools::get_extension("1.2.3"));
+}
+
+TEST(StringTools, CutOffExtension)
+{
+  EXPECT_EQ(std::string{}, epee::string_tools::cut_off_extension(""));
+  EXPECT_EQ(std::string{"/home/user/Monero/wallets/wallet"}, epee::string_tools::cut_off_extension("/home/user/Monero/wallets/wallet"));
+  EXPECT_EQ(std::string{"/home/user/Monero/wallets/wallet"}, epee::string_tools::cut_off_extension("/home/user/Monero/wallets/wallet.keys"));
+}
+
 TEST(NetUtils, IPv4NetworkAddress)
 {
   static_assert(epee::net_utils::ipv4_network_address::get_type_id() == epee::net_utils::address_type::ipv4, "bad ipv4 type id");

tests/unit_tests/multisig.cpp

@@ -80,7 +80,7 @@ static void make_wallet(unsigned int idx, tools::wallet2 &wallet)
     wallet.generate("", "", spendkey, true, false);
     ASSERT_TRUE(test_addresses[idx].address == wallet.get_account().get_public_address_str(cryptonote::TESTNET));
     wallet.decrypt_keys("");
-    ASSERT_TRUE(test_addresses[idx].spendkey == epee::string_tools::pod_to_hex(wallet.get_account().get_keys().m_spend_secret_key));
+    ASSERT_TRUE(test_addresses[idx].spendkey == epee::string_tools::pod_to_hex(unwrap(unwrap(wallet.get_account().get_keys().m_spend_secret_key))));
     wallet.encrypt_keys("");
   }
   catch (const std::exception &e)

tests/unit_tests/node_server.cpp

@@ -224,6 +224,18 @@ TEST(ban, subnet)
   test_core pr_core;
   cryptonote::t_cryptonote_protocol_handler<test_core> cprotocol(pr_core, NULL);
   Server server(cprotocol);
+  {
+    boost::program_options::options_description opts{};
+    Server::init_options(opts);
+    cryptonote::core::init_options(opts);
+
+    char** args = nullptr;
+    boost::program_options::variables_map vm;
+    boost::program_options::store(
+      boost::program_options::parse_command_line(0, args, opts), vm
+    );
+    server.init(vm);
+  }
   cprotocol.set_p2p_endpoint(&server);
 
   ASSERT_TRUE(server.block_subnet(MAKE_IPV4_SUBNET(1,2,3,4,24), 10));

tests/unit_tests/serialization.cpp

@@ -1113,7 +1113,7 @@ TEST(Serialization, portability_signed_tx)
   ASSERT_TRUE(ptx.selected_transfers.front() == 2);
   // ptx.{key_images, tx_key}
   ASSERT_TRUE(ptx.key_images == "<6c3cd6af97c4070a7aef9b1344e7463e29c7cd245076fdb65da447a34da3ca76> ");
-  ASSERT_TRUE(epee::string_tools::pod_to_hex(ptx.tx_key) == "0100000000000000000000000000000000000000000000000000000000000000");
+  ASSERT_TRUE(epee::string_tools::pod_to_hex(unwrap(unwrap(ptx.tx_key))) == "0100000000000000000000000000000000000000000000000000000000000000");
   // ptx.dests
   ASSERT_TRUE(ptx.dests.size() == 1);
   ASSERT_TRUE(ptx.dests[0].amount == 1400000000000);