diff --git a/configure.ac b/configure.ac index 2f27a2aadc..b01cf5af8d 100644 --- a/configure.ac +++ b/configure.ac @@ -82,10 +82,8 @@ fi AC_PROG_OBJCXX ]) -dnl Since libtool 1.5.2 (released 2004-01-25), on Linux libtool no longer -dnl sets RPATH for any directories in the dynamic linker search path. -dnl See more: https://wiki.debian.org/RpathIssue -LT_PREREQ([1.5.2]) +dnl OpenBSD ships with 2.4.2 +LT_PREREQ([2.4.2]) dnl Libtool init checks. LT_INIT([pic-only win32-dll]) @@ -1243,7 +1241,7 @@ dnl Do not change "-I/usr/include" to "-isystem /usr/include" because that dnl is not necessary (/usr/include is already a system directory) and because dnl it would break GCC's #include_next. AC_DEFUN([SUPPRESS_WARNINGS], - [$(echo $1 |${SED} -E -e 's/(^| )-I/\1-isystem /g' -e 's;-isystem /usr/include([/ ]|$);-I/usr/include\1;g')]) + [[$(echo $1 |${SED} -E -e 's/(^| )-I/\1-isystem /g' -e 's;-isystem /usr/include/*( |$);-I/usr/include\1;g')]]) dnl enable-fuzz should disable all other targets if test "x$enable_fuzz" = "xyes"; then diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 30e04d9c8d..a5f23e7754 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -167,7 +167,7 @@ class TestSymbolChecks(unittest.TestCase): executable = 'test3.exe' with open(source, 'w', encoding="utf8") as f: f.write(''' - #include + #include int main() { diff --git a/depends/hosts/openbsd.mk b/depends/hosts/openbsd.mk index dc8393e04c..5988f24bff 100644 --- a/depends/hosts/openbsd.mk +++ b/depends/hosts/openbsd.mk @@ -1,11 +1,11 @@ openbsd_CFLAGS=-pipe -openbsd_CFLAGS_CXXFLAGS=$(openbsd_CFLAGS) +openbsd_CXXFLAGS=$(openbsd_CFLAGS) -openbsd_CFLAGS_release_CFLAGS=-O2 -openbsd_CFLAGS_release_CXXFLAGS=$(openbsd_release_CFLAGS) +openbsd_release_CFLAGS=-O2 +openbsd_release_CXXFLAGS=$(openbsd_release_CFLAGS) -openbsd_CFLAGS_debug_CFLAGS=-O1 -openbsd_CFLAGS_debug_CXXFLAGS=$(openbsd_debug_CFLAGS) +openbsd_debug_CFLAGS=-O1 +openbsd_debug_CXXFLAGS=$(openbsd_debug_CFLAGS) ifeq (86,$(findstring 86,$(build_arch))) i686_openbsd_CC=clang -m32 diff --git a/depends/packages/libevent.mk b/depends/packages/libevent.mk index 748ed510c1..1efe6220d3 100644 --- a/depends/packages/libevent.mk +++ b/depends/packages/libevent.mk @@ -36,5 +36,6 @@ define $(package)_stage_cmds endef define $(package)_postprocess_cmds - rm lib/*.la + rm lib/*.la && \ + rm include/ev*.h endef diff --git a/depends/patches/qt/mac-qmake.conf b/depends/patches/qt/mac-qmake.conf index e4bfaa1463..543407f853 100644 --- a/depends/patches/qt/mac-qmake.conf +++ b/depends/patches/qt/mac-qmake.conf @@ -19,6 +19,4 @@ QMAKE_MAC_SDK.macosx.PlatformPath = /phony !host_build: QMAKE_LFLAGS += -target $${MAC_TARGET} QMAKE_AR = $${CROSS_COMPILE}ar cq QMAKE_RANLIB=$${CROSS_COMPILE}ranlib -QMAKE_LIBTOOL=$${CROSS_COMPILE}libtool -QMAKE_INSTALL_NAME_TOOL=$${CROSS_COMPILE}install_name_tool load(qt_config) diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index a61fcd0871..7525cca863 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -37,7 +37,7 @@ The following dependencies are **optional**: ## Preparation ### 1. Install Required Dependencies -Install the required dependencies the usual way you [install software on FreeBSD](https://www.freebsd.org/doc/en/books/handbook/ports.html) - either with `pkg` or via the Ports collection. The example commands below use `pkg` which is usually run as `root` or via `sudo`. If you want to use `sudo`, and you haven't set it up: [use this guide](http://www.freebsdwiki.net/index.php/Sudo%2C_configuring) to setup `sudo` access on FreeBSD. +Run the following as root to install the base dependencies for building. ```bash pkg install autoconf automake boost-libs git gmake libevent libtool pkgconf diff --git a/src/Makefile.test.include b/src/Makefile.test.include index bbd5543d95..77e5127fcc 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -398,8 +398,19 @@ all-local: $(FUZZ_BINARY) endif %.cpp.test: %.cpp - @echo Running tests: `cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1` from $< - $(AM_V_at)$(TEST_BINARY) --catch_system_errors=no -l test_suite -t "`cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" -- DEBUG_LOG_OUT > $<.log 2>&1 || (cat $<.log && false) + @echo Running tests: $$(\ + cat $< | \ + grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | \ + cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1\ + ) from $< + $(AM_V_at)export TEST_LOGFILE=$(abs_builddir)/$$(\ + echo $< | grep -E -o "(wallet/test/.*\.cpp|test/.*\.cpp)" | $(SED) -e s/\.cpp/.log/ \ + ) && \ + $(TEST_BINARY) --catch_system_errors=no -l test_suite -t "$$(\ + cat $< | \ + grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | \ + cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1\ + )" -- DEBUG_LOG_OUT > "$$TEST_LOGFILE" 2>&1 || (cat "$$TEST_LOGFILE" && false) test/data/%.json.h: test/data/%.json @$(MKDIR_P) $(@D) diff --git a/src/addrman.h b/src/addrman.h index 7f28962595..7debddb842 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -58,6 +58,7 @@ private: mutable int nRandomPos{-1}; friend class CAddrMan; + friend class CAddrManDeterministic; public: @@ -808,6 +809,7 @@ private: void ResetI2PPorts() EXCLUSIVE_LOCKS_REQUIRED(cs); friend class CAddrManTest; + friend class CAddrManDeterministic; }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index ea773505ea..39ca3b04b5 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -334,6 +334,17 @@ void BitcoinApplication::requestShutdown() window->setClientModel(nullptr); pollShutdownTimer->stop(); +#ifdef ENABLE_WALLET + // Delete wallet controller here manually, instead of relying on Qt object + // tracking (https://doc.qt.io/qt-5/objecttrees.html). This makes sure + // walletmodel m_handle_* notification handlers are deleted before wallets + // are unloaded, which can simplify wallet implementations. It also avoids + // these notifications having to be handled while GUI objects are being + // destroyed, making GUI code less fragile as well. + delete m_wallet_controller; + m_wallet_controller = nullptr; +#endif // ENABLE_WALLET + delete clientModel; clientModel = nullptr; @@ -414,6 +425,16 @@ WId BitcoinApplication::getMainWinId() const return window->winId(); } +bool BitcoinApplication::event(QEvent* e) +{ + if (e->type() == QEvent::Quit) { + requestShutdown(); + return true; + } + + return QApplication::event(e); +} + static void SetupUIArgs(ArgsManager& argsman) { argsman.AddArg("-choosedatadir", strprintf(QObject::tr("Choose data directory on startup (default: %u)").toStdString(), DEFAULT_CHOOSE_DATADIR), ArgsManager::ALLOW_ANY, OptionsCategory::GUI); diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h index fccaabcb45..72c42e274d 100644 --- a/src/qt/bitcoin.h +++ b/src/qt/bitcoin.h @@ -98,6 +98,9 @@ Q_SIGNALS: void splashFinished(); void windowShown(BitcoinGUI* window); +protected: + bool event(QEvent* e) override; + private: QThread *coreThread; interfaces::Node& m_node; diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 94d03a3d2f..815553a0e0 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -213,6 +213,9 @@ + + Maximum database cache size. A larger cache can contribute to faster sync, after which the benefit is less pronounced for most use cases. Lowering the cache size will reduce memory usage. Unused mempool memory is shared for this cache. + Size of &database cache @@ -256,6 +259,9 @@ + + Set the number of script verification threads. Negative values correspond to the number of cores you want to leave free to the system. + Number of script &verification threads diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index 9b8933c943..c567e3d906 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -114,7 +114,7 @@ void ModalOverlay::tipUpdate(int count, const QDateTime& blockDate, double nVeri if (sample.first < (currentDate.toMSecsSinceEpoch() - 500 * 1000) || i == blockProcessTime.size() - 1) { progressDelta = blockProcessTime[0].second - sample.second; timeDelta = blockProcessTime[0].first - sample.first; - progressPerHour = progressDelta / (double) timeDelta * 1000 * 3600; + progressPerHour = (progressDelta > 0) ? progressDelta / (double)timeDelta * 1000 * 3600 : 0; remainingMSecs = (progressDelta > 0) ? remainingProgress / progressDelta * timeDelta : -1; break; } diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index c6ae2e9e89..d98a03a633 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -225,10 +225,11 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes UniValue subelement; if (lastResult.isArray()) { - for(char argch: curarg) - if (!IsDigit(argch)) - throw std::runtime_error("Invalid result query"); - subelement = lastResult[LocaleIndependentAtoi(curarg)]; + const auto parsed{ToIntegral(curarg)}; + if (!parsed) { + throw std::runtime_error("Invalid result query"); + } + subelement = lastResult[parsed.value()]; } else if (lastResult.isObject()) subelement = find_value(lastResult, curarg); diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 8b435431dd..71bda68a7a 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -26,6 +26,12 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) const T i = fuzzed_data_provider.ConsumeIntegral(); const T j = fuzzed_data_provider.ConsumeIntegral(); const bool is_addition_overflow_custom = AdditionOverflow(i, j); + const auto maybe_add{CheckedAdd(i, j)}; + const auto sat_add{SaturatingAdd(i, j)}; + assert(is_addition_overflow_custom == !maybe_add.has_value()); + assert(is_addition_overflow_custom == AdditionOverflow(j, i)); + assert(maybe_add == CheckedAdd(j, i)); + assert(sat_add == SaturatingAdd(j, i)); #if defined(HAVE_BUILTIN_ADD_OVERFLOW) T result_builtin; const bool is_addition_overflow_builtin = __builtin_add_overflow(i, j, &result_builtin); @@ -33,11 +39,14 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) if (!is_addition_overflow_custom) { assert(i + j == result_builtin); } -#else - if (!is_addition_overflow_custom) { - (void)(i + j); - } #endif + if (is_addition_overflow_custom) { + assert(sat_add == std::numeric_limits::min() || sat_add == std::numeric_limits::max()); + } else { + const auto add{i + j}; + assert(add == maybe_add.value()); + assert(add == sat_add); + } } } // namespace diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index cf91434f8e..d819444b1b 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -25,10 +26,200 @@ void initialize_addrman() class CAddrManDeterministic : public CAddrMan { public: - void MakeDeterministic(const uint256& random_seed) + FuzzedDataProvider& m_fuzzed_data_provider; + + explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider) + : m_fuzzed_data_provider(fuzzed_data_provider) { - WITH_LOCK(cs, insecure_rand = FastRandomContext{random_seed}); - Clear(); + WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); + if (fuzzed_data_provider.ConsumeBool()) { + m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(m_asmap)) { + m_asmap.clear(); + } + } + } + + /** + * Generate a random address. Always returns a valid address. + */ + CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(cs) + { + CNetAddr addr; + if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(m_fuzzed_data_provider); + } else { + // The networks [1..6] correspond to CNetAddr::BIP155Network (private). + static const std::map net_len_map = {{1, ADDR_IPV4_SIZE}, + {2, ADDR_IPV6_SIZE}, + {4, ADDR_TORV3_SIZE}, + {5, ADDR_I2P_SIZE}, + {6, ADDR_CJDNS_SIZE}}; + uint8_t net = insecure_rand.randrange(5) + 1; // [1..5] + if (net == 3) { + net = 6; + } + + CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); + + s << net; + s << insecure_rand.randbytes(net_len_map.at(net)); + + s >> addr; + } + + // Return a dummy IPv4 5.5.5.5 if we generated an invalid address. + if (!addr.IsValid()) { + in_addr v4_addr = {}; + v4_addr.s_addr = 0x05050505; + addr = CNetAddr{v4_addr}; + } + + return addr; + } + + /** + * Fill this addrman with lots of addresses from lots of sources. + */ + void Fill() + { + LOCK(cs); + + // Add some of the addresses directly to the "tried" table. + + // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% + const size_t n = m_fuzzed_data_provider.ConsumeIntegralInRange(0, 3); + + const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(10, 50); + CNetAddr prev_source; + // Use insecure_rand inside the loops instead of m_fuzzed_data_provider because when + // the latter is exhausted it just returns 0. + for (size_t i = 0; i < num_sources; ++i) { + const auto source = RandAddr(); + const size_t num_addresses = insecure_rand.randrange(500) + 1; // [1..500] + + for (size_t j = 0; j < num_addresses; ++j) { + const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK}; + const auto time_penalty = insecure_rand.randrange(100000001); +#if 1 + // 2.83 sec to fill. + if (n > 0 && mapInfo.size() % n == 0 && mapAddr.find(addr) == mapAddr.end()) { + // Add to the "tried" table (if the bucket slot is free). + const CAddrInfo dummy{addr, source}; + const int bucket = dummy.GetTriedBucket(nKey, m_asmap); + const int bucket_pos = dummy.GetBucketPosition(nKey, false, bucket); + if (vvTried[bucket][bucket_pos] == -1) { + int id; + CAddrInfo* addr_info = Create(addr, source, &id); + vvTried[bucket][bucket_pos] = id; + addr_info->fInTried = true; + ++nTried; + } + } else { + // Add to the "new" table. + Add_(addr, source, time_penalty); + } +#else + // 261.91 sec to fill. + Add_(addr, source, time_penalty); + if (n > 0 && mapInfo.size() % n == 0) { + Good_(addr, false, GetTime()); + } +#endif + // Add 10% of the addresses from more than one source. + if (insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { + Add_(addr, prev_source, time_penalty); + } + } + prev_source = source; + } + } + + /** + * Compare with another AddrMan. + * This compares: + * - the values in `mapInfo` (the keys aka ids are ignored) + * - vvNew entries refer to the same addresses + * - vvTried entries refer to the same addresses + */ + bool operator==(const CAddrManDeterministic& other) + { + LOCK2(cs, other.cs); + + if (mapInfo.size() != other.mapInfo.size() || nNew != other.nNew || + nTried != other.nTried) { + return false; + } + + // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. + // Keys may be different. + + using CAddrInfoHasher = std::function; + using CAddrInfoEq = std::function; + + CNetAddrHash netaddr_hasher; + + CAddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const CAddrInfo& a) { + return netaddr_hasher(static_cast(a)) ^ netaddr_hasher(a.source) ^ + a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried; + }; + + CAddrInfoEq addrinfo_eq = [](const CAddrInfo& lhs, const CAddrInfo& rhs) { + return static_cast(lhs) == static_cast(rhs) && + lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess && + lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount && + lhs.fInTried == rhs.fInTried; + }; + + using Addresses = std::unordered_set; + + const size_t num_addresses{mapInfo.size()}; + + Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : mapInfo) { + addresses.insert(addr); + } + + Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : other.mapInfo) { + other_addresses.insert(addr); + } + + if (addresses != other_addresses) { + return false; + } + + auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(cs, other.cs) { + if (id == -1 && other_id == -1) { + return true; + } + if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { + return false; + } + return mapInfo.at(id) == other.mapInfo.at(other_id); + }; + + // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` + // contains just an id and the address is to be found in `mapInfo.at(id)`. The ids + // themselves may differ between `vvNew` and `other.vvNew`. + for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvNew[i][j], other.vvNew[i][j])) { + return false; + } + } + } + + // Same for `vvTried`. + for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvTried[i][j], other.vvTried[i][j])) { + return false; + } + } + } + + return true; } }; @@ -36,14 +227,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrManDeterministic addr_man; - addr_man.MakeDeterministic(ConsumeUInt256(fuzzed_data_provider)); - if (fuzzed_data_provider.ConsumeBool()) { - addr_man.m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(addr_man.m_asmap)) { - addr_man.m_asmap.clear(); - } - } + CAddrManDeterministic addr_man{fuzzed_data_provider}; if (fuzzed_data_provider.ConsumeBool()) { const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION); @@ -126,3 +310,21 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); data_stream << addr_man; } + +// Check that serialize followed by unserialize produces the same addrman. +FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + SetMockTime(ConsumeTime(fuzzed_data_provider)); + + CAddrManDeterministic addr_man1{fuzzed_data_provider}; + CAddrManDeterministic addr_man2{fuzzed_data_provider}; + addr_man2.m_asmap = addr_man1.m_asmap; + + CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); + + addr_man1.Fill(); + data_stream << addr_man1; + data_stream >> addr_man2; + assert(addr_man1 == addr_man2); +} diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 7e46108116..114fd4b870 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1506,9 +1506,17 @@ static void TestAddMatrixOverflow() constexpr T MAXI{std::numeric_limits::max()}; BOOST_CHECK(!CheckedAdd(T{1}, MAXI)); BOOST_CHECK(!CheckedAdd(MAXI, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{1}, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(MAXI, MAXI)); + BOOST_CHECK_EQUAL(0, CheckedAdd(T{0}, T{0}).value()); BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{0}, MAXI).value()); BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{1}, MAXI - 1).value()); + BOOST_CHECK_EQUAL(MAXI - 1, CheckedAdd(T{1}, MAXI - 2).value()); + BOOST_CHECK_EQUAL(0, SaturatingAdd(T{0}, T{0})); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{0}, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{1}, MAXI - 1)); + BOOST_CHECK_EQUAL(MAXI - 1, SaturatingAdd(T{1}, MAXI - 2)); } /* Check for overflow or underflow */ @@ -1520,9 +1528,17 @@ static void TestAddMatrix() constexpr T MAXI{std::numeric_limits::max()}; BOOST_CHECK(!CheckedAdd(T{-1}, MINI)); BOOST_CHECK(!CheckedAdd(MINI, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{-1}, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(MINI, MINI)); + BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{0}, MINI).value()); BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{-1}, MINI + 1).value()); BOOST_CHECK_EQUAL(-1, CheckedAdd(MINI, MAXI).value()); + BOOST_CHECK_EQUAL(MINI + 1, CheckedAdd(T{-1}, MINI + 2).value()); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{0}, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{-1}, MINI + 1)); + BOOST_CHECK_EQUAL(MINI + 1, SaturatingAdd(T{-1}, MINI + 2)); + BOOST_CHECK_EQUAL(-1, SaturatingAdd(MINI, MAXI)); } BOOST_AUTO_TEST_CASE(util_overflow) diff --git a/src/util/overflow.h b/src/util/overflow.h index c70a8b663e..6b7dd1e8fd 100644 --- a/src/util/overflow.h +++ b/src/util/overflow.h @@ -13,7 +13,7 @@ template [[nodiscard]] bool AdditionOverflow(const T i, const T j) noexcept { static_assert(std::is_integral::value, "Integral required."); - if (std::numeric_limits::is_signed) { + if constexpr (std::numeric_limits::is_signed) { return (i > 0 && j > std::numeric_limits::max() - i) || (i < 0 && j < std::numeric_limits::min() - i); } @@ -29,4 +29,22 @@ template return i + j; } +template +[[nodiscard]] T SaturatingAdd(const T i, const T j) noexcept +{ + if constexpr (std::numeric_limits::is_signed) { + if (i > 0 && j > std::numeric_limits::max() - i) { + return std::numeric_limits::max(); + } + if (i < 0 && j < std::numeric_limits::min() - i) { + return std::numeric_limits::min(); + } + } else { + if (std::numeric_limits::max() - i < j) { + return std::numeric_limits::max(); + } + } + return i + j; +} + #endif // BITCOIN_UTIL_OVERFLOW_H