From c97293ade68018607f342921335a49192e1f6f5a Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Fri, 29 Sep 2023 22:56:10 -0700 Subject: [PATCH 01/18] Disable unused-result warning if possible --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index e8ddab7d79..56243e14da 100644 --- a/configure.ac +++ b/configure.ac @@ -445,6 +445,7 @@ if test "$CXXFLAGS_overridden" = "no"; then AX_CHECK_COMPILE_FLAG([-Wthread-safety], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wloop-analysis], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wredundant-decls], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"], [], [$CXXFLAG_WERROR]) + AX_CHECK_COMPILE_FLAG([-Wunused-result],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-result"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wunused-member-function], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-member-function"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wdate-time], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdate-time"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wconditional-uninitialized], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wconditional-uninitialized"], [], [$CXXFLAG_WERROR]) From 6678e3a40e1431e251b234920c0c58da426a4d48 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Wed, 26 Feb 2025 05:30:10 -0800 Subject: [PATCH 02/18] less win64 tests and artifacts --- .cirrus.yml | 176 +++------------------------------------------------- 1 file changed, 10 insertions(+), 166 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 68de872185..ca2f81b93e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -66,20 +66,6 @@ compute_credits_template: &CREDITS_TEMPLATE # Only use credits for pull requests to the main repo use_compute_credits: $CIRRUS_REPO_FULL_NAME == 'ElementsProject/elements' && $CIRRUS_PR != "" -task: - name: 'lint [bionic]' - << : *BASE_TEMPLATE - container: - image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md - cpu: 1 - memory: 1G - # For faster CI feedback, immediately schedule the linters - << : *CREDITS_TEMPLATE - lint_script: - - ./ci/lint_run_all.sh - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - task: name: "Win64 native [msvc]" << : *FILTER_TEMPLATE @@ -160,165 +146,23 @@ task: - python build_msvc\msvc-autogen.py - msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal -noLogo unit_tests_script: - - src\test_elements.exe -l test_suite - - src\bench_elements.exe > NUL - - python test\util\test_runner.py + # - src\test_elements.exe -l test_suite + # - src\bench_elements.exe > NUL + # - python test\util\test_runner.py - python test\util\rpcauth-test.py functional_tests_script: # Increase the dynamic port range to the maximum allowed value to mitigate "OSError: [WinError 10048] Only one usage of each socket address (protocol/network address/port) is normally permitted". # See: https://docs.microsoft.com/en-us/biztalk/technical-guides/settings-that-can-be-modified-to-improve-network-performance - netsh int ipv4 set dynamicport tcp start=1025 num=64511 - netsh int ipv6 set dynamicport tcp start=1025 num=64511 + - mkdir artifactsdir # Exclude feature_dbcrash for now due to timeout # Exclude also wallet_avoidreuse due to timeout # Ignore failures for now, need to investigate but we really don't use native win64 builds - - python test\functional\test_runner.py --nocleanup --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --extended --exclude feature_dbcrash,wallet_avoidreuse || true - -task: - name: 'ARM [unit tests, no functional tests] [bullseye]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: debian:bullseye - docker_arguments: - CI_IMAGE_NAME_TAG: debian:bullseye - << : *CREDITS_TEMPLATE - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_arm.sh" - -task: - name: 'Win64 [unit tests, no gui tests, no boost::process, no functional tests] [jammy]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:jammy - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_win64.sh" - -task: - name: '32-bit + dash [gui] [Rocky 8]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: quay.io/rockylinux/rockylinux:8 - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - PACKAGE_MANAGER_INSTALL: "yum install -y" - FILE_ENV: "./ci/test/00_setup_env_i686_centos.sh" - -task: - name: '[previous releases, uses qt5 dev package and some depends packages, DEBUG] [unsigned char] [bionic]' - previous_releases_cache: - folder: "releases" - << : *GLOBAL_TASK_TEMPLATE - << : *PERSISTENT_WORKER_TEMPLATE - env: - << : *PERSISTENT_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_qt5.sh" - -task: - name: '[TSan, depends, gui] [2404]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:24.04 - cpu: 6 # Increase CPU and Memory to avoid timeout - memory: 24G - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_tsan.sh" - MAKEJOBS: "-j2" # Avoid excessive memory use due to MSan - -task: - name: '[MSan, depends] [focal]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:focal - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_msan.sh" - -task: - name: '[ASan + LSan + UBSan + integer, no depends] [jammy]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:jammy - memory: 16G # ELEMENTS: need more memory - cpu: 4 # ELEMENTS: cirrus wants more CPUs if you want more memory - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" - -task: - name: '[fuzzer,address,undefined,integer, no depends] [jammy]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:jammy - cpu: 8 # Increase CPU and memory to avoid timeout - memory: 16G - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_fuzz.sh" + - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 feature_block_v4 feature_trim_headers || true + artifacts: + paths: + - artifactsdir/**/*.log + - artifactsdir/**/tmp* + - artifactsdir/**/*.conf -task: - name: '[multiprocess, i686, DEBUG] [focal]' - # Disable for Elements for now; Multiprocess build is not supported or tested and fails CI. - only_if: false - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:focal - cpu: 4 - memory: 16G # The default memory is sometimes just a bit too small, so double everything - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_i686_multiprocess.sh" - -task: - name: '[no wallet, libbitcoinkernel] [bionic]' - << : *GLOBAL_TASK_TEMPLATE - container: - image: ubuntu:bionic - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" - -task: - name: 'macOS 10.15 [gui, no tests] [focal]' - << : *BASE_TEMPLATE - macos_sdk_cache: - folder: "depends/SDKs/$MACOS_SDK" - fingerprint_key: "$MACOS_SDK" - << : *MAIN_TEMPLATE - container: - image: ubuntu:focal - env: - MACOS_SDK: "Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers" - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_mac.sh" - -task: - name: 'macOS 13 native arm64 [gui, sqlite only] [no depends]' - macos_instance: - # Use latest image, but hardcode version to avoid silent upgrades (and breaks) - image: ghcr.io/cirruslabs/macos-ventura-xcode:14.1 # https://cirrus-ci.org/guide/macOS - << : *MACOS_NATIVE_TASK_TEMPLATE - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - CI_USE_APT_INSTALL: "no" - PACKAGE_MANAGER_INSTALL: "echo" # Nothing to do - FILE_ENV: "./ci/test/00_setup_env_mac_native_arm64.sh" - -task: - name: 'ARM64 Android APK [focal]' - # Disable for Elements; Android build is broken and unsupported. - only_if: false - android_sdk_cache: - folder: "depends/SDKs/android" - fingerprint_key: "ANDROID_API_LEVEL=28 ANDROID_BUILD_TOOLS_VERSION=28.0.3 ANDROID_NDK_VERSION=23.1.7779620" - depends_sources_cache: - folder: "depends/sources" - fingerprint_script: git rev-list -1 HEAD ./depends - << : *MAIN_TEMPLATE - container: - image: ubuntu:focal - env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - FILE_ENV: "./ci/test/00_setup_env_android.sh" From a62e16bad89b2c73c2d2200357606ee827514fd9 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 4 Mar 2025 03:40:53 -0800 Subject: [PATCH 03/18] more_tests --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index ca2f81b93e..bb9dfc5f60 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -159,7 +159,7 @@ task: # Exclude feature_dbcrash for now due to timeout # Exclude also wallet_avoidreuse due to timeout # Ignore failures for now, need to investigate but we really don't use native win64 builds - - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 feature_block_v4 feature_trim_headers || true + - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 feature_block_v4 feature_trim_headers feature_block_v4 feature_fedpeg feature_nulldummy feature_sighash_rangeproof rpc_fundrawtransaction rpc_psbt wallet_bumpfee wallet_keypool wallet_multisig_descriptor_psbt wallet_send wallet_taproot wallet_timelock wallet_watchonly || true artifacts: paths: - artifactsdir/**/*.log From 5b21d14606f25568404b1a0bd2df4c8473303495 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 4 Mar 2025 13:49:54 -0800 Subject: [PATCH 04/18] all_tests --- .cirrus.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index bb9dfc5f60..e87376574f 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -159,7 +159,8 @@ task: # Exclude feature_dbcrash for now due to timeout # Exclude also wallet_avoidreuse due to timeout # Ignore failures for now, need to investigate but we really don't use native win64 builds - - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 feature_block_v4 feature_trim_headers feature_block_v4 feature_fedpeg feature_nulldummy feature_sighash_rangeproof rpc_fundrawtransaction rpc_psbt wallet_bumpfee wallet_keypool wallet_multisig_descriptor_psbt wallet_send wallet_taproot wallet_timelock wallet_watchonly || true + - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --extended --exclude feature_dbcrash,wallet_avoidreuse || true + artifacts: paths: - artifactsdir/**/*.log From 913a587949c7c88e45378b9a17178f4591daa859 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 4 Mar 2025 13:50:43 -0800 Subject: [PATCH 05/18] debug: move trim_headers test to the beginning --- test/functional/test_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 710e372752..9a441cfce9 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -88,6 +88,7 @@ BASE_SCRIPTS = [ # Scripts that are run by default. # vv First elements tests vv + 'feature_trim_headers.py', 'example_elements_code_tutorial.py', 'feature_fedpeg.py --legacy-wallet', 'feature_fedpeg.py --pre_transition --legacy-wallet', @@ -113,7 +114,6 @@ 'wallet_elements_regression_1172.py --legacy-wallet', 'wallet_elements_regression_1259.py --legacy-wallet', 'wallet_elements_21million.py', - 'feature_trim_headers.py', # Longest test should go first, to favor running tests in parallel 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', From f85a694ac9b49215312e7c837ada7e15645eb52a Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Sun, 29 May 2022 14:36:53 +0200 Subject: [PATCH 06/18] rpc: Capture potentially large UniValue by ref for rpcdoccheck Github-Pull: 25237 Rebased-From: 20ff4991e548630d7bb5e491fa4d69ec49369872 --- src/rpc/util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 1a65c07ab7..71e896951c 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -597,7 +597,7 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const throw std::runtime_error(ToString()); } const UniValue ret = m_fun(*this, request); - CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [ret](const RPCResult& res) { return res.MatchesType(ret); })); + CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); })); return ret; } From c1519787b5061c906cdc54a7aa15b32754fbd24e Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 2 Sep 2022 12:50:12 +0100 Subject: [PATCH 07/18] Prevent data race for `pathHandlers` Github-Pull: bitcoin/bitcoin#25983 Rebased-From: 4296dde28757d88a7076847484669fb202b47bc8 --- src/httpserver.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index e00c68585e..022703e79d 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -143,7 +143,8 @@ static std::vector rpc_allow_subnets; //! Work queue for handling longer requests off the event loop thread static std::unique_ptr> g_work_queue{nullptr}; //! Handlers for (sub)paths -static std::vector pathHandlers; +static Mutex g_httppathhandlers_mutex; +static std::vector pathHandlers GUARDED_BY(g_httppathhandlers_mutex); //! Bound listening sockets static std::vector boundSockets; @@ -244,6 +245,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg) // Find registered handler for prefix std::string strURI = hreq->GetURI(); std::string path; + LOCK(g_httppathhandlers_mutex); std::vector::const_iterator i = pathHandlers.begin(); std::vector::const_iterator iend = pathHandlers.end(); for (; i != iend; ++i) { @@ -642,11 +644,13 @@ HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler) { LogPrint(BCLog::HTTP, "Registering HTTP handler for %s (exactmatch %d)\n", prefix, exactMatch); + LOCK(g_httppathhandlers_mutex); pathHandlers.push_back(HTTPPathHandler(prefix, exactMatch, handler)); } void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch) { + LOCK(g_httppathhandlers_mutex); std::vector::iterator i = pathHandlers.begin(); std::vector::iterator iend = pathHandlers.end(); for (; i != iend; ++i) From ee2073e9f0169c79c55a8be99d0c5c307df3620e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Sun, 15 Jan 2023 20:18:11 -0500 Subject: [PATCH 08/18] hash: add HashedSourceWriter This class is the counterpart to CHashVerifier, in that it writes data to an underlying source stream, while keeping a hash of the written data. Github-Pull: #26909 Rebased-From: da6c7aeca38e1d0ab5839a374c26af0504d603fc (cherry picked from commit fd94befbc62e5f6b93f87979cc2011508378043d) --- src/hash.h | 24 ++++++++++++++++++++++++ src/test/streams_tests.cpp | 14 ++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/hash.h b/src/hash.h index 68999a5615..34723c345e 100644 --- a/src/hash.h +++ b/src/hash.h @@ -188,6 +188,30 @@ class CHashVerifier : public CHashWriter } }; +/** Writes data to an underlying source stream, while hashing the written data. */ +template +class HashedSourceWriter : public CHashWriter +{ +private: + Source& m_source; + +public: + explicit HashedSourceWriter(Source& source LIFETIMEBOUND) : CHashWriter{source.GetType(), source.GetVersion()}, m_source{source} {} + + void write(Span src) + { + m_source.write(src); + CHashWriter::write(src); + } + + template + HashedSourceWriter& operator<<(const T& obj) + { + ::Serialize(*this, obj); + return *this; + } +}; + /** Compute the 256-bit hash of an object's serialization, with optional sighash byte. */ template uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL_VERSION) diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 0925e2e9ee..e30ae845ef 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -441,4 +441,18 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) fs::remove(streams_test_filename); } +BOOST_AUTO_TEST_CASE(streams_hashed) +{ + CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION); + HashedSourceWriter hash_writer{stream}; + const std::string data{"bitcoin"}; + hash_writer << data; + + CHashVerifier hash_verifier{&stream}; + std::string result; + hash_verifier >> result; + BOOST_CHECK_EQUAL(data, result); + BOOST_CHECK_EQUAL(hash_writer.GetHash(), hash_verifier.GetHash()); +} + BOOST_AUTO_TEST_SUITE_END() From 8b72b8fe7cb9ffca2d1658a8f4b4dd04f81388cd Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 13 Jan 2023 14:12:25 -0500 Subject: [PATCH 09/18] addrdb: Only call Serialize() once The previous logic would call it once for serializing into the filestream, and then again for serializing into the hasher. If AddrMan was changed in between these calls by another thread, the resulting peers.dat would be corrupt with non-matching checksum and data. Fix this by using HashedSourceWriter, which writes the data to the underlying stream and keeps track of the hash in one go. Github-Pull: #26909 Rebased-From: 5eabb61b2386d00e93e6bbb2f493a56d1b326ad9 --- src/addrdb.cpp | 7 +++---- src/addrman.cpp | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 0fa8f3c3da..0d68719478 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -33,10 +33,9 @@ bool SerializeDB(Stream& stream, const Data& data) { // Write and commit header, data try { - CHashWriter hasher(stream.GetType(), stream.GetVersion()); - stream << Params().MessageStart() << data; - hasher << Params().MessageStart() << data; - stream << hasher.GetHash(); + HashedSourceWriter hashwriter{stream}; + hashwriter << Params().MessageStart() << data; + stream << hashwriter.GetHash(); } catch (const std::exception& e) { return error("%s: Serialize or I/O error - %s", __func__, e.what()); } diff --git a/src/addrman.cpp b/src/addrman.cpp index f91a979934..64137e1523 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -1171,8 +1171,7 @@ void AddrMan::Unserialize(Stream& s_) } // explicit instantiation -template void AddrMan::Serialize(CHashWriter& s) const; -template void AddrMan::Serialize(CAutoFile& s) const; +template void AddrMan::Serialize(HashedSourceWriter& s) const; template void AddrMan::Serialize(CDataStream& s) const; template void AddrMan::Unserialize(CAutoFile& s); template void AddrMan::Unserialize(CHashVerifier& s); From 274b2641454f9793df4993002a9dc4eaa7309b89 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 18 Feb 2025 18:17:33 -0800 Subject: [PATCH 10/18] Stop tracking univalue git-subtree --- ci/lint/06_script.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index 52d434afef..65c8022227 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -19,7 +19,6 @@ test/lint/git-subtree-check.sh src/crypto/ctaes test/lint/git-subtree-check.sh src/secp256k1 test/lint/git-subtree-check.sh src/simplicity test/lint/git-subtree-check.sh src/minisketch -test/lint/git-subtree-check.sh src/univalue test/lint/git-subtree-check.sh src/leveldb test/lint/git-subtree-check.sh src/crc32c test/lint/check-doc.py From dd116470f52511535421ff1a4b170e5ecfe635cb Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 9 May 2023 09:21:37 +0200 Subject: [PATCH 11/18] Add UniValue::find_value method --- src/univalue/include/univalue.h | 4 ++-- src/univalue/lib/univalue.cpp | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index fc5cf402be..b216d4035b 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -177,7 +177,7 @@ class UniValue { const UniValue& get_array() const; enum VType type() const { return getType(); } - friend const UniValue& find_value( const UniValue& obj, const std::string& name); + const UniValue& find_value(std::string_view key) const; }; enum jtokentype { @@ -235,6 +235,6 @@ static inline bool json_isspace(int ch) extern const UniValue NullUniValue; -const UniValue& find_value( const UniValue& obj, const std::string& name); +inline const UniValue& find_value(const UniValue& obj, const std::string& name) { return obj.find_value(name); } #endif // __UNIVALUE_H__ diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index c4e59fae74..0bc58ebd91 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -233,12 +233,13 @@ const char *uvTypeName(UniValue::VType t) return nullptr; } -const UniValue& find_value(const UniValue& obj, const std::string& name) +const UniValue& UniValue::find_value(std::string_view key) const { - for (unsigned int i = 0; i < obj.keys.size(); i++) - if (obj.keys[i] == name) - return obj.values.at(i); - + for (unsigned int i = 0; i < keys.size(); ++i) { + if (keys[i] == key) { + return values.at(i); + } + } return NullUniValue; } From f45869b0999e0f4ba82624c64270c76dc8fb2b82 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 18 Feb 2025 15:57:09 -0800 Subject: [PATCH 12/18] use o.find_value instead of find_value(o) to fix dangling pointer warning --- src/bitcoin-cli.cpp | 18 +++++++++--------- src/external_signer.cpp | 6 +++--- src/mainchainrpc.cpp | 4 ++-- src/rpc/mining.cpp | 10 +++++----- src/rpc/rawtransaction.cpp | 2 +- src/rpc/rawtransaction_util.cpp | 20 ++++++++++---------- src/test/system_tests.cpp | 4 ++-- src/wallet/rpc/coins.cpp | 2 +- src/wallet/rpc/spend.cpp | 6 +++--- src/wallet/wallet.cpp | 2 +- 10 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index efc3b5b250..fb19a1797b 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -848,7 +848,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str try { response = CallRPC(rh, strMethod, args, rpcwallet); if (fWait) { - const UniValue& error = find_value(response, "error"); + const UniValue& error = response.find_value("error"); if (!error.isNull() && error["code"].get_int() == RPC_IN_WARMUP) { throw CConnectionFailed("server in warmup"); } @@ -877,8 +877,8 @@ static void ParseResult(const UniValue& result, std::string& strPrint) static void ParseError(const UniValue& error, std::string& strPrint, int& nRet) { if (error.isObject()) { - const UniValue& err_code = find_value(error, "code"); - const UniValue& err_msg = find_value(error, "message"); + const UniValue& err_code = error.find_value("code"); + const UniValue& err_msg = error.find_value("message"); if (!err_code.isNull()) { strPrint = "error code: " + err_code.getValStr() + "\n"; } @@ -904,15 +904,15 @@ static void GetWalletBalances(UniValue& result) { DefaultRequestHandler rh; const UniValue listwallets = ConnectAndCallRPC(&rh, "listwallets", /* args=*/{}); - if (!find_value(listwallets, "error").isNull()) return; - const UniValue& wallets = find_value(listwallets, "result"); + if (!listwallets.find_value("error").isNull()) return; + const UniValue& wallets = listwallets.find_value("result"); if (wallets.size() <= 1) return; UniValue balances(UniValue::VOBJ); for (const UniValue& wallet : wallets.getValues()) { const std::string wallet_name = wallet.get_str(); const UniValue getbalances = ConnectAndCallRPC(&rh, "getbalances", /* args=*/{}, wallet_name); - const UniValue& balance = find_value(getbalances, "result")["mine"]["trusted"]; + const UniValue& balance = getbalances.find_value("result")["mine"]["trusted"]; balances.pushKV(wallet_name, balance); } result.pushKV("balances", balances); @@ -948,7 +948,7 @@ static void GetProgressBar(double progress, std::string& progress_bar) */ static void ParseGetInfoResult(UniValue& result) { - if (!find_value(result, "error").isNull()) return; + if (!result.find_value("error").isNull()) return; std::string RESET, GREEN, BLUE, YELLOW, MAGENTA, CYAN; bool should_colorize = false; @@ -1157,9 +1157,9 @@ static int CommandLineRPC(int argc, char *argv[]) rh.reset(new NetinfoRequestHandler()); } else if (gArgs.GetBoolArg("-generate", false)) { const UniValue getnewaddress{GetNewAddress()}; - const UniValue& error{find_value(getnewaddress, "error")}; + const UniValue& error{getnewaddress.find_value("error")}; if (error.isNull()) { - SetGenerateToAddressArgs(find_value(getnewaddress, "result").get_str(), args); + SetGenerateToAddressArgs(getnewaddress.find_value("result").get_str(), args); rh.reset(new GenerateToAddressRequestHandler()); } else { ParseError(error, strPrint, nRet); diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 75070899c6..6ecda22008 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -30,7 +30,7 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector new_reissuance; for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { if (!rawTx.vin[i].assetIssuance.IsNull()) { - const UniValue& blind_reissuance_v = find_value(request.params[0].get_array()[i].get_obj(), "blind_reissuance"); + const UniValue& blind_reissuance_v = request.params[0].get_array()[i].get_obj().find_value("blind_reissuance"); bool blind_reissuance = blind_reissuance_v.isNull() ? true : blind_reissuance_v.get_bool(); uint256 entropy; CAsset asset; diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 56a472a44c..23a2c5ef80 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -175,7 +175,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal uint256 txid = ParseHashO(o, "txid"); - const UniValue& vout_v = find_value(o, "vout"); + const UniValue& vout_v = o.find_value("vout"); if (!vout_v.isNum()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing vout key"); int nOutput = vout_v.get_int(); @@ -192,7 +192,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal } // set the sequence number if passed in the parameters object - const UniValue& sequenceObj = find_value(o, "sequence"); + const UniValue& sequenceObj = o.find_value("sequence"); if (sequenceObj.isNum()) { int64_t seqNr64 = sequenceObj.get_int64(); if (seqNr64 < 0 || seqNr64 > CTxIn::SEQUENCE_FINAL) { @@ -205,11 +205,11 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal CTxIn in(COutPoint(txid, nOutput), CScript(), nSequence); // Get issuance stuff if it's there - const UniValue& blinding_nonce_v = find_value(o, "asset_blinding_nonce"); - const UniValue& entropy_v = find_value(o, "asset_entropy"); - const UniValue& amount_v = find_value(o, "issuance_amount"); - const UniValue& issuance_tokens_v = find_value(o, "issuance_tokens"); - const UniValue& blind_reissuance_v = find_value(o, "blind_reissuance"); + const UniValue& blinding_nonce_v = o.find_value("asset_blinding_nonce"); + const UniValue& entropy_v = o.find_value("asset_entropy"); + const UniValue& amount_v = o.find_value("issuance_amount"); + const UniValue& issuance_tokens_v = o.find_value("issuance_tokens"); + const UniValue& blind_reissuance_v = o.find_value("blind_reissuance"); if (!amount_v.isNull() && allow_issuance) { if (!amount_v.isNum()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "issuance_amount is not a number"); @@ -242,9 +242,9 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal rawTx.vin.push_back(in); // Get the pegin stuff if it's there - const UniValue& pegin_tx = find_value(o, "pegin_bitcoin_tx"); - const UniValue& pegin_tx_proof = find_value(o, "pegin_txout_proof"); - const UniValue& pegin_script = find_value(o, "pegin_claim_script"); + const UniValue& pegin_tx = o.find_value("pegin_bitcoin_tx"); + const UniValue& pegin_tx_proof = o.find_value("pegin_txout_proof"); + const UniValue& pegin_script = o.find_value("pegin_claim_script"); if (!pegin_tx.isNull() && !pegin_tx_proof.isNull() && !pegin_script.isNull() && allow_peg_in) { if (!IsHex(pegin_script.get_str())) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Given claim_script is not hex."); diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 3f5353b5a2..5aaf7742c2 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -50,7 +50,7 @@ BOOST_AUTO_TEST_CASE(run_command) const UniValue result = RunCommandParseJSON("echo \"{\"success\": true}\""); #endif BOOST_CHECK(result.isObject()); - const UniValue& success = find_value(result, "success"); + const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); BOOST_CHECK_EQUAL(success.getBool(), true); } @@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(run_command) { const UniValue result = RunCommandParseJSON("cat", "{\"success\": true}"); BOOST_CHECK(result.isObject()); - const UniValue& success = find_value(result, "success"); + const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); BOOST_CHECK_EQUAL(success.getBool(), true); } diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 0068e5819d..d3102317b5 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -378,7 +378,7 @@ RPCHelpMan lockunspent() }); const uint256 txid(ParseHashO(o, "txid")); - const int nOutput = find_value(o, "vout").get_int(); + const int nOutput = o.find_value("vout").get_int(); if (nOutput < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative"); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0ead021dbb..022c4f681b 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -634,7 +634,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, for (const UniValue& input : options["input_weights"].get_array().getValues()) { uint256 txid = ParseHashO(input, "txid"); - const UniValue& vout_v = find_value(input, "vout"); + const UniValue& vout_v = input.find_value("vout"); if (!vout_v.isNum()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing vout key"); } @@ -643,7 +643,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative"); } - const UniValue& weight_v = find_value(input, "weight"); + const UniValue& weight_v = input.find_value("weight"); if (!weight_v.isNum()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, missing weight key"); } @@ -1617,7 +1617,7 @@ RPCHelpMan walletcreatefundedpsbt() std::set new_reissuance; for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { if (!rawTx.vin[i].assetIssuance.IsNull()) { - const UniValue& blind_reissuance_v = find_value(request.params[0].get_array()[i].get_obj(), "blind_reissuance"); + const UniValue& blind_reissuance_v = request.params[0].get_array()[i].get_obj().find_value("blind_reissuance"); bool blind_reissuance = blind_reissuance_v.isNull() ? true : blind_reissuance_v.get_bool(); uint256 entropy; CAsset asset; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ad9fe6d123..aca21428be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3701,7 +3701,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); for (bool internal : {false, true}) { - const UniValue& descriptor_vals = find_value(signer_res, internal ? "internal" : "receive"); + const UniValue& descriptor_vals = signer_res.find_value(internal ? "internal" : "receive"); if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) { std::string desc_str = desc_val.getValStr(); From 0d7ef225ce978741e223727aaec5e3d0c297b381 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Wed, 19 Feb 2025 07:48:02 -0800 Subject: [PATCH 13/18] More ::find_value fixes --- src/bitcoin-cli.cpp | 4 ++-- src/external_signer.cpp | 8 +++---- src/httprpc.cpp | 4 ++-- src/mainchainrpc.cpp | 4 ++-- src/qt/rpcconsole.cpp | 6 ++--- src/rpc/rawtransaction_util.cpp | 10 ++++---- src/rpc/request.cpp | 6 ++--- src/rpc/util.cpp | 10 ++++---- src/test/fuzz/rpc.cpp | 2 +- src/test/key_io_tests.cpp | 14 +++++------ src/test/rpc_tests.cpp | 42 ++++++++++++++++----------------- 11 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index fb19a1797b..85048028d5 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -1181,8 +1181,8 @@ static int CommandLineRPC(int argc, char *argv[]) const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name); // Parse reply - UniValue result = find_value(reply, "result"); - const UniValue& error = find_value(reply, "error"); + UniValue result = reply.find_value("result"); + const UniValue& error = reply.find_value("error"); if (error.isNull()) { if (gArgs.GetBoolArg("-getinfo", false)) { if (!gArgs.IsArgSet("-rpcwallet")) { diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 6ecda22008..a16e2e6afc 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -93,19 +93,19 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str const UniValue signer_result = RunCommandParseJSON(command, stdinStr); - if (find_value(signer_result, "error").isStr()) { - error = find_value(signer_result, "error").get_str(); + if (signer_result.find_value("error").isStr()) { + error = signer_result.find_value("error").get_str(); return false; } - if (!find_value(signer_result, "psbt").isStr()) { + if (!signer_result.find_value("psbt").isStr()) { error = "Unexpected result from signer"; return false; } PartiallySignedTransaction signer_psbtx; std::string signer_psbt_error; - if (!DecodeBase64PSBT(signer_psbtx, find_value(signer_result, "psbt").get_str(), signer_psbt_error)) { + if (!DecodeBase64PSBT(signer_psbtx, signer_result.find_value("psbt").get_str(), signer_psbt_error)) { error = strprintf("TX decode failed %s", signer_psbt_error); return false; } diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 5d0b59f7cb..2f5493ed42 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -79,7 +79,7 @@ static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const Uni { // Send error reply from json-rpc error object int nStatus = HTTP_INTERNAL_SERVER_ERROR; - int code = find_value(objError, "code").get_int(); + int code = objError.find_value("code").get_int(); if (code == RPC_INVALID_REQUEST) nStatus = HTTP_BAD_REQUEST; @@ -213,7 +213,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) } else { const UniValue& request = valRequest[reqIdx].get_obj(); // Parse method - std::string strMethod = find_value(request, "method").get_str(); + std::string strMethod = request.find_value("method").get_str(); if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) { LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod); req->WriteReply(HTTP_FORBIDDEN); diff --git a/src/mainchainrpc.cpp b/src/mainchainrpc.cpp index de5001680b..f6bcbc9bb6 100644 --- a/src/mainchainrpc.cpp +++ b/src/mainchainrpc.cpp @@ -169,7 +169,7 @@ bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDept return false; } - UniValue confirmations = find_value(result.get_obj(), "confirmations"); + UniValue confirmations = result.get_obj().find_value("confirmations"); if (!confirmations.isNum() || confirmations.get_int64() < nMinConfirmationDepth) { LogPrintf("Insufficient confirmations (got %s, need at least %d).\n", confirmations.write(), nMinConfirmationDepth); return false; @@ -177,7 +177,7 @@ bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDept // Only perform extra test if nbTxs has been provided (non-zero). if (nbTxs != 0) { - UniValue nTx = find_value(result.get_obj(), "nTx"); + UniValue nTx = result.get_obj().find_value("nTx"); if (!nTx.isNum() || nTx.get_int64() != nbTxs) { LogPrintf("ERROR: Invalid number of transactions in merkle block for %s (got %s, need exactly %d)\n", hash.GetHex(), nTx.write(), nbTxs); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 9fd3b8b6fe..b22d25d507 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -257,7 +257,7 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes subelement = lastResult[parsed.value()]; } else if (lastResult.isObject()) - subelement = find_value(lastResult, curarg); + subelement = lastResult.find_value( curarg); else throw std::runtime_error("Invalid result query"); //no array or object: abort lastResult = subelement; @@ -456,8 +456,8 @@ void RPCExecutor::request(const QString &command, const WalletModel* wallet_mode { try // Nice formatting for standard-format error { - int code = find_value(objError, "code").get_int(); - std::string message = find_value(objError, "message").get_str(); + int code = objError.find_value("code").get_int(); + std::string message = objError.find_value("message").get_str(); Q_EMIT reply(RPCConsole::CMD_ERROR, QString::fromStdString(message) + " (code " + QString::number(code) + ")"); } catch (const std::runtime_error&) // raised when converting to invalid type, i.e. missing code or message diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 23a2c5ef80..f4d61bb283 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -335,7 +335,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal out.nAsset = CAsset(ParseHashO(output, name_)); } else if (name_ == "blinder_index") { // For PSET - psbt_out.m_blinder_index = find_value(output, name_).get_int(); + psbt_out.m_blinder_index = output.find_value(name_).get_int(); } else { CTxDestination destination = DecodeDestination(name_); if (!IsValidDestination(destination)) { @@ -429,7 +429,7 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst uint256 txid = ParseHashO(prevOut, "txid"); - int nOut = find_value(prevOut, "vout").get_int(); + int nOut = prevOut.find_value("vout").get_int(); if (nOut < 0) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "vout cannot be negative"); } @@ -450,7 +450,7 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst newcoin.out.scriptPubKey = scriptPubKey; newcoin.out.nValue = CConfidentialValue(MAX_MONEY); if (prevOut.exists("amount")) { - newcoin.out.nValue = CConfidentialValue(AmountFromValue(find_value(prevOut, "amount"))); + newcoin.out.nValue = CConfidentialValue(AmountFromValue(prevOut.find_value("amount"))); } else if (prevOut.exists("amountcommitment")) { // Segwit sigs require the amount commitment to be sighashed newcoin.out.nValue.vchCommitment = ParseHexO(prevOut, "amountcommitment"); @@ -468,8 +468,8 @@ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keyst {"redeemScript", UniValueType(UniValue::VSTR)}, {"witnessScript", UniValueType(UniValue::VSTR)}, }, true); - UniValue rs = find_value(prevOut, "redeemScript"); - UniValue ws = find_value(prevOut, "witnessScript"); + UniValue rs = prevOut.find_value("redeemScript"); + UniValue ws = prevOut.find_value("witnessScript"); if (rs.isNull() && ws.isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript"); } diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index f332eaa46b..90ff4b995c 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -202,10 +202,10 @@ void JSONRPCRequest::parse(const UniValue& valRequest) const UniValue& request = valRequest.get_obj(); // Parse id now so errors from here on will have the id - id = find_value(request, "id"); + id = request.find_value("id"); // Parse method - UniValue valMethod = find_value(request, "method"); + UniValue valMethod = request.find_value("method"); if (valMethod.isNull()) throw JSONRPCError(RPC_INVALID_REQUEST, "Missing method"); if (!valMethod.isStr()) @@ -218,7 +218,7 @@ void JSONRPCRequest::parse(const UniValue& valRequest) LogPrint(BCLog::RPC, "ThreadRPCServer method=%s user=%s\n", SanitizeString(strMethod), this->authUser); // Parse params - UniValue valParams = find_value(request, "params"); + UniValue valParams = request.find_value("params"); if (valParams.isArray() || valParams.isObject()) params = valParams; else if (valParams.isNull()) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 71e896951c..149f9ff85a 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -66,7 +66,7 @@ void RPCTypeCheckObj(const UniValue& o, bool fStrict) { for (const auto& t : typesExpected) { - const UniValue& v = find_value(o, t.first); + const UniValue& v = o.find_value(t.first); if (!fAllowNull && v.isNull()) throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing %s", t.first)); @@ -113,7 +113,7 @@ uint256 ParseHashV(const UniValue& v, std::string strName) } uint256 ParseHashO(const UniValue& o, std::string strKey) { - return ParseHashV(find_value(o, strKey), strKey); + return ParseHashV(o.find_value(strKey), strKey); } std::vector ParseHexV(const UniValue& v, std::string strName) { @@ -126,7 +126,7 @@ std::vector ParseHexV(const UniValue& v, std::string strName) } std::vector ParseHexO(const UniValue& o, std::string strKey) { - return ParseHexV(find_value(o, strKey), strKey); + return ParseHexV(o.find_value(strKey), strKey); } namespace { @@ -1032,10 +1032,10 @@ std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, Fl if (scanobject.isStr()) { desc_str = scanobject.get_str(); } else if (scanobject.isObject()) { - UniValue desc_uni = find_value(scanobject, "desc"); + UniValue desc_uni = scanobject.find_value("desc"); if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); desc_str = desc_uni.get_str(); - UniValue range_uni = find_value(scanobject, "range"); + UniValue range_uni = scanobject.find_value("range"); if (!range_uni.isNull()) { range = ParseDescriptorRange(range_uni); } diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 150616d79b..4cddc89d6d 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -382,7 +382,7 @@ FUZZ_TARGET_INIT(rpc, initialize_rpc) try { rpc_testing_setup->CallRPC(rpc_command, arguments); } catch (const UniValue& json_rpc_error) { - const std::string error_msg{find_value(json_rpc_error, "message").get_str()}; + const std::string error_msg{json_rpc_error.find_value( "message").get_str()}; // Once c++20 is allowed, starts_with can be used. // if (error_msg.starts_with("Internal bug detected")) { if (0 == error_msg.rfind("Internal bug detected", 0)) { diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp index b06157e99f..30eb45f268 100644 --- a/src/test/key_io_tests.cpp +++ b/src/test/key_io_tests.cpp @@ -37,11 +37,11 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse) std::string exp_base58string = test[0].get_str(); std::vector exp_payload = ParseHex(test[1].get_str()); const UniValue &metadata = test[2].get_obj(); - bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); - SelectParams(find_value(metadata, "chain").get_str()); - bool try_case_flip = find_value(metadata, "tryCaseFlip").isNull() ? false : find_value(metadata, "tryCaseFlip").get_bool(); + bool isPrivkey = metadata.find_value( "isPrivkey").get_bool(); + SelectParams(metadata.find_value( "chain").get_str()); + bool try_case_flip = metadata.find_value( "tryCaseFlip").isNull() ? false : metadata.find_value( "tryCaseFlip").get_bool(); if (isPrivkey) { - bool isCompressed = find_value(metadata, "isCompressed").get_bool(); + bool isCompressed = metadata.find_value( "isCompressed").get_bool(); // Must be valid private key privkey = DecodeSecret(exp_base58string); BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest); @@ -96,10 +96,10 @@ BOOST_AUTO_TEST_CASE(key_io_valid_gen) std::string exp_base58string = test[0].get_str(); std::vector exp_payload = ParseHex(test[1].get_str()); const UniValue &metadata = test[2].get_obj(); - bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); - SelectParams(find_value(metadata, "chain").get_str()); + bool isPrivkey = metadata.find_value( "isPrivkey").get_bool(); + SelectParams(metadata.find_value( "chain").get_str()); if (isPrivkey) { - bool isCompressed = find_value(metadata, "isCompressed").get_bool(); + bool isCompressed = metadata.find_value( "isCompressed").get_bool(); CKey key; key.Set(exp_payload.begin(), exp_payload.end(), isCompressed); assert(key.IsValid()); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 2af68839b1..bb140fede6 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -43,7 +43,7 @@ UniValue RPCTestingSetup::CallRPC(std::string args) return result; } catch (const UniValue& objError) { - throw std::runtime_error(find_value(objError, "message").get_str()); + throw std::runtime_error(objError.find_value("message").get_str()); } } @@ -71,9 +71,9 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams) BOOST_CHECK_THROW(CallRPC("decoderawtransaction DEADBEEF"), std::runtime_error); std::string rawtx = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000"; BOOST_CHECK_NO_THROW(r = CallRPC(std::string("decoderawtransaction ")+rawtx)); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "size").get_int(), 193); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "version").get_int(), 1); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "locktime").get_int(), 0); + BOOST_CHECK_EQUAL(r.get_obj().find_value( "size").get_int(), 193); + BOOST_CHECK_EQUAL(r.get_obj().find_value( "version").get_int(), 1); + BOOST_CHECK_EQUAL(r.get_obj().find_value( "locktime").get_int(), 0); BOOST_CHECK_THROW(CallRPC(std::string("decoderawtransaction ")+rawtx+" extra"), std::runtime_error); BOOST_CHECK_NO_THROW(r = CallRPC(std::string("decoderawtransaction ")+rawtx+" false")); BOOST_CHECK_THROW(r = CallRPC(std::string("decoderawtransaction ")+rawtx+" false extra"), std::runtime_error); @@ -90,20 +90,20 @@ BOOST_AUTO_TEST_CASE(rpc_togglenetwork) UniValue r; r = CallRPC("getnetworkinfo"); - bool netState = find_value(r.get_obj(), "networkactive").get_bool(); + bool netState = r.get_obj().find_value( "networkactive").get_bool(); BOOST_CHECK_EQUAL(netState, true); BOOST_CHECK_NO_THROW(CallRPC("setnetworkactive false")); r = CallRPC("getnetworkinfo"); - int numConnection = find_value(r.get_obj(), "connections").get_int(); + int numConnection = r.get_obj().find_value( "connections").get_int(); BOOST_CHECK_EQUAL(numConnection, 0); - netState = find_value(r.get_obj(), "networkactive").get_bool(); + netState = r.get_obj().find_value( "networkactive").get_bool(); BOOST_CHECK_EQUAL(netState, false); BOOST_CHECK_NO_THROW(CallRPC("setnetworkactive true")); r = CallRPC("getnetworkinfo"); - netState = find_value(r.get_obj(), "networkactive").get_bool(); + netState = r.get_obj().find_value( "networkactive").get_bool(); BOOST_CHECK_EQUAL(netState, true); } @@ -121,9 +121,9 @@ BOOST_AUTO_TEST_CASE(rpc_rawsign) std::string privkey1 = "\"KzsXybp9jX64P5ekX1KUxRQ79Jht9uzW7LorgwE65i5rWACL6LQe\""; std::string privkey2 = "\"Kyhdf5LuKTRx4ge69ybABsiUAWjVRK4XGxAKk2FQLp2HjGMy87Z4\""; r = CallRPC(std::string("signrawtransactionwithkey ")+notsigned+" [] "+prevout); - BOOST_CHECK(find_value(r.get_obj(), "complete").get_bool() == false); + BOOST_CHECK(r.get_obj().find_value( "complete").get_bool() == false); r = CallRPC(std::string("signrawtransactionwithkey ")+notsigned+" ["+privkey1+","+privkey2+"] "+prevout); - BOOST_CHECK(find_value(r.get_obj(), "complete").get_bool() == true); + BOOST_CHECK(r.get_obj().find_value( "complete").get_bool() == true); } BOOST_AUTO_TEST_CASE(rpc_createraw_op_return) @@ -257,7 +257,7 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); UniValue ar = r.get_array(); UniValue o1 = ar[0].get_obj(); - UniValue adr = find_value(o1, "address"); + UniValue adr = o1.find_value( "address"); BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/32"); BOOST_CHECK_NO_THROW(CallRPC(std::string("setban 127.0.0.0 remove"))); BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); @@ -268,8 +268,8 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); - adr = find_value(o1, "address"); - int64_t banned_until{find_value(o1, "banned_until").get_int64()}; + adr = o1.find_value( "address"); + int64_t banned_until{o1.find_value( "banned_until").get_int64()}; BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24"); BOOST_CHECK_EQUAL(banned_until, 9907731200); // absolute time check @@ -283,11 +283,11 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); - adr = find_value(o1, "address"); - banned_until = find_value(o1, "banned_until").get_int64(); - const int64_t ban_created{find_value(o1, "ban_created").get_int64()}; - const int64_t ban_duration{find_value(o1, "ban_duration").get_int64()}; - const int64_t time_remaining{find_value(o1, "time_remaining").get_int64()}; + adr = o1.find_value( "address"); + banned_until = o1.find_value( "banned_until").get_int64(); + const int64_t ban_created{o1.find_value( "ban_created").get_int64()}; + const int64_t ban_duration{o1.find_value( "ban_duration").get_int64()}; + const int64_t time_remaining{o1.find_value( "time_remaining").get_int64()}; BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24"); BOOST_CHECK_EQUAL(banned_until, time_remaining_expected + now.count()); BOOST_CHECK_EQUAL(ban_duration, banned_until - ban_created); @@ -317,7 +317,7 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); - adr = find_value(o1, "address"); + adr = o1.find_value( "address"); BOOST_CHECK_EQUAL(adr.get_str(), "fe80::202:b3ff:fe1e:8329/128"); BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned"))); @@ -325,7 +325,7 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); - adr = find_value(o1, "address"); + adr = o1.find_value( "address"); BOOST_CHECK_EQUAL(adr.get_str(), "2001:db8::/30"); BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned"))); @@ -333,7 +333,7 @@ BOOST_AUTO_TEST_CASE(rpc_ban) BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned"))); ar = r.get_array(); o1 = ar[0].get_obj(); - adr = find_value(o1, "address"); + adr = o1.find_value( "address"); BOOST_CHECK_EQUAL(adr.get_str(), "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/128"); } From df1999c009b49438c73fb5e1230d46ee96c15533 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 10 Mar 2025 11:41:15 -0700 Subject: [PATCH 14/18] feature_trim_headers takes too long --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index e87376574f..08df63a376 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -159,7 +159,7 @@ task: # Exclude feature_dbcrash for now due to timeout # Exclude also wallet_avoidreuse due to timeout # Ignore failures for now, need to investigate but we really don't use native win64 builds - - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --extended --exclude feature_dbcrash,wallet_avoidreuse || true + - python test\functional\test_runner.py --tmpdirprefix=artifactsdir/ --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --extended --exclude feature_dbcrash,wallet_avoidreuse,feature_trim_headers || true artifacts: paths: From 8fc250c4e2853d11436d58fb7df8243fbcfd1e95 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 10 Mar 2025 18:13:07 -0700 Subject: [PATCH 15/18] restore macos native --- .cirrus.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.cirrus.yml b/.cirrus.yml index 08df63a376..e46c1be0ab 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -167,3 +167,14 @@ task: - artifactsdir/**/tmp* - artifactsdir/**/*.conf +task: + name: 'macOS 13 native arm64 [gui, sqlite only] [no depends]' + macos_instance: + # Use latest image, but hardcode version to avoid silent upgrades (and breaks) + image: ghcr.io/cirruslabs/macos-ventura-xcode:14.1 # https://cirrus-ci.org/guide/macOS + << : *MACOS_NATIVE_TASK_TEMPLATE + env: + << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV + CI_USE_APT_INSTALL: "no" + PACKAGE_MANAGER_INSTALL: "echo" # Nothing to do + FILE_ENV: "./ci/test/00_setup_env_mac_native_arm64.sh" From ea9e1dd71d20fa30f91d4cd6be323e4e6899df79 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 10 Mar 2025 18:17:25 -0700 Subject: [PATCH 16/18] test_pip3 --- ci/test/04_install.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index a555d94c18..4bcda79510 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -11,7 +11,8 @@ if [[ $QEMU_USER_CMD == qemu-s390* ]]; then fi if [ "$CI_OS_NAME" == "macos" ]; then - sudo -H pip3 install --upgrade --break-system-packages pip + pip3 --version || true + sudo -H pip3 install --upgrade --break-system-packages pip || sudo -H pip3 install --upgrade pip # shellcheck disable=SC2086 IN_GETOPT_BIN="$(brew --prefix gnu-getopt)/bin/getopt" ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES fi From 4c04a6d2379444385606dab9ca4b26625f93785a Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 11 Mar 2025 07:26:51 -0700 Subject: [PATCH 17/18] Revert "test_pip3" This reverts commit ea9e1dd71d20fa30f91d4cd6be323e4e6899df79. --- ci/test/04_install.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 4bcda79510..a555d94c18 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -11,8 +11,7 @@ if [[ $QEMU_USER_CMD == qemu-s390* ]]; then fi if [ "$CI_OS_NAME" == "macos" ]; then - pip3 --version || true - sudo -H pip3 install --upgrade --break-system-packages pip || sudo -H pip3 install --upgrade pip + sudo -H pip3 install --upgrade --break-system-packages pip # shellcheck disable=SC2086 IN_GETOPT_BIN="$(brew --prefix gnu-getopt)/bin/getopt" ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES fi From ece24bd39beab464a3bfb4cf6b54af4c2462fcf6 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Tue, 11 Mar 2025 07:27:00 -0700 Subject: [PATCH 18/18] Revert "Apply acc06bc91f80ddf4e015dcdf0b984bbdbfcb5ca3 fix also to pip upgrade" This reverts commit 49f151f0133790fcd2bd2261e881d43b7b30fb35. --- ci/test/04_install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index a555d94c18..1e8ec18840 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -11,7 +11,7 @@ if [[ $QEMU_USER_CMD == qemu-s390* ]]; then fi if [ "$CI_OS_NAME" == "macos" ]; then - sudo -H pip3 install --upgrade --break-system-packages pip + sudo -H pip3 install --upgrade pip # shellcheck disable=SC2086 IN_GETOPT_BIN="$(brew --prefix gnu-getopt)/bin/getopt" ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES fi