diff options
164 files changed, 1678 insertions, 1490 deletions
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b92e3c66d0..44521c1af3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -295,3 +295,16 @@ jobs: TEST_RUNNER_EXTRA: ${{ github.event_name != 'pull_request' && '--extended' || '' }} shell: cmd run: py -3 test\functional\test_runner.py --jobs %NUMBER_OF_PROCESSORS% --ci --quiet --tmpdirprefix=%RUNNER_TEMP% --combinedlogslen=99999999 --timeout-factor=%TEST_RUNNER_TIMEOUT_FACTOR% %TEST_RUNNER_EXTRA% + + - name: Clone fuzz corpus + run: | + git clone --depth=1 https://github.com/bitcoin-core/qa-assets "$env:RUNNER_TEMP\qa-assets" + Set-Location "$env:RUNNER_TEMP\qa-assets" + Write-Host "Using qa-assets repo from commit ..." + git log -1 + + - name: Run fuzz binaries + env: + BITCOINFUZZ: "${{ github.workspace}}\\src\\fuzz.exe" + shell: cmd + run: py -3 test\fuzz\test_runner.py --par %NUMBER_OF_PROCESSORS% --loglevel DEBUG %RUNNER_TEMP%\qa-assets\fuzz_seed_corpus diff --git a/Makefile.am b/Makefile.am index cd1509a0ff..e79dc4e21b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -126,7 +126,7 @@ $(OSX_ZIP): deploydir cd $(APP_DIST_DIR) && find . | sort | $(ZIP) -X@ $@ $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING) - INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) STRIP=$(STRIP) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) + OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) deploydir: $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt endif !BUILD_DARWIN diff --git a/build-aux/m4/l_atomic.m4 b/build-aux/m4/l_atomic.m4 index aa00168fce..859ddaabbb 100644 --- a/build-aux/m4/l_atomic.m4 +++ b/build-aux/m4/l_atomic.m4 @@ -7,7 +7,7 @@ dnl warranty. # Clang, when building for 32-bit, # and linking against libstdc++, requires linking with # -latomic if using the C++ atomic library. -# Can be tested with: clang++ test.cpp -m32 +# Can be tested with: clang++ -std=c++20 test.cpp -m32 # # Sourced from http://bugs.debian.org/797228 @@ -27,8 +27,11 @@ m4_define([_CHECK_ATOMIC_testbody], [[ auto t1 = t.load(); t.compare_exchange_strong(t1, 3s); - std::atomic<int64_t> a{}; + std::atomic<double> d{}; + d.store(3.14); + auto d1 = d.load(); + std::atomic<int64_t> a{}; int64_t v = 5; int64_t r = a.fetch_add(v); return static_cast<int>(r); diff --git a/build_msvc/bitcoin.sln b/build_msvc/bitcoin.sln index 0931bf5dfe..9fd6395f59 100644 --- a/build_msvc/bitcoin.sln +++ b/build_msvc/bitcoin.sln @@ -48,7 +48,9 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libtest_util", "libtest_uti EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "test_bitcoin-qt", "test_bitcoin-qt\test_bitcoin-qt.vcxproj", "{51201D5E-D939-4854-AE9D-008F03FF518E}" EndProject -Project("{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}") = "libminisketch", "libminisketch\libminisketch.vcxproj", "{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}" +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libminisketch", "libminisketch\libminisketch.vcxproj", "{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}" +EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "fuzz", "fuzz\fuzz.vcxproj", "{AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -152,6 +154,10 @@ Global {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Debug|x64.Build.0 = Debug|x64 {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Release|x64.ActiveCfg = Release|x64 {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Release|x64.Build.0 = Release|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Debug|x64.ActiveCfg = Debug|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Debug|x64.Build.0 = Debug|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Release|x64.ActiveCfg = Release|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/build_msvc/bitcoind/bitcoind.vcxproj b/build_msvc/bitcoind/bitcoind.vcxproj index a93723d8be..63337ca6a7 100644 --- a/build_msvc/bitcoind/bitcoind.vcxproj +++ b/build_msvc/bitcoind/bitcoind.vcxproj @@ -80,7 +80,7 @@ <ReplaceInFile FilePath="$(ConfigIniOut)" Replace="@BUILD_BITCOIND_TRUE@" By=""></ReplaceInFile> <ReplaceInFile FilePath="$(ConfigIniOut)" - Replace="@ENABLE_FUZZ_BINARY_TRUE@" By="#"></ReplaceInFile> + Replace="@ENABLE_FUZZ_BINARY_TRUE@" By=""></ReplaceInFile> <ReplaceInFile FilePath="$(ConfigIniOut)" Replace="@ENABLE_ZMQ_TRUE@" By=""></ReplaceInFile> <ReplaceInFile FilePath="$(ConfigIniOut)" diff --git a/build_msvc/common.init.vcxproj.in b/build_msvc/common.init.vcxproj.in index 950cc37f0a..71fceb6c66 100644 --- a/build_msvc/common.init.vcxproj.in +++ b/build_msvc/common.init.vcxproj.in @@ -87,10 +87,10 @@ <ClCompile> <WarningLevel>Level3</WarningLevel> <PrecompiledHeader>NotUsing</PrecompiledHeader> - <AdditionalOptions>/utf-8 /Zc:__cplusplus /std:c++20 %(AdditionalOptions)</AdditionalOptions> + <AdditionalOptions>/utf-8 /Zc:preprocessor /Zc:__cplusplus /std:c++20 %(AdditionalOptions)</AdditionalOptions> <DisableSpecificWarnings>4018;4244;4267;4715;4805</DisableSpecificWarnings> <TreatWarningAsError>true</TreatWarningAsError> - <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions)</PreprocessorDefinitions> <AdditionalIncludeDirectories>..\..\src;..\..\src\minisketch\include;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> </ClCompile> <Link> diff --git a/build_msvc/fuzz/fuzz.vcxproj b/build_msvc/fuzz/fuzz.vcxproj new file mode 100644 index 0000000000..fb77251a17 --- /dev/null +++ b/build_msvc/fuzz/fuzz.vcxproj @@ -0,0 +1,91 @@ +<?xml version="1.0" encoding="utf-8"?> +<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="..\common.init.vcxproj" /> + <PropertyGroup Label="Globals"> + <ProjectGuid>{AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}</ProjectGuid> + </PropertyGroup> + <PropertyGroup Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + </PropertyGroup> + <ItemGroup> + <!-- TODO: Fix the code and remove bitdeque.cpp and miniscript.cpp exclusion. --> + <ClCompile Include="..\..\src\test\fuzz\*.cpp" Exclude="..\..\src\test\fuzz\bitdeque.cpp;..\..\src\test\fuzz\miniscript.cpp" /> + <ClCompile Include="..\..\src\test\fuzz\util\descriptor.cpp"> + <ObjectFileName>$(IntDir)test_fuzz_util_descriptor.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\test\fuzz\util\mempool.cpp"> + <ObjectFileName>$(IntDir)test_fuzz_util_mempool.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\test\fuzz\util\net.cpp"> + <ObjectFileName>$(IntDir)test_fuzz_util_net.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\coincontrol.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_coincontrol.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\coinselection.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_coinselection.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\fees.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_fees.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\notifications.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_notifications.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\parse_iso8601.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_parse_iso8601.obj</ObjectFileName> + </ClCompile> + <ClCompile Include="..\..\src\wallet\test\fuzz\scriptpubkeyman.cpp"> + <ObjectFileName>$(IntDir)wallet_test_fuzz_scriptpubkeyman.obj</ObjectFileName> + </ClCompile> + </ItemGroup> + <ItemGroup> + <ProjectReference Include="..\libminisketch\libminisketch.vcxproj"> + <Project>{542007e3-be0d-4b0d-a6b0-aa8813e2558d}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_consensus\libbitcoin_consensus.vcxproj"> + <Project>{2b384fa8-9ee1-4544-93cb-0d733c25e8ce}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_cli\libbitcoin_cli.vcxproj"> + <Project>{0667528c-d734-4009-adf9-c0d6c4a5a5a6}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_common\libbitcoin_common.vcxproj"> + <Project>{7c87e378-df58-482e-aa2f-1bc129bc19ce}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_crypto\libbitcoin_crypto.vcxproj"> + <Project>{6190199c-6cf4-4dad-bfbd-93fa72a760c1}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_node\libbitcoin_node.vcxproj"> + <Project>{460fee33-1fe1-483f-b3bf-931ff8e969a5}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_util\libbitcoin_util.vcxproj"> + <Project>{b53a5535-ee9d-4c6f-9a26-f79ee3bc3754}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_wallet\libbitcoin_wallet.vcxproj"> + <Project>{93b86837-b543-48a5-a89b-7c87abb77df2}</Project> + </ProjectReference> + <ProjectReference Include="..\libbitcoin_zmq\libbitcoin_zmq.vcxproj"> + <Project>{792d487f-f14c-49fc-a9de-3fc150f31c3f}</Project> + </ProjectReference> + <ProjectReference Include="..\libtest_util\libtest_util.vcxproj"> + <Project>{1e065f03-3566-47d0-8fa9-daa72b084e7d}</Project> + </ProjectReference> + <ProjectReference Include="..\libunivalue\libunivalue.vcxproj"> + <Project>{5724ba7d-a09a-4ba8-800b-c4c1561b3d69}</Project> + </ProjectReference> + <ProjectReference Include="..\libsecp256k1\libsecp256k1.vcxproj"> + <Project>{bb493552-3b8c-4a8c-bf69-a6e7a51d2ea6}</Project> + </ProjectReference> + <ProjectReference Include="..\libleveldb\libleveldb.vcxproj"> + <Project>{18430fef-6b61-4c53-b396-718e02850f1b}</Project> + </ProjectReference> + </ItemGroup> + <ItemDefinitionGroup> + <ClCompile> + <DisableSpecificWarnings>4018;4244;4267;4334;4715;4805</DisableSpecificWarnings> + </ClCompile> + </ItemDefinitionGroup> + <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> + <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> + <Import Project="..\common.vcxproj" /> +</Project> diff --git a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in index 482e4333f7..b47d62b295 100644 --- a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in +++ b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in @@ -8,7 +8,6 @@ <ConfigurationType>StaticLibrary</ConfigurationType> </PropertyGroup> <ItemGroup> - <ClCompile Include="..\..\src\common\url.cpp" /> @SOURCE_FILES@ </ItemGroup> <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 36a172e42c..f1c358082d 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index 4f80d7ed42..bf4d1573e3 100755 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -6,15 +6,14 @@ export LC_ALL=C.UTF-8 -export CI_IMAGE_NAME_TAG="docker.io/debian:bookworm" +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_fuzz_valgrind -export PACKAGES="clang llvm libclang-rt-dev libevent-dev libboost-dev libsqlite3-dev valgrind" +export PACKAGES="clang-16 llvm-16 libclang-rt-16-dev libevent-dev libboost-dev libsqlite3-dev valgrind" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=true export FUZZ_TESTS_CONFIG="--valgrind" export GOAL="install" -# Temporarily pin dwarf 4, until using Valgrind 3.20 or later -export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4" +export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang-16 CXX=clang++-16" export CCACHE_MAXSIZE=200M diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 431e4aba49..dd465cac2e 100755 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BITCOIN_CONFIG="--with-sanitizers=memory CPPFLAGS='-U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" export CCACHE_MAXSIZE=250M diff --git a/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh b/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh index 6f0b9cc285..6425120afb 100755 --- a/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh +++ b/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh @@ -8,8 +8,8 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_nowallet_libbitcoinkernel export CI_IMAGE_NAME_TAG="docker.io/ubuntu:22.04" -# Use minimum supported python3.9 (or best-effort 3.10) and clang-14, see doc/dependencies.md -export PACKAGES="python3-zmq clang-14 llvm-14 libc++abi-14-dev libc++-14-dev" -export DEP_OPTS="NO_WALLET=1 CC=clang-14 CXX='clang++-14 -stdlib=libc++'" +# Use minimum supported python3.9 (or best-effort 3.10) and clang-15, see doc/dependencies.md +export PACKAGES="python3-zmq clang-15 llvm-15 libc++abi-15-dev libc++-15-dev" +export DEP_OPTS="NO_WALLET=1 CC=clang-15 CXX='clang++-15 -stdlib=libc++'" export GOAL="install" export BITCOIN_CONFIG="--enable-reduce-exports --enable-experimental-util-chainstate --with-experimental-kernel-lib --enable-shared" diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 4c8658479b..5f422bbdb6 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -16,5 +16,5 @@ export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=false export RUN_TIDY=true export GOAL="install" -export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-${TIDY_LLVM_V}/lib/clang/${TIDY_LLVM_V}/include'" +export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'" export CCACHE_MAXSIZE=200M diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index 9bdb2b7860..720e522a6a 100755 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -6,12 +6,11 @@ export LC_ALL=C.UTF-8 -export CI_IMAGE_NAME_TAG="docker.io/debian:bookworm" +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_valgrind -export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libsqlite3-dev" +export PACKAGES="valgrind clang-16 llvm-16 libclang-rt-16-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libsqlite3-dev" export USE_VALGRIND=1 export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 +export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export GOAL="install" -# Temporarily pin dwarf 4, until using Valgrind 3.20 or later -export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4" # TODO enable GUI +export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang-16 CXX=clang++-16" # TODO enable GUI diff --git a/ci/test/00_setup_env_s390x.sh b/ci/test/00_setup_env_s390x.sh index ca84ecce51..2fd94e253c 100755 --- a/ci/test/00_setup_env_s390x.sh +++ b/ci/test/00_setup_env_s390x.sh @@ -9,8 +9,8 @@ export LC_ALL=C.UTF-8 export HOST=s390x-linux-gnu export PACKAGES="python3-zmq" export CONTAINER_NAME=ci_s390x -export CI_IMAGE_NAME_TAG="docker.io/s390x/debian:bookworm" -export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 +export CI_IMAGE_NAME_TAG="docker.io/s390x/ubuntu:24.04" +export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export RUN_FUNCTIONAL_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-reduce-exports" diff --git a/configure.ac b/configure.ac index 63a012b35d..c7d124a1f1 100644 --- a/configure.ac +++ b/configure.ac @@ -696,6 +696,9 @@ case $host in TARGET_OS=darwin if test $cross_compiling != "yes"; then BUILD_OS=darwin + + AX_CHECK_LINK_FLAG([-Wl,-headerpad_max_install_names], [CORE_LDFLAGS="$CORE_LDFLAGS -Wl,-headerpad_max_install_names"], [], [$LDFLAG_WERROR]) + AC_CHECK_PROG([BREW], [brew], [brew]) if test "$BREW" = "brew"; then dnl These Homebrew packages may be keg-only, meaning that they won't be found @@ -759,7 +762,6 @@ case $host in ;; *) AC_PATH_TOOL([DSYMUTIL], [dsymutil], [dsymutil]) - AC_PATH_TOOL([INSTALL_NAME_TOOL], [install_name_tool], [install_name_tool]) AC_PATH_TOOL([OTOOL], [otool], [otool]) AC_PATH_PROG([ZIP], [zip], [zip]) @@ -772,7 +774,6 @@ case $host in esac fi - AX_CHECK_LINK_FLAG([-Wl,-headerpad_max_install_names], [CORE_LDFLAGS="$CORE_LDFLAGS -Wl,-headerpad_max_install_names"], [], [$LDFLAG_WERROR]) CORE_CPPFLAGS="$CORE_CPPFLAGS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0" dnl ignore deprecated-declarations warnings coming from objcxx code @@ -968,22 +969,6 @@ AC_CHECK_DECLS([setsid]) AC_CHECK_DECLS([pipe2]) -AC_MSG_CHECKING([for __builtin_clzl]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ - (void) __builtin_clzl(0); - ]])], - [ AC_MSG_RESULT([yes]); have_clzl=yes; AC_DEFINE([HAVE_BUILTIN_CLZL], [1], [Define this symbol if you have __builtin_clzl])], - [ AC_MSG_RESULT([no]); have_clzl=no;] -) - -AC_MSG_CHECKING([for __builtin_clzll]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ - (void) __builtin_clzll(0); - ]])], - [ AC_MSG_RESULT([yes]); have_clzll=yes; AC_DEFINE([HAVE_BUILTIN_CLZLL], [1], [Define this symbol if you have __builtin_clzll])], - [ AC_MSG_RESULT([no]); have_clzll=no;] -) - dnl Check for malloc_info (for memory statistics information in getmemoryinfo) AC_MSG_CHECKING([for getmemoryinfo]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <malloc.h>]], @@ -1722,7 +1707,6 @@ AM_CONDITIONAL([ENABLE_QT_TESTS], [test "$BUILD_TEST_QT" = "yes"]) AM_CONDITIONAL([ENABLE_BENCH], [test "$use_bench" = "yes"]) AM_CONDITIONAL([USE_QRCODE], [test "$use_qr" = "yes"]) AM_CONDITIONAL([USE_LCOV], [test "$use_lcov" = "yes"]) -AM_CONDITIONAL([USE_LIBEVENT], [test "$use_libevent" = "yes"]) AM_CONDITIONAL([HARDEN], [test "$use_hardening" = "yes"]) AM_CONDITIONAL([ENABLE_SSE42], [test "$enable_sse42" = "yes"]) AM_CONDITIONAL([ENABLE_SSE41], [test "$enable_sse41" = "yes"]) @@ -1736,7 +1720,6 @@ AM_CONDITIONAL([USE_UPNP], [test "$use_upnp" = "yes"]) dnl for minisketch AM_CONDITIONAL([ENABLE_CLMUL], [test "$enable_clmul" = "yes"]) -AM_CONDITIONAL([HAVE_CLZ], [test "$have_clzl$have_clzll" = "yesyes"]) AC_DEFINE([CLIENT_VERSION_MAJOR], [_CLIENT_VERSION_MAJOR], [Major version]) AC_DEFINE([CLIENT_VERSION_MINOR], [_CLIENT_VERSION_MINOR], [Minor version]) diff --git a/contrib/devtools/gen-manpages.py b/contrib/devtools/gen-manpages.py index 2860e7db99..92acd9a403 100755 --- a/contrib/devtools/gen-manpages.py +++ b/contrib/devtools/gen-manpages.py @@ -62,6 +62,10 @@ with tempfile.NamedTemporaryFile('w', suffix='.h2m') as footer: # Copyright is the same for all binaries, so just use the first. footer.write('[COPYRIGHT]\n') footer.write('\n'.join(versions[0][2]).strip()) + # Create SEE ALSO section + footer.write('\n[SEE ALSO]\n') + footer.write(', '.join(s.rpartition('/')[2] + '(1)' for s in BINARIES)) + footer.write('\n') footer.flush() # Call the binaries through help2man to produce a manual page for each of them. diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 9624221880..1e9b682f3f 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -75,9 +75,8 @@ export OBJC_INCLUDE_PATH="${NATIVE_GCC}/include" export OBJCPLUS_INCLUDE_PATH="${NATIVE_GCC}/include/c++:${NATIVE_GCC}/include" case "$HOST" in - *darwin*) - export LIBRARY_PATH="${NATIVE_GCC}/lib" - ;; + *darwin*) export LIBRARY_PATH="${NATIVE_GCC}/lib" ;; + *mingw*) export LIBRARY_PATH="${NATIVE_GCC}/lib" ;; *) NATIVE_GCC_STATIC="$(store_path gcc-toolchain static)" export LIBRARY_PATH="${NATIVE_GCC}/lib:${NATIVE_GCC_STATIC}/lib" diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 87dcf49bcb..96818c7748 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -110,12 +110,15 @@ desirable for building Bitcoin Core release binaries." (define (gcc-mingw-patches gcc) (package-with-extra-patches gcc - (search-our-patches "gcc-remap-guix-store.patch" - "vmov-alignment.patch"))) + (search-our-patches "gcc-remap-guix-store.patch"))) + +(define (binutils-mingw-patches binutils) + (package-with-extra-patches binutils + (search-our-patches "binutils-unaligned-default.patch"))) (define (make-mingw-pthreads-cross-toolchain target) "Create a cross-compilation toolchain package for TARGET" - (let* ((xbinutils (cross-binutils target)) + (let* ((xbinutils (binutils-mingw-patches (cross-binutils target))) (pthreads-xlibc mingw-w64-x86_64-winpthreads) (pthreads-xgcc (cross-gcc target #:xgcc (gcc-mingw-patches mingw-w64-base-gcc) @@ -496,7 +499,6 @@ inspecting signatures in Mach-O binaries.") moreutils ;; Compression and archiving tar - bzip2 gzip xz ;; Build tools @@ -517,7 +519,6 @@ inspecting signatures in Mach-O binaries.") (cond ((string-suffix? "-mingw32" target) (list ;; Native GCC 12 toolchain gcc-toolchain-12 - (list gcc-toolchain-12 "static") zip (make-mingw-pthreads-cross-toolchain "x86_64-w64-mingw32") nsis-x86_64 diff --git a/contrib/guix/patches/binutils-unaligned-default.patch b/contrib/guix/patches/binutils-unaligned-default.patch new file mode 100644 index 0000000000..d1bc71aee1 --- /dev/null +++ b/contrib/guix/patches/binutils-unaligned-default.patch @@ -0,0 +1,22 @@ +commit 6537181f59ed186a341db621812a6bc35e22eaf6 +Author: fanquake <fanquake@gmail.com> +Date: Wed Apr 10 12:15:52 2024 +0200 + + build: turn on -muse-unaligned-vector-move by default + + This allows us to avoid (more invasively) patching GCC, to avoid + unaligned instruction use. + +diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c +index e0632681477..14a9653abdf 100644 +--- a/gas/config/tc-i386.c ++++ b/gas/config/tc-i386.c +@@ -801,7 +801,7 @@ static unsigned int no_cond_jump_promotion = 0; + static unsigned int sse2avx; + + /* Encode aligned vector move as unaligned vector move. */ +-static unsigned int use_unaligned_vector_move; ++static unsigned int use_unaligned_vector_move = 1; + + /* Encode scalar AVX instructions with specific vector length. */ + static enum diff --git a/contrib/guix/patches/vmov-alignment.patch b/contrib/guix/patches/vmov-alignment.patch deleted file mode 100644 index 96e1cb7cd1..0000000000 --- a/contrib/guix/patches/vmov-alignment.patch +++ /dev/null @@ -1,288 +0,0 @@ -Description: Use unaligned VMOV instructions -Author: Stephen Kitt <skitt@debian.org> -Bug-Debian: https://bugs.debian.org/939559 -See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412 - -Based on a patch originally by Claude Heiland-Allen <claude@mathr.co.uk> - ---- a/gcc/config/i386/sse.md -+++ b/gcc/config/i386/sse.md -@@ -1058,17 +1058,11 @@ - { - if (FLOAT_MODE_P (GET_MODE_INNER (<MODE>mode))) - { -- if (misaligned_operand (operands[1], <MODE>mode)) -- return "vmovu<ssemodesuffix>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -- else -- return "vmova<ssemodesuffix>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -+ return "vmovu<ssemodesuffix>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; - } - else - { -- if (misaligned_operand (operands[1], <MODE>mode)) -- return "vmovdqu<ssescalarsize>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -- else -- return "vmovdqa<ssescalarsize>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -+ return "vmovdqu<ssescalarsize>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; - } - } - [(set_attr "type" "ssemov") -@@ -1184,17 +1178,11 @@ - { - if (FLOAT_MODE_P (GET_MODE_INNER (<MODE>mode))) - { -- if (misaligned_operand (operands[0], <MODE>mode)) -- return "vmovu<ssemodesuffix>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -- else -- return "vmova<ssemodesuffix>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -+ return "vmovu<ssemodesuffix>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; - } - else - { -- if (misaligned_operand (operands[0], <MODE>mode)) -- return "vmovdqu<ssescalarsize>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -- else -- return "vmovdqa<ssescalarsize>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -+ return "vmovdqu<ssescalarsize>\t{%1, %0%{%2%}|%0%{%2%}, %1}"; - } - } - [(set_attr "type" "ssemov") -@@ -7806,7 +7794,7 @@ - "TARGET_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))" - "@ - %vmovlps\t{%1, %0|%q0, %1} -- %vmovaps\t{%1, %0|%0, %1} -+ %vmovups\t{%1, %0|%0, %1} - %vmovlps\t{%1, %d0|%d0, %q1}" - [(set_attr "type" "ssemov") - (set_attr "prefix" "maybe_vex") -@@ -13997,29 +13985,15 @@ - switch (<MODE>mode) - { - case E_V8DFmode: -- if (misaligned_operand (operands[2], <ssequartermode>mode)) -- return "vmovupd\t{%2, %x0|%x0, %2}"; -- else -- return "vmovapd\t{%2, %x0|%x0, %2}"; -+ return "vmovupd\t{%2, %x0|%x0, %2}"; - case E_V16SFmode: -- if (misaligned_operand (operands[2], <ssequartermode>mode)) -- return "vmovups\t{%2, %x0|%x0, %2}"; -- else -- return "vmovaps\t{%2, %x0|%x0, %2}"; -+ return "vmovups\t{%2, %x0|%x0, %2}"; - case E_V8DImode: -- if (misaligned_operand (operands[2], <ssequartermode>mode)) -- return which_alternative == 2 ? "vmovdqu64\t{%2, %x0|%x0, %2}" -+ return which_alternative == 2 ? "vmovdqu64\t{%2, %x0|%x0, %2}" - : "vmovdqu\t{%2, %x0|%x0, %2}"; -- else -- return which_alternative == 2 ? "vmovdqa64\t{%2, %x0|%x0, %2}" -- : "vmovdqa\t{%2, %x0|%x0, %2}"; - case E_V16SImode: -- if (misaligned_operand (operands[2], <ssequartermode>mode)) -- return which_alternative == 2 ? "vmovdqu32\t{%2, %x0|%x0, %2}" -+ return which_alternative == 2 ? "vmovdqu32\t{%2, %x0|%x0, %2}" - : "vmovdqu\t{%2, %x0|%x0, %2}"; -- else -- return which_alternative == 2 ? "vmovdqa32\t{%2, %x0|%x0, %2}" -- : "vmovdqa\t{%2, %x0|%x0, %2}"; - default: - gcc_unreachable (); - } -@@ -21225,63 +21199,27 @@ - switch (get_attr_mode (insn)) - { - case MODE_V16SF: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- return "vmovups\t{%1, %t0|%t0, %1}"; -- else -- return "vmovaps\t{%1, %t0|%t0, %1}"; -+ return "vmovups\t{%1, %t0|%t0, %1}"; - case MODE_V8DF: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- return "vmovupd\t{%1, %t0|%t0, %1}"; -- else -- return "vmovapd\t{%1, %t0|%t0, %1}"; -+ return "vmovupd\t{%1, %t0|%t0, %1}"; - case MODE_V8SF: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- return "vmovups\t{%1, %x0|%x0, %1}"; -- else -- return "vmovaps\t{%1, %x0|%x0, %1}"; -+ return "vmovups\t{%1, %x0|%x0, %1}"; - case MODE_V4DF: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- return "vmovupd\t{%1, %x0|%x0, %1}"; -- else -- return "vmovapd\t{%1, %x0|%x0, %1}"; -+ return "vmovupd\t{%1, %x0|%x0, %1}"; - case MODE_XI: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- { -- if (which_alternative == 2) -- return "vmovdqu\t{%1, %t0|%t0, %1}"; -- else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -- return "vmovdqu64\t{%1, %t0|%t0, %1}"; -- else -- return "vmovdqu32\t{%1, %t0|%t0, %1}"; -- } -+ if (which_alternative == 2) -+ return "vmovdqu\t{%1, %t0|%t0, %1}"; -+ else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -+ return "vmovdqu64\t{%1, %t0|%t0, %1}"; - else -- { -- if (which_alternative == 2) -- return "vmovdqa\t{%1, %t0|%t0, %1}"; -- else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -- return "vmovdqa64\t{%1, %t0|%t0, %1}"; -- else -- return "vmovdqa32\t{%1, %t0|%t0, %1}"; -- } -+ return "vmovdqu32\t{%1, %t0|%t0, %1}"; - case MODE_OI: -- if (misaligned_operand (operands[1], <ssehalfvecmode>mode)) -- { -- if (which_alternative == 2) -- return "vmovdqu\t{%1, %x0|%x0, %1}"; -- else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -- return "vmovdqu64\t{%1, %x0|%x0, %1}"; -- else -- return "vmovdqu32\t{%1, %x0|%x0, %1}"; -- } -+ if (which_alternative == 2) -+ return "vmovdqu\t{%1, %x0|%x0, %1}"; -+ else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -+ return "vmovdqu64\t{%1, %x0|%x0, %1}"; - else -- { -- if (which_alternative == 2) -- return "vmovdqa\t{%1, %x0|%x0, %1}"; -- else if (GET_MODE_SIZE (<ssescalarmode>mode) == 8) -- return "vmovdqa64\t{%1, %x0|%x0, %1}"; -- else -- return "vmovdqa32\t{%1, %x0|%x0, %1}"; -- } -+ return "vmovdqu32\t{%1, %x0|%x0, %1}"; - default: - gcc_unreachable (); - } ---- a/gcc/config/i386/i386.cc -+++ b/gcc/config/i386/i386.cc -@@ -5418,17 +5418,15 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case opcode_int: - if (scalar_mode == E_HFmode) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = misaligned_p ? "vmovdqu32" : "vmovdqa32"; -+ opcode = "vmovdqu32"; - break; - case opcode_float: -- opcode = misaligned_p ? "vmovups" : "vmovaps"; -+ opcode = "vmovups"; - break; - case opcode_double: -- opcode = misaligned_p ? "vmovupd" : "vmovapd"; -+ opcode = "vmovupd"; - break; - } - } -@@ -5438,29 +5436,21 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case E_HFmode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "%vmovdqu"; - break; - case E_SFmode: -- opcode = misaligned_p ? "%vmovups" : "%vmovaps"; -+ opcode = "%vmovups"; - break; - case E_DFmode: -- opcode = misaligned_p ? "%vmovupd" : "%vmovapd"; -+ opcode = "%vmovupd"; - break; - case E_TFmode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - default: - gcc_unreachable (); -@@ -5472,48 +5462,32 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case E_QImode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu8" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu8" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu8" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu8" : "%vmovdqu"; - break; - case E_HImode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "%vmovdqu"; - break; - case E_SImode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu32" : "vmovdqa32"; -+ opcode = "vmovdqu32"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - case E_DImode: - case E_TImode: - case E_OImode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - case E_XImode: -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - break; - default: - gcc_unreachable (); diff --git a/contrib/init/bitcoind.service b/contrib/init/bitcoind.service index 87da17f955..ade8a05926 100644 --- a/contrib/init/bitcoind.service +++ b/contrib/init/bitcoind.service @@ -81,5 +81,8 @@ PrivateDevices=true # Deny the creation of writable and executable memory mappings. MemoryDenyWriteExecute=true +# Restrict ABIs to help ensure MemoryDenyWriteExecute is enforced +SystemCallArchitectures=native + [Install] WantedBy=multi-user.target diff --git a/contrib/macdeploy/README.md b/contrib/macdeploy/README.md index ea599df3d8..d1df3062f8 100644 --- a/contrib/macdeploy/README.md +++ b/contrib/macdeploy/README.md @@ -66,9 +66,8 @@ building for macOS. Apple's version of `binutils` (called `cctools`) contains lots of functionality missing in the FSF's `binutils`. In addition to extra linker options for frameworks and sysroots, several -other tools are needed as well such as `install_name_tool`, `lipo`, and `nmedit`. These -do not build under Linux, so they have been patched to do so. The work here was used as -a starting point: [mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). +other tools are needed as well. These do not build under Linux, so they have been patched to +do so. The work here was used as a starting point: [mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). In order to build a working toolchain, the following source packages are needed from Apple: `cctools`, `dyld`, and `ld64`. diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp index ee91acd5ef..c537f9e7ec 100644 --- a/contrib/valgrind.supp +++ b/contrib/valgrind.supp @@ -13,8 +13,8 @@ # # Note that suppressions may depend on OS and/or library versions. # Tested on: -# * aarch64 (Debian Bookworm system libs, clang, without gui) -# * x86_64 (Debian Bookworm system libs, clang, without gui) +# * aarch64 (Ubuntu Noble system libs, clang, without gui) +# * x86_64 (Ubuntu Noble system libs, clang, without gui) { Suppress libdb warning - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=662917 Memcheck:Cond diff --git a/depends/Makefile b/depends/Makefile index 88aae7ad81..005d9696fb 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -234,7 +234,6 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_ -e 's|@NM@|$(host_NM)|' \ -e 's|@STRIP@|$(host_STRIP)|' \ -e 's|@OTOOL@|$(host_OTOOL)|' \ - -e 's|@INSTALL_NAME_TOOL@|$(host_INSTALL_NAME_TOOL)|' \ -e 's|@DSYMUTIL@|$(host_DSYMUTIL)|' \ -e 's|@build_os@|$(build_os)|' \ -e 's|@host_os@|$(host_os)|' \ diff --git a/depends/README.md b/depends/README.md index a8dfc83e3b..10e0985cf4 100644 --- a/depends/README.md +++ b/depends/README.md @@ -85,6 +85,10 @@ For linux S390X cross compilation: sudo apt-get install g++-s390x-linux-gnu binutils-s390x-linux-gnu +### Install the required dependencies: FreeBSD + + pkg install bash + ### Install the required dependencies: OpenBSD pkg_add bash gtar diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index a5f07643de..554bfd2c3e 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -5,7 +5,6 @@ build_darwin_RANLIB:=$(shell xcrun -f ranlib) build_darwin_STRIP:=$(shell xcrun -f strip) build_darwin_OTOOL:=$(shell xcrun -f otool) build_darwin_NM:=$(shell xcrun -f nm) -build_darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) build_darwin_DSYMUTIL:=$(shell xcrun -f dsymutil) build_darwin_SHA256SUM=shasum -a 256 build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o @@ -18,7 +17,6 @@ darwin_RANLIB:=$(shell xcrun -f ranlib) darwin_STRIP:=$(shell xcrun -f strip) darwin_OTOOL:=$(shell xcrun -f otool) darwin_NM:=$(shell xcrun -f nm) -darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) darwin_DSYMUTIL:=$(shell xcrun -f dsymutil) darwin_native_binutils= darwin_native_toolchain= diff --git a/depends/builders/default.mk b/depends/builders/default.mk index 806d6e7c50..50869cd8a2 100644 --- a/depends/builders/default.mk +++ b/depends/builders/default.mk @@ -12,7 +12,7 @@ build_$(build_os)_$1 ?= $$(default_build_$1) build_$(build_arch)_$(build_os)_$1 ?= $$(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) endef -$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL INSTALL_NAME_TOOL DSYMUTIL TOUCH,$(eval $(call add_build_tool_func,$(var)))) +$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL DSYMUTIL TOUCH,$(eval $(call add_build_tool_func,$(var)))) define add_build_flags_func build_$(build_arch)_$(build_os)_$1 += $(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) diff --git a/depends/config.site.in b/depends/config.site.in index 29b2a67ed0..81975f02b9 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -123,11 +123,6 @@ if test "@host_os@" = darwin; then ac_cv_path_OTOOL="${OTOOL}" fi - if test -n "@INSTALL_NAME_TOOL@"; then - INSTALL_NAME_TOOL="@INSTALL_NAME_TOOL@" - ac_cv_path_INSTALL_NAME_TOOL="${INSTALL_NAME_TOOL}" - fi - if test -n "@DSYMUTIL@"; then DSYMUTIL="@DSYMUTIL@" ac_cv_path_DSYMUTIL="${DSYMUTIL}" diff --git a/depends/description.md b/depends/description.md index 69ee5bd36c..fa345bfe85 100644 --- a/depends/description.md +++ b/depends/description.md @@ -11,7 +11,7 @@ on new hosts. ### No reliance on timestamps File presence is used to determine what needs to be built. This makes the -results distributable and easily digestable by automated builders. +results distributable and easily digestible by automated builders. ### Each build only has its specified dependencies available at build-time. diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index 8beedcc98a..639259ace3 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -39,7 +39,7 @@ llvm_config_prog=$(shell $(SHELL) $(.SHELLFLAGS) "command -v llvm-config") llvm_lib_dir=$(shell $(llvm_config_prog) --libdir) endif -cctools_TOOLS=AR RANLIB STRIP NM OTOOL INSTALL_NAME_TOOL DSYMUTIL +cctools_TOOLS=AR RANLIB STRIP NM OTOOL DSYMUTIL # Make-only lowercase function lc = $(subst A,a,$(subst B,b,$(subst C,c,$(subst D,d,$(subst E,e,$(subst F,f,$(subst G,g,$(subst H,h,$(subst I,i,$(subst J,j,$(subst K,k,$(subst L,l,$(subst M,m,$(subst N,n,$(subst O,o,$(subst P,p,$(subst Q,q,$(subst R,r,$(subst S,s,$(subst T,t,$(subst U,u,$(subst V,v,$(subst W,w,$(subst X,x,$(subst Y,y,$(subst Z,z,$1)))))))))))))))))))))))))) diff --git a/depends/hosts/default.mk b/depends/hosts/default.mk index cf3c90441d..6a6cab6cc6 100644 --- a/depends/hosts/default.mk +++ b/depends/hosts/default.mk @@ -38,5 +38,5 @@ host_$1 = $$($(host_arch)_$(host_os)_$1) host_$(release_type)_$1 = $$($(host_arch)_$(host_os)_$(release_type)_$1) endef -$(foreach tool,CC CXX AR RANLIB STRIP NM OBJCOPY OTOOL INSTALL_NAME_TOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) +$(foreach tool,CC CXX AR RANLIB STRIP NM OBJCOPY OTOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) $(foreach flags,CFLAGS CXXFLAGS CPPFLAGS LDFLAGS, $(eval $(call add_host_flags_func,$(flags)))) diff --git a/depends/packages.md b/depends/packages.md index 0ffdc66d48..7a7a42afa1 100644 --- a/depends/packages.md +++ b/depends/packages.md @@ -162,7 +162,7 @@ From the [Gentoo Wiki entry](https://wiki.gentoo.org/wiki/Project:Quality_Assura > creates. This leads to massive overlinking, which is toxic to the Gentoo > ecosystem, as it leads to a massive number of unnecessary rebuilds. -Where possible, packages are built with Position Independant Code. Either using +Where possible, packages are built with Position Independent Code. Either using the Autotools `--with-pic` flag, or `CMAKE_POSITION_INDEPENDENT_CODE` with CMake. ## Secondary dependencies: diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index ebc097d686..7f0389b30d 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -1,8 +1,8 @@ package=boost $(package)_version=1.81.0 $(package)_download_path=https://boostorg.jfrog.io/artifactory/main/release/$($(package)_version)/source/ -$(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.bz2 -$(package)_sha256_hash=71feeed900fbccca04a3b4f2f84a7c217186f28a940ed8b7ed4725986baf99fa +$(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.gz +$(package)_sha256_hash=205666dea9f6a7cfed87c7a6dfbeb52a2c1b9de55712c9c1a87735d7181452b6 define $(package)_stage_cmds mkdir -p $($(package)_staging_prefix_dir)/include && \ diff --git a/depends/packages/fontconfig.mk b/depends/packages/fontconfig.mk index 444acfe36d..6baaecc55a 100644 --- a/depends/packages/fontconfig.mk +++ b/depends/packages/fontconfig.mk @@ -1,8 +1,8 @@ package=fontconfig $(package)_version=2.12.6 $(package)_download_path=https://www.freedesktop.org/software/fontconfig/release/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=cf0c30807d08f6a28ab46c61b8dbd55c97d2f292cf88f3a07d3384687f31f017 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=064b9ebf060c9e77011733ac9dc0e2ce92870b574cca2405e11f5353a683c334 $(package)_dependencies=freetype expat $(package)_patches=gperf_header_regen.patch diff --git a/depends/packages/libXau.mk b/depends/packages/libXau.mk index aeb14dcd6e..6bafc4f41a 100644 --- a/depends/packages/libXau.mk +++ b/depends/packages/libXau.mk @@ -1,8 +1,8 @@ package=libXau $(package)_version=1.0.9 $(package)_download_path=https://xorg.freedesktop.org/releases/individual/lib/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=ccf8cbf0dbf676faa2ea0a6d64bcc3b6746064722b606c8c52917ed00dcb73ec +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=1f123d8304b082ad63a9e89376400a3b1d4c29e67e3ea07b3f659cccca690eea $(package)_dependencies=xproto # When updating this package, check the default value of diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index caa809ec0b..5a573a18e7 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -1,26 +1,20 @@ package=libnatpmp -$(package)_version=07004b97cf691774efebe70404cf22201e4d330d +$(package)_version=f2433bec24ca3d3f22a8a7840728a3ac177f94ba $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=9321953ceb39d07c25463e266e50d0ae7b64676bb3a986d932b18881ed94f1fb -$(package)_patches=no_libtool.patch +$(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 +$(package)_build_subdir=build -define $(package)_set_vars - $(package)_build_opts=CC="$($(package)_cc)" - $(package)_build_opts_mingw32=CPPFLAGS=-DNATPMP_STATICLIB - $(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" -endef - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/no_libtool.patch +define $(package)_config_cmds + $($(package)_cmake) -S .. -B . endef define $(package)_build_cmds - $(MAKE) libnatpmp.a $($(package)_build_opts) + $(MAKE) natpmp endef define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib &&\ - install *.h $($(package)_staging_prefix_dir)/include &&\ + mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib && \ + install ../natpmp.h ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include && \ install libnatpmp.a $($(package)_staging_prefix_dir)/lib endef diff --git a/depends/packages/libxcb_util.mk b/depends/packages/libxcb_util.mk index 6e4c7359b2..dc4456f85c 100644 --- a/depends/packages/libxcb_util.mk +++ b/depends/packages/libxcb_util.mk @@ -1,8 +1,8 @@ package=libxcb_util $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-$($(package)_version).tar.bz2 -$(package)_sha256_hash=46e49469cb3b594af1d33176cd7565def2be3fa8be4371d62271fabb5eae50e9 +$(package)_file_name=xcb-util-$($(package)_version).tar.gz +$(package)_sha256_hash=0ed0934e2ef4ddff53fcc70fc64fb16fe766cd41ee00330312e20a985fd927a7 $(package)_dependencies=libxcb define $(package)_set_vars diff --git a/depends/packages/libxcb_util_image.mk b/depends/packages/libxcb_util_image.mk index d12d67e8e8..2228250fec 100644 --- a/depends/packages/libxcb_util_image.mk +++ b/depends/packages/libxcb_util_image.mk @@ -1,8 +1,8 @@ package=libxcb_util_image $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-image-$($(package)_version).tar.bz2 -$(package)_sha256_hash=2db96a37d78831d643538dd1b595d7d712e04bdccf8896a5e18ce0f398ea2ffc +$(package)_file_name=xcb-util-image-$($(package)_version).tar.gz +$(package)_sha256_hash=cb2c86190cf6216260b7357a57d9100811bb6f78c24576a3a5bfef6ad3740a42 $(package)_dependencies=libxcb libxcb_util define $(package)_set_vars diff --git a/depends/packages/libxcb_util_keysyms.mk b/depends/packages/libxcb_util_keysyms.mk index d4f72dedbe..56bc33d258 100644 --- a/depends/packages/libxcb_util_keysyms.mk +++ b/depends/packages/libxcb_util_keysyms.mk @@ -1,8 +1,8 @@ package=libxcb_util_keysyms $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-keysyms-$($(package)_version).tar.bz2 -$(package)_sha256_hash=0ef8490ff1dede52b7de533158547f8b454b241aa3e4dcca369507f66f216dd9 +$(package)_file_name=xcb-util-keysyms-$($(package)_version).tar.gz +$(package)_sha256_hash=0807cf078fbe38489a41d755095c58239e1b67299f14460dec2ec811e96caa96 $(package)_dependencies=libxcb xproto define $(package)_set_vars diff --git a/depends/packages/libxcb_util_render.mk b/depends/packages/libxcb_util_render.mk index 28f1fb073c..ee2883feda 100644 --- a/depends/packages/libxcb_util_render.mk +++ b/depends/packages/libxcb_util_render.mk @@ -1,8 +1,8 @@ package=libxcb_util_render $(package)_version=0.3.9 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-renderutil-$($(package)_version).tar.bz2 -$(package)_sha256_hash=c6e97e48fb1286d6394dddb1c1732f00227c70bd1bedb7d1acabefdd340bea5b +$(package)_file_name=xcb-util-renderutil-$($(package)_version).tar.gz +$(package)_sha256_hash=55eee797e3214fe39d0f3f4d9448cc53cffe06706d108824ea37bb79fcedcad5 $(package)_dependencies=libxcb define $(package)_set_vars diff --git a/depends/packages/libxcb_util_wm.mk b/depends/packages/libxcb_util_wm.mk index 3b905ba4ec..a68fd23f8a 100644 --- a/depends/packages/libxcb_util_wm.mk +++ b/depends/packages/libxcb_util_wm.mk @@ -1,8 +1,8 @@ package=libxcb_util_wm $(package)_version=0.4.1 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-wm-$($(package)_version).tar.bz2 -$(package)_sha256_hash=28bf8179640eaa89276d2b0f1ce4285103d136be6c98262b6151aaee1d3c2a3f +$(package)_file_name=xcb-util-wm-$($(package)_version).tar.gz +$(package)_sha256_hash=038b39c4bdc04a792d62d163ba7908f4bb3373057208c07110be73c1b04b8334 $(package)_dependencies=libxcb define $(package)_set_vars diff --git a/depends/packages/qrencode.mk b/depends/packages/qrencode.mk index 4d852d833d..4216646063 100644 --- a/depends/packages/qrencode.mk +++ b/depends/packages/qrencode.mk @@ -1,8 +1,8 @@ package=qrencode $(package)_version=4.1.1 $(package)_download_path=https://fukuchi.org/works/qrencode/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=e455d9732f8041cf5b9c388e345a641fd15707860f928e94507b1961256a6923 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=da448ed4f52aba6bcb0cd48cac0dd51b8692bccc4cd127431402fca6f8171e8e $(package)_patches=cmake_fixups.patch define $(package)_set_vars diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 0acf4cf565..d057b2d410 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -22,6 +22,7 @@ $(package)_patches += fix-macos-linker.patch $(package)_patches += memory_resource.patch $(package)_patches += utc_from_string_no_optimize.patch $(package)_patches += windows_lto.patch +$(package)_patches += zlib-timebits64.patch $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix) $(package)_qttranslations_sha256_hash=24d4c58bc2a40c0f44f59ee64af4192c7d0038c1e45af61646cfc5b65058f271 @@ -251,6 +252,7 @@ define $(package)_preprocess_cmds patch -p1 -i $($(package)_patch_dir)/utc_from_string_no_optimize.patch && \ patch -p1 -i $($(package)_patch_dir)/guix_cross_lib_path.patch && \ patch -p1 -i $($(package)_patch_dir)/windows_lto.patch && \ + patch -p1 -i $($(package)_patch_dir)/zlib-timebits64.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/packages/xproto.mk b/depends/packages/xproto.mk index 7a43c52faf..29c349a21b 100644 --- a/depends/packages/xproto.mk +++ b/depends/packages/xproto.mk @@ -1,8 +1,8 @@ package=xproto $(package)_version=7.0.31 $(package)_download_path=https://xorg.freedesktop.org/releases/individual/proto -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=c6f9747da0bd3a95f86b17fb8dd5e717c8f3ab7f0ece3ba1b247899ec1ef7747 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=6d755eaae27b45c5cc75529a12855fed5de5969b367ed05003944cf901ed43c7 define $(package)_set_vars $(package)_config_opts=--without-fop --without-xmlto --without-xsltproc --disable-specs diff --git a/depends/patches/libnatpmp/no_libtool.patch b/depends/patches/libnatpmp/no_libtool.patch deleted file mode 100644 index 2b9f01f6eb..0000000000 --- a/depends/patches/libnatpmp/no_libtool.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff -ruN libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile ---- libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile 2022-07-05 07:49:50.000000000 +0000 -+++ libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile 2024-01-23 20:59:35.674821779 +0000 -@@ -197,11 +197,7 @@ - $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) - - $(STATICLIB): $(LIBOBJS) --ifneq (, $(findstring darwin, $(OS))) -- $(LIBTOOL) -static -o $@ $? --else - $(AR) crs $@ $? --endif - - $(SHAREDLIB): $(LIBOBJS) - ifneq (, $(findstring darwin, $(OS))) diff --git a/depends/patches/qt/zlib-timebits64.patch b/depends/patches/qt/zlib-timebits64.patch new file mode 100644 index 0000000000..139c1dfa77 --- /dev/null +++ b/depends/patches/qt/zlib-timebits64.patch @@ -0,0 +1,31 @@ +From a566e156b3fa07b566ddbf6801b517a9dba04fa3 Mon Sep 17 00:00:00 2001 +From: Mark Adler <madler@alumni.caltech.edu> +Date: Sat, 29 Jul 2023 22:13:09 -0700 +Subject: [PATCH] Avoid compiler complaints if _TIME_BITS defined when building + zlib. + +zlib does not use time_t, so _TIME_BITS is irrelevant. However it +may be defined anyway as part of a sledgehammer indiscriminately +applied to all builds. + +From https://github.com/madler/zlib/commit/a566e156b3fa07b566ddbf6801b517a9dba04fa3.patch +--- + qtbase/src/3rdparty/zlib/src/gzguts.h | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/qtbase/src/3rdparty/zlib/src/gzguts.h b/qtbase/src/3rdparty/zlib/src/gzguts.h +index e23f831f5..f9375047e 100644 +--- a/qtbase/src/3rdparty/zlib/src/gzguts.h ++++ b/qtbase/src/3rdparty/zlib/src/gzguts.h +@@ -26,9 +26,8 @@ + # ifndef _LARGEFILE_SOURCE + # define _LARGEFILE_SOURCE 1 + # endif +-# ifdef _FILE_OFFSET_BITS +-# undef _FILE_OFFSET_BITS +-# endif ++# undef _FILE_OFFSET_BITS ++# undef _TIME_BITS + #endif + + #ifdef HAVE_HIDDEN diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index aa10e4a891..bf86a0ee4b 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -1,6 +1,6 @@ # FreeBSD Build Guide -**Updated for FreeBSD [12.3](https://www.freebsd.org/releases/12.3R/announce/)** +**Updated for FreeBSD [14.0](https://www.freebsd.org/releases/14.0R/announce/)** This guide describes how to build bitcoind, command-line utilities, and GUI on FreeBSD. @@ -64,10 +64,11 @@ sh/bash: export BDB_PREFIX=[path displayed above] #### GUI Dependencies ###### Qt5 -Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install `qt5`. Skip if you don't intend to use the GUI. +Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install the necessary parts of Qt. Skip if you don't intend to use the GUI. ```bash -pkg install qt5 +pkg install qt5-buildtools qt5-core qt5-gui qt5-linguisttools qt5-testlib qt5-widgets ``` + ###### libqrencode The GUI can encode addresses in a QR Code. To build in QR support for the GUI, install `libqrencode`. Skip if not using the GUI or don't want QR code functionality. diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index 7ed83853a8..264d5f2d08 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -1,6 +1,6 @@ # OpenBSD Build Guide -**Updated for OpenBSD [7.4](https://www.openbsd.org/74.html)** +**Updated for OpenBSD [7.5](https://www.openbsd.org/75.html)** This guide describes how to build bitcoind, command-line utilities, and GUI on OpenBSD. @@ -63,7 +63,7 @@ export BDB_PREFIX="/path/to/bitcoin/depends/x86_64-unknown-openbsd" Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, Qt 5 is required. ```bash -pkg_add qt5 +pkg_add qtbase qttools ``` ## Building Bitcoin Core diff --git a/doc/dependencies.md b/doc/dependencies.md index 1e48d225b0..43973d0c15 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -8,7 +8,7 @@ You can find installation instructions in the `build-*.md` file for your platfor | --- | --- | | [Autoconf](https://www.gnu.org/software/autoconf/) | [2.69](https://github.com/bitcoin/bitcoin/pull/17769) | | [Automake](https://www.gnu.org/software/automake/) | [1.13](https://github.com/bitcoin/bitcoin/pull/18290) | -| [Clang](https://clang.llvm.org) | [14.0](https://github.com/bitcoin/bitcoin/pull/29208) | +| [Clang](https://clang.llvm.org) | [15.0](https://github.com/bitcoin/bitcoin/pull/29165) | | [GCC](https://gcc.gnu.org) | [11.1](https://github.com/bitcoin/bitcoin/pull/29091) | | [Python](https://www.python.org) (scripts, tests) | [3.9](https://github.com/bitcoin/bitcoin/pull/28211) | | [systemtap](https://sourceware.org/systemtap/) ([tracing](tracing.md))| N/A | @@ -35,7 +35,7 @@ You can find installation instructions in the `build-*.md` file for your platfor ### Networking | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/bitcoin/bitcoin/pull/25917) | | No | +| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [f2433be...](https://github.com/bitcoin/bitcoin/pull/29708) | | No | | [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 2.1 | No | ### Notifications diff --git a/doc/descriptors.md b/doc/descriptors.md index 1baf652f30..3b94ec03e4 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -243,7 +243,18 @@ Often it is useful to communicate a description of scripts along with the necessary private keys. For this reason, anywhere a public key or xpub is supported, a private key in WIF format or xprv may be provided instead. This is useful when private keys are necessary for hardened derivation -steps, or for dumping wallet descriptors including private key material. +steps, for signing transactions, or for dumping wallet descriptors +including private key material. + +For example, after importing the following 2-of-3 multisig descriptor +into a wallet, one could use `signrawtransactionwithwallet` +to sign a transaction with the first key: +``` +sh(multi(2,xprv.../84'/0'/0'/0/0,xpub1...,xpub2...)) +``` +Note how the first key is an xprv private key with a specific derivation path, +while the other two are public keys. + ### Compatibility with old wallets diff --git a/doc/external-signer.md b/doc/external-signer.md index de44cdd880..1468e852ef 100644 --- a/doc/external-signer.md +++ b/doc/external-signer.md @@ -150,6 +150,9 @@ Example, display the first native SegWit receive address on Testnet: The command MUST be able to figure out the address type from the descriptor. +The command MUST return an object containing `{"address": "[the address]"}`. +As a sanity check, for devices that support this, it SHOULD ask the device to derive the address. + If <descriptor> contains a master key fingerprint, the command MUST fail if it does not match the fingerprint known by the device. If <descriptor> contains an xpub, the command MUST fail if it does not match the xpub known by the device. diff --git a/doc/managing-wallets.md b/doc/managing-wallets.md index 22e006c963..b99d88877b 100644 --- a/doc/managing-wallets.md +++ b/doc/managing-wallets.md @@ -122,6 +122,22 @@ $ bitcoin-cli -rpcwallet="restored-wallet" getwalletinfo The restored wallet can also be loaded in the GUI via `File` ->`Open wallet`. +## Wallet Passphrase + +Understanding wallet security is crucial for safely storing your Bitcoin. A key aspect is the wallet passphrase, used for encryption. Let's explore its nuances, role, encryption process, and limitations. + +- **Not the Seed:** +The wallet passphrase and the seed are two separate components in wallet security. The seed, or HD seed, functions as a master key for deriving private and public keys in a hierarchical deterministic (HD) wallet. In contrast, the passphrase serves as an additional layer of security specifically designed to secure the private keys within the wallet. The passphrase serves as a safeguard, demanding an additional layer of authentication to access funds in the wallet. + +- **Protection Against Unauthorized Access:** +The passphrase serves as a protective measure, securing your funds in situations where an unauthorized user gains access to your unlocked computer or device while your wallet application is active. Without the passphrase, they would be unable to access your wallet's funds or execute transactions. However, it's essential to be aware that someone with access can potentially compromise the security of your passphrase by installing a keylogger. + +- **Doesn't Encrypt Metadata or Public Keys:** +It's important to note that the passphrase primarily secures the private keys and access to funds within the wallet. It does not encrypt metadata associated with transactions or public keys. Information about your transaction history and the public keys involved may still be visible. + +- **Risk of Fund Loss if Forgotten or Lost:** +If the wallet passphrase is too complex and is subsequently forgotten or lost, there is a risk of losing access to the funds permanently. A forgotten passphrase will result in the inability to unlock the wallet and access the funds. + ## Migrating Legacy Wallets to Descriptor Wallets Legacy wallets (traditional non-descriptor wallets) can be migrated to become Descriptor wallets diff --git a/doc/release-notes-27679.md b/doc/release-notes-27679.md new file mode 100644 index 0000000000..dbbb30428c --- /dev/null +++ b/doc/release-notes-27679.md @@ -0,0 +1,2 @@ +- unix socket paths are now accepted for `-zmqpubrawblock` and `-zmqpubrawtx` with +the format `-zmqpubrawtx=unix:/path/to/file`
\ No newline at end of file diff --git a/doc/release-notes/release-notes-27.0.md b/doc/release-notes/release-notes-27.0.md new file mode 100644 index 0000000000..5060068328 --- /dev/null +++ b/doc/release-notes/release-notes-27.0.md @@ -0,0 +1,217 @@ +Bitcoin Core version 27.0 is now available from: + + <https://bitcoincore.org/bin/bitcoin-core-27.0/> + +This release includes new features, various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + <https://github.com/bitcoin/bitcoin/issues> + +To receive security and update notifications, please subscribe to: + + <https://bitcoincore.org/en/list/announcements/join/> + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes in some cases), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the data directory needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems +using the Linux Kernel 3.17+, macOS 11.0+, and Windows 7 and newer. Bitcoin +Core should also work on most other Unix-like systems but is not as +frequently tested on them. It is not recommended to use Bitcoin Core on +unsupported systems. + +Notable changes +=============== + +libbitcoinconsensus +------------------- + +- libbitcoinconsensus is deprecated and will be removed for v28. This library has + existed for nearly 10 years with very little known uptake or impact. It has + become a maintenance burden. + + The underlying functionality does not change between versions, so any users of + the library can continue to use the final release indefinitely, with the + understanding that Taproot is its final consensus update. + + In the future, libbitcoinkernel will provide a much more useful API that is + aware of the UTXO set, and therefore be able to fully validate transactions and + blocks. (#29189) + +mempool.dat compatibility +------------------------- + +- The `mempool.dat` file created by -persistmempool or the savemempool RPC will + be written in a new format. This new format includes the XOR'ing of transaction + contents to mitigate issues where external programs (such as anti-virus) attempt + to interpret and potentially modify the file. + + This new format can not be read by previous software releases. To allow for a + downgrade, a temporary setting `-persistmempoolv1` has been added to fall back + to the legacy format. (#28207) + +P2P and network changes +----------------------- + +- BIP324 v2 transport is now enabled by default. It remains possible to disable v2 + by running with `-v2transport=0`. (#29347) +- Manual connection options (`-connect`, `-addnode` and `-seednode`) will + now follow `-v2transport` to connect with v2 by default. They will retry with + v1 on failure. (#29058) + +- Network-adjusted time has been removed from consensus code. It is replaced + with (unadjusted) system time. The warning for a large median time offset + (70 minutes or more) is kept. This removes the implicit security assumption of + requiring an honest majority of outbound peers, and increases the importance + of the node operator ensuring their system time is (and stays) correct to not + fall out of consensus with the network. (#28956) + +Mempool Policy Changes +---------------------- + +- Opt-in Topologically Restricted Until Confirmation (TRUC) Transactions policy + (aka v3 transaction policy) is available for use on test networks when + `-acceptnonstdtxn=1` is set. By setting the transaction version number to 3, TRUC transactions + request the application of limits on spending of their unconfirmed outputs. These + restrictions simplify the assessment of incentive compatibility of accepting or + replacing TRUC transactions, thus ensuring any replacements are more profitable for + the node and making fee-bumping more reliable. TRUC transactions are currently + nonstandard and can only be used on test networks where the standardness rules are + relaxed or disabled (e.g. with `-acceptnonstdtxn=1`). (#28948) + +External Signing +---------------- + +- Support for external signing on Windows has been disabled. It will be re-enabled + once the underlying dependency (Boost Process), has been replaced with a different + library. (#28967) + +Updated RPCs +------------ + +- The addnode RPC now follows the `-v2transport` option (now on by default, see above) for making connections. + It remains possible to specify the transport type manually with the v2transport argument of addnode. (#29239) + +Build System +------------ + +- A C++20 capable compiler is now required to build Bitcoin Core. (#28349) +- MacOS releases are configured to use the hardened runtime libraries (#29127) + +Wallet +------ + +- The CoinGrinder coin selection algorithm has been introduced to mitigate unnecessary + large input sets and lower transaction costs at high feerates. CoinGrinder + searches for the input set with minimal weight. Solutions found by + CoinGrinder will produce a change output. CoinGrinder is only active at + elevated feerates (default: 30+ sat/vB, based on `-consolidatefeerate`×3). (#27877) +- The Branch And Bound coin selection algorithm will be disabled when the subtract fee + from outputs feature is used. (#28994) +- If the birth time of a descriptor is detected to be later than the first transaction + involving that descriptor, the birth time will be reset to the earlier time. (#28920) + +Low-level changes +================= + +Pruning +------- + +- When pruning during initial block download, more blocks will be pruned at each + flush in order to speed up the syncing of such nodes. (#20827) + +Init +---- + +- Various fixes to prevent issues where subsequent instances of Bitcoin Core would + result in deletion of files in use by an existing instance. (#28784, #28946) +- Improved handling of empty `settings.json` files. (#29144) + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- 22388o⚡️ +- Aaron Clauson +- Amiti Uttarwar +- Andrew Toth +- Anthony Towns +- Antoine Poinsot +- Ava Chow +- Brandon Odiwuor +- brunoerg +- Chris Stewart +- Cory Fields +- dergoegge +- djschnei21 +- Fabian Jahr +- fanquake +- furszy +- Gloria Zhao +- Greg Sanders +- Hennadii Stepanov +- Hernan Marino +- iamcarlos94 +- ismaelsadeeq +- Jameson Lopp +- Jesse Barton +- John Moffett +- Jon Atack +- josibake +- jrakibi +- Justin Dhillon +- Kashif Smith +- kevkevin +- Kristaps Kaupe +- L0la L33tz +- Luke Dashjr +- Lőrinc +- marco +- MarcoFalke +- Mark Friedenbach +- Marnix +- Martin Leitner-Ankerl +- Martin Zumsande +- Max Edwards +- Murch +- muxator +- naiyoma +- Nikodemas Tuckus +- ns-xvrn +- pablomartin4btc +- Peter Todd +- Pieter Wuille +- Richard Myers +- Roman Zeyde +- Russell Yanofsky +- Ryan Ofsky +- Sebastian Falbesoner +- Sergi Delgado Segura +- Sjors Provoost +- stickies-v +- stratospher +- Supachai Kheawjuy +- TheCharlatan +- UdjinM6 +- Vasil Dimov +- w0xlt +- willcl-ark + + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/). diff --git a/share/rpcauth/README.md b/share/rpcauth/README.md index 6f627b867b..1b3acb1dac 100644 --- a/share/rpcauth/README.md +++ b/share/rpcauth/README.md @@ -15,4 +15,5 @@ positional arguments: optional arguments: -h, --help show this help message and exit + -j, --json output data in json format ``` diff --git a/share/rpcauth/rpcauth.py b/share/rpcauth/rpcauth.py index cc7bba1f8b..506fcf9d91 100755 --- a/share/rpcauth/rpcauth.py +++ b/share/rpcauth/rpcauth.py @@ -7,6 +7,7 @@ from argparse import ArgumentParser from getpass import getpass from secrets import token_hex, token_urlsafe import hmac +import json def generate_salt(size): """Create size byte hex salt""" @@ -24,6 +25,7 @@ def main(): parser = ArgumentParser(description='Create login credentials for a JSON-RPC user') parser.add_argument('username', help='the username for authentication') parser.add_argument('password', help='leave empty to generate a random password or specify "-" to prompt for password', nargs='?') + parser.add_argument("-j", "--json", help="output to json instead of plain-text", action='store_true') args = parser.parse_args() if not args.password: @@ -35,9 +37,13 @@ def main(): salt = generate_salt(16) password_hmac = password_to_hmac(salt, args.password) - print('String to be appended to bitcoin.conf:') - print(f'rpcauth={args.username}:{salt}${password_hmac}') - print(f'Your password:\n{args.password}') + if args.json: + odict={'username':args.username, 'password':args.password, 'rpcauth':f'{args.username}:{salt}${password_hmac}'} + print(json.dumps(odict)) + else: + print('String to be appended to bitcoin.conf:') + print(f'rpcauth={args.username}:{salt}${password_hmac}') + print(f'Your password:\n{args.password}') if __name__ == '__main__': main() diff --git a/src/Makefile.am b/src/Makefile.am index 7673d2f545..c2e0c7b5b8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -320,7 +320,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/spanparsing.h \ util/string.h \ - util/subprocess.hpp \ + util/subprocess.h \ util/syserror.h \ util/task_runner.h \ util/thread.h \ @@ -679,6 +679,7 @@ libbitcoin_common_a_SOURCES = \ common/run_command.cpp \ common/settings.cpp \ common/system.cpp \ + common/url.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -711,11 +712,6 @@ libbitcoin_common_a_SOURCES = \ script/solver.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) - -if USE_LIBEVENT -libbitcoin_common_a_CPPFLAGS += $(EVENT_CFLAGS) -libbitcoin_common_a_SOURCES += common/url.cpp -endif # # util # diff --git a/src/Makefile.minisketch.include b/src/Makefile.minisketch.include index 1363bec34e..c6f894f0ca 100644 --- a/src/Makefile.minisketch.include +++ b/src/Makefile.minisketch.include @@ -12,10 +12,6 @@ LIBMINISKETCH_CPPFLAGS += -DHAVE_CLMUL MINISKETCH_LIBS += $(LIBMINISKETCH_CLMUL) endif -if HAVE_CLZ -LIBMINISKETCH_CPPFLAGS += -DHAVE_CLZ -endif - EXTRA_LIBRARIES += $(MINISKETCH_LIBS) minisketch_libminisketch_clmul_a_SOURCES = $(MINISKETCH_FIELD_CLMUL_SOURCES_INT) $(MINISKETCH_FIELD_CLMUL_HEADERS_INT) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index cf88a02b95..942e0bf69b 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -85,6 +85,7 @@ BITCOIN_TESTS =\ test/checkqueue_tests.cpp \ test/coins_tests.cpp \ test/coinstatsindex_tests.cpp \ + test/common_url_tests.cpp \ test/compilerbug_tests.cpp \ test/compress_tests.cpp \ test/crypto_tests.cpp \ diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 129deeec60..8901d10ef6 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -11,7 +11,6 @@ #include <clientversion.h> #include <common/args.h> #include <common/system.h> -#include <common/url.h> #include <compat/compat.h> #include <compat/stdin.h> #include <policy/feerate.h> @@ -51,7 +50,6 @@ using CliClock = std::chrono::system_clock; const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; @@ -551,7 +549,7 @@ public: peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, - peer.transport_protocol_type.starts_with('v') ? peer.transport_protocol_type[1] : ' ', + (peer.transport_protocol_type.size() == 2 && peer.transport_protocol_type[0] == 'v') ? peer.transport_protocol_type[1] : ' ', PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), peer.last_send ? ToString(time_now - peer.last_send) : "", diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index d5dfbbec27..fe90958a5f 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -11,7 +11,6 @@ #include <clientversion.h> #include <common/args.h> #include <common/system.h> -#include <common/url.h> #include <compat/compat.h> #include <interfaces/init.h> #include <key.h> @@ -28,7 +27,6 @@ #include <tuple> const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; static void SetupWalletToolArgs(ArgsManager& argsman) { diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 4f0a816388..54796c5abb 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -12,7 +12,6 @@ #include <common/args.h> #include <common/init.h> #include <common/system.h> -#include <common/url.h> #include <compat/compat.h> #include <init.h> #include <interfaces/chain.h> @@ -35,7 +34,6 @@ using node::NodeContext; const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; #if HAVE_DECL_FORK diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index e5356490ef..347b486095 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,7 +12,7 @@ #include <univalue.h> #ifdef ENABLE_EXTERNAL_SIGNER -#include <util/subprocess.hpp> +#include <util/subprocess.h> #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) diff --git a/src/common/url.cpp b/src/common/url.cpp index 053e1a825c..ecf88d07ea 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -4,19 +4,36 @@ #include <common/url.h> -#include <event2/http.h> - -#include <cstdlib> +#include <charconv> #include <string> +#include <string_view> +#include <system_error> -std::string urlDecode(const std::string &urlEncoded) { +std::string UrlDecode(std::string_view url_encoded) +{ std::string res; - if (!urlEncoded.empty()) { - char *decoded = evhttp_uridecode(urlEncoded.c_str(), false, nullptr); - if (decoded) { - res = std::string(decoded); - free(decoded); + res.reserve(url_encoded.size()); + + for (size_t i = 0; i < url_encoded.size(); ++i) { + char c = url_encoded[i]; + // Special handling for percent which should be followed by two hex digits + // representing an octet values, see RFC 3986, Section 2.1 Percent-Encoding + if (c == '%' && i + 2 < url_encoded.size()) { + unsigned int decoded_value{0}; + auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16); + + // Only if there is no error and the pointer is set to the end of + // the string, we can be sure both characters were valid hex + if (ec == std::errc{} && p == url_encoded.data() + i + 3) { + res += static_cast<char>(decoded_value); + // Next two characters are part of the percent encoding + i += 2; + continue; + } + // In case of invalid percent encoding, add the '%' and continue } + res += c; } + return res; } diff --git a/src/common/url.h b/src/common/url.h index b16b8241af..203f41c70f 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -6,9 +6,12 @@ #define BITCOIN_COMMON_URL_H #include <string> +#include <string_view> -using UrlDecodeFn = std::string(const std::string& url_encoded); -UrlDecodeFn urlDecode; -extern UrlDecodeFn* const URL_DECODE; +/* Decode a URL. + * + * Notably this implementation does not decode a '+' to a ' '. + */ +std::string UrlDecode(std::string_view url_encoded); #endif // BITCOIN_COMMON_URL_H diff --git a/src/core_read.cpp b/src/core_read.cpp index e32e46d1b9..5956d9df5f 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -256,6 +256,6 @@ util::Result<int> SighashFromStr(const std::string& sighash) if (it != map_sighash_values.end()) { return it->second; } else { - return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")}; + return util::Error{Untranslated("'" + sighash + "' is not a valid sighash parameter.")}; } } diff --git a/src/index/base.cpp b/src/index/base.cpp index a203ce4a9f..e66c89f9e4 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -160,12 +160,24 @@ void BaseIndex::Sync() } const CBlockIndex* pindex_next = WITH_LOCK(cs_main, return NextSyncBlock(pindex, m_chainstate->m_chain)); + // If pindex_next is null, it means pindex is the chain tip, so + // commit data indexed so far. if (!pindex_next) { SetBestBlockIndex(pindex); // No need to handle errors in Commit. See rationale above. Commit(); - m_synced = true; - break; + + // If pindex is still the chain tip after committing, exit the + // sync loop. It is important for cs_main to be locked while + // setting m_synced = true, otherwise a new block could be + // attached while m_synced is still false, and it would not be + // indexed. + LOCK(::cs_main); + pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain); + if (!pindex_next) { + m_synced = true; + break; + } } if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); diff --git a/src/init.cpp b/src/init.cpp index 885c0673dd..47d649be3d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -553,16 +553,12 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will be used.", DEFAULT_TOR_CONTROL, DEFAULT_TOR_CONTROL_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); #ifdef USE_UPNP -#if USE_UPNP - argsman.AddArg("-upnp", "Use UPnP to map the listening port (default: 1 when listening and no -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#else - argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %u)", 0), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#endif + argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %u)", DEFAULT_UPNP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #else hidden_args.emplace_back("-upnp"); #endif #ifdef USE_NATPMP - argsman.AddArg("-natpmp", strprintf("Use NAT-PMP to map the listening port (default: %s)", DEFAULT_NATPMP ? "1 when listening and no -proxy" : "0"), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-natpmp", strprintf("Use NAT-PMP to map the listening port (default: %u)", DEFAULT_NATPMP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #else hidden_args.emplace_back("-natpmp"); #endif // USE_NATPMP @@ -996,10 +992,8 @@ bool AppInitParameterInteraction(const ArgsManager& args) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); // ********************************************************* Step 3: parameter-to-internal-flags - auto result = init::SetLoggingCategories(args); - if (!result) return InitError(util::ErrorString(result)); - result = init::SetLoggingLevel(args); - if (!result) return InitError(util::ErrorString(result)); + if (auto result{init::SetLoggingCategories(args)}; !result) return InitError(util::ErrorString(result)); + if (auto result{init::SetLoggingLevel(args)}; !result) return InitError(util::ErrorString(result)); nConnectTimeout = args.GetIntArg("-timeout", DEFAULT_CONNECT_TIMEOUT); if (nConnectTimeout <= 0) { @@ -1305,30 +1299,33 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - for (const std::string port_option : { - "-i2psam", - "-onion", - "-proxy", - "-rpcbind", - "-torcontrol", - "-whitebind", - "-zmqpubhashblock", - "-zmqpubhashtx", - "-zmqpubrawblock", - "-zmqpubrawtx", - "-zmqpubsequence", + for (const auto &port_option : std::vector<std::pair<std::string, bool>>{ + // arg name UNIX socket support + {"-i2psam", false}, + {"-onion", true}, + {"-proxy", true}, + {"-rpcbind", false}, + {"-torcontrol", false}, + {"-whitebind", false}, + {"-zmqpubhashblock", true}, + {"-zmqpubhashtx", true}, + {"-zmqpubrawblock", true}, + {"-zmqpubrawtx", true}, + {"-zmqpubsequence", true} }) { - for (const std::string& socket_addr : args.GetArgs(port_option)) { + const std::string arg{port_option.first}; + const bool unix{port_option.second}; + for (const std::string& socket_addr : args.GetArgs(arg)) { std::string host_out; uint16_t port_out{0}; if (!SplitHostPort(socket_addr, port_out, host_out)) { #if HAVE_SOCKADDR_UN - // Allow unix domain sockets for -proxy and -onion e.g. unix:/some/file/path - if ((port_option != "-proxy" && port_option != "-onion") || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + // Allow unix domain sockets for some options e.g. unix:/some/file/path + if (!unix || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { + return InitError(InvalidPortErrMsg(arg, socket_addr)); } #else - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + return InitError(InvalidPortErrMsg(arg, socket_addr)); #endif } } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 6114236623..c41f35829d 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -127,7 +127,7 @@ public: virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0; //! Display address on external signer - virtual bool displayAddress(const CTxDestination& dest) = 0; + virtual util::Result<void> displayAddress(const CTxDestination& dest) = 0; //! Lock coin. virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; diff --git a/src/minisketch/.cirrus.yml b/src/minisketch/.cirrus.yml index 4a5353f137..5ceefee2cf 100644 --- a/src/minisketch/.cirrus.yml +++ b/src/minisketch/.cirrus.yml @@ -36,17 +36,6 @@ env_matrix_snippet: &ENV_MATRIX_VALGRIND TESTRUNS: 1 BUILD: -env_matrix_snippet: &ENV_MATRIX_SAN - - env: - ENABLE_FIELDS: 28 - - env: - BUILD: distcheck - - env: - CXXFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" - LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" - UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1" - BENCH: no - env_matrix_snippet: &ENV_MATRIX_SAN_VALGRIND - env: ENABLE_FIELDS: "11,64,37" @@ -72,9 +61,9 @@ task: << : *ENV_MATRIX_SAN_VALGRIND matrix: - env: - CC: gcc + CXX: g++ - env: - CC: clang + CXX: clang++ -gdwarf-4 << : *MERGE_BASE test_script: - ./ci/cirrus.sh @@ -92,30 +81,45 @@ task: << : *ENV_MATRIX_VALGRIND matrix: - env: - CC: i686-linux-gnu-gcc + CXX: i686-linux-gnu-g++ - env: - CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include + CXX: clang++ --target=i686-linux-gnu -gdwarf-4 + CXXFLAGS: -g -O2 -isystem /usr/i686-linux-gnu/include -isystem /usr/i686-linux-gnu/include/c++/10/i686-linux-gnu test_script: - ./ci/cirrus.sh << : *CAT_LOGS task: - name: "x86_64: macOS Catalina" + name: "arm64: macOS Monterey" macos_instance: - image: catalina-base + image: ghcr.io/cirruslabs/macos-monterey-base:latest env: - # Cirrus gives us a fixed number of 12 virtual CPUs. - MAKEFLAGS: -j13 - matrix: - << : *ENV_MATRIX_SAN + # Cirrus gives us a fixed number of 4 virtual CPUs. + MAKEFLAGS: -j5 matrix: - env: - CC: gcc-9 + CXX: g++-11 + # Homebrew's gcc for arm64 has no libubsan. + matrix: + - env: + ENABLE_FIELDS: 28 + - env: + BUILD: distcheck - env: - CC: clang + CXX: clang++ + matrix: + - env: + ENABLE_FIELDS: 28 + - env: + BUILD: distcheck + - env: + CXXFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" + LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" + UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1" + BENCH: no brew_script: - brew update - - brew install automake libtool gcc@9 + - brew install automake libtool gcc@11 << : *MERGE_BASE test_script: - ./ci/cirrus.sh @@ -128,13 +132,11 @@ task: cpu: 4 memory: 2G env: - EXEC_CMD: qemu-s390x -L /usr/s390x-linux-gnu + EXEC_CMD: qemu-s390x HOST: s390x-linux-gnu BUILD: << : *MERGE_BASE test_script: - # https://sourceware.org/bugzilla/show_bug.cgi?id=27008 - - rm /etc/ld.so.cache - ./ci/cirrus.sh << : *CAT_LOGS @@ -146,6 +148,7 @@ task: memory: 2G env: EXEC_CMD: wine + EXEC_EXT: .exe HOST: x86_64-w64-mingw32 BUILD: << : *MERGE_BASE diff --git a/src/minisketch/ci/cirrus.sh b/src/minisketch/ci/cirrus.sh index 02f737ca7f..36250d1651 100755 --- a/src/minisketch/ci/cirrus.sh +++ b/src/minisketch/ci/cirrus.sh @@ -7,7 +7,7 @@ export LC_ALL=C env >> test_env.log -$CC -v || true +$CXX -v || true valgrind --version || true ./autogen.sh @@ -32,10 +32,10 @@ then fi if [ -n "$EXEC_CMD" ]; then - $EXEC_CMD ./test $TESTRUNS - $EXEC_CMD ./test-verify $TESTRUNS + $EXEC_CMD "./test$EXEC_EXT" $TESTRUNS + $EXEC_CMD "./test-verify$EXEC_EXT" $TESTRUNS fi if [ "$BENCH" = "yes" ]; then - $EXEC_CMD ./bench + $EXEC_CMD "./bench$EXEC_EXT" fi diff --git a/src/minisketch/ci/linux-debian.Dockerfile b/src/minisketch/ci/linux-debian.Dockerfile index 63e5412ee7..122af36e1f 100644 --- a/src/minisketch/ci/linux-debian.Dockerfile +++ b/src/minisketch/ci/linux-debian.Dockerfile @@ -8,10 +8,10 @@ RUN apt-get update RUN apt-get install --no-install-recommends --no-upgrade -y \ git ca-certificates \ make automake libtool pkg-config dpkg-dev valgrind qemu-user \ - gcc g++ clang libc6-dbg \ + gcc g++ clang libclang-rt-dev libc6-dbg \ gcc-i686-linux-gnu g++-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \ - g++-s390x-linux-gnu gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ - wine g++-mingw-w64-x86-64 + g++-s390x-linux-gnu libstdc++6:s390x gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ + wine wine64 g++-mingw-w64-x86-64 # Run a dummy command in wine to make it set up configuration RUN wine true || true diff --git a/src/minisketch/configure.ac b/src/minisketch/configure.ac index 83910448a2..cd52d7f412 100644 --- a/src/minisketch/configure.ac +++ b/src/minisketch/configure.ac @@ -104,11 +104,6 @@ esac AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-fvisibility=hidden],[CXXFLAGS="$CXXFLAGS -fvisibility=hidden"],[],[$CXXFLAG_WERROR]) -## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all -## unknown options if any other warning is produced. Test the -Wfoo case, and -## set the -Wno-foo case if it works. -AX_CHECK_COMPILE_FLAG([-Wshift-count-overflow],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-shift-count-overflow"],,[[$CXXFLAG_WERROR]]) - if test "x$use_ccache" != "xno"; then AC_MSG_CHECKING(if ccache should be used) if test x$CCACHE = x; then @@ -119,7 +114,6 @@ if test "x$use_ccache" != "xno"; then fi else use_ccache=yes - CC="$ac_cv_path_CCACHE $CC" CXX="$ac_cv_path_CCACHE $CXX" fi AC_MSG_RESULT($use_ccache) diff --git a/src/minisketch/src/int_utils.h b/src/minisketch/src/int_utils.h index d21ba56f33..2b3d8cb402 100644 --- a/src/minisketch/src/int_utils.h +++ b/src/minisketch/src/int_utils.h @@ -7,13 +7,16 @@ #ifndef _MINISKETCH_INT_UTILS_H_ #define _MINISKETCH_INT_UTILS_H_ +#include <stdint.h> #include <stdlib.h> #include <limits> #include <algorithm> #include <type_traits> -#ifdef _MSC_VER +#if defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L +# include <bit> +#elif defined(_MSC_VER) # include <intrin.h> #endif @@ -54,11 +57,10 @@ class BitWriter { int offset = 0; unsigned char* out; -public: - BitWriter(unsigned char* output) : out(output) {} - template<int BITS, typename I> - inline void Write(I val) { + inline void WriteInner(I val) { + // We right shift by up to 8 bits below. Verify that's well defined for the type I. + static_assert(std::numeric_limits<I>::digits > 8, "BitWriter::WriteInner needs I > 8 bits"); int bits = BITS; if (bits + offset >= 8) { state |= ((val & ((I(1) << (8 - offset)) - 1)) << offset); @@ -77,6 +79,19 @@ public: offset += bits; } + +public: + BitWriter(unsigned char* output) : out(output) {} + + template<int BITS, typename I> + inline void Write(I val) { + // If I is smaller than an unsigned int, invoke WriteInner with argument converted to unsigned. + using compute_type = typename std::conditional< + (std::numeric_limits<I>::digits < std::numeric_limits<unsigned>::digits), + unsigned, I>::type; + return WriteInner<BITS, compute_type>(val); + } + inline void Flush() { if (offset) { *(out++) = state; @@ -129,7 +144,11 @@ constexpr inline I Mask() { return ((I((I(-1)) << (std::numeric_limits<I>::digit /** Compute the smallest power of two that is larger than val. */ template<typename I> static inline int CountBits(I val, int max) { -#ifdef _MSC_VER +#if defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L + // c++20 impl + (void)max; + return std::bit_width(val); +#elif defined(_MSC_VER) (void)max; unsigned long index; unsigned char ret; @@ -175,6 +194,7 @@ public: } static constexpr inline bool IsZero(I a) { return a == 0; } + static constexpr inline bool IsOne(I a) { return a == 1; } static constexpr inline I Mask(I val) { return val & MASK; } static constexpr inline I Shift(I val, int bits) { return ((val << bits) & MASK); } static constexpr inline I UnsafeShift(I val, int bits) { return (val << bits); } @@ -233,7 +253,7 @@ template<typename I, int N, typename L, typename F> inline constexpr I GFMul(con template<typename I, typename F, int BITS, uint32_t MOD> inline I InvExtGCD(I x) { - if (F::IsZero(x)) return x; + if (F::IsZero(x) || F::IsOne(x)) return x; I t(0), newt(1); I r(MOD), newr = x; int rlen = BITS + 1, newrlen = F::Bits(newr, BITS); diff --git a/src/net.cpp b/src/net.cpp index e388f05b03..4801f5c1f9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -412,6 +412,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo // Resolve const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) : m_params.GetDefaultPort()}; + + // Collection of addresses to try to connect to: either all dns resolved addresses if a domain name (pszDest) is provided, or addrConnect otherwise. + std::vector<CAddress> connect_to{}; if (pszDest) { std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; if (!resolved.empty()) { @@ -432,8 +435,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); return nullptr; } + // Add the address to the resolved addresses vector so we can try to connect to it later on + connect_to.push_back(addrConnect); } + } else { + // For resolution via proxy + connect_to.push_back(addrConnect); } + } else { + // Connect via addrConnect directly + connect_to.push_back(addrConnect); } // Connect @@ -443,94 +454,99 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo assert(!addr_bind.IsValid()); std::unique_ptr<i2p::sam::Session> i2p_transient_session; - if (addrConnect.IsValid()) { - const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)}; - bool proxyConnectionFailed = false; + for (auto& target_addr: connect_to) { + if (target_addr.IsValid()) { + const bool use_proxy{GetProxy(target_addr.GetNetwork(), proxy)}; + bool proxyConnectionFailed = false; - if (addrConnect.IsI2P() && use_proxy) { - i2p::Connection conn; - bool connected{false}; + if (target_addr.IsI2P() && use_proxy) { + i2p::Connection conn; + bool connected{false}; - if (m_i2p_sam_session) { - connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed); - } else { - { - LOCK(m_unused_i2p_sessions_mutex); - if (m_unused_i2p_sessions.empty()) { - i2p_transient_session = - std::make_unique<i2p::sam::Session>(proxy, &interruptNet); - } else { - i2p_transient_session.swap(m_unused_i2p_sessions.front()); - m_unused_i2p_sessions.pop(); + if (m_i2p_sam_session) { + connected = m_i2p_sam_session->Connect(target_addr, conn, proxyConnectionFailed); + } else { + { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.empty()) { + i2p_transient_session = + std::make_unique<i2p::sam::Session>(proxy, &interruptNet); + } else { + i2p_transient_session.swap(m_unused_i2p_sessions.front()); + m_unused_i2p_sessions.pop(); + } } - } - connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed); - if (!connected) { - LOCK(m_unused_i2p_sessions_mutex); - if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { - m_unused_i2p_sessions.emplace(i2p_transient_session.release()); + connected = i2p_transient_session->Connect(target_addr, conn, proxyConnectionFailed); + if (!connected) { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { + m_unused_i2p_sessions.emplace(i2p_transient_session.release()); + } } } - } - if (connected) { - sock = std::move(conn.sock); - addr_bind = CAddress{conn.me, NODE_NONE}; + if (connected) { + sock = std::move(conn.sock); + addr_bind = CAddress{conn.me, NODE_NONE}; + } + } else if (use_proxy) { + LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s\n", proxy.ToString(), target_addr.ToStringAddrPort()); + sock = ConnectThroughProxy(proxy, target_addr.ToStringAddr(), target_addr.GetPort(), proxyConnectionFailed); + } else { + // no proxy needed (none set for target network) + sock = ConnectDirectly(target_addr, conn_type == ConnectionType::MANUAL); } - } else if (use_proxy) { - LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s:%s\n", proxy.ToString(), addrConnect.ToStringAddr(), addrConnect.GetPort()); - sock = ConnectThroughProxy(proxy, addrConnect.ToStringAddr(), addrConnect.GetPort(), proxyConnectionFailed); - } else { - // no proxy needed (none set for target network) - sock = ConnectDirectly(addrConnect, conn_type == ConnectionType::MANUAL); + if (!proxyConnectionFailed) { + // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to + // the proxy, mark this as an attempt. + addrman.Attempt(target_addr, fCountFailure); + } + } else if (pszDest && GetNameProxy(proxy)) { + std::string host; + uint16_t port{default_port}; + SplitHostPort(std::string(pszDest), port, host); + bool proxyConnectionFailed; + sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); + } + // Check any other resolved address (if any) if we fail to connect + if (!sock) { + continue; } - if (!proxyConnectionFailed) { - // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to - // the proxy, mark this as an attempt. - addrman.Attempt(addrConnect, fCountFailure); + + NetPermissionFlags permission_flags = NetPermissionFlags::None; + std::vector<NetWhitelistPermissions> whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector<NetWhitelistPermissions>{}; + AddWhitelistPermissionFlags(permission_flags, target_addr, whitelist_permissions); + + // Add node + NodeId id = GetNewNodeId(); + uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); + if (!addr_bind.IsValid()) { + addr_bind = GetBindAddress(*sock); } - } else if (pszDest && GetNameProxy(proxy)) { - std::string host; - uint16_t port{default_port}; - SplitHostPort(std::string(pszDest), port, host); - bool proxyConnectionFailed; - sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); - } - if (!sock) { - return nullptr; - } + CNode* pnode = new CNode(id, + std::move(sock), + target_addr, + CalculateKeyedNetGroup(target_addr), + nonce, + addr_bind, + pszDest ? pszDest : "", + conn_type, + /*inbound_onion=*/false, + CNodeOptions{ + .permission_flags = permission_flags, + .i2p_sam_session = std::move(i2p_transient_session), + .recv_flood_size = nReceiveFloodSize, + .use_v2transport = use_v2transport, + }); + pnode->AddRef(); - NetPermissionFlags permission_flags = NetPermissionFlags::None; - std::vector<NetWhitelistPermissions> whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector<NetWhitelistPermissions>{}; - AddWhitelistPermissionFlags(permission_flags, addrConnect, whitelist_permissions); + // We're making a new connection, harvest entropy from the time (and our peer count) + RandAddEvent((uint32_t)id); - // Add node - NodeId id = GetNewNodeId(); - uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - if (!addr_bind.IsValid()) { - addr_bind = GetBindAddress(*sock); + return pnode; } - CNode* pnode = new CNode(id, - std::move(sock), - addrConnect, - CalculateKeyedNetGroup(addrConnect), - nonce, - addr_bind, - pszDest ? pszDest : "", - conn_type, - /*inbound_onion=*/false, - CNodeOptions{ - .permission_flags = permission_flags, - .i2p_sam_session = std::move(i2p_transient_session), - .recv_flood_size = nReceiveFloodSize, - .use_v2transport = use_v2transport, - }); - pnode->AddRef(); - - // We're making a new connection, harvest entropy from the time (and our peer count) - RandAddEvent((uint32_t)id); - return pnode; + return nullptr; } void CNode::CloseSocketDisconnect() @@ -2256,7 +2272,11 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + // Limit number of IPs learned from a single DNS seed. This limit exists to prevent the results from + // one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is + // bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be + // returned. + unsigned int nMaxIPs = 32; const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { diff --git a/src/netbase.cpp b/src/netbase.cpp index 3ca1a5227a..f0fa298378 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -632,10 +632,7 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti std::unique_ptr<Sock> Proxy::Connect() const { - if (!IsValid()) { - LogPrintf("Cannot connect to invalid Proxy\n"); - return {}; - } + if (!IsValid()) return {}; if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true); @@ -656,7 +653,6 @@ std::unique_ptr<Sock> Proxy::Connect() const socklen_t len = sizeof(addrun); if(!ConnectToSocket(*sock, (struct sockaddr*)&addrun, len, path, /*manual_connection=*/true)) { - LogPrintf("Cannot connect to socket for %s\n", path); return {}; } diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 84c3352b9d..2ad79b6f99 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -191,13 +191,13 @@ std::optional<std::pair<DiagramCheckError, std::string>> ImprovesFeerateDiagram( int64_t replacement_vsize) { // Require that the replacement strictly improves the mempool's feerate diagram. - const auto diagram_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + const auto chunk_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; - if (!diagram_results.has_value()) { - return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(diagram_results).original); + if (!chunk_results.has_value()) { + return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(chunk_results).original); } - if (!std::is_gt(CompareFeerateDiagram(diagram_results.value().second, diagram_results.value().first))) { + if (!std::is_gt(CompareChunks(chunk_results.value().second, chunk_results.value().first))) { return std::make_pair(DiagramCheckError::FAILURE, "insufficient feerate: does not improve feerate diagram"); } return std::nullopt; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index c52ef7cd67..efdc3966d1 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -369,21 +369,22 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con else if(type == Receive) { // Generate a new address to associate with given label - auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); - if (!op_dest) { + if (auto dest{walletModel->wallet().getNewDestination(address_type, strLabel)}) { + strAddress = EncodeDestination(*dest); + } else { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); if (!ctx.isValid()) { // Unlock wallet failed or was cancelled editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); - if (!op_dest) { + if (auto dest_retry{walletModel->wallet().getNewDestination(address_type, strLabel)}) { + strAddress = EncodeDestination(*dest_retry); + } else { editStatus = KEY_GENERATION_FAILURE; return QString(); } } - strAddress = EncodeDestination(*op_dest); } else { diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 084b9a6615..1d1a84e375 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -109,22 +109,26 @@ QFont fixedPitchFont(bool use_embedded_font) return QFontDatabase::systemFont(QFontDatabase::FixedFont); } -// Just some dummy data to generate a convincing random-looking (but consistent) address -static const uint8_t dummydata[] = {0xeb,0x15,0x23,0x1d,0xfc,0xeb,0x60,0x92,0x58,0x86,0xb6,0x7d,0x06,0x52,0x99,0x92,0x59,0x15,0xae,0xb1,0x72,0xc0,0x66,0x47}; - -// Generate a dummy address with invalid CRC, starting with the network prefix. +// Return a pre-generated dummy bech32m address (P2TR) with invalid checksum. static std::string DummyAddress(const CChainParams ¶ms) { - std::vector<unsigned char> sourcedata = params.Base58Prefix(CChainParams::PUBKEY_ADDRESS); - sourcedata.insert(sourcedata.end(), dummydata, dummydata + sizeof(dummydata)); - for(int i=0; i<256; ++i) { // Try every trailing byte - std::string s = EncodeBase58(sourcedata); - if (!IsValidDestinationString(s)) { - return s; - } - sourcedata[sourcedata.size()-1] += 1; - } - return ""; + std::string addr; + switch (params.GetChainType()) { + case ChainType::MAIN: + addr = "bc1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tq2jku9f"; + break; + case ChainType::SIGNET: + case ChainType::TESTNET: + addr = "tb1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqa6qnlg"; + break; + case ChainType::REGTEST: + addr = "bcrt1p35yvjel7srp783ztf8v6jdra7dhfzk5jaun8xz2qp6ws7z80n4tqsr2427"; + break; + } // no default case, so the compiler can warn about missing cases + assert(!addr.empty()); + + if (Assume(!IsValidDestinationString(addr))) return addr; + return {}; } void setupAddressWidget(QValidatedLineEdit *widget, QWidget *parent) diff --git a/src/qt/main.cpp b/src/qt/main.cpp index ded057dbfa..16befd99e8 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,7 +4,6 @@ #include <qt/bitcoin.h> -#include <common/url.h> #include <compat/compat.h> #include <util/translation.h> @@ -17,7 +16,6 @@ extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = [](const char* psz) { return QCoreApplication::translate("bitcoin-core", psz).toStdString(); }; -UrlDecodeFn* const URL_DECODE = urlDecode; const std::function<std::string()> G_TEST_GET_FULL_NAME{}; diff --git a/src/qt/notificator.cpp b/src/qt/notificator.cpp index 2021e5f9dc..551c0ffd13 100644 --- a/src/qt/notificator.cpp +++ b/src/qt/notificator.cpp @@ -112,10 +112,10 @@ FreedesktopImage::FreedesktopImage(const QImage &img): for(unsigned int ptr = 0; ptr < num_pixels; ++ptr) { - image[ptr*BYTES_PER_PIXEL+0] = data[ptr] >> 16; // R - image[ptr*BYTES_PER_PIXEL+1] = data[ptr] >> 8; // G - image[ptr*BYTES_PER_PIXEL+2] = data[ptr]; // B - image[ptr*BYTES_PER_PIXEL+3] = data[ptr] >> 24; // A + image[ptr * BYTES_PER_PIXEL + 0] = char(data[ptr] >> 16); // R + image[ptr * BYTES_PER_PIXEL + 1] = char(data[ptr] >> 8); // G + image[ptr * BYTES_PER_PIXEL + 2] = char(data[ptr]); // B + image[ptr * BYTES_PER_PIXEL + 3] = char(data[ptr] >> 24); // A } } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 1bdf94d3b5..fe000bcbb8 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -569,16 +569,17 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } -bool WalletModel::displayAddress(std::string sAddress) const +void WalletModel::displayAddress(std::string sAddress) const { CTxDestination dest = DecodeDestination(sAddress); - bool res = false; try { - res = m_wallet->displayAddress(dest); + util::Result<void> result = m_wallet->displayAddress(dest); + if (!result) { + QMessageBox::warning(nullptr, tr("Signer error"), QString::fromStdString(util::ErrorString(result).translated)); + } } catch (const std::runtime_error& e) { QMessageBox::critical(nullptr, tr("Can't display address"), e.what()); } - return res; } bool WalletModel::isWalletEnabled() diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 503ee16823..ab2096c1fe 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -130,7 +130,7 @@ public: UnlockContext requestUnlock(); bool bumpFee(uint256 hash, uint256& new_hash); - bool displayAddress(std::string sAddress) const; + void displayAddress(std::string sAddress) const; static bool isWalletEnabled(); diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 09e0771534..4926d1e80b 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -134,7 +134,7 @@ void WalletView::processNewTransaction(const QModelIndex& parent, int start, int return; QString date = ttm->index(start, TransactionTableModel::Date, parent).data().toString(); - qint64 amount = ttm->index(start, TransactionTableModel::Amount, parent).data(Qt::EditRole).toULongLong(); + qint64 amount = ttm->index(start, TransactionTableModel::Amount, parent).data(Qt::EditRole).toLongLong(); QString type = ttm->index(start, TransactionTableModel::Type, parent).data().toString(); QModelIndex index = ttm->index(start, 0, parent); QString address = ttm->data(index, TransactionTableModel::AddressRole).toString(); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a1135c27d4..eed004806a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2174,7 +2174,8 @@ static RPCHelpMan scantxoutset() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { UniValue result(UniValue::VOBJ); - if (request.params[0].get_str() == "status") { + const auto action{self.Arg<std::string>("action")}; + if (action == "status") { CoinsViewScanReserver reserver; if (reserver.reserve()) { // no scan in progress @@ -2182,7 +2183,7 @@ static RPCHelpMan scantxoutset() } result.pushKV("progress", g_scan_progress.load()); return result; - } else if (request.params[0].get_str() == "abort") { + } else if (action == "abort") { CoinsViewScanReserver reserver; if (reserver.reserve()) { // reserve was possible which means no scan was running @@ -2191,7 +2192,7 @@ static RPCHelpMan scantxoutset() // set the abort flag g_should_abort_scan = true; return true; - } else if (request.params[0].get_str() == "start") { + } else if (action == "start") { CoinsViewScanReserver reserver; if (!reserver.reserve()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan already in progress, use action \"abort\" or \"status\""); @@ -2260,7 +2261,7 @@ static RPCHelpMan scantxoutset() result.pushKV("unspents", unspents); result.pushKV("total_amount", ValueFromAmount(total_in)); } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", request.params[0].get_str())); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", action)); } return result; }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f7cdbf52dd..454c262803 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -122,7 +122,7 @@ static RPCHelpMan getnetworkhashps() { ChainstateManager& chainman = EnsureAnyChainman(request.context); LOCK(cs_main); - return GetNetworkHashPS(self.Arg<int>(0), self.Arg<int>(1), chainman.ActiveChain()); + return GetNetworkHashPS(self.Arg<int>("nblocks"), self.Arg<int>("height"), chainman.ActiveChain()); }, }; } @@ -229,12 +229,12 @@ static RPCHelpMan generatetodescriptor() "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - const auto num_blocks{self.Arg<int>(0)}; - const auto max_tries{self.Arg<uint64_t>(2)}; + const auto num_blocks{self.Arg<int>("num_blocks")}; + const auto max_tries{self.Arg<uint64_t>("maxtries")}; CScript coinbase_script; std::string error; - if (!getScriptFromDescriptor(self.Arg<std::string>(1), coinbase_script, error)) { + if (!getScriptFromDescriptor(self.Arg<std::string>("descriptor"), coinbase_script, error)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f935a3b08f..49789f538b 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -322,7 +322,7 @@ static RPCHelpMan addnode() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - const std::string command{request.params[1].get_str()}; + const auto command{self.Arg<std::string>("command")}; if (command != "onetry" && command != "add" && command != "remove") { throw std::runtime_error( self.ToString()); @@ -331,9 +331,9 @@ static RPCHelpMan addnode() NodeContext& node = EnsureAnyNodeContext(request.context); CConnman& connman = EnsureConnman(node); - const std::string node_arg{request.params[0].get_str()}; + const auto node_arg{self.Arg<std::string>("node")}; bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2; - bool use_v2transport = self.MaybeArg<bool>(2).value_or(node_v2transport); + bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport); if (use_v2transport && !node_v2transport) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)"); diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index b8c0080aef..ffc2ee5ab0 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -26,6 +26,7 @@ #include <univalue.h> #include <util/any.h> #include <util/check.h> +#include <util/time.h> #include <stdint.h> #ifdef HAVE_MALLOC_INFO @@ -58,9 +59,11 @@ static RPCHelpMan setmocktime() LOCK(cs_main); const int64_t time{request.params[0].getInt<int64_t>()}; - if (time < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime cannot be negative: %s.", time)); + constexpr int64_t max_time{Ticks<std::chrono::seconds>(std::chrono::nanoseconds::max())}; + if (time < 0 || time > max_time) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime must be in the range [0, %s], not %s.", max_time, time)); } + SetMockTime(time); const NodeContext& node_context{EnsureAnyNodeContext(request.context)}; for (const auto& chain_client : node_context.chain_clients) { diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp index 8c752ba1fd..9f3c24efcf 100644 --- a/src/rpc/signmessage.cpp +++ b/src/rpc/signmessage.cpp @@ -38,9 +38,9 @@ static RPCHelpMan verifymessage() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::string strAddress = request.params[0].get_str(); - std::string strSign = request.params[1].get_str(); - std::string strMessage = request.params[2].get_str(); + std::string strAddress = self.Arg<std::string>("address"); + std::string strSign = self.Arg<std::string>("signature"); + std::string strMessage = self.Arg<std::string>("message"); switch (MessageVerify(strAddress, strSign, strMessage)) { case MessageVerificationResult::ERR_INVALID_ADDRESS: diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 6e332e3855..f683878054 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -24,6 +24,8 @@ #include <util/string.h> #include <util/translation.h> +#include <algorithm> +#include <iterator> #include <string_view> #include <tuple> @@ -729,6 +731,16 @@ std::vector<std::pair<std::string, bool>> RPCHelpMan::GetArgNames() const return ret; } +size_t RPCHelpMan::GetParamIndex(std::string_view key) const +{ + auto it{std::find_if( + m_args.begin(), m_args.end(), [&key](const auto& arg) { return arg.GetName() == key;} + )}; + + CHECK_NONFATAL(it != m_args.end()); // TODO: ideally this is checked at compile time + return std::distance(m_args.begin(), it); +} + std::string RPCHelpMan::ToString() const { std::string ret; diff --git a/src/rpc/util.h b/src/rpc/util.h index f6ee6a317a..177af90c05 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -404,18 +404,25 @@ public: UniValue HandleRequest(const JSONRPCRequest& request) const; /** - * Helper to get a request argument. - * This function only works during m_fun(), i.e. it should only be used in - * RPC method implementations. The helper internally checks whether the - * user-passed argument isNull() and parses (from JSON) and returns the - * user-passed argument, or the default value derived from the RPCArg - * documentation, or a falsy value if no default was given. + * @brief Helper to get a required or default-valued request argument. * - * Use Arg<Type>(i) to get the argument or its default value. Otherwise, - * use MaybeArg<Type>(i) to get the optional argument or a falsy value. + * Use this function when the argument is required or when it has a default value. If the + * argument is optional and may not be provided, use MaybeArg instead. * - * The Type passed to this helper must match the corresponding - * RPCArg::Type. + * This function only works during m_fun(), i.e., it should only be used in + * RPC method implementations. It internally checks whether the user-passed + * argument isNull() and parses (from JSON) and returns the user-passed argument, + * or the default value derived from the RPCArg documentation. + * + * There are two overloads of this function: + * - Use Arg<Type>(size_t i) to get the argument (or the default value) by index. + * - Use Arg<Type>(const std::string& key) to get the argument (or the default value) by key. + * + * The Type passed to this helper must match the corresponding RPCArg::Type. + * + * @return The value of the RPC argument (or the default value) cast to type Type. + * + * @see MaybeArg for handling optional arguments without default values. */ template <typename R> auto Arg(size_t i) const @@ -429,6 +436,34 @@ public: return ArgValue<const R&>(i); } } + template<typename R> + auto Arg(std::string_view key) const + { + return Arg<R>(GetParamIndex(key)); + } + /** + * @brief Helper to get an optional request argument. + * + * Use this function when the argument is optional and does not have a default value. If the + * argument is required or has a default value, use Arg instead. + * + * This function only works during m_fun(), i.e., it should only be used in + * RPC method implementations. It internally checks whether the user-passed + * argument isNull() and parses (from JSON) and returns the user-passed argument, + * or a falsy value if no argument was passed. + * + * There are two overloads of this function: + * - Use MaybeArg<Type>(size_t i) to get the optional argument by index. + * - Use MaybeArg<Type>(const std::string& key) to get the optional argument by key. + * + * The Type passed to this helper must match the corresponding RPCArg::Type. + * + * @return For integral and floating-point types, a std::optional<Type> is returned. + * For other types, a Type* pointer to the argument is returned. If the + * argument is not provided, std::nullopt or a null pointer is returned. + * + * @see Arg for handling arguments that are required or have a default value. + */ template <typename R> auto MaybeArg(size_t i) const { @@ -441,6 +476,11 @@ public: return ArgValue<const R*>(i); } } + template<typename R> + auto MaybeArg(std::string_view key) const + { + return MaybeArg<R>(GetParamIndex(key)); + } std::string ToString() const; /** Return the named args that need to be converted from string to another JSON type */ UniValue GetArgMap() const; @@ -460,6 +500,8 @@ private: mutable const JSONRPCRequest* m_req{nullptr}; // A pointer to the request for the duration of m_fun() template <typename R> R ArgValue(size_t i) const; + //! Return positional index of a parameter using its name as key. + size_t GetParamIndex(std::string_view key) const; }; /** diff --git a/src/script/sign.cpp b/src/script/sign.cpp index be4b357568..22ac062a63 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -295,7 +295,7 @@ struct TapSatisfier: Satisfier<XOnlyPubKey> { //! Conversion from a raw xonly public key. template <typename I> std::optional<XOnlyPubKey> FromPKBytes(I first, I last) const { - CHECK_NONFATAL(last - first == 32); + if (last - first != 32) return {}; XOnlyPubKey pubkey; std::copy(first, last, pubkey.begin()); return pubkey; diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp new file mode 100644 index 0000000000..065c7d97bc --- /dev/null +++ b/src/test/common_url_tests.cpp @@ -0,0 +1,75 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include <common/url.h> + +#include <string> + +#include <boost/test/unit_test.hpp> + +BOOST_AUTO_TEST_SUITE(common_url_tests) + +// These test vectors were ported from test/regress.c in the libevent library +// which used to be a dependency of the UrlDecode function. + +BOOST_AUTO_TEST_CASE(encode_decode_test) { + BOOST_CHECK_EQUAL(UrlDecode("Hello"), "Hello"); + BOOST_CHECK_EQUAL(UrlDecode("99"), "99"); + BOOST_CHECK_EQUAL(UrlDecode(""), ""); + BOOST_CHECK_EQUAL(UrlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"); + BOOST_CHECK_EQUAL(UrlDecode("%20"), " "); + BOOST_CHECK_EQUAL(UrlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); + BOOST_CHECK_EQUAL(UrlDecode("%01%19"), "\x01\x19"); + BOOST_CHECK_EQUAL(UrlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), + "http://www.ietf.org/rfc/rfc3986.txt"); + BOOST_CHECK_EQUAL(UrlDecode("1%2B2%3D3"), "1+2=3"); +} + +BOOST_AUTO_TEST_CASE(decode_malformed_test) { + BOOST_CHECK_EQUAL(UrlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); + + BOOST_CHECK_EQUAL(UrlDecode("%"), "%"); + BOOST_CHECK_EQUAL(UrlDecode("%%"), "%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%"), "%%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%%"), "%%%%"); + + BOOST_CHECK_EQUAL(UrlDecode("+"), "+"); + BOOST_CHECK_EQUAL(UrlDecode("++"), "++"); + + BOOST_CHECK_EQUAL(UrlDecode("?"), "?"); + BOOST_CHECK_EQUAL(UrlDecode("??"), "??"); + + BOOST_CHECK_EQUAL(UrlDecode("%G1"), "%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%2"), "%2"); + BOOST_CHECK_EQUAL(UrlDecode("%ZX"), "%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("valid%20string%G1"), "valid string%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%20invalid%ZX"), " invalid%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("%20%G1%ZX"), " %G1%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("%1 "), "%1 "); + BOOST_CHECK_EQUAL(UrlDecode("% 9"), "% 9"); + BOOST_CHECK_EQUAL(UrlDecode(" %Z "), " %Z "); + BOOST_CHECK_EQUAL(UrlDecode(" % X"), " % X"); + + BOOST_CHECK_EQUAL(UrlDecode("%%ffg"), "%\xffg"); + BOOST_CHECK_EQUAL(UrlDecode("%fg"), "%fg"); + + BOOST_CHECK_EQUAL(UrlDecode("%-1"), "%-1"); + BOOST_CHECK_EQUAL(UrlDecode("%1-"), "%1-"); +} + +BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { + BOOST_CHECK_EQUAL(UrlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); +} + +BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { + std::string result1{"\0\0x\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), result1); + std::string result2{"abc\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), result2); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 2e015b382e..5af3c3d7ed 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -82,43 +82,4 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) } -BOOST_AUTO_TEST_CASE(build_diagram_test) -{ - FeeFrac p1{1000, 100}; - FeeFrac empty{0, 0}; - FeeFrac zero_fee{0, 1}; - FeeFrac oversized_1{4611686000000, 4000000}; - FeeFrac oversized_2{184467440000000, 100000}; - - // Diagram-building will reorder chunks - std::vector<FeeFrac> chunks; - chunks.push_back(p1); - chunks.push_back(zero_fee); - chunks.push_back(empty); - chunks.push_back(oversized_1); - chunks.push_back(oversized_2); - - // Caller in charge of sorting chunks in case chunk size limit - // differs from cluster size limit - std::sort(chunks.begin(), chunks.end(), [](const FeeFrac& a, const FeeFrac& b) { return a > b; }); - - // Chunks are now sorted in reverse order (largest first) - BOOST_CHECK(chunks[0] == empty); // empty is considered "highest" fee - BOOST_CHECK(chunks[1] == oversized_2); - BOOST_CHECK(chunks[2] == oversized_1); - BOOST_CHECK(chunks[3] == p1); - BOOST_CHECK(chunks[4] == zero_fee); - - std::vector<FeeFrac> generated_diagram{BuildDiagramFromChunks(chunks)}; - BOOST_CHECK(generated_diagram.size() == 1 + chunks.size()); - - // Prepended with an empty, then the chunks summed in order as above - BOOST_CHECK(generated_diagram[0] == empty); - BOOST_CHECK(generated_diagram[1] == empty); - BOOST_CHECK(generated_diagram[2] == oversized_2); - BOOST_CHECK(generated_diagram[3] == oversized_2 + oversized_1); - BOOST_CHECK(generated_diagram[4] == oversized_2 + oversized_1 + p1); - BOOST_CHECK(generated_diagram[5] == oversized_2 + oversized_1 + p1 + zero_fee); -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 5100b6f438..071e5fb029 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -24,12 +24,14 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) assert(is_addition_overflow_custom == AdditionOverflow(j, i)); assert(maybe_add == CheckedAdd(j, i)); assert(sat_add == SaturatingAdd(j, i)); +#ifndef _MSC_VER T result_builtin; const bool is_addition_overflow_builtin = __builtin_add_overflow(i, j, &result_builtin); assert(is_addition_overflow_custom == is_addition_overflow_builtin); if (!is_addition_overflow_custom) { assert(i + j == result_builtin); } +#endif if (is_addition_overflow_custom) { assert(sat_add == std::numeric_limits<T>::min() || sat_add == std::numeric_limits<T>::max()); } else { diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index ebc5673e71..c9a3bc86ac 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -33,7 +33,6 @@ #include <optional> #include <stdexcept> #include <stdint.h> -#include <unistd.h> using node::SnapshotMetadata; diff --git a/src/test/fuzz/feeratediagram.cpp b/src/test/fuzz/feeratediagram.cpp index 6d710093cb..1a9c5ee946 100644 --- a/src/test/fuzz/feeratediagram.cpp +++ b/src/test/fuzz/feeratediagram.cpp @@ -16,6 +16,20 @@ namespace { +/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */ +std::vector<FeeFrac> BuildDiagramFromChunks(const Span<const FeeFrac> chunks) +{ + std::vector<FeeFrac> diagram; + diagram.reserve(chunks.size() + 1); + + diagram.emplace_back(0, 0); + for (auto& chunk : chunks) { + diagram.emplace_back(diagram.back() + chunk); + } + return diagram; +} + + /** Evaluate a diagram at a specific size, returning the fee as a fraction. * * Fees in diagram cannot exceed 2^32, as the returned evaluation could overflow @@ -103,7 +117,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram) assert(diagram1.front() == empty); assert(diagram2.front() == empty); - auto real = CompareFeerateDiagram(diagram1, diagram2); + auto real = CompareChunks(chunks1, chunks2); auto sim = CompareDiagrams(diagram1, diagram2); assert(real == sim); diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index a8e490b459..f9915187bd 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -25,7 +25,6 @@ #include <memory> #include <string> #include <tuple> -#include <unistd.h> #include <utility> #include <vector> @@ -135,9 +134,9 @@ void initialize() #if defined(PROVIDE_FUZZ_MAIN_FUNCTION) static bool read_stdin(std::vector<uint8_t>& data) { - uint8_t buffer[1024]; - ssize_t length = 0; - while ((length = read(STDIN_FILENO, buffer, 1024)) > 0) { + std::istream::char_type buffer[1024]; + std::streamsize length; + while ((std::cin.read(buffer, 1024), length = std::cin.gcount()) > 0) { data.insert(data.end(), buffer, buffer + length); } return length == 0; diff --git a/src/test/fuzz/multiplication_overflow.cpp b/src/test/fuzz/multiplication_overflow.cpp index aeef4f24b7..a762a4dfe3 100644 --- a/src/test/fuzz/multiplication_overflow.cpp +++ b/src/test/fuzz/multiplication_overflow.cpp @@ -17,12 +17,18 @@ void TestMultiplicationOverflow(FuzzedDataProvider& fuzzed_data_provider) const T i = fuzzed_data_provider.ConsumeIntegral<T>(); const T j = fuzzed_data_provider.ConsumeIntegral<T>(); const bool is_multiplication_overflow_custom = MultiplicationOverflow(i, j); +#ifndef _MSC_VER T result_builtin; const bool is_multiplication_overflow_builtin = __builtin_mul_overflow(i, j, &result_builtin); assert(is_multiplication_overflow_custom == is_multiplication_overflow_builtin); if (!is_multiplication_overflow_custom) { assert(i * j == result_builtin); } +#else + if (!is_multiplication_overflow_custom) { + (void)(i * j); + } +#endif } } // namespace diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index ab75afe066..2bf47930f4 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -72,7 +72,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) available.insert(i); } - if (add_to_mempool) { + if (add_to_mempool && !pool.exists(GenTxid::Txid(tx->GetHash()))) { LOCK2(cs_main, pool.cs); pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); available.insert(i); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 754aff4e70..64785948f6 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -82,17 +82,6 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) } } -void CheckDiagramConcave(std::vector<FeeFrac>& diagram) -{ - // Diagrams are in monotonically-decreasing feerate order. - FeeFrac last_chunk = diagram.front(); - for (size_t i = 1; i<diagram.size(); ++i) { - FeeFrac next_chunk = diagram[i] - diagram[i-1]; - assert(next_chunk <= last_chunk); - last_chunk = next_chunk; - } -} - FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); @@ -107,6 +96,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) std::vector<CTransaction> mempool_txs; size_t iter{0}; + int32_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(1, 1000000); + + // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow + // Add replacement_vsize since this is added to new diagram during RBF check + int64_t running_vsize_total{replacement_vsize}; + LOCK2(cs_main, pool.cs); LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) @@ -114,19 +109,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) // Make sure txns only have one input, and that a unique input is given to avoid circular references std::optional<CMutableTransaction> parent = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS); if (!parent) { - continue; + return; } assert(iter <= g_outpoints.size()); parent->vin.resize(1); parent->vin[0].prevout = g_outpoints[iter++]; mempool_txs.emplace_back(*parent); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto parent_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += parent_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits<int32_t>::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(parent_entry); if (fuzzed_data_provider.ConsumeBool() && !child->vin.empty()) { child->vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0}; } mempool_txs.emplace_back(*child); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto child_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += child_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits<int32_t>::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(child_entry); if (fuzzed_data_provider.ConsumeBool()) { pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000)); @@ -147,33 +156,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) pool.CalculateDescendants(txiter, all_conflicts); } - // Calculate the feerate diagrams for a replacement. + // Calculate the chunks for a replacement. CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider); - int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(1, 1000000); - auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + auto calc_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; if (calc_results.has_value()) { - // Sanity checks on the diagrams. + // Sanity checks on the chunks. - // Diagrams start at 0. - assert(calc_results->first.front().size == 0); - assert(calc_results->first.front().fee == 0); - assert(calc_results->second.front().size == 0); - assert(calc_results->second.front().fee == 0); - - CheckDiagramConcave(calc_results->first); - CheckDiagramConcave(calc_results->second); + // Feerates are monotonically decreasing. + FeeFrac first_sum; + for (size_t i = 0; i < calc_results->first.size(); ++i) { + first_sum += calc_results->first[i]; + if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i])); + } + FeeFrac second_sum; + for (size_t i = 0; i < calc_results->second.size(); ++i) { + second_sum += calc_results->second[i]; + if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i])); + } - CAmount replaced_fee{0}; - int64_t replaced_size{0}; + FeeFrac replaced; for (auto txiter : all_conflicts) { - replaced_fee += txiter->GetModifiedFee(); - replaced_size += txiter->GetTxSize(); + replaced.fee += txiter->GetModifiedFee(); + replaced.size += txiter->GetTxSize(); } - // The total fee of the new diagram should be the total fee of the old - // diagram - replaced_fee + replacement_fees - assert(calc_results->first.back().fee - replaced_fee + replacement_fees == calc_results->second.back().fee); - assert(calc_results->first.back().size - replaced_size + replacement_vsize == calc_results->second.back().size); + // The total fee & size of the new diagram minus replaced fee & size should be the total + // fee & size of the old diagram minus replacement fee & size. + assert((first_sum - replaced) == (second_sum - FeeFrac{replacement_fees, replacement_vsize})); } // If internals report error, wrapper should too @@ -182,10 +191,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE); } else { // Diagram check succeeded + auto old_sum = std::accumulate(calc_results->first.begin(), calc_results->first.end(), FeeFrac{}); + auto new_sum = std::accumulate(calc_results->second.begin(), calc_results->second.end(), FeeFrac{}); if (!err_tuple.has_value()) { // New diagram's final fee should always match or exceed old diagram's - assert(calc_results->first.back().fee <= calc_results->second.back().fee); - } else if (calc_results->first.back().fee > calc_results->second.back().fee) { + assert(old_sum.fee <= new_sum.fee); + } else if (old_sum.fee > new_sum.fee) { // Or it failed, and if old diagram had higher fees, it should be a failure assert(err_tuple.value().first == DiagramCheckError::FAILURE); } diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index e81efac6e0..631da13803 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -90,7 +90,7 @@ FUZZ_TARGET(string) (void)ToUpper(random_string_1); (void)TrimString(random_string_1); (void)TrimString(random_string_1, random_string_2); - (void)urlDecode(random_string_1); + (void)UrlDecode(random_string_1); (void)ContainsNoNUL(random_string_1); (void)_(random_string_1.c_str()); try { diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 217e4a6d22..9f8d434213 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -202,9 +202,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx7.vout[1].nValue = 1 * COIN; - auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())}; - BOOST_REQUIRE(ancestors_calculated.has_value()); - BOOST_CHECK(*ancestors_calculated == setAncestors); + { + auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())}; + BOOST_REQUIRE(ancestors_calculated.has_value()); + BOOST_CHECK(*ancestors_calculated == setAncestors); + } pool.addUnchecked(entry.FromTx(tx7), setAncestors); BOOST_CHECK_EQUAL(pool.size(), 7U); @@ -260,9 +262,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx10.vout[0].nValue = 10 * COIN; - ancestors_calculated = pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits()); - BOOST_REQUIRE(ancestors_calculated); - BOOST_CHECK(*ancestors_calculated == setAncestors); + { + auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits())}; + BOOST_REQUIRE(ancestors_calculated); + BOOST_CHECK(*ancestors_calculated == setAncestors); + } pool.addUnchecked(entry.FromTx(tx10), setAncestors); diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index ed33969710..19e45c550a 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -426,21 +426,21 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Replacement of size 1 { - const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; + const auto replace_one{pool.CalculateChunksForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; - BOOST_CHECK(replace_one->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(0, 1)}; - BOOST_CHECK(replace_one->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee, low_size}}; + BOOST_CHECK(replace_one->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{0, 1}}; + BOOST_CHECK(replace_one->second == expected_new_chunks); } // Non-zero replacement fee/size { - const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; + const auto replace_one_fee{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one_fee.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; + std::vector<FeeFrac> expected_old_diagram{{low_fee, low_size}}; BOOST_CHECK(replace_one_fee->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; + std::vector<FeeFrac> expected_new_diagram{{high_fee, low_size}}; BOOST_CHECK(replace_one_fee->second == expected_new_diagram); } @@ -451,22 +451,22 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto high_size = entry_high->GetTxSize(); { - const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; + const auto replace_single_chunk{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; BOOST_CHECK(replace_single_chunk.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_single_chunk->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; - BOOST_CHECK(replace_single_chunk->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_single_chunk->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}}; + BOOST_CHECK(replace_single_chunk->second == expected_new_chunks); } // Conflict with the 2nd tx, resulting in new diagram with three entries { - const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; + const auto replace_cpfp_child{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; BOOST_CHECK(replace_cpfp_child.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_cpfp_child->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size), FeeFrac(low_fee + high_fee, low_size + low_size)}; - BOOST_CHECK(replace_cpfp_child->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_cpfp_child->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}, {low_fee, low_size}}; + BOOST_CHECK(replace_cpfp_child->second == expected_new_chunks); } // third transaction causes the topology check to fail @@ -476,7 +476,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto normal_size = entry_normal->GetTxSize(); { - const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; + const auto replace_too_large{pool.CalculateChunksForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; BOOST_CHECK(!replace_too_large.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex())); } @@ -493,12 +493,12 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto low_size_2 = entry_low_2->GetTxSize(); { - const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; + const auto replace_two_chunks_single_cluster{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; BOOST_CHECK(replace_two_chunks_single_cluster.has_value()); - std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(high_fee, high_size_2), FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_diagram); - std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_diagram); + std::vector<FeeFrac> expected_old_chunks{{high_fee, high_size_2}, {low_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_chunks); + std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_chunks); } // You can have more than two direct conflicts if the there are multiple affected clusters, all of size 2 or less @@ -515,10 +515,10 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value(); { - const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; + const auto replace_multiple_clusters{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; BOOST_CHECK(replace_multiple_clusters.has_value()); - BOOST_CHECK(replace_multiple_clusters->first.size() == 4); - BOOST_CHECK(replace_multiple_clusters->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters->first.size() == 3); + BOOST_CHECK(replace_multiple_clusters->second.size() == 1); } // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate @@ -527,11 +527,11 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; + const auto replace_multiple_clusters_2{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; BOOST_CHECK(replace_multiple_clusters_2.has_value()); - BOOST_CHECK(replace_multiple_clusters_2->first.size() == 5); - BOOST_CHECK(replace_multiple_clusters_2->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters_2->first.size() == 4); + BOOST_CHECK(replace_multiple_clusters_2->second.size() == 1); } // Add another descendant to conflict_1, making the cluster size > 2 should fail at this point. @@ -540,86 +540,86 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_grand_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; + const auto replace_cluster_size_3{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; BOOST_CHECK(!replace_cluster_size_3.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); } } -BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) +BOOST_AUTO_TEST_CASE(feerate_chunks_utilities) { - // Sanity check the correctness of the feerate diagram comparison. + // Sanity check the correctness of the feerate chunks comparison. // A strictly better case. - std::vector<FeeFrac> old_diagram{{FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}}; - std::vector<FeeFrac> new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}}; + std::vector<FeeFrac> old_chunks{{{950, 300}, {100, 100}}}; + std::vector<FeeFrac> new_chunks{{{1000, 300}, {50, 100}}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Incomparable diagrams - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1000, 300}, {0, 100}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // Strictly better but smaller size. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 300}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // New diagram is strictly better due to the first chunk, even though // second chunk contributes no fees - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 100}, {0, 100}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Feerate of first new chunk is better with, but second chunk is worse - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {249, 250}, {151, 650}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // If we make the second chunk slightly better, the new diagram now wins. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {250, 250}, {150, 150}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Identical diagrams, cannot be strictly better - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{950, 300}, {100, 100}}; - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_eq(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_eq(CompareChunks(new_chunks, old_chunks))); // Same aggregate fee, but different total size (trigger single tail fee check step) - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}}; // No change in evaluation when tail check needed. - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Trigger multiple tail fee check steps - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}}; - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Multiple tail fee check steps, unordered result - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}, {1, 1}}; + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 0d2460c606..acacb6257d 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -291,6 +291,7 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values) BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("1e-8")), COIN/100000000); BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.1e-7")), COIN/100000000); BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.01e-6")), COIN/100000000); + BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.00000000000000000000000000000000000001e+30")), 1); BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.0000000000000000000000000000000000000000000000000000000000000000000000000001e+68")), COIN/100000000); BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("10000000000000000000000000000000000000000000000000000000000000000e-64")), COIN); BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000e64")), COIN); @@ -581,4 +582,72 @@ BOOST_AUTO_TEST_CASE(help_example) BOOST_CHECK_NE(HelpExampleRpcNamed("foo", {{"arg", true}}), HelpExampleRpcNamed("foo", {{"arg", "true"}})); } +static void CheckRpc(const std::vector<RPCArg>& params, const UniValue& args, RPCHelpMan::RPCMethodImpl test_impl) +{ + auto null_result{RPCResult{RPCResult::Type::NONE, "", "None"}}; + const RPCHelpMan rpc{"dummy", "dummy description", params, null_result, RPCExamples{""}, test_impl}; + JSONRPCRequest req; + req.params = args; + + rpc.HandleRequest(req); +} + +BOOST_AUTO_TEST_CASE(rpc_arg_helper) +{ + constexpr bool DEFAULT_BOOL = true; + constexpr auto DEFAULT_STRING = "default"; + constexpr uint64_t DEFAULT_UINT64_T = 3; + + //! Parameters with which the RPCHelpMan is instantiated + const std::vector<RPCArg> params{ + // Required arg + {"req_int", RPCArg::Type::NUM, RPCArg::Optional::NO, ""}, + {"req_str", RPCArg::Type::STR, RPCArg::Optional::NO, ""}, + // Default arg + {"def_uint64_t", RPCArg::Type::NUM, RPCArg::Default{DEFAULT_UINT64_T}, ""}, + {"def_string", RPCArg::Type::STR, RPCArg::Default{DEFAULT_STRING}, ""}, + {"def_bool", RPCArg::Type::BOOL, RPCArg::Default{DEFAULT_BOOL}, ""}, + // Optional arg without default + {"opt_double", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, ""}, + {"opt_string", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""} + }; + + //! Check that `self.Arg` returns the same value as the `request.params` accessors + RPCHelpMan::RPCMethodImpl check_positional = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + BOOST_CHECK_EQUAL(self.Arg<int>(0), request.params[0].getInt<int>()); + BOOST_CHECK_EQUAL(self.Arg<std::string>(1), request.params[1].get_str()); + BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), request.params[2].isNull() ? DEFAULT_UINT64_T : request.params[2].getInt<uint64_t>()); + BOOST_CHECK_EQUAL(self.Arg<std::string>(3), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str()); + BOOST_CHECK_EQUAL(self.Arg<bool>(4), request.params[4].isNull() ? DEFAULT_BOOL : request.params[4].get_bool()); + if (!request.params[5].isNull()) { + BOOST_CHECK_EQUAL(self.MaybeArg<double>(5).value(), request.params[5].get_real()); + } else { + BOOST_CHECK(!self.MaybeArg<double>(5)); + } + if (!request.params[6].isNull()) { + BOOST_CHECK(self.MaybeArg<std::string>(6)); + BOOST_CHECK_EQUAL(*self.MaybeArg<std::string>(6), request.params[6].get_str()); + } else { + BOOST_CHECK(!self.MaybeArg<std::string>(6)); + } + return UniValue{}; + }; + CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_positional); + CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_positional); + + //! Check that `self.Arg` returns the same value when using index and key + RPCHelpMan::RPCMethodImpl check_named = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + BOOST_CHECK_EQUAL(self.Arg<int>(0), self.Arg<int>("req_int")); + BOOST_CHECK_EQUAL(self.Arg<std::string>(1), self.Arg<std::string>("req_str")); + BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), self.Arg<uint64_t>("def_uint64_t")); + BOOST_CHECK_EQUAL(self.Arg<std::string>(3), self.Arg<std::string>("def_string")); + BOOST_CHECK_EQUAL(self.Arg<bool>(4), self.Arg<bool>("def_bool")); + BOOST_CHECK(self.MaybeArg<double>(5) == self.MaybeArg<double>("opt_double")); + BOOST_CHECK(self.MaybeArg<std::string>(6) == self.MaybeArg<std::string>("opt_string")); + return UniValue{}; + }; + CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_named); + CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_named); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 0af2fdce08..e4142e203c 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1254,6 +1254,30 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) BOOST_CHECK(combined.scriptSig == partial3c); } +/** + * Reproduction of an exception incorrectly raised when parsing a public key inside a TapMiniscript. + */ +BOOST_AUTO_TEST_CASE(sign_invalid_miniscript) +{ + FillableSigningProvider keystore; + SignatureData sig_data; + CMutableTransaction prev, curr; + + // Create a Taproot output which contains a leaf in which a non-32 bytes push is used where a public key is expected + // by the Miniscript parser. This offending Script was found by the RPC fuzzer. + const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")}; + TaprootBuilder builder; + builder.Add(0, {invalid_pubkey}, 0xc0); + XOnlyPubKey nums{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0")}; + builder.Finalize(nums); + prev.vout.emplace_back(0, GetScriptForDestination(builder.GetOutput())); + curr.vin.emplace_back(COutPoint{prev.GetHash(), 0}); + sig_data.tr_spenddata = builder.GetSpendData(); + + // SignSignature can fail but it shouldn't raise an exception (nor crash). + BOOST_CHECK(!SignSignature(keystore, CTransaction(prev), curr, 0, SIGHASH_ALL, sig_data)); +} + BOOST_AUTO_TEST_CASE(script_standard_push) { ScriptError err; diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 8aab2b565c..2de147deea 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -11,7 +11,7 @@ #include <univalue.h> #ifdef ENABLE_EXTERNAL_SIGNER -#include <util/subprocess.hpp> +#include <util/subprocess.h> #endif // ENABLE_EXTERNAL_SIGNER #include <boost/test/unit_test.hpp> diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2c18184261..38350b33cc 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -14,7 +14,6 @@ #include <banman.h> #include <chainparams.h> #include <common/system.h> -#include <common/url.h> #include <consensus/consensus.h> #include <consensus/params.h> #include <consensus/validation.h> @@ -81,7 +80,6 @@ using node::RegenerateCommitments; using node::VerifyLoadedChainstate; const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */ static FastRandomContext g_insecure_rand_ctx_temp_path; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 82eec6241f..06066e38b2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1280,7 +1280,7 @@ std::optional<std::string> CTxMemPool::CheckConflictTopology(const setEntries& d return std::nullopt; } -util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) +util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) { Assume(replacement_vsize > 0); @@ -1335,7 +1335,6 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(old_chunks.begin(), old_chunks.end(), std::greater()); - std::vector<FeeFrac> old_diagram = BuildDiagramFromChunks(old_chunks); std::vector<FeeFrac> new_chunks; @@ -1365,6 +1364,5 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(new_chunks.begin(), new_chunks.end(), std::greater()); - std::vector<FeeFrac> new_diagram = BuildDiagramFromChunks(new_chunks); - return std::make_pair(old_diagram, new_diagram); + return std::make_pair(old_chunks, new_chunks); } diff --git a/src/txmempool.h b/src/txmempool.h index 9dd4d56da7..52a3dc2d7d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -738,7 +738,7 @@ public: } /** - * Calculate the old and new mempool feerate diagrams relating to the + * Calculate the sorted chunks for the old and new mempool relating to the * clusters that would be affected by a potential replacement transaction. * (replacement_fees, replacement_vsize) values are gathered from a * proposed set of replacement transactions that are considered as a single @@ -752,7 +752,7 @@ public: * @param[in] all_conflicts All transactions that would be removed * @return old and new diagram pair respectively, or an error string if the conflicts don't match a calculable topology */ - util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); /* Check that all direct conflicts are in a cluster size of two or less. Each * direct conflict may be in a separate cluster. diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index 8b90448b36..1c724555f3 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -421,7 +421,7 @@ void univalue_readwrite() // Valid, with leading or trailing whitespace BOOST_CHECK(v.read(" 1.0") && (v.get_real() == 1.0)); BOOST_CHECK(v.read("1.0 ") && (v.get_real() == 1.0)); - BOOST_CHECK(v.read("0.00000000000000000000000000000000000001e+30 ") && v.get_real() == 1e-8); + BOOST_CHECK(v.read("0.00000000000000000000000000000000000001e+30 ")); BOOST_CHECK(!v.read(".19e-6")); //should fail, missing leading 0, therefore invalid JSON // Invalid, initial garbage diff --git a/src/util/feefrac.cpp b/src/util/feefrac.cpp index 68fb836936..5b6173835c 100644 --- a/src/util/feefrac.cpp +++ b/src/util/feefrac.cpp @@ -7,39 +7,26 @@ #include <array> #include <vector> -std::vector<FeeFrac> BuildDiagramFromChunks(const Span<const FeeFrac> chunks) -{ - std::vector<FeeFrac> diagram; - diagram.reserve(chunks.size() + 1); - - diagram.emplace_back(0, 0); - for (auto& chunk : chunks) { - diagram.emplace_back(diagram.back() + chunk); - } - return diagram; -} - -std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const FeeFrac> dia1) +std::partial_ordering CompareChunks(Span<const FeeFrac> chunks0, Span<const FeeFrac> chunks1) { /** Array to allow indexed access to input diagrams. */ - const std::array<Span<const FeeFrac>, 2> dias = {dia0, dia1}; + const std::array<Span<const FeeFrac>, 2> chunk = {chunks0, chunks1}; /** How many elements we have processed in each input. */ - size_t next_index[2] = {1, 1}; + size_t next_index[2] = {0, 0}; + /** Accumulated fee/sizes in diagrams, up to next_index[i] - 1. */ + FeeFrac accum[2]; /** Whether the corresponding input is strictly better than the other at least in one place. */ bool better_somewhere[2] = {false, false}; /** Get the first unprocessed point in diagram number dia. */ - const auto next_point = [&](int dia) { return dias[dia][next_index[dia]]; }; + const auto next_point = [&](int dia) { return chunk[dia][next_index[dia]] + accum[dia]; }; /** Get the last processed point in diagram number dia. */ - const auto prev_point = [&](int dia) { return dias[dia][next_index[dia] - 1]; }; - - // Diagrams should be non-empty, and first elements zero in size and fee - Assert(!dia0.empty() && !dia1.empty()); - Assert(prev_point(0).IsEmpty()); - Assert(prev_point(1).IsEmpty()); + const auto prev_point = [&](int dia) { return accum[dia]; }; + /** Move to the next point in diagram number dia. */ + const auto advance = [&](int dia) { accum[dia] += chunk[dia][next_index[dia]++]; }; do { - bool done_0 = next_index[0] == dias[0].size(); - bool done_1 = next_index[1] == dias[1].size(); + bool done_0 = next_index[0] == chunk[0].size(); + bool done_1 = next_index[1] == chunk[1].size(); if (done_0 && done_1) break; // Determine which diagram has the first unprocessed point. If a single side is finished, use the @@ -69,17 +56,16 @@ std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const // If B and P have the same size, B can be marked as processed (in addition to P, see // below), as we've already performed a comparison at this size. - if (point_b.size == point_p.size) ++next_index[!unproc_side]; + if (point_b.size == point_p.size) advance(!unproc_side); } // If P lies above AB, unproc_side is better in P. If P lies below AB, then !unproc_side is // better in P. if (std::is_gt(cmp)) better_somewhere[unproc_side] = true; if (std::is_lt(cmp)) better_somewhere[!unproc_side] = true; - ++next_index[unproc_side]; + advance(unproc_side); // If both diagrams are better somewhere, they are incomparable. if (better_somewhere[0] && better_somewhere[1]) return std::partial_ordering::unordered; - } while(true); // Otherwise compare the better_somewhere values. diff --git a/src/util/feefrac.h b/src/util/feefrac.h index 7102f85f88..9772162010 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -146,15 +146,14 @@ struct FeeFrac } }; -/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */ -std::vector<FeeFrac> BuildDiagramFromChunks(Span<const FeeFrac> chunks); - -/** Compares two feerate diagrams. The shorter one is implicitly - * extended with a horizontal straight line. +/** Compare the feerate diagrams implied by the provided sorted chunks data. + * + * The implied diagram for each starts at (0, 0), then contains for each chunk the cumulative fee + * and size up to that chunk, and then extends infinitely to the right with a horizontal line. * - * A feerate diagram consists of a list of (fee, size) points with the property that size - * is strictly increasing and that the first entry is (0, 0). + * The caller must guarantee that the sum of the FeeFracs in either of the chunks' data set do not + * overflow (so sum fees < 2^63, and sum sizes < 2^31). */ -std::partial_ordering CompareFeerateDiagram(Span<const FeeFrac> dia0, Span<const FeeFrac> dia1); +std::partial_ordering CompareChunks(Span<const FeeFrac> chunks0, Span<const FeeFrac> chunks1); #endif // BITCOIN_UTIL_FEEFRAC_H diff --git a/src/util/result.h b/src/util/result.h index b99995c7e5..122a7638fa 100644 --- a/src/util/result.h +++ b/src/util/result.h @@ -39,6 +39,16 @@ private: std::variant<bilingual_str, T> m_variant; + //! Disallow copy constructor, require Result to be moved for efficiency. + Result(const Result&) = delete; + + //! Disallow operator= to avoid confusion in the future when the Result + //! class gains support for richer error reporting, and callers should have + //! ability to set a new result value without clearing existing error + //! messages. + Result& operator=(const Result&) = delete; + Result& operator=(Result&&) = delete; + template <typename FT> friend bilingual_str ErrorString(const Result<FT>& result); @@ -46,6 +56,8 @@ public: Result() : m_variant{std::in_place_index_t<1>{}, std::monostate{}} {} // constructor for void Result(T obj) : m_variant{std::in_place_index_t<1>{}, std::move(obj)} {} Result(Error error) : m_variant{std::in_place_index_t<0>{}, std::move(error.message)} {} + Result(Result&&) = default; + ~Result() = default; //! std::optional methods, so functions returning optional<T> can change to //! return Result<T> with minimal changes to existing code, and vice versa. diff --git a/src/util/subprocess.hpp b/src/util/subprocess.h index dd53a8fbb4..4acfa8ff83 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.h @@ -1,6 +1,8 @@ +// Based on the https://github.com/arun11299/cpp-subprocess project. + /*! -Documentation for C++ subprocessing libraray. +Documentation for C++ subprocessing library. @copyright The code is licensed under the [MIT License](http://opensource.org/licenses/MIT): @@ -31,8 +33,10 @@ Documentation for C++ subprocessing libraray. @version 1.0.0 */ -#ifndef SUBPROCESS_HPP -#define SUBPROCESS_HPP +#ifndef BITCOIN_UTIL_SUBPROCESS_H +#define BITCOIN_UTIL_SUBPROCESS_H + +#include <util/syserror.h> #include <algorithm> #include <cassert> @@ -106,7 +110,7 @@ namespace subprocess { // from pipe static const size_t SP_MAX_ERR_BUF_SIZ = 1024; -// Default buffer capcity for OutBuffer and ErrBuffer. +// Default buffer capacity for OutBuffer and ErrBuffer. // If the data exceeds this capacity, the buffer size is grown // by 1.5 times its previous capacity static const size_t DEFAULT_BUF_CAP_BYTES = 8192; @@ -148,24 +152,11 @@ class OSError: public std::runtime_error { public: OSError(const std::string& err_msg, int err_code): - std::runtime_error( err_msg + ": " + std::strerror(err_code) ) + std::runtime_error(err_msg + ": " + SysErrorString(err_code)) {} }; //-------------------------------------------------------------------- - -//Environment Variable types -#ifndef _MSC_VER - using env_string_t = std::string; - using env_char_t = char; -#else - using env_string_t = std::wstring; - using env_char_t = wchar_t; -#endif -using env_map_t = std::map<env_string_t, env_string_t>; -using env_vector_t = std::vector<env_char_t>; - -//-------------------------------------------------------------------- namespace util { template <typename R> @@ -303,100 +294,6 @@ namespace util if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0)) throw OSError("SetHandleInformation", 0); } - - // env_map_t MapFromWindowsEnvironment() - // * Imports current Environment in a C-string table - // * Parses the strings by splitting on the first "=" per line - // * Creates a map of the variables - // * Returns the map - inline env_map_t MapFromWindowsEnvironment(){ - wchar_t *variable_strings_ptr; - wchar_t *environment_strings_ptr; - std::wstring delimeter(L"="); - int del_len = delimeter.length(); - env_map_t mapped_environment; - - // Get a pointer to the environment block. - environment_strings_ptr = GetEnvironmentStringsW(); - // If the returned pointer is NULL, exit. - if (environment_strings_ptr == NULL) - { - throw OSError("GetEnvironmentStringsW", 0); - } - - // Variable strings are separated by NULL byte, and the block is - // terminated by a NULL byte. - - variable_strings_ptr = (wchar_t *) environment_strings_ptr; - - //Since the environment map ends with a null, we can loop until we find it. - while (*variable_strings_ptr) - { - // Create a string from Variable String - env_string_t current_line(variable_strings_ptr); - // Find the first "equals" sign. - auto pos = current_line.find(delimeter); - // Assuming it's not missing ... - if(pos!=std::wstring::npos){ - // ... parse the key and value. - env_string_t key = current_line.substr(0, pos); - env_string_t value = current_line.substr(pos + del_len); - // Map the entry. - mapped_environment[key] = value; - } - // Jump to next line in the environment map. - variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1; - } - // We're done with the old environment map buffer. - FreeEnvironmentStringsW(environment_strings_ptr); - - // Return the map. - return mapped_environment; - } - - // env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - // * Creates a vector buffer for the new environment string table - // * Copies in the mapped variables - // * Returns the vector - inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - { - // Make a new environment map buffer. - env_vector_t environment_map_buffer; - // Give it some space. - environment_map_buffer.reserve(4096); - - // And fill'er up. - for(auto kv: source_map){ - // Create the line - env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; - // Add the line to the buffer. - std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); - // Append a null - environment_map_buffer.push_back(0); - } - // Append one last null because of how Windows does it's environment maps. - environment_map_buffer.push_back(0); - - return environment_map_buffer; - } - - // env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map) - // * Merges host environment with new mapped variables - // * Creates and returns string vector based on map - inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){ - // Import the environment map - env_map_t environment_map = MapFromWindowsEnvironment(); - // Merge in the changes with overwrite - for(auto& it: changes_map) - { - environment_map[it.first] = it.second; - } - // Create a Windows-usable Environment Map Buffer - env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map); - - return environment_map_strings_vector; - } - #endif /*! @@ -429,26 +326,6 @@ namespace util } - /*! - * Function: join - * Parameters: - * [in] vec : Vector of strings which needs to be joined to form - * a single string with words seperated by a seperator char. - * [in] sep : String used to seperate 2 words in the joined string. - * Default constructed to ' ' (space). - * [out] string: Joined string. - */ - static inline - std::string join(const std::vector<std::string>& vec, - const std::string& sep = " ") - { - std::string res; - for (auto& elem : vec) res.append(elem + sep); - res.erase(--res.end()); - return res; - } - - #ifndef __USING_WINDOWS__ /*! * Function: set_clo_on_exec @@ -650,56 +527,6 @@ namespace util */ /*! - * The buffer size of the stdin/stdout/stderr - * streams of the child process. - * Default value is 0. - */ -struct bufsize { - explicit bufsize(int siz): bufsiz(siz) {} - int bufsiz = 0; -}; - -/*! - * Option to defer spawning of the child process - * till `Popen::start_process` API is called. - * Default value is false. - */ -struct defer_spawn { - explicit defer_spawn(bool d): defer(d) {} - bool defer = false; -}; - -/*! - * Option to close all file descriptors - * when the child process is spawned. - * The close fd list does not include - * input/output/error if they are explicitly - * set as part of the Popen arguments. - * - * Default value is false. - */ -struct close_fds { - explicit close_fds(bool c): close_all(c) {} - bool close_all = false; -}; - -/*! - * Option to make the child process as the - * session leader and thus the process - * group leader. - * Default value is false. - */ -struct session_leader { - explicit session_leader(bool sl): leader_(sl) {} - bool leader_ = false; -}; - -struct shell { - explicit shell(bool s): shell_(s) {} - bool shell_ = false; -}; - -/*! * Base class for all arguments involving string value. */ struct string_arg @@ -711,7 +538,7 @@ struct string_arg }; /*! - * Option to specify the executable name seperately + * Option to specify the executable name separately * from the args sequence. * In this case the cmd args must only contain the * options required for this executable. @@ -725,34 +552,6 @@ struct executable: string_arg }; /*! - * Option to set the current working directory - * of the spawned process. - * - * Eg: cwd{"/som/path/x"} - */ -struct cwd: string_arg -{ - template <typename T> - cwd(T&& arg): string_arg(std::forward<T>(arg)) {} -}; - -/*! - * Option to specify environment variables required by - * the spawned process. - * - * Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }} - */ -struct environment -{ - environment(env_map_t&& env): - env_(std::move(env)) {} - explicit environment(const env_map_t& env): - env_(env) {} - env_map_t env_; -}; - - -/*! * Used for redirecting input/output/error */ enum IOTYPE { @@ -852,7 +651,7 @@ struct error wr_ch_ = fd; } explicit error(IOTYPE typ) { - assert ((typ == PIPE || typ == STDOUT) && "STDERR not aloowed"); + assert ((typ == PIPE || typ == STDOUT) && "STDERR not allowed"); if (typ == PIPE) { #ifndef __USING_WINDOWS__ std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); @@ -868,44 +667,6 @@ struct error int wr_ch_ = -1; }; -// Impoverished, meager, needy, truly needy -// version of type erasure to store function pointers -// needed to provide the functionality of preexec_func -// ATTN: Can be used only to execute functions with no -// arguments and returning void. -// Could have used more efficient methods, ofcourse, but -// that wont yield me the consistent syntax which I am -// aiming for. If you know, then please do let me know. - -class preexec_func -{ -public: - preexec_func() {} - - template <typename Func> - explicit preexec_func(Func f): holder_(new FuncHolder<Func>(std::move(f))) - {} - - void operator()() { - (*holder_)(); - } - -private: - struct HolderBase { - virtual void operator()() const = 0; - virtual ~HolderBase(){}; - }; - template <typename T> - struct FuncHolder: HolderBase { - FuncHolder(T func): func_(std::move(func)) {} - void operator()() const override { func_(); } - // The function pointer/reference - T func_; - }; - - std::unique_ptr<HolderBase> holder_ = nullptr; -}; - // ~~~~ End Popen Args ~~~~ @@ -967,8 +728,8 @@ namespace detail { // Metaprogram for searching a type within // a variadic parameter pack // This is particularly required to do a compile time -// checking of the arguments provided to 'check_ouput' function -// wherein the user is not expected to provide an 'ouput' option. +// checking of the arguments provided to 'check_output' function +// wherein the user is not expected to provide an 'output' option. template <typename... T> struct param_pack{}; @@ -995,7 +756,7 @@ struct has_type<F, param_pack<H,T...>> { /*! * A helper class to Popen class for setting * options as provided in the Popen constructor - * or in check_ouput arguments. + * or in check_output arguments. * This design allows us to _not_ have any fixed position * to any arguments and specify them in a way similar to what * can be done in python. @@ -1005,17 +766,9 @@ struct ArgumentDeducer ArgumentDeducer(Popen* p): popen_(p) {} void set_option(executable&& exe); - void set_option(cwd&& cwdir); - void set_option(bufsize&& bsiz); - void set_option(environment&& env); - void set_option(defer_spawn&& defer); - void set_option(shell&& sh); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); - void set_option(close_fds&& cfds); - void set_option(preexec_func&& prefunc); - void set_option(session_leader&& sleader); private: Popen* popen_ = nullptr; @@ -1166,9 +919,6 @@ public:// Yes they are public HANDLE g_hChildStd_ERR_Wr = nullptr; #endif - // Buffer size for the IO streams - int bufsiz_ = 0; - // Pipes for communicating with child // Emulates stdin @@ -1198,9 +948,9 @@ private: * interface to the client. * * API's provided by the class: - * 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....) + * 1. Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....) + * 2. Popen("cmd arg1"m output{..}, error{..}, ....) * Command provided in a single string. * 3. wait() - Wait for the child to exit. * 4. retcode() - The return code of the exited child. @@ -1211,13 +961,11 @@ private: * 9. communicate(...) - Get the output/error from the child and close the channels * from the parent side. *10. input() - Get the input channel/File pointer. Can be used for - * cutomizing the way of sending input to child. + * customizing the way of sending input to child. *11. output() - Get the output channel/File pointer. Usually used in case of redirection. See piping examples. - *12. error() - Get the error channel/File poiner. Usually used + *12. error() - Get the error channel/File pointer. Usually used in case of redirection. - *13. start_process() - Start the child process. Only to be used when - * `defer_spawn` option was provided in Popen constructor. */ class Popen { @@ -1235,7 +983,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template <typename... Args> @@ -1247,7 +995,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template <typename... Args> @@ -1258,7 +1006,7 @@ public: // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } /* @@ -1270,8 +1018,6 @@ public: } */ - void start_process() noexcept(false); - int pid() const noexcept { return child_pid_; } int retcode() const noexcept { return retcode_; } @@ -1345,20 +1091,11 @@ private: std::future<void> cleanup_future_; #endif - bool defer_process_start_ = false; - bool close_fds_ = false; - bool has_preexec_fn_ = false; - bool shell_ = false; - bool session_leader_ = false; - std::string exe_name_; - std::string cwd_; - env_map_t env_; - preexec_func preexec_fn_; // Command in string format std::string args_; - // Comamnd provided as sequence + // Command provided as sequence std::vector<std::string> vargs_; std::vector<char*> cargv_; @@ -1389,20 +1126,6 @@ inline void Popen::populate_c_argv() cargv_.push_back(nullptr); } -inline void Popen::start_process() noexcept(false) -{ - // The process was started/tried to be started - // in the constructor itself. - // For explicitly calling this API to start the - // process, 'defer_spawn' argument must be set to - // true in the constructor. - if (!defer_process_start_) { - assert (0); - return; - } - execute_process(); -} - inline int Popen::wait() noexcept(false) { #ifdef __USING_WINDOWS__ @@ -1481,8 +1204,7 @@ inline void Popen::kill(int sig_num) throw OSError("TerminateProcess", 0); } #else - if (session_leader_) killpg(child_pid_, sig_num); - else ::kill(child_pid_, sig_num); + ::kill(child_pid_, sig_num); #endif } @@ -1490,17 +1212,6 @@ inline void Popen::kill(int sig_num) inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ - if (this->shell_) { - throw OSError("shell not currently supported on windows", 0); - } - - void* environment_string_table_ptr = nullptr; - env_vector_t environment_string_vector; - if(this->env_.size()){ - environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_); - environment_string_table_ptr = (void*)environment_string_vector.data(); - } - if (exe_name_.length()) { this->vargs_.insert(this->vargs_.begin(), this->exe_name_); this->populate_c_argv(); @@ -1547,7 +1258,7 @@ inline void Popen::execute_process() noexcept(false) NULL, // primary thread security attributes TRUE, // handles are inherited creation_flags, // creation flags - environment_string_table_ptr, // use provided environment + NULL, // use parent's environment NULL, // use parent's current directory &siStartInfo, // STARTUPINFOW pointer &piProcInfo); // receives PROCESS_INFORMATION @@ -1586,14 +1297,6 @@ inline void Popen::execute_process() noexcept(false) int err_rd_pipe, err_wr_pipe; std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); - if (shell_) { - auto new_cmd = util::join(vargs_); - vargs_.clear(); - vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"}); - vargs_.push_back(new_cmd); - populate_c_argv(); - } - if (exe_name_.length()) { vargs_.insert(vargs_.begin(), exe_name_); populate_c_argv(); @@ -1660,30 +1363,6 @@ namespace detail { popen_->exe_name_ = std::move(exe.arg_value); } - inline void ArgumentDeducer::set_option(cwd&& cwdir) { - popen_->cwd_ = std::move(cwdir.arg_value); - } - - inline void ArgumentDeducer::set_option(bufsize&& bsiz) { - popen_->stream_.bufsiz_ = bsiz.bufsiz; - } - - inline void ArgumentDeducer::set_option(environment&& env) { - popen_->env_ = std::move(env.env_); - } - - inline void ArgumentDeducer::set_option(defer_spawn&& defer) { - popen_->defer_process_start_ = defer.defer; - } - - inline void ArgumentDeducer::set_option(shell&& sh) { - popen_->shell_ = sh.shell_; - } - - inline void ArgumentDeducer::set_option(session_leader&& sleader) { - popen_->session_leader_ = sleader.leader_; - } - inline void ArgumentDeducer::set_option(input&& inp) { if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; @@ -1706,15 +1385,6 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } - inline void ArgumentDeducer::set_option(close_fds&& cfds) { - popen_->close_fds_ = cfds.close_all; - } - - inline void ArgumentDeducer::set_option(preexec_func&& prefunc) { - popen_->preexec_fn_ = std::move(prefunc); - popen_->has_preexec_fn_ = true; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1761,41 +1431,8 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); - // Close all the inherited fd's except the error write pipe - if (parent_->close_fds_) { - int max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) throw OSError("sysconf failed", errno); - - for (int i = 3; i < max_fd; i++) { - if (i == err_wr_pipe_) continue; - close(i); - } - } - - // Change the working directory if provided - if (parent_->cwd_.length()) { - sys_ret = chdir(parent_->cwd_.c_str()); - if (sys_ret == -1) throw OSError("chdir failed", errno); - } - - if (parent_->has_preexec_fn_) { - parent_->preexec_fn_(); - } - - if (parent_->session_leader_) { - sys_ret = setsid(); - if (sys_ret == -1) throw OSError("setsid failed", errno); - } - // Replace the current image with the executable - if (parent_->env_.size()) { - for (auto& kv : parent_->env_) { - setenv(kv.first.c_str(), kv.second.c_str(), 1); - } - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } else { - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); if (sys_ret == -1) throw OSError("execve failed", errno); @@ -1838,16 +1475,7 @@ namespace detail { for (auto& h : handles) { if (h == nullptr) continue; - switch (bufsiz_) { - case 0: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - case 1: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - default: - setvbuf(h, nullptr, _IOFBF, bufsiz_); - }; + setvbuf(h, nullptr, _IONBF, BUFSIZ); } #endif } @@ -1983,4 +1611,4 @@ namespace detail { } -#endif // SUBPROCESS_HPP +#endif // BITCOIN_UTIL_SUBPROCESS_H diff --git a/src/util/time.cpp b/src/util/time.cpp index 456662bd84..f08eb5300a 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -16,11 +16,11 @@ void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } -static std::atomic<int64_t> nMockTime(0); //!< For testing +static std::atomic<std::chrono::seconds> g_mock_time{}; //!< For testing NodeClock::time_point NodeClock::now() noexcept { - const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)}; + const auto mocktime{g_mock_time.load(std::memory_order_relaxed)}; const auto ret{ mocktime.count() ? mocktime : @@ -29,20 +29,16 @@ NodeClock::time_point NodeClock::now() noexcept return time_point{ret}; }; -void SetMockTime(int64_t nMockTimeIn) -{ - Assert(nMockTimeIn >= 0); - nMockTime.store(nMockTimeIn, std::memory_order_relaxed); -} - +void SetMockTime(int64_t nMockTimeIn) { SetMockTime(std::chrono::seconds{nMockTimeIn}); } void SetMockTime(std::chrono::seconds mock_time_in) { - nMockTime.store(mock_time_in.count(), std::memory_order_relaxed); + Assert(mock_time_in >= 0s); + g_mock_time.store(mock_time_in, std::memory_order_relaxed); } std::chrono::seconds GetMockTime() { - return std::chrono::seconds(nMockTime.load(std::memory_order_relaxed)); + return g_mock_time.load(std::memory_order_relaxed); } int64_t GetTime() { return GetTime<std::chrono::seconds>().count(); } diff --git a/src/validation.cpp b/src/validation.cpp index 048c97529a..f57851b4f7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -946,8 +946,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) maybe_rbf_limits.descendant_size_vbytes += conflict->GetSizeWithDescendants(); } - auto ancestors{m_pool.CalculateMemPoolAncestors(*entry, maybe_rbf_limits)}; - if (!ancestors) { + if (auto ancestors{m_pool.CalculateMemPoolAncestors(*entry, maybe_rbf_limits)}) { + ws.m_ancestors = std::move(*ancestors); + } else { // If CalculateMemPoolAncestors fails second time, we want the original error string. // Contracting/payment channels CPFP carve-out: // If the new transaction is relatively small (up to 40k weight) @@ -970,11 +971,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); } - ancestors = m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits); - if (!ancestors) return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); + if (auto ancestors_retry{m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits)}) { + ws.m_ancestors = std::move(*ancestors_retry); + } else { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); + } } - ws.m_ancestors = *ancestors; // Even though just checking direct mempool parents for inheritance would be sufficient, we // check using the full ancestor set here because it's more convenient to use what we have // already calculated. @@ -6142,13 +6145,14 @@ bool ChainstateManager::DeleteSnapshotChainstate() Assert(m_snapshot_chainstate); Assert(m_ibd_chainstate); - fs::path snapshot_datadir = GetSnapshotCoinsDBPath(*m_snapshot_chainstate); + fs::path snapshot_datadir = Assert(node::FindSnapshotChainstateDir(m_options.datadir)).value(); if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) { LogPrintf("Deletion of %s failed. Please remove it manually to continue reindexing.\n", fs::PathToString(snapshot_datadir)); return false; } m_active_chainstate = m_ibd_chainstate.get(); + m_active_chainstate->m_mempool = m_snapshot_chainstate->m_mempool; m_snapshot_chainstate.reset(); return true; } diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index a71f8f9fbc..b5703fa54a 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -9,9 +9,11 @@ #include <wallet/external_signer_scriptpubkeyman.h> #include <iostream> +#include <key_io.h> #include <memory> #include <stdexcept> #include <string> +#include <univalue.h> #include <utility> #include <vector> @@ -51,15 +53,26 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { return signers[0]; } -bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const +util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const { // TODO: avoid the need to infer a descriptor from inside a descriptor wallet + const CScript& scriptPubKey = GetScriptForDestination(dest); auto provider = GetSolvingProvider(scriptPubKey); auto descriptor = InferDescriptor(scriptPubKey, *provider); - signer.DisplayAddress(descriptor->ToString()); - // TODO inspect result - return true; + const UniValue& result = signer.DisplayAddress(descriptor->ToString()); + + const UniValue& error = result.find_value("error"); + if (error.isStr()) return util::Error{strprintf(_("Signer returned error: %s"), error.getValStr())}; + + const UniValue& ret_address = result.find_value("address"); + if (!ret_address.isStr()) return util::Error{_("Signer did not echo address")}; + + if (ret_address.getValStr() != EncodeDestination(dest)) { + return util::Error{strprintf(_("Signer echoed unexpected address %s"), ret_address.getValStr())}; + } + + return util::Result<void>(); } // If sign is true, transaction must previously have been filled diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index c052ce6129..44286456b6 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -9,6 +9,8 @@ #include <memory> +struct bilingual_str; + namespace wallet { class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan { @@ -27,7 +29,11 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan static ExternalSigner GetExternalSigner(); - bool DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const; + /** + * Display address on the device and verify that the returned value matches. + * @returns nothing or an error message + */ + util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index d33e6f3873..0c1cae7253 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -247,7 +247,7 @@ public: return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id) : m_wallet->SetAddressReceiveRequest(batch, dest, id, value); } - bool displayAddress(const CTxDestination& dest) override + util::Result<void> displayAddress(const CTxDestination& dest) override { LOCK(m_wallet->cs_wallet); return m_wallet->DisplayAddress(dest); @@ -286,7 +286,7 @@ public: if (!res) return util::Error{util::ErrorString(res)}; const auto& txr = *res; fee = txr.fee; - change_pos = txr.change_pos ? *txr.change_pos : -1; + change_pos = txr.change_pos ? int(*txr.change_pos) : -1; return txr.tx; } diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7f068c05ef..bed9ec029a 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -789,9 +789,8 @@ RPCHelpMan walletdisplayaddress() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); } - if (!pwallet->DisplayAddress(dest)) { - throw JSONRPCError(RPC_MISC_ERROR, "Failed to display address"); - } + util::Result<void> res = pwallet->DisplayAddress(dest); + if (!res) throw JSONRPCError(RPC_MISC_ERROR, util::ErrorString(res).original); UniValue result(UniValue::VOBJ); result.pushKV("address", request.params[0].get_str()); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 0cb0891141..b6c7396f4b 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -194,15 +194,12 @@ RPCHelpMan getbalance() LOCK(pwallet->cs_wallet); - const auto dummy_value{self.MaybeArg<std::string>(0)}; + const auto dummy_value{self.MaybeArg<std::string>("dummy")}; if (dummy_value && *dummy_value != "*") { throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\"."); } - int min_depth = 0; - if (!request.params[1].isNull()) { - min_depth = request.params[1].getInt<int>(); - } + const auto min_depth{self.Arg<int>("minconf")}; bool include_watchonly = ParseIncludeWatchonly(request.params[2], *pwallet); diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 06ec7db1bc..1252843e9d 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -11,6 +11,7 @@ #include <wallet/context.h> #include <wallet/wallet.h> +#include <string_view> #include <univalue.h> #include <boost/date_time/posix_time/posix_time.hpp> @@ -61,9 +62,9 @@ bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wal bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { - if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { + if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = UrlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4575881d96..2c1ab8d44a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -27,7 +27,6 @@ #include <unordered_map> enum class OutputType; -struct bilingual_str; namespace wallet { struct MigrationData; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 5d23ebd35a..9a7e166e68 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -683,11 +683,11 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co // Vector of results. We will choose the best one based on waste. std::vector<SelectionResult> results; std::vector<util::Result<SelectionResult>> errors; - auto append_error = [&] (const util::Result<SelectionResult>& result) { + auto append_error = [&] (util::Result<SelectionResult>&& result) { // If any specific error message appears here, then something different from a simple "no selection found" happened. // Let's save it, so it can be retrieved to the user if no other selection algorithm succeeded. if (HasErrorMsg(result)) { - errors.emplace_back(result); + errors.emplace_back(std::move(result)); } }; @@ -698,7 +698,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co if (!coin_selection_params.m_subtract_fee_outputs) { if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) { results.push_back(*bnb_result); - } else append_error(bnb_result); + } else append_error(std::move(bnb_result)); } // As Knapsack and SRD can create change, also deduce change weight. @@ -707,25 +707,25 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) { results.push_back(*knapsack_result); - } else append_error(knapsack_result); + } else append_error(std::move(knapsack_result)); if (coin_selection_params.m_effective_feerate > CFeeRate{3 * coin_selection_params.m_long_term_feerate}) { // Minimize input set for feerates of at least 3×LTFRE (default: 30 ṩ/vB+) if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_inputs_weight)}) { cg_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*cg_result); } else { - append_error(cg_result); + append_error(std::move(cg_result)); } } if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) { results.push_back(*srd_result); - } else append_error(srd_result); + } else append_error(std::move(srd_result)); if (results.empty()) { // No solution found, retrieve the first explicit error (if any). // future: add 'severity level' to errors so the worst one can be retrieved instead of the first one. - return errors.empty() ? util::Error() : errors.front(); + return errors.empty() ? util::Error() : std::move(errors.front()); } // If the chosen input set has unconfirmed inputs, check for synergies from overlapping ancestry @@ -818,7 +818,7 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the // transaction at a target feerate. If an attempt fails, more attempts may be made using a more // permissive CoinEligibilityFilter. - util::Result<SelectionResult> res = [&] { + { // Place coins eligibility filters on a scope increasing order. std::vector<SelectionFilter> ordered_filters{ // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six @@ -866,9 +866,9 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin if (CAmount total_amount = available_coins.GetTotalAmount() - total_discarded < value_to_select) { // Special case, too-long-mempool cluster. if (total_amount + total_unconf_long_chain > value_to_select) { - return util::Result<SelectionResult>({_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")}); + return util::Error{_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")}; } - return util::Result<SelectionResult>(util::Error()); // General "Insufficient Funds" + return util::Error{}; // General "Insufficient Funds" } // Walk-through the filters until the solution gets found. @@ -885,19 +885,17 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin // If any specific error message appears here, then something particularly wrong might have happened. // Save the error and continue the selection process. So if no solutions gets found, we can return // the detailed error to the upper layers. - if (HasErrorMsg(res)) res_detailed_errors.emplace_back(res); + if (HasErrorMsg(res)) res_detailed_errors.emplace_back(std::move(res)); } } // Return right away if we have a detailed error - if (!res_detailed_errors.empty()) return res_detailed_errors.front(); + if (!res_detailed_errors.empty()) return std::move(res_detailed_errors.front()); // General "Insufficient Funds" - return util::Result<SelectionResult>(util::Error()); - }(); - - return res; + return util::Error{}; + } } static bool IsCurrentForAntiFeeSniping(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 297432de9e..331590df7f 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -291,7 +291,10 @@ FUZZ_TARGET(coinselection) } std::vector<COutput> utxos; - std::vector<util::Result<SelectionResult>> results{result_srd, result_knapsack, result_bnb}; + std::vector<util::Result<SelectionResult>> results; + results.emplace_back(std::move(result_srd)); + results.emplace_back(std::move(result_knapsack)); + results.emplace_back(std::move(result_bnb)); CAmount new_total_balance{CreateCoins(fuzzed_data_provider, utxos, coin_params, next_locktime)}; if (new_total_balance > 0) { std::set<std::shared_ptr<COutput>> new_utxo_pool; diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 9a515828fe..792079e6c6 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -106,13 +106,11 @@ struct FuzzedWallet { CTxDestination GetDestination(FuzzedDataProvider& fuzzed_data_provider) { auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - util::Result<CTxDestination> op_dest{util::Error{}}; if (fuzzed_data_provider.ConsumeBool()) { - op_dest = wallet->GetNewDestination(type, ""); + return *Assert(wallet->GetNewDestination(type, "")); } else { - op_dest = wallet->GetNewChangeDestination(type); + return *Assert(wallet->GetNewChangeDestination(type)); } - return *Assert(op_dest); } CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { return GetScriptForDestination(GetDestination(fuzzed_data_provider)); } void FundTx(FuzzedDataProvider& fuzzed_data_provider, CMutableTransaction tx) diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 3509bc116f..963c0f838b 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -97,13 +97,11 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup) // so that the recipient's amount is no longer equal to the user's selected target of 299 BTC. // First case, use 'subtract_fee_from_outputs=true' - util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); - BOOST_CHECK(!res_tx.has_value()); + BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control)); // Second case, don't use 'subtract_fee_from_outputs'. recipients[0].fSubtractFeeFromAmount = false; - res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); - BOOST_CHECK(!res_tx.has_value()); + BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 96c4397504..8f4171eb15 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2667,7 +2667,7 @@ void ReserveDestination::ReturnDestination() address = CNoDestination(); } -bool CWallet::DisplayAddress(const CTxDestination& dest) +util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest) { CScript scriptPubKey = GetScriptForDestination(dest); for (const auto& spk_man : GetScriptPubKeyMans(scriptPubKey)) { @@ -2676,9 +2676,9 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) continue; } ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); - return signer_spk_man->DisplayAddress(scriptPubKey, signer); + return signer_spk_man->DisplayAddress(dest, signer); } - return false; + return util::Error{_("There is no ScriptPubKeyManager for this address")}; } bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b49b5a7d0d..6a998fa398 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,8 +537,8 @@ public: bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Display address on an external signer. Returns false if external signer support is not compiled */ - bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Display address on an external signer. */ + util::Result<void> DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index d10db046f5..536e471053 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -8,6 +8,7 @@ #include <kernel/chain.h> #include <kernel/mempool_entry.h> #include <logging.h> +#include <netbase.h> #include <primitives/block.h> #include <primitives/transaction.h> #include <validationinterface.h> @@ -57,7 +58,12 @@ std::unique_ptr<CZMQNotificationInterface> CZMQNotificationInterface::Create(std { std::string arg("-zmq" + entry.first); const auto& factory = entry.second; - for (const std::string& address : gArgs.GetArgs(arg)) { + for (std::string& address : gArgs.GetArgs(arg)) { + // libzmq uses prefix "ipc://" for UNIX domain sockets + if (address.substr(0, ADDR_PREFIX_UNIX.length()) == ADDR_PREFIX_UNIX) { + address.replace(0, ADDR_PREFIX_UNIX.length(), ADDR_PREFIX_IPC); + } + std::unique_ptr<CZMQAbstractNotifier> notifier = factory(); notifier->SetType(entry.first); notifier->SetAddress(address); diff --git a/src/zmq/zmqutil.h b/src/zmq/zmqutil.h index 334b51aa91..bec48c0a56 100644 --- a/src/zmq/zmqutil.h +++ b/src/zmq/zmqutil.h @@ -9,4 +9,7 @@ void zmqError(const std::string& str); +/** Prefix for unix domain socket addresses (which are local filesystem paths) */ +const std::string ADDR_PREFIX_IPC = "ipc://"; // used by libzmq, example "ipc:///root/path/to/file" + #endif // BITCOIN_ZMQ_ZMQUTIL_H diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 3e882f47b8..19cbbcffdb 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -11,9 +11,6 @@ The assumeutxo value generated and used here is committed to in ## Possible test improvements -- TODO: test what happens with -reindex and -reindex-chainstate before the - snapshot is validated, and make sure it's deleted successfully. - Interesting test cases could be loading an assumeutxo snapshot file with: - TODO: Valid hash but invalid snapshot file (bad coin height or @@ -379,6 +376,17 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT) assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT) + for reindex_arg in ['-reindex=1', '-reindex-chainstate=1']: + self.log.info(f"Check that restarting with {reindex_arg} will delete the snapshot chainstate") + self.restart_node(2, extra_args=[reindex_arg, *self.extra_args[2]]) + assert_equal(1, len(n2.getchainstates()["chainstates"])) + for i in range(1, 300): + block = n0.getblock(n0.getblockhash(i), 0) + n2.submitheader(block) + loaded = n2.loadtxoutset(dump_output['path']) + assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT) + assert_equal(loaded['base_height'], SNAPSHOT_BASE_HEIGHT) + normal, snapshot = n2.getchainstates()['chainstates'] assert_equal(normal['blocks'], START_HEIGHT) assert_equal(normal.get('snapshot_blockhash'), None) diff --git a/test/functional/feature_framework_unit_tests.py b/test/functional/feature_framework_unit_tests.py new file mode 100755 index 0000000000..c9754e083c --- /dev/null +++ b/test/functional/feature_framework_unit_tests.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Framework unit tests + +Unit tests for the test framework. +""" + +import sys +import unittest + +from test_framework.test_framework import TEST_EXIT_PASSED, TEST_EXIT_FAILED + +# List of framework modules containing unit tests. Should be kept in sync with +# the output of `git grep unittest.TestCase ./test/functional/test_framework` +TEST_FRAMEWORK_MODULES = [ + "address", + "crypto.bip324_cipher", + "blocktools", + "crypto.chacha20", + "crypto.ellswift", + "key", + "messages", + "crypto.muhash", + "crypto.poly1305", + "crypto.ripemd160", + "script", + "segwit_addr", + "wallet_util", +] + + +def run_unit_tests(): + test_framework_tests = unittest.TestSuite() + for module in TEST_FRAMEWORK_MODULES: + test_framework_tests.addTest( + unittest.TestLoader().loadTestsFromName(f"test_framework.{module}") + ) + result = unittest.TextTestRunner(stream=sys.stdout, verbosity=1, failfast=True).run( + test_framework_tests + ) + if not result.wasSuccessful(): + sys.exit(TEST_EXIT_FAILED) + sys.exit(TEST_EXIT_PASSED) + + +if __name__ == "__main__": + run_unit_tests() + diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 3c3ff1e4a0..9f6f8919de 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -3,7 +3,9 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the ZMQ notification interface.""" +import os import struct +import tempfile from time import sleep from io import BytesIO @@ -30,7 +32,7 @@ from test_framework.util import ( from test_framework.wallet import ( MiniWallet, ) -from test_framework.netutil import test_ipv6_local +from test_framework.netutil import test_ipv6_local, test_unix_socket # Test may be skipped and not have zmq installed @@ -119,6 +121,10 @@ class ZMQTest (BitcoinTestFramework): self.ctx = zmq.Context() try: self.test_basic() + if test_unix_socket(): + self.test_basic(unix=True) + else: + self.log.info("Skipping ipc test, because UNIX sockets are not supported.") self.test_sequence() self.test_mempool_sync() self.test_reorg() @@ -139,7 +145,7 @@ class ZMQTest (BitcoinTestFramework): socket.setsockopt(zmq.IPV6, 1) subscribers.append(ZMQSubscriber(socket, topic.encode())) - self.restart_node(0, [f"-zmqpub{topic}={address}" for topic, address in services]) + self.restart_node(0, [f"-zmqpub{topic}={address.replace('ipc://', 'unix:')}" for topic, address in services]) for i, sub in enumerate(subscribers): sub.socket.connect(services[i][1]) @@ -176,12 +182,19 @@ class ZMQTest (BitcoinTestFramework): return subscribers - def test_basic(self): + def test_basic(self, unix = False): + self.log.info(f"Running basic test with {'ipc' if unix else 'tcp'} protocol") # Invalid zmq arguments don't take down the node, see #17185. self.restart_node(0, ["-zmqpubrawtx=foo", "-zmqpubhashtx=bar"]) address = f"tcp://127.0.0.1:{self.zmq_port_base}" + + if unix: + # Use the shortest temp path possible since paths may have as little as 92-char limit + socket_path = tempfile.NamedTemporaryFile().name + address = f"ipc://{socket_path}" + subs = self.setup_zmq_test([(topic, address) for topic in ["hashblock", "hashtx", "rawblock", "rawtx"]]) hashblock = subs[0] @@ -247,6 +260,8 @@ class ZMQTest (BitcoinTestFramework): ]) assert_equal(self.nodes[1].getzmqnotifications(), []) + if unix: + os.unlink(socket_path) def test_reorg(self): diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py index 1b55cd0a0d..8285b82c19 100755 --- a/test/functional/mempool_accept_v3.py +++ b/test/functional/mempool_accept_v3.py @@ -533,10 +533,10 @@ class MempoolAcceptV3(BitcoinTestFramework): tx_unrelated_replacee = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_unrelated_conflict) assert tx_unrelated_replacee["txid"] in node.getrawmempool() - fee_to_beat_child2 = int(tx_v3_child_2["fee"] * COIN) + fee_to_beat = max(int(tx_v3_child_2["fee"] * COIN), int(tx_unrelated_replacee["fee"]*COIN)) tx_v3_child_3 = self.wallet.create_self_transfer_multi( - utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat_child2*5, version=3 + utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat*2, version=3 ) node.sendrawtransaction(tx_v3_child_3["hex"]) self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_3["txid"]]) diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index da796d3f70..5f2dde8eac 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -308,7 +308,7 @@ class MiningTest(BitcoinTestFramework): # Should ask for the block from a p2p node, if they announce the header as well: peer = node.add_p2p_connection(P2PDataStore()) - peer.wait_for_getheaders(timeout=5) # Drop the first getheaders + peer.wait_for_getheaders(timeout=5, block_hash=block.hashPrevBlock) peer.send_blocks_and_test(blocks=[block], node=node) # Must be active now: assert chain_tip(block.hash, status='active', branchlen=0) in node.getchaintips() diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index 5f4fad6380..23d163aac3 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -25,35 +25,36 @@ def getdescriptors(args): sys.stdout.write(json.dumps({ "receive": [ - "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j", - "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/0/*))#r0grqw5x", - "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/0/*)#x30uthjs", - "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/0/*)#sng9rd4t" + "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/0/*)#aqllu46s", + "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/0/*))#5dh56mgg", + "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/0/*)#h62dxaej", + "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/0/*)#pcd5w87f" ], "internal": [ - "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2", - "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/1/*))#kwx4c3pe", - "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/1/*)#h92akzzg", - "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/1/*)#p8dy7c9n" + "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/1/*)#v567pq2g", + "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/1/*))#pvezzyah", + "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/1/*)#xw0vmgf2", + "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/1/*)#svg4njw3" ] })) def displayaddress(args): - # Several descriptor formats are acceptable, so allowing for potential - # changes to InferDescriptor: if args.fingerprint != "00000001": return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint})) - expected_desc = [ - "wpkh([00000001/84'/1'/0'/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#0yneg42r", - "tr([00000001/86'/1'/0'/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#4vdj9jqk", - ] + expected_desc = { + "wpkh([00000001/84h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#3te6hhy7": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g", + "sh(wpkh([00000001/49h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7))#kz9y5w82": "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp", + "pkh([00000001/44h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#q3pqd8wh": "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9", + "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m": "tb1phw4cgpt6cd30kz9k4wkpwm872cdvhss29jga2xpmftelhqll62mscq0k4g", + "wpkh([00000001/84h/1h/0h/0/1]03a20a46308be0b8ded6dff0a22b10b4245c587ccf23f3b4a303885be3a524f172)#aqpjv5xr": "wrong_address", + } if args.desc not in expected_desc: return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc})) - return sys.stdout.write(json.dumps({"address": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g"})) + return sys.stdout.write(json.dumps({"address": expected_desc[args.desc]})) def signtx(args): if args.fingerprint != "00000001": diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py index f9a8c44be2..ea114e7d70 100755 --- a/test/functional/p2p_addrv2_relay.py +++ b/test/functional/p2p_addrv2_relay.py @@ -11,6 +11,7 @@ import time from test_framework.messages import ( CAddress, msg_addrv2, + msg_sendaddrv2, ) from test_framework.p2p import ( P2PInterface, @@ -75,6 +76,12 @@ class AddrTest(BitcoinTestFramework): self.extra_args = [["-whitelist=addr@127.0.0.1"]] def run_test(self): + self.log.info('Check disconnection when sending sendaddrv2 after verack') + conn = self.nodes[0].add_p2p_connection(P2PInterface()) + with self.nodes[0].assert_debug_log(['sendaddrv2 received after verack from peer=0; disconnecting']): + conn.send_message(msg_sendaddrv2()) + conn.wait_for_disconnect() + self.log.info('Create connection that sends addrv2 messages') addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) msg = msg_addrv2() @@ -89,8 +96,8 @@ class AddrTest(BitcoinTestFramework): msg.addrs = ADDRS msg_size = calc_addrv2_msg_size(ADDRS) with self.nodes[0].assert_debug_log([ - f'received: addrv2 ({msg_size} bytes) peer=0', - f'sending addrv2 ({msg_size} bytes) peer=1', + f'received: addrv2 ({msg_size} bytes) peer=1', + f'sending addrv2 ({msg_size} bytes) peer=2', ]): addr_source.send_and_ping(msg) self.nodes[0].setmocktime(int(time.time()) + 30 * 60) diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 0950579580..9e314db110 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -387,7 +387,7 @@ class CompactBlocksTest(BitcoinTestFramework): if announce == "inv": test_node.send_message(msg_inv([CInv(MSG_BLOCK, block.sha256)])) - test_node.wait_until(lambda: "getheaders" in test_node.last_message, timeout=30) + test_node.wait_for_getheaders(timeout=30) test_node.send_header_for_blocks([block]) else: test_node.send_header_for_blocks([block]) diff --git a/test/functional/p2p_compactblocks_hb.py b/test/functional/p2p_compactblocks_hb.py index c985a1f98d..023b33ff6d 100755 --- a/test/functional/p2p_compactblocks_hb.py +++ b/test/functional/p2p_compactblocks_hb.py @@ -32,10 +32,15 @@ class CompactBlocksConnectionTest(BitcoinTestFramework): self.connect_nodes(peer, 0) self.generate(self.nodes[0], 1) self.disconnect_nodes(peer, 0) - status_to = [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] - status_from = [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] - assert_equal(status_to, status_from) - return status_to + + def status_to(): + return [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] + + def status_from(): + return [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] + + self.wait_until(lambda: status_to() == status_from()) + return status_to() def run_test(self): self.log.info("Testing reserved high-bandwidth mode slot for outbound peer...") diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index c389ff732f..678b006886 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -77,6 +77,7 @@ class DisconnectBanTest(BitcoinTestFramework): self.nodes[1].setmocktime(old_time) self.nodes[1].setban("127.0.0.0/32", "add") self.nodes[1].setban("127.0.0.0/24", "add") + self.nodes[1].setban("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", "add") self.nodes[1].setban("192.168.0.1", "add", 1) # ban for 1 seconds self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[1].listbanned() @@ -85,13 +86,13 @@ class DisconnectBanTest(BitcoinTestFramework): self.log.info("setban: test banning with absolute timestamp") self.nodes[1].setban("192.168.0.2", "add", old_time + 120, True) - # Move time forward by 3 seconds so the third ban has expired + # Move time forward by 3 seconds so the fourth ban has expired self.nodes[1].setmocktime(old_time + 3) - assert_equal(len(self.nodes[1].listbanned()), 4) + assert_equal(len(self.nodes[1].listbanned()), 5) self.log.info("Test ban_duration and time_remaining") for ban in self.nodes[1].listbanned(): - if ban["address"] in ["127.0.0.0/32", "127.0.0.0/24"]: + if ban["address"] in ["127.0.0.0/32", "127.0.0.0/24", "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"]: assert_equal(ban["ban_duration"], 86400) assert_equal(ban["time_remaining"], 86397) elif ban["address"] == "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19": @@ -108,6 +109,7 @@ class DisconnectBanTest(BitcoinTestFramework): assert_equal("127.0.0.0/32", listAfterShutdown[1]['address']) assert_equal("192.168.0.2/32", listAfterShutdown[2]['address']) assert_equal("/19" in listAfterShutdown[3]['address'], True) + assert_equal("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", listAfterShutdown[4]['address']) # Clear ban lists self.nodes[1].clearbanned() diff --git a/test/functional/p2p_handshake.py b/test/functional/p2p_handshake.py index f0b62e291d..dd19fe9333 100755 --- a/test/functional/p2p_handshake.py +++ b/test/functional/p2p_handshake.py @@ -41,6 +41,7 @@ class P2PHandshakeTest(BitcoinTestFramework): peer.sync_with_ping() peer.peer_disconnect() peer.wait_for_disconnect() + self.wait_until(lambda: len(node.getpeerinfo()) == 0) def test_desirable_service_flags(self, node, service_flag_tests, desirable_service_flags, expect_disconnect): """Check that connecting to a peer either fails or succeeds depending on its offered diff --git a/test/functional/p2p_initial_headers_sync.py b/test/functional/p2p_initial_headers_sync.py index e67c384da7..bc6e0fb355 100755 --- a/test/functional/p2p_initial_headers_sync.py +++ b/test/functional/p2p_initial_headers_sync.py @@ -38,9 +38,10 @@ class HeadersSyncTest(BitcoinTestFramework): def run_test(self): self.log.info("Adding a peer to node0") peer1 = self.nodes[0].add_p2p_connection(P2PInterface()) + best_block_hash = int(self.nodes[0].getbestblockhash(), 16) # Wait for peer1 to receive a getheaders - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) # An empty reply will clear the outstanding getheaders request, # allowing additional getheaders requests to be sent to this peer in # the future. @@ -60,17 +61,12 @@ class HeadersSyncTest(BitcoinTestFramework): assert "getheaders" not in peer2.last_message assert "getheaders" not in peer3.last_message - with p2p_lock: - peer1.last_message.pop("getheaders", None) - self.log.info("Have all peers announce a new block") self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) peer1.send_message(msg_headers()) # Send empty response, see above - with p2p_lock: - peer1.last_message.pop("getheaders", None) self.log.info("Check that exactly 1 of {peer2, peer3} received a getheaders in response") count = 0 @@ -80,7 +76,6 @@ class HeadersSyncTest(BitcoinTestFramework): if "getheaders" in p.last_message: count += 1 peer_receiving_getheaders = p - p.last_message.pop("getheaders", None) p.send_message(msg_headers()) # Send empty response, see above assert_equal(count, 1) @@ -89,14 +84,14 @@ class HeadersSyncTest(BitcoinTestFramework): self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Check that the remaining peer received a getheaders as well") expected_peer = peer2 if peer2 == peer_receiving_getheaders: expected_peer = peer3 - expected_peer.wait_for_getheaders() + expected_peer.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Success!") diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index af47c6d9f0..213c748eda 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -191,14 +191,13 @@ class TestP2PConn(P2PInterface): def announce_block_and_wait_for_getdata(self, block, use_header, timeout=60): with p2p_lock: self.last_message.pop("getdata", None) - self.last_message.pop("getheaders", None) msg = msg_headers() msg.headers = [CBlockHeader(block)] if use_header: self.send_message(msg) else: self.send_message(msg_inv(inv=[CInv(MSG_BLOCK, block.sha256)])) - self.wait_for_getheaders(timeout=timeout) + self.wait_for_getheaders(block_hash=block.hashPrevBlock, timeout=timeout) self.send_message(msg) self.wait_for_getdata([block.sha256], timeout=timeout) diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 508d6fe403..27a3aa8fb9 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -311,6 +311,7 @@ class SendHeadersTest(BitcoinTestFramework): # Now that we've synced headers, headers announcements should work tip = self.mine_blocks(1) + expected_hash = tip inv_node.check_last_inv_announcement(inv=[tip]) test_node.check_last_headers_announcement(headers=[tip]) @@ -334,7 +335,10 @@ class SendHeadersTest(BitcoinTestFramework): if j == 0: # Announce via inv test_node.send_block_inv(tip) - test_node.wait_for_getheaders() + if i == 0: + test_node.wait_for_getheaders(block_hash=expected_hash) + else: + assert "getheaders" not in test_node.last_message # Should have received a getheaders now test_node.send_header_for_blocks(blocks) # Test that duplicate inv's won't result in duplicate @@ -521,6 +525,7 @@ class SendHeadersTest(BitcoinTestFramework): self.log.info("Part 5: Testing handling of unconnecting headers") # First we test that receipt of an unconnecting header doesn't prevent # chain sync. + expected_hash = tip for i in range(10): self.log.debug("Part 5.{}: starting...".format(i)) test_node.last_message.pop("getdata", None) @@ -533,15 +538,14 @@ class SendHeadersTest(BitcoinTestFramework): block_time += 1 height += 1 # Send the header of the second block -> this won't connect. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[1]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) test_node.send_header_for_blocks(blocks) test_node.wait_for_getdata([x.sha256 for x in blocks]) [test_node.send_message(msg_block(x)) for x in blocks] test_node.sync_with_ping() assert_equal(int(self.nodes[0].getbestblockhash(), 16), blocks[1].sha256) + expected_hash = blocks[1].sha256 blocks = [] # Now we test that if we repeatedly don't send connecting headers, we @@ -556,13 +560,12 @@ class SendHeadersTest(BitcoinTestFramework): for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Next header will connect, should re-set our count: test_node.send_header_for_blocks([blocks[0]]) + expected_hash = blocks[0].sha256 # Remove the first two entries (blocks[1] would connect): blocks = blocks[2:] @@ -571,10 +574,8 @@ class SendHeadersTest(BitcoinTestFramework): # before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i % len(blocks)]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Eventually this stops working. test_node.send_header_for_blocks([blocks[-1]]) diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index 0e463c5072..7a50f1e605 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -5,6 +5,7 @@ """ Test transaction download behavior """ +from decimal import Decimal import time from test_framework.messages import ( @@ -14,6 +15,7 @@ from test_framework.messages import ( MSG_WTX, msg_inv, msg_notfound, + msg_tx, ) from test_framework.p2p import ( P2PInterface, @@ -22,6 +24,7 @@ from test_framework.p2p import ( from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + fill_mempool, ) from test_framework.wallet import MiniWallet @@ -54,6 +57,7 @@ MAX_GETDATA_INBOUND_WAIT = GETDATA_TX_INTERVAL + INBOUND_PEER_TX_DELAY + TXID_RE class TxDownloadTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + self.extra_args= [['-datacarriersize=100000', '-maxmempool=5', '-persistmempool=0']] * self.num_nodes def test_tx_requests(self): self.log.info("Test that we request transactions from all our peers, eventually") @@ -241,6 +245,29 @@ class TxDownloadTest(BitcoinTestFramework): self.log.info('Check that spurious notfound is ignored') self.nodes[0].p2ps[0].send_message(msg_notfound(vec=[CInv(MSG_TX, 1)])) + def test_rejects_filter_reset(self): + self.log.info('Check that rejected tx is not requested again') + node = self.nodes[0] + fill_mempool(self, node, self.wallet) + self.wallet.rescan_utxos() + mempoolminfee = node.getmempoolinfo()['mempoolminfee'] + peer = node.add_p2p_connection(TestP2PConn()) + low_fee_tx = self.wallet.create_self_transfer(fee_rate=Decimal("0.9")*mempoolminfee) + assert_equal(node.testmempoolaccept([low_fee_tx['hex']])[0]["reject-reason"], "mempool min fee not met") + peer.send_and_ping(msg_tx(low_fee_tx['tx'])) + peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(low_fee_tx['wtxid'], 16))])) + node.setmocktime(int(time.time())) + node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT) + peer.sync_with_ping() + assert_equal(peer.tx_getdata_count, 0) + + self.log.info('Check that rejection filter is cleared after new block comes in') + self.generate(self.wallet, 1, sync_fun=self.no_op) + peer.sync_with_ping() + peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(low_fee_tx['wtxid'], 16))])) + node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT) + peer.wait_for_getdata([int(low_fee_tx['wtxid'], 16)]) + def run_test(self): self.wallet = MiniWallet(self.nodes[0]) @@ -257,16 +284,22 @@ class TxDownloadTest(BitcoinTestFramework): # Run each test against new bitcoind instances, as setting mocktimes has long-term effects on when # the next trickle relay event happens. - for test in [self.test_in_flight_max, self.test_inv_block, self.test_tx_requests]: + for test, with_inbounds in [ + (self.test_in_flight_max, True), + (self.test_inv_block, True), + (self.test_tx_requests, True), + (self.test_rejects_filter_reset, False), + ]: self.stop_nodes() self.start_nodes() self.connect_nodes(1, 0) # Setup the p2p connections self.peers = [] - for node in self.nodes: - for _ in range(NUM_INBOUND): - self.peers.append(node.add_p2p_connection(TestP2PConn())) - self.log.info("Nodes are setup with {} incoming connections each".format(NUM_INBOUND)) + if with_inbounds: + for node in self.nodes: + for _ in range(NUM_INBOUND): + self.peers.append(node.add_p2p_connection(TestP2PConn())) + self.log.info("Nodes are setup with {} incoming connections each".format(NUM_INBOUND)) test() diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index e061030da3..37c42f2533 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -386,12 +386,14 @@ class RPCPackagesTest(BitcoinTestFramework): "-maxmempool=5", "-persistmempool=0", ]) + self.wallet.rescan_utxos() fill_mempool(self, node, self.wallet) minrelay = node.getmempoolinfo()["minrelaytxfee"] parent = self.wallet.create_self_transfer( fee_rate=minrelay, + confirmed_only=True, ) child = self.wallet.create_self_transfer( @@ -411,6 +413,7 @@ class RPCPackagesTest(BitcoinTestFramework): # Reset maxmempool, datacarriersize, reset dynamic mempool minimum feerate, and empty mempool. self.restart_node(0) + self.wallet.rescan_utxos() assert_equal(node.getrawmempool(), []) @@ -420,7 +423,10 @@ class RPCPackagesTest(BitcoinTestFramework): assert_equal(node.getrawmempool(), []) self.log.info("Submitpackage maxburnamount arg testing") - chained_txns_burn = self.wallet.create_self_transfer_chain(chain_length=2) + chained_txns_burn = self.wallet.create_self_transfer_chain( + chain_length=2, + utxo_to_spend=self.wallet.get_utxo(confirmed_only=True), + ) chained_burn_hex = [t["hex"] for t in chained_txns_burn] tx = tx_from_hex(chained_burn_hex[1]) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 016aa3ba11..6ee7e56886 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -16,8 +16,6 @@ from test_framework.messages import ( CTxIn, CTxOut, MAX_BIP125_RBF_SEQUENCE, - WITNESS_SCALE_FACTOR, - ser_compact_size, ) from test_framework.psbt import ( PSBT, @@ -42,6 +40,7 @@ from test_framework.util import ( find_vout_for_address, ) from test_framework.wallet_util import ( + calculate_input_weight, generate_keypair, get_generate_key, ) @@ -752,17 +751,9 @@ class PSBTTest(BitcoinTestFramework): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) low_input_weight = input_weight // 2 high_input_weight = input_weight * 2 @@ -886,7 +877,7 @@ class PSBTTest(BitcoinTestFramework): assert_equal(comb_psbt, psbt) self.log.info("Test walletprocesspsbt raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all") self.log.info("Test decoding PSBT with per-input preimage types") # note that the decodepsbt RPC doesn't check whether preimages and hashes match @@ -992,7 +983,7 @@ class PSBTTest(BitcoinTestFramework): self.nodes[2].sendrawtransaction(processed_psbt['hex']) self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all") if __name__ == '__main__': diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py index bc426d7371..ba86b278bd 100755 --- a/test/functional/rpc_setban.py +++ b/test/functional/rpc_setban.py @@ -64,20 +64,10 @@ class SetBanTests(BitcoinTestFramework): assert self.is_banned(node, tor_addr) assert not self.is_banned(node, ip_addr) - self.log.info("Test the ban list is preserved through restart") - - self.restart_node(1) - assert self.is_banned(node, tor_addr) - assert not self.is_banned(node, ip_addr) - node.setban(tor_addr, "remove") assert not self.is_banned(self.nodes[1], tor_addr) assert not self.is_banned(node, ip_addr) - self.restart_node(1) - assert not self.is_banned(node, tor_addr) - assert not self.is_banned(node, ip_addr) - self.log.info("Test -bantime") self.restart_node(1, ["-bantime=1234"]) self.nodes[1].setban("127.0.0.1", "add") diff --git a/test/functional/rpc_signrawtransactionwithkey.py b/test/functional/rpc_signrawtransactionwithkey.py index 0913f5057e..268584331e 100755 --- a/test/functional/rpc_signrawtransactionwithkey.py +++ b/test/functional/rpc_signrawtransactionwithkey.py @@ -124,7 +124,7 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): self.log.info("Test signing transaction with invalid sighashtype") tx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS) privkeys = [self.nodes[0].get_deterministic_priv_key().key] - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all") def run_test(self): self.successful_signing_test() diff --git a/test/functional/rpc_uptime.py b/test/functional/rpc_uptime.py index cb99e483ec..f8df59d02a 100755 --- a/test/functional/rpc_uptime.py +++ b/test/functional/rpc_uptime.py @@ -23,7 +23,7 @@ class UptimeTest(BitcoinTestFramework): self._test_uptime() def _test_negative_time(self): - assert_raises_rpc_error(-8, "Mocktime cannot be negative: -1.", self.nodes[0].setmocktime, -1) + assert_raises_rpc_error(-8, "Mocktime must be in the range [0, 9223372036], not -1.", self.nodes[0].setmocktime, -1) def _test_uptime(self): wait_time = 10 diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index ce76008c46..00bd1e4017 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -644,15 +644,17 @@ class P2PInterface(P2PConnection): self.wait_until(test_function, timeout=timeout) - def wait_for_getheaders(self, *, timeout=60): - """Waits for a getheaders message. + def wait_for_getheaders(self, block_hash=None, *, timeout=60): + """Waits for a getheaders message containing a specific block hash. - Receiving any getheaders message will satisfy the predicate. the last_message["getheaders"] - value must be explicitly cleared before calling this method, or this will return - immediately with success. TODO: change this method to take a hash value and only - return true if the correct block header has been requested.""" + If no block hash is provided, checks whether any getheaders message has been received by the node.""" def test_function(): - return self.last_message.get("getheaders") + last_getheaders = self.last_message.pop("getheaders", None) + if block_hash is None: + return last_getheaders + if last_getheaders is None: + return False + return block_hash == last_getheaders.locator.vHave[0] self.wait_until(test_function, timeout=timeout) diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 3275517888..7b19d31e17 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -483,7 +483,7 @@ class CScript(bytes): i = 0 while i < len(self): sop_idx = i - opcode = self[i] + opcode = CScriptOp(self[i]) i += 1 if opcode > OP_PUSHDATA4: @@ -590,7 +590,7 @@ class CScript(bytes): n += 1 elif opcode in (OP_CHECKMULTISIG, OP_CHECKMULTISIGVERIFY): if fAccurate and (OP_1 <= lastOpcode <= OP_16): - n += opcode.decode_op_n() + n += lastOpcode.decode_op_n() else: n += 20 lastOpcode = opcode @@ -782,6 +782,20 @@ class TestFrameworkScript(unittest.TestCase): for value in values: self.assertEqual(CScriptNum.decode(CScriptNum.encode(CScriptNum(value))), value) + def test_legacy_sigopcount(self): + # test repeated single sig ops + for n_ops in range(1, 100, 10): + for singlesig_op in (OP_CHECKSIG, OP_CHECKSIGVERIFY): + singlesigs_script = CScript([singlesig_op]*n_ops) + self.assertEqual(singlesigs_script.GetSigOpCount(fAccurate=False), n_ops) + self.assertEqual(singlesigs_script.GetSigOpCount(fAccurate=True), n_ops) + # test multisig op (including accurate counting, i.e. BIP16) + for n in range(1, 16+1): + for multisig_op in (OP_CHECKMULTISIG, OP_CHECKMULTISIGVERIFY): + multisig_script = CScript([CScriptOp.encode_op_n(n), multisig_op]) + self.assertEqual(multisig_script.GetSigOpCount(fAccurate=False), 20) + self.assertEqual(multisig_script.GetSigOpCount(fAccurate=True), n) + def BIP341_sha_prevouts(txTo): return sha256(b"".join(i.prevout.serialize() for i in txTo.vin)) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index dbaf42fdaa..0de09b6440 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -505,6 +505,8 @@ def fill_mempool(test_framework, node, miniwallet): It will not ensure mempools become synced as it is based on a single node and assumes -minrelaytxfee is 1 sat/vbyte. + To avoid unintentional tx dependencies, it is recommended to use separate miniwallets for + mempool filling vs transactions in tests. """ test_framework.log.info("Fill the mempool until eviction is triggered and the mempoolminfee rises") txouts = gen_return_txouts() @@ -522,8 +524,14 @@ def fill_mempool(test_framework, node, miniwallet): # Mine COINBASE_MATURITY - 1 blocks so that the UTXOs are allowed to be spent test_framework.generate(node, 100 - 1) + # Get all UTXOs up front to ensure none of the transactions spend from each other, as that may + # change their effective feerate and thus the order in which they are selected for eviction. + confirmed_utxos = [miniwallet.get_utxo(confirmed_only=True) for _ in range(num_of_batches * tx_batch_size + 1)] + assert_equal(len(confirmed_utxos), num_of_batches * tx_batch_size + 1) + test_framework.log.debug("Create a mempool tx that will be evicted") - tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, fee_rate=relayfee)["txid"] + tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, utxo_to_spend=confirmed_utxos[0], fee_rate=relayfee)["txid"] + del confirmed_utxos[0] # Increase the tx fee rate to give the subsequent transactions a higher priority in the mempool # The tx has an approx. vsize of 65k, i.e. multiplying the previous fee rate (in sats/kvB) @@ -534,7 +542,9 @@ def fill_mempool(test_framework, node, miniwallet): with node.assert_debug_log(["rolling minimum fee bumped"]): for batch_of_txid in range(num_of_batches): fee = (batch_of_txid + 1) * base_fee - create_lots_of_big_transactions(miniwallet, node, fee, tx_batch_size, txouts) + utxos = confirmed_utxos[:tx_batch_size] + create_lots_of_big_transactions(miniwallet, node, fee, tx_batch_size, txouts, utxos) + del confirmed_utxos[:tx_batch_size] test_framework.log.debug("The tx should be evicted by now") # The number of transactions created should be greater than the ones present in the mempool diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 44811918bf..2168e607b2 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful util functions for testing the wallet""" from collections import namedtuple +import unittest from test_framework.address import ( byte_to_base58, @@ -15,6 +16,11 @@ from test_framework.address import ( script_to_p2wsh, ) from test_framework.key import ECKey +from test_framework.messages import ( + CTxIn, + CTxInWitness, + WITNESS_SCALE_FACTOR, +) from test_framework.script_util import ( key_to_p2pkh_script, key_to_p2wpkh_script, @@ -123,6 +129,19 @@ def generate_keypair(compressed=True, wif=False): privkey = bytes_to_wif(privkey.get_bytes(), compressed) return privkey, pubkey +def calculate_input_weight(scriptsig_hex, witness_stack_hex=None): + """Given a scriptSig and a list of witness stack items for an input in hex format, + calculate the total input weight. If the input has no witness data, + `witness_stack_hex` can be set to None.""" + tx_in = CTxIn(scriptSig=bytes.fromhex(scriptsig_hex)) + witness_size = 0 + if witness_stack_hex is not None: + tx_inwit = CTxInWitness() + for witness_item_hex in witness_stack_hex: + tx_inwit.scriptWitness.stack.append(bytes.fromhex(witness_item_hex)) + witness_size = len(tx_inwit.serialize()) + return len(tx_in.serialize()) * WITNESS_SCALE_FACTOR + witness_size + class WalletUnlock(): """ A context manager for unlocking a wallet with a passphrase and automatically locking it afterward. @@ -141,3 +160,42 @@ class WalletUnlock(): def __exit__(self, *args): _ = args self.wallet.walletlock() + + +class TestFrameworkWalletUtil(unittest.TestCase): + def test_calculate_input_weight(self): + SKELETON_BYTES = 32 + 4 + 4 # prevout-txid, prevout-index, sequence + SMALL_LEN_BYTES = 1 # bytes needed for encoding scriptSig / witness item lengths < 253 + LARGE_LEN_BYTES = 3 # bytes needed for encoding scriptSig / witness item lengths >= 253 + + # empty scriptSig, no witness + self.assertEqual(calculate_input_weight(""), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + self.assertEqual(calculate_input_weight("", None), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + # small scriptSig, no witness + scriptSig_small = "00"*252 + self.assertEqual(calculate_input_weight(scriptSig_small, None), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + # small scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_small, []), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # large scriptSig, no witness + scriptSig_large = "00"*253 + self.assertEqual(calculate_input_weight(scriptSig_large, None), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + # large scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_large, []), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # empty scriptSig, 5 small witness stack items + self.assertEqual(calculate_input_weight("", ["00", "11", "22", "33", "44"]), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 5 * SMALL_LEN_BYTES + 5) + # empty scriptSig, 253 small witness stack items + self.assertEqual(calculate_input_weight("", ["00"]*253), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + LARGE_LEN_BYTES + 253 * SMALL_LEN_BYTES + 253) + # small scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_small, ["00"*253]*3), + ((SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) + # large scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_large, ["00"*253]*3), + ((SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 629c9d025a..2b0b24ec05 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -26,7 +26,6 @@ import sys import tempfile import re import logging -import unittest os.environ["REQUIRE_WALLET_TYPE_SET"] = "1" @@ -70,22 +69,7 @@ if platform.system() != 'Windows' or sys.getwindowsversion() >= (10, 0, 14393): TEST_EXIT_PASSED = 0 TEST_EXIT_SKIPPED = 77 -# List of framework modules containing unit tests. Should be kept in sync with -# the output of `git grep unittest.TestCase ./test/functional/test_framework` -TEST_FRAMEWORK_MODULES = [ - "address", - "crypto.bip324_cipher", - "blocktools", - "crypto.chacha20", - "crypto.ellswift", - "key", - "messages", - "crypto.muhash", - "crypto.poly1305", - "crypto.ripemd160", - "script", - "segwit_addr", -] +TEST_FRAMEWORK_UNIT_TESTS = 'feature_framework_unit_tests.py' EXTENDED_SCRIPTS = [ # These tests are not run by default. @@ -256,6 +240,7 @@ BASE_SCRIPTS = [ 'wallet_keypool.py --descriptors', 'wallet_descriptor.py --descriptors', 'p2p_nobloomfilter_messages.py', + TEST_FRAMEWORK_UNIT_TESTS, 'p2p_filter.py', 'rpc_setban.py --v1transport', 'rpc_setban.py --v2transport', @@ -441,7 +426,6 @@ def main(): parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs") parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure') parser.add_argument('--filter', help='filter scripts to run by regular expression') - parser.add_argument('--skipunit', '-u', action='store_true', help='skip unit tests for the test framework') args, unknown_args = parser.parse_known_args() @@ -553,10 +537,9 @@ def main(): combined_logs_len=args.combinedlogslen, failfast=args.failfast, use_term_control=args.ansi, - skipunit=args.skipunit, ) -def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control, skipunit=False): +def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control): args = args or [] # Warn if bitcoind is already running @@ -579,15 +562,6 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= # a hard link or a copy on any platform. See https://github.com/bitcoin/bitcoin/pull/27561. sys.path.append(tests_dir) - if not skipunit: - print("Running Unit Tests for Test Framework Modules") - test_framework_tests = unittest.TestSuite() - for module in TEST_FRAMEWORK_MODULES: - test_framework_tests.addTest(unittest.TestLoader().loadTestsFromName("test_framework.{}".format(module))) - result = unittest.TextTestRunner(verbosity=1, failfast=True).run(test_framework_tests) - if not result.wasSuccessful(): - sys.exit("Early exiting after failure in TestFramework unit tests") - flags = ['--cachedir={}'.format(cache_dir)] + args if enable_coverage: diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index e4ca341b49..0a0a8dba0d 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -9,10 +9,6 @@ from itertools import product from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create -from test_framework.messages import ( - ser_compact_size, - WITNESS_SCALE_FACTOR, -) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -21,7 +17,10 @@ from test_framework.util import ( assert_raises_rpc_error, count_bytes, ) -from test_framework.wallet_util import generate_keypair +from test_framework.wallet_util import ( + calculate_input_weight, + generate_keypair, +) class WalletSendTest(BitcoinTestFramework): @@ -543,17 +542,9 @@ class WalletSendTest(BitcoinTestFramework): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) # Input weight error conditions assert_raises_rpc_error( diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 32a1887153..abfc3c1ba1 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -130,8 +130,9 @@ class WalletSignerTest(BitcoinTestFramework): assert_equal(address_info['hdkeypath'], "m/86h/1h/0h/0/0") self.log.info('Test walletdisplayaddress') - result = hww.walletdisplayaddress(address1) - assert_equal(result, {"address": address1}) + for address in [address1, address2, address3]: + result = hww.walletdisplayaddress(address) + assert_equal(result, {"address": address}) # Handle error thrown by script self.set_mock_result(self.nodes[1], "2") @@ -140,6 +141,13 @@ class WalletSignerTest(BitcoinTestFramework): ) self.clear_mock_result(self.nodes[1]) + # Returned address MUST match: + address_fail = hww.getnewaddress(address_type="bech32") + assert_equal(address_fail, "bcrt1ql7zg7ukh3dwr25ex2zn9jse926f27xy2jz58tm") + assert_raises_rpc_error(-1, 'Signer echoed unexpected address wrong_address', + hww.walletdisplayaddress, address_fail + ) + self.log.info('Prepare mock PSBT') self.nodes[0].sendtoaddress(address4, 1) self.generate(self.nodes[0], 1) diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index b0517f951d..612a2542e7 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -55,7 +55,7 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): def test_with_invalid_sighashtype(self): self.log.info("Test signrawtransactionwithwallet raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all") def script_verification_error_test(self): """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script. diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 558d63e85c..a635175e7c 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -11,6 +11,7 @@ import argparse import configparser import logging import os +import platform import random import subprocess import sys @@ -18,7 +19,7 @@ import sys def get_fuzz_env(*, target, source_dir): symbolizer = os.environ.get('LLVM_SYMBOLIZER_PATH', "/usr/bin/llvm-symbolizer") - return { + fuzz_env = { 'FUZZ': target, 'UBSAN_OPTIONS': f'suppressions={source_dir}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1', @@ -27,6 +28,10 @@ def get_fuzz_env(*, target, source_dir): 'ASAN_SYMBOLIZER_PATH':symbolizer, 'MSAN_SYMBOLIZER_PATH':symbolizer, } + if platform.system() == "Windows": + # On Windows, `env` option must include valid `SystemRoot`. + fuzz_env = {**fuzz_env, 'SystemRoot': os.environ.get('SystemRoot')} + return fuzz_env def main(): diff --git a/test/lint/commit-script-check.sh b/test/lint/commit-script-check.sh index 55c9528dea..fe845ed19e 100755 --- a/test/lint/commit-script-check.sh +++ b/test/lint/commit-script-check.sh @@ -22,6 +22,11 @@ if ! sed --help 2>&1 | grep -q 'GNU'; then exit 1; fi +if ! grep --help 2>&1 | grep -q 'GNU'; then + echo "Error: the installed grep package is not compatible. Please make sure you have GNU grep installed in your system."; + exit 1; +fi + RET=0 PREV_BRANCH=$(git name-rev --name-only HEAD) PREV_HEAD=$(git rev-parse HEAD) diff --git a/test/lint/lint-spelling.py b/test/lint/lint-spelling.py index 8d35fe6670..3e578b218f 100755 --- a/test/lint/lint-spelling.py +++ b/test/lint/lint-spelling.py @@ -14,7 +14,7 @@ from subprocess import check_output, STDOUT, CalledProcessError from lint_ignore_dirs import SHARED_EXCLUDED_SUBTREES IGNORE_WORDS_FILE = 'test/lint/spelling.ignore-words.txt' -FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)contrib/guix/patches", ":(exclude)src/util/subprocess.hpp"] +FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)contrib/guix/patches"] FILES_ARGS += [f":(exclude){dir}" for dir in SHARED_EXCLUDED_SUBTREES] |