diff options
105 files changed, 802 insertions, 487 deletions
diff --git a/.travis/lint_04_install.sh b/.travis/lint_04_install.sh index 34118a57c3..3a6ff73d79 100755 --- a/.travis/lint_04_install.sh +++ b/.travis/lint_04_install.sh @@ -6,4 +6,5 @@ export LC_ALL=C +travis_retry pip install codespell travis_retry pip install flake8 diff --git a/appveyor.yml b/appveyor.yml index 85b1a999d1..99e7b9510b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,36 +5,41 @@ configuration: Release platform: x64 environment: APPVEYOR_SAVE_CACHE_ON_ERROR: true -cache: C:\tools\vcpkg\installed\ + CLCACHE_SERVER: 1 + PACKAGES: boost-filesystem boost-signals2 boost-interprocess boost-test libevent openssl zeromq berkeleydb secp256k1 leveldb +cache: +- C:\tools\vcpkg\installed +- C:\Users\appveyor\clcache +- build_msvc\cache init: -- cmd: set PATH=C:\Python36-x64;%PATH% +- cmd: set PATH=C:\Python36-x64;C:\Python36-x64\Scripts;%PATH% +install: +- cmd: pip install git+https://github.com/frerich/clcache.git +- ps: $packages = $env:PACKAGES -Split ' ' +- ps: for ($i=0; $i -lt $packages.length; $i++) { + $env:ALL_PACKAGES += $packages[$i] + ":" + $env:PLATFORM + "-windows-static " + } +- cmd: git -C C:\Tools\vcpkg pull # This is a temporary fix, can be removed after appveyor update its image to include Microsoft/vcpkg#4046 +- cmd: vcpkg install %ALL_PACKAGES% +- cmd: vcpkg upgrade --no-dry-run +- cmd: del /s /q C:\Tools\vcpkg\installed\%PLATFORM%-windows-static\debug # Remove unused debug library before_build: -- ps: >- - $packages = @( - "boost-filesystem", - "boost-signals2", - "boost-interprocess", - "boost-test", - "libevent", - "openssl", - "zeromq", - "berkeleydb", - "secp256k1", - "leveldb" - ) - - for ($i=0; $i -lt $packages.length; $i++) { - $all_packages += $packages[$i] + ":" + $env:PLATFORM + "-windows-static " - } - - git -C C:\Tools\vcpkg pull # This is a temporary fix, can be removed after appveyor update its image to include Microsoft/vcpkg#4046 - - Invoke-Expression -Command "vcpkg install $all_packages" +- cmd: if not exist build_msvc\cache\ (del build_msvc\cache & mkdir build_msvc\cache) +- cmd: if not exist build_msvc\%PLATFORM%\%CONFIGURATION%\ (mkdir build_msvc\%PLATFORM%\%CONFIGURATION%) +- cmd: if exist build_msvc\cache\*.iobj (move build_msvc\cache\* build_msvc\%PLATFORM%\%CONFIGURATION%\) +- cmd: clcache -M 2147483648 - cmd: python build_msvc\msvc-autogen.py -build: - project: build_msvc\bitcoin.sln - parallel: true - verbosity: minimal +- ps: $files = (Get-ChildItem -Recurse | where {$_.extension -eq ".vcxproj"}).FullName +- ps: for ($i = 0; $i -lt $files.length; $i++) { + (Get-Content $files[$i]).Replace("</RuntimeLibrary>", "</RuntimeLibrary><DebugInformationFormat>None</DebugInformationFormat>").Replace("NDEBUG;", "") | Set-Content $files[$i] + } +- ps: Start-Process clcache-server +build_script: +- cmd: msbuild /p:TrackFileAccess=false /p:CLToolExe=clcache.exe build_msvc\bitcoin.sln /m /v:q /nowarn:C4244;C4267;C4715 /nologo +after_build: +- cmd: move build_msvc\%PLATFORM%\%CONFIGURATION%\*.iobj build_msvc\cache\ +- cmd: move build_msvc\%PLATFORM%\%CONFIGURATION%\*.ipdb build_msvc\cache\ +- cmd: del C:\Users\appveyor\clcache\stats.txt test_script: -- cmd: build_msvc\%PLATFORM%\Release\test_bitcoin.exe +- cmd: build_msvc\%PLATFORM%\%CONFIGURATION%\test_bitcoin.exe deploy: off diff --git a/build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj b/build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj index e6f4885e5e..0be7e7e430 100644 --- a/build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj +++ b/build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj @@ -127,7 +127,7 @@ <PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions> <SDLCheck>false</SDLCheck> <AdditionalIncludeDirectories>..\..\src;</AdditionalIncludeDirectories> - <ExceptionHandling>false</ExceptionHandling> + <ExceptionHandling>Sync</ExceptionHandling> <SuppressStartupBanner>false</SuppressStartupBanner> <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> </ClCompile> @@ -144,7 +144,7 @@ <PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions> <SDLCheck>false</SDLCheck> <AdditionalIncludeDirectories>..\..\src;</AdditionalIncludeDirectories> - <ExceptionHandling>false</ExceptionHandling> + <ExceptionHandling>Sync</ExceptionHandling> <SuppressStartupBanner>false</SuppressStartupBanner> <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> </ClCompile> @@ -163,7 +163,7 @@ <PreprocessorDefinitions>WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions> <SDLCheck>false</SDLCheck> <AdditionalIncludeDirectories>..\..\src;</AdditionalIncludeDirectories> - <ExceptionHandling>false</ExceptionHandling> + <ExceptionHandling>Sync</ExceptionHandling> <SuppressStartupBanner>false</SuppressStartupBanner> <RuntimeLibrary>MultiThreaded</RuntimeLibrary> </ClCompile> @@ -184,7 +184,7 @@ <PreprocessorDefinitions>WIN32;NDEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions> <SDLCheck>false</SDLCheck> <AdditionalIncludeDirectories>..\..\src;</AdditionalIncludeDirectories> - <ExceptionHandling>false</ExceptionHandling> + <ExceptionHandling>Sync</ExceptionHandling> <SuppressStartupBanner>false</SuppressStartupBanner> <RuntimeLibrary>MultiThreaded</RuntimeLibrary> </ClCompile> diff --git a/configure.ac b/configure.ac index 4275796b50..0b96e15b9e 100644 --- a/configure.ac +++ b/configure.ac @@ -130,6 +130,12 @@ AC_ARG_ENABLE(gui-tests, [use_gui_tests=$enableval], [use_gui_tests=$use_tests]) +AC_ARG_WITH([rapidcheck], + [AS_HELP_STRING([--with-rapidcheck], + [enable RapidCheck property based tests (default is yes if librapidcheck is found)])], + [use_rapidcheck=$withval], + [use_rapidcheck=auto]) + AC_ARG_ENABLE(bench, AS_HELP_STRING([--disable-bench],[do not compile benchmarks (default is to compile)]), [use_bench=$enableval], @@ -1134,6 +1140,22 @@ AC_CHECK_DECLS([EVP_MD_CTX_new],,,[AC_INCLUDES_DEFAULT ]) CXXFLAGS="${save_CXXFLAGS}" +dnl RapidCheck Property Based Testing + +enable_property_tests=no +if test "x$use_rapidcheck" = xauto; then + AC_CHECK_HEADERS([rapidcheck.h], [enable_property_tests=yes]) +elif test "x$use_rapidcheck" != xno; then + enable_property_tests=yes +fi + +RAPIDCHECK_LIBS= +if test "x$enable_property_tests" = xyes; then + RAPIDCHECK_LIBS=-lrapidcheck +fi +AC_SUBST(RAPIDCHECK_LIBS) +AM_CONDITIONAL([ENABLE_PROPERTY_TESTS], [test x$enable_property_tests = xyes]) + dnl univalue check need_bundled_univalue=yes diff --git a/contrib/debian/copyright b/contrib/debian/copyright index 21cca7d9ac..e5b9cbaa40 100644 --- a/contrib/debian/copyright +++ b/contrib/debian/copyright @@ -96,7 +96,7 @@ Comment: Site: https://bitcointalk.org/?topic=1756.0 Files: src/qt/res/icons/proxy.png src/qt/res/src/proxy.svg Copyright: Cristian Mircea Messel -Licese: public-domain +License: public-domain License: Expat diff --git a/contrib/devtools/optimize-pngs.py b/contrib/devtools/optimize-pngs.py index 8b1f41f7e1..e9481dbbcf 100755 --- a/contrib/devtools/optimize-pngs.py +++ b/contrib/devtools/optimize-pngs.py @@ -27,7 +27,7 @@ def content_hash(filename): pngcrush = 'pngcrush' git = 'git' folders = ["src/qt/res/movies", "src/qt/res/icons", "share/pixmaps"] -basePath = subprocess.check_output([git, 'rev-parse', '--show-toplevel'], universal_newlines=True).rstrip('\n') +basePath = subprocess.check_output([git, 'rev-parse', '--show-toplevel'], universal_newlines=True, encoding='utf8').rstrip('\n') totalSaveBytes = 0 noHashChange = True @@ -50,7 +50,7 @@ for folder in folders: sys.exit(0) #verify - if "Not a PNG file" in subprocess.check_output([pngcrush, "-n", "-v", file_path], stderr=subprocess.STDOUT, universal_newlines=True): + if "Not a PNG file" in subprocess.check_output([pngcrush, "-n", "-v", file_path], stderr=subprocess.STDOUT, universal_newlines=True, encoding='utf8'): print("PNG file "+file+" is corrupted after crushing, check out pngcursh version") sys.exit(1) diff --git a/contrib/gitian-build.py b/contrib/gitian-build.py index 0e53c3dfd5..2e8c99247d 100755 --- a/contrib/gitian-build.py +++ b/contrib/gitian-build.py @@ -170,7 +170,7 @@ def main(): args.sign_prog = 'true' if args.detach_sign else 'gpg --detach-sign' - # Set enviroment variable USE_LXC or USE_DOCKER, let gitian-builder know that we use lxc or docker + # Set environment variable USE_LXC or USE_DOCKER, let gitian-builder know that we use lxc or docker if args.docker: os.environ['USE_DOCKER'] = '1' elif not args.kvm: @@ -209,7 +209,7 @@ def main(): subprocess.check_call(['git', 'fetch', args.url, 'refs/pull/'+args.version+'/merge']) os.chdir('../gitian-builder/inputs/bitcoin') subprocess.check_call(['git', 'fetch', args.url, 'refs/pull/'+args.version+'/merge']) - args.commit = subprocess.check_output(['git', 'show', '-s', '--format=%H', 'FETCH_HEAD'], universal_newlines=True).strip() + args.commit = subprocess.check_output(['git', 'show', '-s', '--format=%H', 'FETCH_HEAD'], universal_newlines=True, encoding='utf8').strip() args.version = 'pull-' + args.version print(args.commit) subprocess.check_call(['git', 'fetch']) diff --git a/contrib/verify-commits/verify-commits.py b/contrib/verify-commits/verify-commits.py index a9e4977715..544f4dc48d 100755 --- a/contrib/verify-commits/verify-commits.py +++ b/contrib/verify-commits/verify-commits.py @@ -91,7 +91,7 @@ def main(): no_sha1 = True prev_commit = "" initial_commit = current_commit - branch = subprocess.check_output([GIT, 'show', '-s', '--format=%H', initial_commit], universal_newlines=True).splitlines()[0] + branch = subprocess.check_output([GIT, 'show', '-s', '--format=%H', initial_commit], universal_newlines=True, encoding='utf8').splitlines()[0] # Iterate through commits while True: @@ -112,7 +112,7 @@ def main(): if prev_commit != "": print("No parent of {} was signed with a trusted key!".format(prev_commit), file=sys.stderr) print("Parents are:", file=sys.stderr) - parents = subprocess.check_output([GIT, 'show', '-s', '--format=format:%P', prev_commit], universal_newlines=True).splitlines()[0].split(' ') + parents = subprocess.check_output([GIT, 'show', '-s', '--format=format:%P', prev_commit], universal_newlines=True, encoding='utf8').splitlines()[0].split(' ') for parent in parents: subprocess.call([GIT, 'show', '-s', parent], stdout=sys.stderr) else: @@ -122,25 +122,25 @@ def main(): # Check the Tree-SHA512 if (verify_tree or prev_commit == "") and current_commit not in incorrect_sha512_allowed: tree_hash = tree_sha512sum(current_commit) - if ("Tree-SHA512: {}".format(tree_hash)) not in subprocess.check_output([GIT, 'show', '-s', '--format=format:%B', current_commit], universal_newlines=True).splitlines(): + if ("Tree-SHA512: {}".format(tree_hash)) not in subprocess.check_output([GIT, 'show', '-s', '--format=format:%B', current_commit], universal_newlines=True, encoding='utf8').splitlines(): print("Tree-SHA512 did not match for commit " + current_commit, file=sys.stderr) sys.exit(1) # Merge commits should only have two parents - parents = subprocess.check_output([GIT, 'show', '-s', '--format=format:%P', current_commit], universal_newlines=True).splitlines()[0].split(' ') + parents = subprocess.check_output([GIT, 'show', '-s', '--format=format:%P', current_commit], universal_newlines=True, encoding='utf8').splitlines()[0].split(' ') if len(parents) > 2: print("Commit {} is an octopus merge".format(current_commit), file=sys.stderr) sys.exit(1) # Check that the merge commit is clean - commit_time = int(subprocess.check_output([GIT, 'show', '-s', '--format=format:%ct', current_commit], universal_newlines=True).splitlines()[0]) + commit_time = int(subprocess.check_output([GIT, 'show', '-s', '--format=format:%ct', current_commit], universal_newlines=True, encoding='utf8').splitlines()[0]) check_merge = commit_time > time.time() - args.clean_merge * 24 * 60 * 60 # Only check commits in clean_merge days allow_unclean = current_commit in unclean_merge_allowed if len(parents) == 2 and check_merge and not allow_unclean: - current_tree = subprocess.check_output([GIT, 'show', '--format=%T', current_commit], universal_newlines=True).splitlines()[0] + current_tree = subprocess.check_output([GIT, 'show', '--format=%T', current_commit], universal_newlines=True, encoding='utf8').splitlines()[0] subprocess.call([GIT, 'checkout', '--force', '--quiet', parents[0]]) subprocess.call([GIT, 'merge', '--no-ff', '--quiet', parents[1]], stdout=subprocess.DEVNULL) - recreated_tree = subprocess.check_output([GIT, 'show', '--format=format:%T', 'HEAD'], universal_newlines=True).splitlines()[0] + recreated_tree = subprocess.check_output([GIT, 'show', '--format=format:%T', 'HEAD'], universal_newlines=True, encoding='utf8').splitlines()[0] if current_tree != recreated_tree: print("Merge commit {} is not clean".format(current_commit), file=sys.stderr) subprocess.call([GIT, 'diff', current_commit]) diff --git a/depends/Makefile b/depends/Makefile index 3686aaf1f8..da81c4c7ee 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -5,6 +5,7 @@ WORK_PATH = $(BASEDIR)/work BASE_CACHE ?= $(BASEDIR)/built SDK_PATH ?= $(BASEDIR)/SDKs NO_QT ?= +RAPIDCHECK ?= NO_WALLET ?= NO_UPNP ?= FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources @@ -93,6 +94,8 @@ qt_packages_$(NO_QT) = $(qt_packages) $(qt_$(host_os)_packages) $(qt_$(host_arch wallet_packages_$(NO_WALLET) = $(wallet_packages) upnp_packages_$(NO_UPNP) = $(upnp_packages) +rapidcheck_packages_$(RAPIDCHECK) = $(rapidcheck_packages) + packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages) @@ -100,6 +103,10 @@ ifneq ($(qt_packages_),) native_packages += $(qt_native_packages) endif +ifeq ($(rapidcheck_packages_),) +packages += $(rapidcheck_packages) +endif + all_packages = $(packages) $(native_packages) meta_depends = Makefile funcs.mk builders/default.mk hosts/default.mk hosts/$(host_os).mk builders/$(build_os).mk diff --git a/depends/README.md b/depends/README.md index fd343f7010..73bfc8b726 100644 --- a/depends/README.md +++ b/depends/README.md @@ -63,6 +63,7 @@ The following can be set when running make: make FOO=bar NO_WALLET: Don't download/build/cache libs needed to enable the wallet NO_UPNP: Don't download/build/cache packages needed for enabling upnp DEBUG: disable some optimizations and enable more runtime checking + RAPIDCHECK: build rapidcheck (experimental) HOST_ID_SALT: Optional salt to use when generating host package ids BUILD_ID_SALT: Optional salt to use when generating build package ids diff --git a/depends/packages/packages.mk b/depends/packages/packages.mk index 5fe6f98da2..38329d16d7 100644 --- a/depends/packages/packages.mk +++ b/depends/packages/packages.mk @@ -5,6 +5,8 @@ qt_packages = qrencode protobuf zlib qt_linux_packages:=qt expat dbus libxcb xcb_proto libXau xproto freetype fontconfig libX11 xextproto libXext xtrans +rapidcheck_packages = rapidcheck + qt_darwin_packages=qt qt_mingw32_packages=qt diff --git a/depends/packages/rapidcheck.mk b/depends/packages/rapidcheck.mk new file mode 100644 index 0000000000..19cf1cae2e --- /dev/null +++ b/depends/packages/rapidcheck.mk @@ -0,0 +1,18 @@ +package=rapidcheck +$(package)_version=10fc0cb +$(package)_download_path=https://github.com/MarcoFalke/rapidcheck/archive +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=9640926223c00af45bce4c7df8b756b5458a89b2ba74cfe3e404467f13ce26df + +define $(package)_config_cmds + cmake -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=true . +endef + +define $(package)_build_cmds + $(MAKE) && \ + mkdir -p $($(package)_staging_dir)$(host_prefix)/include && \ + cp -a include/* $($(package)_staging_dir)$(host_prefix)/include/ && \ + cp -a extras/boost_test/include/rapidcheck/* $($(package)_staging_dir)$(host_prefix)/include/rapidcheck/ && \ + mkdir -p $($(package)_staging_dir)$(host_prefix)/lib && \ + cp -a librapidcheck.a $($(package)_staging_dir)$(host_prefix)/lib/ +endef diff --git a/doc/build-osx.md b/doc/build-osx.md index 448918e8cb..1fa01d0d6e 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -64,6 +64,17 @@ Build Bitcoin Core make deploy +Disable-wallet mode +-------------------- +When the intention is to run only a P2P node without a wallet, Bitcoin Core may be compiled in +disable-wallet mode with: + + ./configure --disable-wallet + +In this case there is no dependency on Berkeley DB 4.8. + +Mining is also possible in disable-wallet mode using the `getblocktemplate` RPC call. + Running ------- diff --git a/doc/build-unix.md b/doc/build-unix.md index a01ff59fa6..337de12b8d 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -234,8 +234,7 @@ disable-wallet mode with: In this case there is no dependency on Berkeley DB 4.8. -Mining is also possible in disable-wallet mode, but only using the `getblocktemplate` RPC -call not `getwork`. +Mining is also possible in disable-wallet mode using the `getblocktemplate` RPC call. Additional Configure Flags -------------------------- @@ -284,4 +283,3 @@ To build executables for ARM: For further documentation on the depends system see [README.md](../depends/README.md) in the depends directory. - diff --git a/doc/descriptors.md b/doc/descriptors.md new file mode 100644 index 0000000000..c23ac06e8f --- /dev/null +++ b/doc/descriptors.md @@ -0,0 +1,124 @@ +# Support for Output Descriptors in Bitcoin Core + +Since Bitcoin Core v0.17, there is support for Output Descriptors in the +`scantxoutset` RPC call. This is a simple language which can be used to +describe collections of output scripts. + +This document describes the language. For the specifics on usage for scanning +the UTXO set, see the `scantxoutset` RPC help. + +## Features + +Output descriptors currently support: +- Pay-to-pubkey scripts (P2PK), through the `pk` function. +- Pay-to-pubkey-hash scripts (P2PKH), through the `pkh` function. +- Pay-to-witness-pubkey-hash scripts (P2WPKH), through the `wpkh` function. +- Pay-to-script-hash scripts (P2SH), through the `sh` function. +- Pay-to-witness-script-hash scripts (P2WSH), through the `wsh` function. +- Multisig scripts, through the `multi` function. +- Any type of supported address through the `addr` function. +- Raw hex scripts through the `raw` function. +- Public keys (compressed and uncompressed) in hex notation, or BIP32 extended pubkeys with derivation paths. + +## Examples + +- `pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)` represents a P2PK output. +- `pkh(02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5)` represents a P2PKH output. +- `wpkh(02f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9)` represents a P2WPKH output. +- `sh(wpkh(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556))` represents a P2SH-P2WPKH output. +- `combo(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)` represents a P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH output. +- `sh(wsh(pkh(02e493dbf1c10d80f3581e4904930b1404cc6c13900ee0758474fa94abe8c4cd13)))` represents a (overly complicated) P2SH-P2WSH-P2PKH output. +- `multi(1,022f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe4,025cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc)` represents a bare *1-of-2* multisig. +- `sh(multi(2,022f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01,03acd484e2f0c7f65309ad178a9f559abde09796974c57e714c35f110dfc27ccbe))` represents a P2SH *2-of-2* multisig. +- `wsh(multi(2,03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7,03774ae7f858a9411e5ef4246b70c65aac5649980be5c17891bbec17895da008cb,03d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85a))` represents a P2WSH *2-of-3* multisig. +- `sh(wsh(multi(1,03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8,03499fdf9e895e719cfd64e67f07d38e3226aa7b63678949e6e49b241a60e823e4,02d7924d4f7d43ea965a465ae3095ff41131e5946f3c85f79e44adbcf8e27e080e)))` represents a P2SH-P2WSH *1-of-3* multisig. +- `pk(xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8)` refers to a single P2PK output, using the public key part from the specified xpub. +- `pkh(xpub68Gmy5EdvgibQVfPdqkBBCHxA5htiqg55crXYuXoQRKfDBFA1WEjWgP6LHhwBZeNK1VTsfTFUHCdrfp1bgwQ9xv5ski8PX9rL2dZXvgGDnw/1'/2)` refers to a single P2PKH output, using child key *1'/2* of the specified xpub. +- `wsh(multi(1,xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/1/0/*,xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH/1/0/*))` refers to a chain of *1-of-2* P2WSH multisig outputs, using public keys taken from two HD chains with corresponding derivation paths. + +## Reference + +Descriptors consist of several types of expressions. The top level expression is always a `SCRIPT`. + +`SCRIPT` expressions: +- `sh(SCRIPT)` (top level only): P2SH embed the argument. +- `wsh(SCRIPT)` (not inside another 'wsh'): P2WSH embed the argument. +- `pk(KEY)` (anywhere): P2PK output for the given public key. +- `pkh(KEY)` (anywhere): P2PKH output for the given public key (use `addr` if you only know the pubkey hash). +- `wpkh(KEY)` (not inside `wsh`): P2WPKH output for the given compressed pubkey. +- `combo(KEY)` (top level only): an alias for the collection of `pk(KEY)` and `pkh(KEY)`. If the key is compressed, it also includes `wpkh(KEY)` and `sh(wpkh(KEY))`. +- `multi(k,KEY_1,KEY_2,...,KEY_n)` (anywhere): k-of-n multisig script. +- `addr(ADDR)` (top level only): the script which ADDR expands to. +- `raw(HEX)` (top level only): the script whose hex encoding is HEX. + +`KEY` expressions: +- Hex encoded public keys (66 characters starting with `02` or `03`, or 130 characters starting with `04`). + - Inside `wpkh` and `wsh`, only compressed public keys are permitted. +- [WIF](https://en.bitcoin.it/wiki/Wallet_import_format) encoded private keys may be specified instead of the corresponding public key, with the same meaning. +-`xpub` encoded extended public key or `xprv` encoded private key (as defined in [BIP 32](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki)). + - Followed by zero or more `/NUM` unhardened and `/NUM'` hardened BIP32 derivation steps. + - Optionally followed by a single `/*` or `/*'` final step to denote all (direct) unhardened or hardened children. + - The usage of hardened derivation steps requires providing the private key. + - Instead of a `'`, the suffix `h` can be used to denote hardened derivation. + +`ADDR` expressions are any type of supported address: +- P2PKH addresses (base58, of the form `1...`). Note that P2PKH addresses in descriptors cannot be used for P2PK outputs (use the `pk` function instead). +- P2SH addresses (base58, of the form `3...`, defined in [BIP 13](https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki)). +- Segwit addresses (bech32, of the form `bc1...`, defined in [BIP 173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki)). + +## Explanation + +### Single-key scripts + +Many single-key constructions are used in practice, generally including +P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH. Many more combinations are +imaginable, though they may not be optimal: P2SH-P2PK, P2SH-P2PKH, +P2WSH-P2PK, P2WSH-P2PKH, P2SH-P2WSH-P2PK, P2SH-P2WSH-P2PKH. + +To describe these, we model these as functions. The functions `pk` +(P2PK), `pkh` (P2PKH) and `wpkh` (P2WPKH) take as input a public key in +hexadecimal notation (which will be extended later), and return the +corresponding *scriptPubKey*. The functions `sh` (P2SH) and `wsh` (P2WSH) +take as input a script, and return the script describing P2SH and P2WSH +outputs with the input as embedded script. The names of the functions do +not contain "p2" for brevity. + +### Multisig + +Several pieces of software use multi-signature (multisig) scripts based +on Bitcoin's OP_CHECKMULTISIG opcode. To support these, we introduce the +`multi(k,key_1,key_2,...,key_n)` function. It represents a *k-of-n* +multisig policy, where any *k* out of the *n* provided public keys must +sign. + +### BIP32 derived keys and chains + +Most modern wallet software and hardware uses keys that are derived using +BIP32 ("HD keys"). We support these directly by permitting strings +consisting of an extended public key (commonly referred to as an *xpub*) +plus derivation path anywhere a public key is expected. The derivation +path consists of a sequence of 0 or more integers (in the range +*0..2<sup>31</sup>-1*) each optionally followed by `'` or `h`, and +separated by `/` characters. The string may optionally end with the +literal `/*` or `/*'` (or `/*h`) to refer to all unhardened or hardened +child keys instead. + +Whenever a public key is described using a hardened derivation step, the +script cannot be computed without access to the corresponding private +key. + +### Including private keys + +Often it is useful to communicate a description of scripts along with the +necessary private keys. For this reason, anywhere a public key or xpub is +supported, a private key in WIF format or xprv may be provided instead. +This is useful when private keys are necessary for hardened derivation +steps, or for dumping wallet descriptors including private key material. + +### Compatibility with old wallets + +In order to easily represent the sets of scripts currently supported by +existing Bitcoin Core wallets, a convenience function `combo` is +provided, which takes as input a public key, and constructs the P2PK, +P2PKH, P2WPKH, and P2SH-P2WPH scripts for that key. In case the key is +uncompressed, it only constructs P2PK and P2PKH. diff --git a/doc/tor.md b/doc/tor.md index 2d0676c89a..dc0b88618a 100644 --- a/doc/tor.md +++ b/doc/tor.md @@ -93,7 +93,7 @@ API, to create and destroy 'ephemeral' hidden services programmatically. Bitcoin Core has been updated to make use of this. This means that if Tor is running (and proper authentication has been configured), -Bitcoin Core automatically creates a hidden service to listen on. This will positively +Bitcoin Core automatically creates a hidden service to listen on. This will positively affect the number of available .onion nodes. This new feature is enabled by default if Bitcoin Core is listening (`-listen`), and @@ -102,8 +102,9 @@ and, if not disabled, configured using the `-torcontrol` and `-torpassword` sett To show verbose debugging information, pass `-debug=tor`. Connecting to Tor's control socket API requires one of two authentication methods to be -configured. For cookie authentication the user running bitcoind must have write access -to the `CookieAuthFile` specified in Tor configuration. In some cases, this is +configured. It also requires the control socket to be enabled, e.g. put `ControlPort 9051` +in `torrc` config file. For cookie authentication the user running bitcoind must have read +access to the `CookieAuthFile` specified in Tor configuration. In some cases this is preconfigured and the creation of a hidden service is automatic. If permission problems are seen with `-debug=tor` they can be resolved by adding both the user running Tor and the user running bitcoind to the same group and setting permissions appropriately. On diff --git a/src/Makefile.test.include b/src/Makefile.test.include index abfd66efad..c4ee5f5fcc 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -8,6 +8,7 @@ TEST_SRCDIR = test TEST_BINARY=test/test_bitcoin$(EXEEXT) JSON_TEST_FILES = \ + test/data/script_tests.json \ test/data/base58_encode_decode.json \ test/data/blockfilters.json \ test/data/key_io_valid.json \ @@ -83,6 +84,7 @@ BITCOIN_TESTS =\ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/streams_tests.cpp \ + test/sync_tests.cpp \ test/timedata_tests.cpp \ test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ @@ -94,6 +96,15 @@ BITCOIN_TESTS =\ test/validation_block_tests.cpp \ test/versionbits_tests.cpp +if ENABLE_PROPERTY_TESTS +BITCOIN_TESTS += \ + test/key_properties.cpp + +BITCOIN_TEST_SUITE += \ + test/gen/crypto_gen.cpp \ + test/gen/crypto_gen.h +endif + if ENABLE_WALLET BITCOIN_TESTS += \ wallet/test/psbt_wallet_tests.cpp \ @@ -117,7 +128,7 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_SERVER) $(LIBBITCOIN_CLI) $(LIBBITCOIN_C $(LIBLEVELDB) $(LIBLEVELDB_SSE42) $(LIBMEMENV) $(BOOST_LIBS) $(BOOST_UNIT_TEST_FRAMEWORK_LIB) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS) test_test_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -test_test_bitcoin_LDADD += $(LIBBITCOIN_CONSENSUS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) +test_test_bitcoin_LDADD += $(LIBBITCOIN_CONSENSUS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(RAPIDCHECK_LIBS) test_test_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) -static if ENABLE_ZMQ diff --git a/src/bech32.cpp b/src/bech32.cpp index c55f22b9b7..d6b29391a9 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -62,7 +62,7 @@ uint32_t PolyMod(const data& v) // v, it corresponds to x^2 + v0*x + v1 mod g(x). As 1 mod g(x) = 1, that is the starting value // for `c`. uint32_t c = 1; - for (auto v_i : v) { + for (const auto v_i : v) { // We want to update `c` to correspond to a polynomial with one extra term. If the initial // value of `c` consists of the coefficients of c(x) = f(x) mod g(x), we modify it to // correspond to c'(x) = (f(x) * x + v_i) mod g(x), where v_i is the next input to @@ -149,7 +149,7 @@ std::string Encode(const std::string& hrp, const data& values) { data combined = Cat(values, checksum); std::string ret = hrp + '1'; ret.reserve(ret.size() + combined.size()); - for (auto c : combined) { + for (const auto c : combined) { ret += CHARSET[c]; } return ret; diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index c9414e67c7..89149de4bb 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -35,6 +35,7 @@ static void SetupCliArgs() { const auto defaultBaseParams = CreateBaseChainParams(CBaseChainParams::MAIN); const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET); + const auto regtestBaseParams = CreateBaseChainParams(CBaseChainParams::REGTEST); gArgs.AddArg("-?", "This help message", false, OptionsCategory::OPTIONS); gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS); @@ -47,7 +48,7 @@ static void SetupCliArgs() gArgs.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpccookiefile=<loc>", _("Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)"), false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", false, OptionsCategory::OPTIONS); - gArgs.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u or testnet: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort()), false, OptionsCategory::OPTIONS); + gArgs.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u, testnet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcwait", "Wait for RPC server to start", false, OptionsCategory::OPTIONS); gArgs.AddArg("-rpcwallet=<walletname>", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)", false, OptionsCategory::OPTIONS); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 20f0218ac1..9ee3e8eee7 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -385,7 +385,7 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s CScript scriptPubKey = GetScriptForMultisig(required, pubkeys); if (bSegWit) { - for (CPubKey& pubkey : pubkeys) { + for (const CPubKey& pubkey : pubkeys) { if (!pubkey.IsCompressed()) { throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs"); } diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 93bb3e7647..b80cc2c259 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -4,16 +4,15 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <chainparams.h> -#include <consensus/merkle.h> +#include <chainparamsseeds.h> +#include <consensus/merkle.h> #include <tinyformat.h> #include <util.h> #include <utilstrencodings.h> #include <assert.h> -#include <chainparamsseeds.h> - static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesisOutputScript, uint32_t nTime, uint32_t nNonce, uint32_t nBits, int32_t nVersion, const CAmount& genesisReward) { CMutableTransaction txNew; @@ -62,14 +61,6 @@ void CChainParams::UpdateVersionBitsParameters(Consensus::DeploymentPos d, int64 /** * Main network */ -/** - * What makes a good checkpoint block? - * + Is surrounded by blocks with reasonable timestamps - * (no blocks before with a timestamp after, none after with - * timestamp before) - * + Contains no strange transactions - */ - class CMainParams : public CChainParams { public: CMainParams() { diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 15f6873961..4d0b094fa2 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -397,7 +397,7 @@ public: std::array<uint32_t, 8> locs = compute_hashes(e); // Make sure we have not already inserted this element // If we have, make sure that it does not get deleted - for (uint32_t loc : locs) + for (const uint32_t loc : locs) if (table[loc] == e) { please_keep(loc); epoch_flags[loc] = last_epoch; @@ -405,7 +405,7 @@ public: } for (uint8_t depth = 0; depth < depth_limit; ++depth) { // First try to insert to an empty slot, if one exists - for (uint32_t loc : locs) { + for (const uint32_t loc : locs) { if (!collection_flags.bit_is_set(loc)) continue; table[loc] = std::move(e); @@ -467,7 +467,7 @@ public: inline bool contains(const Element& e, const bool erase) const { std::array<uint32_t, 8> locs = compute_hashes(e); - for (uint32_t loc : locs) + for (const uint32_t loc : locs) if (table[loc] == e) { if (erase) allow_erase(loc); diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index f5fb715800..58d8cc2c9d 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -78,7 +78,7 @@ static void SetMaxOpenFiles(leveldb::Options *options) { // do not interfere with select() loops. On 64-bit Unix hosts this value is // also OK, because up to that amount LevelDB will use an mmap // implementation that does not use extra file descriptors (the fds are - // closed after being mmaped). + // closed after being mmap'ed). // // Increasing the value beyond the default is dangerous because LevelDB will // fall back to a non-mmap implementation when the file count is too large. diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 0fdc3e5ff3..326f7f6b64 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -69,7 +69,7 @@ class WorkQueue { private: /** Mutex protects entire object */ - std::mutex cs; + Mutex cs; std::condition_variable cond; std::deque<std::unique_ptr<WorkItem>> queue; bool running; @@ -88,7 +88,7 @@ public: /** Enqueue a work item */ bool Enqueue(WorkItem* item) { - std::unique_lock<std::mutex> lock(cs); + LOCK(cs); if (queue.size() >= maxDepth) { return false; } @@ -102,7 +102,7 @@ public: while (true) { std::unique_ptr<WorkItem> i; { - std::unique_lock<std::mutex> lock(cs); + WAIT_LOCK(cs, lock); while (running && queue.empty()) cond.wait(lock); if (!running) @@ -116,7 +116,7 @@ public: /** Interrupt and exit loops */ void Interrupt() { - std::unique_lock<std::mutex> lock(cs); + LOCK(cs); running = false; cond.notify_all(); } diff --git a/src/init.cpp b/src/init.cpp index bd330459f6..a3b04c6c44 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -343,8 +343,10 @@ void SetupServerArgs() { const auto defaultBaseParams = CreateBaseChainParams(CBaseChainParams::MAIN); const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET); + const auto regtestBaseParams = CreateBaseChainParams(CBaseChainParams::REGTEST); const auto defaultChainParams = CreateChainParams(CBaseChainParams::MAIN); const auto testnetChainParams = CreateChainParams(CBaseChainParams::TESTNET); + const auto regtestChainParams = CreateChainParams(CBaseChainParams::REGTEST); // Hidden Options std::vector<std::string> hidden_args = {"-rpcssl", "-benchmark", "-h", "-help", "-socks", "-tor", "-debugnet", "-whitelistalwaysrelay", @@ -366,7 +368,7 @@ void SetupServerArgs() gArgs.AddArg("-datadir=<dir>", "Specify data directory", false, OptionsCategory::OPTIONS); gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), true, OptionsCategory::OPTIONS); gArgs.AddArg("-dbcache=<n>", strprintf("Set database cache size in megabytes (%d to %d, default: %d)", nMinDbCache, nMaxDbCache, nDefaultDbCache), false, OptionsCategory::OPTIONS); - gArgs.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (0 to disable; default: %s)", DEFAULT_DEBUGLOGFILE), false, OptionsCategory::OPTIONS); + gArgs.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (-nodebuglogfile to disable; default: %s)", DEFAULT_DEBUGLOGFILE), false, OptionsCategory::OPTIONS); gArgs.AddArg("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER), true, OptionsCategory::OPTIONS); gArgs.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", false, OptionsCategory::OPTIONS); gArgs.AddArg("-loadblock=<file>", "Imports blocks from external blk000??.dat file on startup", false, OptionsCategory::OPTIONS); @@ -386,7 +388,7 @@ void SetupServerArgs() "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), false, OptionsCategory::OPTIONS); gArgs.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk", false, OptionsCategory::OPTIONS); - gArgs.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks", false, OptionsCategory::OPTIONS); + gArgs.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.", false, OptionsCategory::OPTIONS); #ifndef WIN32 gArgs.AddArg("-sysperms", "Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)", false, OptionsCategory::OPTIONS); #else @@ -398,7 +400,7 @@ void SetupServerArgs() gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), false, OptionsCategory::CONNECTION); gArgs.AddArg("-bantime=<n>", strprintf("Number of seconds to keep misbehaving peers from reconnecting (default: %u)", DEFAULT_MISBEHAVING_BANTIME), false, OptionsCategory::CONNECTION); gArgs.AddArg("-bind=<addr>", "Bind to given address and always listen on it. Use [host]:port notation for IPv6", false, OptionsCategory::CONNECTION); - gArgs.AddArg("-connect=<ip>", "Connect only to the specified node; -connect=0 disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", false, OptionsCategory::CONNECTION); + gArgs.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", false, OptionsCategory::CONNECTION); gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", false, OptionsCategory::CONNECTION); gArgs.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), false, OptionsCategory::CONNECTION); gArgs.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", false, OptionsCategory::CONNECTION); @@ -412,12 +414,12 @@ void SetupServerArgs() gArgs.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), false, OptionsCategory::CONNECTION); gArgs.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), false, OptionsCategory::CONNECTION); gArgs.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h), 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), false, OptionsCategory::CONNECTION); - gArgs.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor hidden services (default: -proxy)", false, OptionsCategory::CONNECTION); + gArgs.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", false, OptionsCategory::CONNECTION); gArgs.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", false, OptionsCategory::CONNECTION); gArgs.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), false, OptionsCategory::CONNECTION); gArgs.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), false, OptionsCategory::CONNECTION); - gArgs.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u or testnet: %u)", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort()), false, OptionsCategory::CONNECTION); - gArgs.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy", false, OptionsCategory::CONNECTION); + gArgs.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet: %u, regtest: %u)", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), false, OptionsCategory::CONNECTION); + gArgs.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", false, OptionsCategory::CONNECTION); gArgs.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), false, OptionsCategory::CONNECTION); gArgs.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", false, OptionsCategory::CONNECTION); gArgs.AddArg("-timeout=<n>", strprintf("Specify connection timeout in milliseconds (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), false, OptionsCategory::CONNECTION); @@ -452,8 +454,8 @@ void SetupServerArgs() gArgs.AddArg("-checkblocks=<n>", strprintf("How many blocks to check at startup (default: %u, 0 = all)", DEFAULT_CHECKBLOCKS), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-checklevel=<n>", strprintf("How thorough the block verification of -checkblocks is (0-4, default: %u)", DEFAULT_CHECKLEVEL), true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-checkblockindex", strprintf("Do a full consistency check for mapBlockIndex, setBlockIndexCandidates, chainActive and mapBlocksUnlinked occasionally. (default: %u)", defaultChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-checkmempool=<n>", strprintf("Run checks every <n> transactions (default: %u)", defaultChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-checkblockindex", strprintf("Do a full consistency check for mapBlockIndex, setBlockIndexCandidates, chainActive and mapBlocksUnlinked occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-checkmempool=<n>", strprintf("Run checks every <n> transactions (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-checkpoints", strprintf("Disable expensive verification for known chain history (default: %u)", DEFAULT_CHECKPOINTS_ENABLED), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-deprecatedrpc=<method>", "Allows deprecated RPC method(s) to be used", true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-dropmessagestest=<n>", "Randomly drop 1 of every <n> network messages", true, OptionsCategory::DEBUG_TEST); @@ -465,7 +467,7 @@ void SetupServerArgs() gArgs.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-vbparams=deployment:start:end", "Use given start/end times for specified version bits deployment (regtest-only)", true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-addrmantest", "Allows to test address relay on localhost", true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-debug=<category>", strprintf("Output debugging information (default: %u, supplying <category> is optional)", 0) + ". " + + gArgs.AddArg("-debug=<category>", "Output debugging information (default: -nodebug, supplying <category> is optional). " "If <category> is not supplied or if <category> = 1, output all debugging information. <category> can be: " + ListLogCategories() + ".", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-debugexclude=<category>", strprintf("Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except one or more specified categories."), false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-help-debug", "Show all debugging options (usage: --help -help-debug)", false, OptionsCategory::DEBUG_TEST); @@ -478,7 +480,7 @@ void SetupServerArgs() gArgs.AddArg("-maxtxfee=<amt>", strprintf("Maximum total fees (in %s) to use in a single wallet transaction or raw transaction; setting this too low may abort large transactions (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set debuglogfile=0)", false, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-uacomment=<cmt>", "Append comment to the user agent string", false, OptionsCategory::DEBUG_TEST); @@ -507,7 +509,7 @@ void SetupServerArgs() gArgs.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost, or if -rpcallowip has been specified, 0.0.0.0 and :: i.e., all addresses)", false, OptionsCategory::RPC); gArgs.AddArg("-rpccookiefile=<loc>", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", false, OptionsCategory::RPC); gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", false, OptionsCategory::RPC); - gArgs.AddArg("-rpcport=<port>", strprintf("Listen for JSON-RPC connections on <port> (default: %u or testnet: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort()), false, OptionsCategory::RPC); + gArgs.AddArg("-rpcport=<port>", strprintf("Listen for JSON-RPC connections on <port> (default: %u, testnet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), false, OptionsCategory::RPC); gArgs.AddArg("-rpcserialversion", strprintf("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) or segwit(1) (default: %d)", DEFAULT_RPC_SERIALIZE_VERSION), false, OptionsCategory::RPC); gArgs.AddArg("-rpcservertimeout=<n>", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), true, OptionsCategory::RPC); gArgs.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), false, OptionsCategory::RPC); @@ -561,17 +563,17 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex } static bool fHaveGenesis = false; -static CWaitableCriticalSection cs_GenesisWait; -static CConditionVariable condvar_GenesisWait; +static Mutex g_genesis_wait_mutex; +static std::condition_variable g_genesis_wait_cv; static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex) { if (pBlockIndex != nullptr) { { - WaitableLock lock_GenesisWait(cs_GenesisWait); + LOCK(g_genesis_wait_mutex); fHaveGenesis = true; } - condvar_GenesisWait.notify_all(); + g_genesis_wait_cv.notify_all(); } } @@ -1661,12 +1663,12 @@ bool AppInitMain() // Wait for genesis block to be processed { - WaitableLock lock(cs_GenesisWait); + WAIT_LOCK(g_genesis_wait_mutex, lock); // We previously could hang here if StartShutdown() is called prior to // ThreadImport getting started, so instead we just wait on a timer to // check ShutdownRequested() regularly. while (!fHaveGenesis && !ShutdownRequested()) { - condvar_GenesisWait.wait_for(lock, std::chrono::milliseconds(500)); + g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); } uiInterface.NotifyBlockTip_disconnect(BlockNotifyGenesisWait); } diff --git a/src/net.cpp b/src/net.cpp index df2cb54b7a..c51a1b4a74 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -344,7 +344,7 @@ CNode* CConnman::FindNode(const CService& addr) bool CConnman::CheckIncomingNonce(uint64_t nonce) { LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + for (const CNode* pnode : vNodes) { if (!pnode->fSuccessfullyConnected && !pnode->fInbound && pnode->GetLocalNonce() == nonce) return false; } @@ -1227,7 +1227,7 @@ void CConnman::ThreadSocketHandler() if(vNodesSize != nPrevNodeCount) { nPrevNodeCount = vNodesSize; if(clientInterface) - clientInterface->NotifyNumConnectionsChanged(nPrevNodeCount); + clientInterface->NotifyNumConnectionsChanged(vNodesSize); } // @@ -1614,7 +1614,7 @@ void CConnman::ThreadDNSAddressSeed() LOCK(cs_vNodes); int nRelevant = 0; - for (auto pnode : vNodes) { + for (const CNode* pnode : vNodes) { nRelevant += pnode->fSuccessfullyConnected && !pnode->fFeeler && !pnode->fOneShot && !pnode->m_manual_connection && !pnode->fInbound; } if (nRelevant >= 2) { @@ -1733,7 +1733,7 @@ int CConnman::GetExtraOutboundCount() int nOutbound = 0; { LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + for (const CNode* pnode : vNodes) { if (!pnode->fInbound && !pnode->m_manual_connection && !pnode->fFeeler && !pnode->fDisconnect && !pnode->fOneShot && pnode->fSuccessfullyConnected) { ++nOutbound; } @@ -1803,7 +1803,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) std::set<std::vector<unsigned char> > setConnected; { LOCK(cs_vNodes); - for (CNode* pnode : vNodes) { + for (const CNode* pnode : vNodes) { if (!pnode->fInbound && !pnode->m_manual_connection) { // Netgroups for inbound and addnode peers are not excluded because our goal here // is to not use multiple of our limited outbound slots on a single netgroup @@ -2065,7 +2065,7 @@ void CConnman::ThreadMessageHandler() pnode->Release(); } - std::unique_lock<std::mutex> lock(mutexMsgProc); + WAIT_LOCK(mutexMsgProc, lock); if (!fMoreWork) { condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), [this] { return fMsgProcWake; }); } @@ -2347,7 +2347,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) flagInterruptMsgProc = false; { - std::unique_lock<std::mutex> lock(mutexMsgProc); + LOCK(mutexMsgProc); fMsgProcWake = false; } @@ -427,7 +427,7 @@ private: bool fMsgProcWake; std::condition_variable condMsgProc; - std::mutex mutexMsgProc; + Mutex mutexMsgProc; std::atomic<bool> flagInterruptMsgProc; CThreadInterrupt interruptNet; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9f950831c4..b48a3bd221 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -882,7 +882,7 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb // Erase orphan transactions included or precluded by this block if (vOrphanErase.size()) { int nErased = 0; - for (uint256 &orphanHash : vOrphanErase) { + for (const uint256& orphanHash : vOrphanErase) { nErased += EraseOrphanTx(orphanHash); } LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); @@ -2288,7 +2288,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } } - for (uint256 hash : vEraseQueue) + for (const uint256& hash : vEraseQueue) EraseOrphanTx(hash); } else if (fMissingInputs) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 712d20b71e..f1d3074c2f 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -416,7 +416,7 @@ void BitcoinApplication::requestShutdown() #ifdef ENABLE_WALLET window->removeAllWallets(); - for (WalletModel *walletModel : m_wallet_models) { + for (const WalletModel* walletModel : m_wallet_models) { delete walletModel; } m_wallet_models.clear(); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 5321cd94fd..632ce0750a 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -614,8 +614,11 @@ void BitcoinGUI::createTrayIconMenu() #endif // Configuration of the tray icon (or dock icon) icon menu +#ifndef Q_OS_MAC + // Note: On Mac, the dock icon's menu already has show / hide action. trayIconMenu->addAction(toggleHideAction); trayIconMenu->addSeparator(); +#endif trayIconMenu->addAction(sendCoinsMenuAction); trayIconMenu->addAction(receiveCoinsMenuAction); trayIconMenu->addSeparator(); @@ -1273,7 +1276,7 @@ void UnitDisplayStatusBarControl::mousePressEvent(QMouseEvent *event) void UnitDisplayStatusBarControl::createContextMenu() { menu = new QMenu(this); - for (BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) + for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) { QAction *menuAction = new QAction(QString(BitcoinUnits::longName(u)), this); menuAction->setData(QVariant(u)); diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 59a751fb12..8cfedca57f 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -65,7 +65,7 @@ public: interfaces::Node::NodesStats nodes_stats; node.getNodesStats(nodes_stats); cachedNodeStats.reserve(nodes_stats.size()); - for (auto& node_stats : nodes_stats) + for (const auto& node_stats : nodes_stats) { CNodeCombinedStats stats; stats.nodeStats = std::get<0>(node_stats); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index eed71397ab..6f66bc19e1 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -348,7 +348,7 @@ void SendCoinsDialog::on_sendButton_clicked() questionString.append("<hr />"); CAmount totalAmount = currentTransaction.getTotalTransactionAmount() + txFee; QStringList alternativeUnits; - for (BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) + for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) { if(u != model->getOptionsModel()->getDisplayUnit()) alternativeUnits.append(BitcoinUnits::formatHtmlWithUnit(u, totalAmount)); diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 0b111cc6d7..1eff4f6b65 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -200,7 +200,7 @@ void SplashScreen::unsubscribeFromCoreSignals() // Disconnect signals from client m_handler_init_message->disconnect(); m_handler_show_progress->disconnect(); - for (auto& handler : m_connected_wallet_handlers) { + for (const auto& handler : m_connected_wallet_handlers) { handler->disconnect(); } m_connected_wallet_handlers.clear(); diff --git a/src/qt/test/util.h b/src/qt/test/util.h index 324386c139..5363c94547 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -5,7 +5,7 @@ * Press "Ok" button in message box dialog. * * @param text - Optionally store dialog text. - * @param msec - Number of miliseconds to pause before triggering the callback. + * @param msec - Number of milliseconds to pause before triggering the callback. */ void ConfirmMessage(QString* text = nullptr, int msec = 0); diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index f087e7569c..3aed3f2b97 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -141,10 +141,10 @@ void TrafficGraphWidget::updateRates() } float tmax = 0.0f; - for (float f : vSamplesIn) { + for (const float f : vSamplesIn) { if(f > tmax) tmax = f; } - for (float f : vSamplesOut) { + for (const float f : vSamplesOut) { if(f > tmax) tmax = f; } fMax = tmax; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 6286ddc0e0..59332be754 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -152,13 +152,13 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall else { isminetype fAllFromMe = ISMINE_SPENDABLE; - for (isminetype mine : wtx.txin_is_mine) + for (const isminetype mine : wtx.txin_is_mine) { if(fAllFromMe > mine) fAllFromMe = mine; } isminetype fAllToMe = ISMINE_SPENDABLE; - for (isminetype mine : wtx.txout_is_mine) + for (const isminetype mine : wtx.txout_is_mine) { if(fAllToMe > mine) fAllToMe = mine; } diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index c767de5eda..d1a7527ac7 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -77,14 +77,14 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const interface { bool involvesWatchAddress = false; isminetype fAllFromMe = ISMINE_SPENDABLE; - for (isminetype mine : wtx.txin_is_mine) + for (const isminetype mine : wtx.txin_is_mine) { if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; if(fAllFromMe > mine) fAllFromMe = mine; } isminetype fAllToMe = ISMINE_SPENDABLE; - for (isminetype mine : wtx.txout_is_mine) + for (const isminetype mine : wtx.txout_is_mine) { if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; if(fAllToMe > mine) fAllToMe = mine; diff --git a/src/random.cpp b/src/random.cpp index 6c5ad5ac96..503d5b3636 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -12,6 +12,7 @@ #include <wincrypt.h> #endif #include <logging.h> // for LogPrint() +#include <sync.h> // for WAIT_LOCK #include <utiltime.h> // for GetTime() #include <stdlib.h> @@ -295,7 +296,7 @@ void RandAddSeedSleep() } -static std::mutex cs_rng_state; +static Mutex cs_rng_state; static unsigned char rng_state[32] = {0}; static uint64_t rng_counter = 0; @@ -305,7 +306,7 @@ static void AddDataToRng(void* data, size_t len) { hasher.Write((const unsigned char*)data, len); unsigned char buf[64]; { - std::unique_lock<std::mutex> lock(cs_rng_state); + WAIT_LOCK(cs_rng_state, lock); hasher.Write(rng_state, sizeof(rng_state)); hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter)); ++rng_counter; @@ -337,7 +338,7 @@ void GetStrongRandBytes(unsigned char* out, int num) // Combine with and update state { - std::unique_lock<std::mutex> lock(cs_rng_state); + WAIT_LOCK(cs_rng_state, lock); hasher.Write(rng_state, sizeof(rng_state)); hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter)); ++rng_counter; diff --git a/src/rest.cpp b/src/rest.cpp index 12358cf50d..6ba15172fa 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -464,7 +464,7 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart) oss >> fCheckMemPool; oss >> vOutPoints; } - } catch (const std::ios_base::failure& e) { + } catch (const std::ios_base::failure&) { // abort in case of unreadable binary data return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a77fea8ea8..6fb04ab005 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -50,7 +50,7 @@ struct CUpdatedBlock int height; }; -static std::mutex cs_blockchange; +static Mutex cs_blockchange; static std::condition_variable cond_blockchange; static CUpdatedBlock latestblock; @@ -225,7 +225,7 @@ static UniValue waitfornewblock(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); block = latestblock; if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&block]{return latestblock.height != block.height || latestblock.hash != block.hash || !IsRPCRunning(); }); @@ -267,7 +267,7 @@ static UniValue waitforblock(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&hash]{return latestblock.hash == hash || !IsRPCRunning();}); else @@ -310,7 +310,7 @@ static UniValue waitforblockheight(const JSONRPCRequest& request) CUpdatedBlock block; { - std::unique_lock<std::mutex> lock(cs_blockchange); + WAIT_LOCK(cs_blockchange, lock); if(timeout) cond_blockchange.wait_for(lock, std::chrono::milliseconds(timeout), [&height]{return latestblock.height >= height || !IsRPCRunning();}); else @@ -426,7 +426,7 @@ static void entryToJSON(UniValue &info, const CTxMemPoolEntry &e) EXCLUSIVE_LOCK UniValue spent(UniValue::VARR); const CTxMemPool::txiter &it = mempool.mapTx.find(tx.GetHash()); const CTxMemPool::setEntries &setChildren = mempool.GetMemPoolChildren(it); - for (const CTxMemPool::txiter &childiter : setChildren) { + for (CTxMemPool::txiter childiter : setChildren) { spent.push_back(childiter->GetTx().GetHash().ToString()); } @@ -2058,7 +2058,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) "or more path elements separated by \"/\", and optionally ending in \"/*\" (unhardened), or \"/*'\" or \"/*h\" (hardened) to specify all\n" "unhardened or hardened child keys.\n" "In the latter case, a range needs to be specified by below if different from 1000.\n" - "For more information on output descriptors, see the documentation at TODO\n" + "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n" "\nArguments:\n" "1. \"action\" (string, required) The action to execute\n" " \"start\" for starting a scan\n" diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 544bc62c36..add335eb8a 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -16,8 +16,7 @@ class UniValue; static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5; /** - * Get the difficulty of the net wrt to the given block index, or the chain tip if - * not provided. + * Get the difficulty of the net wrt to the given block index. * * @return A floating point number that is a multiple of the main net minimum * difficulty (4295032833 hashes). diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 623b0bd86a..621b86eec8 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -470,7 +470,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); - WaitableLock lock(g_best_block_mutex); + WAIT_LOCK(g_best_block_mutex, lock); while (g_best_block == hashWatchedChain && IsRPCRunning()) { if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) @@ -922,7 +922,7 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) UniValue result(UniValue::VOBJ); - for (FeeEstimateHorizon horizon : {FeeEstimateHorizon::SHORT_HALFLIFE, FeeEstimateHorizon::MED_HALFLIFE, FeeEstimateHorizon::LONG_HALFLIFE}) { + for (const FeeEstimateHorizon horizon : {FeeEstimateHorizon::SHORT_HALFLIFE, FeeEstimateHorizon::MED_HALFLIFE, FeeEstimateHorizon::LONG_HALFLIFE}) { CFeeRate feeRate; EstimationResult buckets; diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 40418545c1..b7d05cef11 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -19,11 +19,6 @@ #include <timedata.h> #include <util.h> #include <utilstrencodings.h> -#ifdef ENABLE_WALLET -#include <wallet/rpcwallet.h> -#include <wallet/wallet.h> -#include <wallet/walletdb.h> -#endif #include <warnings.h> #include <stdint.h> @@ -67,29 +62,18 @@ static UniValue validateaddress(const JSONRPCRequest& request) ret.pushKV("isvalid", isValid); if (isValid) { + std::string currentAddress = EncodeDestination(dest); + ret.pushKV("address", currentAddress); -#ifdef ENABLE_WALLET - if (HasWallets() && IsDeprecatedRPCEnabled("validateaddress")) { - ret.pushKVs(getaddressinfo(request)); - } -#endif - if (ret["address"].isNull()) { - std::string currentAddress = EncodeDestination(dest); - ret.pushKV("address", currentAddress); - - CScript scriptPubKey = GetScriptForDestination(dest); - ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end())); + CScript scriptPubKey = GetScriptForDestination(dest); + ret.pushKV("scriptPubKey", HexStr(scriptPubKey.begin(), scriptPubKey.end())); - UniValue detail = DescribeAddress(dest); - ret.pushKVs(detail); - } + UniValue detail = DescribeAddress(dest); + ret.pushKVs(detail); } return ret; } -// Needed even with !ENABLE_WALLET, to pass (ignored) pointers around -class CWallet; - static UniValue createmultisig(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) @@ -130,8 +114,7 @@ static UniValue createmultisig(const JSONRPCRequest& request) if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) { pubkeys.push_back(HexToPubKey(keys[i].get_str())); } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\nNote that from v0.16, createmultisig no longer accepts addresses." - " Users must use addmultisigaddress to create multisig addresses with addresses known to the wallet.", keys[i].get_str())); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str())); } } @@ -461,7 +444,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "control", "getmemoryinfo", &getmemoryinfo, {"mode"} }, { "control", "logging", &logging, {"include", "exclude"}}, - { "util", "validateaddress", &validateaddress, {"address"} }, /* uses wallet if enabled */ + { "util", "validateaddress", &validateaddress, {"address"} }, { "util", "createmultisig", &createmultisig, {"nrequired","keys"} }, { "util", "verifymessage", &verifymessage, {"address","signature","message"} }, { "util", "signmessagewithprivkey", &signmessagewithprivkey, {"privkey","message"} }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 343b7d3b05..169a452659 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -164,7 +164,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) obj.pushKV("synced_headers", statestats.nSyncHeight); obj.pushKV("synced_blocks", statestats.nCommonHeight); UniValue heights(UniValue::VARR); - for (int height : statestats.vHeightInFlight) { + for (const int height : statestats.vHeightInFlight) { heights.push_back(height); } obj.pushKV("inflight", heights); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 248b963c0b..6acf383646 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -27,9 +27,6 @@ #include <txmempool.h> #include <uint256.h> #include <utilstrencodings.h> -#ifdef ENABLE_WALLET -#include <wallet/rpcwallet.h> -#endif #include <future> #include <stdint.h> @@ -824,8 +821,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival view.AddCoin(out, std::move(newcoin), true); } - // if redeemScript given and not using the local wallet (private keys - // given), add redeemScript to the keystore so it can be signed: + // if redeemScript and private keys were given, add redeemScript to the keystore so it can be signed if (is_temp_keystore && (scriptPubKey.IsPayToScriptHash() || scriptPubKey.IsPayToWitnessScriptHash())) { RPCTypeCheckObj(prevOut, { @@ -980,102 +976,10 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) UniValue signrawtransaction(const JSONRPCRequest& request) { -#ifdef ENABLE_WALLET - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); -#endif - - if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) - throw std::runtime_error( - "signrawtransaction \"hexstring\" ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] [\"privatekey1\",...] sighashtype )\n" - "\nDEPRECATED. Sign inputs for raw transaction (serialized, hex-encoded).\n" - "The second optional argument (may be null) is an array of previous transaction outputs that\n" - "this transaction depends on but may not yet be in the block chain.\n" - "The third optional argument (may be null) is an array of base58-encoded private\n" - "keys that, if given, will be the only keys used to sign the transaction.\n" -#ifdef ENABLE_WALLET - + HelpRequiringPassphrase(pwallet) + "\n" -#endif - "\nArguments:\n" - "1. \"hexstring\" (string, required) The transaction hex string\n" - "2. \"prevtxs\" (string, optional) An json array of previous dependent transaction outputs\n" - " [ (json array of json objects, or 'null' if none provided)\n" - " {\n" - " \"txid\":\"id\", (string, required) The transaction id\n" - " \"vout\":n, (numeric, required) The output number\n" - " \"scriptPubKey\": \"hex\", (string, required) script key\n" - " \"redeemScript\": \"hex\", (string, required for P2SH or P2WSH) redeem script\n" - " \"amount\": value (numeric, required) The amount spent\n" - " }\n" - " ,...\n" - " ]\n" - "3. \"privkeys\" (string, optional) A json array of base58-encoded private keys for signing\n" - " [ (json array of strings, or 'null' if none provided)\n" - " \"privatekey\" (string) private key in base58-encoding\n" - " ,...\n" - " ]\n" - "4. \"sighashtype\" (string, optional, default=ALL) The signature hash type. Must be one of\n" - " \"ALL\"\n" - " \"NONE\"\n" - " \"SINGLE\"\n" - " \"ALL|ANYONECANPAY\"\n" - " \"NONE|ANYONECANPAY\"\n" - " \"SINGLE|ANYONECANPAY\"\n" - - "\nResult:\n" - "{\n" - " \"hex\" : \"value\", (string) The hex-encoded raw transaction with signature(s)\n" - " \"complete\" : true|false, (boolean) If the transaction has a complete set of signatures\n" - " \"errors\" : [ (json array of objects) Script verification errors (if there are any)\n" - " {\n" - " \"txid\" : \"hash\", (string) The hash of the referenced, previous transaction\n" - " \"vout\" : n, (numeric) The index of the output to spent and used as input\n" - " \"scriptSig\" : \"hex\", (string) The hex-encoded signature script\n" - " \"sequence\" : n, (numeric) Script sequence number\n" - " \"error\" : \"text\" (string) Verification or signing error related to the input\n" - " }\n" - " ,...\n" - " ]\n" - "}\n" - - "\nExamples:\n" - + HelpExampleCli("signrawtransaction", "\"myhex\"") - + HelpExampleRpc("signrawtransaction", "\"myhex\"") - ); - - if (!IsDeprecatedRPCEnabled("signrawtransaction")) { - throw JSONRPCError(RPC_METHOD_DEPRECATED, "signrawtransaction is deprecated and will be fully removed in v0.18. " - "To use signrawtransaction in v0.17, restart bitcoind with -deprecatedrpc=signrawtransaction.\n" - "Projects should transition to using signrawtransactionwithkey and signrawtransactionwithwallet before upgrading to v0.18"); - } - - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VARR, UniValue::VSTR}, true); - - // Make a JSONRPCRequest to pass on to the right signrawtransaction* command - JSONRPCRequest new_request; - new_request.id = request.id; - new_request.params.setArray(); - - // For signing with private keys - if (!request.params[2].isNull()) { - new_request.params.push_back(request.params[0]); - // Note: the prevtxs and privkeys are reversed for signrawtransactionwithkey - new_request.params.push_back(request.params[2]); - new_request.params.push_back(request.params[1]); - new_request.params.push_back(request.params[3]); - return signrawtransactionwithkey(new_request); - } else { -#ifdef ENABLE_WALLET - // Otherwise sign with the wallet which does not take a privkeys parameter - new_request.params.push_back(request.params[0]); - new_request.params.push_back(request.params[1]); - new_request.params.push_back(request.params[3]); - return signrawtransactionwithwallet(new_request); -#else - // If we have made it this far, then wallet is disabled and no private keys were given, so fail here. - throw JSONRPCError(RPC_INVALID_PARAMETER, "No private keys available."); -#endif - } + // This method should be removed entirely in V0.19, along with the entries in the + // CRPCCommand table and rpc/client.cpp. + throw JSONRPCError(RPC_METHOD_DEPRECATED, "signrawtransaction was removed in v0.18.\n" + "Clients should transition to using signrawtransactionwithkey and signrawtransactionwithwallet"); } static UniValue sendrawtransaction(const JSONRPCRequest& request) @@ -1084,7 +988,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) throw std::runtime_error( "sendrawtransaction \"hexstring\" ( allowhighfees )\n" "\nSubmits raw transaction (serialized, hex-encoded) to local node and network.\n" - "\nAlso see createrawtransaction and signrawtransaction calls.\n" + "\nAlso see createrawtransaction and signrawtransactionwithkey calls.\n" "\nArguments:\n" "1. \"hexstring\" (string, required) The hex string of the raw transaction)\n" "2. allowhighfees (boolean, optional, default=false) Allow high fees\n" @@ -1094,7 +998,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) "\nCreate a transaction\n" + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + "Sign the transaction, and get back the hex\n" - + HelpExampleCli("signrawtransaction", "\"myhex\"") + + + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + "\nSend the transaction (signed hex)\n" + HelpExampleCli("sendrawtransaction", "\"signedhex\"") + "\nAs a json rpc call\n" @@ -1199,7 +1103,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) "\nCreate a transaction\n" + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\" : \\\"mytxid\\\",\\\"vout\\\":0}]\" \"{\\\"myaddress\\\":0.01}\"") + "Sign the transaction, and get back the hex\n" - + HelpExampleCli("signrawtransaction", "\"myhex\"") + + + HelpExampleCli("signrawtransactionwithwallet", "\"myhex\"") + "\nTest acceptance of the transaction (signed hex)\n" + HelpExampleCli("testmempoolaccept", "\"signedhex\"") + "\nAs a json rpc call\n" @@ -1740,7 +1644,7 @@ UniValue converttopsbt(const JSONRPCRequest& request) " will continue. If false, RPC will fail if any signatures are present.\n" "3. iswitness (boolean, optional) Whether the transaction hex is a serialized witness transaction.\n" " If iswitness is not present, heuristic tests will be used in decoding. If true, only witness deserializaion\n" - " will be tried. If false, only non-witness deserialization wil be tried. Only has an effect if\n" + " will be tried. If false, only non-witness deserialization will be tried. Only has an effect if\n" " permitsigdata is true.\n" "\nResult:\n" " \"psbt\" (string) The resulting raw transaction (base64-encoded string)\n" @@ -1800,7 +1704,7 @@ static const CRPCCommand commands[] = { "rawtransactions", "decodescript", &decodescript, {"hexstring"} }, { "rawtransactions", "sendrawtransaction", &sendrawtransaction, {"hexstring","allowhighfees"} }, { "rawtransactions", "combinerawtransaction", &combinerawtransaction, {"txs"} }, - { "rawtransactions", "signrawtransaction", &signrawtransaction, {"hexstring","prevtxs","privkeys","sighashtype"} }, /* uses wallet if enabled */ + { "hidden", "signrawtransaction", &signrawtransaction, {"hexstring","prevtxs","privkeys","sighashtype"} }, { "rawtransactions", "signrawtransactionwithkey", &signrawtransactionwithkey, {"hexstring","privkeys","prevtxs","sighashtype"} }, { "rawtransactions", "testmempoolaccept", &testmempoolaccept, {"rawtxs","allowhighfees"} }, { "rawtransactions", "decodepsbt", &decodepsbt, {"psbt"} }, diff --git a/src/scheduler.h b/src/scheduler.h index 0c2551cf4f..953d6c37de 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -111,7 +111,7 @@ public: /** * Add a callback to be executed. Callbacks are executed serially * and memory is release-acquire consistent between callback executions. - * Practially, this means that callbacks can behave as if they are executed + * Practically, this means that callbacks can behave as if they are executed * in order by a single thread. */ void AddToProcessQueue(std::function<void (void)> func); diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index f366b99ec3..45b097dde6 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -373,7 +373,7 @@ enum class ParseScriptContext { P2WSH, }; -/** Parse a constant. If succesful, sp is updated to skip the constant and return true. */ +/** Parse a constant. If successful, sp is updated to skip the constant and return true. */ bool Const(const std::string& str, Span<const char>& sp) { if ((size_t)sp.size() >= str.size() && std::equal(str.begin(), str.end(), sp.begin())) { @@ -383,7 +383,7 @@ bool Const(const std::string& str, Span<const char>& sp) return false; } -/** Parse a function call. If succesful, sp is updated to be the function's argument(s). */ +/** Parse a function call. If successful, sp is updated to be the function's argument(s). */ bool Func(const std::string& str, Span<const char>& sp) { if ((size_t)sp.size() >= str.size() + 2 && sp[str.size()] == '(' && sp[sp.size() - 1] == ')' && std::equal(str.begin(), str.end(), sp.begin())) { diff --git a/src/script/descriptor.h b/src/script/descriptor.h index e079c72e92..87e07369c7 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -22,55 +22,8 @@ // they can be included inside by changing public keys to private keys (WIF // format), and changing xpubs by xprvs. // -// 1. Examples -// -// A P2PK descriptor with a fixed public key: -// - pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798) -// -// A P2SH-P2WSH-P2PKH descriptor with a fixed public key: -// - sh(wsh(pkh(02e493dbf1c10d80f3581e4904930b1404cc6c13900ee0758474fa94abe8c4cd13))) -// -// A bare 1-of-2 multisig descriptor: -// - multi(1,022f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe4,025cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc) -// -// A chain of P2PKH outputs (this needs the corresponding private key to derive): -// - pkh(xpub68Gmy5EdvgibQVfPdqkBBCHxA5htiqg55crXYuXoQRKfDBFA1WEjWgP6LHhwBZeNK1VTsfTFUHCdrfp1bgwQ9xv5ski8PX9rL2dZXvgGDnw/1'/2/*) -// -// 2. Grammar description: -// -// X: xpub or xprv encoded extended key -// I: decimal encoded integer -// H: Hex encoded byte array -// A: Address in P2PKH, P2SH, or Bech32 encoding -// -// S (Scripts): -// * pk(P): Pay-to-pubkey (P2PK) output for public key P. -// * pkh(P): Pay-to-pubkey-hash (P2PKH) output for public key P. -// * wpkh(P): Pay-to-witness-pubkey-hash (P2WPKH) output for public key P. -// * sh(S): Pay-to-script-hash (P2SH) output for script S -// * wsh(S): Pay-to-witness-script-hash (P2WSH) output for script S -// * combo(P): combination of P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH for public key P. -// * multi(I,L): k-of-n multisig for given public keys -// * addr(A): Output to address -// * raw(H): scriptPubKey with raw bytes -// -// P (Public keys): -// * H: fixed public key (or WIF-encoded private key) -// * E: extended public key -// * E/*: (ranged) all unhardened direct children of an extended public key -// * E/*': (ranged) all hardened direct children of an extended public key -// -// L (Comma-separated lists of public keys): -// * P -// * L,P -// -// E (Extended public keys): -// * X -// * E/I: unhardened child -// * E/I': hardened child -// * E/Ih: hardened child (alternative notation) -// -// The top level is S. +// Reference documentation about the descriptor language can be found in +// doc/descriptors.md. /** Interface for parsed descriptor objects. */ struct Descriptor { diff --git a/src/streams.h b/src/streams.h index d4a309be3c..dc20f7a9da 100644 --- a/src/streams.h +++ b/src/streams.h @@ -529,7 +529,7 @@ public: explicit BitStreamReader(IStream& istream) : m_istream(istream) {} /** Read the specified number of bits from the stream. The data is returned - * in the nbits least signficant bits of a 64-bit uint. + * in the nbits least significant bits of a 64-bit uint. */ uint64_t Read(int nbits) { if (nbits < 0 || nbits > 64) { diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 070b3ed80e..8d577cf521 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -75,7 +75,7 @@ void* Arena::alloc(size_t size) // Create the used-chunk, taking its space from the end of the free-chunk const size_t size_remaining = size_ptr_it->first - size; - auto alloced = chunks_used.emplace(size_ptr_it->second + size_remaining, size).first; + auto allocated = chunks_used.emplace(size_ptr_it->second + size_remaining, size).first; chunks_free_end.erase(size_ptr_it->second + size_ptr_it->first); if (size_ptr_it->first == size) { // whole chunk is used up @@ -88,7 +88,7 @@ void* Arena::alloc(size_t size) } size_to_free_chunk.erase(size_ptr_it); - return reinterpret_cast<void*>(alloced->first); + return reinterpret_cast<void*>(allocated->first); } void Arena::free(void *ptr) diff --git a/src/sync.cpp b/src/sync.cpp index 255eb4f00b..c9aa98dcd6 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -100,7 +100,11 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, } LogPrintf(" %s\n", i.second.ToString()); } - assert(false); + if (g_debug_lockorder_abort) { + fprintf(stderr, "Assertion failed: detected inconsistent lock order at %s:%i, details in debug log.\n", __FILE__, __LINE__); + abort(); + } + throw std::logic_error("potential deadlock detected"); } static void push_lock(void* c, const CLockLocation& locklocation) @@ -189,4 +193,6 @@ void DeleteLock(void* cs) } } +bool g_debug_lockorder_abort = true; + #endif /* DEBUG_LOCKORDER */ diff --git a/src/sync.h b/src/sync.h index 11c1253757..40709bdd7f 100644 --- a/src/sync.h +++ b/src/sync.h @@ -46,14 +46,42 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII // // /////////////////////////////// +#ifdef DEBUG_LOCKORDER +void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); +void LeaveCritical(); +std::string LocksHeld(); +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void DeleteLock(void* cs); + /** - * Template mixin that adds -Wthread-safety locking - * annotations to a subset of the mutex API. + * Call abort() if a potential lock order deadlock bug is detected, instead of + * just logging information and throwing a logic_error. Defaults to true, and + * set to false in DEBUG_LOCKORDER unit tests. + */ +extern bool g_debug_lockorder_abort; +#else +void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} +void static inline LeaveCritical() {} +void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} +void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +void static inline DeleteLock(void* cs) {} +#endif +#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) +#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) + +/** + * Template mixin that adds -Wthread-safety locking annotations and lock order + * checking to a subset of the mutex API. */ template <typename PARENT> class LOCKABLE AnnotatedMixin : public PARENT { public: + ~AnnotatedMixin() { + DeleteLock((void*)this); + } + void lock() EXCLUSIVE_LOCK_FUNCTION() { PARENT::lock(); @@ -68,64 +96,36 @@ public: { return PARENT::try_lock(); } -}; -#ifdef DEBUG_LOCKORDER -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); -void LeaveCritical(); -std::string LocksHeld(); -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); -void DeleteLock(void* cs); -#else -void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} -void static inline LeaveCritical() {} -void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} -void static inline DeleteLock(void* cs) {} -#endif -#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) -#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) + using UniqueLock = std::unique_lock<PARENT>; +}; /** * Wrapped mutex: supports recursive locking, but no waiting * TODO: We should move away from using the recursive lock by default. */ -class CCriticalSection : public AnnotatedMixin<std::recursive_mutex> -{ -public: - ~CCriticalSection() { - DeleteLock((void*)this); - } -}; +typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection; /** Wrapped mutex: supports waiting but not recursive locking */ -typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection; - -/** Just a typedef for std::condition_variable, can be wrapped later if desired */ -typedef std::condition_variable CConditionVariable; - -/** Just a typedef for std::unique_lock, can be wrapped later if desired */ -typedef std::unique_lock<std::mutex> WaitableLock; +typedef AnnotatedMixin<std::mutex> Mutex; #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char* pszName, const char* pszFile, int nLine); #endif -/** Wrapper around std::unique_lock<CCriticalSection> */ -class SCOPED_LOCKABLE CCriticalBlock +/** Wrapper around std::unique_lock style lock for Mutex. */ +template <typename Mutex, typename Base = typename Mutex::UniqueLock> +class SCOPED_LOCKABLE UniqueLock : public Base { private: - std::unique_lock<CCriticalSection> lock; - void Enter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex())); #ifdef DEBUG_LOCKCONTENTION - if (!lock.try_lock()) { + if (!Base::try_lock()) { PrintLockContention(pszName, pszFile, nLine); #endif - lock.lock(); + Base::lock(); #ifdef DEBUG_LOCKCONTENTION } #endif @@ -133,15 +133,15 @@ private: bool TryEnter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); - lock.try_lock(); - if (!lock.owns_lock()) + EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true); + Base::try_lock(); + if (!Base::owns_lock()) LeaveCritical(); - return lock.owns_lock(); + return Base::owns_lock(); } public: - CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock) + UniqueLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock) { if (fTry) TryEnter(pszName, pszFile, nLine); @@ -149,35 +149,41 @@ public: Enter(pszName, pszFile, nLine); } - CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) + UniqueLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) { if (!pmutexIn) return; - lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock); + *static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock); if (fTry) TryEnter(pszName, pszFile, nLine); else Enter(pszName, pszFile, nLine); } - ~CCriticalBlock() UNLOCK_FUNCTION() + ~UniqueLock() UNLOCK_FUNCTION() { - if (lock.owns_lock()) + if (Base::owns_lock()) LeaveCritical(); } operator bool() { - return lock.owns_lock(); + return Base::owns_lock(); } }; +template<typename MutexArg> +using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>; + #define PASTE(x, y) x ## y #define PASTE2(x, y) PASTE(x, y) -#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) -#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__) -#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) +#define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) +#define LOCK2(cs1, cs2) \ + DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \ + DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__); +#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true) +#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 39d0061c1c..2732598b4f 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -155,7 +155,7 @@ static void Correct_Queue_range(std::vector<size_t> range) } // Make vChecks here to save on malloc (this test can be slow...) std::vector<FakeCheckCheckCompletion> vChecks; - for (auto i : range) { + for (const size_t i : range) { size_t total = i; FakeCheckCheckCompletion::n_calls = 0; CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get()); @@ -253,7 +253,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) } for (auto times = 0; times < 10; ++times) { - for (bool end_fails : {true, false}) { + for (const bool end_fails : {true, false}) { CCheckQueueControl<FailingCheck> control(fail_queue.get()); { std::vector<FailingCheck> vChecks; diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 63b91b74c0..6f4b5ecd26 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -508,7 +508,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) Coin cc4; ss4 >> cc4; BOOST_CHECK_MESSAGE(false, "We should have thrown"); - } catch (const std::ios_base::failure& e) { + } catch (const std::ios_base::failure&) { } // Very large scriptPubKey (3*10^9 bytes) past the end of the stream @@ -521,7 +521,7 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) Coin cc5; ss5 >> cc5; BOOST_CHECK_MESSAGE(false, "We should have thrown"); - } catch (const std::ios_base::failure& e) { + } catch (const std::ios_base::failure&) { } } @@ -719,7 +719,7 @@ static void CheckAddCoinBase(CAmount base_value, CAmount cache_value, CAmount mo test.cache.AddCoin(OUTPOINT, Coin(std::move(output), 1, coinbase), coinbase); test.cache.SelfTest(); GetCoinsMapEntry(test.cache.map(), result_value, result_flags); - } catch (std::logic_error& e) { + } catch (std::logic_error&) { result_value = FAIL; result_flags = NO_ENTRY; } @@ -736,7 +736,7 @@ static void CheckAddCoinBase(CAmount base_value, CAmount cache_value, CAmount mo template <typename... Args> static void CheckAddCoin(Args&&... args) { - for (CAmount base_value : {ABSENT, PRUNED, VALUE1}) + for (const CAmount base_value : {ABSENT, PRUNED, VALUE1}) CheckAddCoinBase(base_value, std::forward<Args>(args)...); } @@ -780,7 +780,7 @@ void CheckWriteCoins(CAmount parent_value, CAmount child_value, CAmount expected WriteCoinsViewEntry(test.cache, child_value, child_flags); test.cache.SelfTest(); GetCoinsMapEntry(test.cache.map(), result_value, result_flags); - } catch (std::logic_error& e) { + } catch (std::logic_error&) { result_value = FAIL; result_flags = NO_ENTRY; } @@ -848,10 +848,10 @@ BOOST_AUTO_TEST_CASE(ccoins_write) // they would be too repetitive (the parent cache is never updated in these // cases). The loop below covers these cases and makes sure the parent cache // is always left unchanged. - for (CAmount parent_value : {ABSENT, PRUNED, VALUE1}) - for (CAmount child_value : {ABSENT, PRUNED, VALUE2}) - for (char parent_flags : parent_value == ABSENT ? ABSENT_FLAGS : FLAGS) - for (char child_flags : child_value == ABSENT ? ABSENT_FLAGS : CLEAN_FLAGS) + for (const CAmount parent_value : {ABSENT, PRUNED, VALUE1}) + for (const CAmount child_value : {ABSENT, PRUNED, VALUE2}) + for (const char parent_flags : parent_value == ABSENT ? ABSENT_FLAGS : FLAGS) + for (const char child_flags : child_value == ABSENT ? ABSENT_FLAGS : CLEAN_FLAGS) CheckWriteCoins(parent_value, child_value, parent_value, parent_flags, child_flags, parent_flags); } diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp index ec7aba34ca..dbceb9d2e0 100644 --- a/src/test/cuckoocache_tests.cpp +++ b/src/test/cuckoocache_tests.cpp @@ -82,11 +82,11 @@ static double test_cache(size_t megabytes, double load) */ std::vector<uint256> hashes_insert_copy = hashes; /** Do the insert */ - for (uint256& h : hashes_insert_copy) + for (const uint256& h : hashes_insert_copy) set.insert(h); /** Count the hits */ uint32_t count = 0; - for (uint256& h : hashes) + for (const uint256& h : hashes) count += set.contains(h, false); double hit_rate = ((double)count) / ((double)n_insert); return hit_rate; @@ -323,7 +323,7 @@ static void test_cache_generations() reads.push_back(inserts[i]); for (uint32_t i = n_insert - (n_insert / 4); i < n_insert; ++i) reads.push_back(inserts[i]); - for (auto h : inserts) + for (const auto& h : inserts) c.insert(h); } }; diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 4dff1e5c72..9957ac074b 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -26,7 +26,7 @@ BOOST_FIXTURE_TEST_SUITE(dbwrapper_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(dbwrapper) { // Perform tests both obfuscated and non-obfuscated. - for (bool obfuscate : {false, true}) { + for (const bool obfuscate : {false, true}) { fs::path ph = SetDataDir(std::string("dbwrapper").append(obfuscate ? "_true" : "_false")); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'k'; @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) BOOST_AUTO_TEST_CASE(dbwrapper_batch) { // Perform tests both obfuscated and non-obfuscated. - for (bool obfuscate : {false, true}) { + for (const bool obfuscate : {false, true}) { fs::path ph = SetDataDir(std::string("dbwrapper_batch").append(obfuscate ? "_true" : "_false")); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); @@ -82,7 +82,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) BOOST_AUTO_TEST_CASE(dbwrapper_iterator) { // Perform tests both obfuscated and non-obfuscated. - for (bool obfuscate : {false, true}) { + for (const bool obfuscate : {false, true}) { fs::path ph = SetDataDir(std::string("dbwrapper_iterator").append(obfuscate ? "_true" : "_false")); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) if (x & 1) BOOST_CHECK(dbw.Write(key, value)); } - for (int seek_start : {0x00, 0x80}) { + for (const int seek_start : {0x00, 0x80}) { it->Seek((uint8_t)seek_start); for (unsigned int x=seek_start; x<255; ++x) { uint8_t key; @@ -262,7 +262,7 @@ struct StringContentsSerializer { try { READWRITE(c); str.push_back(c); - } catch (const std::ios_base::failure& e) { + } catch (const std::ios_base::failure&) { break; } } @@ -291,7 +291,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) } std::unique_ptr<CDBIterator> it(const_cast<CDBWrapper&>(dbw).NewIterator()); - for (int seek_start : {0, 5}) { + for (const int seek_start : {0, 5}) { snprintf(buf, sizeof(buf), "%d", seek_start); StringContentsSerializer seek_key(buf); it->Seek(seek_key); diff --git a/src/test/gen/crypto_gen.cpp b/src/test/gen/crypto_gen.cpp new file mode 100644 index 0000000000..ca8c65806f --- /dev/null +++ b/src/test/gen/crypto_gen.cpp @@ -0,0 +1,19 @@ +// Copyright (c) 2018 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 <test/gen/crypto_gen.h> + +#include <key.h> + +#include <rapidcheck/gen/Arbitrary.h> +#include <rapidcheck/Gen.h> +#include <rapidcheck/gen/Predicate.h> +#include <rapidcheck/gen/Container.h> + +/** Generates 1 to 20 keys for OP_CHECKMULTISIG */ +rc::Gen<std::vector<CKey>> MultisigKeys() +{ + return rc::gen::suchThat(rc::gen::arbitrary<std::vector<CKey>>(), [](const std::vector<CKey>& keys) { + return keys.size() >= 1 && keys.size() <= 15; + }); +}; diff --git a/src/test/gen/crypto_gen.h b/src/test/gen/crypto_gen.h new file mode 100644 index 0000000000..7c2fb0350f --- /dev/null +++ b/src/test/gen/crypto_gen.h @@ -0,0 +1,63 @@ +// Copyright (c) 2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#ifndef BITCOIN_TEST_GEN_CRYPTO_GEN_H +#define BITCOIN_TEST_GEN_CRYPTO_GEN_H + +#include <key.h> +#include <random.h> +#include <uint256.h> +#include <rapidcheck/gen/Arbitrary.h> +#include <rapidcheck/Gen.h> +#include <rapidcheck/gen/Create.h> +#include <rapidcheck/gen/Numeric.h> + +/** Generates 1 to 15 keys for OP_CHECKMULTISIG */ +rc::Gen<std::vector<CKey>> MultisigKeys(); + +namespace rc +{ +/** Generator for a new CKey */ +template <> +struct Arbitrary<CKey> { + static Gen<CKey> arbitrary() + { + return rc::gen::map<int>([](int x) { + CKey key; + key.MakeNewKey(true); + return key; + }); + }; +}; + +/** Generator for a CPrivKey */ +template <> +struct Arbitrary<CPrivKey> { + static Gen<CPrivKey> arbitrary() + { + return gen::map(gen::arbitrary<CKey>(), [](const CKey& key) { + return key.GetPrivKey(); + }); + }; +}; + +/** Generator for a new CPubKey */ +template <> +struct Arbitrary<CPubKey> { + static Gen<CPubKey> arbitrary() + { + return gen::map(gen::arbitrary<CKey>(), [](const CKey& key) { + return key.GetPubKey(); + }); + }; +}; +/** Generates a arbitrary uint256 */ +template <> +struct Arbitrary<uint256> { + static Gen<uint256> arbitrary() + { + return rc::gen::just(GetRandHash()); + }; +}; +} //namespace rc +#endif diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index eec7b38af7..7592330b10 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -24,7 +24,7 @@ static void ResetArgs(const std::string& strArg) // Convert to char*: std::vector<const char*> vecChar; - for (std::string& s : vecArg) + for (const std::string& s : vecArg) vecChar.push_back(s.c_str()); std::string error; diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp index 7e475ac610..a0c10d8ddd 100644 --- a/src/test/key_io_tests.cpp +++ b/src/test/key_io_tests.cpp @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(key_io_invalid) std::string exp_base58string = test[0].get_str(); // must be invalid as public and as private key - for (auto chain : { CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::REGTEST }) { + for (const auto& chain : { CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::REGTEST }) { SelectParams(chain); destination = DecodeDestination(exp_base58string); BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid pubkey in mainnet:" + strTest); diff --git a/src/test/key_properties.cpp b/src/test/key_properties.cpp new file mode 100644 index 0000000000..14e3c85359 --- /dev/null +++ b/src/test/key_properties.cpp @@ -0,0 +1,53 @@ +// Copyright (c) 2018 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 <key.h> + +#include <base58.h> +#include <script/script.h> +#include <uint256.h> +#include <util.h> +#include <utilstrencodings.h> +#include <test/test_bitcoin.h> +#include <string> +#include <vector> + +#include <boost/test/unit_test.hpp> +#include <rapidcheck/boost_test.h> +#include <rapidcheck/gen/Arbitrary.h> +#include <rapidcheck/Gen.h> + +#include <test/gen/crypto_gen.h> + +BOOST_FIXTURE_TEST_SUITE(key_properties, BasicTestingSetup) + +/** Check CKey uniqueness */ +RC_BOOST_PROP(key_uniqueness, (const CKey& key1, const CKey& key2)) +{ + RC_ASSERT(!(key1 == key2)); +} + +/** Verify that a private key generates the correct public key */ +RC_BOOST_PROP(key_generates_correct_pubkey, (const CKey& key)) +{ + CPubKey pubKey = key.GetPubKey(); + RC_ASSERT(key.VerifyPubKey(pubKey)); +} + +/** Create a CKey using the 'Set' function must give us the same key */ +RC_BOOST_PROP(key_set_symmetry, (const CKey& key)) +{ + CKey key1; + key1.Set(key.begin(), key.end(), key.IsCompressed()); + RC_ASSERT(key1 == key); +} + +/** Create a CKey, sign a piece of data, then verify it with the public key */ +RC_BOOST_PROP(key_sign_symmetry, (const CKey& key, const uint256& hash)) +{ + std::vector<unsigned char> vchSig; + key.Sign(hash, vchSig, 0); + const CPubKey& pubKey = key.GetPubKey(); + RC_ASSERT(pubKey.Verify(hash, vchSig)); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 4c56938ec9..eaa8b16182 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) unsigned char pchMsgTmp[4]; ssPeers1 >> pchMsgTmp; ssPeers1 >> addrman1; - } catch (const std::exception& e) { + } catch (const std::exception&) { exceptionThrown = true; } @@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) unsigned char pchMsgTmp[4]; ssPeers1 >> pchMsgTmp; ssPeers1 >> addrman1; - } catch (const std::exception& e) { + } catch (const std::exception&) { exceptionThrown = true; } // Even through de-serialization failed addrman is not left in a clean state. diff --git a/src/test/skiplist_tests.cpp b/src/test/skiplist_tests.cpp index bcd2a9c7b6..c0754618fb 100644 --- a/src/test/skiplist_tests.cpp +++ b/src/test/skiplist_tests.cpp @@ -146,7 +146,7 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_test) BOOST_AUTO_TEST_CASE(findearliestatleast_edge_test) { std::list<CBlockIndex> blocks; - for (unsigned int timeMax : {100, 100, 100, 200, 200, 200, 300, 300, 300}) { + for (const unsigned int timeMax : {100, 100, 100, 200, 200, 200, 300, 300, 300}) { CBlockIndex* prev = blocks.empty() ? nullptr : &blocks.back(); blocks.emplace_back(); blocks.back().nHeight = prev ? prev->nHeight + 1 : 0; diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp new file mode 100644 index 0000000000..df0380546e --- /dev/null +++ b/src/test/sync_tests.cpp @@ -0,0 +1,52 @@ +// Copyright (c) 2012-2017 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 <sync.h> +#include <test/test_bitcoin.h> + +#include <boost/test/unit_test.hpp> + +namespace { +template <typename MutexType> +void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2) +{ + { + LOCK2(mutex1, mutex2); + } + bool error_thrown = false; + try { + LOCK2(mutex2, mutex1); + } catch (const std::logic_error& e) { + BOOST_CHECK_EQUAL(e.what(), "potential deadlock detected"); + error_thrown = true; + } + #ifdef DEBUG_LOCKORDER + BOOST_CHECK(error_thrown); + #else + BOOST_CHECK(!error_thrown); + #endif +} +} // namespace + +BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(potential_deadlock_detected) +{ + #ifdef DEBUG_LOCKORDER + bool prev = g_debug_lockorder_abort; + g_debug_lockorder_abort = false; + #endif + + CCriticalSection rmutex1, rmutex2; + TestPotentialDeadLockDetected(rmutex1, rmutex2); + + Mutex mutex1, mutex2; + TestPotentialDeadLockDetected(mutex1, mutex2); + + #ifdef DEBUG_LOCKORDER + g_debug_lockorder_abort = prev; + #endif +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index a89bf785f1..0d7c9f0abf 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -29,7 +29,7 @@ void CConnmanTest::AddNode(CNode& node) void CConnmanTest::ClearNodes() { LOCK(g_connman->cs_vNodes); - for (CNode* node : g_connman->vNodes) { + for (const CNode* node : g_connman->vNodes) { delete node; } g_connman->vNodes.clear(); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 33f0a83b7e..c527ad448c 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -65,7 +65,7 @@ unsigned int ParseScriptFlags(std::string strFlags) std::vector<std::string> words; boost::algorithm::split(words, strFlags, boost::algorithm::is_any_of(",")); - for (std::string word : words) + for (const std::string& word : words) { if (!mapFlagNames.count(word)) BOOST_ERROR("Bad test: unknown verification flag '" << word << "'"); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 8e2f5abe66..c74eb7531a 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -245,7 +245,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg) testArgs.ParseParameters(7, (char**)argv_test, error); // Each letter should be set. - for (char opt : "abcdef") + for (const char opt : "abcdef") BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt); // Nothing else should be in the map @@ -394,7 +394,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) && test_args.GetArg("-iii", "xxx") == "xxx" ); - for (bool def : {false, true}) { + for (const bool def : {false, true}) { BOOST_CHECK(test_args.GetBoolArg("-a", def) && test_args.GetBoolArg("-b", def) && !test_args.GetBoolArg("-ccc", def) diff --git a/src/threadinterrupt.cpp b/src/threadinterrupt.cpp index d08d52e4a9..340106ed99 100644 --- a/src/threadinterrupt.cpp +++ b/src/threadinterrupt.cpp @@ -5,6 +5,8 @@ #include <threadinterrupt.h> +#include <sync.h> + CThreadInterrupt::CThreadInterrupt() : flag(false) {} CThreadInterrupt::operator bool() const @@ -20,7 +22,7 @@ void CThreadInterrupt::reset() void CThreadInterrupt::operator()() { { - std::unique_lock<std::mutex> lock(mut); + LOCK(mut); flag.store(true, std::memory_order_release); } cond.notify_all(); @@ -28,7 +30,7 @@ void CThreadInterrupt::operator()() bool CThreadInterrupt::sleep_for(std::chrono::milliseconds rel_time) { - std::unique_lock<std::mutex> lock(mut); + WAIT_LOCK(mut, lock); return !cond.wait_for(lock, rel_time, [this]() { return flag.load(std::memory_order_acquire); }); } diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h index c30bd3d657..9c6fccfcde 100644 --- a/src/threadinterrupt.h +++ b/src/threadinterrupt.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_THREADINTERRUPT_H #define BITCOIN_THREADINTERRUPT_H +#include <sync.h> + #include <atomic> #include <chrono> #include <condition_variable> @@ -28,7 +30,7 @@ public: private: std::condition_variable cond; - std::mutex mut; + Mutex mut; std::atomic<bool> flag; }; diff --git a/src/threadsafety.h b/src/threadsafety.h index a2f45e5fce..47e6b2ea38 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -10,7 +10,7 @@ // TL;DR Add GUARDED_BY(mutex) to member variables. The others are // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo); // -// See http://clang.llvm.org/docs/LanguageExtensions.html#threadsafety +// See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html // for documentation. The clang compiler can do advanced static analysis // of locking when given the -Wthread-safety option. #define LOCKABLE __attribute__((lockable)) diff --git a/src/timedata.cpp b/src/timedata.cpp index 1599508306..291111feb2 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -94,7 +94,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) { // If nobody has a time different than ours but within 5 minutes of ours, give a warning bool fMatch = false; - for (int64_t nOffset : vSorted) + for (const int64_t nOffset : vSorted) if (nOffset != 0 && abs64(nOffset) < 5 * 60) fMatch = true; @@ -109,7 +109,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) } if (LogAcceptCategory(BCLog::NET)) { - for (int64_t n : vSorted) { + for (const int64_t n : vSorted) { LogPrint(BCLog::NET, "%+d ", n); /* Continued */ } LogPrint(BCLog::NET, "| "); /* Continued */ diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 05217149e3..39434e4bb6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -193,7 +193,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr } const setEntries & setMemPoolParents = GetMemPoolParents(stageit); - for (const txiter &phash : setMemPoolParents) { + for (txiter phash : setMemPoolParents) { // If this is a new ancestor, add it. if (setAncestors.count(phash) == 0) { parentHashes.insert(phash); @@ -454,7 +454,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants stage.erase(it); const setEntries &setChildren = GetMemPoolChildren(it); - for (const txiter &childiter : setChildren) { + for (txiter childiter : setChildren) { if (!setDescendants.count(childiter)) { stage.insert(childiter); } @@ -899,7 +899,7 @@ size_t CTxMemPool::DynamicMemoryUsage() const { void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) { AssertLockHeld(cs); UpdateForRemoveFromMempool(stage, updateDescendants); - for (const txiter& it : stage) { + for (txiter it : stage) { removeUnchecked(it, reason); } } diff --git a/src/util.cpp b/src/util.cpp index 3bb52e9b3d..84d8175389 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -818,11 +818,11 @@ static std::string TrimString(const std::string& str, const std::string& pattern return str.substr(front, end - front + 1); } -static std::vector<std::pair<std::string, std::string>> GetConfigOptions(std::istream& stream) +static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>> &options) { - std::vector<std::pair<std::string, std::string>> options; std::string str, prefix; std::string::size_type pos; + int linenr = 1; while (std::getline(stream, str)) { if ((pos = str.find('#')) != std::string::npos) { str = str.substr(0, pos); @@ -832,21 +832,34 @@ static std::vector<std::pair<std::string, std::string>> GetConfigOptions(std::is if (!str.empty()) { if (*str.begin() == '[' && *str.rbegin() == ']') { prefix = str.substr(1, str.size() - 2) + '.'; + } else if (*str.begin() == '-') { + error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str); + return false; } else if ((pos = str.find('=')) != std::string::npos) { std::string name = prefix + TrimString(str.substr(0, pos), pattern); std::string value = TrimString(str.substr(pos + 1), pattern); options.emplace_back(name, value); + } else { + error = strprintf("parse error on line %i: %s", linenr, str); + if (str.size() >= 2 && str.substr(0, 2) == "no") { + error += strprintf(", if you intended to specify a negated option, use %s=1 instead", str); + } + return false; } } + ++linenr; } - return options; + return true; } bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys) { LOCK(cs_args); - - for (const std::pair<std::string, std::string>& option : GetConfigOptions(stream)) { + std::vector<std::pair<std::string, std::string>> options; + if (!GetConfigOptions(stream, error, options)) { + return false; + } + for (const std::pair<std::string, std::string>& option : options) { std::string strKey = std::string("-") + option.first; std::string strValue = option.second; diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 2326383586..4940267bae 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -72,7 +72,7 @@ bool IsHexNumber(const std::string& str) if (str.size() > 2 && *str.begin() == '0' && *(str.begin()+1) == 'x') { starting_location = 2; } - for (auto c : str.substr(starting_location)) { + for (const char c : str.substr(starting_location)) { if (HexDigit(c) < 0) return false; } // Return false for empty string or "0x". diff --git a/src/validation.cpp b/src/validation.cpp index f6257ffaed..7ec0c6a961 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -217,8 +217,8 @@ CCriticalSection cs_main; BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex; CChain& chainActive = g_chainstate.chainActive; CBlockIndex *pindexBestHeader = nullptr; -CWaitableCriticalSection g_best_block_mutex; -CConditionVariable g_best_block_cv; +Mutex g_best_block_mutex; +std::condition_variable g_best_block_cv; uint256 g_best_block; int nScriptCheckThreads = 0; std::atomic_bool fImporting(false); @@ -421,7 +421,7 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool // lock on a mempool input, so we can use the return value of // CheckSequenceLocks to indicate the LockPoints validity int maxInputHeight = 0; - for (int height : prevheights) { + for (const int height : prevheights) { // Can ignore mempool inputs since we'll fail if they had non-zero locks if (height != tip->nHeight+1) { maxInputHeight = std::max(maxInputHeight, height); @@ -2239,7 +2239,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar mempool.AddTransactionsUpdated(1); { - WaitableLock lock(g_best_block_mutex); + LOCK(g_best_block_mutex); g_best_block = pindexNew->GetBlockHash(); g_best_block_cv.notify_all(); } @@ -4291,7 +4291,7 @@ void UnloadBlockIndex() warningcache[b].clear(); } - for (BlockMap::value_type& entry : mapBlockIndex) { + for (const BlockMap::value_type& entry : mapBlockIndex) { delete entry.second; } mapBlockIndex.clear(); @@ -4492,7 +4492,7 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) // Build forward-pointing map of the entire block tree. std::multimap<CBlockIndex*,CBlockIndex*> forward; - for (auto& entry : mapBlockIndex) { + for (const std::pair<const uint256, CBlockIndex*>& entry : mapBlockIndex) { forward.insert(std::make_pair(entry.second->pprev, entry.second)); } diff --git a/src/validation.h b/src/validation.h index c4c9b8b5ba..3df6456eca 100644 --- a/src/validation.h +++ b/src/validation.h @@ -151,8 +151,8 @@ extern BlockMap& mapBlockIndex; extern uint64_t nLastBlockTx; extern uint64_t nLastBlockWeight; extern const std::string strMessageMagic; -extern CWaitableCriticalSection g_best_block_mutex; -extern CConditionVariable g_best_block_cv; +extern Mutex g_best_block_mutex; +extern std::condition_variable g_best_block_cv; extern uint256 g_best_block; extern std::atomic_bool fImporting; extern std::atomic_bool fReindex; diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 4fa9603011..729e4e39b0 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -311,7 +311,7 @@ bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn) return false; fUseCrypto = true; - for (KeyMap::value_type& mKey : mapKeys) + for (const KeyMap::value_type& mKey : mapKeys) { const CKey &key = mKey.second; CPubKey vchPubKey = key.GetPubKey(); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index d0fe51801e..dfd60ae5eb 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -72,7 +72,7 @@ BerkeleyEnvironment* GetWalletEnv(const fs::path& wallet_path, std::string& data database_filename = "wallet.dat"; } LOCK(cs_db); - // Note: An ununsed temporary BerkeleyEnvironment object may be created inside the + // Note: An unused temporary BerkeleyEnvironment object may be created inside the // emplace function if the key already exists. This is a little inefficient, // but not a big concern since the map will be changed in the future to hold // pointers instead of objects, anyway. @@ -503,7 +503,7 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo // be implemented, so no equality checks are needed at all. (Newer // versions of BDB have an set_lk_exclusive method for this // purpose, but the older version we use does not.) - for (auto& env : g_dbenvs) { + for (const auto& env : g_dbenvs) { CheckUniqueFileid(env.second, strFilename, *pdb_temp); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 60f14e5886..af36d321d5 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -41,7 +41,7 @@ int64_t static DecodeDumpTime(const std::string &str) { std::string static EncodeDumpString(const std::string &str) { std::stringstream ret; - for (unsigned char c : str) { + for (const unsigned char c : str) { if (c <= 32 || c >= 128 || c == '%') { ret << '%' << HexStr(&c, &c + 1); } else { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 66ec070982..e419f8d164 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -278,7 +278,6 @@ static UniValue setlabel(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); } - std::string old_label = pwallet->mapAddressBook[dest].name; std::string label = LabelFromValue(request.params[1]); if (IsMine(*pwallet, dest)) { @@ -2338,7 +2337,7 @@ static UniValue listlockunspent(const JSONRPCRequest& request) UniValue ret(UniValue::VARR); - for (COutPoint &outpt : vOutpts) { + for (const COutPoint& outpt : vOutpts) { UniValue o(UniValue::VOBJ); o.pushKV("txid", outpt.hash.GetHex()); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6cbfbc1f90..e41f829f02 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1484,7 +1484,7 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall // Look up the inputs. We should have already checked that this transaction // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our // wallet, with a valid index into the vout array, and the ability to sign. - for (auto& input : tx.vin) { + for (const CTxIn& input : tx.vin) { const auto mi = wallet->mapWallet.find(input.prevout.hash); if (mi == wallet->mapWallet.end()) { return -1; @@ -1723,7 +1723,7 @@ void CWallet::ReacceptWalletTransactions() } // Try to add wallet transactions to memory pool - for (std::pair<const int64_t, CWalletTx*>& item : mapSorted) { + for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) { CWalletTx& wtx = *(item.second); CValidationState state; wtx.AcceptToMemoryPool(maxTxFee, state); @@ -1969,7 +1969,7 @@ std::vector<uint256> CWallet::ResendWalletTransactionsBefore(int64_t nTime, CCon continue; mapSorted.insert(std::make_pair(wtx.nTimeReceived, &wtx)); } - for (std::pair<const unsigned int, CWalletTx*>& item : mapSorted) + for (const std::pair<const unsigned int, CWalletTx*>& item : mapSorted) { CWalletTx& wtx = *item.second; if (wtx.RelayWalletTransaction(connman)) @@ -2260,7 +2260,7 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const AvailableCoins(availableCoins); - for (auto& coin : availableCoins) { + for (const COutput& coin : availableCoins) { CTxDestination address; if (coin.fSpendable && ExtractDestination(FindNonChangeParentOutput(*coin.tx->tx, coin.i).scriptPubKey, address)) { @@ -2270,7 +2270,7 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const std::vector<COutPoint> lockedCoins; ListLockedCoins(lockedCoins); - for (const auto& output : lockedCoins) { + for (const COutPoint& output : lockedCoins) { auto it = mapWallet.find(output.hash); if (it != mapWallet.end()) { int depth = it->second.GetDepthInMainChain(); @@ -3146,17 +3146,17 @@ bool CWallet::NewKeyPool() LOCK(cs_wallet); WalletBatch batch(*database); - for (int64_t nIndex : setInternalKeyPool) { + for (const int64_t nIndex : setInternalKeyPool) { batch.ErasePool(nIndex); } setInternalKeyPool.clear(); - for (int64_t nIndex : setExternalKeyPool) { + for (const int64_t nIndex : setExternalKeyPool) { batch.ErasePool(nIndex); } setExternalKeyPool.clear(); - for (int64_t nIndex : set_pre_split_keypool) { + for (const int64_t nIndex : set_pre_split_keypool) { batch.ErasePool(nIndex); } set_pre_split_keypool.clear(); @@ -3433,7 +3433,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() { bool any_mine = false; // group all input addresses with each other - for (CTxIn txin : pcoin->tx->vin) + for (const CTxIn& txin : pcoin->tx->vin) { CTxDestination address; if(!IsMine(txin)) /* If this input isn't mine, ignore it */ @@ -3447,7 +3447,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() // group change with input addresses if (any_mine) { - for (CTxOut txout : pcoin->tx->vout) + for (const CTxOut& txout : pcoin->tx->vout) if (IsChange(txout)) { CTxDestination txoutAddr; @@ -3483,7 +3483,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() // make a set of all the groups hit by this new group std::set< std::set<CTxDestination>* > hits; std::map< CTxDestination, std::set<CTxDestination>* >::iterator it; - for (CTxDestination address : _grouping) + for (const CTxDestination& address : _grouping) if ((it = setmap.find(address)) != setmap.end()) hits.insert((*it).second); @@ -3498,12 +3498,12 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() uniqueGroupings.insert(merged); // update setmap - for (CTxDestination element : *merged) + for (const CTxDestination& element : *merged) setmap[element] = merged; } std::set< std::set<CTxDestination> > ret; - for (std::set<CTxDestination>* uniqueGrouping : uniqueGroupings) + for (const std::set<CTxDestination>* uniqueGrouping : uniqueGroupings) { ret.insert(*uniqueGrouping); delete uniqueGrouping; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5ce4d080e5..5e85151358 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -520,7 +520,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if ((wss.nKeys + wss.nCKeys + wss.nWatchKeys) != wss.nKeyMeta) pwallet->UpdateTimeFirstKey(1); - for (uint256 hash : wss.vWalletUpgrade) + for (const uint256& hash : wss.vWalletUpgrade) WriteTx(pwallet->mapWallet.at(hash)); // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: @@ -611,7 +611,7 @@ DBErrors WalletBatch::ZapSelectTx(std::vector<uint256>& vTxHashIn, std::vector<u // erase each matching wallet TX bool delerror = false; std::vector<uint256>::iterator it = vTxHashIn.begin(); - for (uint256 hash : vTxHash) { + for (const uint256& hash : vTxHash) { while (it < vTxHashIn.end() && (*it) < hash) { it++; } @@ -642,7 +642,7 @@ DBErrors WalletBatch::ZapWalletTx(std::vector<CWalletTx>& vWtx) return err; // erase each wallet TX - for (uint256& hash : vTxHash) { + for (const uint256& hash : vTxHash) { if (!EraseTx(hash)) return DBErrors::CORRUPT; } diff --git a/test/functional/example_test.py b/test/functional/example_test.py index a8c1474876..3edd760b90 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -76,7 +76,7 @@ class ExampleTest(BitcoinTestFramework): def set_test_params(self): """Override test parameters for your individual test. - This method must be overridden and num_nodes must be exlicitly set.""" + This method must be overridden and num_nodes must be explicitly set.""" self.setup_clean_chain = True self.num_nodes = 3 # Use self.extra_args to change command-line arguments for the nodes diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 40b5c27580..71c3a396c1 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -989,7 +989,7 @@ class FullBlockTest(BitcoinTestFramework): assert_equal(get_legacy_sigopcount_block(b73), MAX_BLOCK_SIGOPS + 1) self.sync_blocks([b73], success=False, reject_reason='bad-blk-sigops', reconnect=True) - # b74/75 - if we push an invalid script element, all prevous sigops are counted, + # b74/75 - if we push an invalid script element, all previous sigops are counted, # but sigops after the element are not counted. # # The invalid script element is that the push_data indicates that diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 62091048f9..9be59b32b4 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -14,8 +14,29 @@ class ConfArgsTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 + def test_config_file_parser(self): + # Assume node is stopped + + inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf') + with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf: + conf.write('includeconf={}\n'.format(inc_conf_file_path)) + + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('-dash=1\n') + self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: -dash=1, options in configuration file must be specified without leading -') + + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('nono\n') + self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead') + + with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: + conf.write('') # clear + def run_test(self): self.stop_node(0) + + self.test_config_file_parser() + # Remove the -datadir argument so it doesn't override the config file self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")] diff --git a/test/functional/mining_getblocktemplate_longpoll.py b/test/functional/mining_getblocktemplate_longpoll.py index 2bcbe8db7b..1259754c5a 100755 --- a/test/functional/mining_getblocktemplate_longpoll.py +++ b/test/functional/mining_getblocktemplate_longpoll.py @@ -15,8 +15,8 @@ class LongpollThread(threading.Thread): def __init__(self, node): threading.Thread.__init__(self) # query current longpollid - templat = node.getblocktemplate() - self.longpollid = templat['longpollid'] + template = node.getblocktemplate() + self.longpollid = template['longpollid'] # create a new connection to the node, we can't use the same # connection from two threads self.node = get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir) @@ -31,11 +31,11 @@ class GetBlockTemplateLPTest(BitcoinTestFramework): def run_test(self): self.log.info("Warning: this test will take about 70 seconds in the best case. Be patient.") self.nodes[0].generate(10) - templat = self.nodes[0].getblocktemplate() - longpollid = templat['longpollid'] + template = self.nodes[0].getblocktemplate() + longpollid = template['longpollid'] # longpollid should not change between successive invocations if nothing else happens - templat2 = self.nodes[0].getblocktemplate() - assert(templat2['longpollid'] == longpollid) + template2 = self.nodes[0].getblocktemplate() + assert(template2['longpollid'] == longpollid) # Test 1: test that the longpolling wait if we do nothing thr = LongpollThread(self.nodes[0]) diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py index be0290edfd..69d5dd9f9a 100755 --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -76,7 +76,7 @@ class InvalidTxRequestTest(BitcoinTestFramework): self.reconnect_p2p(num_connections=2) self.log.info('Test orphan transaction handling ... ') - # Create a root transaction that we withhold until all dependend transactions + # Create a root transaction that we withhold until all dependent transactions # are sent out and in the orphan cache SCRIPT_PUB_KEY_OP_TRUE = b'\x51\x75' * 15 + b'\x51' tx_withhold = CTransaction() diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index c987bf4b05..4740740d42 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -89,7 +89,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): sync_blocks([self.nodes[0], self.nodes[2]], timeout=5) except: pass - # node2 must remain at heigh 0 + # node2 must remain at height 0 assert_equal(self.nodes[2].getblockheader(self.nodes[2].getbestblockhash())['height'], 0) # now connect also to node 1 (non pruned) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 51c57e88e0..043fa67f77 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -460,7 +460,7 @@ class SegWitTest(BitcoinTestFramework): blocks are permitted to contain witnesses).""" # node2 doesn't need to be connected for this test. - # (If it's connected, node0 may propogate an invalid block to it over + # (If it's connected, node0 may propagate an invalid block to it over # compact blocks and the nodes would have inconsistent tips.) disconnect_nodes(self.nodes[0], 2) diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py index 32088fe3e1..58074803cc 100755 --- a/test/functional/rpc_deprecated.py +++ b/test/functional/rpc_deprecated.py @@ -18,13 +18,7 @@ class DeprecatedRpcTest(BitcoinTestFramework): # self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses") # assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()]) # self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()]) - - self.log.info("Test validateaddress deprecation") - SOME_ADDRESS = "mnvGjUy3NMj67yJ6gkK5o9e5RS33Z2Vqcu" # This is just some random address to pass as a parameter to validateaddress - dep_validate_address = self.nodes[0].validateaddress(SOME_ADDRESS) - assert "ismine" not in dep_validate_address - not_dep_val = self.nodes[1].validateaddress(SOME_ADDRESS) - assert "ismine" in not_dep_val + pass if __name__ == '__main__': DeprecatedRpcTest().main() diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index e878ded258..ceca40527f 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test RPC help output.""" -from test_framework.test_framework import BitcoinTestFramework +from test_framework.test_framework import BitcoinTestFramework, is_zmq_enabled from test_framework.util import assert_equal, assert_raises_rpc_error class HelpRpcTest(BitcoinTestFramework): @@ -25,7 +25,13 @@ class HelpRpcTest(BitcoinTestFramework): # command titles titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')] - assert_equal(titles, ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq']) + + components = ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet'] + + if is_zmq_enabled(self): + components.append('Zmq') + + assert_equal(titles, components) if __name__ == '__main__': HelpRpcTest().main() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index d558de5fe1..a693b7e4bb 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -105,7 +105,7 @@ class PSBTTest(BitcoinTestFramework): signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex']) assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].converttopsbt, signedtx['hex']) - # Explicilty allow converting non-empty txs + # Explicitly allow converting non-empty txs new_psbt = self.nodes[0].converttopsbt(rawtx['hex']) self.nodes[0].decodepsbt(new_psbt) diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 32b099294f..f6eea1a027 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -42,10 +42,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): # 2) No script verification error occurred assert 'errors' not in rawTxSigned - # Perform the same test on signrawtransaction - rawTxSigned2 = self.nodes[0].signrawtransaction(rawTx, inputs, privKeys) - assert_equal(rawTxSigned, rawTxSigned2) - def script_verification_error_test(self): """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script. @@ -112,10 +108,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(rawTxSigned['errors'][1]['vout'], inputs[2]['vout']) assert not rawTxSigned['errors'][0]['witness'] - # Perform same test with signrawtransaction - rawTxSigned2 = self.nodes[0].signrawtransaction(rawTx, scripts, privKeys) - assert_equal(rawTxSigned, rawTxSigned2) - # Now test signing failure for transaction with input witnesses p2wpkh_raw_tx = "01000000000102fff7f7881a8099afa6940d42d1e7f6362bec38171ea3edf433541db4e4ad969f00000000494830450221008b9d1dc26ba6a9cb62127b02742fa9d754cd3bebf337f7a55d114c8e5cdd30be022040529b194ba3f9281a99f2b1c0a19c0489bc22ede944ccf4ecbab4cc618ef3ed01eeffffffef51e1b804cc89d182d279655c3aa89e815b1b309fe287d9b2b55d57b90ec68a0100000000ffffffff02202cb206000000001976a9148280b37df378db99f66f85c95a783a76ac7a6d5988ac9093510d000000001976a9143bde42dbee7e4dbe6a21b2d50ce2f0167faa815988ac000247304402203609e17b84f6a7d30c80bfa610b5b4542f32a8a0d5447a12fb1366d7f01cc44a0220573a954c4518331561406f90300e8f3358f51928d43c212a8caed02de67eebee0121025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee635711000000" @@ -140,10 +132,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(rawTxSigned['errors'][1]['witness'], ["304402203609e17b84f6a7d30c80bfa610b5b4542f32a8a0d5447a12fb1366d7f01cc44a0220573a954c4518331561406f90300e8f3358f51928d43c212a8caed02de67eebee01", "025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee6357"]) assert not rawTxSigned['errors'][0]['witness'] - # Perform same test with signrawtransaction - rawTxSigned2 = self.nodes[0].signrawtransaction(p2wpkh_raw_tx) - assert_equal(rawTxSigned, rawTxSigned2) - def run_test(self): self.successful_signing_test() self.script_verification_error_test() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 987ade4044..35004fb588 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -122,7 +122,7 @@ def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, script_pub_key=C """Return one-input, one-output transaction object spending the prevtx's n-th output with the given amount. - Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend ouput. + Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend output. """ tx = CTransaction() assert(n < len(prevtx.vout)) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index b876d9bd76..0e76b52570 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -488,8 +488,13 @@ def skip_if_no_py3_zmq(): def skip_if_no_bitcoind_zmq(test_instance): """Skip the running test if bitcoind has not been compiled with zmq support.""" + if not is_zmq_enabled(test_instance): + raise SkipTest("bitcoind has not been built with zmq enabled.") + + +def is_zmq_enabled(test_instance): + """Checks whether zmq is enabled or not.""" config = configparser.ConfigParser() config.read_file(open(test_instance.options.configfile)) - if not config["components"].getboolean("ENABLE_ZMQ"): - raise SkipTest("bitcoind has not been built with zmq enabled.") + return config["components"].getboolean("ENABLE_ZMQ") diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index ab8e0e57d1..0c4a603167 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -26,8 +26,8 @@ SET_DOC_OPTIONAL = set(['-rpcssl', '-benchmark', '-h', '-help', '-socks', '-tor' def main(): - used = check_output(CMD_GREP_ARGS, shell=True, universal_newlines=True) - docd = check_output(CMD_GREP_DOCS, shell=True, universal_newlines=True) + used = check_output(CMD_GREP_ARGS, shell=True, universal_newlines=True, encoding='utf8') + docd = check_output(CMD_GREP_DOCS, shell=True, universal_newlines=True, encoding='utf8') args_used = set(re.findall(re.compile(REGEX_ARG), used)) args_docd = set(re.findall(re.compile(REGEX_DOC), docd)).union(SET_DOC_OPTIONAL) diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index b8d105b49b..3972baed1d 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -25,7 +25,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog" "qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel" "qt/walletmodel -> qt/walletmodeltransaction -> qt/walletmodel" - "rpc/rawtransaction -> wallet/rpcwallet -> rpc/rawtransaction" "txmempool -> validation -> txmempool" "validation -> validationinterface -> validation" "wallet/coincontrol -> wallet/wallet -> wallet/coincontrol" diff --git a/test/lint/lint-format-strings.sh b/test/lint/lint-format-strings.sh index 17f846d29b..2c443abf6b 100755 --- a/test/lint/lint-format-strings.sh +++ b/test/lint/lint-format-strings.sh @@ -33,7 +33,9 @@ if ! python3 -m doctest test/lint/lint-format-strings.py; then fi for S in "${FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS[@]}"; do IFS="," read -r FUNCTION_NAME SKIP_ARGUMENTS <<< "${S}" - mapfile -t MATCHING_FILES < <(git grep --full-name -l "${FUNCTION_NAME}" -- "*.c" "*.cpp" "*.h" | sort | grep -vE "^src/(leveldb|secp256k1|tinyformat|univalue)") + for MATCHING_FILE in $(git grep --full-name -l "${FUNCTION_NAME}" -- "*.c" "*.cpp" "*.h" | sort | grep -vE "^src/(leveldb|secp256k1|tinyformat|univalue)"); do + MATCHING_FILES+=("${MATCHING_FILE}") + done if ! test/lint/lint-format-strings.py --skip-arguments "${SKIP_ARGUMENTS}" "${FUNCTION_NAME}" "${MATCHING_FILES[@]}"; then EXIT_CODE=1 fi diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index a5b97ca1e9..cbee437c91 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -197,11 +197,11 @@ REGEXP_IGNORE_KNOWN_VIOLATIONS=$(join_array "|" "${KNOWN_VIOLATIONS[@]}") # Invoke "git grep" only once in order to minimize run-time REGEXP_LOCALE_DEPENDENT_FUNCTIONS=$(join_array "|" "${LOCALE_DEPENDENT_FUNCTIONS[@]}") -GIT_GREP_OUTPUT=$(git grep -E "[^a-zA-Z0-9_\`'\"<>](${REGEXP_LOCALE_DEPENDENT_FUNCTIONS}(|_r|_s))[^a-zA-Z0-9_\`'\"<>]" -- "*.cpp" "*.h") +GIT_GREP_OUTPUT=$(git grep -E "[^a-zA-Z0-9_\`'\"<>](${REGEXP_LOCALE_DEPENDENT_FUNCTIONS}(_r|_s)?)[^a-zA-Z0-9_\`'\"<>]" -- "*.cpp" "*.h") EXIT_CODE=0 for LOCALE_DEPENDENT_FUNCTION in "${LOCALE_DEPENDENT_FUNCTIONS[@]}"; do - MATCHES=$(grep -E "[^a-zA-Z0-9_\`'\"<>]${LOCALE_DEPENDENT_FUNCTION}(|_r|_s)[^a-zA-Z0-9_\`'\"<>]" <<< "${GIT_GREP_OUTPUT}" | \ + MATCHES=$(grep -E "[^a-zA-Z0-9_\`'\"<>]${LOCALE_DEPENDENT_FUNCTION}(_r|_s)?[^a-zA-Z0-9_\`'\"<>]" <<< "${GIT_GREP_OUTPUT}" | \ grep -vE "\.(c|cpp|h):\s*(//|\*|/\*|\").*${LOCALE_DEPENDENT_FUNCTION}" | \ grep -vE 'fprintf\(.*(stdout|stderr)') if [[ ${REGEXP_IGNORE_EXTERNAL_DEPENDENCIES} != "" ]]; then diff --git a/test/lint/lint-python-utf8-encoding.sh b/test/lint/lint-python-utf8-encoding.sh index 14183a5ccf..d03c20205d 100755 --- a/test/lint/lint-python-utf8-encoding.sh +++ b/test/lint/lint-python-utf8-encoding.sh @@ -17,4 +17,12 @@ if [[ ${OUTPUT} != "" ]]; then echo "${OUTPUT}" EXIT_CODE=1 fi +OUTPUT=$(git grep "check_output(" -- "*.py" | grep "universal_newlines=True" | grep -vE "encoding=.(ascii|utf8|utf-8).") +if [[ ${OUTPUT} != "" ]]; then + echo "Python's check_output(...) seems to be used to get program outputs without explicitly" + echo "specifying encoding=\"utf8\":" + echo + echo "${OUTPUT}" + EXIT_CODE=1 +fi exit ${EXIT_CODE} diff --git a/test/lint/lint-python.sh b/test/lint/lint-python.sh index 7e73790517..4f4542ff0c 100755 --- a/test/lint/lint-python.sh +++ b/test/lint/lint-python.sh @@ -79,4 +79,12 @@ export LC_ALL=C # W605 invalid escape sequence "x" # W606 'async' and 'await' are reserved keywords starting with Python 3.7 -flake8 --ignore=B,C,E,F,I,N,W --select=E101,E112,E113,E115,E116,E125,E129,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,E901,E902,F401,F402,F403,F404,F405,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W191,W291,W292,W293,W504,W601,W602,W603,W604,W605,W606 . +if ! command -v flake8 > /dev/null; then + echo "Skipping Python linting since flake8 is not installed. Install by running \"pip3 install flake8\"" + exit 0 +elif PYTHONWARNINGS="ignore" flake8 --version | grep -q "Python 2"; then + echo "Skipping Python linting since flake8 is running under Python 2. Install the Python 3 version of flake8 by running \"pip3 install flake8\"" + exit 0 +fi + +PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=E101,E112,E113,E115,E116,E125,E129,E131,E133,E223,E224,E242,E266,E271,E272,E273,E274,E275,E304,E306,E401,E402,E502,E701,E702,E703,E714,E721,E741,E742,E743,E901,E902,F401,F402,F403,F404,F405,F406,F407,F601,F602,F621,F622,F631,F701,F702,F703,F704,F705,F706,F707,F811,F812,F821,F822,F823,F831,F841,W191,W291,W292,W293,W504,W601,W602,W603,W604,W605,W606 . diff --git a/test/lint/lint-shell-locale.sh b/test/lint/lint-shell-locale.sh index efd8081b8c..084dc93f76 100755 --- a/test/lint/lint-shell-locale.sh +++ b/test/lint/lint-shell-locale.sh @@ -16,7 +16,7 @@ for SHELL_SCRIPT in $(git ls-files -- "*.sh" | grep -vE "src/(secp256k1|univalue if grep -q "# This script is intentionally locale dependent by not setting \"export LC_ALL=C\"" "${SHELL_SCRIPT}"; then continue fi - FIRST_NON_COMMENT_LINE=$(grep -vE '^(#.*|)$' "${SHELL_SCRIPT}" | head -1) + FIRST_NON_COMMENT_LINE=$(grep -vE '^(#.*)?$' "${SHELL_SCRIPT}" | head -1) if [[ ${FIRST_NON_COMMENT_LINE} != "export LC_ALL=C" && ${FIRST_NON_COMMENT_LINE} != "export LC_ALL=C.UTF-8" ]]; then echo "Missing \"export LC_ALL=C\" (to avoid locale dependence) as first non-comment non-empty line in ${SHELL_SCRIPT}" EXIT_CODE=1 diff --git a/test/lint/lint-shell.sh b/test/lint/lint-shell.sh index 5e1e136e7d..9af3c10ed6 100755 --- a/test/lint/lint-shell.sh +++ b/test/lint/lint-shell.sh @@ -16,7 +16,14 @@ if [ "$TRAVIS" = "true" ]; then unset LC_ALL fi +if ! command -v shellcheck > /dev/null; then + echo "Skipping shell linting since shellcheck is not installed." + exit 0 +fi + # Disabled warnings: +# SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet). +# SC1117: Backslash is literal in "\.". Prefer explicit escaping: "\\.". # SC2001: See if you can use ${variable//search/replace} instead. # SC2004: $/${} is unnecessary on arithmetic variables. # SC2005: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'. @@ -33,5 +40,8 @@ fi # SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. # SC2166: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined. # SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?. -shellcheck -e SC2001,SC2004,SC2005,SC2006,SC2016,SC2028,SC2046,SC2048,SC2066,SC2086,SC2116,SC2148,SC2162,SC2166,SC2181 \ +# SC2206: Quote to prevent word splitting, or split robustly with mapfile or read -a. +# SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting). +# SC2230: which is non-standard. Use builtin 'command -v' instead. +shellcheck -e SC1087,SC1117,SC2001,SC2004,SC2005,SC2006,SC2016,SC2028,SC2046,SC2048,SC2066,SC2086,SC2116,SC2148,SC2162,SC2166,SC2181,SC2206,SC2207,SC2230 \ $(git ls-files -- "*.sh" | grep -vE 'src/(secp256k1|univalue)/') diff --git a/test/lint/lint-spelling.ignore-words.txt b/test/lint/lint-spelling.ignore-words.txt new file mode 100644 index 0000000000..9a49f32271 --- /dev/null +++ b/test/lint/lint-spelling.ignore-words.txt @@ -0,0 +1,6 @@ +cas +hights +mor +objext +unselect +useable diff --git a/test/lint/lint-spelling.sh b/test/lint/lint-spelling.sh new file mode 100755 index 0000000000..5d672698a7 --- /dev/null +++ b/test/lint/lint-spelling.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# +# Warn in case of spelling errors. +# Note: Will exit successfully regardless of spelling errors. + +export LC_ALL=C + +IGNORE_WORDS_FILE=test/lint/lint-spelling.ignore-words.txt +if ! codespell --check-filenames --disable-colors --quiet-level=7 --ignore-words=${IGNORE_WORDS_FILE} $(git ls-files -- ":(exclude)build-aux/m4/" ":(exclude)contrib/seeds/*.txt" ":(exclude)depends/" ":(exclude)doc/release-notes/" ":(exclude)src/leveldb/" ":(exclude)src/qt/locale/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/"); then + echo "^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in ${IGNORE_WORDS_FILE}" +fi |