From 85c78e08ec857e51a9748d1a2492d1d3794b221a Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Mon, 20 Sep 2021 12:19:38 +0200 Subject: build: Restrict check for CRC32C intrinsic to aarch64 `crc32c`'s hardware accelerated code doesn't handle ARM 32-bit at all. Make the check in `configure.ac` check for this architecture explicitly. For the release binaries, the current `configure.ac` check happens to work: it enables it on aarch64 but disables it for armhf. However some combination of compiler version and settings might ostensibly cause this check to succeed on armhf (as reported on IRC). So make the 64-bit platform requirement explicit. Github-Pull: #23045 Rebased-From: f2747d1602ec4e1128356b861b2167daf66a845b --- configure.ac | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a8db268fac..0d36313619 100644 --- a/configure.ac +++ b/configure.ac @@ -568,13 +568,17 @@ AX_CHECK_COMPILE_FLAG([-march=armv8-a+crc+crypto],[[ARM_CRC_CXXFLAGS="-march=arm TEMP_CXXFLAGS="$CXXFLAGS" CXXFLAGS="$CXXFLAGS $ARM_CRC_CXXFLAGS" -AC_MSG_CHECKING(for ARM CRC32 intrinsics) +AC_MSG_CHECKING(for AArch64 CRC32 intrinsics) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ #include #include ]],[[ +#ifdef __aarch64__ __crc32cb(0, 0); __crc32ch(0, 0); __crc32cw(0, 0); __crc32cd(0, 0); vmull_p64(0, 0); +#else +#error "crc32c library does not support hardware acceleration on 32-bit ARM" +#endif ]])], [ AC_MSG_RESULT(yes); enable_arm_crc=yes; ], [ AC_MSG_RESULT(no)] -- cgit v1.2.3 From db76db7329f6357c5226cd08611fe0f669c002af Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 22 Sep 2021 11:32:25 +0200 Subject: Fix (inverse) meaning of -persistmempool Github-Pull: #23061 Rebased-From: faff17bbde6dcb1482a6210bc48b3192603a446f --- src/init.cpp | 2 +- src/validation.cpp | 2 +- test/functional/mempool_persist.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 9afd76d62d..7b13296f7c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -217,7 +217,7 @@ void Shutdown(NodeContext& node) node.banman.reset(); node.addrman.reset(); - if (node.mempool && node.mempool->IsLoaded() && node.args->GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(*node.mempool); } diff --git a/src/validation.cpp b/src/validation.cpp index 26333d7026..e060899ea0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3822,7 +3822,7 @@ bool CChainState::LoadBlockIndexDB() void CChainState::LoadMempool(const ArgsManager& args) { if (!m_mempool) return; - if (args.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { ::LoadMempool(*m_mempool, *this); } m_mempool->SetIsLoaded(!ShutdownRequested()); diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 752b925b92..1ae95fced3 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -141,7 +141,7 @@ class MempoolPersistTest(BitcoinTestFramework): self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 6 transactions") os.rename(mempooldat0, mempooldat1) self.stop_nodes() - self.start_node(1, extra_args=[]) + self.start_node(1, extra_args=["-persistmempool"]) assert self.nodes[1].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[1].getrawmempool()), 6) -- cgit v1.2.3 From 92d44ff36cc12e34f93bfcc4ec31ffae8787100c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 23 Sep 2021 16:56:32 +0200 Subject: doc: Add 23061 release notes Github-Pull: #23061 Rebased-From: faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a --- doc/release-notes.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 28d9487777..6f03d7d1ac 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -60,6 +60,12 @@ New settings Updated settings ---------------- +- In previous releases, the meaning of the command line option + `-persistmempool` (without a value provided) incorrectly disabled mempool + persistence. `-persistmempool` is now treated like other boolean options to + mean `-persistmempool=1`. Passing `-persistmempool=0`, `-persistmempool=1` + and `-nopersistmempool` is unaffected. (#23061) + Tools and Utilities ------------------- -- cgit v1.2.3 From c1cdeddd905b5444eac330d565b297b3d4941c5d Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 27 Sep 2021 20:26:08 -0400 Subject: guix: Fix powerpc64(le) dynamic linker name I used Guix's values for the powerpc64(le) dynamic linkers, and the /lib-prefix seems to be a Guix-ism rather than standard. The standard path for the linker-loaders start with /lib64. I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file. For future reference, loader path values can also be found on glibc's website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16 Github-Pull: #23148 Rebased-From: b96adcbfae90b3e041754f11624cac04c1999e8c --- contrib/guix/libexec/build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 356bd70070..efa0620ef4 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -169,8 +169,8 @@ case "$HOST" in arm-linux-gnueabihf) echo /lib/ld-linux-armhf.so.3 ;; aarch64-linux-gnu) echo /lib/ld-linux-aarch64.so.1 ;; riscv64-linux-gnu) echo /lib/ld-linux-riscv64-lp64d.so.1 ;; - powerpc64-linux-gnu) echo /lib/ld64.so.1;; - powerpc64le-linux-gnu) echo /lib/ld64.so.2;; + powerpc64-linux-gnu) echo /lib64/ld64.so.1;; + powerpc64le-linux-gnu) echo /lib64/ld64.so.2;; *) exit 1 ;; esac ) -- cgit v1.2.3 From c95b188fc08387d0a89668e56bce3a4fad1ee611 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 2 Jul 2021 11:10:25 +0800 Subject: system: skip trying to set the locale on NetBSD Just treat it the same as the other BSDs. Fixes #17379. Github-Pull: #22390 Rebased-From: fdd71448e78f442ffd93a3a3398a5062eaba9f1b --- src/util/system.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index 258ba2f235..65c16fcd97 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1301,7 +1301,7 @@ void SetupEnvironment() #endif // On most POSIX systems (e.g. Linux, but not BSD) the environment's locale // may be invalid, in which case the "C.UTF-8" locale is used as fallback. -#if !defined(WIN32) && !defined(MAC_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) +#if !defined(WIN32) && !defined(MAC_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__NetBSD__) try { std::locale(""); // Raises a runtime error if current locale is invalid } catch (const std::runtime_error&) { -- cgit v1.2.3 From a5a153882609c8d77118a88a9a440d4966c8d0ef Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 27 Aug 2021 17:50:30 +0300 Subject: build, qt: Fix typo in QtInputSupport check Github-Pull: #22820 Rebased-From: e251726affe97da745362c82567c2377ceb07d21 --- build-aux/m4/bitcoin_qt.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4 index 5b5a8ed16e..1e979edf0f 100644 --- a/build-aux/m4/bitcoin_qt.m4 +++ b/build-aux/m4/bitcoin_qt.m4 @@ -350,7 +350,7 @@ AC_DEFUN([_BITCOIN_QT_CHECK_STATIC_LIBS], [ PKG_CHECK_MODULES([QT_FONTDATABASE], [${qt_lib_prefix}FontDatabaseSupport${qt_lib_suffix}], [QT_LIBS="$QT_FONTDATABASE_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_THEME], [${qt_lib_prefix}ThemeSupport${qt_lib_suffix}], [QT_LIBS="$QT_THEME_LIBS $QT_LIBS"]) if test "x$TARGET_OS" = xlinux; then - PKG_CHECK_MODULES([QT_INPUT], [${qt_lib_prefix}XcbQpa], [QT_LIBS="$QT_INPUT_LIBS $QT_LIBS"]) + PKG_CHECK_MODULES([QT_INPUT], [${qt_lib_prefix}InputSupport], [QT_LIBS="$QT_INPUT_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_SERVICE], [${qt_lib_prefix}ServiceSupport], [QT_LIBS="$QT_SERVICE_LIBS $QT_LIBS"]) PKG_CHECK_MODULES([QT_XCBQPA], [${qt_lib_prefix}XcbQpa], [QT_LIBS="$QT_XCBQPA_LIBS $QT_LIBS"]) elif test "x$TARGET_OS" = xdarwin; then -- cgit v1.2.3 From c671c6f4706d17cccfe5c35950235f8777a7975f Mon Sep 17 00:00:00 2001 From: Saibato Date: Mon, 23 Aug 2021 12:49:10 -0400 Subject: the result of CWallet::IsHDEnabled() was initialized with true. But in case of no keys or a blank hd wallet the iterator would be skipped and not set to false but true, since the loop would be not entered. That had resulted in a wrong return and subsequent false HD and watch-only icon display in gui when reloading a wallet after closing. Update src/wallet/wallet.cpp Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Github-Pull: #22781 Rebased-From: 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e --- src/wallet/wallet.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9a61ca698d..c139788c78 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1360,9 +1360,10 @@ CAmount CWallet::GetDebit(const CTransaction& tx, const isminefilter& filter) co bool CWallet::IsHDEnabled() const { // All Active ScriptPubKeyMans must be HD for this to be true - bool result = true; + bool result = false; for (const auto& spk_man : GetActiveScriptPubKeyMans()) { - result &= spk_man->IsHDEnabled(); + if (!spk_man->IsHDEnabled()) return false; + result = true; } return result; } -- cgit v1.2.3 From 7febe4f3c7f482390c4aa6fc528e2ee3fb34b142 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 5 Sep 2021 11:32:59 +0200 Subject: consensus: don't call GetBlockPos in ReadBlockFromDisk without lock Github-Pull: #22895 Rebased-From: 350e034e64d175f3db4c85ddca42e76e279912f6 --- src/node/blockstorage.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 0083b74b33..b64fc8d944 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -394,18 +394,14 @@ bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::P bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams) { - FlatFilePos blockPos; - { - LOCK(cs_main); - blockPos = pindex->GetBlockPos(); - } + const FlatFilePos block_pos{WITH_LOCK(cs_main, return pindex->GetBlockPos())}; - if (!ReadBlockFromDisk(block, blockPos, consensusParams)) { + if (!ReadBlockFromDisk(block, block_pos, consensusParams)) { return false; } if (block.GetHash() != pindex->GetBlockHash()) { return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s", - pindex->ToString(), pindex->GetBlockPos().ToString()); + pindex->ToString(), block_pos.ToString()); } return true; } -- cgit v1.2.3 From 282863a7e9ddfb14ef02182945ca1978699dbe52 Mon Sep 17 00:00:00 2001 From: Joan Karadimov Date: Fri, 22 Oct 2021 01:19:27 +0300 Subject: refactor: include a missing header in fs.cpp ... needed for std::numeric_limits::max on WIN32 Github-Pull: #23335 Rebased-From: 077a875d94b51e3c87381133657be98989c8643e --- src/fs.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fs.cpp b/src/fs.cpp index 89c7ad27dc..a7cfa627e1 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -16,6 +16,7 @@ #define NOMINMAX #endif #include +#include #include #endif -- cgit v1.2.3 From 227ae652542451834faddbaffb54fc384e9156e6 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 21 Oct 2021 16:03:03 +0200 Subject: wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v). Github-Pull: #23333 Rebased-From: 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31 --- src/wallet/wallet.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c139788c78..5d67097913 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3187,7 +3187,8 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern auto spk_man = m_spk_managers.at(id).get(); spk_mans[type] = spk_man; - if (spk_mans_other[type] == spk_man) { + const auto it = spk_mans_other.find(type); + if (it != spk_mans_other.end() && it->second == spk_man) { spk_mans_other.erase(type); } -- cgit v1.2.3 From bd7e08e36bf2e1238ddf8cc01433f8db82f848c9 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 10 Sep 2021 20:24:44 -0400 Subject: fees: Always round up fee calculated from a feerate When calculating the fee for a given tx size from a fee rate, we should always round up to the next satoshi. Otherwise, if we round down (via truncation), the calculated fee may result in a fee with a feerate slightly less than targeted. This is particularly important for coin selection as a slightly lower feerate than expected can result in a variety of issues. Github-Pull: #22949 Rebased-From: 0fbaef9676a1dcb84bcf95afd8d994831ab327b6 --- src/policy/feerate.cpp | 6 +++++- src/policy/feerate.h | 1 + src/test/amount_tests.cpp | 10 +++++----- src/test/transaction_tests.cpp | 4 ++-- test/functional/wallet_keypool.py | 2 +- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp index 25b9282b4e..ce149067b7 100644 --- a/src/policy/feerate.cpp +++ b/src/policy/feerate.cpp @@ -7,6 +7,8 @@ #include +#include + CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes) { const int64_t nSize{num_bytes}; @@ -22,7 +24,9 @@ CAmount CFeeRate::GetFee(uint32_t num_bytes) const { const int64_t nSize{num_bytes}; - CAmount nFee = nSatoshisPerK * nSize / 1000; + // Be explicit that we're converting from a double to int64_t (CAmount) here. + // We've previously had issues with the silent double->int64_t conversion. + CAmount nFee{static_cast(std::ceil(nSatoshisPerK * nSize / 1000.0))}; if (nFee == 0 && nSize != 0) { if (nSatoshisPerK > 0) nFee = CAmount(1); diff --git a/src/policy/feerate.h b/src/policy/feerate.h index d296d32774..57bb5d02c0 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -48,6 +48,7 @@ public: CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes); /** * Return the fee in satoshis for the given size in bytes. + * If the calculated fee would have fractional satoshis, then the returned fee will always be rounded up to the nearest satoshi. */ CAmount GetFee(uint32_t num_bytes) const; /** diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp index 77b7758a17..12193ccb59 100644 --- a/src/test/amount_tests.cpp +++ b/src/test/amount_tests.cpp @@ -48,13 +48,13 @@ BOOST_AUTO_TEST_CASE(GetFeeTest) BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), CAmount(-9e3)); feeRate = CFeeRate(123); - // Truncates the result, if not integer + // Rounds up the result, if not integer BOOST_CHECK_EQUAL(feeRate.GetFee(0), CAmount(0)); BOOST_CHECK_EQUAL(feeRate.GetFee(8), CAmount(1)); // Special case: returns 1 instead of 0 - BOOST_CHECK_EQUAL(feeRate.GetFee(9), CAmount(1)); - BOOST_CHECK_EQUAL(feeRate.GetFee(121), CAmount(14)); - BOOST_CHECK_EQUAL(feeRate.GetFee(122), CAmount(15)); - BOOST_CHECK_EQUAL(feeRate.GetFee(999), CAmount(122)); + BOOST_CHECK_EQUAL(feeRate.GetFee(9), CAmount(2)); + BOOST_CHECK_EQUAL(feeRate.GetFee(121), CAmount(15)); + BOOST_CHECK_EQUAL(feeRate.GetFee(122), CAmount(16)); + BOOST_CHECK_EQUAL(feeRate.GetFee(999), CAmount(123)); BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), CAmount(123)); BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), CAmount(1107)); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 40c53cb2ec..17bd561807 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -807,12 +807,12 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) // nDustThreshold = 182 * 3702 / 1000 dustRelayFee = CFeeRate(3702); // dust: - t.vout[0].nValue = 673 - 1; + t.vout[0].nValue = 674 - 1; reason.clear(); BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); BOOST_CHECK_EQUAL(reason, "dust"); // not dust: - t.vout[0].nValue = 673; + t.vout[0].nValue = 674; BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 28bfc9116f..9286387f96 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -179,7 +179,7 @@ class KeyPoolTest(BitcoinTestFramework): assert_equal("psbt" in res, True) # create a transaction without change at the maximum fee rate, such that the output is still spendable: - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.0008824}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.0008823}) assert_equal("psbt" in res, True) assert_equal(res["fee"], Decimal("0.00009706")) -- cgit v1.2.3 From f66bc42957ad2e86982c8c487f821683d3009b43 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 10 Sep 2021 20:27:41 -0400 Subject: tests: Test for assertion when feerate is rounded down When calculating a txs absolute fee, if the fee is rounded down to the nearest satoshi, it is possible for the coin selection algorithms to undercalculate the fee needed. This can lead to an assertion error in some situations. One such scenario is added to rpc_fundrawtransaction.py. Github-Pull: #22949 Rebased-From: ce2cc44afd51f3df4ee7f14ea05b8da229183923 --- test/functional/rpc_fundrawtransaction.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index fcc310394a..bbd3dbb10c 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -100,6 +100,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_transaction_too_large() self.test_include_unsafe() self.test_22670() + self.test_feerate_rounding() def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" @@ -1026,6 +1027,33 @@ class RawTransactionsTest(BitcoinTestFramework): do_fund_send(upper_bound) self.restart_node(0) + self.connect_nodes(0, 1) + self.connect_nodes(0, 2) + self.connect_nodes(0, 3) + + def test_feerate_rounding(self): + self.log.info("Test that rounding of GetFee does not result in an assertion") + + self.nodes[1].createwallet("roundtest") + w = self.nodes[1].get_wallet_rpc("roundtest") + + addr = w.getnewaddress(address_type="bech32") + self.nodes[0].sendtoaddress(addr, 1) + self.nodes[0].generate(1) + self.sync_all() + + # A P2WPKH input costs 68 vbytes; With a single P2WPKH output, the rest of the tx is 42 vbytes for a total of 110 vbytes. + # At a feerate of 1.85 sat/vb, the input will need a fee of 125.8 sats and the rest 77.7 sats + # The entire tx fee should be 203.5 sats. + # Coin selection rounds the fee individually instead of at the end (due to how CFeeRate::GetFee works). + # If rounding down (which is the incorrect behavior), then the calculated fee will be 125 + 77 = 202. + # If rounding up, then the calculated fee will be 126 + 78 = 204. + # In the former case, the calculated needed fee is higher than the actual fee being paid, so an assertion is reached + # To test this does not happen, we subtract 202 sats from the input value. If working correctly, this should + # fail with insufficient funds rather than bitcoind asserting. + rawtx = w.createrawtransaction(inputs=[], outputs=[{self.nodes[0].getnewaddress(address_type="bech32"): 1 - 0.00000202}]) + assert_raises_rpc_error(-4, "Insufficient funds", w.fundrawtransaction, rawtx, {"fee_rate": 1.85}) + if __name__ == '__main__': RawTransactionsTest().main() -- cgit v1.2.3 From c768bfa08af034c744402d4294cc323d653b97b8 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 8 Oct 2021 13:57:48 -0400 Subject: tests: Calculate fees more similarly to CFeeRate::GetFee Because of floating point precision issues, not all of the rounding done is always correct. To fix this, the fee calculation for assert_fee_amount is changed to better reflect how CFeeRate::GetFee does it. First the feerate is converted to an int representing sat/kvb. Then this is multiplied by the transaction size, divivided by 1000, and rounded up to the nearest sat. The result is then converted back to BTC (divided by 1e8) and then rounded down to the nearest sat to avoid precision errors. Github-Pull: #22949 Rebased-From: 80dc829be7f8c3914074b85bb4c125baba18cb2c --- test/functional/test_framework/util.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 35dbfbba8d..72fd83e3f7 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -35,13 +35,14 @@ def assert_approx(v, vexp, vspan=0.00001): raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan))) -def assert_fee_amount(fee, tx_size, fee_per_kB): - """Assert the fee was in range""" - target_fee = round(tx_size * fee_per_kB / 1000, 8) +def assert_fee_amount(fee, tx_size, feerate_BTC_kvB): + """Assert the fee is in range.""" + target_fee = get_fee(tx_size, feerate_BTC_kvB) if fee < target_fee: raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee))) # allow the wallet's estimation to be at most 2 bytes off - if fee > (tx_size + 2) * fee_per_kB / 1000: + high_fee = get_fee(tx_size + 2, feerate_BTC_kvB) + if fee > high_fee: raise AssertionError("Fee of %s BTC too high! (Should be %s BTC)" % (str(fee), str(target_fee))) @@ -222,6 +223,18 @@ def str_to_b64str(string): return b64encode(string.encode('utf-8')).decode('ascii') +def ceildiv(a, b): + """Divide 2 ints and round up to next int rather than round down""" + return -(-a // b) + + +def get_fee(tx_size, feerate_btc_kvb): + """Calculate the fee in BTC given a feerate is BTC/kvB. Reflects CFeeRate::GetFee""" + feerate_sat_kvb = int(feerate_btc_kvb * Decimal(1e8)) # Fee in sat/kvb as an int to avoid float precision errors + target_fee_sat = ceildiv(feerate_sat_kvb * tx_size, 1000) # Round calculated fee up to nearest sat + return satoshi_round(target_fee_sat / Decimal(1e8)) # Truncate BTC result to nearest sat + + def satoshi_round(amount): return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN) -- cgit v1.2.3 From 801b0f05aaf974ab9b0e3f7b59948564638d593f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 24 Nov 2021 14:03:12 +0530 Subject: build: patch qt to explicitly define previously implicit header include macOS Monterey has refactored some includes such that implicitly defined headers were no longer exposed and that in turns breaks building Qt on macOS 12. Additional Resources: - https://bugreports.qt.io/browse/QTBUG-97855 - https://codereview.qt-project.org/c/qt/qtbase/+/378706 - https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/cocoa?id=dece6f5840463ae2ddf927d65eb1b3680e34a547 Github-Pull: #23580 Rebased-From: 8196b0a2bc63c35769eca213cba4a5844a737b90 --- depends/packages/qt.mk | 2 ++ depends/patches/qt/fix_montery_include.patch | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 depends/patches/qt/fix_montery_include.patch diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 0c3ef8c82f..81518780cd 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -13,6 +13,7 @@ $(package)_patches+= fix_android_qmake_conf.patch fix_android_jni_static.patch d $(package)_patches+= no_sdk_version_check.patch $(package)_patches+= fix_lib_paths.patch fix_android_pch.patch $(package)_patches+= qtbase-moc-ignore-gcc-macro.patch fix_limits_header.patch +$(package)_patches+= fix_montery_include.patch $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix) $(package)_qttranslations_sha256_hash=577b0668a777eb2b451c61e8d026d79285371597ce9df06b6dee6c814164b7c3 @@ -232,6 +233,7 @@ define $(package)_preprocess_cmds patch -p1 -i $($(package)_patch_dir)/fix_lib_paths.patch && \ patch -p1 -i $($(package)_patch_dir)/qtbase-moc-ignore-gcc-macro.patch && \ patch -p1 -i $($(package)_patch_dir)/fix_limits_header.patch && \ + patch -p1 -i $($(package)_patch_dir)/fix_montery_include.patch && \ mkdir -p qtbase/mkspecs/macx-clang-linux &&\ cp -f qtbase/mkspecs/macx-clang/qplatformdefs.h qtbase/mkspecs/macx-clang-linux/ &&\ cp -f $($(package)_patch_dir)/mac-qmake.conf qtbase/mkspecs/macx-clang-linux/qmake.conf && \ diff --git a/depends/patches/qt/fix_montery_include.patch b/depends/patches/qt/fix_montery_include.patch new file mode 100644 index 0000000000..38b700addf --- /dev/null +++ b/depends/patches/qt/fix_montery_include.patch @@ -0,0 +1,21 @@ +From dece6f5840463ae2ddf927d65eb1b3680e34a547 +From: Øystein Heskestad +Date: Wed, 27 Oct 2021 13:07:46 +0200 +Subject: [PATCH] Add missing macOS header file that was indirectly included before + +See: https://bugreports.qt.io/browse/QTBUG-97855 + +Upstream Commits: + - Qt 6.2: c884bf138a21dd7320e35cef34d24e22e74d7ce0 + +diff --git a/qtbase/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h b/qtbase/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h +index e070ba97..07c75b04 100644 +--- a/qtbase/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h ++++ b/qtbase/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h +@@ -40,6 +40,7 @@ + #ifndef QIOSURFACEGRAPHICSBUFFER_H + #define QIOSURFACEGRAPHICSBUFFER_H + ++#include + #include + #include -- cgit v1.2.3 From 2f60fc6d8c6b1d8e74c340fed495b76deac4a048 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 13 Nov 2021 11:26:30 +0100 Subject: ci: Replace soon EOL hirsute with jammy Github-Pull: #23504 Rebased-From: fafa66e424cd0c4a4ac3175e0d3b15a54626aa4b --- .cirrus.yml | 8 ++++---- ci/test/00_setup_env_native_asan.sh | 2 +- ci/test/00_setup_env_native_tsan.sh | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 04365430b9..aa3ea614b2 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -122,10 +122,10 @@ task: FILE_ENV: "./ci/test/00_setup_env_native_qt5.sh" task: - name: '[depends, sanitizers: thread (TSan), no gui] [hirsute]' + name: '[depends, sanitizers: thread (TSan), no gui] [jammy]' << : *GLOBAL_TASK_TEMPLATE container: - image: ubuntu:hirsute + image: ubuntu:jammy cpu: 6 # Increase CPU and Memory to avoid timeout memory: 24G env: @@ -143,10 +143,10 @@ task: FILE_ENV: "./ci/test/00_setup_env_native_msan.sh" task: - name: '[no depends, sanitizers: address/leak (ASan + LSan) + undefined (UBSan) + integer] [hirsute]' + name: '[no depends, sanitizers: address/leak (ASan + LSan) + undefined (UBSan) + integer] [jammy]' << : *GLOBAL_TASK_TEMPLATE container: - image: ubuntu:hirsute + image: ubuntu:jammy env: << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index ab185b6e71..947c4b2891 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_asan export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev" -export DOCKER_NAME_TAG=ubuntu:hirsute +export DOCKER_NAME_TAG=ubuntu:22.04 export NO_DEPENDS=1 export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++" diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index a5082bdaab..5d0880ff4a 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_tsan -export DOCKER_NAME_TAG=ubuntu:hirsute +export DOCKER_NAME_TAG=ubuntu:22.04 export PACKAGES="clang llvm libc++abi-dev libc++-dev python3-zmq" export DEP_OPTS="CC=clang CXX='clang++ -stdlib=libc++'" export GOAL="install" -- cgit v1.2.3 From 269553fe73b17f8acda3071a48836c66092d31d0 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 2 Feb 2022 17:52:05 +0100 Subject: test: Call ceildiv helper with integer It returns an incorrect result when called with a Decimal, for which the "//" operator works differently. Also drop unnecessary call to satoshi_round. Github-Pull: #24239 Rebased-From: d1fab9d5d27a2db2546db0f610e0f6929ec4864e --- test/functional/test_framework/util.py | 11 +++++++++-- test/functional/wallet_send.py | 9 +++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 72fd83e3f7..8a41ec4370 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -37,6 +37,7 @@ def assert_approx(v, vexp, vspan=0.00001): def assert_fee_amount(fee, tx_size, feerate_BTC_kvB): """Assert the fee is in range.""" + assert isinstance(tx_size, int) target_fee = get_fee(tx_size, feerate_BTC_kvB) if fee < target_fee: raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee))) @@ -224,7 +225,13 @@ def str_to_b64str(string): def ceildiv(a, b): - """Divide 2 ints and round up to next int rather than round down""" + """ + Divide 2 ints and round up to next int rather than round down + Implementation requires python integers, which have a // operator that does floor division. + Other types like decimal.Decimal whose // operator truncates towards 0 will not work. + """ + assert isinstance(a, int) + assert isinstance(b, int) return -(-a // b) @@ -232,7 +239,7 @@ def get_fee(tx_size, feerate_btc_kvb): """Calculate the fee in BTC given a feerate is BTC/kvB. Reflects CFeeRate::GetFee""" feerate_sat_kvb = int(feerate_btc_kvb * Decimal(1e8)) # Fee in sat/kvb as an int to avoid float precision errors target_fee_sat = ceildiv(feerate_sat_kvb * tx_size, 1000) # Round calculated fee up to nearest sat - return satoshi_round(target_fee_sat / Decimal(1e8)) # Truncate BTC result to nearest sat + return target_fee_sat / Decimal(1e8) # Return result in BTC def satoshi_round(amount): diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index d24d1693af..d469af600e 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -15,6 +15,7 @@ from test_framework.util import ( assert_fee_amount, assert_greater_than, assert_raises_rpc_error, + count_bytes, ) class WalletSendTest(BitcoinTestFramework): @@ -318,20 +319,20 @@ class WalletSendTest(BitcoinTestFramework): res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] - assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007")) + assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00007")) # "unset" and None are treated the same for estimate_mode res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] - assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002")) + assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00002")) res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] - assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531")) + assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00004531")) res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=3, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] - assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003")) + assert_fee_amount(fee, count_bytes(res["hex"]), Decimal("0.00003")) # Test that passing fee_rate as both an argument and an option raises. self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=1, fee_rate=1, add_to_wallet=False, -- cgit v1.2.3