From 3bb3920e6bf949566e33a49be04b5c264317fcaf Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 17:06:36 -0300 Subject: [PATCH 01/23] Cosmetic: move decrypt_cbc to be beside encrypt_cbc --- crypto/src/channel_encryption.cpp | 92 +++++++++++++++---------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/crypto/src/channel_encryption.cpp b/crypto/src/channel_encryption.cpp index 9aee1fbd..17ecece2 100644 --- a/crypto/src/channel_encryption.cpp +++ b/crypto/src/channel_encryption.cpp @@ -132,6 +132,52 @@ std::string ChannelEncryption::encrypt_cbc( return output; } +std::string ChannelEncryption::decrypt_cbc( + std::string_view ciphertext_, const x25519_pubkey& pubKey) const { + + auto ciphertext = to_uchar(ciphertext_); + + const auto sharedKey = calculate_shared_secret(private_key_, pubKey); + + // Initialise cipher context + const EVP_CIPHER* cipher = EVP_aes_256_cbc(); + aes256cbc_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; + auto* ctx = ctx_ptr.get(); + + // We prepend the iv on the beginning of the ciphertext; extract it + auto iv = ciphertext.substr(0, EVP_CIPHER_iv_length(cipher)); + ciphertext.remove_prefix(iv.size()); + + // libssl docs say we need up to block size of extra buffer space: + std::string output; + output.resize(ciphertext.size() + EVP_CIPHER_block_size(cipher)); + + // Initialise cipher context + if (EVP_DecryptInit_ex(ctx, cipher, nullptr, sharedKey.data(), iv.data()) <= 0) { + throw std::runtime_error("Could not initialise decryption context"); + } + + int len; + auto* o = reinterpret_cast(output.data()); + + // Decrypt every full blocks + if (EVP_DecryptUpdate(ctx, o, &len, ciphertext.data(), ciphertext.size()) <= 0) { + throw std::runtime_error("Could not decrypt block"); + } + o += len; + + // Decrypt any remaining partial blocks + if (EVP_DecryptFinal_ex(ctx, o, &len) <= 0) { + throw std::runtime_error("Could not finalise decryption"); + } + o += len; + + // Remove excess buffer space + output.resize(reinterpret_cast(o) - output.data()); + + return output; +} + std::string ChannelEncryption::encrypt_gcm( std::string_view plaintext_, const x25519_pubkey& pubKey) const { @@ -195,52 +241,6 @@ std::string ChannelEncryption::decrypt_gcm( return output; } -std::string ChannelEncryption::decrypt_cbc( - std::string_view ciphertext_, const x25519_pubkey& pubKey) const { - - auto ciphertext = to_uchar(ciphertext_); - - const auto sharedKey = calculate_shared_secret(private_key_, pubKey); - - // Initialise cipher context - const EVP_CIPHER* cipher = EVP_aes_256_cbc(); - aes256cbc_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; - auto* ctx = ctx_ptr.get(); - - // We prepend the iv on the beginning of the ciphertext; extract it - auto iv = ciphertext.substr(0, EVP_CIPHER_iv_length(cipher)); - ciphertext.remove_prefix(iv.size()); - - // libssl docs say we need up to block size of extra buffer space: - std::string output; - output.resize(ciphertext.size() + EVP_CIPHER_block_size(cipher)); - - // Initialise cipher context - if (EVP_DecryptInit_ex(ctx, cipher, nullptr, sharedKey.data(), iv.data()) <= 0) { - throw std::runtime_error("Could not initialise decryption context"); - } - - int len; - auto* o = reinterpret_cast(output.data()); - - // Decrypt every full blocks - if (EVP_DecryptUpdate(ctx, o, &len, ciphertext.data(), ciphertext.size()) <= 0) { - throw std::runtime_error("Could not decrypt block"); - } - o += len; - - // Decrypt any remaining partial blocks - if (EVP_DecryptFinal_ex(ctx, o, &len) <= 0) { - throw std::runtime_error("Could not finalise decryption"); - } - o += len; - - // Remove excess buffer space - output.resize(reinterpret_cast(o) - output.data()); - - return output; -} - static std::array xchacha20_shared_key( const x25519_pubkey& local_pub, From 344735ab09eea3b7ab605058faeb02e2bf79c177 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 17:26:48 -0300 Subject: [PATCH 02/23] Abstract openssl encrypt/decryption call --- crypto/src/channel_encryption.cpp | 48 +++++++++++++++++++------------ 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/crypto/src/channel_encryption.cpp b/crypto/src/channel_encryption.cpp index 17ecece2..4732c60b 100644 --- a/crypto/src/channel_encryption.cpp +++ b/crypto/src/channel_encryption.cpp @@ -57,7 +57,7 @@ struct aes256_evp_deleter { } }; -using aes256cbc_ctx_ptr = std::unique_ptr; +using aes256_ctx_ptr = std::unique_ptr; } @@ -87,16 +87,13 @@ std::string ChannelEncryption::decrypt(EncryptType type, std::string_view cipher throw std::runtime_error{"Invalid decryption type"}; } -std::string ChannelEncryption::encrypt_cbc( - std::string_view plaintext_, const x25519_pubkey& pubKey) const { - - auto plaintext = to_uchar(plaintext_); - - const auto sharedKey = calculate_shared_secret(private_key_, pubKey); +static std::string encrypt_openssl( + const EVP_CIPHER* cipher, + std::basic_string_view plaintext, + const std::array& key) { // Initialise cipher context - const EVP_CIPHER* cipher = EVP_aes_256_cbc(); - aes256cbc_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; + aes256_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; auto* ctx = ctx_ptr.get(); std::string output; @@ -109,7 +106,7 @@ std::string ChannelEncryption::encrypt_cbc( const auto* iv = o; o += ivLength; - if (EVP_EncryptInit_ex(ctx, cipher, nullptr, sharedKey.data(), iv) <= 0) { + if (EVP_EncryptInit_ex(ctx, cipher, nullptr, key.data(), iv) <= 0) { throw std::runtime_error("Could not initialise encryption context"); } @@ -132,16 +129,13 @@ std::string ChannelEncryption::encrypt_cbc( return output; } -std::string ChannelEncryption::decrypt_cbc( - std::string_view ciphertext_, const x25519_pubkey& pubKey) const { - - auto ciphertext = to_uchar(ciphertext_); - - const auto sharedKey = calculate_shared_secret(private_key_, pubKey); +static std::string decrypt_openssl( + const EVP_CIPHER* cipher, + std::basic_string_view ciphertext, + const std::array& key) { // Initialise cipher context - const EVP_CIPHER* cipher = EVP_aes_256_cbc(); - aes256cbc_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; + aes256_ctx_ptr ctx_ptr{EVP_CIPHER_CTX_new()}; auto* ctx = ctx_ptr.get(); // We prepend the iv on the beginning of the ciphertext; extract it @@ -153,7 +147,7 @@ std::string ChannelEncryption::decrypt_cbc( output.resize(ciphertext.size() + EVP_CIPHER_block_size(cipher)); // Initialise cipher context - if (EVP_DecryptInit_ex(ctx, cipher, nullptr, sharedKey.data(), iv.data()) <= 0) { + if (EVP_DecryptInit_ex(ctx, cipher, nullptr, key.data(), iv.data()) <= 0) { throw std::runtime_error("Could not initialise decryption context"); } @@ -178,6 +172,22 @@ std::string ChannelEncryption::decrypt_cbc( return output; } +std::string ChannelEncryption::encrypt_cbc( + std::string_view plaintext_, const x25519_pubkey& pubKey) const { + return encrypt_openssl( + EVP_aes_256_cbc(), + to_uchar(plaintext_), + calculate_shared_secret(private_key_, pubKey)); +} + +std::string ChannelEncryption::decrypt_cbc( + std::string_view ciphertext_, const x25519_pubkey& pubKey) const { + return decrypt_openssl( + EVP_aes_256_cbc(), + to_uchar(ciphertext_), + calculate_shared_secret(private_key_, pubKey)); +} + std::string ChannelEncryption::encrypt_gcm( std::string_view plaintext_, const x25519_pubkey& pubKey) const { From 6fe0c65528aef453181cb25d9c3ed177a9d35c12 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 18:03:30 -0300 Subject: [PATCH 03/23] Switch aes-gcm implementation to openssl sodium's aes_gcm implementation is only available on amd64 chips with AESNI instructions; openssl's is more broadly supported. --- crypto/src/channel_encryption.cpp | 90 +++++++++++-------------------- httpserver/main.cpp | 7 +-- 2 files changed, 33 insertions(+), 64 deletions(-) diff --git a/crypto/src/channel_encryption.cpp b/crypto/src/channel_encryption.cpp index 4732c60b..5c9e785b 100644 --- a/crypto/src/channel_encryption.cpp +++ b/crypto/src/channel_encryption.cpp @@ -1,8 +1,11 @@ #include "channel_encryption.hpp" #include -#include -#include +#include +#include +#include +#include +#include #include #include "utils.hpp" @@ -89,6 +92,7 @@ std::string ChannelEncryption::decrypt(EncryptType type, std::string_view cipher static std::string encrypt_openssl( const EVP_CIPHER* cipher, + int taglen, std::basic_string_view plaintext, const std::array& key) { @@ -100,7 +104,7 @@ static std::string encrypt_openssl( // Start the output with the iv, then output space plus an extra possible 'blockSize' (according // to libssl docs) for the cipher data. const int ivLength = EVP_CIPHER_iv_length(cipher); - output.resize(ivLength + plaintext.size() + EVP_CIPHER_block_size(cipher)); + output.resize(ivLength + plaintext.size() + EVP_CIPHER_block_size(cipher) + taglen); auto* o = reinterpret_cast(output.data()); randombytes_buf(o, ivLength); const auto* iv = o; @@ -123,6 +127,11 @@ static std::string encrypt_openssl( } o += len; + // Add the tag, if applicable (e.g. aes-gcm) + if (taglen > 0 && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, taglen, o) <= 0) + throw std::runtime_error{"Failed to copy encryption tag"}; + o += taglen; + // Remove excess buffer space output.resize(reinterpret_cast(o) - output.data()); @@ -131,6 +140,7 @@ static std::string encrypt_openssl( static std::string decrypt_openssl( const EVP_CIPHER* cipher, + int taglen, std::basic_string_view ciphertext, const std::array& key) { @@ -142,6 +152,12 @@ static std::string decrypt_openssl( auto iv = ciphertext.substr(0, EVP_CIPHER_iv_length(cipher)); ciphertext.remove_prefix(iv.size()); + // We also append the tag (if applicable) so extract it: + if (ciphertext.size() < taglen) + throw std::runtime_error{"Encrypted value is too short"}; + auto tag = ciphertext.substr(ciphertext.size() - taglen); + ciphertext.remove_suffix(tag.size()); + // libssl docs say we need up to block size of extra buffer space: std::string output; output.resize(ciphertext.size() + EVP_CIPHER_block_size(cipher)); @@ -160,6 +176,9 @@ static std::string decrypt_openssl( } o += len; + if (!tag.empty() && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, taglen, (void*) tag.data()) <= 0) + throw std::runtime_error{"Could not set decryption tag"}; + // Decrypt any remaining partial blocks if (EVP_DecryptFinal_ex(ctx, o, &len) <= 0) { throw std::runtime_error("Could not finalise decryption"); @@ -175,7 +194,7 @@ static std::string decrypt_openssl( std::string ChannelEncryption::encrypt_cbc( std::string_view plaintext_, const x25519_pubkey& pubKey) const { return encrypt_openssl( - EVP_aes_256_cbc(), + EVP_aes_256_cbc(), 0, to_uchar(plaintext_), calculate_shared_secret(private_key_, pubKey)); } @@ -183,7 +202,7 @@ std::string ChannelEncryption::encrypt_cbc( std::string ChannelEncryption::decrypt_cbc( std::string_view ciphertext_, const x25519_pubkey& pubKey) const { return decrypt_openssl( - EVP_aes_256_cbc(), + EVP_aes_256_cbc(), 0, to_uchar(ciphertext_), calculate_shared_secret(private_key_, pubKey)); } @@ -191,64 +210,19 @@ std::string ChannelEncryption::decrypt_cbc( std::string ChannelEncryption::encrypt_gcm( std::string_view plaintext_, const x25519_pubkey& pubKey) const { - auto plaintext = to_uchar(plaintext_); - - const auto derived_key = derive_symmetric_key(private_key_, pubKey); - - // Output will be nonce(12B) || ciphertext || tag(16B) - std::string output; - output.resize( - crypto_aead_aes256gcm_NPUBBYTES + plaintext.size() + crypto_aead_aes256gcm_ABYTES); - auto* nonce = reinterpret_cast(output.data()); - randombytes_buf(nonce, crypto_aead_aes256gcm_NPUBBYTES); - - auto* ciphertext = nonce + crypto_aead_aes256gcm_NPUBBYTES; - unsigned long long ciphertext_len; - - crypto_aead_aes256gcm_encrypt( - ciphertext, &ciphertext_len, - plaintext.data(), plaintext.size(), - nullptr, 0, // ad, adlen - nullptr, // nsec (not used by aes256gcm) - nonce, - derived_key.data()); - - output.resize(crypto_aead_aes256gcm_NPUBBYTES + ciphertext_len); - return output; + return encrypt_openssl( + EVP_aes_256_gcm(), 16 /* tag length */, + to_uchar(plaintext_), + derive_symmetric_key(private_key_, pubKey)); } std::string ChannelEncryption::decrypt_gcm( std::string_view ciphertext_, const x25519_pubkey& pubKey) const { - const auto derived_key = derive_symmetric_key(private_key_, pubKey); - - auto ciphertext = to_uchar(ciphertext_); - - // Remove the nonce that we stick on the beginning: - auto nonce = ciphertext.substr(0, crypto_aead_aes256gcm_NPUBBYTES); - ciphertext.remove_prefix(nonce.size()); - - // Plaintext output will be ABYTES shorter than the ciphertext - std::string output; - output.resize(ciphertext.size() - crypto_aead_aes256gcm_ABYTES); - - auto outPtr = reinterpret_cast(&output[0]); - - unsigned long long decrypted_len; - if (int result = crypto_aead_aes256gcm_decrypt( - reinterpret_cast(output.data()), &decrypted_len, - nullptr, // nsec, always null for aes256gcm - ciphertext.data(), ciphertext.size(), - nullptr, 0, // ad, adlen - nonce.data(), - derived_key.data()); - result != 0) { - throw std::runtime_error("Could not decrypt (AES-GCM)"); - } - - assert(output.size() == decrypted_len); - - return output; + return decrypt_openssl( + EVP_aes_256_gcm(), 16 /* tag length */, + to_uchar(ciphertext_), + derive_symmetric_key(private_key_, pubKey)); } static std::array diff --git a/httpserver/main.cpp b/httpserver/main.cpp index 054acee4..289d0910 100644 --- a/httpserver/main.cpp +++ b/httpserver/main.cpp @@ -14,7 +14,7 @@ #include "lmq_server.h" #include "request_handler.h" -#include +#include #include #include @@ -119,11 +119,6 @@ int main(int argc, char* argv[]) { return EXIT_FAILURE; } - if (crypto_aead_aes256gcm_is_available() == 0) { - OXEN_LOG(error, "AES-256-GCM is not available on this CPU"); - return EXIT_FAILURE; - } - { const auto fd_limit = util::get_fd_limit(); if (fd_limit != -1) { From eb806b8188fc7cfba1475e5fe6998af80148aee8 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 18:16:51 -0300 Subject: [PATCH 04/23] Remove unused variables Avoids (fatal with default -Werror) warnings under clang. --- httpserver/http_connection.cpp | 2 -- httpserver/main.cpp | 2 -- httpserver/service_node.cpp | 3 --- 3 files changed, 7 deletions(-) diff --git a/httpserver/http_connection.cpp b/httpserver/http_connection.cpp index 0b8f67f5..969d89da 100644 --- a/httpserver/http_connection.cpp +++ b/httpserver/http_connection.cpp @@ -794,8 +794,6 @@ bool connection_t::parse_header(const char* first, Args... args) { return parse_header(first) && parse_header(args...); } -constexpr auto LONG_POLL_TIMEOUT = std::chrono::milliseconds(20000); - /// Move this out of `connection_t` Process client request /// Decouple responding from http diff --git a/httpserver/main.cpp b/httpserver/main.cpp index 289d0910..c8ed5eeb 100644 --- a/httpserver/main.cpp +++ b/httpserver/main.cpp @@ -34,8 +34,6 @@ extern "C" { namespace fs = std::filesystem; -constexpr int EXIT_INVALID_PORT = 2; - int main(int argc, char* argv[]) { oxen::command_line_parser parser; diff --git a/httpserver/service_node.cpp b/httpserver/service_node.cpp index 660256f3..c418c47d 100644 --- a/httpserver/service_node.cpp +++ b/httpserver/service_node.cpp @@ -35,9 +35,6 @@ namespace oxen { using storage::Item; -constexpr std::array RETRY_INTERVALS = { - 1s, 5s, 10s, 20s, 40s, 80s, 160s, 320s}; - constexpr std::chrono::milliseconds RELAY_INTERVAL = 350ms; static void make_sn_request(boost::asio::io_context& ioc, const sn_record_t& sn, From bb08064c7425a3f86875798ac627497486682302 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 18:17:19 -0300 Subject: [PATCH 05/23] Avoid clang copy warning & simplify --- httpserver/ip_utils.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/httpserver/ip_utils.cpp b/httpserver/ip_utils.cpp index 5e80eaf5..e0f843f4 100644 --- a/httpserver/ip_utils.cpp +++ b/httpserver/ip_utils.cpp @@ -47,10 +47,10 @@ std::array bogonRanges = { FromIPv4(0, 0, 0, 0, 8), static bool is_ip_public_inner(const uint32_t ip) { - for(const auto ipRange: bogonRanges) { - uint32_t netstart = (std::get<0>(ipRange) & std::get<1>(ipRange)); // first ip in subnet - uint32_t netend = (netstart | ~std::get<1>(ipRange)); // last ip in subnet - if ((ip >= netstart) && (ip <= netend)) + for(const auto& [block, netmask]: bogonRanges) { + uint32_t netstart = block & netmask; // first ip in subnet + uint32_t netend = netstart | ~netmask; // last ip in subnet + if (ip >= netstart && ip <= netend) return false; } return true; From 6b6476298ecfde42a1de9acf8043c6e976296199 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 22 Apr 2021 20:03:01 -0300 Subject: [PATCH 06/23] Update dependencies --- cmake/StaticBuild.cmake | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/cmake/StaticBuild.cmake b/cmake/StaticBuild.cmake index c6e549d2..42ea6bcd 100644 --- a/cmake/StaticBuild.cmake +++ b/cmake/StaticBuild.cmake @@ -5,18 +5,18 @@ set(LOCAL_MIRROR "" CACHE STRING "local mirror path/URL for lib downloads") -set(OPENSSL_VERSION 1.1.1i CACHE STRING "openssl version") +set(OPENSSL_VERSION 1.1.1k CACHE STRING "openssl version") set(OPENSSL_MIRROR ${LOCAL_MIRROR} https://www.openssl.org/source CACHE STRING "openssl download mirror(s)") set(OPENSSL_SOURCE openssl-${OPENSSL_VERSION}.tar.gz) -set(OPENSSL_HASH SHA256=e8be6a35fe41d10603c3cc635e93289ed00bf34b79671a3a4de64fcee00d5242 +set(OPENSSL_HASH SHA256=892a0875b9872acd04a9fde79b1f943075d5ea162415de3047c327df33fbaee5 CACHE STRING "openssl source hash") -set(BOOST_VERSION 1.75.0 CACHE STRING "boost version") +set(BOOST_VERSION 1.76.0 CACHE STRING "boost version") set(BOOST_MIRROR ${LOCAL_MIRROR} https://dl.bintray.com/boostorg/release/${BOOST_VERSION}/source CACHE STRING "boost download mirror(s)") string(REPLACE "." "_" BOOST_VERSION_ ${BOOST_VERSION}) set(BOOST_SOURCE boost_${BOOST_VERSION_}.tar.bz2) -set(BOOST_HASH SHA256=953db31e016db7bb207f11432bef7df100516eeb746843fa0486a222e3fd49cb +set(BOOST_HASH SHA256=f0397ba6e982c4450f27bf32a2a83292aba035b827a5623a14636ea583318c41 CACHE STRING "boost source hash") set(SODIUM_VERSION 1.0.18 CACHE STRING "libsodium version") @@ -28,18 +28,18 @@ set(SODIUM_SOURCE libsodium-${SODIUM_VERSION}.tar.gz) set(SODIUM_HASH SHA512=17e8638e46d8f6f7d024fe5559eccf2b8baf23e143fadd472a7d29d228b186d86686a5e6920385fe2020729119a5f12f989c3a782afbd05a8db4819bb18666ef CACHE STRING "libsodium source hash") -set(SQLITE3_VERSION 3340000 CACHE STRING "sqlite3 version") -set(SQLITE3_MIRROR ${LOCAL_MIRROR} https://www.sqlite.org/2020 +set(SQLITE3_VERSION 3350500 CACHE STRING "sqlite3 version") +set(SQLITE3_MIRROR ${LOCAL_MIRROR} https://www.sqlite.org/2021 CACHE STRING "sqlite3 download mirror(s)") set(SQLITE3_SOURCE sqlite-autoconf-${SQLITE3_VERSION}.tar.gz) -set(SQLITE3_HASH SHA512=75a1a2d86ab41354941b8574e780b1eae09c3c01f8da4b08f606b96962b80550f739ec7e9b1ceb07bba1cedced6d18a1408e4c10ff645eb1829d368ad308cf2f +set(SQLITE3_HASH SHA512=039af796f79fc4517be0bd5ba37886264d49da309e234ae6fccdb488ef0109ed2b917fc3e6c1fc7224dff4f736824c653aaf8f0a37550c5ebc14d035cb8ac737 CACHE STRING "sqlite3 source hash") -set(ZMQ_VERSION 4.3.3 CACHE STRING "libzmq version") +set(ZMQ_VERSION 4.3.4 CACHE STRING "libzmq version") set(ZMQ_MIRROR ${LOCAL_MIRROR} https://github.com/zeromq/libzmq/releases/download/v${ZMQ_VERSION} CACHE STRING "libzmq mirror(s)") set(ZMQ_SOURCE zeromq-${ZMQ_VERSION}.tar.gz) -set(ZMQ_HASH SHA512=4c18d784085179c5b1fcb753a93813095a12c8d34970f2e1bfca6499be6c9d67769c71c68b7ca54ff181b20390043170e89733c22f76ff1ea46494814f7095b1 +set(ZMQ_HASH SHA512=e198ef9f82d392754caadd547537666d4fba0afd7d027749b3adae450516bcf284d241d4616cad3cb4ad9af8c10373d456de92dc6d115b037941659f141e7c0e CACHE STRING "libzmq source hash") @@ -192,7 +192,7 @@ set(OPENSSL_VERSION 1.1.1) set(boost_threadapi "pthread") -set(boost_bootstrap_cxx "CXX=${deps_cxx}") +set(boost_bootstrap_cxx "--cxx=${deps_cxx}") set(boost_toolset "") set(boost_extra "") if(USE_LTO) @@ -226,8 +226,10 @@ if(APPLE AND CMAKE_OSX_DEPLOYMENT_TARGET) endif() set(boost_libs program_options system) +set(boost_with_libs_extra) if(BUILD_TESTS) list(APPEND boost_libs unit_test_framework) + set(boost_with_libs_extra "${boost_with_libs_extra} --with-unit_test_framework") endif() string(REPLACE ";" "," boost_with_libraries "${boost_libs}") set(boost_static_libraries) @@ -238,14 +240,15 @@ endforeach() build_external(boost # PATCH_COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/user-config.bjam tools/build/src/user-config.jam CONFIGURE_COMMAND - ${CMAKE_COMMAND} -E env ${boost_bootstrap_cxx} - ./bootstrap.sh --without-icu --prefix=${DEPS_DESTDIR} --with-toolset=${boost_toolset} - --with-libraries=${boost_with_libraries} - BUILD_COMMAND true + ./tools/build/src/engine/build.sh ${boost_toolset} ${boost_bootstrap_cxx} + BUILD_COMMAND + cp tools/build/src/engine/b2 . INSTALL_COMMAND ./b2 -d0 variant=release link=static runtime-link=static optimization=speed ${boost_extra} - threading=multi threadapi=${boost_threadapi} ${boost_buildflags} cxxstd=14 visibility=global + threading=multi threadapi=${boost_threadapi} ${boost_buildflags} cxxstd=17 visibility=global --disable-icu --user-config=${CMAKE_CURRENT_BINARY_DIR}/user-config.bjam + --prefix=${DEPS_DESTDIR} --exec-prefix=${DEPS_DESTDIR} --libdir=${DEPS_DESTDIR}/lib --includedir=${DEPS_DESTDIR}/include + --with-program_options --with-system ${boost_with_libs_extra} install BUILD_BYPRODUCTS ${boost_static_libraries} @@ -255,9 +258,9 @@ add_library(boost_core INTERFACE) add_dependencies(boost_core INTERFACE boost_external) target_include_directories(boost_core SYSTEM INTERFACE ${DEPS_DESTDIR}/include) add_library(Boost::boost ALIAS boost_core) -foreach(lib ${boost_libs}) - add_static_target(Boost::${lib} boost_external libboost_${lib}.a) - target_link_libraries(Boost::${lib} INTERFACE boost_core) +foreach(boostlib ${boost_libs}) + add_static_target(Boost::${boostlib} boost_external libboost_${boostlib}.a) + target_link_libraries(Boost::${boostlib} INTERFACE boost_core) endforeach() set(Boost_FOUND ON) set(Boost_VERSION ${BOOST_VERSION}) @@ -275,7 +278,7 @@ build_external(sodium) add_static_target(sodium sodium_external libsodium.a) -if(ZMQ_VERSION VERSION_LESS 4.3.4 AND CMAKE_CROSSCOMPILING AND ARCH_TRIPLET MATCHES mingw) +if(CMAKE_CROSSCOMPILING AND ARCH_TRIPLET MATCHES mingw) set(zmq_patch PATCH_COMMAND patch -p1 -i ${PROJECT_SOURCE_DIR}/utils/build_scripts/libzmq-mingw-closesocket.patch) endif() From 59ed42061a6476188654887512ea9e1f995da9cd Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 13:32:20 -0300 Subject: [PATCH 07/23] Actually apply the testing backoff max time --- httpserver/reachability_testing.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/httpserver/reachability_testing.cpp b/httpserver/reachability_testing.cpp index c079d10c..ae50b402 100644 --- a/httpserver/reachability_testing.cpp +++ b/httpserver/reachability_testing.cpp @@ -118,10 +118,12 @@ void reachability_testing::add_failing_node(const legacy_pubkey& pk, int previou using namespace std::chrono; if (previous_failures < 0) previous_failures = 0; - auto next_test = steady_clock::now() + duration_cast( + auto next_test_in = duration_cast( previous_failures * TESTING_BACKOFF + fseconds{TESTING_INTERVAL(util::rng())}); + if (next_test_in > TESTING_BACKOFF_MAX) + next_test_in = TESTING_BACKOFF_MAX; - failing_queue.emplace(pk, next_test, previous_failures + 1); + failing_queue.emplace(pk, steady_clock::now() + next_test_in, previous_failures + 1); } } // namespace oxen From eb786d255b69ed3ab0d6be694af04c0b955f6415 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 19:28:39 -0300 Subject: [PATCH 08/23] Only warn about missing pubkeys if above threshold Uses the same 3% threshold we use for going to the bootstrap node (also move the threshold ratio into a constant). --- httpserver/service_node.cpp | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/httpserver/service_node.cpp b/httpserver/service_node.cpp index 660256f3..3d3bfdb3 100644 --- a/httpserver/service_node.cpp +++ b/httpserver/service_node.cpp @@ -40,6 +40,11 @@ constexpr std::array RETRY_INTERVALS = { constexpr std::chrono::milliseconds RELAY_INTERVAL = 350ms; +// Threshold of missing data records at which we start warning and consult bootstrap nodes (mainly +// so that we don't bother producing warning spam or going to the bootstrap just for a few new nodes +// that will often have missing info for a few minutes). +using MISSING_PUBKEY_THRESHOLD = std::ratio<3, 100>; + static void make_sn_request(boost::asio::io_context& ioc, const sn_record_t& sn, std::shared_ptr req, http_callback_t&& cb) { @@ -130,6 +135,8 @@ parse_swarm_update(const std::string& response_body, bool from_json_rpc = false) const json service_node_states = result.at("service_node_states"); + int missing_aux_pks = 0, total = 0; + for (const auto& sn_json : service_node_states) { /// We want to include (test) decommissioned nodes, but not /// partially funded ones. @@ -137,14 +144,19 @@ parse_swarm_update(const std::string& response_body, bool from_json_rpc = false) continue; } + total++; + const auto& pk_hex = sn_json.at("service_node_pubkey").get_ref(); const auto& pk_x25519_hex = sn_json.at("pubkey_x25519").get_ref(); const auto& pk_ed25519_hex = sn_json.at("pubkey_ed25519").get_ref(); if (pk_x25519_hex.empty() || pk_ed25519_hex.empty()) { - // These will always either both be present or neither present - OXEN_LOG(warn, "ed25519/x25519 pubkeys are missing from service node info"); + // These will always either both be present or neither present. If they are missing + // there isn't much we can do: it means the remote hasn't transmitted them yet (or + // our local oxend hasn't received them yet). + missing_aux_pks++; + OXEN_LOG(debug, "ed25519/x25519 pubkeys are missing from service node info {}", pk_hex); continue; } @@ -152,8 +164,7 @@ parse_swarm_update(const std::string& response_body, bool from_json_rpc = false) sn_json.at("public_ip").get_ref(), sn_json.at("storage_port").get(), sn_json.at("storage_lmq_port").get(), - legacy_pubkey::from_hex( - sn_json.at("service_node_pubkey").get_ref()), + legacy_pubkey::from_hex(pk_hex), ed25519_pubkey::from_hex(pk_ed25519_hex), x25519_pubkey::from_hex(pk_x25519_hex)}; @@ -171,6 +182,12 @@ parse_swarm_update(const std::string& response_body, bool from_json_rpc = false) } } + if (missing_aux_pks > + MISSING_PUBKEY_THRESHOLD::num*total/MISSING_PUBKEY_THRESHOLD::den) { + OXEN_LOG(warn, "Missing ed25519/x25519 pubkeys for {}/{} service nodes; " + "oxend may be out of sync with the network", missing_aux_pks, total); + } + } catch (const std::exception& e) { OXEN_LOG(critical, "Bad oxend rpc response: invalid json ({})", e.what()); throw std::runtime_error("Failed to parse swarm update"); @@ -629,7 +646,8 @@ void ServiceNode::update_swarms() { auto [missing, total] = count_missing_data(bu); if (total >= (oxen::is_mainnet ? 100 : 10) - && missing < 3*total/100) { + && missing < + MISSING_PUBKEY_THRESHOLD::num*total/MISSING_PUBKEY_THRESHOLD::den) { OXEN_LOG(info, "Initialized from oxend with {}/{} SN records", total-missing, total); syncing_ = false; From 587ce89780456540a08694b46228c461a28802de Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 23:39:44 -0300 Subject: [PATCH 09/23] Fix master -> stable branch rename --- cmake/archive.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/archive.cmake b/cmake/archive.cmake index 8a0c80d7..293947fe 100644 --- a/cmake/archive.cmake +++ b/cmake/archive.cmake @@ -6,7 +6,7 @@ find_package(Git) set(git_tag "-unknown") if(GIT_FOUND) execute_process(COMMAND "${GIT_EXECUTABLE}" rev-parse HEAD RESULT_VARIABLE ret OUTPUT_VARIABLE curr_commit OUTPUT_STRIP_TRAILING_WHITESPACE) - execute_process(COMMAND "${GIT_EXECUTABLE}" rev-parse master RESULT_VARIABLE ret2 OUTPUT_VARIABLE stable_commit OUTPUT_STRIP_TRAILING_WHITESPACE) + execute_process(COMMAND "${GIT_EXECUTABLE}" rev-parse stable RESULT_VARIABLE ret2 OUTPUT_VARIABLE stable_commit OUTPUT_STRIP_TRAILING_WHITESPACE) if(NOT ret AND curr_commit STREQUAL "${stable_commit}") # Get the tag description; for a tagged release this will be just the tag (v1.2.3); for # something following a tag this will be something like "v1.2.3-2-abcdef" for something 2 From 8ecb53e369b4b344733f663338fb5453e0a08fe5 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 19:59:16 -0300 Subject: [PATCH 10/23] Updated required libsodium to 1.0.18, add download option We require 1.0.18 now, apparently, so bump the required version and add a -DDOWNLOAD_LIBSODIUM (copied from lokinet) that can download and build it when required (without needing to do a full static build of everything). --- .drone.jsonnet | 5 ++-- CMakeLists.txt | 27 +++++++++++++++---- cmake/DownloadLibSodium.cmake | 51 +++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 cmake/DownloadLibSodium.cmake diff --git a/.drone.jsonnet b/.drone.jsonnet index e63dc70d..91beca72 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -113,12 +113,13 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), debian_pipeline("Debian clang-11 (amd64)", "debian:sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), - debian_pipeline("Debian buster (i386)", "i386/debian:buster"), + debian_pipeline("Debian buster (i386)", "i386/debian:buster", cmake_extra='-DDOWNLOAD_SODIUM=ON'), debian_pipeline("Ubuntu focal (amd64)", "ubuntu:focal"), // ARM builds (ARM64 and armhf) debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), - debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false), + debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false, + cmake_extra='-DDOWNLOAD_SODIUM=ON'), // Static build (on bionic) which gets uploaded to oxen.rocks: debian_pipeline("Static (bionic amd64)", "ubuntu:bionic", deps='g++-8 '+static_build_deps, diff --git a/CMakeLists.txt b/CMakeLists.txt index 7f68de5e..6e069671 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,12 +59,29 @@ if(BUILD_STATIC_DEPS) include(StaticBuild) else() find_package(PkgConfig REQUIRED) - pkg_check_modules(SODIUM REQUIRED IMPORTED_TARGET libsodium>=1.0.17) - add_library(sodium INTERFACE) - target_link_libraries(sodium INTERFACE PkgConfig::SODIUM) - # Need this target export so that loki-mq properly picks up sodium - export(TARGETS sodium NAMESPACE sodium:: FILE sodium-exports.cmake) + if(NOT TARGET sodium) + # Allow -D DOWNLOAD_SODIUM=FORCE to download without even checking for a local libsodium + option(DOWNLOAD_SODIUM "Allow libsodium to be downloaded and built locally if not found on the system" OFF) + if(NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) + pkg_check_modules(SODIUM libsodium>=1.0.18 IMPORTED_TARGET) + endif() + + add_library(sodium INTERFACE) + if(SODIUM_FOUND AND NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) + target_link_libraries(sodium INTERFACE PkgConfig::SODIUM) + else() + if(NOT DOWNLOAD_SODIUM AND NOT BUILD_STATIC_DEPS) + message(FATAL_ERROR "Could not find libsodium >= 1.0.18; either install it on your system or use -DDOWNLOAD_SODIUM=ON to download and build an internal copy") + endif() + message(STATUS "Sodium >= 1.0.18 not found, but DOWNLOAD_SODIUM specified, so downloading it") + include(DownloadLibSodium) + target_link_libraries(sodium INTERFACE sodium_vendor) + endif() + + # Need this target export so that loki-mq properly picks up sodium + export(TARGETS sodium NAMESPACE sodium:: FILE sodium-exports.cmake) + endif() find_package(Boost REQUIRED system program_options) diff --git a/cmake/DownloadLibSodium.cmake b/cmake/DownloadLibSodium.cmake new file mode 100644 index 00000000..1b2ab84e --- /dev/null +++ b/cmake/DownloadLibSodium.cmake @@ -0,0 +1,51 @@ + +set(LIBSODIUM_PREFIX ${CMAKE_BINARY_DIR}/libsodium) +set(LIBSODIUM_URL https://github.com/jedisct1/libsodium/releases/download/1.0.18-RELEASE/libsodium-1.0.18.tar.gz https://download.libsodium.org/libsodium/releases/libsodium-1.0.18.tar.gz) +set(LIBSODIUM_HASH SHA512=17e8638e46d8f6f7d024fe5559eccf2b8baf23e143fadd472a7d29d228b186d86686a5e6920385fe2020729119a5f12f989c3a782afbd05a8db4819bb18666ef) + +if(SODIUM_TARBALL_URL) + # make a build time override of the tarball url so we can fetch it if the original link goes away + set(LIBSODIUM_URL ${SODIUM_TARBALL_URL}) +endif() + + +file(MAKE_DIRECTORY ${LIBSODIUM_PREFIX}/include) + +include(ExternalProject) +include(ProcessorCount) +ProcessorCount(PROCESSOR_COUNT) +if(PROCESSOR_COUNT EQUAL 0) + set(PROCESSOR_COUNT 1) +endif() + +set(sodium_cc ${CMAKE_C_COMPILER}) +if(CCACHE_PROGRAM) + set(sodium_cc "${CCACHE_PROGRAM} ${sodium_cc}") +endif() +set(SODIUM_CONFIGURE ./configure --prefix=${LIBSODIUM_PREFIX} --enable-static --disable-shared --with-pic --quiet CC=${sodium_cc}) +if (CMAKE_C_COMPILER_ARG1) + set(SODIUM_CONFIGURE ${SODIUM_CONFIGURE} CPPFLAGS=${CMAKE_C_COMPILER_ARG1}) +endif() + +if (CROSS_TARGET) + set(SODIUM_CONFIGURE ${SODIUM_CONFIGURE} --target=${CROSS_TARGET} --host=${CROSS_TARGET}) +endif() + + +ExternalProject_Add(libsodium_external + BUILD_IN_SOURCE ON + PREFIX ${LIBSODIUM_PREFIX} + URL ${LIBSODIUM_URL} + URL_HASH ${LIBSODIUM_HASH} + CONFIGURE_COMMAND ${SODIUM_CONFIGURE} + BUILD_COMMAND make -j${PROCESSOR_COUNT} + INSTALL_COMMAND ${MAKE} + BUILD_BYPRODUCTS ${LIBSODIUM_PREFIX}/lib/libsodium.a ${LIBSODIUM_PREFIX}/include + ) + +add_library(sodium_vendor STATIC IMPORTED GLOBAL) +add_dependencies(sodium_vendor libsodium_external) +set_target_properties(sodium_vendor PROPERTIES + IMPORTED_LOCATION ${LIBSODIUM_PREFIX}/lib/libsodium.a + INTERFACE_INCLUDE_DIRECTORIES ${LIBSODIUM_PREFIX}/include + ) From 281c0d8cb94c3aee9606d3cfe039354b39c37699 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 23:30:27 -0300 Subject: [PATCH 11/23] Buster deps fix (maybe?) --- .drone.jsonnet | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 91beca72..b7d39b69 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -113,13 +113,14 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), debian_pipeline("Debian clang-11 (amd64)", "debian:sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), - debian_pipeline("Debian buster (i386)", "i386/debian:buster", cmake_extra='-DDOWNLOAD_SODIUM=ON'), + debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps+' file', + cmake_extra='-DDOWNLOAD_SODIUM=ON'), debian_pipeline("Ubuntu focal (amd64)", "ubuntu:focal"), // ARM builds (ARM64 and armhf) debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false, - cmake_extra='-DDOWNLOAD_SODIUM=ON'), + cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps+' file'), // Static build (on bionic) which gets uploaded to oxen.rocks: debian_pipeline("Static (bionic amd64)", "ubuntu:bionic", deps='g++-8 '+static_build_deps, From 10b612861e7c419076decb1767a53208a9f008da Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 23 Apr 2021 23:37:26 -0300 Subject: [PATCH 12/23] Use our preloaded docker base images --- .drone.jsonnet | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index b7d39b69..bd0aee6d 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -4,6 +4,8 @@ local default_deps_base='libsystemd-dev libboost-program-options-dev libboost-sy local default_deps='g++ ' + default_deps_base; // g++ sometimes needs replacement local submodules_commands = ['git fetch --tags', 'git submodule update --init --recursive --depth=1']; +local docker_base = 'registry.oxen.rocks/lokinet-ci-'; + local submodules = { name: 'submodules', image: 'drone/git', @@ -12,7 +14,6 @@ local submodules = { local apt_get_quiet = 'apt-get -o=Dpkg::Use-Pty=0 -q'; - // Regular build on a debian-like system: local debian_pipeline(name, image, arch='amd64', @@ -109,13 +110,13 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op [ // Various debian builds - debian_pipeline("Debian (w/ tests) (amd64)", "debian:sid", lto=true, run_tests=true), + debian_pipeline("Debian (w/ tests) (amd64)", docker_base+"debian-sid", lto=true, run_tests=true), debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), - debian_pipeline("Debian clang-11 (amd64)", "debian:sid", deps='clang-11 '+default_deps_base, + debian_pipeline("Debian clang-11 (amd64)", docker_base+"debian-sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps+' file', cmake_extra='-DDOWNLOAD_SODIUM=ON'), - debian_pipeline("Ubuntu focal (amd64)", "ubuntu:focal"), + debian_pipeline("Ubuntu focal (amd64)", docker_base+"ubuntu-focal"), // ARM builds (ARM64 and armhf) debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), @@ -123,7 +124,7 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps+' file'), // Static build (on bionic) which gets uploaded to oxen.rocks: - debian_pipeline("Static (bionic amd64)", "ubuntu:bionic", deps='g++-8 '+static_build_deps, + debian_pipeline("Static (bionic amd64)", docker_base+"ubuntu-bionic", deps='g++-8 '+static_build_deps, cmake_extra='-DBUILD_STATIC_DEPS=ON -DCMAKE_C_COMPILER=gcc-8 -DCMAKE_CXX_COMPILER=g++-8', build_tests=false, lto=true, extra_cmds=static_check_and_upload), From a0ab97807954ae0c3e535fe1fce588697d5c40e1 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 00:07:40 -0300 Subject: [PATCH 13/23] Try more buster stuff --- .drone.jsonnet | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index bd0aee6d..f21e3df4 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -1,11 +1,11 @@ local default_deps_base='libsystemd-dev libboost-program-options-dev libboost-system-dev libboost-test-dev ' + 'libsqlite3-dev libsodium-dev libssl-dev pkg-config'; -local default_deps='g++ ' + default_deps_base; // g++ sometimes needs replacement - -local submodules_commands = ['git fetch --tags', 'git submodule update --init --recursive --depth=1']; +local default_deps_nocxx='libsodium-dev ' + default_deps_base; // libsodium-dev needs to be >= 1.0.18 +local default_deps='g++ ' + default_deps_nocxx; // g++ sometimes needs replacement local docker_base = 'registry.oxen.rocks/lokinet-ci-'; +local submodules_commands = ['git fetch --tags', 'git submodule update --init --recursive --depth=1']; local submodules = { name: 'submodules', image: 'drone/git', @@ -114,14 +114,14 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), debian_pipeline("Debian clang-11 (amd64)", docker_base+"debian-sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), - debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps+' file', + debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps_base+' g++ file', cmake_extra='-DDOWNLOAD_SODIUM=ON'), debian_pipeline("Ubuntu focal (amd64)", docker_base+"ubuntu-focal"), // ARM builds (ARM64 and armhf) debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false, - cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps+' file'), + cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps_base+' g++ file'), // Static build (on bionic) which gets uploaded to oxen.rocks: debian_pipeline("Static (bionic amd64)", docker_base+"ubuntu-bionic", deps='g++-8 '+static_build_deps, From 5c4a0321b1256bee924a131a7f6e50476ce0906a Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 00:07:50 -0300 Subject: [PATCH 14/23] Sync up sodium includes with lokinet --- .drone.jsonnet | 4 ++-- CMakeLists.txt | 64 +++++++++++++++++++++++++++++++------------------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index f21e3df4..dfab9694 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -114,14 +114,14 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), debian_pipeline("Debian clang-11 (amd64)", docker_base+"debian-sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), - debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps_base+' g++ file', + debian_pipeline("Debian buster (i386)", "i386/debian:buster", deps=default_deps_base+' g++ make file', cmake_extra='-DDOWNLOAD_SODIUM=ON'), debian_pipeline("Ubuntu focal (amd64)", docker_base+"ubuntu-focal"), // ARM builds (ARM64 and armhf) debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false, - cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps_base+' g++ file'), + cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps_base+' g++ make file'), // Static build (on bionic) which gets uploaded to oxen.rocks: debian_pipeline("Static (bionic amd64)", docker_base+"ubuntu-bionic", deps='g++-8 '+static_build_deps, diff --git a/CMakeLists.txt b/CMakeLists.txt index 6e069671..6b93c949 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,33 +59,32 @@ if(BUILD_STATIC_DEPS) include(StaticBuild) else() find_package(PkgConfig REQUIRED) + find_package(Boost REQUIRED system program_options) + find_package(OpenSSL REQUIRED) +endif() - if(NOT TARGET sodium) - # Allow -D DOWNLOAD_SODIUM=FORCE to download without even checking for a local libsodium - option(DOWNLOAD_SODIUM "Allow libsodium to be downloaded and built locally if not found on the system" OFF) - if(NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) - pkg_check_modules(SODIUM libsodium>=1.0.18 IMPORTED_TARGET) - endif() +if(NOT TARGET sodium) + # Allow -D DOWNLOAD_SODIUM=FORCE to download without even checking for a local libsodium + option(DOWNLOAD_SODIUM "Allow libsodium to be downloaded and built locally if not found on the system" OFF) + if(NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) + find_package(PkgConfig REQUIRED) + pkg_check_modules(SODIUM libsodium>=1.0.18 IMPORTED_TARGET) + endif() - add_library(sodium INTERFACE) - if(SODIUM_FOUND AND NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) - target_link_libraries(sodium INTERFACE PkgConfig::SODIUM) - else() - if(NOT DOWNLOAD_SODIUM AND NOT BUILD_STATIC_DEPS) - message(FATAL_ERROR "Could not find libsodium >= 1.0.18; either install it on your system or use -DDOWNLOAD_SODIUM=ON to download and build an internal copy") - endif() - message(STATUS "Sodium >= 1.0.18 not found, but DOWNLOAD_SODIUM specified, so downloading it") - include(DownloadLibSodium) - target_link_libraries(sodium INTERFACE sodium_vendor) + add_library(sodium INTERFACE) + if(SODIUM_FOUND AND NOT DOWNLOAD_SODIUM STREQUAL "FORCE" AND NOT BUILD_STATIC_DEPS) + target_link_libraries(sodium INTERFACE PkgConfig::SODIUM) + else() + if(NOT DOWNLOAD_SODIUM AND NOT BUILD_STATIC_DEPS) + message(FATAL_ERROR "Could not find libsodium >= 1.0.18; either install it on your system or use -DDOWNLOAD_SODIUM=ON to download and build an internal copy") endif() - - # Need this target export so that loki-mq properly picks up sodium - export(TARGETS sodium NAMESPACE sodium:: FILE sodium-exports.cmake) + message(STATUS "Sodium >= 1.0.18 not found, but DOWNLOAD_SODIUM specified, so downloading it") + include(DownloadLibSodium) + target_link_libraries(sodium INTERFACE sodium_vendor) endif() - find_package(Boost REQUIRED system program_options) - - find_package(OpenSSL REQUIRED) + # Need this target export so that loki-mq properly picks up sodium + export(TARGETS sodium NAMESPACE sodium:: FILE sodium-exports.cmake) endif() include(cmake/check_for_std_filesystem.cmake) @@ -98,8 +97,25 @@ add_subdirectory(utils) add_subdirectory(crypto) add_subdirectory(storage) add_subdirectory(httpserver) -set(BUILD_SHARED_LIBS OFF CACHE BOOL "disable shared libraries") # Tells loki-mq to do a static build -add_subdirectory(vendors/loki-mq) + + + +option(FORCE_OXENMQ_SUBMODULE "force using oxenmq submodule" OFF) +if(NOT FORCE_OXENMQ_SUBMODULE) + find_package(PkgConfig REQUIRED) + pkg_check_modules(OXENMQ liboxenmq>=1.2.5) +endif() +if(OXENMQ_FOUND) + add_library(oxenmq INTERFACE) + link_dep_libs(oxenmq INTERFACE "${OXENMQ_LIBRARY_DIRS}" ${OXENMQ_LIBRARIES}) + target_include_directories(oxenmq INTERFACE ${OXENMQ_INCLUDE_DIRS}) + add_library(oxenmq::oxenmq ALIAS oxenmq) + message(STATUS "Found system liboxenmq ${OXENMQ_VERSION}") +else() + message(STATUS "using oxenmq submodule") + set(BUILD_SHARED_LIBS OFF CACHE BOOL "disable shared libraries") # Tells oxen-mq to do a static build + add_subdirectory(vendors/loki-mq) +endif() if (BUILD_TESTS) add_subdirectory(unit_test) From 4e62418c059e2c68fb8994563140f7564426cf8e Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 00:19:35 -0300 Subject: [PATCH 15/23] Add simple test --- .drone.jsonnet | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.drone.jsonnet b/.drone.jsonnet index dfab9694..686a3d76 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -22,6 +22,7 @@ local debian_pipeline(name, image, lto=false, build_tests=true, run_tests=false, # Runs full test suite + test_oxen_storage=true, # Makes sure oxen-storage --version runs cmake_extra='', extra_cmds=[], extra_steps=[], @@ -54,6 +55,7 @@ local debian_pipeline(name, image, cmake_extra, 'ninja -j' + jobs + ' -v', ] + + (if test_oxen_storage then ['./httpserver/oxen-storage --version'] else []) + (if run_tests then ['./unit_test/Test'] else []) + extra_cmds, } From a3fce7978f437b98d15fe3d85ddfac1aee66d571 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 00:51:03 -0300 Subject: [PATCH 16/23] macos compilation fixes Missing headers + tricking macos into being happy. --- crypto/include/oxend_key.h | 1 + httpserver/http_connection.cpp | 2 +- httpserver/swarm.h | 1 + utils/include/utils.hpp | 9 ++++++--- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crypto/include/oxend_key.h b/crypto/include/oxend_key.h index 646ac5fb..fe2b1fe5 100644 --- a/crypto/include/oxend_key.h +++ b/crypto/include/oxend_key.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include diff --git a/httpserver/http_connection.cpp b/httpserver/http_connection.cpp index 969d89da..94a98e3c 100644 --- a/httpserver/http_connection.cpp +++ b/httpserver/http_connection.cpp @@ -848,7 +848,7 @@ void connection_t::process_client_req_rate_limited() { plaintext = std::move(plain_text)]( const error_code& ec) { self->request_handler_.process_client_req( - plaintext, [wself = std::weak_ptr{self}]( + plaintext, [wself = std::weak_ptr{self}]( oxen::Response res) { auto self = wself.lock(); if (!self) { diff --git a/httpserver/swarm.h b/httpserver/swarm.h index 210bded4..41b7e3f2 100644 --- a/httpserver/swarm.h +++ b/httpserver/swarm.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include "oxen_common.h" diff --git a/utils/include/utils.hpp b/utils/include/utils.hpp index cf0f1842..bec4a5cc 100644 --- a/utils/include/utils.hpp +++ b/utils/include/utils.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -43,10 +44,12 @@ inline bool starts_with(std::string_view str, std::string_view prefix) { } /// Joins [begin, end) with a delimiter and returns the resulting string. Elements can be anything -/// that can be sent to an ostream via `<<`. -template +/// that can be sent to an ostream via `<<`. The OSS template here is mainly to trick the compiler +/// (especially macos clang) into being happy with this include even when std::ostringstream isn't +/// yet available (and to put the include responsibility on the caller). +template std::string join(std::string_view delimiter, It begin, It end) { - std::ostringstream o; + OSS o; if (begin != end) o << *begin++; while (begin != end) From d3a4f35c27282a329f5eb6e4c80bd94d53784226 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 03:08:41 -0300 Subject: [PATCH 17/23] Namespace encrypt unit test to avoid macos symbol conflict --- unit_test/encrypt.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unit_test/encrypt.cpp b/unit_test/encrypt.cpp index f1631f2c..38b0da98 100644 --- a/unit_test/encrypt.cpp +++ b/unit_test/encrypt.cpp @@ -5,7 +5,7 @@ #include "oxend_key.h" #include "channel_encryption.hpp" -using namespace oxen; +namespace oxen { BOOST_AUTO_TEST_SUITE(encrypt) @@ -99,3 +99,5 @@ BOOST_AUTO_TEST_CASE(xchacha20) { } BOOST_AUTO_TEST_SUITE_END() + +} From dd9495596fbdcd78ba77b3a833c13aecaed46ab5 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 03:17:27 -0300 Subject: [PATCH 18/23] Build and run tests everywhere --- .drone.jsonnet | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 686a3d76..796c56d0 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -21,7 +21,7 @@ local debian_pipeline(name, image, build_type='Release', lto=false, build_tests=true, - run_tests=false, # Runs full test suite + run_tests=true, # Runs full test suite test_oxen_storage=true, # Makes sure oxen-storage --version runs cmake_extra='', extra_cmds=[], @@ -67,7 +67,8 @@ local mac_builder(name, build_type='Release', lto=false, build_tests=true, - run_tests=false, + run_tests=true, + test_oxen_storage=true, # Makes sure oxen-storage --version runs cmake_extra='', extra_cmds=[], extra_steps=[], @@ -94,6 +95,7 @@ local mac_builder(name, cmake_extra, 'ninja -j' + jobs + ' -v' ] + + (if test_oxen_storage then ['./httpserver/oxen-storage --version'] else []) + (if run_tests then ['./unit_test/Test'] else []) + extra_cmds, } @@ -112,7 +114,7 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op [ // Various debian builds - debian_pipeline("Debian (w/ tests) (amd64)", docker_base+"debian-sid", lto=true, run_tests=true), + debian_pipeline("Debian (amd64)", docker_base+"debian-sid", lto=true), debian_pipeline("Debian Debug (amd64)", "debian:sid", build_type='Debug'), debian_pipeline("Debian clang-11 (amd64)", docker_base+"debian-sid", deps='clang-11 '+default_deps_base, cmake_extra='-DCMAKE_C_COMPILER=clang-11 -DCMAKE_CXX_COMPILER=clang++-11 ', lto=true), @@ -121,18 +123,18 @@ local static_build_deps='autoconf automake make file libtool pkg-config patch op debian_pipeline("Ubuntu focal (amd64)", docker_base+"ubuntu-focal"), // ARM builds (ARM64 and armhf) - debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64", build_tests=false), - debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", build_tests=false, + debian_pipeline("Debian (ARM64)", "debian:sid", arch="arm64"), + debian_pipeline("Debian buster (armhf)", "arm32v7/debian:buster", arch="arm64", cmake_extra='-DDOWNLOAD_SODIUM=ON', deps=default_deps_base+' g++ make file'), // Static build (on bionic) which gets uploaded to oxen.rocks: debian_pipeline("Static (bionic amd64)", docker_base+"ubuntu-bionic", deps='g++-8 '+static_build_deps, cmake_extra='-DBUILD_STATIC_DEPS=ON -DCMAKE_C_COMPILER=gcc-8 -DCMAKE_CXX_COMPILER=g++-8', - build_tests=false, lto=true, extra_cmds=static_check_and_upload), + lto=true, extra_cmds=static_check_and_upload), // Macos builds: mac_builder('macOS (Static)', cmake_extra='-DBUILD_STATIC_DEPS=ON', - build_tests=false, lto=true, extra_cmds=static_check_and_upload), - mac_builder('macOS (Release)', run_tests=true), - mac_builder('macOS (Debug)', build_type='Debug', cmake_extra='-DBUILD_DEBUG_UTILS=ON'), + lto=true, extra_cmds=static_check_and_upload), + mac_builder('macOS (Release)'), + mac_builder('macOS (Debug)', build_type='Debug'), ] From 23a47cf34dda9105df77e9e6fb4f9abed73e4e35 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 24 Apr 2021 19:12:20 -0300 Subject: [PATCH 19/23] Fix serialize on 32-bit arches The size before the string was being serialized as a size_t, which differs on different arches; fixed it to a uin64_t so that it matches amd64. Also remove an undesirable `reserve()`: reserving the amount needed for an intermediate result tends to result in *more* allocations happening rather than less. Plus the reserve size was wrong (allocating only 4 bytes for the 8 byte size_t value). --- httpserver/serialization.cpp | 4 +--- unit_test/serialization.cpp | 12 +++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/httpserver/serialization.cpp b/httpserver/serialization.cpp index 71b331c0..127dddbb 100644 --- a/httpserver/serialization.cpp +++ b/httpserver/serialization.cpp @@ -19,9 +19,7 @@ static void serialize_integer(std::string& buf, T a) { } static void serialize(std::string& buf, const std::string& str) { - - buf.reserve(buf.size() + str.size() + 4); - serialize_integer(buf, str.size()); + serialize_integer(buf, str.size()); buf += str; } diff --git a/unit_test/serialization.cpp b/unit_test/serialization.cpp index 7627bca3..5bf84692 100644 --- a/unit_test/serialization.cpp +++ b/unit_test/serialization.cpp @@ -12,15 +12,25 @@ BOOST_AUTO_TEST_SUITE(serialization) BOOST_AUTO_TEST_CASE(it_serializes_and_deserializes) { const auto pub_key = - "054368520005786b249bcd461d28f75e560ea794014eeb17fcf6003f37d876783e"; + "054368520005786b249bcd461d28f75e560ea794014eeb17fcf6003f37d876783e"s; const auto data = "data"; const auto hash = "hash"; const uint64_t timestamp = 12345678; const uint64_t ttl = 3456000; message_t msg{pub_key, data, hash, ttl, timestamp}; + std::string msg_serialized; + serialize_message(msg_serialized, msg); + const auto expected_serialized = oxenmq::to_hex(pub_key) + + "040000000000000068617368" // size+hash + "040000000000000064617461" // size+data + "00bc340000000000" // ttl + "4e61bc0000000000" // timestamp + "0000000000000000"s; // nonce + BOOST_CHECK_EQUAL(oxenmq::to_hex(msg_serialized), expected_serialized); const std::vector inputs{msg, msg}; const std::vector batches = serialize_messages(inputs); BOOST_CHECK_EQUAL(batches.size(), 1); + BOOST_CHECK_EQUAL(oxenmq::to_hex(batches[0]), expected_serialized + expected_serialized); const auto messages = deserialize_messages(batches[0]); BOOST_CHECK_EQUAL(messages.size(), 2); From 9206e640a9c1abd8872bc26a0c461c944265fc40 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sun, 25 Apr 2021 20:12:34 -0300 Subject: [PATCH 20/23] Separate Database from boost::asio Database has a dependency on boost::asio so that it can set up a timer, but this is awkward as it couples the Database class with an implementation detail of the Database user. Fix this by removing it, making the cleanup timer callback the responsibility of the caller. This also fixes some spurious failures due to race conditions between the threads in the storage test code. --- httpserver/service_node.cpp | 7 ++++-- httpserver/service_node.h | 2 +- storage/include/Database.hpp | 24 +++++++++--------- storage/src/Database.cpp | 17 +++++-------- unit_test/storage.cpp | 49 ++++++++++-------------------------- 5 files changed, 37 insertions(+), 62 deletions(-) diff --git a/httpserver/service_node.cpp b/httpserver/service_node.cpp index c418c47d..a2841555 100644 --- a/httpserver/service_node.cpp +++ b/httpserver/service_node.cpp @@ -55,10 +55,10 @@ ServiceNode::ServiceNode( sn_record_t address, const legacy_seckey& skey, OxenmqServer& lmq_server, - const std::string& db_location, + const std::filesystem::path& db_location, const bool force_start) : ioc_(ioc), - db_(std::make_unique(ioc, db_location)), + db_(std::make_unique(db_location)), our_address_{std::move(address)}, our_seckey_{skey}, lmq_server_(lmq_server), @@ -72,6 +72,9 @@ ServiceNode::ServiceNode( this->syncing_ = false; #endif + lmq_server->add_timer([this] { std::lock_guard l{sn_mutex_}; db_->clean_expired(); }, + Database::CLEANUP_PERIOD); + lmq_server_->add_timer([this] { std::lock_guard l{sn_mutex_}; all_stats_.cleanup(); }, STATS_CLEANUP_INTERVAL); diff --git a/httpserver/service_node.h b/httpserver/service_node.h index d1b317e2..2eaf5738 100644 --- a/httpserver/service_node.h +++ b/httpserver/service_node.h @@ -186,7 +186,7 @@ class ServiceNode { sn_record_t address, const legacy_seckey& skey, OxenmqServer& omq_server, - const std::string& db_location, + const std::filesystem::path& db_location, bool force_start); // Return info about this node as it is advertised to other nodes diff --git a/storage/include/Database.hpp b/storage/include/Database.hpp index 5db03657..2d5410f8 100644 --- a/storage/include/Database.hpp +++ b/storage/include/Database.hpp @@ -4,24 +4,26 @@ #include "oxen_common.h" #include +#include #include #include #include #include -#include - struct sqlite3; struct sqlite3_stmt; namespace oxen { -constexpr auto DB_CLEANUP_PERIOD = std::chrono::seconds(10); - +// Storage database class. class Database { public: - Database(boost::asio::io_context& ioc, const std::string& db_path, - std::chrono::milliseconds cleanup_period = DB_CLEANUP_PERIOD); + // Recommended period for calling clean_expired() + inline static constexpr auto CLEANUP_PERIOD = 10s; + + // Constructor. Note that you *must* also set up a timer that runs periodically (every + // CLEANUP_PERIOD is recommended) and calls clean_expired(). + explicit Database(const std::filesystem::path& db_path); ~Database(); enum class DuplicateHandling { IGNORE, FAIL }; @@ -46,12 +48,13 @@ class Database { // Get message by `msg_hash`, return true if found bool retrieve_by_hash(const std::string& msg_hash, storage::Item& item); + // Removes expired messages from the database; the Database owner should call this periodically. + void clean_expired(); + private: sqlite3_stmt* prepare_statement(const std::string& query); - void open_and_prepare(const std::string& db_path); - void perform_cleanup(); + void open_and_prepare(const std::filesystem::path& db_path); - private: sqlite3* db; sqlite3_stmt* save_stmt; sqlite3_stmt* save_or_ignore_stmt; @@ -62,9 +65,6 @@ class Database { sqlite3_stmt* get_by_index_stmt; sqlite3_stmt* get_by_hash_stmt; sqlite3_stmt* delete_expired_stmt; - - const std::chrono::milliseconds cleanup_period; - boost::asio::steady_timer cleanup_timer_; }; } // namespace oxen diff --git a/storage/src/Database.cpp b/storage/src/Database.cpp index 2047b3fb..920e922b 100644 --- a/storage/src/Database.cpp +++ b/storage/src/Database.cpp @@ -19,15 +19,13 @@ Database::~Database() { sqlite3_close(db); } -Database::Database(boost::asio::io_context& ioc, const std::string& db_path, - std::chrono::milliseconds cleanup_period) - : cleanup_period(cleanup_period), cleanup_timer_(ioc) { +Database::Database(const std::filesystem::path& db_path) { open_and_prepare(db_path); - perform_cleanup(); + clean_expired(); } -void Database::perform_cleanup() { +void Database::clean_expired() { const auto now_ms = util::get_time_ms(); sqlite3_bind_int64(delete_expired_stmt, 1, now_ms); @@ -51,9 +49,6 @@ void Database::perform_cleanup() { if (reset_rc != SQLITE_OK && reset_rc != rc) { fprintf(stderr, "sql error: unexpected value from sqlite3_reset"); } - - cleanup_timer_.expires_after(this->cleanup_period); - cleanup_timer_.async_wait(std::bind(&Database::perform_cleanup, this)); } sqlite3_stmt* Database::prepare_statement(const std::string& query) { @@ -139,9 +134,9 @@ static void check_page_size(sqlite3* db) { } } -void Database::open_and_prepare(const std::string& db_path) { - const std::string file_path = db_path + "/storage.db"; - int rc = sqlite3_open_v2(file_path.c_str(), &db, +void Database::open_and_prepare(const std::filesystem::path& db_path) { + const auto file_path = db_path / "storage.db"; + int rc = sqlite3_open_v2(file_path.u8string().c_str(), &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL); diff --git a/unit_test/storage.cpp b/unit_test/storage.cpp index 8728065e..4c2b7da3 100644 --- a/unit_test/storage.cpp +++ b/unit_test/storage.cpp @@ -33,8 +33,7 @@ BOOST_AUTO_TEST_SUITE(storage) BOOST_AUTO_TEST_CASE(it_creates_the_database_file) { StorageRAIIFixture fixture; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; BOOST_CHECK(std::filesystem::exists("storage.db")); } @@ -48,15 +47,13 @@ BOOST_AUTO_TEST_CASE(it_stores_data_persistently) { const uint64_t ttl = 123456; const uint64_t timestamp = util::get_time_ms(); { - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; BOOST_CHECK(storage.store(hash, pubkey, bytes, ttl, timestamp, nonce)); // the database is closed when storage goes out of scope } { // re-open the database - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; std::vector items; const auto lastHash = ""; @@ -82,8 +79,7 @@ BOOST_AUTO_TEST_CASE(it_returns_false_when_storing_existing_hash) { const uint64_t ttl = 123456; const uint64_t timestamp = util::get_time_ms(); - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; BOOST_CHECK(storage.store(hash, pubkey, bytes, ttl, timestamp, nonce)); // store using the same hash, FAIL is default behaviour @@ -102,8 +98,7 @@ BOOST_AUTO_TEST_CASE( const uint64_t ttl = 123456; const uint64_t timestamp = util::get_time_ms(); - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; BOOST_CHECK(storage.store(hash, pubkey, bytes, ttl, timestamp, nonce)); // store using the same hash @@ -114,8 +109,7 @@ BOOST_AUTO_TEST_CASE( BOOST_AUTO_TEST_CASE(it_only_returns_entries_for_specified_pubkey) { StorageRAIIFixture fixture; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; BOOST_CHECK(storage.store("hash0", "mypubkey", "bytesasstring0", 100000, util::get_time_ms(), "nonce")); @@ -142,8 +136,7 @@ BOOST_AUTO_TEST_CASE(it_only_returns_entries_for_specified_pubkey) { BOOST_AUTO_TEST_CASE(it_returns_entries_older_than_lasthash) { StorageRAIIFixture fixture; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; const size_t num_entries = 100; for (size_t i = 0; i < num_entries; i++) { @@ -176,14 +169,7 @@ BOOST_AUTO_TEST_CASE(it_removes_expired_entries) { const auto pubkey = "mypubkey"; - boost::asio::io_context ioc; - - Database storage(ioc, ".", 100ms); - - /// Note: `Database` is not thread safe - /// and not meant to be used in this way; - /// However, it should be fine for tests - std::thread t([&]() { ioc.run(); }); + Database storage{"."}; BOOST_CHECK(storage.store("hash0", pubkey, "bytesasstring0", 100000, util::get_time_ms(), "nonce")); @@ -195,11 +181,8 @@ BOOST_AUTO_TEST_CASE(it_removes_expired_entries) { BOOST_CHECK(storage.retrieve(pubkey, items, lastHash)); BOOST_CHECK_EQUAL(items.size(), 2); } - // the timer kicks in every 10 seconds - // give 100ms to perform the cleanup - std::cout << "waiting for cleanup timer..." << std::endl; - std::this_thread::sleep_for(100ms + 100ms); - + std::this_thread::sleep_for(5ms); + storage.clean_expired(); { std::vector items; const auto lastHash = ""; @@ -207,9 +190,6 @@ BOOST_AUTO_TEST_CASE(it_removes_expired_entries) { BOOST_CHECK_EQUAL(items.size(), 1); BOOST_CHECK_EQUAL(items[0].hash, "hash0"); } - - ioc.stop(); - t.join(); } BOOST_AUTO_TEST_CASE(it_stores_data_in_bulk) { @@ -223,8 +203,7 @@ BOOST_AUTO_TEST_CASE(it_stores_data_in_bulk) { const size_t num_items = 100; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; // bulk store { @@ -257,8 +236,7 @@ BOOST_AUTO_TEST_CASE(it_stores_data_in_bulk_even_when_overlaps) { const size_t num_items = 100; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; // insert existing BOOST_CHECK(storage.store("0", pubkey, bytes, ttl, timestamp, nonce)); @@ -286,8 +264,7 @@ BOOST_AUTO_TEST_CASE(it_stores_data_in_bulk_even_when_overlaps) { BOOST_AUTO_TEST_CASE(it_checks_the_retrieve_limit_works) { StorageRAIIFixture fixture; - boost::asio::io_context ioc; - Database storage(ioc, "."); + Database storage{"."}; const size_t num_entries = 100; for (size_t i = 0; i < num_entries; i++) { From 53536c50ba930f89e7ae282453417859e4f76110 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sun, 25 Apr 2021 20:40:38 -0300 Subject: [PATCH 21/23] Fix arg for boost unit test static build --- cmake/StaticBuild.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/StaticBuild.cmake b/cmake/StaticBuild.cmake index 42ea6bcd..36c042ba 100644 --- a/cmake/StaticBuild.cmake +++ b/cmake/StaticBuild.cmake @@ -229,7 +229,7 @@ set(boost_libs program_options system) set(boost_with_libs_extra) if(BUILD_TESTS) list(APPEND boost_libs unit_test_framework) - set(boost_with_libs_extra "${boost_with_libs_extra} --with-unit_test_framework") + list(APPEND boost_with_libs_extra --with-test) endif() string(REPLACE ";" "," boost_with_libraries "${boost_libs}") set(boost_static_libraries) From c2b73ce278d326d3a9eff1cbb98a32f646e50f00 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 26 Apr 2021 12:11:41 -0300 Subject: [PATCH 22/23] Bump SS version to 2.1.1 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6b93c949..7670cf2e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,7 +13,7 @@ endif() cmake_minimum_required(VERSION 3.10) project(storage_server - VERSION 2.1.0 + VERSION 2.1.1 LANGUAGES CXX C) option(INTEGRATION_TEST "build for integration test" OFF) From c434ebb0534c9159a35602398e8de1b39bc58741 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 27 Apr 2021 11:31:28 -0300 Subject: [PATCH 23/23] Squelch duplicate swarm updates Don't fire another swarm update if we already have one in progress; this avoids, in particular, firing tons of unnecessary get_service_nodes at oxend when oxend is in the middle of syncing. --- httpserver/service_node.cpp | 6 ++++++ httpserver/service_node.h | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/httpserver/service_node.cpp b/httpserver/service_node.cpp index fe6e8ebf..643dc3d1 100644 --- a/httpserver/service_node.cpp +++ b/httpserver/service_node.cpp @@ -593,6 +593,11 @@ void ServiceNode::relay_buffered_messages() { void ServiceNode::update_swarms() { + if (updating_swarms_.exchange(true)) { + OXEN_LOG(debug, "Swarm update already in progress, not sending another update request"); + return; + } + std::lock_guard guard(sn_mutex_); OXEN_LOG(debug, "Swarm update triggered"); @@ -618,6 +623,7 @@ void ServiceNode::update_swarms() { lmq_server_.oxend_request("rpc.get_service_nodes", [this](bool success, std::vector data) { + updating_swarms_ = false; if (!success || data.size() < 2) { OXEN_LOG(critical, "Failed to contact local oxend for service node list"); return; diff --git a/httpserver/service_node.h b/httpserver/service_node.h index 2eaf5738..b9f8780f 100644 --- a/httpserver/service_node.h +++ b/httpserver/service_node.h @@ -102,6 +102,11 @@ class ServiceNode { std::atomic oxend_pings_ = 0; // Consecutive successful pings, used for batching logs about it + // Will be set to true while we have an outstanding update_swarms() call so that we squelch + // other update_swarms() until it finishes (or fails), to avoid spamming oxend (particularly + // when syncing when we get tons of block notifications quickly). + std::atomic updating_swarms_ = false; + reachability_testing reach_records_; /// Container for recently received messages directly from