diff options
80 files changed, 1033 insertions, 410 deletions
diff --git a/Makefile.am b/Makefile.am index 2ff6dd0a11..b746299a42 100644 --- a/Makefile.am +++ b/Makefile.am @@ -334,14 +334,14 @@ clean-local: clean-docs test-security-check: if TARGET_DARWIN - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO endif if TARGET_WINDOWS - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE endif if TARGET_LINUX - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF - $(AM_V_at) CC='$(CC)' CFLAGS='$(CFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF + $(AM_V_at) CXX='$(CXX)' CXXFLAGS='$(CXXFLAGS)' CPPFLAGS='$(CPPFLAGS)' LDFLAGS='$(LDFLAGS)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF endif diff --git a/configure.ac b/configure.ac index af0d1d1505..439bb508e7 100644 --- a/configure.ac +++ b/configure.ac @@ -482,11 +482,12 @@ TEMP_CXXFLAGS="$CXXFLAGS" CXXFLAGS="$SSE41_CXXFLAGS $CXXFLAGS" AC_MSG_CHECKING([for SSE4.1 intrinsics]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #include <stdint.h> #include <immintrin.h> ]],[[ - __m128i l = _mm_set1_epi32(0); - return _mm_extract_epi32(l, 3); + __m128i a = _mm_set1_epi32(0); + __m128i b = _mm_set1_epi32(1); + __m128i r = _mm_blend_epi16(a, b, 0xFF); + return _mm_extract_epi32(r, 3); ]])], [ AC_MSG_RESULT([yes]); enable_sse41=yes; AC_DEFINE([ENABLE_SSE41], [1], [Define this symbol to build code that uses SSE4.1 intrinsics]) ], [ AC_MSG_RESULT([no])] @@ -1317,6 +1318,7 @@ if test "$use_reduce_exports" = "yes"; then AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [CORE_CXXFLAGS="$CORE_CXXFLAGS -fvisibility=hidden"], [AC_MSG_ERROR([Cannot set hidden symbol visibility. Use --disable-reduce-exports.])], [$CXXFLAG_WERROR]) AX_CHECK_LINK_FLAG([-Wl,--exclude-libs,ALL], [RELDFLAGS="-Wl,--exclude-libs,ALL"], [], [$LDFLAG_WERROR]) + AX_CHECK_LINK_FLAG([-Wl,-no_exported_symbols], [LIBTOOL_APP_LDFLAGS="$LIBTOOL_APP_LDFLAGS -Wl,-no_exported_symbols"], [], [$LDFLAG_WERROR]) fi if test "$use_tests" = "yes"; then diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 7bfd4d98da..de372cbd39 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -15,10 +15,10 @@ from utils import determine_wellknown_cmd def write_testcode(filename): with open(filename, 'w', encoding="utf8") as f: f.write(''' - #include <stdio.h> + #include <cstdio> int main() { - printf("the quick brown fox jumps over the lazy god\\n"); + std::printf("the quick brown fox jumps over the lazy god\\n"); return 0; } ''') @@ -34,17 +34,17 @@ def env_flags() -> list[str]: # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for # reference. flags: list[str] = [] - for var in ['CFLAGS', 'CPPFLAGS', 'LDFLAGS']: + for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']: flags += filter(None, os.environ.get(var, '').split(' ')) return flags -def call_security_check(cc: str, source: str, executable: str, options) -> tuple: - subprocess.run([*cc,source,'-o',executable] + env_flags() + options, check=True) +def call_security_check(cxx: str, source: str, executable: str, options) -> tuple: + subprocess.run([*cxx,source,'-o',executable] + env_flags() + options, check=True) p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, text=True) return (p.returncode, p.stdout.rstrip()) -def get_arch(cc, source, executable): - subprocess.run([*cc, source, '-o', executable] + env_flags(), check=True) +def get_arch(cxx, source, executable): + subprocess.run([*cxx, source, '-o', executable] + env_flags(), check=True) binary = lief.parse(executable) arch = binary.abstract.header.architecture os.remove(executable) @@ -52,93 +52,93 @@ def get_arch(cc, source, executable): class TestSecurityChecks(unittest.TestCase): def test_ELF(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1' - cc = determine_wellknown_cmd('CC', 'gcc') + cxx = determine_wellknown_cmd('CXX', 'g++') write_testcode(source) - arch = get_arch(cc, source, executable) + arch = get_arch(cxx, source, executable) if arch == lief.ARCHITECTURES.X86: - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE NX RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), (1, executable+': failed separate_code CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']), (0, '')) else: - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE NX RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), (1, executable+': failed separate_code')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), (0, '')) clean_files(source, executable) def test_PE(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1.exe' - cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') + cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++') write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--disable-nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fno-stack-protector']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION CONTROL_FLOW Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-no-pie','-fno-PIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--disable-dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA CONTROL_FLOW')) # -pie -fPIE does nothing unless --dynamicbase is also supplied - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--disable-high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), (0, '')) clean_files(source, executable) def test_MACHO(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1' - cc = determine_wellknown_cmd('CC', 'clang') + cxx = determine_wellknown_cmd('CXX', 'clang++') write_testcode(source) - arch = get_arch(cc, source, executable) + arch = get_arch(cxx, source, executable) if arch == lief.ARCHITECTURES.X86: - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS PIE CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']), (1, executable+': failed NOUNDEFS Canary CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']), (1, executable+': failed NOUNDEFS CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-fcf-protection=full', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-fcf-protection=full', '-Wl,-fixup_chains']), (0, '')) else: # arm64 darwin doesn't support non-PIE binaries, control flow or executable stacks - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS BRANCH_PROTECTION')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), + self.assertEqual(call_security_check(cxx, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS')) - self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), + self.assertEqual(call_security_check(cxx, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (0, '')) diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index b00004586c..454dbc6bd2 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -11,17 +11,17 @@ import unittest from utils import determine_wellknown_cmd -def call_symbol_check(cc: list[str], source, executable, options): +def call_symbol_check(cxx: list[str], source, executable, options): # This should behave the same as AC_TRY_LINK, so arrange well-known flags # in the same order as autoconf would. # # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for # reference. env_flags: list[str] = [] - for var in ['CFLAGS', 'CPPFLAGS', 'LDFLAGS']: + for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']: env_flags += filter(None, os.environ.get(var, '').split(' ')) - subprocess.run([*cc,source,'-o',executable] + env_flags + options, check=True) + subprocess.run([*cxx,source,'-o',executable] + env_flags + options, check=True) p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, text=True) os.remove(source) os.remove(executable) @@ -29,13 +29,13 @@ def call_symbol_check(cc: list[str], source, executable, options): class TestSymbolChecks(unittest.TestCase): def test_ELF(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1' - cc = determine_wellknown_cmd('CC', 'gcc') + cxx = determine_wellknown_cmd('CXX', 'g++') # -lutil is part of the libc6 package so a safe bet that it's installed # it's also out of context enough that it's unlikely to ever become a real dependency - source = 'test2.c' + source = 'test2.cpp' executable = 'test2' with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -48,31 +48,31 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-lutil']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-lutil']), (1, executable + ': libutil.so.1 is not in ALLOWED_LIBRARIES!\n' + executable + ': failed LIBRARY_DEPENDENCIES')) # finally, check a simple conforming binary - source = 'test3.c' + source = 'test3.cpp' executable = 'test3' with open(source, 'w', encoding="utf8") as f: f.write(''' - #include <stdio.h> + #include <cstdio> int main() { - printf("42"); + std::printf("42"); return 0; } ''') - self.assertEqual(call_symbol_check(cc, source, executable, []), + self.assertEqual(call_symbol_check(cxx, source, executable, []), (0, '')) def test_MACHO(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1' - cc = determine_wellknown_cmd('CC', 'clang') + cxx = determine_wellknown_cmd('CXX', 'clang++') with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -86,11 +86,11 @@ class TestSymbolChecks(unittest.TestCase): ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), (1, 'libexpat.1.dylib is not in ALLOWED_LIBRARIES!\n' + f'{executable}: failed DYNAMIC_LIBRARIES MIN_OS SDK')) - source = 'test2.c' + source = 'test2.cpp' executable = 'test2' with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -103,10 +103,10 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), (1, f'{executable}: failed MIN_OS SDK')) - source = 'test3.c' + source = 'test3.cpp' executable = 'test3' with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -116,13 +116,13 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,11.0', '-Wl,11.4']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,11.0', '-Wl,11.4']), (1, f'{executable}: failed SDK')) def test_PE(self): - source = 'test1.c' + source = 'test1.cpp' executable = 'test1.exe' - cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') + cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++') with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -135,11 +135,11 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-lpdh', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-lpdh', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']), (1, 'pdh.dll is not in ALLOWED_LIBRARIES!\n' + executable + ': failed DYNAMIC_LIBRARIES')) - source = 'test2.c' + source = 'test2.cpp' executable = 'test2.exe' with open(source, 'w', encoding="utf8") as f: @@ -150,10 +150,10 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']), (1, executable + ': failed SUBSYSTEM_VERSION')) - source = 'test3.c' + source = 'test3.cpp' executable = 'test3.exe' with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -166,7 +166,7 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-lole32', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']), + self.assertEqual(call_symbol_check(cxx, source, executable, ['-lole32', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,1']), (0, '')) diff --git a/doc/policy/packages.md b/doc/policy/packages.md index a220bdd17f..9b321799f1 100644 --- a/doc/policy/packages.md +++ b/doc/policy/packages.md @@ -38,11 +38,11 @@ The following rules are enforced for all packages: * Only limited package replacements are currently considered. (#28984) - - All direct conflicts must signal replacement (or have `-mempoolfullrbf=1` set). + - All direct conflicts must signal replacement (or the node must have `-mempoolfullrbf=1` set). - Packages are 1-parent-1-child, with no in-mempool ancestors of the package. - - All conflicting clusters(connected components of mempool transactions) must be clusters of up to size 2. + - All conflicting clusters (connected components of mempool transactions) must be clusters of up to size 2. - No more than MAX_REPLACEMENT_CANDIDATES transactions can be replaced, analogous to regular [replacement rule](./mempool-replacements.md) 5). @@ -56,7 +56,7 @@ The following rules are enforced for all packages: - *Rationale*: Basic support for package RBF can be used by wallets by making chains of no longer than two, then directly conflicting - those chains when needed. Combined with V3 transactions this can + those chains when needed. Combined with TRUC transactions this can result in more robust fee bumping. More general package RBF may be enabled in the future. diff --git a/src/Makefile.am b/src/Makefile.am index 0ae5effdbe..72dd942c40 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,15 +51,15 @@ LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE) if ENABLE_SSE41 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41) +if ENABLE_X86_SHANI +LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la +LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI) +endif endif if ENABLE_AVX2 LIBBITCOIN_CRYPTO_AVX2 = crypto/libbitcoin_crypto_avx2.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_AVX2) endif -if ENABLE_X86_SHANI -LIBBITCOIN_CRYPTO_X86_SHANI = crypto/libbitcoin_crypto_x86_shani.la -LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_X86_SHANI) -endif if ENABLE_ARM_SHANI LIBBITCOIN_CRYPTO_ARM_SHANI = crypto/libbitcoin_crypto_arm_shani.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_ARM_SHANI) @@ -622,7 +622,7 @@ crypto_libbitcoin_crypto_x86_shani_la_LDFLAGS = $(AM_LDFLAGS) -static crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS = $(AM_CPPFLAGS) crypto_libbitcoin_crypto_x86_shani_la_CXXFLAGS += $(X86_SHANI_CXXFLAGS) -crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_X86_SHANI +crypto_libbitcoin_crypto_x86_shani_la_CPPFLAGS += -DENABLE_SSE41 -DENABLE_X86_SHANI crypto_libbitcoin_crypto_x86_shani_la_SOURCES = crypto/sha256_x86_shani.cpp # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index cd2626b330..7e3aa369c7 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -54,6 +54,7 @@ bench_bench_bitcoin_SOURCES = \ bench/rollingbloom.cpp \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ + bench/sign_transaction.cpp \ bench/streams_findbyte.cpp \ bench/strencodings.cpp \ bench/util_time.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 8651eea942..0993a65eff 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -309,6 +309,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/crypto_aes256.cpp \ test/fuzz/crypto_aes256cbc.cpp \ test/fuzz/crypto_chacha20.cpp \ + test/fuzz/crypto_chacha20poly1305.cpp \ test/fuzz/crypto_common.cpp \ test/fuzz/crypto_diff_fuzz_chacha20.cpp \ test/fuzz/crypto_hkdf_hmac_sha256_l32.cpp \ diff --git a/src/bench/logging.cpp b/src/bench/logging.cpp index c97c4e151b..8a745a0ba7 100644 --- a/src/bench/logging.cpp +++ b/src/bench/logging.cpp @@ -20,7 +20,7 @@ static void Logging(benchmark::Bench& bench, const std::vector<const char*>& ext TestingSetup test_setup{ ChainType::REGTEST, - extra_args, + {.extra_args = extra_args}, }; bench.run([&] { log(); }); diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index fe3e204fb3..3c82f55c19 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -106,7 +106,7 @@ static void ComplexMemPool(benchmark::Bench& bench) static void MempoolCheck(benchmark::Bench& bench) { FastRandomContext det_rand{true}; - auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {"-checkmempool=1"}); + auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {.extra_args = {"-checkmempool=1"}}); CTxMemPool& pool = *testing_setup.get()->m_node.mempool; LOCK2(cs_main, pool.cs); testing_setup->PopulateMempool(det_rand, 400, true); diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp new file mode 100644 index 0000000000..7a2e26e339 --- /dev/null +++ b/src/bench/sign_transaction.cpp @@ -0,0 +1,70 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <bench/bench.h> +#include <addresstype.h> +#include <coins.h> +#include <key.h> +#include <primitives/transaction.h> +#include <pubkey.h> +#include <script/interpreter.h> +#include <script/script.h> +#include <script/sign.h> +#include <uint256.h> +#include <util/translation.h> + +enum class InputType { + P2WPKH, // segwitv0, witness-pubkey-hash (ECDSA signature) + P2TR, // segwitv1, taproot key-path spend (Schnorr signature) +}; + +static void SignTransactionSingleInput(benchmark::Bench& bench, InputType input_type) +{ + ECC_Context ecc_context{}; + + FlatSigningProvider keystore; + std::vector<CScript> prev_spks; + + // Create a bunch of keys / UTXOs to avoid signing with the same key repeatedly + for (int i = 0; i < 32; i++) { + CKey privkey = GenerateRandomKey(); + CPubKey pubkey = privkey.GetPubKey(); + CKeyID key_id = pubkey.GetID(); + keystore.keys.emplace(key_id, privkey); + keystore.pubkeys.emplace(key_id, pubkey); + + // Create specified locking script type + CScript prev_spk; + switch (input_type) { + case InputType::P2WPKH: prev_spk = GetScriptForDestination(WitnessV0KeyHash(pubkey)); break; + case InputType::P2TR: prev_spk = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey{pubkey})); break; + default: assert(false); + } + prev_spks.push_back(prev_spk); + } + + // Simple 1-input tx with artificial outpoint + // (note that for the purpose of signing with SIGHASH_ALL we don't need any outputs) + COutPoint prevout{/*hashIn=*/Txid::FromUint256(uint256::ONE), /*nIn=*/1337}; + CMutableTransaction unsigned_tx; + unsigned_tx.vin.emplace_back(prevout); + + // Benchmark. + int iter = 0; + bench.minEpochIterations(100).run([&] { + CMutableTransaction tx{unsigned_tx}; + std::map<COutPoint, Coin> coins; + CScript prev_spk = prev_spks[(iter++) % prev_spks.size()]; + coins[prevout] = Coin(CTxOut(10000, prev_spk), /*nHeightIn=*/100, /*fCoinBaseIn=*/false); + std::map<int, bilingual_str> input_errors; + bool complete = SignTransaction(tx, &keystore, coins, SIGHASH_ALL, input_errors); + assert(complete); + }); +} + +static void SignTransactionECDSA(benchmark::Bench& bench) { SignTransactionSingleInput(bench, InputType::P2WPKH); } +static void SignTransactionSchnorr(benchmark::Bench& bench) { SignTransactionSingleInput(bench, InputType::P2TR); } + +BENCHMARK(SignTransactionECDSA, benchmark::PriorityLevel::HIGH); +BENCHMARK(SignTransactionSchnorr, benchmark::PriorityLevel::HIGH); diff --git a/src/core_read.cpp b/src/core_read.cpp index 114f92fc45..0ba271a8d2 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex, uint256& result) util::Result<int> SighashFromStr(const std::string& sighash) { - static std::map<std::string, int> map_sighash_values = { + static const std::map<std::string, int> map_sighash_values = { {std::string("DEFAULT"), int(SIGHASH_DEFAULT)}, {std::string("ALL"), int(SIGHASH_ALL)}, {std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)}, diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index c883bd2f03..89d7204808 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -621,7 +621,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem } } -#if defined(ENABLE_X86_SHANI) +#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI) if (have_x86_shani) { Transform = sha256_x86_shani::Transform; TransformD64 = TransformD64Wrapper<sha256_x86_shani::Transform>; diff --git a/src/crypto/sha256_x86_shani.cpp b/src/crypto/sha256_x86_shani.cpp index 79871bfcc1..7471828193 100644 --- a/src/crypto/sha256_x86_shani.cpp +++ b/src/crypto/sha256_x86_shani.cpp @@ -6,7 +6,7 @@ // Written and placed in public domain by Jeffrey Walton. // Based on code from Intel, and by Sean Gulley for the miTLS project. -#ifdef ENABLE_X86_SHANI +#if defined(ENABLE_SSE41) && defined(ENABLE_X86_SHANI) #include <stdint.h> #include <immintrin.h> diff --git a/src/init.cpp b/src/init.cpp index e4b65fbfa9..9e570d6128 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -296,10 +296,11 @@ void Shutdown(NodeContext& node) StopTorControl(); + if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); // After everything has been shut down, but before things get flushed, stop the - // scheduler and load block thread. + // the scheduler. After this point, SyncWithValidationInterfaceQueue() should not be called anymore + // as this would prevent the shutdown from completing. if (node.scheduler) node.scheduler->stop(); - if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); // After the threads that potentially access these pointers have been stopped, // destruct and reset all to nullptr. @@ -1880,6 +1881,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) CService onion_service_target; if (!connOptions.onion_binds.empty()) { onion_service_target = connOptions.onion_binds.front(); + } else if (!connOptions.vBinds.empty()) { + onion_service_target = connOptions.vBinds.front(); } else { onion_service_target = DefaultOnionServiceTarget(); connOptions.onion_binds.push_back(onion_service_target); diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 974490561a..cebe97edb7 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -5,9 +5,11 @@ #ifndef BITCOIN_INTERFACES_MINING_H #define BITCOIN_INTERFACES_MINING_H +#include <node/types.h> +#include <uint256.h> + #include <memory> #include <optional> -#include <uint256.h> namespace node { struct CBlockTemplate; @@ -41,10 +43,10 @@ public: * Construct a new block template * * @param[in] script_pub_key the coinbase output - * @param[in] use_mempool set false to omit mempool transactions + * @param[in] options options for creating the block * @returns a block template */ - virtual std::unique_ptr<node::CBlockTemplate> createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0; + virtual std::unique_ptr<node::CBlockTemplate> createNewBlock(const CScript& script_pub_key, const node::BlockCreateOptions& options={}) = 0; /** * Processes new block. A valid new block is automatically relayed to peers. diff --git a/src/interfaces/node.h b/src/interfaces/node.h index a56e79148d..b9b2306ce3 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -162,6 +162,9 @@ public: //! Get mempool dynamic usage. virtual size_t getMempoolDynamicUsage() = 0; + //! Get mempool maximum memory usage. + virtual size_t getMempoolMaxUsage() = 0; + //! Get header tip height and time. virtual bool getHeaderTip(int& height, int64_t& block_time) = 0; diff --git a/src/net.cpp b/src/net.cpp index d265d78548..3d3f9f4ba7 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3202,24 +3202,36 @@ bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlag bool CConnman::InitBinds(const Options& options) { - bool fBound = false; for (const auto& addrBind : options.vBinds) { - fBound |= Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None); + if (!Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None)) { + return false; + } } for (const auto& addrBind : options.vWhiteBinds) { - fBound |= Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags); + if (!Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags)) { + return false; + } } for (const auto& addr_bind : options.onion_binds) { - fBound |= Bind(addr_bind, BF_DONT_ADVERTISE, NetPermissionFlags::None); + if (!Bind(addr_bind, BF_REPORT_ERROR | BF_DONT_ADVERTISE, NetPermissionFlags::None)) { + return false; + } } if (options.bind_on_any) { + // Don't consider errors to bind on IPv6 "::" fatal because the host OS + // may not have IPv6 support and the user did not explicitly ask us to + // bind on that. + const CService ipv6_any{in6_addr(IN6ADDR_ANY_INIT), GetListenPort()}; // :: + Bind(ipv6_any, BF_NONE, NetPermissionFlags::None); + struct in_addr inaddr_any; inaddr_any.s_addr = htonl(INADDR_ANY); - struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT; - fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::None); - fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::None); + const CService ipv4_any{inaddr_any, GetListenPort()}; // 0.0.0.0 + if (!Bind(ipv4_any, BF_REPORT_ERROR, NetPermissionFlags::None)) { + return false; + } } - return fBound; + return true; } bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) @@ -991,8 +991,8 @@ public: /** Mutex for anything that is only accessed via the msg processing thread */ static Mutex g_msgproc_mutex; - /** Initialize a peer (setup state, queue any initial messages) */ - virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0; + /** Initialize a peer (setup state) */ + virtual void InitializeNode(const CNode& node, ServiceFlags our_services) = 0; /** Handle removal of a peer (clear state) */ virtual void FinalizeNode(const CNode& node) = 0; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2764562f57..d674758abd 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -243,6 +243,9 @@ struct Peer { * Most peers use headers-first syncing, which doesn't use this mechanism */ uint256 m_continuation_block GUARDED_BY(m_block_inv_mutex) {}; + /** Set to true once initial VERSION message was sent (only relevant for outbound peers). */ + bool m_outbound_version_message_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; + /** This peer's reported block height when we connected */ std::atomic<int> m_starting_height{-1}; @@ -498,7 +501,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); /** Implement NetEventsInterface */ - void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void InitializeNode(const CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); bool HasAllDesirableServiceFlags(ServiceFlags services) const override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override @@ -1659,7 +1662,7 @@ void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_s if (state) state->m_last_block_announcement = time_in_seconds; } -void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) +void PeerManagerImpl::InitializeNode(const CNode& node, ServiceFlags our_services) { NodeId nodeid = node.GetId(); { @@ -1677,9 +1680,6 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); } - if (!node.IsInboundConn()) { - PushNodeVersion(node, *peer); - } } void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) @@ -5326,6 +5326,10 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt PeerRef peer = GetPeerRef(pfrom->GetId()); if (peer == nullptr) return false; + // For outbound connections, ensure that the initial VERSION message + // has been sent first before processing any incoming messages + if (!pfrom->IsInboundConn() && !peer->m_outbound_version_message_sent) return false; + { LOCK(peer->m_getdata_requests_mutex); if (!peer->m_getdata_requests.empty()) { @@ -5817,6 +5821,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // disconnect misbehaving peers even before the version handshake is complete. if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true; + // Initiate version handshake for outbound connections + if (!pto->IsInboundConn() && !peer->m_outbound_version_message_sent) { + PushNodeVersion(*pto, *peer); + peer->m_outbound_version_message_sent = true; + } + // Don't send anything until the version handshake is complete if (!pto->fSuccessfullyConnected || pto->fDisconnect) return true; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 80aceb312a..c50625f58d 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -588,7 +588,7 @@ const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) return nullptr; } -bool BlockManager::IsBlockPruned(const CBlockIndex& block) +bool BlockManager::IsBlockPruned(const CBlockIndex& block) const { AssertLockHeld(::cs_main); return m_have_pruned && !(block.nStatus & BLOCK_HAVE_DATA) && (block.nTx > 0); @@ -703,15 +703,10 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in { const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; - if (pos.IsNull()) { - LogError("%s: no undo data available\n", __func__); - return false; - } - // Open history file to read AutoFile filein{OpenUndoFile(pos, true)}; if (filein.IsNull()) { - LogError("%s: OpenUndoFile failed\n", __func__); + LogError("%s: OpenUndoFile failed for %s\n", __func__, pos.ToString()); return false; } @@ -723,13 +718,13 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in verifier >> blockundo; filein >> hashChecksum; } catch (const std::exception& e) { - LogError("%s: Deserialize or I/O error - %s\n", __func__, e.what()); + LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString()); return false; } // Verify checksum if (hashChecksum != verifier.GetHash()) { - LogError("%s: Checksum mismatch\n", __func__); + LogError("%s: Checksum mismatch at %s\n", __func__, pos.ToString()); return false; } @@ -986,7 +981,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const // Open history file to append AutoFile fileout{OpenBlockFile(pos)}; if (fileout.IsNull()) { - LogError("WriteBlockToDisk: OpenBlockFile failed\n"); + LogError("%s: OpenBlockFile failed\n", __func__); return false; } @@ -997,7 +992,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const // Write block long fileOutPos = ftell(fileout.Get()); if (fileOutPos < 0) { - LogError("WriteBlockToDisk: ftell failed\n"); + LogError("%s: ftell failed\n", __func__); return false; } pos.nPos = (unsigned int)fileOutPos; @@ -1016,7 +1011,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid if (block.GetUndoPos().IsNull()) { FlatFilePos _pos; if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) { - LogError("ConnectBlock(): FindUndoPos failed\n"); + LogError("%s: FindUndoPos failed\n", __func__); return false; } if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) { @@ -1055,7 +1050,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons // Open history file to read AutoFile filein{OpenBlockFile(pos, true)}; if (filein.IsNull()) { - LogError("ReadBlockFromDisk: OpenBlockFile failed for %s\n", pos.ToString()); + LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); return false; } @@ -1069,13 +1064,13 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons // Check the header if (!CheckProofOfWork(block.GetHash(), block.nBits, GetConsensus())) { - LogError("ReadBlockFromDisk: Errors in block header at %s\n", pos.ToString()); + LogError("%s: Errors in block header at %s\n", __func__, pos.ToString()); return false; } // Signet only: check block solution if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) { - LogError("ReadBlockFromDisk: Errors in block solution at %s\n", pos.ToString()); + LogError("%s: Errors in block solution at %s\n", __func__, pos.ToString()); return false; } @@ -1090,8 +1085,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co return false; } if (block.GetHash() != index.GetBlockHash()) { - LogError("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s\n", - index.ToString(), block_pos.ToString()); + LogError("%s: GetHash() doesn't match index for %s at %s\n", __func__, index.ToString(), block_pos.ToString()); return false; } return true; diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 0a46d79764..a946b4ea94 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -404,7 +404,7 @@ public: bool m_have_pruned = false; //! Check whether the block associated with this index entry is pruned or not. - bool IsBlockPruned(const CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool IsBlockPruned(const CBlockIndex& block) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Create or update a prune lock identified by its name void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index fa151407fa..46d36f83f8 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -278,6 +278,7 @@ public: int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; } size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; } size_t getMempoolDynamicUsage() override { return m_context->mempool ? m_context->mempool->DynamicMemoryUsage() : 0; } + size_t getMempoolMaxUsage() override { return m_context->mempool ? m_context->mempool->m_opts.max_size_bytes : 0; } bool getHeaderTip(int& height, int64_t& block_time) override { LOCK(::cs_main); @@ -883,12 +884,11 @@ public: return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root); } - std::unique_ptr<CBlockTemplate> createNewBlock(const CScript& script_pub_key, bool use_mempool) override + std::unique_ptr<CBlockTemplate> createNewBlock(const CScript& script_pub_key, const BlockCreateOptions& options) override { - BlockAssembler::Options options; - ApplyArgsManOptions(gArgs, options); - - return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr, options}.CreateNewBlock(script_pub_key); + BlockAssembler::Options assemble_options{options}; + ApplyArgsManOptions(*Assert(m_node.args), assemble_options); + return BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key); } NodeContext* context() override { return &m_node; } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 291f1d5fc7..fa2d979b86 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -59,14 +59,17 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) static BlockAssembler::Options ClampOptions(BlockAssembler::Options options) { - // Limit weight to between 4K and DEFAULT_BLOCK_MAX_WEIGHT for sanity: - options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, 4000, DEFAULT_BLOCK_MAX_WEIGHT); + Assert(options.coinbase_max_additional_weight <= DEFAULT_BLOCK_MAX_WEIGHT); + Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST); + // Limit weight to between coinbase_max_additional_weight and DEFAULT_BLOCK_MAX_WEIGHT for sanity: + // Coinbase (reserved) outputs can safely exceed -blockmaxweight, but the rest of the block template will be empty. + options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.coinbase_max_additional_weight, DEFAULT_BLOCK_MAX_WEIGHT); return options; } BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options) : chainparams{chainstate.m_chainman.GetParams()}, - m_mempool{mempool}, + m_mempool{options.use_mempool ? mempool : nullptr}, m_chainstate{chainstate}, m_options{ClampOptions(options)} { @@ -87,8 +90,8 @@ void BlockAssembler::resetBlock() inBlock.clear(); // Reserve space for coinbase tx - nBlockWeight = 4000; - nBlockSigOpsCost = 400; + nBlockWeight = m_options.coinbase_max_additional_weight; + nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops; // These counters do not include coinbase tx nBlockTx = 0; @@ -379,7 +382,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele ++nConsecutiveFailed; if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight > - m_options.nBlockMaxWeight - 4000) { + m_options.nBlockMaxWeight - m_options.coinbase_max_additional_weight) { // Give up if we're close to full and haven't succeeded in a while break; } diff --git a/src/node/miner.h b/src/node/miner.h index 622ca16c8f..efd773eb31 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_NODE_MINER_H #define BITCOIN_NODE_MINER_H +#include <node/types.h> #include <policy/policy.h> #include <primitives/block.h> #include <txmempool.h> @@ -153,7 +154,7 @@ private: Chainstate& m_chainstate; public: - struct Options { + struct Options : BlockCreateOptions { // Configuration parameters for the block size size_t nBlockMaxWeight{DEFAULT_BLOCK_MAX_WEIGHT}; CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE}; diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 58422c4439..d7d15554b3 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -174,7 +174,7 @@ MiniMiner::MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries, SanityCheck(); } -// Compare by min(ancestor feerate, individual feerate), then iterator +// Compare by min(ancestor feerate, individual feerate), then txid // // Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate // parents do not incentive inclusion of their children. Therefore the mining algorithm only considers @@ -183,21 +183,13 @@ struct AncestorFeerateComparator { template<typename I> bool operator()(const I& a, const I& b) const { - auto min_feerate = [](const MiniMinerMempoolEntry& e) -> CFeeRate { - const CAmount ancestor_fee{e.GetModFeesWithAncestors()}; - const int64_t ancestor_size{e.GetSizeWithAncestors()}; - const CAmount tx_fee{e.GetModifiedFee()}; - const int64_t tx_size{e.GetTxSize()}; - // Comparing ancestor feerate with individual feerate: - // ancestor_fee / ancestor_size <= tx_fee / tx_size - // Avoid division and possible loss of precision by - // multiplying both sides by the sizes: - return ancestor_fee * tx_size < tx_fee * ancestor_size ? - CFeeRate(ancestor_fee, ancestor_size) : - CFeeRate(tx_fee, tx_size); + auto min_feerate = [](const MiniMinerMempoolEntry& e) -> FeeFrac { + FeeFrac self_feerate(e.GetModifiedFee(), e.GetTxSize()); + FeeFrac ancestor_feerate(e.GetModFeesWithAncestors(), e.GetSizeWithAncestors()); + return std::min(ancestor_feerate, self_feerate); }; - CFeeRate a_feerate{min_feerate(a->second)}; - CFeeRate b_feerate{min_feerate(b->second)}; + FeeFrac a_feerate{min_feerate(a->second)}; + FeeFrac b_feerate{min_feerate(b->second)}; if (a_feerate != b_feerate) { return a_feerate > b_feerate; } diff --git a/src/node/types.h b/src/node/types.h index 0461e85f43..bb8d17ef43 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -13,6 +13,8 @@ #ifndef BITCOIN_NODE_TYPES_H #define BITCOIN_NODE_TYPES_H +#include <cstddef> + namespace node { enum class TransactionError { OK, //!< No error @@ -24,6 +26,24 @@ enum class TransactionError { MAX_BURN_EXCEEDED, INVALID_PACKAGE, }; + +struct BlockCreateOptions { + /** + * Set false to omit mempool transactions + */ + bool use_mempool{true}; + /** + * The maximum additional weight which the pool will add to the coinbase + * scriptSig, witness and outputs. This must include any additional + * weight needed for larger CompactSize encoded lengths. + */ + size_t coinbase_max_additional_weight{4000}; + /** + * The maximum additional sigops which the pool will add in coinbase + * transaction outputs. + */ + size_t coinbase_output_max_additional_sigops{400}; +}; } // namespace node #endif // BITCOIN_NODE_TYPES_H diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 2f3bad37e6..0b03e3071c 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -53,7 +53,7 @@ ClientModel::ClientModel(interfaces::Node& node, OptionsModel *_optionsModel, QO connect(timer, &QTimer::timeout, [this] { // no locking required at this point // the following calls will acquire the required lock - Q_EMIT mempoolSizeChanged(m_node.getMempoolSize(), m_node.getMempoolDynamicUsage()); + Q_EMIT mempoolSizeChanged(m_node.getMempoolSize(), m_node.getMempoolDynamicUsage(), m_node.getMempoolMaxUsage()); Q_EMIT bytesChanged(m_node.getTotalBytesRecv(), m_node.getTotalBytesSent()); }); connect(m_thread, &QThread::finished, timer, &QObject::deleteLater); diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index 624056b5df..7727359f99 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -113,7 +113,7 @@ private: Q_SIGNALS: void numConnectionsChanged(int count); void numBlocksChanged(int count, const QDateTime& blockDate, double nVerificationProgress, SyncType header, SynchronizationState sync_state); - void mempoolSizeChanged(long count, size_t mempoolSizeInBytes); + void mempoolSizeChanged(long count, size_t mempoolSizeInBytes, size_t mempoolMaxSizeInBytes); void networkActiveChanged(bool networkActive); void alertsChanged(const QString &warnings); void bytesChanged(quint64 totalBytesIn, quint64 totalBytesOut); diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index 7bc6ccdc49..7580f6b47a 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -25,6 +25,7 @@ ModalOverlay::ModalOverlay(bool enable_wallet, QWidget* parent) parent->installEventFilter(this); raise(); } + ui->closeButton->installEventFilter(this); blockProcessTime.clear(); setVisible(false); @@ -60,6 +61,11 @@ bool ModalOverlay::eventFilter(QObject * obj, QEvent * ev) { raise(); } } + + if (obj == ui->closeButton && ev->type() == QEvent::FocusOut && layerIsVisible) { + ui->closeButton->setFocus(Qt::OtherFocusReason); + } + return QWidget::eventFilter(obj, ev); } @@ -187,6 +193,10 @@ void ModalOverlay::showHide(bool hide, bool userRequested) m_animation.setEndValue(QPoint(0, hide ? height() : 0)); m_animation.start(QAbstractAnimation::KeepWhenStopped); layerIsVisible = !hide; + + if (layerIsVisible) { + ui->closeButton->setFocus(Qt::OtherFocusReason); + } } void ModalOverlay::closeClicked() diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index ee53a59bb5..949b1b7775 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -92,6 +92,8 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet) { ui->setupUi(this); + ui->verticalLayout->setStretchFactor(ui->tabWidget, 1); + /* Main elements init */ ui->databaseCache->setMinimum(nMinDbCache); ui->databaseCache->setMaximum(nMaxDbCache); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index edf417a7cb..fb731e4e90 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1000,15 +1000,16 @@ void RPCConsole::setNumBlocks(int count, const QDateTime& blockDate, double nVer } } -void RPCConsole::setMempoolSize(long numberOfTxs, size_t dynUsage) +void RPCConsole::setMempoolSize(long numberOfTxs, size_t dynUsage, size_t maxUsage) { ui->mempoolNumberTxs->setText(QString::number(numberOfTxs)); - if (dynUsage < 1000000) { - ui->mempoolSize->setText(QObject::tr("%1 kB").arg(dynUsage / 1000.0, 0, 'f', 2)); - } else { - ui->mempoolSize->setText(QObject::tr("%1 MB").arg(dynUsage / 1000000.0, 0, 'f', 2)); - } + const auto cur_usage_str = dynUsage < 1000000 ? + QObject::tr("%1 kB").arg(dynUsage / 1000.0, 0, 'f', 2) : + QObject::tr("%1 MB").arg(dynUsage / 1000000.0, 0, 'f', 2); + const auto max_usage_str = QObject::tr("%1 MB").arg(maxUsage / 1000000.0, 0, 'f', 2); + + ui->mempoolSize->setText(cur_usage_str + " / " + max_usage_str); } void RPCConsole::on_lineEdit_returnPressed() @@ -1400,4 +1401,4 @@ void RPCConsole::updateWindowTitle() const QString chainType = QString::fromStdString(Params().GetChainTypeString()); const QString title = tr("Node window - [%1]").arg(chainType); this->setWindowTitle(title); -}
\ No newline at end of file +} diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index d6a5035c33..4747e611d0 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -121,7 +121,7 @@ public Q_SLOTS: /** Set number of blocks and last block date shown in the UI */ void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, SyncType synctype); /** Set size (number of transactions and memory usage) of the mempool in the UI */ - void setMempoolSize(long numberOfTxs, size_t dynUsage); + void setMempoolSize(long numberOfTxs, size_t dynUsage, size_t maxUsage); /** Go forward or back in history */ void browseHistory(int offset); /** Scroll console view to end */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 1019c0de24..9899a13a1e 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -801,14 +801,14 @@ std::optional<int> GetPruneHeight(const BlockManager& blockman, const CChain& ch // If the chain tip is pruned, everything is pruned. if (!((chain_tip->nStatus & BLOCK_HAVE_MASK) == BLOCK_HAVE_MASK)) return chain_tip->nHeight; - const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))}; + const auto& first_unpruned{*CHECK_NONFATAL(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))}; if (&first_unpruned == first_block) { // All blocks between first_block and chain_tip have data, so nothing is pruned. return std::nullopt; } // Block before the first unpruned block is the last pruned block. - return Assert(first_unpruned.pprev)->nHeight; + return CHECK_NONFATAL(first_unpruned.pprev)->nHeight; } static RPCHelpMan pruneblockchain() diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 7dfe69a6c5..b866fa484b 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -146,6 +146,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "fundrawtransaction", 1, "conf_target"}, { "fundrawtransaction", 1, "replaceable"}, { "fundrawtransaction", 1, "solving_data"}, + { "fundrawtransaction", 1, "max_tx_weight"}, { "fundrawtransaction", 2, "iswitness" }, { "walletcreatefundedpsbt", 0, "inputs" }, { "walletcreatefundedpsbt", 1, "outputs" }, @@ -164,6 +165,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "walletcreatefundedpsbt", 3, "conf_target"}, { "walletcreatefundedpsbt", 3, "replaceable"}, { "walletcreatefundedpsbt", 3, "solving_data"}, + { "walletcreatefundedpsbt", 3, "max_tx_weight"}, { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletprocesspsbt", 1, "sign" }, { "walletprocesspsbt", 3, "bip32derivs" }, @@ -208,6 +210,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 4, "conf_target"}, { "send", 4, "replaceable"}, { "send", 4, "solving_data"}, + { "send", 4, "max_tx_weight"}, { "sendall", 0, "recipients" }, { "sendall", 1, "conf_target" }, { "sendall", 3, "fee_rate"}, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 7e420dcd9b..8482ce6eb2 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -371,7 +371,7 @@ static RPCHelpMan generateblock() ChainstateManager& chainman = EnsureChainman(node); { - std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)}; + std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, {.use_mempool = false})}; if (!blocktemplate) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); } @@ -778,9 +778,7 @@ static RPCHelpMan getblocktemplate() } ENTER_CRITICAL_SECTION(cs_main); - std::optional<uint256> maybe_tip{miner.getTipHash()}; - CHECK_NONFATAL(maybe_tip); - tip = maybe_tip.value(); + tip = CHECK_NONFATAL(miner.getTipHash()).value(); if (!IsRPCRunning()) throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down"); diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 0987db194c..ae9dba6a50 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -116,13 +116,13 @@ std::string DescriptorChecksum(const Span<const char>& span) * As a result, within-group-of-32 errors count as 1 symbol, as do cross-group errors that don't affect * the position within the groups. */ - static std::string INPUT_CHARSET = + static const std::string INPUT_CHARSET = "0123456789()[],'/*abcdefgh@:$%{}" "IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~" "ijklmnopqrstuvwxyzABCDEFGH`#\"\\ "; /** The character set for the checksum itself (same as bech32). */ - static std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; + static const std::string CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; uint64_t c = 1; int cls = 0; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 22ac062a63..6e26ec11e0 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -831,7 +831,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, } ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) { + if (!sigdata.complete && !VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) { if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { // Unable to sign input and verification failed (possible attempt to partially sign). input_errors[i] = Untranslated("Unable to sign input, invalid stack size (possibly missing key)"); diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 023a5e8e70..7810d91a77 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -28,7 +28,7 @@ struct NoLockLoggingTestingSetup : public TestingSetup { NoLockLoggingTestingSetup() #ifdef DEBUG_LOCKCONTENTION - : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-debugexclude=lock"}} {} + : TestingSetup{ChainType::MAIN, {.extra_args = { "-debugexclude=lock" } }} {} #else : TestingSetup{ChainType::MAIN} {} #endif diff --git a/src/test/fuzz/crypto_chacha20poly1305.cpp b/src/test/fuzz/crypto_chacha20poly1305.cpp new file mode 100644 index 0000000000..2b39a06094 --- /dev/null +++ b/src/test/fuzz/crypto_chacha20poly1305.cpp @@ -0,0 +1,200 @@ +// Copyright (c) 2020-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <crypto/chacha20poly1305.h> +#include <random.h> +#include <span.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstddef> +#include <cstdint> +#include <vector> + +constexpr static inline void crypt_till_rekey(FSChaCha20Poly1305& aead, int rekey_interval, bool encrypt) +{ + for (int i = 0; i < rekey_interval; ++i) { + std::byte dummy_tag[FSChaCha20Poly1305::EXPANSION] = {{}}; + if (encrypt) { + aead.Encrypt(Span{dummy_tag}.first(0), Span{dummy_tag}.first(0), dummy_tag); + } else { + aead.Decrypt(dummy_tag, Span{dummy_tag}.first(0), Span{dummy_tag}.first(0)); + } + } +} + +FUZZ_TARGET(crypto_aeadchacha20poly1305) +{ + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + + auto key = provider.ConsumeBytes<std::byte>(32); + key.resize(32); + AEADChaCha20Poly1305 aead(key); + + // Initialize RNG deterministically, to generate contents and AAD. We assume that there are no + // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid + // reading the actual data for those from the fuzzer input (which would need large amounts of + // data). + InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>()); + + LIMITED_WHILE(provider.ConsumeBool(), 10000) + { + // Mode: + // - Bit 0: whether to use single-plain Encrypt/Decrypt; otherwise use a split at prefix. + // - Bit 2: whether this ciphertext will be corrupted (making it the last sent one) + // - Bit 3-4: controls the maximum aad length (max 511 bytes) + // - Bit 5-7: controls the maximum content length (max 16383 bytes, for performance reasons) + unsigned mode = provider.ConsumeIntegral<uint8_t>(); + bool use_splits = mode & 1; + bool damage = mode & 4; + unsigned aad_length_bits = 3 * ((mode >> 3) & 3); + unsigned aad_length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << aad_length_bits) - 1); + unsigned length_bits = 2 * ((mode >> 5) & 7); + unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1); + // Generate aad and content. + auto aad = rng.randbytes<std::byte>(aad_length); + auto plain = rng.randbytes<std::byte>(length); + std::vector<std::byte> cipher(length + AEADChaCha20Poly1305::EXPANSION); + // Generate nonce + AEADChaCha20Poly1305::Nonce96 nonce = {(uint32_t)rng(), rng()}; + + if (use_splits && length > 0) { + size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length); + aead.Encrypt(Span{plain}.first(split_index), Span{plain}.subspan(split_index), aad, nonce, cipher); + } else { + aead.Encrypt(plain, aad, nonce, cipher); + } + + // Test Keystream output + std::vector<std::byte> keystream(length); + aead.Keystream(nonce, keystream); + for (size_t i = 0; i < length; ++i) { + assert((plain[i] ^ keystream[i]) == cipher[i]); + } + + std::vector<std::byte> decrypted_contents(length); + bool ok{false}; + + // damage the key + unsigned key_position = provider.ConsumeIntegralInRange<unsigned>(0, 31); + std::byte damage_val{(uint8_t)(1U << (key_position & 7))}; + std::vector<std::byte> bad_key = key; + bad_key[key_position] ^= damage_val; + + AEADChaCha20Poly1305 bad_aead(bad_key); + ok = bad_aead.Decrypt(cipher, aad, nonce, decrypted_contents); + assert(!ok); + + // Optionally damage 1 bit in either the cipher (corresponding to a change in transit) + // or the aad (to make sure that decryption will fail if the AAD mismatches). + if (damage) { + unsigned damage_bit = provider.ConsumeIntegralInRange<unsigned>(0, (cipher.size() + aad.size()) * 8U - 1U); + unsigned damage_pos = damage_bit >> 3; + std::byte damage_val{(uint8_t)(1U << (damage_bit & 7))}; + if (damage_pos >= cipher.size()) { + aad[damage_pos - cipher.size()] ^= damage_val; + } else { + cipher[damage_pos] ^= damage_val; + } + } + + if (use_splits && length > 0) { + size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length); + ok = aead.Decrypt(cipher, aad, nonce, Span{decrypted_contents}.first(split_index), Span{decrypted_contents}.subspan(split_index)); + } else { + ok = aead.Decrypt(cipher, aad, nonce, decrypted_contents); + } + + // Decryption *must* fail if the packet was damaged, and succeed if it wasn't. + assert(!ok == damage); + if (!ok) break; + assert(decrypted_contents == plain); + } +} + +FUZZ_TARGET(crypto_fschacha20poly1305) +{ + FuzzedDataProvider provider{buffer.data(), buffer.size()}; + + uint32_t rekey_interval = provider.ConsumeIntegralInRange<size_t>(32, 512); + auto key = provider.ConsumeBytes<std::byte>(32); + key.resize(32); + FSChaCha20Poly1305 enc_aead(key, rekey_interval); + FSChaCha20Poly1305 dec_aead(key, rekey_interval); + + // Initialize RNG deterministically, to generate contents and AAD. We assume that there are no + // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid + // reading the actual data for those from the fuzzer input (which would need large amounts of + // data). + InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>()); + + LIMITED_WHILE(provider.ConsumeBool(), 10000) + { + // Mode: + // - Bit 0: whether to use single-plain Encrypt/Decrypt; otherwise use a split at prefix. + // - Bit 2: whether this ciphertext will be corrupted (making it the last sent one) + // - Bit 3-4: controls the maximum aad length (max 511 bytes) + // - Bit 5-7: controls the maximum content length (max 16383 bytes, for performance reasons) + unsigned mode = provider.ConsumeIntegral<uint8_t>(); + bool use_splits = mode & 1; + bool damage = mode & 4; + unsigned aad_length_bits = 3 * ((mode >> 3) & 3); + unsigned aad_length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << aad_length_bits) - 1); + unsigned length_bits = 2 * ((mode >> 5) & 7); + unsigned length = provider.ConsumeIntegralInRange<unsigned>(0, (1 << length_bits) - 1); + // Generate aad and content. + auto aad = rng.randbytes<std::byte>(aad_length); + auto plain = rng.randbytes<std::byte>(length); + std::vector<std::byte> cipher(length + FSChaCha20Poly1305::EXPANSION); + + crypt_till_rekey(enc_aead, rekey_interval, true); + if (use_splits && length > 0) { + size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length); + enc_aead.Encrypt(Span{plain}.first(split_index), Span{plain}.subspan(split_index), aad, cipher); + } else { + enc_aead.Encrypt(plain, aad, cipher); + } + + std::vector<std::byte> decrypted_contents(length); + bool ok{false}; + + // damage the key + unsigned key_position = provider.ConsumeIntegralInRange<unsigned>(0, 31); + std::byte damage_val{(uint8_t)(1U << (key_position & 7))}; + std::vector<std::byte> bad_key = key; + bad_key[key_position] ^= damage_val; + + FSChaCha20Poly1305 bad_fs_aead(bad_key, rekey_interval); + crypt_till_rekey(bad_fs_aead, rekey_interval, false); + ok = bad_fs_aead.Decrypt(cipher, aad, decrypted_contents); + assert(!ok); + + // Optionally damage 1 bit in either the cipher (corresponding to a change in transit) + // or the aad (to make sure that decryption will fail if the AAD mismatches). + if (damage) { + unsigned damage_bit = provider.ConsumeIntegralInRange<unsigned>(0, (cipher.size() + aad.size()) * 8U - 1U); + unsigned damage_pos = damage_bit >> 3; + std::byte damage_val{(uint8_t)(1U << (damage_bit & 7))}; + if (damage_pos >= cipher.size()) { + aad[damage_pos - cipher.size()] ^= damage_val; + } else { + cipher[damage_pos] ^= damage_val; + } + } + + crypt_till_rekey(dec_aead, rekey_interval, false); + if (use_splits && length > 0) { + size_t split_index = provider.ConsumeIntegralInRange<size_t>(1, length); + ok = dec_aead.Decrypt(cipher, aad, Span{decrypted_contents}.first(split_index), Span{decrypted_contents}.subspan(split_index)); + } else { + ok = dec_aead.Decrypt(cipher, aad, decrypted_contents); + } + + // Decryption *must* fail if the packet was damaged, and succeed if it wasn't. + assert(!ok == damage); + if (!ok) break; + assert(decrypted_contents == plain); + } +} diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index b9a5560ffb..6a3f4d6dfe 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -72,6 +72,14 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse) // out strings which could correspond to a descriptor containing a too large derivation path. if (HasDeepDerivPath(buffer)) return; + // Some fragments can take a virtually unlimited number of sub-fragments (thresh, multi_a) but + // may perform quadratic operations on them. Limit the number of sub-fragments per fragment. + if (HasTooManySubFrag(buffer)) return; + + // The script building logic performs quadratic copies in the number of nested wrappers. Limit + // the number of nested wrappers per fragment. + if (HasTooManyWrappers(buffer)) return; + const std::string mocked_descriptor{buffer.begin(), buffer.end()}; if (const auto descriptor = MOCKED_DESC_CONVERTER.GetDescriptor(mocked_descriptor)) { FlatSigningProvider signing_provider; @@ -83,8 +91,10 @@ FUZZ_TARGET(mocked_descriptor_parse, .init = initialize_mocked_descriptor_parse) FUZZ_TARGET(descriptor_parse, .init = initialize_descriptor_parse) { - // See comment above for rationale. + // See comments above for rationales. if (HasDeepDerivPath(buffer)) return; + if (HasTooManySubFrag(buffer)) return; + if (HasTooManyWrappers(buffer)) return; const std::string descriptor(buffer.begin(), buffer.end()); FlatSigningProvider signing_provider; diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index 3a1663364f..51de4d0166 100644 --- a/src/test/fuzz/mini_miner.cpp +++ b/src/test/fuzz/mini_miner.cpp @@ -188,9 +188,9 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner) auto mock_template_txids = mini_miner.GetMockTemplateTxids(); // MiniMiner doesn't add a coinbase tx. assert(mock_template_txids.count(blocktemplate->block.vtx[0]->GetHash()) == 0); - mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash()); - assert(mock_template_txids.size() <= blocktemplate->block.vtx.size()); - assert(mock_template_txids.size() >= blocktemplate->block.vtx.size()); + auto [iter, new_entry] = mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash()); + assert(new_entry); + assert(mock_template_txids.size() == blocktemplate->block.vtx.size()); for (const auto& tx : blocktemplate->block.vtx) { assert(mock_template_txids.count(tx->GetHash())); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index d10d9dafe8..6373eac1c3 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -42,7 +42,7 @@ void initialize_process_message() static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>( /*chain_type=*/ChainType::REGTEST, - /*extra_args=*/{"-txreconciliation"}); + {.extra_args = {"-txreconciliation"}}); g_setup = testing_setup.get(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { MineBlock(g_setup->m_node, CScript() << OP_TRUE); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 38acd432fa..62f38967a3 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -32,7 +32,7 @@ void initialize_process_messages() { static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>( /*chain_type=*/ChainType::REGTEST, - /*extra_args=*/{"-txreconciliation"}); + {.extra_args = {"-txreconciliation"}}); g_setup = testing_setup.get(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { MineBlock(g_setup->m_node, CScript() << OP_TRUE); diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 4e52c1c091..9122617e46 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -41,7 +41,7 @@ using util::ToString; namespace { struct RPCFuzzTestingSetup : public TestingSetup { - RPCFuzzTestingSetup(const ChainType chain_type, const std::vector<const char*>& extra_args) : TestingSetup{chain_type, extra_args} + RPCFuzzTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts} { } diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index 0fed2bc5e1..9e52e990a2 100644 --- a/src/test/fuzz/util/descriptor.cpp +++ b/src/test/fuzz/util/descriptor.cpp @@ -4,6 +4,9 @@ #include <test/fuzz/util/descriptor.h> +#include <ranges> +#include <stack> + void MockedDescriptorConverter::Init() { // The data to use as a private key or a seed for an xprv. std::array<std::byte, 32> key_data{std::byte{1}}; @@ -84,3 +87,59 @@ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth) } return false; } + +bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs, const size_t max_nested_subs) +{ + // We use a stack because there may be many nested sub-frags. + std::stack<int> counts; + for (const auto& ch: buff) { + // The fuzzer may generate an input with a ton of parentheses. Rule out pathological cases. + if (counts.size() > max_nested_subs) return true; + + if (ch == '(') { + // A new fragment was opened, create a new sub-count for it and start as one since any fragment with + // parentheses has at least one sub. + counts.push(1); + } else if (ch == ',' && !counts.empty()) { + // When encountering a comma, account for an additional sub in the last opened fragment. If it exceeds the + // limit, bail. + if (++counts.top() > max_subs) return true; + } else if (ch == ')' && !counts.empty()) { + // Fragment closed! Drop its sub count and resume to counting the number of subs for its parent. + counts.pop(); + } + } + return false; +} + +bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers) +{ + // The number of nested wrappers. Nested wrappers are always characters which follow each other so we don't have to + // use a stack as we do above when counting the number of sub-fragments. + std::optional<int> count; + + // We want to detect nested wrappers. A wrapper is a character prepended to a fragment, separated by a colon. There + // may be more than one wrapper, in which case the colon is not repeated. For instance `jjjjj:pk()`. To count + // wrappers we iterate in reverse and use the colon to detect the end of a wrapper expression and count how many + // characters there are since the beginning of the expression. We stop counting when we encounter a character + // indicating the beginning of a new expression. + for (const auto ch: buff | std::views::reverse) { + // A colon, start counting. + if (ch == ':') { + // The colon itself is not a wrapper so we start at 0. + count = 0; + } else if (count) { + // If we are counting wrappers, stop when we crossed the beginning of the wrapper expression. Otherwise keep + // counting and bail if we reached the limit. + // A wrapper may only ever occur as the first sub of a descriptor/miniscript expression ('('), as the + // first Taproot leaf in a pair ('{') or as the nth sub in each case (','). + if (ch == ',' || ch == '(' || ch == '{') { + count.reset(); + } else if (++*count > max_wrappers) { + return true; + } + } + } + + return false; +} diff --git a/src/test/fuzz/util/descriptor.h b/src/test/fuzz/util/descriptor.h index cd41dbafa3..ea928c39f0 100644 --- a/src/test/fuzz/util/descriptor.h +++ b/src/test/fuzz/util/descriptor.h @@ -55,4 +55,25 @@ constexpr int MAX_DEPTH{2}; */ bool HasDeepDerivPath(const FuzzBufferType& buff, const int max_depth = MAX_DEPTH); +//! Default maximum number of sub-fragments. +constexpr int MAX_SUBS{1'000}; +//! Maximum number of nested sub-fragments we'll allow in a descriptor. +constexpr size_t MAX_NESTED_SUBS{10'000}; + +/** + * Whether the buffer, if it represents a valid descriptor, contains a fragment with more + * sub-fragments than the given maximum. + */ +bool HasTooManySubFrag(const FuzzBufferType& buff, const int max_subs = MAX_SUBS, + const size_t max_nested_subs = MAX_NESTED_SUBS); + +//! Default maximum number of wrappers per fragment. +constexpr int MAX_WRAPPERS{100}; + +/** + * Whether the buffer, if it represents a valid descriptor, contains a fragment with more + * wrappers than the given maximum. + */ +bool HasTooManyWrappers(const FuzzBufferType& buff, const int max_wrappers = MAX_WRAPPERS); + #endif // BITCOIN_TEST_FUZZ_UTIL_DESCRIPTOR_H diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index 48ed266abe..b0f1a1251a 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -23,7 +23,7 @@ FUZZ_TARGET(utxo_total_supply) ChainTestingSetup test_setup{ ChainType::REGTEST, { - "-testactivationheight=bip34@2", + .extra_args = {"-testactivationheight=bip34@2"}, }, }; // Create chainstate diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp index 0512c6134f..bb9ca88019 100644 --- a/src/test/i2p_tests.cpp +++ b/src/test/i2p_tests.cpp @@ -23,8 +23,8 @@ class EnvTestingSetup : public BasicTestingSetup { public: explicit EnvTestingSetup(const ChainType chainType = ChainType::MAIN, - const std::vector<const char*>& extra_args = {}) - : BasicTestingSetup{chainType, extra_args}, + TestOpts opts = {}) + : BasicTestingSetup{chainType, opts}, m_prev_log_level{LogInstance().LogLevel()}, m_create_sock_orig{CreateSock} { diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index 5f38ce112c..2dde6daee5 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -31,7 +31,7 @@ struct LogIPsTestingSetup : public TestingSetup { LogIPsTestingSetup() - : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-logips"}} {} + : TestingSetup{ChainType::MAIN, {.extra_args = {"-logips"}}} {} }; BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup) diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index e3b215ad83..af36a95693 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -17,7 +17,7 @@ struct Dersig100Setup : public TestChain100Setup { Dersig100Setup() - : TestChain100Setup{ChainType::REGTEST, {"-testactivationheight=dersig@102"}} {} + : TestChain100Setup{ChainType::REGTEST, {.extra_args = {"-testactivationheight=dersig@102"}}} {} }; bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index d1903df6ac..beefc32bee 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -28,7 +28,8 @@ void ConnmanTestMsg::Handshake(CNode& node, auto& connman{*this}; peerman.InitializeNode(node, local_services); - FlushSendBuffer(node); // Drop the version message added by InitializeNode. + peerman.SendMessages(&node); + FlushSendBuffer(node); // Drop the version message added by SendMessages. CSerializedNetMsg msg_version{ NetMsg::Make(NetMsgType::VERSION, diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 5633757b97..60f9261e7e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -108,7 +108,7 @@ static void ExitFailure(std::string_view str_err) exit(EXIT_FAILURE); } -BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args) +BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts) : m_args{} { m_node.shutdown = &m_interrupt; @@ -125,7 +125,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto "-debugexclude=libevent", "-debugexclude=leveldb", }, - extra_args); + opts.extra_args); if (G_TEST_COMMAND_LINE_ARGUMENTS) { arguments = Cat(arguments, G_TEST_COMMAND_LINE_ARGUMENTS()); } @@ -211,8 +211,8 @@ BasicTestingSetup::~BasicTestingSetup() gArgs.ClearArgs(); } -ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args) - : BasicTestingSetup(chainType, extra_args) +ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) + : BasicTestingSetup(chainType, opts) { const CChainParams& chainparams = Params(); @@ -295,13 +295,11 @@ void ChainTestingSetup::LoadVerifyActivateChainstate() TestingSetup::TestingSetup( const ChainType chainType, - const std::vector<const char*>& extra_args, - const bool coins_db_in_memory, - const bool block_tree_db_in_memory) - : ChainTestingSetup(chainType, extra_args) + TestOpts opts) + : ChainTestingSetup(chainType, opts) { - m_coins_db_in_memory = coins_db_in_memory; - m_block_tree_db_in_memory = block_tree_db_in_memory; + m_coins_db_in_memory = opts.coins_db_in_memory; + m_block_tree_db_in_memory = opts.block_tree_db_in_memory; // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. RegisterAllCoreRPCCommands(tableRPC); @@ -330,11 +328,9 @@ TestingSetup::TestingSetup( } TestChain100Setup::TestChain100Setup( - const ChainType chain_type, - const std::vector<const char*>& extra_args, - const bool coins_db_in_memory, - const bool block_tree_db_in_memory) - : TestingSetup{ChainType::REGTEST, extra_args, coins_db_in_memory, block_tree_db_in_memory} + const ChainType chain_type, + TestOpts opts) + : TestingSetup{ChainType::REGTEST, opts} { SetMockTime(1598887952); constexpr std::array<unsigned char, 32> vchKey = { diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index dbd66e3585..e8b630af94 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -48,6 +48,12 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os static constexpr CAmount CENT{1000000}; +struct TestOpts { + std::vector<const char*> extra_args{}; + bool coins_db_in_memory{true}; + bool block_tree_db_in_memory{true}; +}; + /** Basic testing setup. * This just configures logging, data dir and chain parameters. */ @@ -55,7 +61,7 @@ struct BasicTestingSetup { util::SignalInterrupt m_interrupt; node::NodeContext m_node; // keep as first member to be destructed last - explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {}); + explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {}); ~BasicTestingSetup(); fs::path m_path_root; @@ -73,7 +79,7 @@ struct ChainTestingSetup : public BasicTestingSetup { bool m_coins_db_in_memory{true}; bool m_block_tree_db_in_memory{true}; - explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {}); + explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {}); ~ChainTestingSetup(); // Supplies a chainstate, if one is needed @@ -85,9 +91,7 @@ struct ChainTestingSetup : public BasicTestingSetup { struct TestingSetup : public ChainTestingSetup { explicit TestingSetup( const ChainType chainType = ChainType::MAIN, - const std::vector<const char*>& extra_args = {}, - const bool coins_db_in_memory = true, - const bool block_tree_db_in_memory = true); + TestOpts = {}); }; /** Identical to TestingSetup, but chain set to regtest */ @@ -106,9 +110,7 @@ class CScript; struct TestChain100Setup : public TestingSetup { TestChain100Setup( const ChainType chain_type = ChainType::REGTEST, - const std::vector<const char*>& extra_args = {}, - const bool coins_db_in_memory = true, - const bool block_tree_db_in_memory = true); + TestOpts = {}); /** * Create a new block with just given transactions, coinbase paying to @@ -220,16 +222,16 @@ struct TestChain100Setup : public TestingSetup { * be used in "hot loops", for example fuzzing or benchmarking. */ template <class T = const BasicTestingSetup> -std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, const std::vector<const char*>& extra_args = {}) +std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, TestOpts opts = {}) { - const std::vector<const char*> arguments = Cat( + opts.extra_args = Cat( { "-nodebuglogfile", "-nodebug", }, - extra_args); + opts.extra_args); - return std::make_unique<T>(chain_type, arguments); + return std::make_unique<T>(chain_type, opts); } CBlock getBlock13b8a(); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 1641c4cd22..f93e3cdfb1 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -167,9 +167,10 @@ struct SnapshotTestSetup : TestChain100Setup { // destructive filesystem operations. SnapshotTestSetup() : TestChain100Setup{ {}, - {}, - /*coins_db_in_memory=*/false, - /*block_tree_db_in_memory=*/false, + { + .coins_db_in_memory = false, + .block_tree_db_in_memory = false, + }, } { } diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index 1c724555f3..2577c682d7 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -20,17 +20,17 @@ try { \ (stmt); \ assert(0 && "No exception caught"); \ - } catch (excMatch & e) { \ - } catch (...) { \ - assert(0 && "Wrong exception caught"); \ - } \ + } catch (excMatch&) { \ + } catch (...) { \ + assert(0 && "Wrong exception caught"); \ + } \ } #define BOOST_CHECK_NO_THROW(stmt) { \ try { \ (stmt); \ - } catch (...) { \ - assert(0); \ - } \ + } catch (...) { \ + assert(0); \ + } \ } void univalue_constructor() diff --git a/src/validation.cpp b/src/validation.cpp index 74f0e4975c..c49ec404ca 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1208,7 +1208,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn const CFeeRate package_feerate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize); if (package_feerate <= parent_feerate) { return package_state.Invalid(PackageValidationResult::PCKG_POLICY, - "package RBF failed: package feerate is less than parent feerate", + "package RBF failed: package feerate is less than or equal to parent feerate", strprintf("package feerate %s <= parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString())); } @@ -2399,15 +2399,6 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Ch } -static SteadyClock::duration time_check{}; -static SteadyClock::duration time_forks{}; -static SteadyClock::duration time_connect{}; -static SteadyClock::duration time_verify{}; -static SteadyClock::duration time_undo{}; -static SteadyClock::duration time_index{}; -static SteadyClock::duration time_total{}; -static int64_t num_blocks_total = 0; - /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ @@ -2452,7 +2443,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, uint256 hashPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->GetBlockHash(); assert(hashPrevBlock == view.GetBestBlock()); - num_blocks_total++; + m_chainman.num_blocks_total++; // Special case for the genesis block, skipping connection of its transactions // (its coinbase is unspendable) @@ -2494,11 +2485,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } const auto time_1{SteadyClock::now()}; - time_check += time_1 - time_start; + m_chainman.time_check += time_1 - time_start; LogPrint(BCLog::BENCH, " - Sanity checks: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_1 - time_start), - Ticks<SecondsDouble>(time_check), - Ticks<MillisecondsDouble>(time_check) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_check), + Ticks<MillisecondsDouble>(m_chainman.time_check) / m_chainman.num_blocks_total); // Do not allow blocks that contain transactions which 'overwrite' older transactions, // unless those are already completely spent. @@ -2596,11 +2587,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, unsigned int flags{GetBlockScriptFlags(*pindex, m_chainman)}; const auto time_2{SteadyClock::now()}; - time_forks += time_2 - time_1; + m_chainman.time_forks += time_2 - time_1; LogPrint(BCLog::BENCH, " - Fork checks: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_2 - time_1), - Ticks<SecondsDouble>(time_forks), - Ticks<MillisecondsDouble>(time_forks) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_forks), + Ticks<MillisecondsDouble>(m_chainman.time_forks) / m_chainman.num_blocks_total); CBlockUndo blockundo; @@ -2687,12 +2678,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, UpdateCoins(tx, view, i == 0 ? undoDummy : blockundo.vtxundo.back(), pindex->nHeight); } const auto time_3{SteadyClock::now()}; - time_connect += time_3 - time_2; + m_chainman.time_connect += time_3 - time_2; LogPrint(BCLog::BENCH, " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(), Ticks<MillisecondsDouble>(time_3 - time_2), Ticks<MillisecondsDouble>(time_3 - time_2) / block.vtx.size(), nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_3 - time_2) / (nInputs - 1), - Ticks<SecondsDouble>(time_connect), - Ticks<MillisecondsDouble>(time_connect) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_connect), + Ticks<MillisecondsDouble>(m_chainman.time_connect) / m_chainman.num_blocks_total); CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, params.GetConsensus()); if (block.vtx[0]->GetValueOut() > blockReward) { @@ -2705,12 +2696,12 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-validation-failed"); } const auto time_4{SteadyClock::now()}; - time_verify += time_4 - time_2; + m_chainman.time_verify += time_4 - time_2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, Ticks<MillisecondsDouble>(time_4 - time_2), nInputs <= 1 ? 0 : Ticks<MillisecondsDouble>(time_4 - time_2) / (nInputs - 1), - Ticks<SecondsDouble>(time_verify), - Ticks<MillisecondsDouble>(time_verify) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_verify), + Ticks<MillisecondsDouble>(m_chainman.time_verify) / m_chainman.num_blocks_total); if (fJustCheck) return true; @@ -2720,11 +2711,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } const auto time_5{SteadyClock::now()}; - time_undo += time_5 - time_4; + m_chainman.time_undo += time_5 - time_4; LogPrint(BCLog::BENCH, " - Write undo data: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_5 - time_4), - Ticks<SecondsDouble>(time_undo), - Ticks<MillisecondsDouble>(time_undo) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_undo), + Ticks<MillisecondsDouble>(m_chainman.time_undo) / m_chainman.num_blocks_total); if (!pindex->IsValid(BLOCK_VALID_SCRIPTS)) { pindex->RaiseValidity(BLOCK_VALID_SCRIPTS); @@ -2735,11 +2726,11 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, view.SetBestBlock(pindex->GetBlockHash()); const auto time_6{SteadyClock::now()}; - time_index += time_6 - time_5; + m_chainman.time_index += time_6 - time_5; LogPrint(BCLog::BENCH, " - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_5), - Ticks<SecondsDouble>(time_index), - Ticks<MillisecondsDouble>(time_index) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_index), + Ticks<MillisecondsDouble>(m_chainman.time_index) / m_chainman.num_blocks_total); TRACE6(validation, block_connected, block_hash.data(), @@ -3097,11 +3088,6 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra return true; } -static SteadyClock::duration time_connect_total{}; -static SteadyClock::duration time_flush{}; -static SteadyClock::duration time_chainstate{}; -static SteadyClock::duration time_post_connect{}; - struct PerBlockConnectTrace { CBlockIndex* pindex = nullptr; std::shared_ptr<const CBlock> pblock; @@ -3188,31 +3174,31 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, return false; } time_3 = SteadyClock::now(); - time_connect_total += time_3 - time_2; - assert(num_blocks_total > 0); + m_chainman.time_connect_total += time_3 - time_2; + assert(m_chainman.num_blocks_total > 0); LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_3 - time_2), - Ticks<SecondsDouble>(time_connect_total), - Ticks<MillisecondsDouble>(time_connect_total) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_connect_total), + Ticks<MillisecondsDouble>(m_chainman.time_connect_total) / m_chainman.num_blocks_total); bool flushed = view.Flush(); assert(flushed); } const auto time_4{SteadyClock::now()}; - time_flush += time_4 - time_3; + m_chainman.time_flush += time_4 - time_3; LogPrint(BCLog::BENCH, " - Flush: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_4 - time_3), - Ticks<SecondsDouble>(time_flush), - Ticks<MillisecondsDouble>(time_flush) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_flush), + Ticks<MillisecondsDouble>(m_chainman.time_flush) / m_chainman.num_blocks_total); // Write the chain state to disk, if necessary. if (!FlushStateToDisk(state, FlushStateMode::IF_NEEDED)) { return false; } const auto time_5{SteadyClock::now()}; - time_chainstate += time_5 - time_4; + m_chainman.time_chainstate += time_5 - time_4; LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_5 - time_4), - Ticks<SecondsDouble>(time_chainstate), - Ticks<MillisecondsDouble>(time_chainstate) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_chainstate), + Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total); // Remove conflicting transactions from the mempool.; if (m_mempool) { m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight); @@ -3223,16 +3209,16 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; - time_post_connect += time_6 - time_5; - time_total += time_6 - time_1; + m_chainman.time_post_connect += time_6 - time_5; + m_chainman.time_total += time_6 - time_1; LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_5), - Ticks<SecondsDouble>(time_post_connect), - Ticks<MillisecondsDouble>(time_post_connect) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_post_connect), + Ticks<MillisecondsDouble>(m_chainman.time_post_connect) / m_chainman.num_blocks_total); LogPrint(BCLog::BENCH, "- Connect block: %.2fms [%.2fs (%.2fms/blk)]\n", Ticks<MillisecondsDouble>(time_6 - time_1), - Ticks<SecondsDouble>(time_total), - Ticks<MillisecondsDouble>(time_total) / num_blocks_total); + Ticks<SecondsDouble>(m_chainman.time_total), + Ticks<MillisecondsDouble>(m_chainman.time_total) / m_chainman.num_blocks_total); // If we are the background validation chainstate, check to see if we are done // validating the snapshot (i.e. our tip has reached the snapshot's base block). @@ -3419,25 +3405,24 @@ static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_i return SynchronizationState::INIT_DOWNLOAD; } -static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main) +bool ChainstateManager::NotifyHeaderTip() { bool fNotify = false; bool fInitialBlockDownload = false; - static CBlockIndex* pindexHeaderOld = nullptr; CBlockIndex* pindexHeader = nullptr; { - LOCK(cs_main); - pindexHeader = chainman.m_best_header; + LOCK(GetMutex()); + pindexHeader = m_best_header; - if (pindexHeader != pindexHeaderOld) { + if (pindexHeader != m_last_notified_header) { fNotify = true; - fInitialBlockDownload = chainman.IsInitialBlockDownload(); - pindexHeaderOld = pindexHeader; + fInitialBlockDownload = IsInitialBlockDownload(); + m_last_notified_header = pindexHeader; } } - // Send block tip changed notifications without cs_main + // Send block tip changed notifications without the lock held if (fNotify) { - chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false); + GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false); } return fNotify; } @@ -4393,7 +4378,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& } } } - if (NotifyHeaderTip(*this)) { + if (NotifyHeaderTip()) { if (IsInitialBlockDownload() && ppindex && *ppindex) { const CBlockIndex& last_accepted{**ppindex}; int64_t blocks_left{(NodeClock::now() - last_accepted.Time()) / GetConsensus().PowTargetSpacing()}; @@ -4564,7 +4549,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock>& blo } } - NotifyHeaderTip(*this); + NotifyHeaderTip(); BlockValidationState state; // Only used to report errors, not invalidity - ignore it if (!ActiveChainstate().ActivateBestChain(state, block)) { @@ -5141,7 +5126,7 @@ void ChainstateManager::LoadExternalBlockFile( } } - NotifyHeaderTip(*this); + NotifyHeaderTip(); if (!blocks_with_unknown_parent) continue; @@ -5167,7 +5152,7 @@ void ChainstateManager::LoadExternalBlockFile( } range.first++; blocks_with_unknown_parent->erase(it); - NotifyHeaderTip(*this); + NotifyHeaderTip(); } } } catch (const std::exception& e) { diff --git a/src/validation.h b/src/validation.h index 9f99ba796b..08e672c620 100644 --- a/src/validation.h +++ b/src/validation.h @@ -906,6 +906,11 @@ private: CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr}; + /** The last header for which a headerTip notification was issued. */ + CBlockIndex* m_last_notified_header GUARDED_BY(GetMutex()){nullptr}; + + bool NotifyHeaderTip() LOCKS_EXCLUDED(GetMutex()); + //! Internal helper for ActivateSnapshot(). //! //! De-serialization of a snapshot that is created with @@ -949,6 +954,21 @@ private: //! A queue for script verifications that have to be performed by worker threads. CCheckQueue<CScriptCheck> m_script_check_queue; + //! Timers and counters used for benchmarking validation in both background + //! and active chainstates. + SteadyClock::duration GUARDED_BY(::cs_main) time_check{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_forks{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_connect{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_verify{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_undo{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_index{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_total{}; + int64_t GUARDED_BY(::cs_main) num_blocks_total{0}; + SteadyClock::duration GUARDED_BY(::cs_main) time_connect_total{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_flush{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_chainstate{}; + SteadyClock::duration GUARDED_BY(::cs_main) time_post_connect{}; + public: using Options = kernel::ChainstateManagerOpts; diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index b2f813383d..d36314312a 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -115,6 +115,8 @@ public: std::optional<uint32_t> m_locktime; //! Version std::optional<uint32_t> m_version; + //! Caps weight of resulting tx + std::optional<int> m_max_tx_weight{std::nullopt}; CCoinControl(); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index edde912ce0..b6fee37c95 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -84,14 +84,14 @@ struct { * bound of the range. * @param const CAmount& cost_of_change This is the cost of creating and spending a change output. * This plus selection_target is the upper bound of the range. - * @param int max_weight The maximum weight available for the input set. + * @param int max_selection_weight The maximum allowed weight for a selection result to be valid. * @returns The result of this coin selection algorithm, or std::nullopt */ static const size_t TOTAL_TRIES = 100000; util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, - int max_weight) + int max_selection_weight) { SelectionResult result(selection_target, SelectionAlgorithm::BNB); CAmount curr_value = 0; @@ -128,7 +128,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing backtrack = true; - } else if (curr_selection_weight > max_weight) { // Exceeding weight for standard tx, cannot find more solutions by adding more inputs + } else if (curr_selection_weight > max_selection_weight) { // Selected UTXOs weight exceeds the maximum weight allowed, cannot find more solutions by adding more inputs max_tx_weight_exceeded = true; // at least one selection attempt exceeded the max weight backtrack = true; } else if (curr_value >= selection_target) { // Selected value is within range @@ -319,10 +319,10 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool * group with multiple as a heavier UTXO with the combined amount here.) * @param const CAmount& selection_target This is the minimum amount that we need for the transaction without considering change. * @param const CAmount& change_target The minimum budget for creating a change output, by which we increase the selection_target. - * @param int max_weight The maximum permitted weight for the input set. + * @param int max_selection_weight The maximum allowed weight for a selection result to be valid. * @returns The result of this coin selection algorithm, or std::nullopt */ -util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight) +util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight) { std::sort(utxo_pool.begin(), utxo_pool.end(), descending_effval_weight); // The sum of UTXO amounts after this UTXO index, e.g. lookahead[5] = Σ(UTXO[6+].amount) @@ -359,7 +359,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c // The weight of the currently selected input set, and the weight of the best selection int curr_weight = 0; - int best_selection_weight = max_weight; // Tie is fine, because we prefer lower selection amount + int best_selection_weight = max_selection_weight; // Tie is fine, because we prefer lower selection amount // Whether the input sets generated during this search have exceeded the maximum transaction weight at any point bool max_tx_weight_exceeded = false; @@ -436,8 +436,8 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c // Insufficient funds with lookahead: CUT should_cut = true; } else if (curr_weight > best_selection_weight) { - // best_selection_weight is initialized to max_weight - if (curr_weight > max_weight) max_tx_weight_exceeded = true; + // best_selection_weight is initialized to max_selection_weight + if (curr_weight > max_selection_weight) max_tx_weight_exceeded = true; // Worse weight than best solution. More UTXOs only increase weight: // CUT if last selected group had minimal weight, else SHIFT if (utxo_pool[curr_tail].m_weight <= min_tail_weight[curr_tail]) { @@ -535,7 +535,7 @@ public: }; util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng, - int max_weight) + int max_selection_weight) { SelectionResult result(target_value, SelectionAlgorithm::SRD); std::priority_queue<OutputGroup, std::vector<OutputGroup>, MinOutputGroupComparator> heap; @@ -565,14 +565,14 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx // If the selection weight exceeds the maximum allowed size, remove the least valuable inputs until we // are below max weight. - if (weight > max_weight) { + if (weight > max_selection_weight) { max_tx_weight_exceeded = true; // mark it in case we don't find any useful result. do { const OutputGroup& to_remove_group = heap.top(); selected_eff_value -= to_remove_group.GetSelectionAmount(); weight -= to_remove_group.m_weight; heap.pop(); - } while (!heap.empty() && weight > max_weight); + } while (!heap.empty() && weight > max_selection_weight); } // Now check if we are above the target @@ -597,11 +597,12 @@ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utx * nTargetValue, with indices corresponding to groups. If the ith * entry is true, that means the ith group in groups was selected. * param@[out] nBest Total amount of subset chosen that is closest to nTargetValue. + * paramp[in] max_selection_weight The maximum allowed weight for a selection result to be valid. * param@[in] iterations Maximum number of tries. */ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue, - std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) + std::vector<char>& vfBest, CAmount& nBest, int max_selection_weight, int iterations = 1000) { std::vector<char> vfIncluded; @@ -613,6 +614,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v { vfIncluded.assign(groups.size(), false); CAmount nTotal = 0; + int selected_coins_weight{0}; bool fReachedTarget = false; for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) { @@ -627,9 +629,9 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) { nTotal += groups[i].GetSelectionAmount(); + selected_coins_weight += groups[i].m_weight; vfIncluded[i] = true; - if (nTotal >= nTargetValue) - { + if (nTotal >= nTargetValue && selected_coins_weight <= max_selection_weight) { fReachedTarget = true; // If the total is between nTargetValue and nBest, it's our new best // approximation. @@ -639,6 +641,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v vfBest = vfIncluded; } nTotal -= groups[i].GetSelectionAmount(); + selected_coins_weight -= groups[i].m_weight; vfIncluded[i] = false; } } @@ -648,10 +651,11 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v } util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, - CAmount change_target, FastRandomContext& rng, int max_weight) + CAmount change_target, FastRandomContext& rng, int max_selection_weight) { SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK); + bool max_weight_exceeded{false}; // List of values less than target std::optional<OutputGroup> lowest_larger; // Groups with selection amount smaller than the target and any change we might produce. @@ -662,6 +666,10 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c std::shuffle(groups.begin(), groups.end(), rng); for (const OutputGroup& group : groups) { + if (group.m_weight > max_selection_weight) { + max_weight_exceeded = true; + continue; + } if (group.GetSelectionAmount() == nTargetValue) { result.AddInput(group); return result; @@ -677,11 +685,18 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c for (const auto& group : applicable_groups) { result.AddInput(group); } - return result; + if (result.GetWeight() <= max_selection_weight) return result; + else max_weight_exceeded = true; + + // Try something else + result.Clear(); } if (nTotalLower < nTargetValue) { - if (!lowest_larger) return util::Error(); + if (!lowest_larger) { + if (max_weight_exceeded) return ErrorMaxWeightExceeded(); + return util::Error(); + } result.AddInput(*lowest_larger); return result; } @@ -691,9 +706,9 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c std::vector<char> vfBest; CAmount nBest; - ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest); + ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest, max_selection_weight); if (nBest != nTargetValue && nTotalLower >= nTargetValue + change_target) { - ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + change_target, vfBest, nBest); + ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + change_target, vfBest, nBest, max_selection_weight); } // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, @@ -709,7 +724,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c } // If the result exceeds the maximum allowed size, return closest UTXO above the target - if (result.GetWeight() > max_weight) { + if (result.GetWeight() > max_selection_weight) { // No coin above target, nothing to do. if (!lowest_larger) return ErrorMaxWeightExceeded(); @@ -728,7 +743,7 @@ util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, c LogPrint(BCLog::SELECTCOINS, "%stotal %s\n", log_message, FormatMoney(nBest)); } } - + Assume(result.GetWeight() <= max_selection_weight); return result; } diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 416273be7a..08889c8e06 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -139,9 +139,9 @@ struct CoinSelectionParams { /** Randomness to use in the context of coin selection. */ FastRandomContext& rng_fast; /** Size of a change output in bytes, determined by the output type. */ - size_t change_output_size = 0; + int change_output_size = 0; /** Size of the input to spend a change output in virtual bytes. */ - size_t change_spend_size = 0; + int change_spend_size = 0; /** Mininmum change to target in Knapsack solver and CoinGrinder: * select coins to cover the payment and at least this value of change. */ CAmount m_min_change_target{0}; @@ -162,7 +162,7 @@ struct CoinSelectionParams { CFeeRate m_discard_feerate; /** Size of the transaction before coin selection, consisting of the header and recipient * output(s), excluding the inputs and change output(s). */ - size_t tx_noinputs_size = 0; + int tx_noinputs_size = 0; /** Indicate that we are subtracting the fee from outputs */ bool m_subtract_fee_outputs = false; /** When true, always spend all (up to OUTPUT_GROUP_MAX_ENTRIES) or none of the outputs @@ -174,10 +174,13 @@ struct CoinSelectionParams { * 1) Received from other wallets, 2) replacing other txs, 3) that have been replaced. */ bool m_include_unsafe_inputs = false; + /** The maximum weight for this transaction. */ + std::optional<int> m_max_tx_weight{std::nullopt}; - CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, + CoinSelectionParams(FastRandomContext& rng_fast, int change_output_size, int change_spend_size, CAmount min_change_target, CFeeRate effective_feerate, - CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) + CFeeRate long_term_feerate, CFeeRate discard_feerate, int tx_noinputs_size, bool avoid_partial, + std::optional<int> max_tx_weight = std::nullopt) : rng_fast{rng_fast}, change_output_size(change_output_size), change_spend_size(change_spend_size), @@ -186,7 +189,8 @@ struct CoinSelectionParams { m_long_term_feerate(long_term_feerate), m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size), - m_avoid_partial_spends(avoid_partial) + m_avoid_partial_spends(avoid_partial), + m_max_tx_weight(max_tx_weight) { } CoinSelectionParams(FastRandomContext& rng_fast) @@ -440,9 +444,9 @@ public: }; util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, - int max_weight); + int max_selection_weight); -util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight); +util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_selection_weight); /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible * outputs until the target is satisfied @@ -450,15 +454,15 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c * @param[in] utxo_pool The positive effective value OutputGroups eligible for selection * @param[in] target_value The target value to select for * @param[in] rng The randomness source to shuffle coins - * @param[in] max_weight The maximum allowed weight for a selection result to be valid + * @param[in] max_selection_weight The maximum allowed weight for a selection result to be valid * @returns If successful, a valid SelectionResult, otherwise, util::Error */ util::Result<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, CAmount change_fee, FastRandomContext& rng, - int max_weight); + int max_selection_weight); // Original coin selection algorithm as a fallback util::Result<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, - CAmount change_target, FastRandomContext& rng, int max_weight); + CAmount change_target, FastRandomContext& rng, int max_selection_weight); } // namespace wallet #endif // BITCOIN_WALLET_COINSELECTION_H diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 0c2ad06eea..35c93337c1 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2011-2022 The Bitcoin Core developers +// Copyright (c) 2011-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -534,7 +534,7 @@ RPCHelpMan getaddressinfo() {RPCResult::Type::BOOL, "solvable", "If we know how to spend coins sent to this address, ignoring the possible lack of private keys."}, {RPCResult::Type::STR, "desc", /*optional=*/true, "A descriptor for spending coins sent to this address (only when solvable)."}, {RPCResult::Type::STR, "parent_desc", /*optional=*/true, "The descriptor used to derive this address if this is a descriptor wallet"}, - {RPCResult::Type::BOOL, "isscript", "If the key is a script."}, + {RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script."}, {RPCResult::Type::BOOL, "ischange", "If the address was used for change output."}, {RPCResult::Type::BOOL, "iswitness", "If the address is a witness address."}, {RPCResult::Type::NUM, "witness_version", /*optional=*/true, "The version number of the witness program."}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index ac2a4826f0..8ea51e65c7 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -542,6 +542,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact {"minconf", UniValueType(UniValue::VNUM)}, {"maxconf", UniValueType(UniValue::VNUM)}, {"input_weights", UniValueType(UniValue::VARR)}, + {"max_tx_weight", UniValueType(UniValue::VNUM)}, }, true, true); @@ -701,6 +702,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact } } + if (options.exists("max_tx_weight")) { + coinControl.m_max_tx_weight = options["max_tx_weight"].getInt<int>(); + } + if (recipients.empty()) throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); @@ -786,6 +791,8 @@ RPCHelpMan fundrawtransaction() }, }, }, + {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n" + "Transaction building will fail if this can not be satisfied."}, }, FundTxDoc()), RPCArgOptions{ @@ -1240,6 +1247,8 @@ RPCHelpMan send() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, + {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n" + "Transaction building will fail if this can not be satisfied."}, }, FundTxDoc()), RPCArgOptions{.oneline_description="options"}}, @@ -1287,6 +1296,9 @@ RPCHelpMan send() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; + if (options.exists("max_tx_weight")) { + coin_control.m_max_tx_weight = options["max_tx_weight"].getInt<int>(); + } SetOptionsInputWeights(options["inputs"], options); // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly @@ -1697,6 +1709,8 @@ RPCHelpMan walletcreatefundedpsbt() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, + {"max_tx_weight", RPCArg::Type::NUM, RPCArg::Default{MAX_STANDARD_TX_WEIGHT}, "The maximum acceptable transaction weight.\n" + "Transaction building will fail if this can not be satisfied."}, }, FundTxDoc()), RPCArgOptions{.oneline_description="options"}}, diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 11a42eb469..7abf7f59c0 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -695,26 +695,35 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co } }; - // Maximum allowed weight - int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); + // Maximum allowed weight for selected coins. + int max_transaction_weight = coin_selection_params.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT); + int tx_weight_no_input = coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR; + int max_selection_weight = max_transaction_weight - tx_weight_no_input; + if (max_selection_weight <= 0) { + return util::Error{_("Maximum transaction weight is less than transaction weight without inputs")}; + } // SFFO frequently causes issues in the context of changeless input sets: skip BnB when SFFO is active 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)}) { + if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_selection_weight)}) { results.push_back(*bnb_result); } else append_error(std::move(bnb_result)); } - // As Knapsack and SRD can create change, also deduce change weight. - max_inputs_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR); + // Deduct change weight because remaining Coin Selection algorithms can create change output + int change_outputs_weight = coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR; + max_selection_weight -= change_outputs_weight; + if (max_selection_weight < 0 && results.empty()) { + return util::Error{_("Maximum transaction weight is too low, can not accommodate change output")}; + } // 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)}) { + if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_selection_weight)}) { results.push_back(*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)}) { + if (auto cg_result{CoinGrinder(groups.positive_group, nTargetValue, coin_selection_params.m_min_change_target, max_selection_weight)}) { cg_result->RecalculateWaste(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 { @@ -722,7 +731,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co } } - if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) { + if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_selection_weight)}) { results.push_back(*srd_result); } else append_error(std::move(srd_result)); @@ -801,7 +810,7 @@ util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& av coin_selection_params.m_change_fee); // Verify we haven't exceeded the maximum allowed weight - int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); + int max_inputs_weight = coin_selection_params.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT) - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); if (op_selection_result->GetWeight() > max_inputs_weight) { return util::Error{_("The combination of the pre-selected inputs and the wallet automatic inputs selection exceeds the transaction maximum weight. " "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; @@ -1012,7 +1021,11 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs; - + coin_selection_params.m_max_tx_weight = coin_control.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT); + int minimum_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR; + if (coin_selection_params.m_max_tx_weight.value() < minimum_tx_weight || coin_selection_params.m_max_tx_weight.value() > MAX_STANDARD_TX_WEIGHT) { + return util::Error{strprintf(_("Maximum transaction weight must be between %d and %d"), minimum_tx_weight, MAX_STANDARD_TX_WEIGHT)}; + } // Set the long term feerate estimate to the wallet's consolidate feerate coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) @@ -1077,7 +1090,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( if (change_spend_size == -1) { coin_selection_params.change_spend_size = DUMMY_NESTED_P2WPKH_INPUT_SIZE; } else { - coin_selection_params.change_spend_size = (size_t)change_spend_size; + coin_selection_params.change_spend_size = change_spend_size; } // Set discard feerate diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 7bd92b471c..eb9c349c22 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -1097,13 +1097,13 @@ BOOST_AUTO_TEST_CASE(effective_value_test) static util::Result<SelectionResult> CoinGrinder(const CAmount& target, const CoinSelectionParams& cs_params, const node::NodeContext& m_node, - int max_weight, + int max_selection_weight, std::function<CoinsResult(CWallet&)> coin_setup) { std::unique_ptr<CWallet> wallet = NewWallet(m_node); CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups; - return CoinGrinder(group.positive_group, target, cs_params.m_min_change_target, max_weight); + return CoinGrinder(group.positive_group, target, cs_params.m_min_change_target, max_selection_weight); } BOOST_AUTO_TEST_CASE(coin_grinder_tests) @@ -1135,8 +1135,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 1) Insufficient funds, select all provided coins and fail // ######################################################### CAmount target = 49.5L * COIN; - int max_weight = 10'000; // high enough to not fail for this reason. - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 10'000; // high enough to not fail for this reason. + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 10; ++j) { add_coin(available_coins, wallet, CAmount(1 * COIN)); @@ -1153,8 +1153,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 2) Test max weight exceeded // ########################### CAmount target = 29.5L * COIN; - int max_weight = 3000; - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 3000; + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 10; ++j) { add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true); @@ -1171,8 +1171,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 3) Test selection when some coins surpass the max allowed weight while others not. --> must find a good solution // ################################################################################################################ CAmount target = 25.33L * COIN; - int max_weight = 10'000; // WU - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 10'000; // WU + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 60; ++j) { // 60 UTXO --> 19,8 BTC total --> 60 × 272 WU = 16320 WU add_coin(available_coins, wallet, CAmount(0.33 * COIN), CFeeRate(5000), 144, false, 0, true); @@ -1193,8 +1193,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 4) Test that two less valuable UTXOs with a combined lower weight are preferred over a more valuable heavier UTXO // ################################################################################################################# CAmount target = 1.9L * COIN; - int max_weight = 400'000; // WU - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 400'000; // WU + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; add_coin(available_coins, wallet, CAmount(2 * COIN), CFeeRate(5000), 144, false, 0, true, 148); add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true, 68); @@ -1215,8 +1215,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 5) Test finding a solution in a UTXO pool with mixed weights // ################################################################################################################ CAmount target = 30L * COIN; - int max_weight = 400'000; // WU - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 400'000; // WU + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 5; ++j) { // Add heavy coins {3, 6, 9, 12, 15} @@ -1244,8 +1244,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 6) Test that the lightest solution among many clones is found // ################################################################################################################# CAmount target = 9.9L * COIN; - int max_weight = 400'000; // WU - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 400'000; // WU + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; // Expected Result: 4 + 3 + 2 + 1 = 10 BTC at 400 vB add_coin(available_coins, wallet, CAmount(4 * COIN), CFeeRate(5000), 144, false, 0, true, 100); @@ -1283,8 +1283,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) // 7) Test that lots of tiny UTXOs can be skipped if they are too heavy while there are enough funds in lookahead // ################################################################################################################# CAmount target = 1.9L * COIN; - int max_weight = 40000; // WU - const auto& res = CoinGrinder(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 40000; // WU + const auto& res = CoinGrinder(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; add_coin(available_coins, wallet, CAmount(1.8 * COIN), CFeeRate(5000), 144, false, 0, true, 2500); add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(5000), 144, false, 0, true, 1000); @@ -1308,13 +1308,13 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) static util::Result<SelectionResult> SelectCoinsSRD(const CAmount& target, const CoinSelectionParams& cs_params, const node::NodeContext& m_node, - int max_weight, + int max_selection_weight, std::function<CoinsResult(CWallet&)> coin_setup) { std::unique_ptr<CWallet> wallet = NewWallet(m_node); CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups; - return SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_weight); + return SelectCoinsSRD(group.positive_group, target, cs_params.m_change_fee, cs_params.rng_fast, max_selection_weight); } BOOST_AUTO_TEST_CASE(srd_tests) @@ -1342,8 +1342,8 @@ BOOST_AUTO_TEST_CASE(srd_tests) // 1) Insufficient funds, select all provided coins and fail // ######################################################### CAmount target = 49.5L * COIN; - int max_weight = 10000; // high enough to not fail for this reason. - const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 10000; // high enough to not fail for this reason. + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 10; ++j) { add_coin(available_coins, wallet, CAmount(1 * COIN)); @@ -1360,8 +1360,8 @@ BOOST_AUTO_TEST_CASE(srd_tests) // 2) Test max weight exceeded // ########################### CAmount target = 49.5L * COIN; - int max_weight = 3000; - const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 3000; + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 10; ++j) { /* 10 × 1 BTC + 10 × 2 BTC = 30 BTC. 20 × 272 WU = 5440 WU */ @@ -1379,8 +1379,8 @@ BOOST_AUTO_TEST_CASE(srd_tests) // 3) Test selection when some coins surpass the max allowed weight while others not. --> must find a good solution // ################################################################################################################ CAmount target = 25.33L * COIN; - int max_weight = 10000; // WU - const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + int max_selection_weight = 10000; // WU + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_selection_weight, [&](CWallet& wallet) { CoinsResult available_coins; for (int j = 0; j < 60; ++j) { // 60 UTXO --> 19,8 BTC total --> 60 × 272 WU = 16320 WU add_coin(available_coins, wallet, CAmount(0.33 * COIN), CFeeRate(0), 144, false, 0, true); @@ -1415,7 +1415,7 @@ static bool has_coin(const CoinSet& set, CAmount amount) return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; }); } -BOOST_AUTO_TEST_CASE(check_max_weight) +BOOST_AUTO_TEST_CASE(check_max_selection_weight) { const CAmount target = 49.5L * COIN; CCoinControl cc; diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 644a8dd7ad..209c87fd42 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -195,11 +195,11 @@ FUZZ_TARGET(coin_grinder_is_optimal) if (best_weight < std::numeric_limits<int>::max()) { // Sufficient funds and acceptable weight: CoinGrinder should find at least one solution - int high_max_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(best_weight, std::numeric_limits<int>::max()); + int high_max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(best_weight, std::numeric_limits<int>::max()); - auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, high_max_weight); + auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, high_max_selection_weight); assert(result_cg); - assert(result_cg->GetWeight() <= high_max_weight); + assert(result_cg->GetWeight() <= high_max_selection_weight); assert(result_cg->GetSelectedEffectiveValue() >= target + coin_params.m_min_change_target); assert(best_weight < result_cg->GetWeight() || (best_weight == result_cg->GetWeight() && best_amount <= result_cg->GetSelectedEffectiveValue())); if (result_cg->GetAlgoCompleted()) { @@ -210,8 +210,8 @@ FUZZ_TARGET(coin_grinder_is_optimal) } // CoinGrinder cannot ever find a better solution than the brute-forced best, or there is none in the first place - int low_max_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, best_weight - 1); - auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, low_max_weight); + int low_max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, best_weight - 1); + auto result_cg = CoinGrinder(group_pos, target, coin_params.m_min_change_target, low_max_selection_weight); // Max_weight should have been exceeded, or there were insufficient funds assert(!result_cg); } @@ -256,29 +256,34 @@ FUZZ_TARGET(coinselection) (void)group.EligibleForSpending(filter); } + int max_selection_weight = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()); + // Run coinselection algorithms auto result_bnb = coin_params.m_subtract_fee_outputs ? util::Error{Untranslated("BnB disabled when SFFO is enabled")} : - SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, MAX_STANDARD_TX_WEIGHT); + SelectCoinsBnB(group_pos, target, coin_params.m_cost_of_change, max_selection_weight); if (result_bnb) { assert(result_bnb->GetChange(coin_params.min_viable_change, coin_params.m_change_fee) == 0); assert(result_bnb->GetSelectedValue() >= target); + assert(result_bnb->GetWeight() <= max_selection_weight); (void)result_bnb->GetShuffledInputVector(); (void)result_bnb->GetInputSet(); } - auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, MAX_STANDARD_TX_WEIGHT); + auto result_srd = SelectCoinsSRD(group_pos, target, coin_params.m_change_fee, fast_random_context, max_selection_weight); if (result_srd) { assert(result_srd->GetSelectedValue() >= target); assert(result_srd->GetChange(CHANGE_LOWER, coin_params.m_change_fee) > 0); // Demonstrate that SRD creates change of at least CHANGE_LOWER + assert(result_srd->GetWeight() <= max_selection_weight); result_srd->RecalculateWaste(coin_params.min_viable_change, coin_params.m_cost_of_change, coin_params.m_change_fee); (void)result_srd->GetShuffledInputVector(); (void)result_srd->GetInputSet(); } CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)}; - auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, MAX_STANDARD_TX_WEIGHT); + auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, max_selection_weight); if (result_knapsack) { assert(result_knapsack->GetSelectedValue() >= target); + assert(result_knapsack->GetWeight() <= max_selection_weight); result_knapsack->RecalculateWaste(coin_params.min_viable_change, coin_params.m_cost_of_change, coin_params.m_change_fee); (void)result_knapsack->GetShuffledInputVector(); (void)result_knapsack->GetInputSet(); diff --git a/src/wallet/test/fuzz/crypter.cpp b/src/wallet/test/fuzz/crypter.cpp index 62dd1bfde0..814136476b 100644 --- a/src/wallet/test/fuzz/crypter.cpp +++ b/src/wallet/test/fuzz/crypter.cpp @@ -27,36 +27,36 @@ FUZZ_TARGET(crypter, .init = initialize_crypter) // These values are regularly updated within `CallOneOf` std::vector<unsigned char> cipher_text_ed; CKeyingMaterial plain_text_ed; - const std::vector<unsigned char> random_key = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<unsigned char> random_key = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE); LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10000) { CallOneOf( fuzzed_data_provider, [&] { - const std::string random_string = fuzzed_data_provider.ConsumeRandomLengthString(); + const std::string random_string = fuzzed_data_provider.ConsumeRandomLengthString(100); SecureString secure_string(random_string.begin(), random_string.end()); const unsigned int derivation_method = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<unsigned int>(); // Limiting the value of nRounds since it is otherwise uselessly expensive and causes a timeout when fuzzing. crypt.SetKeyFromPassphrase(/*strKeyData=*/secure_string, - /*chSalt=*/ConsumeRandomLengthByteVector(fuzzed_data_provider), + /*chSalt=*/ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_SALT_SIZE), /*nRounds=*/fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 25000), /*nDerivationMethod=*/derivation_method); }, [&] { - const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32); + const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE); const CKeyingMaterial new_key(random_vector.begin(), random_vector.end()); - const std::vector<unsigned char>& new_IV = ConsumeFixedLengthByteVector(fuzzed_data_provider, 16); + const std::vector<unsigned char>& new_IV = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_IV_SIZE); crypt.SetKey(new_key, new_IV); }, [&] { - const std::vector<unsigned char> random_vector = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<unsigned char> random_vector = ConsumeFixedLengthByteVector(fuzzed_data_provider, WALLET_CRYPTO_KEY_SIZE); plain_text_ed = CKeyingMaterial(random_vector.begin(), random_vector.end()); }, [&] { - cipher_text_ed = ConsumeRandomLengthByteVector(fuzzed_data_provider); + cipher_text_ed = ConsumeRandomLengthByteVector(fuzzed_data_provider, 64); }, [&] { (void)crypt.Encrypt(plain_text_ed, cipher_text_ed); @@ -82,7 +82,7 @@ FUZZ_TARGET(crypter, .init = initialize_crypter) } const CPubKey pub_key = *random_pub_key; const CKeyingMaterial master_key(random_key.begin(), random_key.end()); - const std::vector<unsigned char> crypted_secret = ConsumeRandomLengthByteVector(fuzzed_data_provider); + const std::vector<unsigned char> crypted_secret = ConsumeRandomLengthByteVector(fuzzed_data_provider, 64); CKey key; DecryptKey(master_key, crypted_secret, pub_key, key); }); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1e98cb0771..bb1789f109 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2225,8 +2225,8 @@ std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo // Complete if every input is now signed complete = true; - for (const auto& input : psbtx.inputs) { - complete &= PSBTInputSigned(input); + for (size_t i = 0; i < psbtx.inputs.size(); ++i) { + complete &= PSBTInputSignedAndVerified(psbtx, i, &txdata); } return {}; diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py index 5cd031f852..ed2328b76f 100755 --- a/test/functional/feature_bind_extra.py +++ b/test/functional/feature_bind_extra.py @@ -27,7 +27,7 @@ class BindExtraTest(BitcoinTestFramework): # Avoid any -bind= on the command line. Force the framework to avoid # adding -bind=127.0.0.1. self.bind_to_localhost_only = False - self.num_nodes = 2 + self.num_nodes = 3 def skip_test_if_missing_module(self): # Due to OS-specific network stats queries, we only run on Linux. @@ -60,14 +60,21 @@ class BindExtraTest(BitcoinTestFramework): ) port += 2 + # Node2, no -bind=...=onion, thus no extra port for Tor target. + self.expected.append( + [ + [f"-bind=127.0.0.1:{port}"], + [(loopback_ipv4, port)] + ], + ) + port += 1 + self.extra_args = list(map(lambda e: e[0], self.expected)) - self.add_nodes(self.num_nodes, self.extra_args) - # Don't start the nodes, as some of them would collide trying to bind on the same port. + self.setup_nodes() def run_test(self): - for i in range(len(self.expected)): - self.log.info(f"Starting node {i} with {self.expected[i][0]}") - self.start_node(i) + for i, (args, expected_services) in enumerate(self.expected): + self.log.info(f"Checking listening ports of node {i} with {args}") pid = self.nodes[i].process.pid binds = set(get_bind_addrs(pid)) # Remove IPv6 addresses because on some CI environments "::1" is not configured @@ -78,9 +85,7 @@ class BindExtraTest(BitcoinTestFramework): binds = set(filter(lambda e: len(e[0]) != ipv6_addr_len_bytes, binds)) # Remove RPC ports. They are not relevant for this test. binds = set(filter(lambda e: e[1] != rpc_port(i), binds)) - assert_equal(binds, set(self.expected[i][1])) - self.stop_node(i) - self.log.info(f"Stopped node {i}") + assert_equal(binds, set(expected_services)) if __name__ == '__main__': BindExtraTest().main() diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py index ceb9530394..e9658aa8d0 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -168,11 +168,20 @@ class PackageRBFTest(BitcoinTestFramework): self.assert_mempool_contents(expected=package_txns1) self.log.info("Check replacement pays for incremental bandwidth") - package_hex3, package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE) - pkg_results3 = node.submitpackage(package_hex3) - assert_equal(f"package RBF failed: insufficient anti-DoS fees, rejecting replacement {package_txns3[1].rehash()}, not enough additional fees to relay; 0.00 < 0.00000{sum([tx.get_vsize() for tx in package_txns3])}", pkg_results3["package_msg"]) - + _, placeholder_txns3 = self.create_simple_package(coin) + package_3_size = sum([tx.get_vsize() for tx in placeholder_txns3]) + incremental_sats_required = Decimal(package_3_size) / COIN + incremental_sats_short = incremental_sats_required - Decimal("0.00000001") + # Recreate the package with slightly higher fee once we know the size of the new package, but still short of required fee + failure_package_hex3, failure_package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE + incremental_sats_short) + assert_equal(package_3_size, sum([tx.get_vsize() for tx in failure_package_txns3])) + pkg_results3 = node.submitpackage(failure_package_hex3) + assert_equal(f"package RBF failed: insufficient anti-DoS fees, rejecting replacement {failure_package_txns3[1].rehash()}, not enough additional fees to relay; {incremental_sats_short} < {incremental_sats_required}", pkg_results3["package_msg"]) self.assert_mempool_contents(expected=package_txns1) + + success_package_hex3, success_package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE + incremental_sats_required) + node.submitpackage(success_package_hex3) + self.assert_mempool_contents(expected=success_package_txns3) self.generate(node, 1) self.log.info("Check Package RBF must have strict cpfp structure") @@ -180,11 +189,14 @@ class PackageRBFTest(BitcoinTestFramework): package_hex4, package_txns4 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE) node.submitpackage(package_hex4) self.assert_mempool_contents(expected=package_txns4) - package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE - Decimal("0.00000001")) + package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE) pkg_results5 = node.submitpackage(package_hex5) - assert 'package RBF failed: package feerate is less than parent feerate' in pkg_results5["package_msg"] - + assert 'package RBF failed: package feerate is less than or equal to parent feerate' in pkg_results5["package_msg"] self.assert_mempool_contents(expected=package_txns4) + + package_hex5_1, package_txns5_1 = self.create_simple_package(coin, parent_fee=DEFAULT_CHILD_FEE, child_fee=DEFAULT_CHILD_FEE + Decimal("0.00000001")) + node.submitpackage(package_hex5_1) + self.assert_mempool_contents(expected=package_txns5_1) self.generate(node, 1) def test_package_rbf_max_conflicts(self): diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py index bd766a279e..0775fd873d 100755 --- a/test/functional/p2p_add_connections.py +++ b/test/functional/p2p_add_connections.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2020-2021 The Bitcoin Core developers +# Copyright (c) 2020-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test add_outbound_p2p_connection test framework functionality""" @@ -11,6 +11,14 @@ from test_framework.util import ( check_node_connections, ) + +class VersionSender(P2PInterface): + def on_open(self): + assert self.on_connection_send_msg is not None + self.send_version() + assert self.on_connection_send_msg is None + + class P2PFeelerReceiver(P2PInterface): def on_version(self, message): # The bitcoind node closes feeler connections as soon as a version @@ -106,5 +114,19 @@ class P2PAddConnections(BitcoinTestFramework): # Feeler connections do not request tx relay assert_equal(feeler_conn.last_message["version"].relay, 0) + self.log.info("Send version message early to node") + # Normally the test framework would be shy and send the version message + # only after it received one. See the on_version method. Check that + # bitcoind behaves properly when a version is sent unexpectedly (but + # tolerably) early. + # + # This checks that bitcoind sends its own version prior to processing + # the remote version (and replying with a verack). Otherwise it would + # be violating its own rules, such as "non-version message before + # version handshake". + ver_conn = self.nodes[0].add_outbound_p2p_connection(VersionSender(), p2p_idx=6, connection_type="outbound-full-relay", supports_v2_p2p=False, advertise_v2_p2p=False) + ver_conn.sync_with_ping() + + if __name__ == '__main__': P2PAddConnections().main() diff --git a/test/functional/p2p_handshake.py b/test/functional/p2p_handshake.py index 21959ae522..9536e74893 100755 --- a/test/functional/p2p_handshake.py +++ b/test/functional/p2p_handshake.py @@ -17,6 +17,7 @@ from test_framework.messages import ( NODE_WITNESS, ) from test_framework.p2p import P2PInterface +from test_framework.util import p2p_port # Desirable service flags for outbound non-pruned and pruned peers. Note that @@ -88,9 +89,11 @@ class P2PHandshakeTest(BitcoinTestFramework): with node.assert_debug_log([f"feeler connection completed"]): self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True) - # TODO: re-add test introduced in commit 5d2fb14bafe4e80c0a482d99e5ebde07c477f000 - # ("test: p2p: check that connecting to ourself leads to disconnect") once - # the race condition causing issue #30368 is fixed + self.log.info("Check that connecting to ourself leads to immediate disconnect") + with node.assert_debug_log(["connected to self", "disconnecting"]): + node_listen_addr = f"127.0.0.1:{p2p_port(0)}" + node.addconnection(node_listen_addr, "outbound-full-relay", self.options.v2transport) + self.wait_until(lambda: len(node.getpeerinfo()) == 0) if __name__ == '__main__': diff --git a/test/functional/p2p_v2_misbehaving.py b/test/functional/p2p_v2_misbehaving.py index e45a63b3b0..0789425bcb 100755 --- a/test/functional/p2p_v2_misbehaving.py +++ b/test/functional/p2p_v2_misbehaving.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. import random +import time from enum import Enum from test_framework.messages import MAGIC_BYTES @@ -136,6 +137,7 @@ class EncryptedP2PMisbehaving(BitcoinTestFramework): self.log.info('Sending ellswift bytes in parts to ensure that response from responder is received only when') self.log.info('ellswift bytes have a mismatch from the 16 bytes(network magic followed by "version\\x00\\x00\\x00\\x00\\x00")') node0 = self.nodes[0] + node0.setmocktime(int(time.time())) self.log.info('Sending first 4 bytes of ellswift which match network magic') self.log.info('If a response is received, assertion failure would happen in our custom data_received() function') peer1 = node0.add_p2p_connection(MisbehavingV2Peer(TestType.EARLY_KEY_RESPONSE), wait_for_verack=False, send_version=False, supports_v2_p2p=True, wait_for_v2_handshake=False) @@ -143,9 +145,14 @@ class EncryptedP2PMisbehaving(BitcoinTestFramework): self.log.info('Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is') self.log.info('expected now, our custom data_received() function wouldn\'t result in assertion failure') peer1.v2_state.can_data_be_received = True + self.wait_until(lambda: peer1.v2_state.ellswift_ours) peer1.send_raw_message(peer1.v2_state.ellswift_ours[4:] + peer1.v2_state.sent_garbage) + # Ensure that the bytes sent after 4 bytes network magic are actually received. + self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4) + self.wait_until(lambda: node0.getpeerinfo()[-1]["bytessent"] > 0) with node0.assert_debug_log(['V2 handshake timeout peer=0']): - peer1.wait_for_disconnect(timeout=5) + node0.bumpmocktime(4) # `InactivityCheck()` triggers now + peer1.wait_for_disconnect(timeout=1) self.log.info('successful disconnection since modified ellswift was sent as response') def test_v2disconnection(self): @@ -154,7 +161,7 @@ class EncryptedP2PMisbehaving(BitcoinTestFramework): expected_debug_message = [ [], # EARLY_KEY_RESPONSE ["V2 transport error: missing garbage terminator, peer=1"], # EXCESS_GARBAGE - ["V2 handshake timeout peer=2"], # WRONG_GARBAGE_TERMINATOR + ["V2 handshake timeout peer=3"], # WRONG_GARBAGE_TERMINATOR ["V2 transport error: packet decryption failure"], # WRONG_GARBAGE ["V2 transport error: packet decryption failure"], # SEND_NO_AAD [], # SEND_NON_EMPTY_VERSION_PACKET @@ -167,8 +174,13 @@ class EncryptedP2PMisbehaving(BitcoinTestFramework): self.log.info(f"No disconnection for {test_type.name}") else: with node0.assert_debug_log(expected_debug_message[test_type.value], timeout=5): - peer = node0.add_p2p_connection(MisbehavingV2Peer(test_type), wait_for_verack=False, send_version=False, supports_v2_p2p=True, expect_success=False) - peer.wait_for_disconnect() + node0.setmocktime(int(time.time())) + peer1 = node0.add_p2p_connection(MisbehavingV2Peer(test_type), wait_for_verack=False, send_version=False, supports_v2_p2p=True, expect_success=False) + # Make a passing connection for more robust disconnection checking. + peer2 = node0.add_p2p_connection(P2PInterface()) + assert peer2.is_connected + node0.bumpmocktime(4) # `InactivityCheck()` triggers now + peer1.wait_for_disconnect() self.log.info(f"Expected disconnection for {test_type.name}") diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index 6759b69dd1..33f12484ad 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2020-2022 The Bitcoin Core developers +# Copyright (c) 2020-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test error messages for 'getaddressinfo' and 'validateaddress' RPC commands.""" @@ -12,6 +12,7 @@ from test_framework.util import ( ) BECH32_VALID = 'bcrt1qtmp74ayg7p24uslctssvjm06q5phz4yrxucgnv' +BECH32_VALID_UNKNOWN_WITNESS = 'bcrt1p424qxxyd0r' BECH32_VALID_CAPITALS = 'BCRT1QPLMTZKC2XHARPPZDLNPAQL78RSHJ68U33RAH7R' BECH32_VALID_MULTISIG = 'bcrt1qdg3myrgvzw7ml9q0ejxhlkyxm7vl9r56yzkfgvzclrf4hkpx9yfqhpsuks' @@ -80,6 +81,7 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): # Valid Bech32 self.check_valid(BECH32_VALID) + self.check_valid(BECH32_VALID_UNKNOWN_WITNESS) self.check_valid(BECH32_VALID_CAPITALS) self.check_valid(BECH32_VALID_MULTISIG) @@ -109,6 +111,7 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, BECH32_INVALID_PREFIX) assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", node.getaddressinfo, BASE58_INVALID_PREFIX) assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, INVALID_ADDRESS) + assert "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS) def run_test(self): self.test_validateaddress() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index a56960adff..8c3adff3cf 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -8,6 +8,9 @@ from decimal import Decimal from itertools import product from random import randbytes +from test_framework.blocktools import ( + MAX_STANDARD_TX_WEIGHT, +) from test_framework.descriptors import descsum_create from test_framework.key import H_POINT from test_framework.messages import ( @@ -16,6 +19,7 @@ from test_framework.messages import ( CTxIn, CTxOut, MAX_BIP125_RBF_SEQUENCE, + WITNESS_SCALE_FACTOR, ) from test_framework.psbt import ( PSBT, @@ -30,6 +34,7 @@ from test_framework.psbt import ( PSBT_OUT_TAP_TREE, ) from test_framework.script import CScript, OP_TRUE +from test_framework.script_util import MIN_STANDARD_TX_NONWITNESS_SIZE from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_approx, @@ -68,6 +73,28 @@ class PSBTTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_psbt_incomplete_after_invalid_modification(self): + self.log.info("Check that PSBT is correctly marked as incomplete after invalid modification") + node = self.nodes[2] + wallet = node.get_wallet_rpc(self.default_wallet_name) + address = wallet.getnewaddress() + wallet.sendtoaddress(address=address, amount=1.0) + self.generate(node, nblocks=1, sync_fun=lambda: self.sync_all(self.nodes[:2])) + + utxos = wallet.listunspent(addresses=[address]) + psbt = wallet.createpsbt([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{wallet.getnewaddress(): 0.9999}]) + signed_psbt = wallet.walletprocesspsbt(psbt)["psbt"] + + # Modify the raw transaction by changing the output address, so the signature is no longer valid + signed_psbt_obj = PSBT.from_base64(signed_psbt) + substitute_addr = wallet.getnewaddress() + raw = wallet.createrawtransaction([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{substitute_addr: 0.9999}]) + signed_psbt_obj.g.map[PSBT_GLOBAL_UNSIGNED_TX] = bytes.fromhex(raw) + + # Check that the walletprocesspsbt call succeeds but also recognizes that the transaction is not complete + signed_psbt_incomplete = wallet.walletprocesspsbt(signed_psbt_obj.to_base64(), finalize=False) + assert signed_psbt_incomplete["complete"] is False + def test_utxo_conversion(self): self.log.info("Check that non-witness UTXOs are removed for segwit v1+ inputs") mining_node = self.nodes[2] @@ -186,6 +213,46 @@ class PSBTTest(BitcoinTestFramework): # Create and fund a raw tx for sending 10 BTC psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt'] + self.log.info("Test for invalid maximum transaction weights") + dest_arg = [{self.nodes[0].getnewaddress(): 1}] + min_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR + assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": -1}) + assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": 0}) + assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": MAX_STANDARD_TX_WEIGHT + 1}) + + # Base transaction vsize: version (4) + locktime (4) + input count (1) + witness overhead (1) = 10 vbytes + base_tx_vsize = 10 + # One P2WPKH output vsize: outpoint (31 vbytes) + p2wpkh_output_vsize = 31 + # 1 vbyte for output count + output_count = 1 + tx_weight_without_inputs = (base_tx_vsize + output_count + p2wpkh_output_vsize) * WITNESS_SCALE_FACTOR + # min_tx_weight is greater than transaction weight without inputs + assert_greater_than(min_tx_weight, tx_weight_without_inputs) + + # In order to test for when the passed max weight is less than the transaction weight without inputs + # Define destination with two outputs. + dest_arg_large = [{self.nodes[0].getnewaddress(): 1}, {self.nodes[0].getnewaddress(): 1}] + large_tx_vsize_without_inputs = base_tx_vsize + output_count + (p2wpkh_output_vsize * 2) + large_tx_weight_without_inputs = large_tx_vsize_without_inputs * WITNESS_SCALE_FACTOR + assert_greater_than(large_tx_weight_without_inputs, min_tx_weight) + # Test for max_tx_weight less than Transaction weight without inputs + assert_raises_rpc_error(-4, "Maximum transaction weight is less than transaction weight without inputs", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": min_tx_weight}) + assert_raises_rpc_error(-4, "Maximum transaction weight is less than transaction weight without inputs", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": large_tx_weight_without_inputs}) + + # Test for max_tx_weight just enough to include inputs but not change output + assert_raises_rpc_error(-4, "Maximum transaction weight is too low, can not accommodate change output", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": (large_tx_vsize_without_inputs + 1) * WITNESS_SCALE_FACTOR}) + self.log.info("Test that a funded PSBT is always faithful to max_tx_weight option") + large_tx_vsize_with_change = large_tx_vsize_without_inputs + p2wpkh_output_vsize + # It's enough but won't accommodate selected input size + assert_raises_rpc_error(-4, "The inputs size exceeds the maximum weight", self.nodes[0].walletcreatefundedpsbt, [], dest_arg_large, 0, {"max_tx_weight": (large_tx_vsize_with_change) * WITNESS_SCALE_FACTOR}) + + max_tx_weight_sufficient = 1000 # 1k vbytes is enough + psbt = self.nodes[0].walletcreatefundedpsbt(outputs=dest_arg,locktime=0, options={"max_tx_weight": max_tx_weight_sufficient})["psbt"] + weight = self.nodes[0].decodepsbt(psbt)["tx"]["weight"] + # ensure the transaction's weight is below the specified max_tx_weight. + assert_greater_than_or_equal(max_tx_weight_sufficient, weight) + # If inputs are specified, do not automatically add more: utxo1 = self.nodes[0].listunspent()[0] assert_raises_rpc_error(-4, "The preselected coins total amount does not cover the transaction target. " @@ -589,6 +656,7 @@ class PSBTTest(BitcoinTestFramework): if self.options.descriptors: self.test_utxo_conversion() + self.test_psbt_incomplete_after_invalid_modification() self.test_input_confs_control() diff --git a/test/functional/test-shell.md b/test/functional/test-shell.md index 4cd62c4ef3..d79c4a0ab6 100644 --- a/test/functional/test-shell.md +++ b/test/functional/test-shell.md @@ -169,7 +169,7 @@ can be called after the TestShell is shut down. | Test parameter key | Default Value | Description | |---|---|---| -| `bind_to_localhost_only` | `True` | Binds bitcoind RPC services to `127.0.0.1` if set to `True`.| +| `bind_to_localhost_only` | `True` | Binds bitcoind P2P services to `127.0.0.1` if set to `True`.| | `cachedir` | `"/path/to/bitcoin/test/cache"` | Sets the bitcoind datadir directory. | | `chain` | `"regtest"` | Sets the chain-type for the underlying test bitcoind processes. | | `configfile` | `"/path/to/bitcoin/test/config.ini"` | Sets the location of the test framework config file. | diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index f0dc866f69..338b7fa3b0 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -48,6 +48,7 @@ from .util import assert_equal MAX_BLOCK_SIGOPS = 20000 MAX_BLOCK_SIGOPS_WEIGHT = MAX_BLOCK_SIGOPS * WITNESS_SCALE_FACTOR +MAX_STANDARD_TX_WEIGHT = 400000 # Genesis block time (regtest) TIME_GENESIS_BLOCK = 1296688602 diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 0f0083191d..b73566b0e9 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -39,6 +39,7 @@ from .util import ( rpc_url, wait_until_helper_internal, p2p_port, + tor_port, ) BITCOIND_PROC_WAIT_TIMEOUT = 60 @@ -88,8 +89,11 @@ class TestNode(): self.coverage_dir = coverage_dir self.cwd = cwd self.descriptors = descriptors + self.has_explicit_bind = False if extra_conf is not None: append_config(self.datadir_path, extra_conf) + # Remember if there is bind=... in the config file. + self.has_explicit_bind = any(e.startswith("bind=") for e in extra_conf) # Most callers will just need to add extra args to the standard list below. # For those callers that need more flexibility, they can just set the args property directly. # Note that common args are set in the config file (see initialize_datadir) @@ -210,6 +214,17 @@ class TestNode(): if extra_args is None: extra_args = self.extra_args + # If listening and no -bind is given, then bitcoind would bind P2P ports on + # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is + # a unique port chosen by the test framework and configured as port=P in + # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to + # 127.0.0.1:tor_port(). + will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args) + has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args) + if will_listen and not has_explicit_bind: + extra_args.append(f"-bind=0.0.0.0:{p2p_port(self.index)}") + extra_args.append(f"-bind=127.0.0.1:{tor_port(self.index)}=onion") + self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args) # Add a new stdout and stderr file each time bitcoind is started diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index f3d080fdde..de756691e0 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -316,9 +316,9 @@ def sha256sum_file(filename): # The maximum number of nodes a single test can spawn MAX_NODES = 12 -# Don't assign rpc or p2p ports lower than this +# Don't assign p2p, rpc or tor ports lower than this PORT_MIN = int(os.getenv('TEST_RUNNER_PORT_MIN', default=11000)) -# The number of ports to "reserve" for p2p and rpc, each +# The number of ports to "reserve" for p2p, rpc and tor, each PORT_RANGE = 5000 @@ -358,7 +358,11 @@ def p2p_port(n): def rpc_port(n): - return PORT_MIN + PORT_RANGE + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES) + return p2p_port(n) + PORT_RANGE + + +def tor_port(n): + return p2p_port(n) + PORT_RANGE * 2 def rpc_url(datadir, i, chain, rpchost): diff --git a/test/lint/lint-assertions.py b/test/lint/lint-assertions.py index d9f86b22b8..5d01b13fd4 100755 --- a/test/lint/lint-assertions.py +++ b/test/lint/lint-assertions.py @@ -27,8 +27,9 @@ def main(): # checks should be used over assert. See: src/util/check.h # src/rpc/server.cpp is excluded from this check since it's mostly meta-code. exit_code = git_grep([ - "-nE", - r"\<(A|a)ss(ume|ert) *\(.*\);", + "--line-number", + "--extended-regexp", + r"\<(A|a)ss(ume|ert)\(", "--", "src/rpc/", "src/wallet/rpc*", @@ -38,8 +39,9 @@ def main(): # The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`, # which is an unnecessary Boost dependency. exit_code |= git_grep([ - "-E", - r"BOOST_ASSERT *\(.*\);", + "--line-number", + "--extended-regexp", + r"BOOST_ASSERT\(", "--", "*.cpp", "*.h", |