From ffa27af24da81a97d6c4912ae0e10bc5b6f17f69 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 10 May 2024 16:06:03 -0400 Subject: [PATCH 01/13] test: Add check-deps.sh script to check for unexpected library dependencies --- contrib/devtools/check-deps.sh | 203 +++++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100755 contrib/devtools/check-deps.sh diff --git a/contrib/devtools/check-deps.sh b/contrib/devtools/check-deps.sh new file mode 100755 index 00000000000..9d2eebe14d8 --- /dev/null +++ b/contrib/devtools/check-deps.sh @@ -0,0 +1,203 @@ +#!/usr/bin/env bash + +export LC_ALL=C +set -Eeuo pipefail + +# Declare paths to libraries +declare -A LIBS +LIBS[cli]="libbitcoin_cli.a" +LIBS[common]="libbitcoin_common.a" +LIBS[consensus]="libbitcoin_consensus.a" +LIBS[crypto]="crypto/.libs/libbitcoin_crypto_base.a crypto/.libs/libbitcoin_crypto_x86_shani.a crypto/.libs/libbitcoin_crypto_sse41.a crypto/.libs/libbitcoin_crypto_avx2.a" +LIBS[node]="libbitcoin_node.a" +LIBS[util]="libbitcoin_util.a" +LIBS[wallet]="libbitcoin_wallet.a" +LIBS[wallet_tool]="libbitcoin_wallet_tool.a" + +# Declare allowed dependencies "X Y" where X is allowed to depend on Y. This +# list is taken from doc/design/libraries.md. +ALLOWED_DEPENDENCIES=( + "cli common" + "cli util" + "common consensus" + "common crypto" + "common util" + "consensus crypto" + "node common" + "node consensus" + "node crypto" + "node kernel" + "node util" + "util crypto" + "wallet common" + "wallet crypto" + "wallet util" + "wallet_tool util" + "wallet_tool wallet" +) + +# Add minor dependencies omitted from doc/design/libraries.md to keep the +# dependency diagram simple. +ALLOWED_DEPENDENCIES+=( + "wallet consensus" + "wallet_tool common" + "wallet_tool crypto" +) + +# Declare list of known errors that should be suppressed. +declare -A SUPPRESS +# init.cpp file currently calls Berkeley DB sanity check function on startup, so +# there is an undocumented dependency of the node library on the wallet library. +SUPPRESS["libbitcoin_node_a-init.o libbitcoin_wallet_a-bdb.o _ZN6wallet27BerkeleyDatabaseSanityCheckEv"]=1 +# init/common.cpp file calls InitError and InitWarning from interface_ui which +# is currently part of the node library. interface_ui should just be part of the +# common library instead, and is moved in +# https://github.com/bitcoin/bitcoin/issues/10102 +SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z11InitWarningRK13bilingual_str"]=1 +SUPPRESS["libbitcoin_common_a-common.o libbitcoin_node_a-interface_ui.o _Z9InitErrorRK13bilingual_str"]=1 +# rpc/external_signer.cpp adds defines node RPC methods but is built as part of the +# common library. It should be moved to the node library instead. +SUPPRESS["libbitcoin_common_a-external_signer.o libbitcoin_node_a-server.o _ZN9CRPCTable13appendCommandERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPK11CRPCCommand"]=1 + +usage() { + echo "Usage: $(basename "${BASH_SOURCE[0]}") [BUILD_DIR]" +} + +# Output makefile targets, converting library .a paths to libtool .la targets +lib_targets() { + for lib in "${!LIBS[@]}"; do + for lib_path in ${LIBS[$lib]}; do + # shellcheck disable=SC2001 + sed 's:/.libs/\(.*\)\.a$:/\1.la:g' <<<"$lib_path" + done + done +} + +# Extract symbol names and object names and write to text files +extract_symbols() { + local temp_dir="$1" + for lib in "${!LIBS[@]}"; do + for lib_path in ${LIBS[$lib]}; do + nm -o "$lib_path" | grep ' T ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_exports.txt" + nm -o "$lib_path" | grep ' U ' | awk '{print $3, $1}' >> "${temp_dir}/${lib}_imports.txt" + awk '{print $1}' "${temp_dir}/${lib}_exports.txt" | sort -u > "${temp_dir}/${lib}_exported_symbols.txt" + awk '{print $1}' "${temp_dir}/${lib}_imports.txt" | sort -u > "${temp_dir}/${lib}_imported_symbols.txt" + done + done +} + +# Lookup object name(s) corresponding to symbol name in text file +obj_names() { + local symbol="$1" + local txt_file="$2" + sed -n "s/^$symbol [^:]\\+:\\([^:]\\+\\):[^:]*\$/\\1/p" "$txt_file" | sort -u +} + +# Iterate through libraries and find disallowed dependencies +check_libraries() { + local temp_dir="$1" + local result=0 + for src in "${!LIBS[@]}"; do + for dst in "${!LIBS[@]}"; do + if [ "$src" != "$dst" ] && ! is_allowed "$src" "$dst"; then + if ! check_disallowed "$src" "$dst" "$temp_dir"; then + result=1 + fi + fi + done + done + check_not_suppressed + return $result +} + +# Return whether src library is allowed to depend on dst. +is_allowed() { + local src="$1" + local dst="$2" + for allowed in "${ALLOWED_DEPENDENCIES[@]}"; do + if [ "$src $dst" = "$allowed" ]; then + return 0 + fi + done + return 1 +} + +# Return whether src library imports any symbols from dst, assuming src is not +# allowed to depend on dst. +check_disallowed() { + local src="$1" + local dst="$2" + local temp_dir="$3" + local result=0 + + # Loop over symbol names exported by dst and imported by src + while read symbol; do + local dst_obj + dst_obj=$(obj_names "$symbol" "${temp_dir}/${dst}_exports.txt") + while read src_obj; do + if ! check_suppress "$src_obj" "$dst_obj" "$symbol"; then + echo "Error: $src_obj depends on $dst_obj symbol '$(c++filt "$symbol")', can suppess with:" + echo " SUPPRESS[\"$src_obj $dst_obj $symbol\"]=1" + result=1 + fi + done < <(obj_names "$symbol" "${temp_dir}/${src}_imports.txt") + done < <(comm -12 "${temp_dir}/${dst}_exported_symbols.txt" "${temp_dir}/${src}_imported_symbols.txt") + return $result +} + +# Declare array to track errors which were suppressed. +declare -A SUPPRESSED + +# Return whether error should be suppressed and record suppresssion in +# SUPPRESSED array. +check_suppress() { + local src_obj="$1" + local dst_obj="$2" + local symbol="$3" + for suppress in "${!SUPPRESS[@]}"; do + read suppress_src suppress_dst suppress_pattern <<<"$suppress" + if [[ "$src_obj" == "$suppress_src" && "$dst_obj" == "$suppress_dst" && "$symbol" =~ $suppress_pattern ]]; then + SUPPRESSED["$suppress"]=1 + return 0 + fi + done + return 1 +} + +# Warn about error which were supposed to be suppress, but were not encountered. +check_not_suppressed() { + for suppress in "${!SUPPRESS[@]}"; do + if [[ ! -v SUPPRESSED[$suppress] ]]; then + echo >&2 "Warning: suppression '$suppress' was ignored, consider deleting." + fi + done +} + +# Check arguments. +if [ "$#" = 0 ]; then + BUILD_DIR="$(dirname "${BASH_SOURCE[0]}")/../../src" +elif [ "$#" = 1 ]; then + BUILD_DIR="$1" +else + echo >&2 "Error: wrong number of arguments." + usage >&2 + exit 1 +fi +if [ ! -f "$BUILD_DIR/Makefile" ]; then + echo >&2 "Error: directory '$BUILD_DIR' does not contain a makefile, please specify path to build directory for library targets." + usage >&2 + exit 1 +fi + +# Build libraries and run checks. +cd "$BUILD_DIR" +# shellcheck disable=SC2046 +make -j"$(nproc)" $(lib_targets) +TEMP_DIR="$(mktemp -d)" +extract_symbols "$TEMP_DIR" +if check_libraries "$TEMP_DIR"; then + echo "Success! No unexpected dependencies were detected." +else + echo >&2 "Error: Unexpected dependencies were detected. Check previous output." +fi +rm -r "$TEMP_DIR" From 5b9309420cc9721a0d5745b6ad3166a4bdbd1508 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 10 May 2024 15:45:07 -0400 Subject: [PATCH 02/13] build: move chainparamsbase from util to common Move chainparamsbase from util to common, because util library should not depend on the common library and chainparamsbase uses the ArgsManager class in common. --- src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index b749651b726..1fd0cdd7a80 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -669,6 +669,7 @@ libbitcoin_common_a_SOURCES = \ addresstype.cpp \ base58.cpp \ bech32.cpp \ + chainparamsbase.cpp \ chainparams.cpp \ coins.cpp \ common/args.cpp \ @@ -719,7 +720,6 @@ libbitcoin_util_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) libbitcoin_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_util_a_SOURCES = \ support/lockedpool.cpp \ - chainparamsbase.cpp \ clientversion.cpp \ logging.cpp \ random.cpp \ From cc5f29fbea15d33e4d1aa95591253c6b86953fe7 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 10 May 2024 15:45:07 -0400 Subject: [PATCH 03/13] build: move memory_cleanse from util to crypto Move memory_cleanse from util to crypto because the crypto library should not depend on other libraries, and it calls memory_cleanse. --- src/Makefile.am | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 1fd0cdd7a80..bb61f2d0276 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -587,7 +587,8 @@ crypto_libbitcoin_crypto_base_la_SOURCES = \ crypto/sha512.cpp \ crypto/sha512.h \ crypto/siphash.cpp \ - crypto/siphash.h + crypto/siphash.h \ + support/cleanse.cpp # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and # CXXFLAGS above @@ -725,7 +726,6 @@ libbitcoin_util_a_SOURCES = \ random.cpp \ randomenv.cpp \ streams.cpp \ - support/cleanse.cpp \ sync.cpp \ util/asmap.cpp \ util/batchpriority.cpp \ @@ -968,7 +968,6 @@ libbitcoinkernel_la_SOURCES = \ script/solver.cpp \ signet.cpp \ streams.cpp \ - support/cleanse.cpp \ support/lockedpool.cpp \ sync.cpp \ txdb.cpp \ From 6861f954f8ff42c87ad638037adae86a5bd89600 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 10 May 2024 15:45:07 -0400 Subject: [PATCH 04/13] util: move util/message to common/signmessage Move util/message to common/signmessage so it is named more clearly, and because the util library is not supposed to depend on other libraries besides the crypto library. The signmessage functions use CKey, CPubKey, PKHash, and DecodeDestination functions in the consensus and common libraries. --- src/Makefile.am | 4 ++-- src/{util/message.cpp => common/signmessage.cpp} | 2 +- src/{util/message.h => common/signmessage.h} | 6 +++--- src/interfaces/wallet.h | 2 +- src/qt/signverifymessagedialog.cpp | 2 +- src/rpc/signmessage.cpp | 2 +- src/test/fuzz/message.cpp | 2 +- src/test/util_tests.cpp | 2 +- src/wallet/rpc/signmessage.cpp | 2 +- src/wallet/scriptpubkeyman.h | 2 +- src/wallet/wallet.cpp | 2 +- 11 files changed, 14 insertions(+), 14 deletions(-) rename src/{util/message.cpp => common/signmessage.cpp} (98%) rename src/{util/message.h => common/signmessage.h} (95%) diff --git a/src/Makefile.am b/src/Makefile.am index bb61f2d0276..54000a7eb19 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -144,6 +144,7 @@ BITCOIN_CORE_H = \ compat/cpuid.h \ compat/endian.h \ common/settings.h \ + common/signmessage.h \ common/system.h \ compressor.h \ consensus/consensus.h \ @@ -308,7 +309,6 @@ BITCOIN_CORE_H = \ util/hasher.h \ util/insert.h \ util/macros.h \ - util/message.h \ util/moneystr.h \ util/overflow.h \ util/overloaded.h \ @@ -680,6 +680,7 @@ libbitcoin_common_a_SOURCES = \ common/interfaces.cpp \ common/run_command.cpp \ common/settings.cpp \ + common/signmessage.cpp \ common/system.cpp \ common/url.cpp \ compressor.cpp \ @@ -742,7 +743,6 @@ libbitcoin_util_a_SOURCES = \ util/hasher.cpp \ util/sock.cpp \ util/syserror.cpp \ - util/message.cpp \ util/moneystr.cpp \ util/rbf.cpp \ util/readwritefile.cpp \ diff --git a/src/util/message.cpp b/src/common/signmessage.cpp similarity index 98% rename from src/util/message.cpp rename to src/common/signmessage.cpp index 1afb28cc10d..1612751e44d 100644 --- a/src/util/message.cpp +++ b/src/common/signmessage.cpp @@ -3,12 +3,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include #include #include -#include #include #include diff --git a/src/util/message.h b/src/common/signmessage.h similarity index 95% rename from src/util/message.h rename to src/common/signmessage.h index d0e2422574b..215b563bbb2 100644 --- a/src/util/message.h +++ b/src/common/signmessage.h @@ -3,8 +3,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_MESSAGE_H -#define BITCOIN_UTIL_MESSAGE_H +#ifndef BITCOIN_COMMON_SIGNMESSAGE_H +#define BITCOIN_COMMON_SIGNMESSAGE_H #include @@ -74,4 +74,4 @@ uint256 MessageHash(const std::string& message); std::string SigningResultString(const SigningResult res); -#endif // BITCOIN_UTIL_MESSAGE_H +#endif // BITCOIN_COMMON_SIGNMESSAGE_H diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index c41f35829d7..f1bfffa7d46 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -6,13 +6,13 @@ #define BITCOIN_INTERFACES_WALLET_H #include +#include #include #include #include #include