diff options
39 files changed, 269 insertions, 310 deletions
diff --git a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in index a5702a83ba..972d6d05d7 100644 --- a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in +++ b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.in @@ -10,6 +10,12 @@ </PropertyGroup> <ItemGroup> @SOURCE_FILES@ + <ClCompile Include="..\..\src\bench\coin_selection.cpp" /> + <ClCompile Include="..\..\src\bench\wallet_balance.cpp" /> + <ClCompile Include="..\..\src\bench\wallet_create.cpp" /> + <ClCompile Include="..\..\src\bench\wallet_create_tx.cpp" /> + <ClCompile Include="..\..\src\bench\wallet_ismine.cpp" /> + <ClCompile Include="..\..\src\bench\wallet_loading.cpp" /> </ItemGroup> <ItemGroup> <ProjectReference Include="..\libbitcoin_consensus\libbitcoin_consensus.vcxproj"> diff --git a/build_msvc/fuzz/fuzz.vcxproj b/build_msvc/fuzz/fuzz.vcxproj index e7ee211b2d..7c72703c93 100644 --- a/build_msvc/fuzz/fuzz.vcxproj +++ b/build_msvc/fuzz/fuzz.vcxproj @@ -9,8 +9,7 @@ <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> </PropertyGroup> <ItemGroup> - <!-- TODO: Fix the code and remove miniscript.cpp exclusion. --> - <ClCompile Include="..\..\src\test\fuzz\*.cpp" Exclude="..\..\src\test\fuzz\miniscript.cpp" /> + <ClCompile Include="..\..\src\test\fuzz\*.cpp" /> <ClCompile Include="..\..\src\test\fuzz\util\descriptor.cpp"> <ObjectFileName>$(IntDir)test_fuzz_util_descriptor.obj</ObjectFileName> </ClCompile> @@ -80,11 +79,6 @@ <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" /> diff --git a/build_msvc/test_bitcoin/test_bitcoin.vcxproj b/build_msvc/test_bitcoin/test_bitcoin.vcxproj index 0ae3819e50..b5aa58057f 100644 --- a/build_msvc/test_bitcoin/test_bitcoin.vcxproj +++ b/build_msvc/test_bitcoin/test_bitcoin.vcxproj @@ -59,11 +59,6 @@ <Project>{18430fef-6b61-4c53-b396-718e02850f1b}</Project> </ProjectReference> </ItemGroup> - <ItemDefinitionGroup> - <ClCompile> - <DisableSpecificWarnings>4018;4244;4267;4703;4715;4805</DisableSpecificWarnings> - </ClCompile> - </ItemDefinitionGroup> <Target Name="RawBenchHeaderGen" BeforeTargets="PrepareForBuild"> <PropertyGroup> <ErrorText>There was an error executing the JSON test header generation task.</ErrorText> diff --git a/ci/test/00_setup_env_i686_multiprocess.sh b/ci/test/00_setup_env_i686_multiprocess.sh index 00a4d781c2..f6463438d3 100755 --- a/ci/test/00_setup_env_i686_multiprocess.sh +++ b/ci/test/00_setup_env_i686_multiprocess.sh @@ -14,5 +14,5 @@ export DEP_OPTS="DEBUG=1 MULTIPROCESS=1" export GOAL="install" export TEST_RUNNER_EXTRA="--v2transport" export BITCOIN_CONFIG="--enable-debug CC='clang -m32' CXX='clang++ -m32' \ -CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" +CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='-Wno-error=documentation'" export BITCOIND=bitcoin-node # Used in functional tests diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index bce5ee74b6..bf80d5d435 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -16,4 +16,4 @@ export GOAL="deploy" # Prior to 11.0.0, the mingw-w64 headers were missing noreturn attributes, causing warnings when # cross-compiling for Windows. https://sourceforge.net/p/mingw-w64/bugs/306/ # https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe -export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests CXXFLAGS=-Wno-return-type" +export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'" diff --git a/configure.ac b/configure.ac index c7d124a1f1..ee4400f21a 100644 --- a/configure.ac +++ b/configure.ac @@ -401,37 +401,35 @@ if test "$enable_werror" = "yes"; then ERROR_CXXFLAGS=$CXXFLAG_WERROR fi -if test "$CXXFLAGS_overridden" = "no"; then - AX_CHECK_COMPILE_FLAG([-Wall], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wextra], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wgnu], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wgnu"], [], [$CXXFLAG_WERROR]) - dnl some compilers will ignore -Wformat-security without -Wformat, so just combine the two here. - AX_CHECK_COMPILE_FLAG([-Wformat -Wformat-security], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat -Wformat-security"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wvla], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wshadow-field], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wthread-safety], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wloop-analysis], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wloop-analysis"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wredundant-decls], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wunused-member-function], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-member-function"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wdate-time], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdate-time"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wconditional-uninitialized], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wconditional-uninitialized"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wduplicated-branches], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-branches"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wduplicated-cond], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-cond"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wlogical-op], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wlogical-op"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Woverloaded-virtual], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Woverloaded-virtual"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wsuggest-override], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wsuggest-override"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wimplicit-fallthrough"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wunreachable-code], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wdocumentation], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"], [], [$CXXFLAG_WERROR]) - - dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all - dnl unknown options if any other warning is produced. Test the -Wfoo case, and - dnl set the -Wno-foo case if it works. - AX_CHECK_COMPILE_FLAG([-Wunused-parameter], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wself-assign], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-self-assign"], [], [$CXXFLAG_WERROR]) - if test "$suppress_external_warnings" != "yes" ; then - AX_CHECK_COMPILE_FLAG([-Wdeprecated-copy], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-copy"], [], [$CXXFLAG_WERROR]) - fi +AX_CHECK_COMPILE_FLAG([-Wall], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wextra], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wgnu], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wgnu"], [], [$CXXFLAG_WERROR]) +dnl some compilers will ignore -Wformat-security without -Wformat, so just combine the two here. +AX_CHECK_COMPILE_FLAG([-Wformat -Wformat-security], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat -Wformat-security"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wvla], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wshadow-field], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wthread-safety], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wloop-analysis], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wloop-analysis"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wredundant-decls], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wunused-member-function], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-member-function"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wdate-time], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdate-time"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wconditional-uninitialized], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wconditional-uninitialized"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wduplicated-branches], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-branches"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wduplicated-cond], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wduplicated-cond"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wlogical-op], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wlogical-op"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Woverloaded-virtual], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Woverloaded-virtual"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wsuggest-override], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wsuggest-override"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wimplicit-fallthrough"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wunreachable-code], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wdocumentation], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"], [], [$CXXFLAG_WERROR]) + +dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all +dnl unknown options if any other warning is produced. Test the -Wfoo case, and +dnl set the -Wno-foo case if it works. +AX_CHECK_COMPILE_FLAG([-Wunused-parameter], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wself-assign], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-self-assign"], [], [$CXXFLAG_WERROR]) +if test "$suppress_external_warnings" != "yes" ; then + AX_CHECK_COMPILE_FLAG([-Wdeprecated-copy], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-copy"], [], [$CXXFLAG_WERROR]) fi dnl Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review. diff --git a/depends/funcs.mk b/depends/funcs.mk index 494ed5d324..d8a72421b4 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -182,6 +182,7 @@ $(1)_cmake=env CC="$$($(1)_cc)" \ cmake -DCMAKE_INSTALL_PREFIX:PATH="$$($($(1)_type)_prefix)" \ -DCMAKE_INSTALL_LIBDIR=lib/ \ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DCMAKE_VERBOSE_MAKEFILE:BOOL=$(V) \ $$($(1)_config_opts) ifeq ($($(1)_type),build) $(1)_cmake += -DCMAKE_INSTALL_RPATH:PATH="$$($($(1)_type)_prefix)/lib" diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index 5698a7cbb1..077e3bb1ee 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -1,30 +1,31 @@ package=miniupnpc -$(package)_version=2.2.2 +$(package)_version=2.2.7 $(package)_download_path=https://miniupnp.tuxfamily.org/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz -$(package)_sha256_hash=888fb0976ba61518276fe1eda988589c700a3f2a69d71089260d75562afd3687 -$(package)_patches=dont_leak_info.patch respect_mingw_cflags.patch no_libtool.patch +$(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1 +$(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch +$(package)_build_subdir=build -# Next time this package is updated, ensure that _WIN32_WINNT is still properly set. -# See discussion in https://github.com/bitcoin/bitcoin/pull/25964. define $(package)_set_vars -$(package)_build_opts=CC="$($(package)_cc)" -$(package)_build_opts_mingw32=-f Makefile.mingw CFLAGS="$($(package)_cflags) -D_WIN32_WINNT=0x0601" -$(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" +$(package)_config_opts = -DUPNPC_BUILD_SAMPLE=OFF -DUPNPC_BUILD_SHARED=OFF +$(package)_config_opts += -DUPNPC_BUILD_STATIC=ON -DUPNPC_BUILD_TESTS=OFF +$(package)_config_opts_mingw32 += -DMINIUPNPC_TARGET_WINDOWS_VERSION=0x0601 endef define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/dont_leak_info.patch && \ - patch -p1 < $($(package)_patch_dir)/respect_mingw_cflags.patch && \ - patch -p1 < $($(package)_patch_dir)/no_libtool.patch + patch -p1 < $($(package)_patch_dir)/cmake_get_src_addr.patch && \ + patch -p1 < $($(package)_patch_dir)/fix_windows_snprintf.patch +endef + +define $(package)_config_cmds + $($(package)_cmake) -S .. -B . endef define $(package)_build_cmds - $(MAKE) libminiupnpc.a $($(package)_build_opts) + $(MAKE) endef define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include/miniupnpc $($(package)_staging_prefix_dir)/lib &&\ - install *.h $($(package)_staging_prefix_dir)/include/miniupnpc &&\ - install libminiupnpc.a $($(package)_staging_prefix_dir)/lib + cmake --install . --prefix $($(package)_staging_prefix_dir) endef diff --git a/depends/patches/miniupnpc/cmake_get_src_addr.patch b/depends/patches/miniupnpc/cmake_get_src_addr.patch new file mode 100644 index 0000000000..bae1b738f3 --- /dev/null +++ b/depends/patches/miniupnpc/cmake_get_src_addr.patch @@ -0,0 +1,22 @@ +commit cb2026239c2a3aff393952ccb0ee1c448189402d +Author: fanquake <fanquake@gmail.com> +Date: Fri Mar 22 14:03:54 2024 +0000 + + build: add MINIUPNPC_GET_SRC_ADDR to CMake build + + This mirrors the autotools build. + + See https://github.com/miniupnp/miniupnp/pull/721. + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 1aa95a8..0cacf3e 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -31,6 +31,7 @@ endif () + if (NOT WIN32) + target_compile_definitions(miniupnpc-private INTERFACE + MINIUPNPC_SET_SOCKET_TIMEOUT ++ MINIUPNPC_GET_SRC_ADDR + _BSD_SOURCE _DEFAULT_SOURCE) + if (NOT APPLE AND NOT CMAKE_SYSTEM_NAME MATCHES ".*BSD" AND NOT CMAKE_SYSTEM_NAME STREQUAL "SunOS") + # add_definitions (-D_POSIX_C_SOURCE=200112L) diff --git a/depends/patches/miniupnpc/dont_leak_info.patch b/depends/patches/miniupnpc/dont_leak_info.patch index 512f9c50ea..95a09a26dc 100644 --- a/depends/patches/miniupnpc/dont_leak_info.patch +++ b/depends/patches/miniupnpc/dont_leak_info.patch @@ -1,31 +1,31 @@ -commit 8815452257437ba36607d0e2381c01142d1c7bb0 +commit 51f6dd991c29af66fb4f64c6feb2787cce23a1a7 Author: fanquake <fanquake@gmail.com> -Date: Thu Nov 19 10:51:19 2020 +0800 +Date: Mon Jan 8 11:21:40 2024 +0000 Don't leak OS and miniupnpc version info in User-Agent -diff --git a//minisoap.c b/minisoap.c -index 7860667..775580b 100644 ---- a/minisoap.c -+++ b/minisoap.c +diff --git a/src/minisoap.c b/src/minisoap.c +index 903ac5f..046e0ea 100644 +--- a/src/minisoap.c ++++ b/src/minisoap.c @@ -90,7 +90,7 @@ int soapPostSubmit(SOCKET fd, headerssize = snprintf(headerbuf, sizeof(headerbuf), "POST %s HTTP/%s\r\n" "Host: %s%s\r\n" -- "User-Agent: " OS_STRING ", " UPNP_VERSION_STRING ", MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" +- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" + "User-Agent: " UPNP_VERSION_STRING "\r\n" "Content-Length: %d\r\n" + #if (UPNP_VERSION_MAJOR == 1) && (UPNP_VERSION_MINOR == 0) "Content-Type: text/xml\r\n" - "SOAPAction: \"%s\"\r\n" -diff --git a/miniwget.c b/miniwget.c -index d5b7970..05aeb9c 100644 ---- a/miniwget.c -+++ b/miniwget.c +diff --git a/src/miniwget.c b/src/miniwget.c +index e76a5e5..0cc36fe 100644 +--- a/src/miniwget.c ++++ b/src/miniwget.c @@ -444,7 +444,7 @@ miniwget3(const char * host, "GET %s HTTP/%s\r\n" "Host: %s:%d\r\n" "Connection: Close\r\n" -- "User-Agent: " OS_STRING ", " UPNP_VERSION_STRING ", MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" +- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" + "User-Agent: " UPNP_VERSION_STRING "\r\n" "\r\n", diff --git a/depends/patches/miniupnpc/fix_windows_snprintf.patch b/depends/patches/miniupnpc/fix_windows_snprintf.patch new file mode 100644 index 0000000000..ff9e26231e --- /dev/null +++ b/depends/patches/miniupnpc/fix_windows_snprintf.patch @@ -0,0 +1,25 @@ +commit a1e9de80ab99b4c956a6a4e21d3e0de6f7a1014d +Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> +Date: Sat Apr 20 15:14:47 2024 +0100 + + Fix macro expression that guards `snprintf` for Windows + + Otherwise, the `snprintf` is still wrongly emulated for the following + cases: + - mingw-w64 6.0.0 or new with ucrt + - mingw-w64 8.0.0 or new with iso c ext + +--- a/src/win32_snprintf.h ++++ b/src/win32_snprintf.h +@@ -23,9 +23,9 @@ + (defined(_MSC_VER) && _MSC_VER < 1900) /* Visual Studio older than 2015 */ || \ + (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR) && defined(__NO_ISOCEXT)) /* mingw32 without iso c ext */ || \ + (defined(__MINGW64_VERSION_MAJOR) && /* mingw-w64 not ... */ !( \ +- (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0)) /* ... with ansi stdio */ || \ ++ (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0) /* ... with ansi stdio */ || \ + (__MINGW64_VERSION_MAJOR >= 6 && defined(_UCRT)) /* ... at least 6.0.0 with ucrt */ || \ +- (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT)) /* ... at least 8.0.0 with iso c ext */ || \ ++ (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT))) /* ... at least 8.0.0 with iso c ext */ || \ + 0) || \ + 0) + diff --git a/depends/patches/miniupnpc/no_libtool.patch b/depends/patches/miniupnpc/no_libtool.patch deleted file mode 100644 index bb7d4a87ef..0000000000 --- a/depends/patches/miniupnpc/no_libtool.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff -ruN miniupnpc-2.2.2/Makefile miniupnpc-2.2.2.new/Makefile ---- miniupnpc-2.2.2/Makefile 2020-11-27 18:25:02.000000000 +0000 -+++ miniupnpc-2.2.2.new/Makefile 2024-01-23 20:58:08.387188527 +0000 -@@ -298,11 +298,7 @@ - makedepend -Y -- $(CFLAGS) -- $(SRCS) 2>/dev/null - - $(LIBRARY): $(LIBOBJS) --ifneq (, $(findstring darwin, $(OS))) -- $(LIBTOOL) -static -o $@ $? --else - $(AR) crs $@ $? --endif - - $(SHAREDLIBRARY): $(LIBOBJS) - ifneq (, $(findstring darwin, $(OS))) diff --git a/depends/patches/miniupnpc/respect_mingw_cflags.patch b/depends/patches/miniupnpc/respect_mingw_cflags.patch deleted file mode 100644 index a44580ddab..0000000000 --- a/depends/patches/miniupnpc/respect_mingw_cflags.patch +++ /dev/null @@ -1,23 +0,0 @@ -commit fec515a7ac9991a0ee91068fda046b54b191155e -Author: fanquake <fanquake@gmail.com> -Date: Wed Jul 27 15:52:37 2022 +0100 - - build: respect CFLAGS in makefile.mingw - - Similar to the other Makefile. - - Cherry-pick of https://github.com/miniupnp/miniupnp/pull/619. - -diff --git a/Makefile.mingw b/Makefile.mingw -index 2bff7bd..88430d2 100644 ---- a/Makefile.mingw -+++ b/Makefile.mingw -@@ -19,7 +19,7 @@ else - RM = rm -f - endif - #CFLAGS = -Wall -g -DDEBUG -D_WIN32_WINNT=0X501 --CFLAGS = -Wall -W -Wstrict-prototypes -Os -DNDEBUG -D_WIN32_WINNT=0X501 -+CFLAGS ?= -Wall -W -Wstrict-prototypes -Os -DNDEBUG -D_WIN32_WINNT=0X501 - LDLIBS = -lws2_32 -liphlpapi - # -lwsock32 - # -liphlpapi is needed for GetBestRoute() and GetIpAddrTable() diff --git a/doc/build-osx.md b/doc/build-osx.md index 5c3dc1ac7f..20c92ab7a4 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -51,6 +51,20 @@ To install, run the following from your terminal: brew install automake libtool boost pkg-config libevent ``` +For macOS 11 (Big Sur) and 12 (Monterey) you need to install a more recent version of llvm. + +``` bash +brew install llvm +``` + +And append the following to the configure commands below: + +``` bash +CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++ +``` + +Try `llvm@17` if compilation fails with the default version of llvm. + ### 4. Clone Bitcoin repository `git` should already be installed by default on your system. diff --git a/doc/dependencies.md b/doc/dependencies.md index 43973d0c15..2b9d9128d3 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -36,7 +36,7 @@ You can find installation instructions in the `build-*.md` file for your platfor | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | | [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 | +| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.7](https://github.com/bitcoin/bitcoin/pull/29707) | 2.1 | No | ### Notifications | Dependency | Releases | Version used | Minimum required | Runtime | diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 32f55f51e1..d7e772ac82 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -51,9 +51,14 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } +#ifndef _MSC_VER +// TODO: Being built with MSVC, the fs::remove_all() call in +// the WalletCreate() fails with the error "The process cannot +// access the file because it is being used by another process." #ifdef USE_SQLITE BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW); BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW); #endif +#endif } // namespace wallet diff --git a/src/init.cpp b/src/init.cpp index 4d7638cd6e..1e6d296551 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -634,7 +634,7 @@ void SetupServerArgs(ArgsManager& argsman) MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); - argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, + argsman.AddArg("-permitbaremultisig", strprintf("Relay transactions creating non-P2SH multisig outputs (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); @@ -1297,7 +1297,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - for (const auto &port_option : std::vector<std::pair<std::string, bool>>{ + for ([[maybe_unused]] const auto& [arg, unix] : std::vector<std::pair<std::string, bool>>{ // arg name UNIX socket support {"-i2psam", false}, {"-onion", true}, @@ -1309,10 +1309,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) {"-zmqpubhashtx", true}, {"-zmqpubrawblock", true}, {"-zmqpubrawtx", true}, - {"-zmqpubsequence", true} + {"-zmqpubsequence", true}, }) { - 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}; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bdbf077ab5..9b5983b9d0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -600,15 +600,6 @@ private: void ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, const std::list<CTransactionRef>& replaced_transactions) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); - /** Handle the results of package validation: calls ProcessValidTx and ProcessInvalidTx for - * individual transactions, and caches rejection for the package as a group. - * @param[in] senders Must contain the nodeids of the peers that provided each transaction - * in package, in the same order. - * */ - void ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector<NodeId>& senders) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); - - /** A package to validate */ struct PackageToValidate { const Package m_txns; const std::vector<NodeId> m_senders; @@ -633,6 +624,12 @@ private: } }; + /** Handle the results of package validation: calls ProcessValidTx and ProcessInvalidTx for + * individual transactions, and caches rejection for the package as a group. + */ + void ProcessPackageResult(const PackageToValidate& package_to_validate, const PackageMempoolAcceptResult& package_result) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + /** Look for a child of this transaction in the orphanage to form a 1-parent-1-child package, * skipping any combinations that have already been tried. Return the resulting package along with * the senders of its respective transactions, or std::nullopt if no package is found. */ @@ -3252,22 +3249,21 @@ void PeerManagerImpl::ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, c } } -void PeerManagerImpl::ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector<NodeId>& senders) +void PeerManagerImpl::ProcessPackageResult(const PackageToValidate& package_to_validate, const PackageMempoolAcceptResult& package_result) { AssertLockNotHeld(m_peer_mutex); AssertLockHeld(g_msgproc_mutex); AssertLockHeld(cs_main); + const auto& package = package_to_validate.m_txns; + const auto& senders = package_to_validate.m_senders; + if (package_result.m_state.IsInvalid()) { m_recent_rejects_reconsiderable.insert(GetPackageHash(package)); } // We currently only expect to process 1-parent-1-child packages. Remove if this changes. if (!Assume(package.size() == 2)) return; - // No package results to look through for PCKG_POLICY or PCKG_MEMPOOL_ERROR - if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY || - package_result.m_state.GetResult() == PackageValidationResult::PCKG_MEMPOOL_ERROR) return; - // Iterate backwards to erase in-package descendants from the orphanage before they become // relevant in AddChildrenToWorkSet. auto package_iter = package.rbegin(); @@ -3276,14 +3272,14 @@ void PeerManagerImpl::ProcessPackageResult(const Package& package, const Package const auto& tx = *package_iter; const NodeId nodeid = *senders_iter; const auto it_result{package_result.m_tx_results.find(tx->GetWitnessHash())}; - if (Assume(it_result != package_result.m_tx_results.end())) { + + // It is not guaranteed that a result exists for every transaction. + if (it_result != package_result.m_tx_results.end()) { const auto& tx_result = it_result->second; switch (tx_result.m_result_type) { case MempoolAcceptResult::ResultType::VALID: { - Assume(tx_result.m_replaced_transactions.has_value()); - std::list<CTransactionRef> empty_replacement_list; - ProcessValidTx(nodeid, tx, tx_result.m_replaced_transactions.value_or(empty_replacement_list)); + ProcessValidTx(nodeid, tx, tx_result.m_replaced_transactions); break; } case MempoolAcceptResult::ResultType::INVALID: @@ -3378,9 +3374,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::TXPACKAGES, " accepted orphan tx %s (wtxid=%s)\n", orphanHash.ToString(), orphan_wtxid.ToString()); - Assume(result.m_replaced_transactions.has_value()); - std::list<CTransactionRef> empty_replacement_list; - ProcessValidTx(peer.m_id, porphanTx, result.m_replaced_transactions.value_or(empty_replacement_list)); + ProcessValidTx(peer.m_id, porphanTx, result.m_replaced_transactions); return true; } else if (state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) { LogPrint(BCLog::TXPACKAGES, " invalid orphan tx %s (wtxid=%s) from peer=%d. %s\n", @@ -4553,7 +4547,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)}; LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(), package_result.m_state.IsValid() ? "package accepted" : "package rejected"); - ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders); + ProcessPackageResult(package_to_validate.value(), package_result); } } // If a tx is detected by m_recent_rejects it is ignored. Because we haven't @@ -4578,9 +4572,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const TxValidationState& state = result.m_state; if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { - Assume(result.m_replaced_transactions.has_value()); - std::list<CTransactionRef> empty_replacement_list; - ProcessValidTx(pfrom.GetId(), ptx, result.m_replaced_transactions.value_or(empty_replacement_list)); + ProcessValidTx(pfrom.GetId(), ptx, result.m_replaced_transactions); pfrom.m_last_tx_time = GetTime<std::chrono::seconds>(); } else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) @@ -4670,7 +4662,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)}; LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(), package_result.m_state.IsValid() ? "package accepted" : "package rejected"); - ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders); + ProcessPackageResult(package_to_validate.value(), package_result); } } @@ -5205,7 +5197,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, std::vector<unsigned char> vData; vRecv >> vData; - // Nodes must NEVER send a data item > 520 bytes (the max size for a script data object, + // Nodes must NEVER send a data item > MAX_SCRIPT_ELEMENT_SIZE bytes (the max size for a script data object, // and thus, the maximum size any matched object can have) in a filteradd message bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 566e5ec55a..c72d9deacb 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -85,7 +85,7 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, { // Add script to keystore keystore.AddCScript(script); - // Note that scripts over 520 bytes are not yet supported. + // Note that scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are not yet supported. switch (type) { case OutputType::LEGACY: return ScriptHash(script); diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index 99d2a6d514..693adcdfd0 100644 --- a/src/policy/packages.cpp +++ b/src/policy/packages.cpp @@ -156,7 +156,10 @@ uint256 GetPackageHash(const std::vector<CTransactionRef>& transactions) [](const auto& tx){ return tx->GetWitnessHash(); }); // Sort in ascending order - std::sort(wtxids_copy.begin(), wtxids_copy.end(), [](const auto& lhs, const auto& rhs) { return lhs.GetHex() < rhs.GetHex(); }); + std::sort(wtxids_copy.begin(), wtxids_copy.end(), [](const auto& lhs, const auto& rhs) { + return std::lexicographical_compare(std::make_reverse_iterator(lhs.end()), std::make_reverse_iterator(lhs.begin()), + std::make_reverse_iterator(rhs.end()), std::make_reverse_iterator(rhs.begin())); + }); // Get sha256 hash of the wtxids concatenated in this order HashWriter hashwriter; diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index d08ec4fb7f..d8b4b907e4 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -111,7 +111,7 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat for (const CTxIn& txin : tx.vin) { // Biggest 'standard' txin involving only keys is a 15-of-15 P2SH - // multisig with compressed keys (remember the 520 byte limit on + // multisig with compressed keys (remember the MAX_SCRIPT_ELEMENT_SIZE byte limit on // redeemScript size). That works out to a (15*(33+1))+3=513 byte // redeemScript, 513+1+15*(73+1)+3=1627 bytes of scriptSig, which // we round off to 1650(MAX_STANDARD_SCRIPTSIG_SIZE) bytes for diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 920bb9ea7f..e6fe3c7ebd 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -994,10 +994,8 @@ static RPCHelpMan submitpackage() fees.pushKV("effective-includes", effective_includes_res); } result_inner.pushKV("fees", fees); - if (it->second.m_replaced_transactions.has_value()) { - for (const auto& ptx : it->second.m_replaced_transactions.value()) { - replaced_txids.insert(ptx->GetHash()); - } + for (const auto& ptx : it->second.m_replaced_transactions) { + replaced_txids.insert(ptx->GetHash()); } break; } diff --git a/src/script/miniscript.cpp b/src/script/miniscript.cpp index 344a81bdf0..455bd56283 100644 --- a/src/script/miniscript.cpp +++ b/src/script/miniscript.cpp @@ -231,7 +231,8 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector<Ty Type acc_tl = "k"_mst; for (size_t i = 0; i < sub_types.size(); ++i) { Type t = sub_types[i]; - if (!(t << (i ? "Wdu"_mst : "Bdu"_mst))) return ""_mst; // Require Bdu, Wdu, Wdu, ... + static constexpr auto WDU{"Wdu"_mst}, BDU{"Bdu"_mst}; + if (!(t << (i ? WDU : BDU))) return ""_mst; // Require Bdu, Wdu, Wdu, ... if (!(t << "e"_mst)) all_e = false; if (!(t << "m"_mst)) all_m = false; if (t << "s"_mst) num_s += 1; diff --git a/src/script/miniscript.h b/src/script/miniscript.h index f635fa7340..4880f32410 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -123,12 +123,12 @@ class Type { //! Internal bitmap of properties (see ""_mst operator for details). uint32_t m_flags; - //! Internal constructor used by the ""_mst operator. - explicit constexpr Type(uint32_t flags) : m_flags(flags) {} + //! Internal constructor. + explicit constexpr Type(uint32_t flags) noexcept : m_flags(flags) {} public: - //! The only way to publicly construct a Type is using this literal operator. - friend constexpr Type operator"" _mst(const char* c, size_t l); + //! Construction function used by the ""_mst operator. + static consteval Type Make(uint32_t flags) noexcept { return Type(flags); } //! Compute the type with the union of properties. constexpr Type operator|(Type x) const { return Type(m_flags | x.m_flags); } @@ -150,11 +150,11 @@ public: }; //! Literal operator to construct Type objects. -inline constexpr Type operator"" _mst(const char* c, size_t l) { - Type typ{0}; +inline consteval Type operator"" _mst(const char* c, size_t l) { + Type typ{Type::Make(0)}; for (const char *p = c; p < c + l; p++) { - typ = typ | Type( + typ = typ | Type::Make( *p == 'B' ? 1 << 0 : // Base type *p == 'V' ? 1 << 1 : // Verify type *p == 'K' ? 1 << 2 : // Key type @@ -548,7 +548,8 @@ private: for (const auto& sub : subs) { subsize += sub->ScriptSize(); } - Type sub0type = subs.size() > 0 ? subs[0]->GetType() : ""_mst; + static constexpr auto NONE_MST{""_mst}; + Type sub0type = subs.size() > 0 ? subs[0]->GetType() : NONE_MST; return internal::ComputeScriptLen(fragment, sub0type, subsize, k, subs.size(), keys.size(), m_script_ctx); } @@ -712,9 +713,10 @@ private: for (const auto& sub : subs) sub_types.push_back(sub->GetType()); } // All other nodes than THRESH can be computed just from the types of the 0-3 subexpressions. - Type x = subs.size() > 0 ? subs[0]->GetType() : ""_mst; - Type y = subs.size() > 1 ? subs[1]->GetType() : ""_mst; - Type z = subs.size() > 2 ? subs[2]->GetType() : ""_mst; + static constexpr auto NONE_MST{""_mst}; + Type x = subs.size() > 0 ? subs[0]->GetType() : NONE_MST; + Type y = subs.size() > 1 ? subs[1]->GetType() : NONE_MST; + Type z = subs.size() > 2 ? subs[2]->GetType() : NONE_MST; return SanitizeType(ComputeType(fragment, x, y, z, sub_types, k, data.size(), subs.size(), keys.size(), m_script_ctx)); } diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp index 0d0ee59ab3..947424c4ac 100644 --- a/src/test/fuzz/miniscript.cpp +++ b/src/test/fuzz/miniscript.cpp @@ -391,6 +391,7 @@ std::optional<NodeInfo> ConsumeNodeStable(MsCtx script_ctx, FuzzedDataProvider& bool allow_K = (type_needed == ""_mst) || (type_needed << "K"_mst); bool allow_V = (type_needed == ""_mst) || (type_needed << "V"_mst); bool allow_W = (type_needed == ""_mst) || (type_needed << "W"_mst); + static constexpr auto B{"B"_mst}, K{"K"_mst}, V{"V"_mst}, W{"W"_mst}; switch (provider.ConsumeIntegral<uint8_t>()) { case 0: @@ -440,22 +441,22 @@ std::optional<NodeInfo> ConsumeNodeStable(MsCtx script_ctx, FuzzedDataProvider& } case 11: if (!(allow_B || allow_K || allow_V)) return {}; - return {{{"B"_mst, type_needed, type_needed}, Fragment::ANDOR}}; + return {{{B, type_needed, type_needed}, Fragment::ANDOR}}; case 12: if (!(allow_B || allow_K || allow_V)) return {}; - return {{{"V"_mst, type_needed}, Fragment::AND_V}}; + return {{{V, type_needed}, Fragment::AND_V}}; case 13: if (!allow_B) return {}; - return {{{"B"_mst, "W"_mst}, Fragment::AND_B}}; + return {{{B, W}, Fragment::AND_B}}; case 15: if (!allow_B) return {}; - return {{{"B"_mst, "W"_mst}, Fragment::OR_B}}; + return {{{B, W}, Fragment::OR_B}}; case 16: if (!allow_V) return {}; - return {{{"B"_mst, "V"_mst}, Fragment::OR_C}}; + return {{{B, V}, Fragment::OR_C}}; case 17: if (!allow_B) return {}; - return {{{"B"_mst, "B"_mst}, Fragment::OR_D}}; + return {{{B, B}, Fragment::OR_D}}; case 18: if (!(allow_B || allow_K || allow_V)) return {}; return {{{type_needed, type_needed}, Fragment::OR_I}}; @@ -472,25 +473,25 @@ std::optional<NodeInfo> ConsumeNodeStable(MsCtx script_ctx, FuzzedDataProvider& } case 20: if (!allow_W) return {}; - return {{{"B"_mst}, Fragment::WRAP_A}}; + return {{{B}, Fragment::WRAP_A}}; case 21: if (!allow_W) return {}; - return {{{"B"_mst}, Fragment::WRAP_S}}; + return {{{B}, Fragment::WRAP_S}}; case 22: if (!allow_B) return {}; - return {{{"K"_mst}, Fragment::WRAP_C}}; + return {{{K}, Fragment::WRAP_C}}; case 23: if (!allow_B) return {}; - return {{{"V"_mst}, Fragment::WRAP_D}}; + return {{{V}, Fragment::WRAP_D}}; case 24: if (!allow_V) return {}; - return {{{"B"_mst}, Fragment::WRAP_V}}; + return {{{B}, Fragment::WRAP_V}}; case 25: if (!allow_B) return {}; - return {{{"B"_mst}, Fragment::WRAP_J}}; + return {{{B}, Fragment::WRAP_J}}; case 26: if (!allow_B) return {}; - return {{{"B"_mst}, Fragment::WRAP_N}}; + return {{{B}, Fragment::WRAP_N}}; case 27: { if (!allow_B || !IsTapscript(script_ctx)) return {}; const auto k = provider.ConsumeIntegral<uint16_t>(); @@ -528,20 +529,23 @@ struct SmartInfo { /* Construct a set of interesting type requirements to reason with (sections of BKVWzondu). */ std::vector<Type> types; + static constexpr auto B_mst{"B"_mst}, K_mst{"K"_mst}, V_mst{"V"_mst}, W_mst{"W"_mst}; + static constexpr auto d_mst{"d"_mst}, n_mst{"n"_mst}, o_mst{"o"_mst}, u_mst{"u"_mst}, z_mst{"z"_mst}; + static constexpr auto NONE_mst{""_mst}; for (int base = 0; base < 4; ++base) { /* select from B,K,V,W */ - Type type_base = base == 0 ? "B"_mst : base == 1 ? "K"_mst : base == 2 ? "V"_mst : "W"_mst; + Type type_base = base == 0 ? B_mst : base == 1 ? K_mst : base == 2 ? V_mst : W_mst; for (int zo = 0; zo < 3; ++zo) { /* select from z,o,(none) */ - Type type_zo = zo == 0 ? "z"_mst : zo == 1 ? "o"_mst : ""_mst; + Type type_zo = zo == 0 ? z_mst : zo == 1 ? o_mst : NONE_mst; for (int n = 0; n < 2; ++n) { /* select from (none),n */ if (zo == 0 && n == 1) continue; /* z conflicts with n */ if (base == 3 && n == 1) continue; /* W conflicts with n */ - Type type_n = n == 0 ? ""_mst : "n"_mst; + Type type_n = n == 0 ? NONE_mst : n_mst; for (int d = 0; d < 2; ++d) { /* select from (none),d */ if (base == 2 && d == 1) continue; /* V conflicts with d */ - Type type_d = d == 0 ? ""_mst : "d"_mst; + Type type_d = d == 0 ? NONE_mst : d_mst; for (int u = 0; u < 2; ++u) { /* select from (none),u */ if (base == 2 && u == 1) continue; /* V conflicts with u */ - Type type_u = u == 0 ? ""_mst : "u"_mst; + Type type_u = u == 0 ? NONE_mst : u_mst; Type type = type_base | type_zo | type_n | type_d | type_u; types.push_back(type); } @@ -683,7 +687,7 @@ struct SmartInfo /* Find which types are useful. The fuzzer logic only cares about constructing * B,V,K,W nodes, so any type that isn't needed in any recipe (directly or * indirectly) for the construction of those is uninteresting. */ - std::set<Type> useful_types{"B"_mst, "V"_mst, "K"_mst, "W"_mst}; + std::set<Type> useful_types{B_mst, V_mst, K_mst, W_mst}; // Find the transitive closure by adding types until the set of types does not change. while (true) { size_t set_size = useful_types.size(); diff --git a/src/test/fuzz/poolresource.cpp b/src/test/fuzz/poolresource.cpp index ce64ef6472..f764d9f8db 100644 --- a/src/test/fuzz/poolresource.cpp +++ b/src/test/fuzz/poolresource.cpp @@ -63,9 +63,9 @@ public: { if (m_total_allocated > 0x1000000) return; size_t alignment_bits = m_provider.ConsumeIntegralInRange<size_t>(0, 7); - size_t alignment = 1 << alignment_bits; + size_t alignment = size_t{1} << alignment_bits; size_t size_bits = m_provider.ConsumeIntegralInRange<size_t>(0, 16 - alignment_bits); - size_t size = m_provider.ConsumeIntegralInRange<size_t>(1U << size_bits, (1U << (size_bits + 1)) - 1U) << alignment_bits; + size_t size = m_provider.ConsumeIntegralInRange<size_t>(size_t{1} << size_bits, (size_t{1} << (size_bits + 1)) - 1U) << alignment_bits; Allocate(size, alignment); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 0b4019d5eb..9f0aedf29b 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -139,7 +139,6 @@ void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, b Assert(wtxid_in_mempool); Assert(res.m_state.IsValid()); Assert(!res.m_state.IsInvalid()); - Assert(res.m_replaced_transactions); Assert(res.m_vsize); Assert(res.m_base_fees); Assert(res.m_effective_feerate); @@ -154,7 +153,6 @@ void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, b Assert(res.m_state.IsInvalid()); const bool is_reconsiderable{res.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE}; - Assert(!res.m_replaced_transactions); Assert(!res.m_vsize); Assert(!res.m_base_fees); // Fee information is provided if the failure is TX_RECONSIDERABLE. diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 0903f987f6..e666e11758 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -143,8 +143,8 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_CHECK_EQUAL(reader.size(), 5U); BOOST_CHECK(!reader.empty()); - // Read a single byte as a signed char. - signed char b; + // Read a single byte as a int8_t. + int8_t b; reader >> b; BOOST_CHECK_EQUAL(b, -1); BOOST_CHECK_EQUAL(reader.size(), 4U); diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 71cf8aca60..870cdd0b32 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -68,12 +68,6 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns, return strprintf("tx %s unexpectedly failed: %s", wtxid.ToString(), atmp_result.m_state.ToString()); } - //m_replaced_transactions should exist iff the result was VALID - if (atmp_result.m_replaced_transactions.has_value() != valid) { - return strprintf("tx %s result should %shave m_replaced_transactions", - wtxid.ToString(), valid ? "" : "not "); - } - // m_vsize and m_base_fees should exist iff the result was VALID or MEMPOOL_ENTRY const bool mempool_entry{atmp_result.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY}; if (atmp_result.m_base_fees.has_value() != (valid || mempool_entry)) { diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index fe2d2ba592..1c02066047 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -12,6 +12,7 @@ #include <test/util/random.h> #include <test/util/setup_common.h> #include <uint256.h> +#include <util/check.h> #include <validation.h> #include <vector> @@ -102,14 +103,14 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); - Chainstate& background_cs{*[&] { + Chainstate& background_cs{*Assert([&]() -> Chainstate* { for (Chainstate* cs : chainman.GetAll()) { if (cs != &chainman.ActiveChainstate()) { return cs; } } - assert(false); - }()}; + return nullptr; + }())}; // Append the first block to the background chain. BlockValidationState state; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 82206e56c3..92055ded72 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -278,7 +278,7 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac // Convert to a vector of CTransactionRef std::vector<CTransactionRef> children_found; children_found.reserve(iters.size()); - for (const auto child_iter : iters) { + for (const auto& child_iter : iters) { children_found.emplace_back(child_iter->second.tx); } return children_found; @@ -310,7 +310,7 @@ std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDiff // Convert iterators to pair<CTransactionRef, NodeId> std::vector<std::pair<CTransactionRef, NodeId>> children_found; children_found.reserve(iters.size()); - for (const auto child_iter : iters) { + for (const auto& child_iter : iters) { children_found.emplace_back(child_iter->second.tx, child_iter->second.fromPeer); } return children_found; diff --git a/src/util/subprocess.h b/src/util/subprocess.h index 4acfa8ff83..af408b31d4 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -159,12 +159,6 @@ public: //-------------------------------------------------------------------- namespace util { - template <typename R> - inline bool is_ready(std::shared_future<R> const &f) - { - return f.wait_for(std::chrono::seconds(0)) == std::future_status::ready; - } - inline void quote_argument(const std::wstring &argument, std::wstring &command_line, bool force) { @@ -676,8 +670,8 @@ struct error * This is basically used to determine the length of the actual * data stored inside the dynamically resized vector. * - * This is what is returned as the output to communicate and check_output - * functions, so, users must know about this class. + * This is what is returned as the output to the communicate + * function, so, users must know about this class. * * OutBuffer and ErrBuffer are just different typedefs to this class. */ @@ -688,22 +682,6 @@ public: explicit Buffer(size_t cap) { buf.resize(cap); } void add_cap(size_t cap) { buf.resize(cap); } -#if 0 - Buffer(const Buffer& other): - buf(other.buf), - length(other.length) - { - std::cout << "COPY" << std::endl; - } - - Buffer(Buffer&& other): - buf(std::move(other.buf)), - length(other.length) - { - std::cout << "MOVE" << std::endl; - } -#endif - public: std::vector<char> buf; size_t length = 0; @@ -724,39 +702,9 @@ class Popen; */ 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_output' function -// wherein the user is not expected to provide an 'output' option. - -template <typename... T> struct param_pack{}; - -template <typename F, typename T> struct has_type; - -template <typename F> -struct has_type<F, param_pack<>> { - static constexpr bool value = false; -}; - -template <typename F, typename... T> -struct has_type<F, param_pack<F, T...>> { - static constexpr bool value = true; -}; - -template <typename F, typename H, typename... T> -struct has_type<F, param_pack<H,T...>> { - static constexpr bool value = - std::is_same<F, typename std::decay<H>::type>::value ? true : has_type<F, param_pack<T...>>::value; -}; - -//---- - /*! * A helper class to Popen class for setting - * options as provided in the Popen constructor - * or in check_output arguments. + * options as provided in the Popen constructor. * 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. @@ -948,24 +896,23 @@ private: * interface to the client. * * API's provided by the class: - * 1. Popen({"cmd"}, output{..}, error{..}, ....) + * Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * 2. Popen("cmd arg1"m output{..}, error{..}, ....) + * Popen("cmd arg1", 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. - * 5. pid() - PID of the spawned child. - * 6. poll() - Check the status of the running child. - * 7. kill(sig_num) - Kill the child. SIGTERM used by default. - * 8. send(...) - Send input to the input channel of the child. - * 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 - * 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 pointer. Usually used - in case of redirection. + * wait() - Wait for the child to exit. + * retcode() - The return code of the exited child. + * pid() - PID of the spawned child. + * poll() - Check the status of the running child. + * send(...) - Send input to the input channel of the child. + * communicate(...) - Get the output/error from the child and close the channels + * from the parent side. + * input() - Get the input channel/File pointer. Can be used for + * customizing the way of sending input to child. + * output() - Get the output channel/File pointer. Usually used + in case of redirection. See piping examples. + * error() - Get the error channel/File pointer. Usually used + in case of redirection. */ class Popen { @@ -1009,15 +956,6 @@ public: execute_process(); } -/* - ~Popen() - { -#ifdef __USING_WINDOWS__ - CloseHandle(this->process_handle_); -#endif - } -*/ - int pid() const noexcept { return child_pid_; } int retcode() const noexcept { return retcode_; } @@ -1026,10 +964,6 @@ public: int poll() noexcept(false); - // Does not fail, Caller is expected to recheck the - // status with a call to poll() - void kill(int sig_num = 9); - void set_out_buf_cap(size_t cap) { stream_.set_out_buf_cap(cap); } void set_err_buf_cap(size_t cap) { stream_.set_err_buf_cap(cap); } @@ -1197,18 +1131,6 @@ inline int Popen::poll() noexcept(false) #endif } -inline void Popen::kill(int sig_num) -{ -#ifdef __USING_WINDOWS__ - if (!TerminateProcess(this->process_handle_, (UINT)sig_num)) { - throw OSError("TerminateProcess", 0); - } -#else - ::kill(child_pid_, sig_num); -#endif -} - - inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ diff --git a/src/validation.h b/src/validation.h index e3b2a2d59b..28b045fe80 100644 --- a/src/validation.h +++ b/src/validation.h @@ -113,7 +113,6 @@ void PruneBlockFilesManual(Chainstate& active_chainstate, int nManualPruneHeight *| txid in mempool? | yes | no | no* | yes | yes | *| wtxid in mempool? | yes | no | no* | yes | no | *| m_state | yes, IsValid() | yes, IsInvalid() | yes, IsInvalid() | yes, IsValid() | yes, IsValid() | -*| m_replaced_transactions | yes | no | no | no | no | *| m_vsize | yes | no | no | yes | no | *| m_base_fees | yes | no | no | yes | no | *| m_effective_feerate | yes | yes | no | no | no | @@ -139,7 +138,7 @@ struct MempoolAcceptResult { const TxValidationState m_state; /** Mempool transactions replaced by the tx. */ - const std::optional<std::list<CTransactionRef>> m_replaced_transactions; + const std::list<CTransactionRef> m_replaced_transactions; /** Virtual size as used by the mempool, calculated using serialized size and sigops. */ const std::optional<int64_t> m_vsize; /** Raw base fees in satoshis. */ diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 19cbbcffdb..2842d82d80 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -13,8 +13,6 @@ The assumeutxo value generated and used here is committed to in Interesting test cases could be loading an assumeutxo snapshot file with: -- TODO: Valid hash but invalid snapshot file (bad coin height or - bad other serialization) - TODO: Valid snapshot file, but referencing a snapshot block that turns out to be invalid, or has an invalid parent - TODO: Valid snapshot file and snapshot block, but the block is not on the @@ -98,18 +96,23 @@ class AssumeutxoTest(BitcoinTestFramework): self.log.info(" - snapshot file with alternated UTXO data") cases = [ - [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b"], # wrong outpoint hash - [(1).to_bytes(4, "little"), 32, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad"], # wrong outpoint index - [b"\x81", 36, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8"], # wrong coin code VARINT((coinbase ? 1 : 0) | (height << 1)) - [b"\x80", 36, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5"], # another wrong coin code + # (content, offset, wrong_hash, custom_message) + [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b", None], # wrong outpoint hash + [(1).to_bytes(4, "little"), 32, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad", None], # wrong outpoint index + [b"\x81", 36, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None], # wrong coin code VARINT + [b"\x80", 36, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5", None], # another wrong coin code + [b"\x84\x58", 36, None, "[snapshot] bad snapshot data after deserializing 0 coins"], # wrong coin case with height 364 and coinbase 0 + [b"\xCA\xD2\x8F\x5A", 41, None, "[snapshot] bad snapshot data after deserializing 0 coins - bad tx out value"], # Amount exceeds MAX_MONEY ] - for content, offset, wrong_hash in cases: + for content, offset, wrong_hash, custom_message in cases: with open(bad_snapshot_path, "wb") as f: f.write(valid_snapshot_contents[:(32 + 8 + offset)]) f.write(content) f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):]) - expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27, got {wrong_hash}") + + log_msg = custom_message if custom_message is not None else f"[snapshot] bad snapshot content hash: expected a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27, got {wrong_hash}" + expected_error(log_msg=log_msg) def test_headers_not_synced(self, valid_snapshot_path): for node in self.nodes[1:]: diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index e85541d0ec..e7d65b4539 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -10,7 +10,6 @@ from test_framework.blocktools import ( create_block, add_witness_commitment, MAX_BLOCK_SIGOPS_WEIGHT, - WITNESS_SCALE_FACTOR, ) from test_framework.messages import ( COutPoint, @@ -20,6 +19,7 @@ from test_framework.messages import ( CTxOut, SEQUENCE_FINAL, tx_from_hex, + WITNESS_SCALE_FACTOR, ) from test_framework.script import ( ANNEX_TAG, diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 272e932fcc..b00be5f4f0 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -28,6 +28,8 @@ from test_framework.script import ( OP_HASH160, OP_RETURN, OP_TRUE, + SIGHASH_ALL, + sign_input_legacy, ) from test_framework.script_util import ( DUMMY_MIN_OP_RETURN_SCRIPT, @@ -386,5 +388,24 @@ class MempoolAcceptanceTest(BitcoinTestFramework): maxfeerate=0, ) + self.log.info('Spending a confirmed bare multisig is okay') + address = self.wallet.get_address() + tx = tx_from_hex(raw_tx_reference) + privkey, pubkey = generate_keypair() + tx.vout[0].scriptPubKey = keys_to_multisig_script([pubkey] * 3, k=1) # Some bare multisig script (1-of-3) + tx.rehash() + self.generateblock(node, address, [tx.serialize().hex()]) + tx_spend = CTransaction() + tx_spend.vin.append(CTxIn(COutPoint(tx.sha256, 0), b"")) + tx_spend.vout.append(CTxOut(tx.vout[0].nValue - int(fee*COIN), script_to_p2wsh_script(CScript([OP_TRUE])))) + tx_spend.rehash() + sign_input_legacy(tx_spend, 0, tx.vout[0].scriptPubKey, privkey, sighash_type=SIGHASH_ALL) + tx_spend.vin[0].scriptSig = bytes(CScript([OP_0])) + tx_spend.vin[0].scriptSig + self.check_mempool_result( + result_expected=[{'txid': tx_spend.rehash(), 'allowed': True, 'vsize': tx_spend.get_vsize(), 'fees': { 'base': Decimal('0.00000700')}}], + rawtxs=[tx_spend.serialize().hex()], + maxfeerate=0, + ) + if __name__ == '__main__': MempoolAcceptanceTest().main() diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 213c748eda..45bbd7f1c3 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -1054,7 +1054,7 @@ class SegWitTest(BitcoinTestFramework): @subtest def test_max_witness_push_length(self): - """Test that witness stack can only allow up to 520 byte pushes.""" + """Test that witness stack can only allow up to MAX_SCRIPT_ELEMENT_SIZE byte pushes.""" block = self.build_next_block() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index cfd923bab3..f0dc866f69 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -28,6 +28,7 @@ from .messages import ( ser_uint256, tx_from_hex, uint256_from_str, + WITNESS_SCALE_FACTOR, ) from .script import ( CScript, @@ -45,7 +46,6 @@ from .script_util import ( ) from .util import assert_equal -WITNESS_SCALE_FACTOR = 4 MAX_BLOCK_SIGOPS = 20000 MAX_BLOCK_SIGOPS_WEIGHT = MAX_BLOCK_SIGOPS * WITNESS_SCALE_FACTOR diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index e22e047e4b..f054f99011 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -137,9 +137,9 @@ fn lint_trailing_whitespace() -> LintResult { if trailing_space { Err(r#" ^^^ -Trailing whitespace is problematic, because git may warn about it, or editors may remove it by -default, forcing developers in the future to either undo the changes manually or spend time on -review. +Trailing whitespace (including Windows line endings [CR LF]) is problematic, because git may warn +about it, or editors may remove it by default, forcing developers in the future to either undo the +changes manually or spend time on review. Thus, it is best to remove the trailing space now. |