diff --git a/Makefile.am b/Makefile.am index ea2cba9fd2fa..6f3dfaa7500a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -187,7 +187,7 @@ $(COV_TOOL_WRAPPER): @chmod +x $(COV_TOOL_WRAPPER) baseline.info: $(COV_TOOL_WRAPPER) - $(LCOV) -c -i -d $(abs_builddir)/src -o $@ + $(LCOV) $(LCOV_OPTS) -c -i -d $(abs_builddir)/src -o $@ baseline_filtered.info: baseline.info $(abs_builddir)/contrib/filter-lcov.py $(LCOV_FILTER_PATTERN) $< $@ @@ -221,13 +221,13 @@ functional_test_filtered.info: functional_test.info $(LCOV) -a $@ $(LCOV_OPTS) -o $@ fuzz_coverage.info: fuzz_filtered.info - $(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a fuzz_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt + $(LCOV) $(LCOV_OPTS) -a baseline_filtered.info -a fuzz_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt test_dash_coverage.info: baseline_filtered.info test_dash_filtered.info - $(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a test_dash_filtered.info -o $@ + $(LCOV) $(LCOV_OPTS) -a baseline_filtered.info -a test_dash_filtered.info -o $@ total_coverage.info: test_dash_filtered.info functional_test_filtered.info - $(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a test_dash_filtered.info -a functional_test_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt + $(LCOV) $(LCOV_OPTS) -a baseline_filtered.info -a test_dash_filtered.info -a functional_test_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt fuzz.coverage/.dirstamp: fuzz_coverage.info $(GENHTML) -s $(LCOV_OPTS) $< -o $(@D) diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index dcb9fb8dbf24..cd16ef2f6f2b 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -17,3 +17,4 @@ export RUN_FUZZ_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --enable-fuzz --with-sanitizers=fuzzer,address,undefined,float-divide-by-zero,integer \ CC='clang-19 -ftrivial-auto-var-init=pattern' CXX='clang++-19 -ftrivial-auto-var-init=pattern'" +export LLVM_SYMBOLIZER_PATH="/usr/bin/llvm-symbolizer-17" diff --git a/src/.clang-tidy b/src/.clang-tidy index 720a45cc500d..1fd6882503a3 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,6 +1,7 @@ Checks: ' -*, bugprone-argument-comment, +bugprone-string-constructor, bugprone-use-after-move, misc-unused-using-decls, modernize-use-default-member-init, diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 56ddd8a12156..e5073563318a 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -500,11 +500,14 @@ SECONDARY: $(QT_QM) $(srcdir)/qt/dashstrings.cpp: FORCE @test -n $(XGETTEXT) || echo "xgettext is required for updating translations" - $(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES) + $(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py \ + $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) \ + $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES) \ + $(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM) # The resulted dash_en.xlf source file should follow Transifex requirements. # See: https://docs.transifex.com/formats/xliff#how-to-distinguish-between-a-source-file-and-a-translation-file -translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) qt/bitcoin.cpp $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM) +translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM) @test -n $(LUPDATE) || echo "lupdate is required for updating translations" $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) -no-obsolete -I $(srcdir) -locations relative $^ -ts $(srcdir)/qt/locale/dash_en.ts @test -n $(LCONVERT) || echo "lconvert is required for updating translations" diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 6d13f7efb454..0d8610fa181e 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -129,9 +129,9 @@ bool IsSQLiteFile(const fs::path& path) file.close(); - // Check the magic, see https://sqlite.org/fileformat2.html + // Check the magic, see https://sqlite.org/fileformat.html std::string magic_str(magic, 16); - if (magic_str != std::string("SQLite format 3", 16)) { + if (magic_str != std::string{"SQLite format 3\000", 16}) { return false; } diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp index a46846c1d4a3..eb02ca20d0be 100644 --- a/src/wallet/transaction.cpp +++ b/src/wallet/transaction.cpp @@ -4,6 +4,10 @@ #include +#include + +using interfaces::FoundBlock; + namespace wallet { bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const { @@ -24,4 +28,25 @@ int64_t CWalletTx::GetTxTime() const int64_t n = nTimeSmart; return n ? n : nTimeReceived; } + +void CWalletTx::updateState(interfaces::Chain& chain) +{ + bool active; + auto lookup_block = [&](const uint256& hash, int& height, TxState& state) { + // If tx block (or conflicting block) was reorged out of chain + // while the wallet was shutdown, change tx status to UNCONFIRMED + // and reset block height, hash, and index. ABANDONED tx don't have + // associated blocks and don't need to be updated. The case where a + // transaction was reorged out while online and then reconfirmed + // while offline is covered by the rescan logic. + if (!chain.findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) { + state = TxStateInactive{}; + } + }; + if (auto* conf = state()) { + lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, m_state); + } else if (auto* conf = state()) { + lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, m_state); + } +} } // namespace wallet diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 3717be956832..dcded2f9d631 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -20,6 +20,10 @@ #include #include +namespace interfaces { +class Chain; +} // namespace interfaces + namespace wallet { //! State of transaction confirmed in a block. struct TxStateConfirmed { @@ -309,6 +313,10 @@ class CWalletTx template const T* state() const { return std::get_if(&m_state); } template T* state() { return std::get_if(&m_state); } + //! Update transaction state when attaching to a chain, filling in heights + //! of conflicted and confirmed blocks + void updateState(interfaces::Chain& chain); + bool isAbandoned() const { return state() && state()->abandoned; } bool isConflicted() const { return state(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a45038667d73..13411992d6f9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1157,23 +1157,7 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx // If wallet doesn't have a chain (e.g when using dash-wallet tool), // don't bother to update txn. if (HaveChain()) { - bool active; - auto lookup_block = [&](const uint256& hash, int& height, TxState& state) { - // If tx block (or conflicting block) was reorged out of chain - // while the wallet was shutdown, change tx status to UNCONFIRMED - // and reset block height, hash, and index. ABANDONED tx don't have - // associated blocks and don't need to be updated. The case where a - // transaction was reorged out while online and then reconfirmed - // while offline is covered by the rescan logic. - if (!chain().findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) { - state = TxStateInactive{}; - } - }; - if (auto* conf = wtx.state()) { - lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, wtx.m_state); - } else if (auto* conf = wtx.state()) { - lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, wtx.m_state); - } + wtx.updateState(chain()); } if (/* insertion took place */ ins.second) { wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -4062,8 +4046,10 @@ int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const { AssertLockHeld(cs_wallet); if (auto* conf = wtx.state()) { + assert(conf->confirmed_block_height >= 0); return GetLastBlockHeight() - conf->confirmed_block_height + 1; } else if (auto* conf = wtx.state()) { + assert(conf->conflicting_block_height >= 0); return -1 * (GetLastBlockHeight() - conf->conflicting_block_height + 1); } else { return 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e7949928ed4d..c8d8ec503bc4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -593,6 +593,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * <0 : conflicts with a transaction this deep in the blockchain * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain + * + * Preconditions: it is only valid to call this function when the wallet is + * online and the block index is loaded. So this cannot be called by + * bitcoin-wallet tool code or by wallet migration code. If this is called + * without the wallet being online, it won't be able able to determine the + * the height of the last block processed, or the heights of blocks + * referenced in transaction, and might cause assert failures. */ int GetTxDepthInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsTxInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py index 67fe0a04b391..83c9c1550bb9 100755 --- a/test/functional/mempool_datacarrier.py +++ b/test/functional/mempool_datacarrier.py @@ -13,12 +13,11 @@ ) from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import TestNode -from test_framework.util import ( - assert_raises_rpc_error, - random_bytes, -) +from test_framework.util import assert_raises_rpc_error from test_framework.wallet import MiniWallet +from random import randbytes + class DataCarrierTest(BitcoinTestFramework): def set_test_params(self): @@ -48,11 +47,11 @@ def run_test(self): self.wallet = MiniWallet(self.nodes[0]) # By default, only 80 bytes are used for data (+1 for OP_RETURN, +2 for the pushdata opcodes). - default_size_data = random_bytes(MAX_OP_RETURN_RELAY - 3) - too_long_data = random_bytes(MAX_OP_RETURN_RELAY - 2) - small_data = random_bytes(MAX_OP_RETURN_RELAY - 4) - one_byte = random_bytes(1) - zero_bytes = random_bytes(0) + default_size_data = randbytes(MAX_OP_RETURN_RELAY - 3) + too_long_data = randbytes(MAX_OP_RETURN_RELAY - 2) + small_data = randbytes(MAX_OP_RETURN_RELAY - 4) + one_byte = randbytes(1) + zero_bytes = randbytes(0) self.log.info("Testing null data transaction with default -datacarrier and -datacarriersize values.") self.test_null_data_transaction(node=self.nodes[0], data=default_size_data, success=True) diff --git a/test/functional/p2p_net_deadlock.py b/test/functional/p2p_net_deadlock.py index b1b9e67c6d47..dd052605198f 100755 --- a/test/functional/p2p_net_deadlock.py +++ b/test/functional/p2p_net_deadlock.py @@ -6,7 +6,7 @@ import threading from test_framework.messages import MAX_PROTOCOL_MESSAGE_LENGTH from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import random_bytes +from random import randbytes class NetDeadlockTest(BitcoinTestFramework): def set_test_params(self): @@ -18,7 +18,7 @@ def run_test(self): node1 = self.nodes[1] self.log.info("Simultaneously send a large message on both sides") - rand_msg = random_bytes(MAX_PROTOCOL_MESSAGE_LENGTH).hex() + rand_msg = randbytes(MAX_PROTOCOL_MESSAGE_LENGTH).hex() thread0 = threading.Thread(target=node0.sendmsgtopeer, args=(0, "unknown", rand_msg)) thread1 = threading.Thread(target=node1.sendmsgtopeer, args=(0, "unknown", rand_msg)) diff --git a/test/functional/p2p_v2_misbehaving.py b/test/functional/p2p_v2_misbehaving.py index 60c2876cf5d2..472520d227c3 100755 --- a/test/functional/p2p_v2_misbehaving.py +++ b/test/functional/p2p_v2_misbehaving.py @@ -7,7 +7,6 @@ from enum import Enum from test_framework.messages import MAGIC_BYTES -from test_framework.util import random_bytes from test_framework.p2p import P2PInterface from test_framework.test_framework import BitcoinTestFramework from test_framework.util import random_bitflip @@ -91,7 +90,7 @@ def complete_handshake(self, response): class NonEmptyVersionPacketState(EncryptedP2PState): """"Add option for sending non-empty transport version packet.""" def complete_handshake(self, response): - self.transport_version = random_bytes(5) + self.transport_version = random.randbytes(5) return super().complete_handshake(response) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index fbf7c8d81598..c34efbcca315 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -7,6 +7,7 @@ from decimal import Decimal from itertools import product +from random import randbytes from test_framework.descriptors import descsum_create from test_framework.key import ECKey @@ -34,7 +35,6 @@ assert_greater_than, assert_raises_rpc_error, find_output, - random_bytes, ) from test_framework.wallet_util import bytes_to_wif @@ -612,10 +612,10 @@ def test_psbt_input_keys(psbt_input, keys): self.log.info("Test decoding PSBT with per-input preimage types") # note that the decodepsbt RPC doesn't check whether preimages and hashes match - hash_ripemd160, preimage_ripemd160 = random_bytes(20), random_bytes(50) - hash_sha256, preimage_sha256 = random_bytes(32), random_bytes(50) - hash_hash160, preimage_hash160 = random_bytes(20), random_bytes(50) - hash_hash256, preimage_hash256 = random_bytes(32), random_bytes(50) + hash_ripemd160, preimage_ripemd160 = randbytes(20), randbytes(50) + hash_sha256, preimage_sha256 = randbytes(32), randbytes(50) + hash_hash160, preimage_hash160 = randbytes(20), randbytes(50) + hash_hash256, preimage_hash256 = randbytes(32), randbytes(50) tx = CTransaction() tx.vin = [CTxIn(outpoint=COutPoint(hash=int('aa' * 32, 16), n=0), scriptSig=b""), diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index a19c60ddc64c..30aef6b7544b 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -310,10 +310,6 @@ def sha256sum_file(filename): d = f.read(4096) return h.digest() -# TODO: Remove and use random.randbytes(n) directly -def random_bytes(n): - """Return a random bytes object of length n.""" - return random.randbytes(n) # RPC/P2P connection constants and functions ############################################ diff --git a/test/functional/test_framework/v2_p2p.py b/test/functional/test_framework/v2_p2p.py index c1c6e7afe258..dcc90c38d537 100644 --- a/test/functional/test_framework/v2_p2p.py +++ b/test/functional/test_framework/v2_p2p.py @@ -12,7 +12,6 @@ from .crypto.hkdf import hkdf_sha256 from .key import TaggedHash from .messages import MAGIC_BYTES -from .util import random_bytes CHACHA20POLY1305_EXPANSION = 16 @@ -156,7 +155,7 @@ def generate_keypair_and_garbage(self, garbage_len=None): self.privkey_ours, self.ellswift_ours = ellswift_create() if garbage_len is None: garbage_len = random.randrange(MAX_GARBAGE_LEN + 1) - self.sent_garbage = random_bytes(garbage_len) + self.sent_garbage = random.randbytes(garbage_len) return self.ellswift_ours + self.sent_garbage def initiate_v2_handshake(self): diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index f5dd73565078..4a7e6c63cddb 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -17,11 +17,14 @@ def get_fuzz_env(*, target, source_dir): + symbolizer = os.environ.get('LLVM_SYMBOLIZER_PATH', "/usr/bin/llvm-symbolizer") return { 'FUZZ': target, 'UBSAN_OPTIONS': f'suppressions={source_dir}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1', + 'UBSAN_SYMBOLIZER_PATH':symbolizer, "ASAN_OPTIONS": "detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1", + 'ASAN_SYMBOLIZER_PATH':symbolizer, }