diff options
108 files changed, 1649 insertions, 1160 deletions
diff --git a/configure.ac b/configure.ac index c2e34a52ca..48f104bdca 100644 --- a/configure.ac +++ b/configure.ac @@ -148,9 +148,9 @@ AC_ARG_WITH([qrencode], AC_ARG_ENABLE([hardening], [AS_HELP_STRING([--disable-hardening], - [do not attempt to harden the resulting executables (default is to harden)])], + [do not attempt to harden the resulting executables (default is to harden when possible)])], [use_hardening=$enableval], - [use_hardening=yes]) + [use_hardening=auto]) AC_ARG_ENABLE([reduce-exports], [AS_HELP_STRING([--enable-reduce-exports], @@ -219,6 +219,13 @@ AC_ARG_ENABLE([debug], [enable_debug=$enableval], [enable_debug=no]) +# Enable gprof profiling +AC_ARG_ENABLE([gprof], + [AS_HELP_STRING([--enable-gprof], + [use gprof profiling compiler flags (default is no)])], + [enable_gprof=$enableval], + [enable_gprof=no]) + # Turn warnings into errors AC_ARG_ENABLE([werror], [AS_HELP_STRING([--enable-werror], @@ -558,12 +565,30 @@ else AC_SEARCH_LIBS([clock_gettime],[rt]) fi +if test "x$enable_gprof" = xyes; then + dnl -pg is incompatible with -pie. Since hardening and profiling together doesn't make sense, + dnl we simply make them mutually exclusive here. Additionally, hardened toolchains may force + dnl -pie by default, in which case it needs to be turned off with -no-pie. + + if test x$use_hardening = xyes; then + AC_MSG_ERROR(gprof profiling is not compatible with hardening. Reconfigure with --disable-hardening or --disable-gprof) + fi + use_hardening=no + AX_CHECK_COMPILE_FLAG([-pg],[GPROF_CXXFLAGS="-pg"], + [AC_MSG_ERROR(gprof profiling requested but not available)], [[$CXXFLAG_WERROR]]) + + AX_CHECK_LINK_FLAG([[-no-pie]], [GPROF_LDFLAGS="-no-pie"]) + AX_CHECK_LINK_FLAG([[-pg]],[GPROF_LDFLAGS="$GPROF_LDFLAGS -pg"], + [AC_MSG_ERROR(gprof profiling requested but not available)], [[$GPROF_LDFLAGS]]) +fi + if test x$TARGET_OS != xwindows; then # All windows code is PIC, forcing it on just adds useless compile warnings AX_CHECK_COMPILE_FLAG([-fPIC],[PIC_FLAGS="-fPIC"]) fi if test x$use_hardening != xno; then + use_hardening=yes AX_CHECK_COMPILE_FLAG([-Wstack-protector],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -Wstack-protector"]) AX_CHECK_COMPILE_FLAG([-fstack-protector-all],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-protector-all"]) @@ -1242,6 +1267,8 @@ AC_SUBST(BITCOIN_TX_NAME) AC_SUBST(RELDFLAGS) AC_SUBST(ERROR_CXXFLAGS) +AC_SUBST(GPROF_CXXFLAGS) +AC_SUBST(GPROF_LDFLAGS) AC_SUBST(HARDENED_CXXFLAGS) AC_SUBST(HARDENED_CPPFLAGS) AC_SUBST(HARDENED_LDFLAGS) @@ -1335,6 +1362,7 @@ echo " with bench = $use_bench" echo " with upnp = $use_upnp" echo " use asm = $use_asm" echo " debug enabled = $enable_debug" +echo " gprof enabled = $enable_gprof" echo " werror = $enable_werror" echo echo " target os = $TARGET_OS" diff --git a/contrib/devtools/lint-whitespace.sh b/contrib/devtools/lint-whitespace.sh index af9a57910a..c5b9408ff2 100755 --- a/contrib/devtools/lint-whitespace.sh +++ b/contrib/devtools/lint-whitespace.sh @@ -7,12 +7,26 @@ # Check for new lines in diff that introduce trailing whitespace. # We can't run this check unless we know the commit range for the PR. + +while getopts "?" opt; do + case $opt in + ?) + echo "Usage: .lint-whitespace.sh [N]" + echo " TRAVIS_COMMIT_RANGE='<commit range>' .lint-whitespace.sh" + echo " .lint-whitespace.sh -?" + echo "Checks unstaged changes, the previous N commits, or a commit range." + echo "TRAVIS_COMMIT_RANGE='47ba2c3...ee50c9e' .lint-whitespace.sh" + exit 0 + ;; + esac +done + if [ -z "${TRAVIS_COMMIT_RANGE}" ]; then - echo "Cannot run lint-whitespace.sh without commit range. To run locally, use:" - echo "TRAVIS_COMMIT_RANGE='<commit range>' .lint-whitespace.sh" - echo "For example:" - echo "TRAVIS_COMMIT_RANGE='47ba2c3...ee50c9e' .lint-whitespace.sh" - exit 1 + if [ "$1" ]; then + TRAVIS_COMMIT_RANGE="HEAD~$1...HEAD" + else + TRAVIS_COMMIT_RANGE="HEAD" + fi fi showdiff() { @@ -59,7 +73,7 @@ if showdiff | grep -E -q '^\+.*\s+$'; then fi # Check if tab characters were found in the diff. -if showcodediff | grep -P -q '^\+.*\t'; then +if showcodediff | perl -nle '$MATCH++ if m{^\+.*\t}; END{exit 1 unless $MATCH>0}' > /dev/null; then echo "This diff appears to have added new lines with tab characters instead of spaces." echo "The following changes were suspected:" FILENAME="" @@ -81,7 +95,7 @@ if showcodediff | grep -P -q '^\+.*\t'; then fi echo "$line" fi - done < <(showcodediff | grep -P '^(diff --git |@@|\+.*\t)') + done < <(showcodediff | perl -nle 'print if m{^(diff --git |@@|\+.*\t)}') RET=1 fi diff --git a/contrib/gitian-keys/keys.txt b/contrib/gitian-keys/keys.txt index 47da725b74..593fba1d09 100644 --- a/contrib/gitian-keys/keys.txt +++ b/contrib/gitian-keys/keys.txt @@ -1,7 +1,6 @@ 617C90010B3BD370B0AC7D424BB42E31C79111B8 Akira Takizawa -152812300785C96444D3334D17565732E08E5E41 Andrew Chow E944AE667CF960B1004BC32FCA662BE18B877A60 Andreas Schildbach -07DF3E57A548CCFB7530709189BBB8663E2E65CE Matt Corallo (BlueMatt) +152812300785C96444D3334D17565732E08E5E41 Andrew Chow 912FD3228387123DC97E0E57D5566241A0295FA9 BtcDrak C519EBCF3B926298946783EFF6430754120EC2F4 Christian Decker (cdecker) F20F56EF6A067F70E8A5C99FFF95FAA971697405 centaur @@ -9,21 +8,23 @@ C060A6635913D98A3587D7DB1C2491FFEB0EF770 Cory Fields BF6273FAEF7CC0BA1F562E50989F6B3048A116B5 Dev Random 9A1689B60D1B3CCE9262307A2F40A9BF167FBA47 Erik Mossberg (erkmos) D35176BE9264832E4ACA8986BF0792FBE95DC863 fivepiece -E777299FC265DD04793070EB944D35F9AC3DB76A Michael Ford 01CDF4627A3B88AAE4A571C87588242FBE38D3A8 Gavin Andresen D3CC177286005BB8FF673294C5242A1AB3936517 jl2012 32EE5C4C3FA15CCADB46ABE529D4BCB6416F53EC Jonas Schnelli 4B4E840451149DD7FB0D633477DFAB5C3108B9A8 Jorge Timon -71A3B16735405025D447E8F274810B012346C9A6 Wladimir J. van der Laan +C42AFF7C61B3E44A1454CD3557AF762DB3353322 Karl-Johan Alm (kallewoof) E463A93F5F3117EEDE6C7316BD02942421F4889F Luke Dashjr B8B3F1C0E58C15DB6A81D30C3648A882F4316B9B Marco Falke +07DF3E57A548CCFB7530709189BBB8663E2E65CE Matt Corallo (BlueMatt) CA03882CB1FC067B5D3ACFE4D300116E1C875A3D MeshCollider +E777299FC265DD04793070EB944D35F9AC3DB76A Michael Ford 9692B91BBF0E8D34DFD33B1882C5C009628ECF0C Michagogo -37EC7D7B0A217CDB4B4E007E7FAB114267E4FA04 Peter Todd +77E72E69DA7EE0A148C06B21B34821D4944DE5F7 Nils Schneider D62A803E27E7F43486035ADBBCD04D8E9CCCAC2A Paul Rabahy +37EC7D7B0A217CDB4B4E007E7FAB114267E4FA04 Peter Todd D762373D24904A3E42F33B08B9A408E71DAAC974 Pieter Wuille (Location: Leuven, Belgium) 133EAC179436F14A5CF1B794860FEB804E669320 Pieter Wuille ED9BDF7AD6A55E232E84524257FF9BDBCC301009 Sjors Provoost -77E72E69DA7EE0A148C06B21B34821D4944DE5F7 Nils Schneider -79D00BAC68B56D422F945A8F8E3A8F3247DBCBBF Willy Ko AEC1884398647C47413C1C3FB1179EB7347DC10D Warren Togami +79D00BAC68B56D422F945A8F8E3A8F3247DBCBBF Willy Ko +71A3B16735405025D447E8F274810B012346C9A6 Wladimir J. van der Laan diff --git a/contrib/verify-commits/trusted-git-root b/contrib/verify-commits/trusted-git-root index c60f8ab695..e560b98d02 100644 --- a/contrib/verify-commits/trusted-git-root +++ b/contrib/verify-commits/trusted-git-root @@ -1 +1 @@ -82bcf405f6db1d55b684a1f63a4aabad376cdad7 +11049f4fe62606d1b0380a9ef800ac130f0fbadf diff --git a/contrib/zmq/zmq_sub.py b/contrib/zmq/zmq_sub.py index 6e44c56f52..60768dc59a 100755..100644 --- a/contrib/zmq/zmq_sub.py +++ b/contrib/zmq/zmq_sub.py @@ -38,7 +38,7 @@ port = 28332 class ZMQHandler(): def __init__(self): - self.loop = zmq.asyncio.install() + self.loop = asyncio.get_event_loop() self.zmqContext = zmq.asyncio.Context() self.zmqSubSocket = self.zmqContext.socket(zmq.SUB) diff --git a/contrib/zmq/zmq_sub3.4.py b/contrib/zmq/zmq_sub3.4.py index 536352d5ff..0df843c9a3 100755..100644 --- a/contrib/zmq/zmq_sub3.4.py +++ b/contrib/zmq/zmq_sub3.4.py @@ -42,7 +42,7 @@ port = 28332 class ZMQHandler(): def __init__(self): - self.loop = zmq.asyncio.install() + self.loop = asyncio.get_event_loop() self.zmqContext = zmq.asyncio.Context() self.zmqSubSocket = self.zmqContext.socket(zmq.SUB) diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index 3f97221e10..cde523370f 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -1,9 +1,9 @@ package=zeromq -$(package)_version=4.2.2 +$(package)_version=4.2.3 $(package)_download_path=https://github.com/zeromq/libzmq/releases/download/v$($(package)_version)/ $(package)_file_name=$(package)-$($(package)_version).tar.gz -$(package)_sha256_hash=5b23f4ca9ef545d5bd3af55d305765e3ee06b986263b31967435d285a3e6df6b -$(package)_patches=0001-fix-build-with-older-mingw64.patch +$(package)_sha256_hash=8f1e2b2aade4dbfde98d82366d61baef2f62e812530160d2e6d0a5bb24e40bc0 +$(package)_patches=0001-fix-build-with-older-mingw64.patch 0002-disable-pthread_set_name_np.patch define $(package)_set_vars $(package)_config_opts=--without-docs --disable-shared --without-libsodium --disable-curve --disable-curve-keygen --disable-perf @@ -13,7 +13,7 @@ endef define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/0001-fix-build-with-older-mingw64.patch && \ - ./autogen.sh + patch -p1 < $($(package)_patch_dir)/0002-disable-pthread_set_name_np.patch endef define $(package)_config_cmds diff --git a/depends/patches/zeromq/0002-disable-pthread_set_name_np.patch b/depends/patches/zeromq/0002-disable-pthread_set_name_np.patch new file mode 100644 index 0000000000..d220b54f3e --- /dev/null +++ b/depends/patches/zeromq/0002-disable-pthread_set_name_np.patch @@ -0,0 +1,35 @@ +From 6e6b47d5ab381c3df3b30bb0b0a6cf210dfb1eba Mon Sep 17 00:00:00 2001 +From: Cory Fields <cory-nospam-@coryfields.com> +Date: Mon, 5 Mar 2018 14:22:05 -0500 +Subject: [PATCH] disable pthread_set_name_np + +pthread_set_name_np adds a Glibc requirement on >= 2.12. +--- + src/thread.cpp | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/thread.cpp b/src/thread.cpp +index 4fc59c3e..c3fdfd46 100644 +--- a/src/thread.cpp ++++ b/src/thread.cpp +@@ -220,7 +220,7 @@ void zmq::thread_t::setThreadName(const char *name_) + */ + if (!name_) + return; +- ++#if 0 + #if defined(ZMQ_HAVE_PTHREAD_SETNAME_1) + int rc = pthread_setname_np(name_); + if(rc) return; +@@ -233,6 +233,8 @@ void zmq::thread_t::setThreadName(const char *name_) + #elif defined(ZMQ_HAVE_PTHREAD_SET_NAME) + pthread_set_name_np(descriptor, name_); + #endif ++#endif ++ return; + } + + #endif +-- +2.11.1 + diff --git a/doc/build-unix.md b/doc/build-unix.md index b823c23e0c..2d10484a65 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -326,6 +326,7 @@ For the wallet (optional): Then build using: ./autogen.sh + ./configure --disable-wallet # OR ./configure BDB_CFLAGS="-I${BDB_PREFIX}/include" BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx" gmake diff --git a/doc/dependencies.md b/doc/dependencies.md index 52ec8e3575..7aa96c4c9b 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -26,5 +26,5 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | Qt | [5.7.1](https://download.qt.io/official_releases/qt/) | 4.7+ | No | | | | XCB | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L94) (Linux only) | | xkbcommon | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L93) (Linux only) | -| ZeroMQ | [4.2.2](https://github.com/zeromq/libzmq/releases) | | No | | | +| ZeroMQ | [4.2.3](https://github.com/zeromq/libzmq/releases) | | No | | | | zlib | [1.2.11](http://zlib.net/) | | | | No | diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 278bf65792..a5468c3be3 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -142,6 +142,10 @@ Development tips and tricks Run configure with the --enable-debug option, then make. Or run configure with CXXFLAGS="-g -ggdb -O0" or whatever debug flags you need. +**compiling for gprof profiling** + +Run configure with the --enable-gprof option, then make. + **debug.log** If the code is behaving strangely, take a look in the debug.log file in the data directory; diff --git a/doc/release-notes.md b/doc/release-notes.md index ac49dc7909..8fcd2a9163 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -63,6 +63,36 @@ RPC changes - The `fundrawtransaction` rpc will reject the previously deprecated `reserveChangeKey` option. +External wallet files +--------------------- + +The `-wallet=<path>` option now accepts full paths instead of requiring wallets +to be located in the -walletdir directory. + +Newly created wallet format +--------------------------- + +If `-wallet=<path>` is specified with a path that does not exist, it will now +create a wallet directory at the specified location (containing a wallet.dat +data file, a db.log file, and database/log.?????????? files) instead of just +creating a data file at the path and storing log files in the parent +directory. This should make backing up wallets more straightforward than +before because the specified wallet path can just be directly archived without +having to look in the parent directory for transaction log files. + +For backwards compatibility, wallet paths that are names of existing data files +in the `-walletdir` directory will continue to be accepted and interpreted the +same as before. + +Low-level RPC changes +--------------------- + +- When bitcoin is not started with any `-wallet=<path>` options, the name of + the default wallet returned by `getwalletinfo` and `listwallets` RPCs is + now the empty string `""` instead of `"wallet.dat"`. If bitcoin is started + with any `-wallet=<path>` options, there is no change in behavior, and the + name of any wallet is just its `<path>` string. + Credits ======= diff --git a/doc/release-process.md b/doc/release-process.md index 430a5a7ed3..a988c74ba5 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -63,7 +63,7 @@ If you're using the automated script (found in [contrib/gitian-build.sh](/contri Setup Gitian descriptors: pushd ./bitcoin - export SIGNER=(your Gitian key, ie bluematt, sipa, etc) + export SIGNER="(your Gitian key, ie bluematt, sipa, etc)" export VERSION=(new version, e.g. 0.8.0) git fetch git checkout v${VERSION} @@ -93,7 +93,9 @@ Create the OS X SDK tarball, see the [OS X readme](README_osx.md) for details, a ### Optional: Seed the Gitian sources cache and offline git repositories -By default, Gitian will fetch source files as needed. To cache them ahead of time: +NOTE: Gitian is sometimes unable to download files. If you have errors, try the step below. + +By default, Gitian will fetch source files as needed. To cache them ahead of time, make sure you have checked out the tag you want to build in bitcoin, then: pushd ./gitian-builder make -C ../bitcoin/depends download SOURCES_PATH=`pwd`/cache/common @@ -113,16 +115,16 @@ The gbuild invocations below <b>DO NOT DO THIS</b> by default. pushd ./gitian-builder ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml - ./bin/gsign --signer $SIGNER --release ${VERSION}-linux --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml + ./bin/gsign --signer "$SIGNER" --release ${VERSION}-linux --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml mv build/out/bitcoin-*.tar.gz build/out/src/bitcoin-*.tar.gz ../ ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-win.yml - ./bin/gsign --signer $SIGNER --release ${VERSION}-win-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-win.yml + ./bin/gsign --signer "$SIGNER" --release ${VERSION}-win-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-win.yml mv build/out/bitcoin-*-win-unsigned.tar.gz inputs/bitcoin-win-unsigned.tar.gz mv build/out/bitcoin-*.zip build/out/bitcoin-*.exe ../ ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml - ./bin/gsign --signer $SIGNER --release ${VERSION}-osx-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml + ./bin/gsign --signer "$SIGNER" --release ${VERSION}-osx-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml mv build/out/bitcoin-*-osx-unsigned.tar.gz inputs/bitcoin-osx-unsigned.tar.gz mv build/out/bitcoin-*.tar.gz build/out/bitcoin-*.dmg ../ popd @@ -152,9 +154,9 @@ Verify the signatures Commit your signature to gitian.sigs: pushd gitian.sigs - git add ${VERSION}-linux/${SIGNER} - git add ${VERSION}-win-unsigned/${SIGNER} - git add ${VERSION}-osx-unsigned/${SIGNER} + git add ${VERSION}-linux/"${SIGNER}" + git add ${VERSION}-win-unsigned/"${SIGNER}" + git add ${VERSION}-osx-unsigned/"${SIGNER}" git commit -a git push # Assuming you can push to the gitian.sigs tree popd @@ -199,7 +201,7 @@ Create (and optionally verify) the signed OS X binary: pushd ./gitian-builder ./bin/gbuild -i --commit signature=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml - ./bin/gsign --signer $SIGNER --release ${VERSION}-osx-signed --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml + ./bin/gsign --signer "$SIGNER" --release ${VERSION}-osx-signed --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml ./bin/gverify -v -d ../gitian.sigs/ -r ${VERSION}-osx-signed ../bitcoin/contrib/gitian-descriptors/gitian-osx-signer.yml mv build/out/bitcoin-osx-signed.dmg ../bitcoin-${VERSION}-osx.dmg popd @@ -208,7 +210,7 @@ Create (and optionally verify) the signed Windows binaries: pushd ./gitian-builder ./bin/gbuild -i --commit signature=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml - ./bin/gsign --signer $SIGNER --release ${VERSION}-win-signed --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml + ./bin/gsign --signer "$SIGNER" --release ${VERSION}-win-signed --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml ./bin/gverify -v -d ../gitian.sigs/ -r ${VERSION}-win-signed ../bitcoin/contrib/gitian-descriptors/gitian-win-signer.yml mv build/out/bitcoin-*win64-setup.exe ../bitcoin-${VERSION}-win64-setup.exe mv build/out/bitcoin-*win32-setup.exe ../bitcoin-${VERSION}-win32-setup.exe @@ -217,8 +219,8 @@ Create (and optionally verify) the signed Windows binaries: Commit your signature for the signed OS X/Windows binaries: pushd gitian.sigs - git add ${VERSION}-osx-signed/${SIGNER} - git add ${VERSION}-win-signed/${SIGNER} + git add ${VERSION}-osx-signed/"${SIGNER}" + git add ${VERSION}-win-signed/"${SIGNER}" git commit -a git push # Assuming you can push to the gitian.sigs tree popd diff --git a/src/Makefile.am b/src/Makefile.am index ac822d6c5e..0a9370c85c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -4,8 +4,8 @@ DIST_SUBDIRS = secp256k1 univalue -AM_LDFLAGS = $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) -AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS) +AM_LDFLAGS = $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) +AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) AM_CPPFLAGS = $(HARDENED_CPPFLAGS) EXTRA_LIBRARIES = @@ -105,6 +105,7 @@ BITCOIN_CORE_H = \ indirectmap.h \ init.h \ key.h \ + key_io.h \ keystore.h \ dbwrapper.h \ limitedmap.h \ @@ -327,6 +328,7 @@ libbitcoin_common_a_SOURCES = \ core_read.cpp \ core_write.cpp \ key.cpp \ + key_io.cpp \ keystore.cpp \ netaddress.cpp \ netbase.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index d4d972b2bb..4ee9102519 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -9,13 +9,13 @@ TEST_SRCDIR = test TEST_BINARY=test/test_bitcoin$(EXEEXT) JSON_TEST_FILES = \ - test/data/script_tests.json \ - test/data/base58_keys_valid.json \ test/data/base58_encode_decode.json \ - test/data/base58_keys_invalid.json \ + test/data/key_io_valid.json \ + test/data/key_io_invalid.json \ + test/data/script_tests.json \ + test/data/sighash.json \ test/data/tx_invalid.json \ - test/data/tx_valid.json \ - test/data/sighash.json + test/data/tx_valid.json RAW_TEST_FILES = @@ -45,6 +45,7 @@ BITCOIN_TESTS =\ test/DoS_tests.cpp \ test/getarg_tests.cpp \ test/hash_tests.cpp \ + test/key_io_tests.cpp \ test/key_tests.cpp \ test/limitedmap_tests.cpp \ test/dbwrapper_tests.cpp \ diff --git a/src/addrman.cpp b/src/addrman.cpp index 9eefffb45b..e811dd4bea 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -187,7 +187,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId) info.fInTried = true; } -void CAddrMan::Good_(const CService& addr, int64_t nTime) +void CAddrMan::Good_(const CService& addr, bool test_before_evict, int64_t nTime) { int nId; @@ -233,10 +233,22 @@ void CAddrMan::Good_(const CService& addr, int64_t nTime) if (nUBucket == -1) return; - LogPrint(BCLog::ADDRMAN, "Moving %s to tried\n", addr.ToString()); + // which tried bucket to move the entry to + int tried_bucket = info.GetTriedBucket(nKey); + int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); + + // Will moving this address into tried evict another entry? + if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) { + LogPrint(BCLog::ADDRMAN, "Collision inserting element into tried table, moving %s to m_tried_collisions=%d\n", addr.ToString(), m_tried_collisions.size()); + if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) { + m_tried_collisions.insert(nId); + } + } else { + LogPrint(BCLog::ADDRMAN, "Moving %s to tried\n", addr.ToString()); - // move nId to the tried tables - MakeTried(info, nId); + // move nId to the tried tables + MakeTried(info, nId); + } } bool CAddrMan::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) @@ -521,3 +533,82 @@ void CAddrMan::SetServices_(const CService& addr, ServiceFlags nServices) int CAddrMan::RandomInt(int nMax){ return GetRandInt(nMax); } + +void CAddrMan::ResolveCollisions_() +{ + for (std::set<int>::iterator it = m_tried_collisions.begin(); it != m_tried_collisions.end();) { + int id_new = *it; + + bool erase_collision = false; + + // If id_new not found in mapInfo remove it from m_tried_collisions + if (mapInfo.count(id_new) != 1) { + erase_collision = true; + } else { + CAddrInfo& info_new = mapInfo[id_new]; + + // Which tried bucket to move the entry to. + int tried_bucket = info_new.GetTriedBucket(nKey); + int tried_bucket_pos = info_new.GetBucketPosition(nKey, false, tried_bucket); + if (!info_new.IsValid()) { // id_new may no longer map to a valid address + erase_collision = true; + } else if (vvTried[tried_bucket][tried_bucket_pos] != -1) { // The position in the tried bucket is not empty + + // Get the to-be-evicted address that is being tested + int id_old = vvTried[tried_bucket][tried_bucket_pos]; + CAddrInfo& info_old = mapInfo[id_old]; + + // Has successfully connected in last X hours + if (GetAdjustedTime() - info_old.nLastSuccess < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { + erase_collision = true; + } else if (GetAdjustedTime() - info_old.nLastTry < ADDRMAN_REPLACEMENT_HOURS*(60*60)) { // attempted to connect and failed in last X hours + + // Give address at least 60 seconds to successfully connect + if (GetAdjustedTime() - info_old.nLastTry > 60) { + LogPrint(BCLog::ADDRMAN, "Swapping %s for %s in tried table\n", info_new.ToString(), info_old.ToString()); + + // Replaces an existing address already in the tried table with the new address + Good_(info_new, false, GetAdjustedTime()); + erase_collision = true; + } + } + } else { // Collision is not actually a collision anymore + Good_(info_new, false, GetAdjustedTime()); + erase_collision = true; + } + } + + if (erase_collision) { + m_tried_collisions.erase(it++); + } else { + it++; + } + } +} + +CAddrInfo CAddrMan::SelectTriedCollision_() +{ + if (m_tried_collisions.size() == 0) return CAddrInfo(); + + std::set<int>::iterator it = m_tried_collisions.begin(); + + // Selects a random element from m_tried_collisions + std::advance(it, GetRandInt(m_tried_collisions.size())); + int id_new = *it; + + // If id_new not found in mapInfo remove it from m_tried_collisions + if (mapInfo.count(id_new) != 1) { + m_tried_collisions.erase(it); + return CAddrInfo(); + } + + CAddrInfo& newInfo = mapInfo[id_new]; + + // which tried bucket to move the entry to + int tried_bucket = newInfo.GetTriedBucket(nKey); + int tried_bucket_pos = newInfo.GetBucketPosition(nKey, false, tried_bucket); + + int id_old = vvTried[tried_bucket][tried_bucket_pos]; + + return mapInfo[id_old]; +} diff --git a/src/addrman.h b/src/addrman.h index 38da754afb..67423c6c55 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -165,6 +165,9 @@ public: //! ... in at least this many days #define ADDRMAN_MIN_FAIL_DAYS 7 +//! how recent a successful connection should be before we allow an address to be evicted from tried +#define ADDRMAN_REPLACEMENT_HOURS 4 + //! the maximum percentage of nodes to return in a getaddr call #define ADDRMAN_GETADDR_MAX_PCT 23 @@ -176,6 +179,9 @@ public: #define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2) #define ADDRMAN_BUCKET_SIZE (1 << ADDRMAN_BUCKET_SIZE_LOG2) +//! the maximum number of tried addr collisions to store +#define ADDRMAN_SET_TRIED_COLLISION_SIZE 10 + /** * Stochastical (IP) address manager */ @@ -212,6 +218,9 @@ private: //! last time Good was called (memory only) int64_t nLastGood; + //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discpline used to resolve these collisions. + std::set<int> m_tried_collisions; + protected: //! secret key to randomize bucket select with uint256 nKey; @@ -239,7 +248,7 @@ protected: void ClearNew(int nUBucket, int nUBucketPos); //! Mark an entry "good", possibly moving it from "new" to "tried". - void Good_(const CService &addr, int64_t nTime); + void Good_(const CService &addr, bool test_before_evict, int64_t time); //! Add an entry to the "new" table. bool Add_(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty); @@ -250,6 +259,12 @@ protected: //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. CAddrInfo Select_(bool newOnly); + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions_(); + + //! Return a random to-be-evicted tried table address. + CAddrInfo SelectTriedCollision_(); + //! Wraps GetRandInt to allow tests to override RandomInt and make it determinismistic. virtual int RandomInt(int nMax); @@ -537,11 +552,11 @@ public: } //! Mark an entry as accessible. - void Good(const CService &addr, int64_t nTime = GetAdjustedTime()) + void Good(const CService &addr, bool test_before_evict = true, int64_t nTime = GetAdjustedTime()) { LOCK(cs); Check(); - Good_(addr, nTime); + Good_(addr, test_before_evict, nTime); Check(); } @@ -554,6 +569,28 @@ public: Check(); } + //! See if any to-be-evicted tried table entries have been tested and if so resolve the collisions. + void ResolveCollisions() + { + LOCK(cs); + Check(); + ResolveCollisions_(); + Check(); + } + + //! Randomly select an address in tried that another address is attempting to evict. + CAddrInfo SelectTriedCollision() + { + CAddrInfo ret; + { + LOCK(cs); + Check(); + ret = SelectTriedCollision_(); + Check(); + } + return ret; + } + /** * Choose an address to connect to. */ diff --git a/src/arith_uint256.h b/src/arith_uint256.h index dc2627592e..3f4cc8c2bf 100644 --- a/src/arith_uint256.h +++ b/src/arith_uint256.h @@ -85,7 +85,7 @@ public: base_uint ret; for (int i = 0; i < WIDTH; i++) ret.pn[i] = ~pn[i]; - ret++; + ++ret; return ret; } diff --git a/src/base58.cpp b/src/base58.cpp index 499afbe382..982e123a1d 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -4,20 +4,12 @@ #include <base58.h> -#include <bech32.h> #include <hash.h> -#include <script/script.h> #include <uint256.h> -#include <utilstrencodings.h> -#include <boost/variant/apply_visitor.hpp> -#include <boost/variant/static_visitor.hpp> - -#include <algorithm> #include <assert.h> #include <string.h> - /** All alphanumeric characters except for "0", "I", "O", and "l" */ static const char* pszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; @@ -151,227 +143,3 @@ bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRe { return DecodeBase58Check(str.c_str(), vchRet); } - -CBase58Data::CBase58Data() -{ - vchVersion.clear(); - vchData.clear(); -} - -void CBase58Data::SetData(const std::vector<unsigned char>& vchVersionIn, const void* pdata, size_t nSize) -{ - vchVersion = vchVersionIn; - vchData.resize(nSize); - if (!vchData.empty()) - memcpy(vchData.data(), pdata, nSize); -} - -void CBase58Data::SetData(const std::vector<unsigned char>& vchVersionIn, const unsigned char* pbegin, const unsigned char* pend) -{ - SetData(vchVersionIn, (void*)pbegin, pend - pbegin); -} - -bool CBase58Data::SetString(const char* psz, unsigned int nVersionBytes) -{ - std::vector<unsigned char> vchTemp; - bool rc58 = DecodeBase58Check(psz, vchTemp); - if ((!rc58) || (vchTemp.size() < nVersionBytes)) { - vchData.clear(); - vchVersion.clear(); - return false; - } - vchVersion.assign(vchTemp.begin(), vchTemp.begin() + nVersionBytes); - vchData.resize(vchTemp.size() - nVersionBytes); - if (!vchData.empty()) - memcpy(vchData.data(), vchTemp.data() + nVersionBytes, vchData.size()); - memory_cleanse(vchTemp.data(), vchTemp.size()); - return true; -} - -bool CBase58Data::SetString(const std::string& str) -{ - return SetString(str.c_str()); -} - -std::string CBase58Data::ToString() const -{ - std::vector<unsigned char> vch = vchVersion; - vch.insert(vch.end(), vchData.begin(), vchData.end()); - return EncodeBase58Check(vch); -} - -int CBase58Data::CompareTo(const CBase58Data& b58) const -{ - if (vchVersion < b58.vchVersion) - return -1; - if (vchVersion > b58.vchVersion) - return 1; - if (vchData < b58.vchData) - return -1; - if (vchData > b58.vchData) - return 1; - return 0; -} - -namespace -{ -class DestinationEncoder : public boost::static_visitor<std::string> -{ -private: - const CChainParams& m_params; - -public: - DestinationEncoder(const CChainParams& params) : m_params(params) {} - - std::string operator()(const CKeyID& id) const - { - std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::PUBKEY_ADDRESS); - data.insert(data.end(), id.begin(), id.end()); - return EncodeBase58Check(data); - } - - std::string operator()(const CScriptID& id) const - { - std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); - data.insert(data.end(), id.begin(), id.end()); - return EncodeBase58Check(data); - } - - std::string operator()(const WitnessV0KeyHash& id) const - { - std::vector<unsigned char> data = {0}; - ConvertBits<8, 5, true>(data, id.begin(), id.end()); - return bech32::Encode(m_params.Bech32HRP(), data); - } - - std::string operator()(const WitnessV0ScriptHash& id) const - { - std::vector<unsigned char> data = {0}; - ConvertBits<8, 5, true>(data, id.begin(), id.end()); - return bech32::Encode(m_params.Bech32HRP(), data); - } - - std::string operator()(const WitnessUnknown& id) const - { - if (id.version < 1 || id.version > 16 || id.length < 2 || id.length > 40) { - return {}; - } - std::vector<unsigned char> data = {(unsigned char)id.version}; - ConvertBits<8, 5, true>(data, id.program, id.program + id.length); - return bech32::Encode(m_params.Bech32HRP(), data); - } - - std::string operator()(const CNoDestination& no) const { return {}; } -}; - -CTxDestination DecodeDestination(const std::string& str, const CChainParams& params) -{ - std::vector<unsigned char> data; - uint160 hash; - if (DecodeBase58Check(str, data)) { - // base58-encoded Bitcoin addresses. - // Public-key-hash-addresses have version 0 (or 111 testnet). - // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. - const std::vector<unsigned char>& pubkey_prefix = params.Base58Prefix(CChainParams::PUBKEY_ADDRESS); - if (data.size() == hash.size() + pubkey_prefix.size() && std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin())) { - std::copy(data.begin() + pubkey_prefix.size(), data.end(), hash.begin()); - return CKeyID(hash); - } - // Script-hash-addresses have version 5 (or 196 testnet). - // The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script. - const std::vector<unsigned char>& script_prefix = params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); - if (data.size() == hash.size() + script_prefix.size() && std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) { - std::copy(data.begin() + script_prefix.size(), data.end(), hash.begin()); - return CScriptID(hash); - } - } - data.clear(); - auto bech = bech32::Decode(str); - if (bech.second.size() > 0 && bech.first == params.Bech32HRP()) { - // Bech32 decoding - int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16) - // The rest of the symbols are converted witness program bytes. - if (ConvertBits<5, 8, false>(data, bech.second.begin() + 1, bech.second.end())) { - if (version == 0) { - { - WitnessV0KeyHash keyid; - if (data.size() == keyid.size()) { - std::copy(data.begin(), data.end(), keyid.begin()); - return keyid; - } - } - { - WitnessV0ScriptHash scriptid; - if (data.size() == scriptid.size()) { - std::copy(data.begin(), data.end(), scriptid.begin()); - return scriptid; - } - } - return CNoDestination(); - } - if (version > 16 || data.size() < 2 || data.size() > 40) { - return CNoDestination(); - } - WitnessUnknown unk; - unk.version = version; - std::copy(data.begin(), data.end(), unk.program); - unk.length = data.size(); - return unk; - } - } - return CNoDestination(); -} -} // namespace - -void CBitcoinSecret::SetKey(const CKey& vchSecret) -{ - assert(vchSecret.IsValid()); - SetData(Params().Base58Prefix(CChainParams::SECRET_KEY), vchSecret.begin(), vchSecret.size()); - if (vchSecret.IsCompressed()) - vchData.push_back(1); -} - -CKey CBitcoinSecret::GetKey() -{ - CKey ret; - assert(vchData.size() >= 32); - ret.Set(vchData.begin(), vchData.begin() + 32, vchData.size() > 32 && vchData[32] == 1); - return ret; -} - -bool CBitcoinSecret::IsValid() const -{ - bool fExpectedFormat = vchData.size() == 32 || (vchData.size() == 33 && vchData[32] == 1); - bool fCorrectVersion = vchVersion == Params().Base58Prefix(CChainParams::SECRET_KEY); - return fExpectedFormat && fCorrectVersion; -} - -bool CBitcoinSecret::SetString(const char* pszSecret) -{ - return CBase58Data::SetString(pszSecret) && IsValid(); -} - -bool CBitcoinSecret::SetString(const std::string& strSecret) -{ - return SetString(strSecret.c_str()); -} - -std::string EncodeDestination(const CTxDestination& dest) -{ - return boost::apply_visitor(DestinationEncoder(Params()), dest); -} - -CTxDestination DecodeDestination(const std::string& str) -{ - return DecodeDestination(str, Params()); -} - -bool IsValidDestinationString(const std::string& str, const CChainParams& params) -{ - return IsValidDestination(DecodeDestination(str, params)); -} - -bool IsValidDestinationString(const std::string& str) -{ - return IsValidDestinationString(str, Params()); -} diff --git a/src/base58.h b/src/base58.h index 39eb4eacc2..8f2833bec9 100644 --- a/src/base58.h +++ b/src/base58.h @@ -14,12 +14,6 @@ #ifndef BITCOIN_BASE58_H #define BITCOIN_BASE58_H -#include <chainparams.h> -#include <key.h> -#include <pubkey.h> -#include <script/standard.h> -#include <support/allocators/zeroafterfree.h> - #include <string> #include <vector> @@ -56,95 +50,12 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -inline bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet); +bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -inline bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet); - -/** - * Base class for all base58-encoded data - */ -class CBase58Data -{ -protected: - //! the version byte(s) - std::vector<unsigned char> vchVersion; - - //! the actually encoded data - typedef std::vector<unsigned char, zero_after_free_allocator<unsigned char> > vector_uchar; - vector_uchar vchData; - - CBase58Data(); - void SetData(const std::vector<unsigned char> &vchVersionIn, const void* pdata, size_t nSize); - void SetData(const std::vector<unsigned char> &vchVersionIn, const unsigned char *pbegin, const unsigned char *pend); - -public: - bool SetString(const char* psz, unsigned int nVersionBytes = 1); - bool SetString(const std::string& str); - std::string ToString() const; - int CompareTo(const CBase58Data& b58) const; - - bool operator==(const CBase58Data& b58) const { return CompareTo(b58) == 0; } - bool operator<=(const CBase58Data& b58) const { return CompareTo(b58) <= 0; } - bool operator>=(const CBase58Data& b58) const { return CompareTo(b58) >= 0; } - bool operator< (const CBase58Data& b58) const { return CompareTo(b58) < 0; } - bool operator> (const CBase58Data& b58) const { return CompareTo(b58) > 0; } -}; - -/** - * A base58-encoded secret key - */ -class CBitcoinSecret : public CBase58Data -{ -public: - void SetKey(const CKey& vchSecret); - CKey GetKey(); - bool IsValid() const; - bool SetString(const char* pszSecret); - bool SetString(const std::string& strSecret); - - CBitcoinSecret(const CKey& vchSecret) { SetKey(vchSecret); } - CBitcoinSecret() {} -}; - -template<typename K, int Size, CChainParams::Base58Type Type> class CBitcoinExtKeyBase : public CBase58Data -{ -public: - void SetKey(const K &key) { - unsigned char vch[Size]; - key.Encode(vch); - SetData(Params().Base58Prefix(Type), vch, vch+Size); - } - - K GetKey() { - K ret; - if (vchData.size() == Size) { - // If base58 encoded data does not hold an ext key, return a !IsValid() key - ret.Decode(vchData.data()); - } - return ret; - } - - CBitcoinExtKeyBase(const K &key) { - SetKey(key); - } - - CBitcoinExtKeyBase(const std::string& strBase58c) { - SetString(strBase58c.c_str(), Params().Base58Prefix(Type).size()); - } - - CBitcoinExtKeyBase() {} -}; - -typedef CBitcoinExtKeyBase<CExtKey, BIP32_EXTKEY_SIZE, CChainParams::EXT_SECRET_KEY> CBitcoinExtKey; -typedef CBitcoinExtKeyBase<CExtPubKey, BIP32_EXTKEY_SIZE, CChainParams::EXT_PUBLIC_KEY> CBitcoinExtPubKey; - -std::string EncodeDestination(const CTxDestination& dest); -CTxDestination DecodeDestination(const std::string& str); -bool IsValidDestinationString(const std::string& str); -bool IsValidDestinationString(const std::string& str, const CChainParams& params); +bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet); #endif // BITCOIN_BASE58_H diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 6f438b60e9..98965840c7 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -32,7 +32,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<CO // (https://github.com/bitcoin/bitcoin/issues/7883#issuecomment-224807484) static void CoinSelection(benchmark::State& state) { - const CWallet wallet; + const CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); std::vector<COutput> vCoins; LOCK(wallet.cs_wallet); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index a60d3b3b6d..41f1e5786c 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -46,7 +46,7 @@ std::string HelpMessageCli() strUsage += HelpMessageOpt("-rpcport=<port>", strprintf(_("Connect to JSON-RPC on <port> (default: %u or testnet: %u)"), defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort())); strUsage += HelpMessageOpt("-rpcuser=<user>", _("Username for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcwait", _("Wait for RPC server to start")); - strUsage += HelpMessageOpt("-rpcwallet=<walletname>", _("Send RPC for non-default wallet on RPC server (argument is wallet filename in bitcoind directory, required if bitcoind/-Qt runs with multiple wallets)")); + strUsage += HelpMessageOpt("-rpcwallet=<walletname>", _("Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)")); strUsage += HelpMessageOpt("-stdin", _("Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.")); strUsage += HelpMessageOpt("-stdinrpcpass", strprintf(_("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password."))); @@ -339,8 +339,8 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // check if we should use a special wallet endpoint std::string endpoint = "/"; - std::string walletName = gArgs.GetArg("-rpcwallet", ""); - if (!walletName.empty()) { + if (!gArgs.GetArgs("-rpcwallet").empty()) { + std::string walletName = gArgs.GetArg("-rpcwallet", ""); char *encodedURI = evhttp_uriencode(walletName.c_str(), walletName.size(), false); if (encodedURI) { endpoint = "/wallet/"+ std::string(encodedURI); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index a9f7264f68..fcd836fb45 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -6,11 +6,11 @@ #include <config/bitcoin-config.h> #endif -#include <base58.h> #include <clientversion.h> #include <coins.h> #include <consensus/consensus.h> #include <core_io.h> +#include <key_io.h> #include <keystore.h> #include <policy/policy.h> #include <policy/rbf.h> @@ -563,12 +563,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) for (unsigned int kidx = 0; kidx < keysObj.size(); kidx++) { if (!keysObj[kidx].isStr()) throw std::runtime_error("privatekey not a std::string"); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(keysObj[kidx].getValStr()); - if (!fGood) + CKey key = DecodeSecret(keysObj[kidx].getValStr()); + if (!key.IsValid()) { throw std::runtime_error("privatekey not valid"); - - CKey key = vchSecret.GetKey(); + } tempKeystore.AddKey(key); } diff --git a/src/core_write.cpp b/src/core_write.cpp index ab6918e41d..91742b7d1b 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -4,9 +4,9 @@ #include <core_io.h> -#include <base58.h> #include <consensus/consensus.h> #include <consensus/validation.h> +#include <key_io.h> #include <script/script.h> #include <script/standard.h> #include <serialize.h> diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 4e1e403f69..6cac625abc 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -89,6 +89,7 @@ static leveldb::Options GetOptions(size_t nCacheSize) } CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bool fWipe, bool obfuscate) + : m_name(fs::basename(path)) { penv = nullptr; readoptions.verify_checksums = true; @@ -155,11 +156,30 @@ CDBWrapper::~CDBWrapper() bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync) { + const bool log_memory = LogAcceptCategory(BCLog::LEVELDB); + double mem_before = 0; + if (log_memory) { + mem_before = DynamicMemoryUsage() / 1024 / 1024; + } leveldb::Status status = pdb->Write(fSync ? syncoptions : writeoptions, &batch.batch); dbwrapper_private::HandleError(status); + if (log_memory) { + double mem_after = DynamicMemoryUsage() / 1024 / 1024; + LogPrint(BCLog::LEVELDB, "WriteBatch memory usage: db=%s, before=%.1fMiB, after=%.1fMiB\n", + m_name, mem_before, mem_after); + } return true; } +size_t CDBWrapper::DynamicMemoryUsage() const { + std::string memory; + if (!pdb->GetProperty("leveldb.approximate-memory-usage", &memory)) { + LogPrint(BCLog::LEVELDB, "Failed to get approximate-memory-usage property\n"); + return 0; + } + return stoul(memory); +} + // Prefixed with null character to avoid collisions with other keys // // We must use a string constructor which specifies length so that we copy diff --git a/src/dbwrapper.h b/src/dbwrapper.h index a29938ce33..6f80eedc7a 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -198,6 +198,9 @@ private: //! the database itself leveldb::DB* pdb; + //! the name of this database + std::string m_name; + //! a key used for optional XOR-obfuscation of the database std::vector<unsigned char> obfuscate_key; @@ -284,6 +287,9 @@ public: bool WriteBatch(CDBBatch& batch, bool fSync = false); + // Get an estimate of LevelDB memory usage (in bytes). + size_t DynamicMemoryUsage() const; + // not available for LevelDB; provide for compatibility with BDB bool Flush() { diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 5e9e419744..82ae733006 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -4,9 +4,9 @@ #include <httprpc.h> -#include <base58.h> #include <chainparams.h> #include <httpserver.h> +#include <key_io.h> #include <rpc/protocol.h> #include <rpc/server.h> #include <random.h> diff --git a/src/init.cpp b/src/init.cpp index f763c3a435..659f97fec6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -926,7 +926,7 @@ bool AppInitParameterInteraction() nMaxConnections = std::max(nUserMaxConnections, 0); // Trim requested connection counts, to fit into system limitations - nMaxConnections = std::max(std::min(nMaxConnections, (int)(FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS)), 0); + nMaxConnections = std::max(std::min(nMaxConnections, FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS), 0); nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS); if (nFD < MIN_CORE_FILEDESCRIPTORS) return InitError(_("Not enough file descriptors available.")); diff --git a/src/key.cpp b/src/key.cpp index ffed989be1..042e687772 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -170,7 +170,7 @@ CPrivKey CKey::GetPrivKey() const { size_t privkeylen; privkey.resize(PRIVATE_KEY_SIZE); privkeylen = PRIVATE_KEY_SIZE; - ret = ec_privkey_export_der(secp256k1_context_sign, (unsigned char*) privkey.data(), &privkeylen, begin(), fCompressed ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED); + ret = ec_privkey_export_der(secp256k1_context_sign, privkey.data(), &privkeylen, begin(), fCompressed ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED); assert(ret); privkey.resize(privkeylen); return privkey; @@ -199,7 +199,7 @@ bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, uint32_ secp256k1_ecdsa_signature sig; int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, test_case ? extra_entropy : nullptr); assert(ret); - secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, (unsigned char*)vchSig.data(), &nSigLen, &sig); + secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); return true; } @@ -226,7 +226,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) secp256k1_ecdsa_recoverable_signature sig; int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); - secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, (unsigned char*)&vchSig[1], &rec, &sig); + secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig); assert(ret); assert(rec != -1); vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); diff --git a/src/key_io.cpp b/src/key_io.cpp new file mode 100644 index 0000000000..c2dc511989 --- /dev/null +++ b/src/key_io.cpp @@ -0,0 +1,227 @@ +// Copyright (c) 2014-2016 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_io.h> + +#include <base58.h> +#include <bech32.h> +#include <script/script.h> +#include <utilstrencodings.h> + +#include <boost/variant/apply_visitor.hpp> +#include <boost/variant/static_visitor.hpp> + +#include <assert.h> +#include <string.h> +#include <algorithm> + +namespace +{ +class DestinationEncoder : public boost::static_visitor<std::string> +{ +private: + const CChainParams& m_params; + +public: + DestinationEncoder(const CChainParams& params) : m_params(params) {} + + std::string operator()(const CKeyID& id) const + { + std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::PUBKEY_ADDRESS); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } + + std::string operator()(const CScriptID& id) const + { + std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } + + std::string operator()(const WitnessV0KeyHash& id) const + { + std::vector<unsigned char> data = {0}; + data.reserve(33); + ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, id.begin(), id.end()); + return bech32::Encode(m_params.Bech32HRP(), data); + } + + std::string operator()(const WitnessV0ScriptHash& id) const + { + std::vector<unsigned char> data = {0}; + data.reserve(53); + ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, id.begin(), id.end()); + return bech32::Encode(m_params.Bech32HRP(), data); + } + + std::string operator()(const WitnessUnknown& id) const + { + if (id.version < 1 || id.version > 16 || id.length < 2 || id.length > 40) { + return {}; + } + std::vector<unsigned char> data = {(unsigned char)id.version}; + data.reserve(1 + (id.length * 8 + 4) / 5); + ConvertBits<8, 5, true>([&](unsigned char c) { data.push_back(c); }, id.program, id.program + id.length); + return bech32::Encode(m_params.Bech32HRP(), data); + } + + std::string operator()(const CNoDestination& no) const { return {}; } +}; + +CTxDestination DecodeDestination(const std::string& str, const CChainParams& params) +{ + std::vector<unsigned char> data; + uint160 hash; + if (DecodeBase58Check(str, data)) { + // base58-encoded Bitcoin addresses. + // Public-key-hash-addresses have version 0 (or 111 testnet). + // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. + const std::vector<unsigned char>& pubkey_prefix = params.Base58Prefix(CChainParams::PUBKEY_ADDRESS); + if (data.size() == hash.size() + pubkey_prefix.size() && std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin())) { + std::copy(data.begin() + pubkey_prefix.size(), data.end(), hash.begin()); + return CKeyID(hash); + } + // Script-hash-addresses have version 5 (or 196 testnet). + // The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script. + const std::vector<unsigned char>& script_prefix = params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); + if (data.size() == hash.size() + script_prefix.size() && std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) { + std::copy(data.begin() + script_prefix.size(), data.end(), hash.begin()); + return CScriptID(hash); + } + } + data.clear(); + auto bech = bech32::Decode(str); + if (bech.second.size() > 0 && bech.first == params.Bech32HRP()) { + // Bech32 decoding + int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16) + // The rest of the symbols are converted witness program bytes. + data.reserve(((bech.second.size() - 1) * 5) / 8); + if (ConvertBits<5, 8, false>([&](unsigned char c) { data.push_back(c); }, bech.second.begin() + 1, bech.second.end())) { + if (version == 0) { + { + WitnessV0KeyHash keyid; + if (data.size() == keyid.size()) { + std::copy(data.begin(), data.end(), keyid.begin()); + return keyid; + } + } + { + WitnessV0ScriptHash scriptid; + if (data.size() == scriptid.size()) { + std::copy(data.begin(), data.end(), scriptid.begin()); + return scriptid; + } + } + return CNoDestination(); + } + if (version > 16 || data.size() < 2 || data.size() > 40) { + return CNoDestination(); + } + WitnessUnknown unk; + unk.version = version; + std::copy(data.begin(), data.end(), unk.program); + unk.length = data.size(); + return unk; + } + } + return CNoDestination(); +} +} // namespace + +CKey DecodeSecret(const std::string& str) +{ + CKey key; + std::vector<unsigned char> data; + if (DecodeBase58Check(str, data)) { + const std::vector<unsigned char>& privkey_prefix = Params().Base58Prefix(CChainParams::SECRET_KEY); + if ((data.size() == 32 + privkey_prefix.size() || (data.size() == 33 + privkey_prefix.size() && data.back() == 1)) && + std::equal(privkey_prefix.begin(), privkey_prefix.end(), data.begin())) { + bool compressed = data.size() == 33 + privkey_prefix.size(); + key.Set(data.begin() + privkey_prefix.size(), data.begin() + privkey_prefix.size() + 32, compressed); + } + } + memory_cleanse(data.data(), data.size()); + return key; +} + +std::string EncodeSecret(const CKey& key) +{ + assert(key.IsValid()); + std::vector<unsigned char> data = Params().Base58Prefix(CChainParams::SECRET_KEY); + data.insert(data.end(), key.begin(), key.end()); + if (key.IsCompressed()) { + data.push_back(1); + } + std::string ret = EncodeBase58Check(data); + memory_cleanse(data.data(), data.size()); + return ret; +} + +CExtPubKey DecodeExtPubKey(const std::string& str) +{ + CExtPubKey key; + std::vector<unsigned char> data; + if (DecodeBase58Check(str, data)) { + const std::vector<unsigned char>& prefix = Params().Base58Prefix(CChainParams::EXT_PUBLIC_KEY); + if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { + key.Decode(data.data() + prefix.size()); + } + } + return key; +} + +std::string EncodeExtPubKey(const CExtPubKey& key) +{ + std::vector<unsigned char> data = Params().Base58Prefix(CChainParams::EXT_PUBLIC_KEY); + size_t size = data.size(); + data.resize(size + BIP32_EXTKEY_SIZE); + key.Encode(data.data() + size); + std::string ret = EncodeBase58Check(data); + return ret; +} + +CExtKey DecodeExtKey(const std::string& str) +{ + CExtKey key; + std::vector<unsigned char> data; + if (DecodeBase58Check(str, data)) { + const std::vector<unsigned char>& prefix = Params().Base58Prefix(CChainParams::EXT_SECRET_KEY); + if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { + key.Decode(data.data() + prefix.size()); + } + } + return key; +} + +std::string EncodeExtKey(const CExtKey& key) +{ + std::vector<unsigned char> data = Params().Base58Prefix(CChainParams::EXT_SECRET_KEY); + size_t size = data.size(); + data.resize(size + BIP32_EXTKEY_SIZE); + key.Encode(data.data() + size); + std::string ret = EncodeBase58Check(data); + memory_cleanse(data.data(), data.size()); + return ret; +} + +std::string EncodeDestination(const CTxDestination& dest) +{ + return boost::apply_visitor(DestinationEncoder(Params()), dest); +} + +CTxDestination DecodeDestination(const std::string& str) +{ + return DecodeDestination(str, Params()); +} + +bool IsValidDestinationString(const std::string& str, const CChainParams& params) +{ + return IsValidDestination(DecodeDestination(str, params)); +} + +bool IsValidDestinationString(const std::string& str) +{ + return IsValidDestinationString(str, Params()); +} diff --git a/src/key_io.h b/src/key_io.h new file mode 100644 index 0000000000..6fc9a8059a --- /dev/null +++ b/src/key_io.h @@ -0,0 +1,29 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2015 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_KEYIO_H +#define BITCOIN_KEYIO_H + +#include <chainparams.h> +#include <key.h> +#include <pubkey.h> +#include <script/standard.h> + +#include <string> + +CKey DecodeSecret(const std::string& str); +std::string EncodeSecret(const CKey& key); + +CExtKey DecodeExtKey(const std::string& str); +std::string EncodeExtKey(const CExtKey& extkey); +CExtPubKey DecodeExtPubKey(const std::string& str); +std::string EncodeExtPubKey(const CExtPubKey& extpubkey); + +std::string EncodeDestination(const CTxDestination& dest); +CTxDestination DecodeDestination(const std::string& str); +bool IsValidDestinationString(const std::string& str); +bool IsValidDestinationString(const std::string& str, const CChainParams& params); + +#endif // BITCOIN_KEYIO_H diff --git a/src/net.cpp b/src/net.cpp index c7c19f83a1..33a60ac96e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1631,7 +1631,8 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - if (LookupHost(host.c_str(), vIPs, 0, true)) + unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + if (LookupHost(host.c_str(), vIPs, nMaxIPs, true)) { for (const CNetAddr& ip : vIPs) { @@ -1828,11 +1829,18 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) } } + addrman.ResolveCollisions(); + int64_t nANow = GetAdjustedTime(); int nTries = 0; while (!interruptNet) { - CAddrInfo addr = addrman.Select(fFeeler); + CAddrInfo addr = addrman.SelectTriedCollision(); + + // SelectTriedCollision returns an invalid address if it is empty. + if (!fFeeler || !addr.IsValid()) { + addr = addrman.Select(fFeeler); + } // if we selected an invalid address, restart if (!addr.IsValid() || setConnected.count(addr.GetGroup()) || IsLocal(addr)) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ddf0dbbbea..482a206c8b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -401,6 +401,12 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) { } } +/** + * When a peer sends us a valid block, instruct it to announce blocks to us + * using CMPCTBLOCK if possible by adding its nodeid to the end of + * lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by + * removing the first element if necessary. + */ void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) { AssertLockHeld(cs_main); CNodeState* nodestate = State(nodeid); @@ -749,7 +755,11 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) return nEvicted; } -// Requires cs_main. +/** + * Mark a misbehaving peer to be banned depending upon the value of `-banscore`. + * + * Requires cs_main. + */ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) { if (howmuch == 0) @@ -808,6 +818,10 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, CScheduler &schedu scheduler.scheduleEvery(std::bind(&PeerLogicValidation::CheckForStaleTipAndEvictPeers, this, consensusParams), EXTRA_PEER_CHECK_INTERVAL * 1000); } +/** + * Evict orphan txn pool entries (EraseOrphanTx) based on a newly connected + * block. Also save the time of the last tip update. + */ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex, const std::vector<CTransactionRef>& vtxConflicted) { LOCK(g_cs_orphans); @@ -828,7 +842,7 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb } } - // Erase orphan transactions include or precluded by this block + // Erase orphan transactions included or precluded by this block if (vOrphanErase.size()) { int nErased = 0; for (uint256 &orphanHash : vOrphanErase) { @@ -847,6 +861,10 @@ static std::shared_ptr<const CBlockHeaderAndShortTxIDs> most_recent_compact_bloc static uint256 most_recent_block_hash; static bool fWitnessesPresentInMostRecentCompactBlock; +/** + * Maintain state about the best-seen block and fast-announce a compact block + * to compatible peers. + */ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) { std::shared_ptr<const CBlockHeaderAndShortTxIDs> pcmpctblock = std::make_shared<const CBlockHeaderAndShortTxIDs> (*pblock, true); const CNetMsgMaker msgMaker(PROTOCOL_VERSION); @@ -888,6 +906,10 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: }); } +/** + * Update our best height and announce any block hashes which weren't previously + * in chainActive to our peers. + */ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { const int nNewHeight = pindexNew->nHeight; connman->SetBestHeight(nNewHeight); @@ -920,6 +942,10 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB nTimeBestReceived = GetTime(); } +/** + * Handle invalid block rejection and consequent peer banning, maintain which + * peers announce compact blocks. + */ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) { LOCK(cs_main); diff --git a/src/net_processing.h b/src/net_processing.h index b534ef01c3..ff1ebc59da 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -42,13 +42,26 @@ private: public: explicit PeerLogicValidation(CConnman* connman, CScheduler &scheduler); + /** + * Overridden from CValidationInterface. + */ void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected, const std::vector<CTransactionRef>& vtxConflicted) override; + /** + * Overridden from CValidationInterface. + */ void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; + /** + * Overridden from CValidationInterface. + */ void BlockChecked(const CBlock& block, const CValidationState& state) override; + /** + * Overridden from CValidationInterface. + */ void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) override; - + /** Initialize a peer by adding it to mapNodeState and pushing a message requesting its version */ void InitializeNode(CNode* pnode) override; + /** Handle removal of a peer by updating various state and removing it from mapNodeState */ void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) override; /** Process protocol messages received from a given node */ bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override; @@ -61,8 +74,11 @@ public: */ bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override; + /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ void ConsiderEviction(CNode *pto, int64_t time_in_seconds); + /** Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound */ void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams); + /** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */ void EvictExtraOutboundPeers(int64_t time_in_seconds); private: diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index ffb5bff4de..4f9a79d654 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -7,10 +7,9 @@ #include <qt/guiutil.h> #include <qt/walletmodel.h> -#include <base58.h> +#include <key_io.h> #include <wallet/wallet.h> - #include <QFont> #include <QDebug> diff --git a/src/qt/bitcoinaddressvalidator.cpp b/src/qt/bitcoinaddressvalidator.cpp index 395ab447d2..6a76358a78 100644 --- a/src/qt/bitcoinaddressvalidator.cpp +++ b/src/qt/bitcoinaddressvalidator.cpp @@ -4,7 +4,7 @@ #include <qt/bitcoinaddressvalidator.h> -#include <base58.h> +#include <key_io.h> /* Base58 characters are: "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 4e868b7c17..427eb95a84 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -923,6 +923,7 @@ void BitcoinGUI::message(const QString &title, const QString &message, unsigned showNormalIfMinimized(); QMessageBox mBox(static_cast<QMessageBox::Icon>(nMBoxIcon), strTitle, message, buttons, this); + mBox.setTextFormat(Qt::PlainText); int r = mBox.exec(); if (ret != nullptr) *ret = r == QMessageBox::Ok; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 8d2e5619e0..b83755ab30 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -15,6 +15,7 @@ #include <wallet/coincontrol.h> #include <init.h> +#include <key_io.h> #include <policy/fees.h> #include <policy/policy.h> #include <validation.h> // For mempool diff --git a/src/qt/forms/modaloverlay.ui b/src/qt/forms/modaloverlay.ui index fdc52dc455..b5a69c578d 100644 --- a/src/qt/forms/modaloverlay.ui +++ b/src/qt/forms/modaloverlay.ui @@ -351,6 +351,12 @@ QLabel { color: rgb(40,40,40); }</string> <property name="text"> <string>Hide</string> </property> + <property name="focusPolicy"> + <enum>Qt::StrongFocus</enum> + </property> + <property name="default"> + <bool>true</bool> + </property> </widget> </item> </layout> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index a46e0561b9..7c3c68bfef 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -9,7 +9,10 @@ #include <qt/qvalidatedlineedit.h> #include <qt/walletmodel.h> +#include <base58.h> +#include <chainparams.h> #include <primitives/transaction.h> +#include <key_io.h> #include <init.h> #include <policy/policy.h> #include <protocol.h> diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp index b0ef475b35..357e98a53c 100644 --- a/src/qt/paymentrequestplus.cpp +++ b/src/qt/paymentrequestplus.cpp @@ -9,6 +9,7 @@ #include <qt/paymentrequestplus.h> +#include <script/script.h> #include <util.h> #include <stdexcept> diff --git a/src/qt/paymentrequestplus.h b/src/qt/paymentrequestplus.h index be3923304f..b1b60cf582 100644 --- a/src/qt/paymentrequestplus.h +++ b/src/qt/paymentrequestplus.h @@ -10,7 +10,8 @@ #include <qt/paymentrequest.pb.h> #pragma GCC diagnostic pop -#include <base58.h> +#include <amount.h> +#include <script/script.h> #include <openssl/x509.h> diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index bc69d4f945..8ad4fa31f1 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -8,9 +8,9 @@ #include <qt/guiutil.h> #include <qt/optionsmodel.h> -#include <base58.h> #include <chainparams.h> #include <policy/policy.h> +#include <key_io.h> #include <ui_interface.h> #include <util.h> #include <wallet/wallet.h> diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index ef36aab1a4..ec7edd48cd 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -14,8 +14,8 @@ #include <qt/platformstyle.h> #include <qt/sendcoinsentry.h> -#include <base58.h> #include <chainparams.h> +#include <key_io.h> #include <wallet/coincontrol.h> #include <validation.h> // mempool and minRelayTxFee #include <ui_interface.h> @@ -376,6 +376,12 @@ void SendCoinsDialog::on_sendButton_clicked() void SendCoinsDialog::clear() { + // Clear coin control settings + CoinControlDialog::coinControl()->UnSelectAll(); + ui->checkBoxCoinControlChange->setChecked(false); + ui->lineEditCoinControlChange->clear(); + coinControlUpdateLabels(); + // Remove entries until only one left while(ui->entries->count()) { diff --git a/src/qt/signverifymessagedialog.cpp b/src/qt/signverifymessagedialog.cpp index 364dcd6f45..8dade8df79 100644 --- a/src/qt/signverifymessagedialog.cpp +++ b/src/qt/signverifymessagedialog.cpp @@ -10,8 +10,8 @@ #include <qt/platformstyle.h> #include <qt/walletmodel.h> -#include <base58.h> #include <init.h> +#include <key_io.h> #include <validation.h> // For strMessageMagic #include <wallet/wallet.h> diff --git a/src/qt/test/paymentservertests.cpp b/src/qt/test/paymentservertests.cpp index 6e80625123..29ef4b4c9e 100644 --- a/src/qt/test/paymentservertests.cpp +++ b/src/qt/test/paymentservertests.cpp @@ -8,6 +8,7 @@ #include <qt/test/paymentrequestdata.h> #include <amount.h> +#include <chainparams.h> #include <random.h> #include <script/script.h> #include <script/standard.h> diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index cd49292138..976aadc0af 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -10,6 +10,7 @@ #include <qt/transactiontablemodel.h> #include <qt/transactionview.h> #include <qt/walletmodel.h> +#include <key_io.h> #include <test/test_bitcoin.h> #include <validation.h> #include <wallet/wallet.h> @@ -157,9 +158,7 @@ void TestGUI() for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - bitdb.MakeMock(); - std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - CWallet wallet(std::move(dbw)); + CWallet wallet("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet.LoadWallet(firstRun); { @@ -260,9 +259,6 @@ void TestGUI() QPushButton* removeRequestButton = receiveCoinsDialog.findChild<QPushButton*>("removeRequestButton"); removeRequestButton->click(); QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1); - - bitdb.Flush(true); - bitdb.Reset(); } } diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 6f30e56327..ec5a66bc9f 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -9,8 +9,8 @@ #include <qt/paymentserver.h> #include <qt/transactionrecord.h> -#include <base58.h> #include <consensus/consensus.h> +#include <key_io.h> #include <validation.h> #include <script/script.h> #include <timedata.h> @@ -240,7 +240,7 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco if (wtx.mapValue.count("comment") && !wtx.mapValue["comment"].empty()) strHTML += "<br><b>" + tr("Comment") + ":</b><br>" + GUIUtil::HtmlEscape(wtx.mapValue["comment"], true) + "<br>"; - strHTML += "<b>" + tr("Transaction ID") + ":</b> " + rec->getTxID() + "<br>"; + strHTML += "<b>" + tr("Transaction ID") + ":</b> " + rec->getTxHash() + "<br>"; strHTML += "<b>" + tr("Transaction total size") + ":</b> " + QString::number(wtx.tx->GetTotalSize()) + " bytes<br>"; strHTML += "<b>" + tr("Transaction virtual size") + ":</b> " + QString::number(GetVirtualTransactionSize(*wtx.tx)) + " bytes<br>"; strHTML += "<b>" + tr("Output index") + ":</b> " + QString::number(rec->getOutputIndex()) + "<br>"; diff --git a/src/qt/transactiondescdialog.cpp b/src/qt/transactiondescdialog.cpp index 161fccd462..7bf4d3351c 100644 --- a/src/qt/transactiondescdialog.cpp +++ b/src/qt/transactiondescdialog.cpp @@ -14,7 +14,7 @@ TransactionDescDialog::TransactionDescDialog(const QModelIndex &idx, QWidget *pa ui(new Ui::TransactionDescDialog) { ui->setupUi(this); - setWindowTitle(tr("Details for %1").arg(idx.data(TransactionTableModel::TxIDRole).toString())); + setWindowTitle(tr("Details for %1").arg(idx.data(TransactionTableModel::TxHashRole).toString())); QString desc = idx.data(TransactionTableModel::LongDescriptionRole).toString(); ui->detailText->setHtml(desc); } diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index 39d03fa547..a702461f7a 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -36,7 +36,7 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool(); QString address = index.data(TransactionTableModel::AddressRole).toString(); QString label = index.data(TransactionTableModel::LabelRole).toString(); - QString txid = index.data(TransactionTableModel::TxIDRole).toString(); + QString txid = index.data(TransactionTableModel::TxHashRole).toString(); qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); int status = index.data(TransactionTableModel::StatusRole).toInt(); diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index de3e885e8f..19cdb0fdea 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -4,8 +4,8 @@ #include <qt/transactionrecord.h> -#include <base58.h> #include <consensus/consensus.h> +#include <key_io.h> #include <validation.h> #include <timedata.h> #include <wallet/wallet.h> @@ -254,7 +254,7 @@ bool TransactionRecord::statusUpdateNeeded() const return status.cur_num_blocks != chainActive.Height() || status.needsUpdate; } -QString TransactionRecord::getTxID() const +QString TransactionRecord::getTxHash() const { return QString::fromStdString(hash.ToString()); } diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 29a3cd8de7..5321d05d15 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -129,7 +129,7 @@ public: bool involvesWatchAddress; /** Return the unique identifier for this transaction (part) */ - QString getTxID() const; + QString getTxHash() const; /** Return the output index of the subtransaction */ int getOutputIndex() const; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 626d4c0bdc..84800125fe 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -615,10 +615,8 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const return walletModel->getAddressTableModel()->labelForAddress(QString::fromStdString(rec->address)); case AmountRole: return qint64(rec->credit + rec->debit); - case TxIDRole: - return rec->getTxID(); case TxHashRole: - return QString::fromStdString(rec->hash.ToString()); + return rec->getTxHash(); case TxHexRole: return priv->getTxHex(rec); case TxPlainTextRole: diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index 8f58962d17..781874d160 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -56,8 +56,6 @@ public: LabelRole, /** Net amount of transaction */ AmountRole, - /** Unique identifier */ - TxIDRole, /** Transaction hash */ TxHashRole, /** Transaction data, hex-encoded */ diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 006f9fe443..26391452da 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -371,7 +371,7 @@ void TransactionView::exportClicked() writer.addColumn(tr("Label"), 0, TransactionTableModel::LabelRole); writer.addColumn(tr("Address"), 0, TransactionTableModel::AddressRole); writer.addColumn(BitcoinUnits::getAmountColumnTitle(model->getOptionsModel()->getDisplayUnit()), 0, TransactionTableModel::FormattedAmountRole); - writer.addColumn(tr("ID"), 0, TransactionTableModel::TxIDRole); + writer.addColumn(tr("ID"), 0, TransactionTableModel::TxHashRole); if(!writer.write()) { Q_EMIT message(tr("Exporting Failed"), tr("There was an error trying to save the transaction history to %1.").arg(filename), @@ -455,7 +455,7 @@ void TransactionView::copyAmount() void TransactionView::copyTxID() { - GUIUtil::copyEntryData(transactionView, 0, TransactionTableModel::TxIDRole); + GUIUtil::copyEntryData(transactionView, 0, TransactionTableModel::TxHashRole); } void TransactionView::copyTxHex() diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 34954a6bfa..e7d9d276d7 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -14,8 +14,8 @@ #include <qt/sendcoinsdialog.h> #include <qt/transactiontablemodel.h> -#include <base58.h> #include <chain.h> +#include <key_io.h> #include <keystore.h> #include <validation.h> #include <net.h> // for g_connman diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 9e13de79be..811996b98f 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -5,6 +5,11 @@ #ifndef BITCOIN_QT_WALLETMODEL_H #define BITCOIN_QT_WALLETMODEL_H +#include <amount.h> +#include <key.h> +#include <serialize.h> +#include <script/standard.h> + #include <qt/paymentrequestplus.h> #include <qt/walletmodeltransaction.h> diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8007cebc37..f2a1fd048f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -372,6 +372,9 @@ std::string EntryDescriptionString() " \"wtxid\" : hash, (string) hash of serialized transaction, including witness data\n" " \"depends\" : [ (array) unconfirmed transactions used as inputs for this transaction\n" " \"transactionid\", (string) parent transaction id\n" + " ... ]\n" + " \"spentby\" : [ (array) unconfirmed transactions spending outputs from this transaction\n" + " \"transactionid\", (string) child transaction id\n" " ... ]\n"; } @@ -406,6 +409,15 @@ void entryToJSON(UniValue &info, const CTxMemPoolEntry &e) } info.pushKV("depends", depends); + + 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) { + spent.push_back(childiter->GetTx().GetHash().ToString()); + } + + info.pushKV("spentby", spent); } UniValue mempoolToJSON(bool fVerbose) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index a95ea0cf92..0eeb3f98b3 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -43,6 +43,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listreceivedbyaddress", 0, "minconf" }, { "listreceivedbyaddress", 1, "include_empty" }, { "listreceivedbyaddress", 2, "include_watchonly" }, + { "listreceivedbyaddress", 3, "address_filter" }, { "listreceivedbyaccount", 0, "minconf" }, { "listreceivedbyaccount", 1, "include_empty" }, { "listreceivedbyaccount", 2, "include_watchonly" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3f3bfa0cfd..3073a49d0d 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> #include <amount.h> #include <chain.h> #include <chainparams.h> @@ -13,6 +12,7 @@ #include <core_io.h> #include <init.h> #include <validation.h> +#include <key_io.h> #include <miner.h> #include <net.h> #include <policy/fees.h> diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f573c7dbeb..49e865a64a 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -3,12 +3,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> #include <chain.h> #include <clientversion.h> #include <core_io.h> #include <crypto/ripemd160.h> #include <init.h> +#include <key_io.h> #include <validation.h> #include <httpserver.h> #include <net.h> @@ -224,13 +224,10 @@ UniValue signmessagewithprivkey(const JSONRPCRequest& request) std::string strPrivkey = request.params[0].get_str(); std::string strMessage = request.params[1].get_str(); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strPrivkey); - if (!fGood) + CKey key = DecodeSecret(strPrivkey); + if (!key.IsValid()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); - CKey key = vchSecret.GetKey(); - if (!key.IsValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + } CHashWriter ss(SER_GETHASH, 0); ss << strMessageMagic; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 813afde4db..8dcfb48e9a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> #include <chain.h> #include <coins.h> #include <consensus/validation.h> @@ -12,6 +11,7 @@ #include <keystore.h> #include <validation.h> #include <validationinterface.h> +#include <key_io.h> #include <merkleblock.h> #include <net.h> #include <policy/policy.h> @@ -896,13 +896,9 @@ UniValue signrawtransactionwithkey(const JSONRPCRequest& request) const UniValue& keys = request.params[1].get_array(); for (unsigned int idx = 0; idx < keys.size(); ++idx) { UniValue k = keys[idx]; - CBitcoinSecret vchSecret; - if (!vchSecret.SetString(k.get_str())) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); - } - CKey key = vchSecret.GetKey(); + CKey key = DecodeSecret(k.get_str()); if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); } keystore.AddKey(key); } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index e5b4f6ca77..35401bf876 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -5,9 +5,9 @@ #include <rpc/server.h> -#include <base58.h> #include <fs.h> #include <init.h> +#include <key_io.h> #include <random.h> #include <sync.h> #include <ui_interface.h> diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index cdcb68d15f..593962e710 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> +#include <key_io.h> #include <keystore.h> #include <pubkey.h> #include <rpc/protocol.h> diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 2cdff7ee57..927b0267ca 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -226,23 +226,25 @@ bool static CheckPubKeyEncoding(const valtype &vchPubKey, unsigned int flags, co } bool static CheckMinimalPush(const valtype& data, opcodetype opcode) { + // Excludes OP_1NEGATE, OP_1-16 since they are by definition minimal + assert(0 <= opcode && opcode <= OP_PUSHDATA4); if (data.size() == 0) { - // Could have used OP_0. + // Should have used OP_0. return opcode == OP_0; } else if (data.size() == 1 && data[0] >= 1 && data[0] <= 16) { - // Could have used OP_1 .. OP_16. - return opcode == OP_1 + (data[0] - 1); + // Should have used OP_1 .. OP_16. + return false; } else if (data.size() == 1 && data[0] == 0x81) { - // Could have used OP_1NEGATE. - return opcode == OP_1NEGATE; + // Should have used OP_1NEGATE. + return false; } else if (data.size() <= 75) { - // Could have used a direct push (opcode indicating number of bytes pushed + those bytes). + // Must have used a direct push (opcode indicating number of bytes pushed + those bytes). return opcode == data.size(); } else if (data.size() <= 255) { - // Could have used OP_PUSHDATA. + // Must have used OP_PUSHDATA. return opcode == OP_PUSHDATA1; } else if (data.size() <= 65535) { - // Could have used OP_PUSHDATA2. + // Must have used OP_PUSHDATA2. return opcode == OP_PUSHDATA2; } return true; diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index b338d6d366..0d8bd90119 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -52,6 +52,17 @@ public: { CAddrMan::Delete(nId); } + + // Simulates connection failure so that we can test eviction of offline nodes + void SimConnFail(CService& addr) + { + int64_t nLastSuccess = 1; + Good_(addr, true, nLastSuccess); // Set last good connection in the deep past. + + bool count_failure = false; + int64_t nLastTry = GetAdjustedTime()-61; + Attempt(addr, count_failure, nLastTry); + } }; static CNetAddr ResolveIP(const char* ip) @@ -226,7 +237,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) BOOST_CHECK_EQUAL(addrman.size(), 0); for (unsigned int i = 1; i < 18; i++) { - CService addr = ResolveService("250.1.1." + boost::to_string(i)); + CService addr = ResolveService("250.1.1." + std::to_string(i)); addrman.Add(CAddress(addr, NODE_NONE), source); //Test: No collision in new table yet. @@ -252,7 +263,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) BOOST_CHECK_EQUAL(addrman.size(), 0); for (unsigned int i = 1; i < 80; i++) { - CService addr = ResolveService("250.1.1." + boost::to_string(i)); + CService addr = ResolveService("250.1.1." + std::to_string(i)); addrman.Add(CAddress(addr, NODE_NONE), source); addrman.Good(CAddress(addr, NODE_NONE)); @@ -385,7 +396,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) for (unsigned int i = 1; i < (8 * 256); i++) { int octet1 = i % 256; int octet2 = i >> 8 % 256; - std::string strAddr = boost::to_string(octet1) + "." + boost::to_string(octet2) + ".1.23"; + std::string strAddr = std::to_string(octet1) + "." + std::to_string(octet2) + ".1.23"; CAddress addr = CAddress(ResolveService(strAddr), NODE_NONE); // Ensure that for all addrs in addrman, isTerrible == false. @@ -436,8 +447,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) std::set<int> buckets; for (int i = 0; i < 255; i++) { CAddrInfo infoi = CAddrInfo( - CAddress(ResolveService("250.1.1." + boost::to_string(i)), NODE_NONE), - ResolveIP("250.1.1." + boost::to_string(i))); + CAddress(ResolveService("250.1.1." + std::to_string(i)), NODE_NONE), + ResolveIP("250.1.1." + std::to_string(i))); int bucket = infoi.GetTriedBucket(nKey1); buckets.insert(bucket); } @@ -448,8 +459,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) buckets.clear(); for (int j = 0; j < 255; j++) { CAddrInfo infoj = CAddrInfo( - CAddress(ResolveService("250." + boost::to_string(j) + ".1.1"), NODE_NONE), - ResolveIP("250." + boost::to_string(j) + ".1.1")); + CAddress(ResolveService("250." + std::to_string(j) + ".1.1"), NODE_NONE), + ResolveIP("250." + std::to_string(j) + ".1.1")); int bucket = infoj.GetTriedBucket(nKey1); buckets.insert(bucket); } @@ -488,8 +499,8 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) std::set<int> buckets; for (int i = 0; i < 255; i++) { CAddrInfo infoi = CAddrInfo( - CAddress(ResolveService("250.1.1." + boost::to_string(i)), NODE_NONE), - ResolveIP("250.1.1." + boost::to_string(i))); + CAddress(ResolveService("250.1.1." + std::to_string(i)), NODE_NONE), + ResolveIP("250.1.1." + std::to_string(i))); int bucket = infoi.GetNewBucket(nKey1); buckets.insert(bucket); } @@ -501,7 +512,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) for (int j = 0; j < 4 * 255; j++) { CAddrInfo infoj = CAddrInfo(CAddress( ResolveService( - boost::to_string(250 + (j / 255)) + "." + boost::to_string(j % 256) + ".1.1"), NODE_NONE), + std::to_string(250 + (j / 255)) + "." + std::to_string(j % 256) + ".1.1"), NODE_NONE), ResolveIP("251.4.1.1")); int bucket = infoj.GetNewBucket(nKey1); buckets.insert(bucket); @@ -514,7 +525,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) for (int p = 0; p < 255; p++) { CAddrInfo infoj = CAddrInfo( CAddress(ResolveService("250.1.1.1"), NODE_NONE), - ResolveIP("250." + boost::to_string(p) + ".1.1")); + ResolveIP("250." + std::to_string(p) + ".1.1")); int bucket = infoj.GetNewBucket(nKey1); buckets.insert(bucket); } @@ -522,4 +533,158 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) // than 64 buckets. BOOST_CHECK(buckets.size() > 64); } + + +BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + BOOST_CHECK(addrman.size() == 0); + + // Empty addrman should return blank addrman info. + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collisions yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Ensure Good handles duplicates well. + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == 22); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + +} + +BOOST_AUTO_TEST_CASE(addrman_noevict) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collision yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Collision between 23 and 19. + CService addr23 = ResolveService("250.1.1.23"); + addrman.Add(CAddress(addr23, NODE_NONE), source); + addrman.Good(addr23); + + BOOST_CHECK(addrman.size() == 23); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.19:0"); + + // 23 should be discarded and 19 not evicted. + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Lets create two collisions. + for (unsigned int i = 24; i < 33; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Cause a collision. + CService addr33 = ResolveService("250.1.1.33"); + addrman.Add(CAddress(addr33, NODE_NONE), source); + addrman.Good(addr33); + BOOST_CHECK(addrman.size() == 33); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.27:0"); + + // Cause a second collision. + addrman.Add(CAddress(addr23, NODE_NONE), source); + addrman.Good(addr23); + BOOST_CHECK(addrman.size() == 33); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() != "[::]:0"); + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); +} + +BOOST_AUTO_TEST_CASE(addrman_evictionworks) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + BOOST_CHECK(addrman.size() == 0); + + // Empty addrman should return blank addrman info. + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // Add twenty two addresses. + CNetAddr source = ResolveIP("252.2.2.2"); + for (unsigned int i = 1; i < 23; i++) { + CService addr = ResolveService("250.1.1."+std::to_string(i)); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + // No collision yet. + BOOST_CHECK(addrman.size() == i); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + } + + // Collision between 23 and 19. + CService addr = ResolveService("250.1.1.23"); + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.size() == 23); + CAddrInfo info = addrman.SelectTriedCollision(); + BOOST_CHECK(info.ToString() == "250.1.1.19:0"); + + // Ensure test of address fails, so that it is evicted. + addrman.SimConnFail(info); + + // Should swap 23 for 19. + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // If 23 was swapped for 19, then this should cause no collisions. + addrman.Add(CAddress(addr, NODE_NONE), source); + addrman.Good(addr); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); + + // If we insert 19 is should collide with 23. + CService addr19 = ResolveService("250.1.1.19"); + addrman.Add(CAddress(addr19, NODE_NONE), source); + addrman.Good(addr19); + + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.23:0"); + + addrman.ResolveCollisions(); + BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); +} + + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index b9ac62a437..1210c7a7ee 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -16,9 +16,9 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++) { std::string strEnc = EncodeBase32(vstrIn[i]); - BOOST_CHECK(strEnc == vstrOut[i]); + BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); std::string strDec = DecodeBase32(vstrOut[i]); - BOOST_CHECK(strDec == vstrIn[i]); + BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } } diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index a2d4f82695..f90d4f90cb 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -2,17 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> - #include <test/data/base58_encode_decode.json.h> -#include <test/data/base58_keys_invalid.json.h> -#include <test/data/base58_keys_valid.json.h> -#include <key.h> -#include <script/script.h> +#include <base58.h> #include <test/test_bitcoin.h> -#include <uint256.h> -#include <util.h> #include <utilstrencodings.h> #include <univalue.h> @@ -73,135 +66,4 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); } -// Goal: check that parsed keys match test payload -BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) -{ - UniValue tests = read_json(std::string(json_tests::base58_keys_valid, json_tests::base58_keys_valid + sizeof(json_tests::base58_keys_valid))); - CBitcoinSecret secret; - CTxDestination destination; - SelectParams(CBaseChainParams::MAIN); - - for (unsigned int idx = 0; idx < tests.size(); idx++) { - UniValue test = tests[idx]; - std::string strTest = test.write(); - if (test.size() < 3) { // Allow for extra stuff (useful for comments) - BOOST_ERROR("Bad test: " << strTest); - continue; - } - std::string exp_base58string = test[0].get_str(); - std::vector<unsigned char> exp_payload = ParseHex(test[1].get_str()); - const UniValue &metadata = test[2].get_obj(); - bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); - SelectParams(find_value(metadata, "chain").get_str()); - bool try_case_flip = find_value(metadata, "tryCaseFlip").isNull() ? false : find_value(metadata, "tryCaseFlip").get_bool(); - if (isPrivkey) { - bool isCompressed = find_value(metadata, "isCompressed").get_bool(); - // Must be valid private key - BOOST_CHECK_MESSAGE(secret.SetString(exp_base58string), "!SetString:"+ strTest); - BOOST_CHECK_MESSAGE(secret.IsValid(), "!IsValid:" + strTest); - CKey privkey = secret.GetKey(); - BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest); - BOOST_CHECK_MESSAGE(privkey.size() == exp_payload.size() && std::equal(privkey.begin(), privkey.end(), exp_payload.begin()), "key mismatch:" + strTest); - - // Private key must be invalid public key - destination = DecodeDestination(exp_base58string); - BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid privkey as pubkey:" + strTest); - } else { - // Must be valid public key - destination = DecodeDestination(exp_base58string); - CScript script = GetScriptForDestination(destination); - BOOST_CHECK_MESSAGE(IsValidDestination(destination), "!IsValid:" + strTest); - BOOST_CHECK_EQUAL(HexStr(script), HexStr(exp_payload)); - - // Try flipped case version - for (char& c : exp_base58string) { - if (c >= 'a' && c <= 'z') { - c = (c - 'a') + 'A'; - } else if (c >= 'A' && c <= 'Z') { - c = (c - 'A') + 'a'; - } - } - destination = DecodeDestination(exp_base58string); - BOOST_CHECK_MESSAGE(IsValidDestination(destination) == try_case_flip, "!IsValid case flipped:" + strTest); - if (IsValidDestination(destination)) { - script = GetScriptForDestination(destination); - BOOST_CHECK_EQUAL(HexStr(script), HexStr(exp_payload)); - } - - // Public key must be invalid private key - secret.SetString(exp_base58string); - BOOST_CHECK_MESSAGE(!secret.IsValid(), "IsValid pubkey as privkey:" + strTest); - } - } -} - -// Goal: check that generated keys match test vectors -BOOST_AUTO_TEST_CASE(base58_keys_valid_gen) -{ - UniValue tests = read_json(std::string(json_tests::base58_keys_valid, json_tests::base58_keys_valid + sizeof(json_tests::base58_keys_valid))); - - for (unsigned int idx = 0; idx < tests.size(); idx++) { - UniValue test = tests[idx]; - std::string strTest = test.write(); - if (test.size() < 3) // Allow for extra stuff (useful for comments) - { - BOOST_ERROR("Bad test: " << strTest); - continue; - } - std::string exp_base58string = test[0].get_str(); - std::vector<unsigned char> exp_payload = ParseHex(test[1].get_str()); - const UniValue &metadata = test[2].get_obj(); - bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); - SelectParams(find_value(metadata, "chain").get_str()); - if (isPrivkey) { - bool isCompressed = find_value(metadata, "isCompressed").get_bool(); - CKey key; - key.Set(exp_payload.begin(), exp_payload.end(), isCompressed); - assert(key.IsValid()); - CBitcoinSecret secret; - secret.SetKey(key); - BOOST_CHECK_MESSAGE(secret.ToString() == exp_base58string, "result mismatch: " + strTest); - } else { - CTxDestination dest; - CScript exp_script(exp_payload.begin(), exp_payload.end()); - ExtractDestination(exp_script, dest); - std::string address = EncodeDestination(dest); - - BOOST_CHECK_EQUAL(address, exp_base58string); - } - } - - SelectParams(CBaseChainParams::MAIN); -} - - -// Goal: check that base58 parsing code is robust against a variety of corrupted data -BOOST_AUTO_TEST_CASE(base58_keys_invalid) -{ - UniValue tests = read_json(std::string(json_tests::base58_keys_invalid, json_tests::base58_keys_invalid + sizeof(json_tests::base58_keys_invalid))); // Negative testcases - CBitcoinSecret secret; - CTxDestination destination; - - for (unsigned int idx = 0; idx < tests.size(); idx++) { - UniValue test = tests[idx]; - std::string strTest = test.write(); - if (test.size() < 1) // Allow for extra stuff (useful for comments) - { - BOOST_ERROR("Bad test: " << strTest); - continue; - } - 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 }) { - SelectParams(chain); - destination = DecodeDestination(exp_base58string); - BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid pubkey in mainnet:" + strTest); - secret.SetString(exp_base58string); - BOOST_CHECK_MESSAGE(!secret.IsValid(), "IsValid privkey in mainnet:" + strTest); - } - } -} - - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index b31f73f3b5..f785cede81 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -16,9 +16,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) for (unsigned int i=0; i<sizeof(vstrIn)/sizeof(vstrIn[0]); i++) { std::string strEnc = EncodeBase64(vstrIn[i]); - BOOST_CHECK(strEnc == vstrOut[i]); + BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); std::string strDec = DecodeBase64(strEnc); - BOOST_CHECK(strDec == vstrIn[i]); + BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } } diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 438ddc177d..3c9ff1877d 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -4,8 +4,8 @@ #include <boost/test/unit_test.hpp> -#include <base58.h> #include <key.h> +#include <key_io.h> #include <uint256.h> #include <util.h> #include <utilstrencodings.h> @@ -99,20 +99,12 @@ void RunTest(const TestVector &test) { pubkey.Encode(data); // Test private key - CBitcoinExtKey b58key; b58key.SetKey(key); - BOOST_CHECK(b58key.ToString() == derive.prv); - - CBitcoinExtKey b58keyDecodeCheck(derive.prv); - CExtKey checkKey = b58keyDecodeCheck.GetKey(); - assert(checkKey == key); //ensure a base58 decoded key also matches + BOOST_CHECK(EncodeExtKey(key) == derive.prv); + BOOST_CHECK(DecodeExtKey(derive.prv) == key); //ensure a base58 decoded key also matches // Test public key - CBitcoinExtPubKey b58pubkey; b58pubkey.SetKey(pubkey); - BOOST_CHECK(b58pubkey.ToString() == derive.pub); - - CBitcoinExtPubKey b58PubkeyDecodeCheck(derive.pub); - CExtPubKey checkPubKey = b58PubkeyDecodeCheck.GetKey(); - assert(checkPubKey == pubkey); //ensure a base58 decoded pubkey also matches + BOOST_CHECK(EncodeExtPubKey(pubkey) == derive.pub); + BOOST_CHECK(DecodeExtPubKey(derive.pub) == pubkey); //ensure a base58 decoded pubkey also matches // Derive new keys CExtKey keyNew; diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index af5533b109..73c8eb5168 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -4,9 +4,9 @@ #include <bloom.h> -#include <base58.h> #include <clientversion.h> #include <key.h> +#include <key_io.h> #include <merkleblock.h> #include <primitives/block.h> #include <random.h> @@ -85,10 +85,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) BOOST_AUTO_TEST_CASE(bloom_create_insert_key) { std::string strSecret = std::string("5Kg1gnAjaLfKiwhhPpGS3QfRg2m6awQvaj98JCZBZQ5SuS2F15C"); - CBitcoinSecret vchSecret; - BOOST_CHECK(vchSecret.SetString(strSecret)); - - CKey key = vchSecret.GetKey(); + CKey key = DecodeSecret(strSecret); CPubKey pubkey = key.GetPubKey(); std::vector<unsigned char> vchPubKey(pubkey.begin(), pubkey.end()); diff --git a/src/test/data/base58_keys_invalid.json b/src/test/data/key_io_invalid.json index 2056c7491c..2056c7491c 100644 --- a/src/test/data/base58_keys_invalid.json +++ b/src/test/data/key_io_invalid.json diff --git a/src/test/data/base58_keys_valid.json b/src/test/data/key_io_valid.json index 8418a6002d..8418a6002d 100644 --- a/src/test/data/base58_keys_valid.json +++ b/src/test/data/key_io_valid.json diff --git a/src/test/data/tx_valid.json b/src/test/data/tx_valid.json index 7e39ec7599..0bcecc58fe 100644 --- a/src/test/data/tx_valid.json +++ b/src/test/data/tx_valid.json @@ -516,5 +516,9 @@ [[["9628667ad48219a169b41b020800162287d2c0f713c04157e95c484a8dcb7592", 7500, "0x00 0x20 0x9b66c15b4e0b4eb49fa877982cafded24859fe5b0e2dbfbe4f0df1de7743fd52", 200000]], "010000000001019275cb8d4a485ce95741c013f7c0d28722160008021bb469a11982d47a6628964c1d000000ffffffff0101000000000000000007004830450220487fb382c4974de3f7d834c1b617fe15860828c7f96454490edd6d891556dcc9022100baf95feb48f845d5bfc9882eb6aeefa1bc3790e39f59eaa46ff7f15ae626c53e0148304502205286f726690b2e9b0207f0345711e63fa7012045b9eb0f19c2458ce1db90cf43022100e89f17f86abc5b149eba4115d4f128bcf45d77fb3ecdd34f594091340c0395960101022102966f109c54e85d3aee8321301136cedeb9fc710fdef58a9de8a73942f8e567c021034ffc99dd9a79dd3cb31e2ab3e0b09e0e67db41ac068c625cd1f491576016c84e9552af4830450220487fb382c4974de3f7d834c1b617fe15860828c7f96454490edd6d891556dcc9022100baf95feb48f845d5bfc9882eb6aeefa1bc3790e39f59eaa46ff7f15ae626c53e0148304502205286f726690b2e9b0207f0345711e63fa7012045b9eb0f19c2458ce1db90cf43022100e89f17f86abc5b149eba4115d4f128bcf45d77fb3ecdd34f594091340c039596017500000000", "P2SH,WITNESS"], +["Test long outputs, which are streamed using length-prefixed bitcoin strings. This might be surprising."], +[[["1111111111111111111111111111111111111111111111111111111111111111", 0, "0x00 0x14 0x751e76e8199196d454941c45d1b3a323f1433bd6", 5000000]], +"0100000000010111111111111111111111111111111111111111111111111111111111111111110000000000ffffffff0130244c0000000000fd02014cdc1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111175210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ac02483045022100c1a4a6581996a7fdfea77d58d537955a5655c1d619b6f3ab6874f28bb2e19708022056402db6fede03caae045a3be616a1a2d0919a475ed4be828dc9ff21f24063aa01210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179800000000", "P2SH,WITNESS"], + ["Make diffs cleaner by leaving a comment here without comma at the end"] ] diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp new file mode 100644 index 0000000000..1ac1e0015b --- /dev/null +++ b/src/test/key_io_tests.cpp @@ -0,0 +1,149 @@ +// Copyright (c) 2011-2016 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/data/key_io_invalid.json.h> +#include <test/data/key_io_valid.json.h> + +#include <key.h> +#include <key_io.h> +#include <script/script.h> +#include <utilstrencodings.h> +#include <test/test_bitcoin.h> + +#include <boost/test/unit_test.hpp> + +#include <univalue.h> + +extern UniValue read_json(const std::string& jsondata); + +BOOST_FIXTURE_TEST_SUITE(key_io_tests, BasicTestingSetup) + +// Goal: check that parsed keys match test payload +BOOST_AUTO_TEST_CASE(key_io_valid_parse) +{ + UniValue tests = read_json(std::string(json_tests::key_io_valid, json_tests::key_io_valid + sizeof(json_tests::key_io_valid))); + CKey privkey; + CTxDestination destination; + SelectParams(CBaseChainParams::MAIN); + + for (unsigned int idx = 0; idx < tests.size(); idx++) { + UniValue test = tests[idx]; + std::string strTest = test.write(); + if (test.size() < 3) { // Allow for extra stuff (useful for comments) + BOOST_ERROR("Bad test: " << strTest); + continue; + } + std::string exp_base58string = test[0].get_str(); + std::vector<unsigned char> exp_payload = ParseHex(test[1].get_str()); + const UniValue &metadata = test[2].get_obj(); + bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); + SelectParams(find_value(metadata, "chain").get_str()); + bool try_case_flip = find_value(metadata, "tryCaseFlip").isNull() ? false : find_value(metadata, "tryCaseFlip").get_bool(); + if (isPrivkey) { + bool isCompressed = find_value(metadata, "isCompressed").get_bool(); + // Must be valid private key + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest); + BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest); + BOOST_CHECK_MESSAGE(privkey.size() == exp_payload.size() && std::equal(privkey.begin(), privkey.end(), exp_payload.begin()), "key mismatch:" + strTest); + + // Private key must be invalid public key + destination = DecodeDestination(exp_base58string); + BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid privkey as pubkey:" + strTest); + } else { + // Must be valid public key + destination = DecodeDestination(exp_base58string); + CScript script = GetScriptForDestination(destination); + BOOST_CHECK_MESSAGE(IsValidDestination(destination), "!IsValid:" + strTest); + BOOST_CHECK_EQUAL(HexStr(script), HexStr(exp_payload)); + + // Try flipped case version + for (char& c : exp_base58string) { + if (c >= 'a' && c <= 'z') { + c = (c - 'a') + 'A'; + } else if (c >= 'A' && c <= 'Z') { + c = (c - 'A') + 'a'; + } + } + destination = DecodeDestination(exp_base58string); + BOOST_CHECK_MESSAGE(IsValidDestination(destination) == try_case_flip, "!IsValid case flipped:" + strTest); + if (IsValidDestination(destination)) { + script = GetScriptForDestination(destination); + BOOST_CHECK_EQUAL(HexStr(script), HexStr(exp_payload)); + } + + // Public key must be invalid private key + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid pubkey as privkey:" + strTest); + } + } +} + +// Goal: check that generated keys match test vectors +BOOST_AUTO_TEST_CASE(key_io_valid_gen) +{ + UniValue tests = read_json(std::string(json_tests::key_io_valid, json_tests::key_io_valid + sizeof(json_tests::key_io_valid))); + + for (unsigned int idx = 0; idx < tests.size(); idx++) { + UniValue test = tests[idx]; + std::string strTest = test.write(); + if (test.size() < 3) // Allow for extra stuff (useful for comments) + { + BOOST_ERROR("Bad test: " << strTest); + continue; + } + std::string exp_base58string = test[0].get_str(); + std::vector<unsigned char> exp_payload = ParseHex(test[1].get_str()); + const UniValue &metadata = test[2].get_obj(); + bool isPrivkey = find_value(metadata, "isPrivkey").get_bool(); + SelectParams(find_value(metadata, "chain").get_str()); + if (isPrivkey) { + bool isCompressed = find_value(metadata, "isCompressed").get_bool(); + CKey key; + key.Set(exp_payload.begin(), exp_payload.end(), isCompressed); + assert(key.IsValid()); + BOOST_CHECK_MESSAGE(EncodeSecret(key) == exp_base58string, "result mismatch: " + strTest); + } else { + CTxDestination dest; + CScript exp_script(exp_payload.begin(), exp_payload.end()); + ExtractDestination(exp_script, dest); + std::string address = EncodeDestination(dest); + + BOOST_CHECK_EQUAL(address, exp_base58string); + } + } + + SelectParams(CBaseChainParams::MAIN); +} + + +// Goal: check that base58 parsing code is robust against a variety of corrupted data +BOOST_AUTO_TEST_CASE(key_io_invalid) +{ + UniValue tests = read_json(std::string(json_tests::key_io_invalid, json_tests::key_io_invalid + sizeof(json_tests::key_io_invalid))); // Negative testcases + CKey privkey; + CTxDestination destination; + + for (unsigned int idx = 0; idx < tests.size(); idx++) { + UniValue test = tests[idx]; + std::string strTest = test.write(); + if (test.size() < 1) // Allow for extra stuff (useful for comments) + { + BOOST_ERROR("Bad test: " << strTest); + continue; + } + 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 }) { + SelectParams(chain); + destination = DecodeDestination(exp_base58string); + BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid pubkey in mainnet:" + strTest); + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid privkey in mainnet:" + strTest); + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 55ee1ecf6b..64c57f0705 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -4,7 +4,7 @@ #include <key.h> -#include <base58.h> +#include <key_io.h> #include <script/script.h> #include <uint256.h> #include <util.h> @@ -32,21 +32,16 @@ BOOST_FIXTURE_TEST_SUITE(key_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(key_test1) { - CBitcoinSecret bsecret1, bsecret2, bsecret1C, bsecret2C, baddress1; - BOOST_CHECK( bsecret1.SetString (strSecret1)); - BOOST_CHECK( bsecret2.SetString (strSecret2)); - BOOST_CHECK( bsecret1C.SetString(strSecret1C)); - BOOST_CHECK( bsecret2C.SetString(strSecret2C)); - BOOST_CHECK(!baddress1.SetString(strAddressBad)); - - CKey key1 = bsecret1.GetKey(); - BOOST_CHECK(key1.IsCompressed() == false); - CKey key2 = bsecret2.GetKey(); - BOOST_CHECK(key2.IsCompressed() == false); - CKey key1C = bsecret1C.GetKey(); - BOOST_CHECK(key1C.IsCompressed() == true); - CKey key2C = bsecret2C.GetKey(); - BOOST_CHECK(key2C.IsCompressed() == true); + CKey key1 = DecodeSecret(strSecret1); + BOOST_CHECK(key1.IsValid() && !key1.IsCompressed()); + CKey key2 = DecodeSecret(strSecret2); + BOOST_CHECK(key2.IsValid() && !key2.IsCompressed()); + CKey key1C = DecodeSecret(strSecret1C); + BOOST_CHECK(key1C.IsValid() && key1C.IsCompressed()); + CKey key2C = DecodeSecret(strSecret2C); + BOOST_CHECK(key2C.IsValid() && key2C.IsCompressed()); + CKey bad_key = DecodeSecret(strAddressBad); + BOOST_CHECK(!bad_key.IsValid()); CPubKey pubkey1 = key1. GetPubKey(); CPubKey pubkey2 = key2. GetPubKey(); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 108c1a063e..892e4f2dac 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -5,8 +5,8 @@ #include <rpc/server.h> #include <rpc/client.h> -#include <base58.h> #include <core_io.h> +#include <key_io.h> #include <netbase.h> #include <test/test_bitcoin.h> diff --git a/src/util.cpp b/src/util.cpp index dcf7ed38b1..82c99a3c2f 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -79,6 +79,7 @@ #include <openssl/crypto.h> #include <openssl/rand.h> #include <openssl/conf.h> +#include <thread> // Application startup time (used for uptime calculation) const int64_t nStartupTime = GetTime(); @@ -927,11 +928,7 @@ bool SetupNetworking() int GetNumCores() { -#if BOOST_VERSION >= 105600 - return boost::thread::physical_concurrency(); -#else // Must fall back to hardware_concurrency, which unfortunately counts virtual cores - return boost::thread::hardware_concurrency(); -#endif + return std::thread::hardware_concurrency(); } std::string CopyrightHolders(const std::string& strPrefix) diff --git a/src/util.h b/src/util.h index 9490a5678f..e4170d8aa2 100644 --- a/src/util.h +++ b/src/util.h @@ -312,9 +312,8 @@ std::string HelpMessageGroup(const std::string& message); std::string HelpMessageOpt(const std::string& option, const std::string& message); /** - * Return the number of physical cores available on the current system. - * @note This does not count virtual cores, such as those provided by HyperThreading - * when boost is newer than 1.56. + * Return the number of cores available on the current system. + * @note This does count virtual cores, such as those provided by HyperThreading. */ int GetNumCores(); diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index ebafe078f4..d1025fc7bf 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -127,46 +127,11 @@ std::string EncodeBase64(const unsigned char* pch, size_t len) { static const char *pbase64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; - std::string strRet; - strRet.reserve((len+2)/3*4); - - int mode=0, left=0; - const unsigned char *pchEnd = pch+len; - - while (pch<pchEnd) - { - int enc = *(pch++); - switch (mode) - { - case 0: // we have no bits - strRet += pbase64[enc >> 2]; - left = (enc & 3) << 4; - mode = 1; - break; - - case 1: // we have two bits - strRet += pbase64[left | (enc >> 4)]; - left = (enc & 15) << 2; - mode = 2; - break; - - case 2: // we have four bits - strRet += pbase64[left | (enc >> 6)]; - strRet += pbase64[enc & 63]; - mode = 0; - break; - } - } - - if (mode) - { - strRet += pbase64[left]; - strRet += '='; - if (mode == 1) - strRet += '='; - } - - return strRet; + std::string str; + str.reserve(((len + 2) / 3) * 4); + ConvertBits<8, 6, true>([&](int v) { str += pbase64[v]; }, pch, pch + len); + while (str.size() % 4) str += '='; + return str; } std::string EncodeBase64(const std::string& str) @@ -193,68 +158,32 @@ std::vector<unsigned char> DecodeBase64(const char* p, bool* pfInvalid) -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1 }; - if (pfInvalid) - *pfInvalid = false; - - std::vector<unsigned char> vchRet; - vchRet.reserve(strlen(p)*3/4); - - int mode = 0; - int left = 0; - - while (1) - { - int dec = decode64_table[(unsigned char)*p]; - if (dec == -1) break; - p++; - switch (mode) - { - case 0: // we have no bits and get 6 - left = dec; - mode = 1; - break; - - case 1: // we have 6 bits and keep 4 - vchRet.push_back((left<<2) | (dec>>4)); - left = dec & 15; - mode = 2; - break; - - case 2: // we have 4 bits and get 6, we keep 2 - vchRet.push_back((left<<4) | (dec>>2)); - left = dec & 3; - mode = 3; - break; - - case 3: // we have 2 bits and get 6 - vchRet.push_back((left<<6) | dec); - mode = 0; - break; - } + const char* e = p; + std::vector<uint8_t> val; + val.reserve(strlen(p)); + while (*p != 0) { + int x = decode64_table[(unsigned char)*p]; + if (x == -1) break; + val.push_back(x); + ++p; } - if (pfInvalid) - switch (mode) - { - case 0: // 4n base64 characters processed: ok - break; - - case 1: // 4n+1 base64 character processed: impossible - *pfInvalid = true; - break; - - case 2: // 4n+2 base64 characters processed: require '==' - if (left || p[0] != '=' || p[1] != '=' || decode64_table[(unsigned char)p[2]] != -1) - *pfInvalid = true; - break; - - case 3: // 4n+3 base64 characters processed: require '=' - if (left || p[0] != '=' || decode64_table[(unsigned char)p[1]] != -1) - *pfInvalid = true; - break; + std::vector<unsigned char> ret; + ret.reserve((val.size() * 3) / 4); + bool valid = ConvertBits<6, 8, false>([&](unsigned char c) { ret.push_back(c); }, val.begin(), val.end()); + + const char* q = p; + while (valid && *p != 0) { + if (*p != '=') { + valid = false; + break; } + ++p; + } + valid = valid && (p - e) % 4 == 0 && p - q < 4; + if (pfInvalid) *pfInvalid = !valid; - return vchRet; + return ret; } std::string DecodeBase64(const std::string& str) @@ -267,59 +196,11 @@ std::string EncodeBase32(const unsigned char* pch, size_t len) { static const char *pbase32 = "abcdefghijklmnopqrstuvwxyz234567"; - std::string strRet; - strRet.reserve((len+4)/5*8); - - int mode=0, left=0; - const unsigned char *pchEnd = pch+len; - - while (pch<pchEnd) - { - int enc = *(pch++); - switch (mode) - { - case 0: // we have no bits - strRet += pbase32[enc >> 3]; - left = (enc & 7) << 2; - mode = 1; - break; - - case 1: // we have three bits - strRet += pbase32[left | (enc >> 6)]; - strRet += pbase32[(enc >> 1) & 31]; - left = (enc & 1) << 4; - mode = 2; - break; - - case 2: // we have one bit - strRet += pbase32[left | (enc >> 4)]; - left = (enc & 15) << 1; - mode = 3; - break; - - case 3: // we have four bits - strRet += pbase32[left | (enc >> 7)]; - strRet += pbase32[(enc >> 2) & 31]; - left = (enc & 3) << 3; - mode = 4; - break; - - case 4: // we have two bits - strRet += pbase32[left | (enc >> 5)]; - strRet += pbase32[enc & 31]; - mode = 0; - } - } - - static const int nPadding[5] = {0, 6, 4, 3, 1}; - if (mode) - { - strRet += pbase32[left]; - for (int n=0; n<nPadding[mode]; n++) - strRet += '='; - } - - return strRet; + std::string str; + str.reserve(((len + 4) / 5) * 8); + ConvertBits<8, 5, true>([&](int v) { str += pbase32[v]; }, pch, pch + len); + while (str.size() % 8) str += '='; + return str; } std::string EncodeBase32(const std::string& str) @@ -346,102 +227,32 @@ std::vector<unsigned char> DecodeBase32(const char* p, bool* pfInvalid) -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1 }; - if (pfInvalid) - *pfInvalid = false; - - std::vector<unsigned char> vchRet; - vchRet.reserve((strlen(p))*5/8); - - int mode = 0; - int left = 0; - - while (1) - { - int dec = decode32_table[(unsigned char)*p]; - if (dec == -1) break; - p++; - switch (mode) - { - case 0: // we have no bits and get 5 - left = dec; - mode = 1; - break; - - case 1: // we have 5 bits and keep 2 - vchRet.push_back((left<<3) | (dec>>2)); - left = dec & 3; - mode = 2; - break; - - case 2: // we have 2 bits and keep 7 - left = left << 5 | dec; - mode = 3; - break; - - case 3: // we have 7 bits and keep 4 - vchRet.push_back((left<<1) | (dec>>4)); - left = dec & 15; - mode = 4; - break; - - case 4: // we have 4 bits, and keep 1 - vchRet.push_back((left<<4) | (dec>>1)); - left = dec & 1; - mode = 5; - break; - - case 5: // we have 1 bit, and keep 6 - left = left << 5 | dec; - mode = 6; - break; - - case 6: // we have 6 bits, and keep 3 - vchRet.push_back((left<<2) | (dec>>3)); - left = dec & 7; - mode = 7; - break; - - case 7: // we have 3 bits, and keep 0 - vchRet.push_back((left<<5) | dec); - mode = 0; - break; - } + const char* e = p; + std::vector<uint8_t> val; + val.reserve(strlen(p)); + while (*p != 0) { + int x = decode32_table[(unsigned char)*p]; + if (x == -1) break; + val.push_back(x); + ++p; } - if (pfInvalid) - switch (mode) - { - case 0: // 8n base32 characters processed: ok - break; - - case 1: // 8n+1 base32 characters processed: impossible - case 3: // +3 - case 6: // +6 - *pfInvalid = true; - break; - - case 2: // 8n+2 base32 characters processed: require '======' - if (left || p[0] != '=' || p[1] != '=' || p[2] != '=' || p[3] != '=' || p[4] != '=' || p[5] != '=' || decode32_table[(unsigned char)p[6]] != -1) - *pfInvalid = true; - break; - - case 4: // 8n+4 base32 characters processed: require '====' - if (left || p[0] != '=' || p[1] != '=' || p[2] != '=' || p[3] != '=' || decode32_table[(unsigned char)p[4]] != -1) - *pfInvalid = true; - break; - - case 5: // 8n+5 base32 characters processed: require '===' - if (left || p[0] != '=' || p[1] != '=' || p[2] != '=' || decode32_table[(unsigned char)p[3]] != -1) - *pfInvalid = true; - break; - - case 7: // 8n+7 base32 characters processed: require '=' - if (left || p[0] != '=' || decode32_table[(unsigned char)p[1]] != -1) - *pfInvalid = true; - break; + std::vector<unsigned char> ret; + ret.reserve((val.size() * 5) / 8); + bool valid = ConvertBits<5, 8, false>([&](unsigned char c) { ret.push_back(c); }, val.begin(), val.end()); + + const char* q = p; + while (valid && *p != 0) { + if (*p != '=') { + valid = false; + break; } + ++p; + } + valid = valid && (p - e) % 8 == 0 && p - q < 8; + if (pfInvalid) *pfInvalid = !valid; - return vchRet; + return ret; } std::string DecodeBase32(const std::string& str) diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 994e6abbad..1c9cca90b2 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -151,7 +151,7 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); /** Convert from one power-of-2 number base to another. */ template<int frombits, int tobits, bool pad, typename O, typename I> -bool ConvertBits(O& out, I it, I end) { +bool ConvertBits(const O& outfn, I it, I end) { size_t acc = 0; size_t bits = 0; constexpr size_t maxv = (1 << tobits) - 1; @@ -161,12 +161,12 @@ bool ConvertBits(O& out, I it, I end) { bits += frombits; while (bits >= tobits) { bits -= tobits; - out.push_back((acc >> bits) & maxv); + outfn((acc >> bits) & maxv); } ++it; } if (pad) { - if (bits) out.push_back((acc << (tobits - bits)) & maxv); + if (bits) outfn((acc << (tobits - bits)) & maxv); } else if (bits >= frombits || ((acc << (tobits - bits)) & maxv)) { return false; } diff --git a/src/validation.cpp b/src/validation.cpp index a77362f5d6..51e40c17b5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1856,12 +1856,65 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // before the first had been spent. Since those coinbases are sufficiently buried its no longer possible to create further // duplicate transactions descending from the known pairs either. // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check. + + // BIP34 requires that a block at height X (block X) has its coinbase + // scriptSig start with a CScriptNum of X (indicated height X). The above + // logic of no longer requiring BIP30 once BIP34 activates is flawed in the + // case that there is a block X before the BIP34 height of 227,931 which has + // an indicated height Y where Y is greater than X. The coinbase for block + // X would also be a valid coinbase for block Y, which could be a BIP30 + // violation. An exhaustive search of all mainnet coinbases before the + // BIP34 height which have an indicated height greater than the block height + // reveals many occurrences. The 3 lowest indicated heights found are + // 209,921, 490,897, and 1,983,702 and thus coinbases for blocks at these 3 + // heights would be the first opportunity for BIP30 to be violated. + + // The search reveals a great many blocks which have an indicated height + // greater than 1,983,702, so we simply remove the optimization to skip + // BIP30 checking for blocks at height 1,983,702 or higher. Before we reach + // that block in another 25 years or so, we should take advantage of a + // future consensus change to do a new and improved version of BIP34 that + // will actually prevent ever creating any duplicate coinbases in the + // future. + static constexpr int BIP34_IMPLIES_BIP30_LIMIT = 1983702; + + // There is no potential to create a duplicate coinbase at block 209,921 + // because this is still before the BIP34 height and so explicit BIP30 + // checking is still active. + + // The final case is block 176,684 which has an indicated height of + // 490,897. Unfortunately, this issue was not discovered until about 2 weeks + // before block 490,897 so there was not much opportunity to address this + // case other than to carefully analyze it and determine it would not be a + // problem. Block 490,897 was, in fact, mined with a different coinbase than + // block 176,684, but it is important to note that even if it hadn't been or + // is remined on an alternate fork with a duplicate coinbase, we would still + // not run into a BIP30 violation. This is because the coinbase for 176,684 + // is spent in block 185,956 in transaction + // d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781. This + // spending transaction can't be duplicated because it also spends coinbase + // 0328dd85c331237f18e781d692c92de57649529bd5edf1d01036daea32ffde29. This + // coinbase has an indicated height of over 4.2 billion, and wouldn't be + // duplicatable until that height, and it's currently impossible to create a + // chain that long. Nevertheless we may wish to consider a future soft fork + // which retroactively prevents block 490,897 from creating a duplicate + // coinbase. The two historical BIP30 violations often provide a confusing + // edge case when manipulating the UTXO and it would be simpler not to have + // another edge case to deal with. + + // testnet3 has no blocks before the BIP34 height with indicated heights + // post BIP34 before approximately height 486,000,000 and presumably will + // be reset before it reaches block 1,983,702 and starts doing unnecessary + // BIP30 checking again. assert(pindex->pprev); CBlockIndex *pindexBIP34height = pindex->pprev->GetAncestor(chainparams.GetConsensus().BIP34Height); //Only continue to enforce if we're below BIP34 activation height or the block hash at that height doesn't correspond. fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == chainparams.GetConsensus().BIP34Hash)); - if (fEnforceBIP30) { + // TODO: Remove BIP30 checking from block height 1,983,702 on, once we have a + // consensus change that ensures coinbases at those heights can not + // duplicate earlier coinbases. + if (fEnforceBIP30 || pindex->nHeight >= BIP34_IMPLIES_BIP30_LIMIT) { for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 23c6279128..ebe7b48da0 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -52,20 +52,55 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) } } } + +CCriticalSection cs_db; +std::map<std::string, CDBEnv> g_dbenvs; //!< Map from directory name to open db environment. } // namespace +CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) +{ + fs::path env_directory; + if (fs::is_regular_file(wallet_path)) { + // Special case for backwards compatibility: if wallet path points to an + // existing file, treat it as the path to a BDB data file in a parent + // directory that also contains BDB log files. + env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + } else { + // Normal case: Interpret wallet path as a directory path containing + // data and log files. + env_directory = wallet_path; + database_filename = "wallet.dat"; + } + LOCK(cs_db); + // Note: An ununsed temporary CDBEnv 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. + return &g_dbenvs.emplace(std::piecewise_construct, std::forward_as_tuple(env_directory.string()), std::forward_as_tuple(env_directory)).first->second; +} + // // CDB // -CDBEnv bitdb; - -void CDBEnv::EnvShutdown() +void CDBEnv::Close() { if (!fDbEnvInit) return; fDbEnvInit = false; + + for (auto& db : mapDb) { + auto count = mapFileUseCount.find(db.first); + assert(count == mapFileUseCount.end() || count->second == 0); + if (db.second) { + db.second->close(0); + delete db.second; + db.second = nullptr; + } + } + int ret = dbenv->close(0); if (ret != 0) LogPrintf("CDBEnv::EnvShutdown: Error %d shutting down database environment: %s\n", ret, DbEnv::strerror(ret)); @@ -80,29 +115,25 @@ void CDBEnv::Reset() fMockDb = false; } -CDBEnv::CDBEnv() +CDBEnv::CDBEnv(const fs::path& dir_path) : strPath(dir_path.string()) { Reset(); } CDBEnv::~CDBEnv() { - EnvShutdown(); + Close(); } -void CDBEnv::Close() -{ - EnvShutdown(); -} - -bool CDBEnv::Open(const fs::path& pathIn, bool retry) +bool CDBEnv::Open(bool retry) { if (fDbEnvInit) return true; boost::this_thread::interruption_point(); - strPath = pathIn.string(); + fs::path pathIn = strPath; + TryCreateDirectories(pathIn); if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); return false; @@ -150,7 +181,7 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) // failure is ok (well, not really, but it's not worse than what we started with) } // try opening it again one more time - if (!Open(pathIn, false)) { + if (!Open(false /* retry */)) { // if it still fails, it probably means we can't even create the database env return false; } @@ -209,12 +240,15 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, recoverFunc_type return RECOVER_FAIL; // Try to recover: - bool fRecovered = (*recoverFunc)(strFile, out_backup_filename); + bool fRecovered = (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename); return (fRecovered ? RECOVER_OK : RECOVER_FAIL); } -bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +bool CDB::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) { + std::string filename; + CDBEnv* env = GetWalletEnv(file_path, filename); + // Recovery procedure: // move wallet file to walletfilename.timestamp.bak // Call Salvage with fAggressive=true to @@ -225,7 +259,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco int64_t now = GetTime(); newFilename = strprintf("%s.%d.bak", filename, now); - int result = bitdb.dbenv->dbrename(nullptr, filename.c_str(), nullptr, + int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, newFilename.c_str(), DB_AUTO_COMMIT); if (result == 0) LogPrintf("Renamed %s to %s\n", filename, newFilename); @@ -236,7 +270,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } std::vector<CDBEnv::KeyValPair> salvagedData; - bool fSuccess = bitdb.Salvage(newFilename, true, salvagedData); + bool fSuccess = env->Salvage(newFilename, true, salvagedData); if (salvagedData.empty()) { LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename); @@ -244,7 +278,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(bitdb.dbenv.get(), 0); + std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -257,7 +291,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return false; } - DbTxn* ptxn = bitdb.TxnBegin(); + DbTxn* ptxn = env->TxnBegin(); for (CDBEnv::KeyValPair& row : salvagedData) { if (recoverKVcallback) @@ -279,8 +313,12 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return fSuccess; } -bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) +bool CDB::VerifyEnvironment(const fs::path& file_path, std::string& errorStr) { + std::string walletFile; + CDBEnv* env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0)); LogPrintf("Using wallet %s\n", walletFile); @@ -291,7 +329,7 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return false; } - if (!bitdb.Open(walletDir, true)) { + if (!env->Open(true /* retry */)) { errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); return false; } @@ -299,12 +337,16 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return true; } -bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) +bool CDB::VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) { + std::string walletFile; + CDBEnv* env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + if (fs::exists(walletDir / walletFile)) { std::string backup_filename; - CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc, backup_filename); + CDBEnv::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename); if (r == CDBEnv::RECOVER_OK) { warningStr = strprintf(_("Warning: Wallet file corrupt, data salvaged!" @@ -414,8 +456,8 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb nFlags |= DB_CREATE; { - LOCK(env->cs_db); - if (!env->Open(GetWalletDir())) + LOCK(cs_db); + if (!env->Open(false /* retry */)) throw std::runtime_error("CDB: Failed to open database environment."); pdb = env->mapDb[strFilename]; @@ -442,7 +484,25 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (ret != 0) { throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } - CheckUniqueFileid(*env, strFilename, *pdb_temp); + + // Call CheckUniqueFileid on the containing BDB environment to + // avoid BDB data consistency bugs that happen when different data + // files in the same environment have the same fileid. + // + // Also call CheckUniqueFileid on all the other g_dbenvs to prevent + // bitcoin from opening the same data file through another + // environment when the file is referenced through equivalent but + // not obviously identical symlinked or hard linked or bind mounted + // paths. In the future a more relaxed check for equal inode and + // device ids could be done instead, which would allow opening + // different backup copies of a wallet at the same time. Maybe even + // more ideally, an exclusive lock for accessing the database could + // 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) { + CheckUniqueFileid(env.second, strFilename, *pdb_temp); + } pdb = pdb_temp.release(); env->mapDb[strFilename] = pdb; @@ -490,7 +550,7 @@ void CDB::Close() Flush(); { - LOCK(env->cs_db); + LOCK(cs_db); --env->mapFileUseCount[strFile]; } } @@ -518,7 +578,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) const std::string& strFile = dbw.strFile; while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file env->CloseDb(strFile); @@ -646,7 +706,7 @@ bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) bool ret = false; CDBEnv *env = dbw.env; const std::string& strFile = dbw.strFile; - TRY_LOCK(bitdb.cs_db,lockDb); + TRY_LOCK(cs_db, lockDb); if (lockDb) { // Don't do this if any databases are in use @@ -694,7 +754,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file diff --git a/src/wallet/db.h b/src/wallet/db.h index 787135e400..b1ce451534 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -11,6 +11,7 @@ #include <serialize.h> #include <streams.h> #include <sync.h> +#include <util.h> #include <version.h> #include <atomic> @@ -32,20 +33,19 @@ private: // shutdown problems/crashes caused by a static initialized internal pointer. std::string strPath; - void EnvShutdown(); - public: - mutable CCriticalSection cs_db; std::unique_ptr<DbEnv> dbenv; std::map<std::string, int> mapFileUseCount; std::map<std::string, Db*> mapDb; - CDBEnv(); + CDBEnv(const fs::path& env_directory); ~CDBEnv(); void Reset(); void MakeMock(); bool IsMock() const { return fMockDb; } + bool IsInitialized() const { return fDbEnvInit; } + fs::path Directory() const { return strPath; } /** * Verify that database file strFile is OK. If it is not, @@ -56,7 +56,7 @@ public: enum VerifyResult { VERIFY_OK, RECOVER_OK, RECOVER_FAIL }; - typedef bool (*recoverFunc_type)(const std::string& strFile, std::string& out_backup_filename); + typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename); VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename); /** * Salvage data from a file that Verify says is bad. @@ -68,7 +68,7 @@ public: typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult); - bool Open(const fs::path& path, bool retry = 0); + bool Open(bool retry); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); @@ -85,7 +85,8 @@ public: } }; -extern CDBEnv bitdb; +/** Get CDBEnv and database filename given a wallet path. */ +CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename); /** An instance of this class represents one database. * For BerkeleyDB this is just a (env, strFile) tuple. @@ -100,9 +101,33 @@ public: } /** Create DB handle to real database */ - CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in) : - nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0), env(env_in), strFile(strFile_in) + CWalletDBWrapper(const fs::path& wallet_path, bool mock = false) : + nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0) { + env = GetWalletEnv(wallet_path, strFile); + if (mock) { + env->Close(); + env->Reset(); + env->MakeMock(); + } + } + + /** Return object for accessing database at specified path. */ + static std::unique_ptr<CWalletDBWrapper> Create(const fs::path& path) + { + return MakeUnique<CWalletDBWrapper>(path); + } + + /** Return object for accessing dummy database with no read/write capabilities. */ + static std::unique_ptr<CWalletDBWrapper> CreateDummy() + { + return MakeUnique<CWalletDBWrapper>(); + } + + /** Return object for accessing temporary in-memory database. */ + static std::unique_ptr<CWalletDBWrapper> CreateMock() + { + return MakeUnique<CWalletDBWrapper>("", true /* mock */); } /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero @@ -113,10 +138,6 @@ public: */ bool Backup(const std::string& strDest); - /** Get a name for this database, for debugging etc. - */ - std::string GetName() const { return strFile; } - /** Make sure all changes are flushed to disk. */ void Flush(bool shutdown); @@ -161,15 +182,15 @@ public: void Flush(); void Close(); - static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* flush the wallet passively (TRY_LOCK) ideal to be called periodically */ static bool PeriodicFlush(CWalletDBWrapper& dbw); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr); + static bool VerifyEnvironment(const fs::path& file_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); public: template <typename K, typename T> @@ -329,7 +350,7 @@ public: { if (!pdb || activeTxn) return false; - DbTxn* ptxn = bitdb.TxnBegin(); + DbTxn* ptxn = env->TxnBegin(); if (!ptxn) return false; activeTxn = ptxn; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 9ac48bff77..e028cf4210 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -35,7 +35,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup")); - strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=<path>", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to <walletdir> if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in <walletdir>.)")); strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST)); strUsage += HelpMessageOpt("-walletdir=<dir>", _("Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)")); strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); @@ -66,7 +66,7 @@ bool WalletParameterInteraction() return true; } - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + gArgs.SoftSetArg("-wallet", ""); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { @@ -230,18 +230,22 @@ bool VerifyWallets() std::set<fs::path> wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - if (boost::filesystem::path(walletFile).filename() != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. -wallet parameter must only specify a filename (not a path)."), walletFile)); - } - - if (SanitizeString(walletFile, SAFE_CHARS_FILENAME) != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. Invalid characters in -wallet filename."), walletFile)); - } - + // Do some checking on wallet path. It should be either a: + // + // 1. Path where a directory can be created. + // 2. Path to an existing directory. + // 3. Path to a symlink to a directory. + // 4. For backwards compatibility, the name of a data file in -walletdir. fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); - - if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { - return InitError(strprintf(_("Error loading wallet %s. -wallet filename must be a regular file."), walletFile)); + fs::file_type path_type = fs::symlink_status(wallet_path).type(); + if (!(path_type == fs::file_not_found || path_type == fs::directory_file || + (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || + (path_type == fs::regular_file && fs::path(walletFile).filename() == walletFile))) { + return InitError(strprintf( + _("Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " + "database/log.?????????? files can be stored, a location where such a directory could be created, " + "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)"), + walletFile, GetWalletDir())); } if (!wallet_paths.insert(wallet_path).second) { @@ -249,21 +253,21 @@ bool VerifyWallets() } std::string strError; - if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), strError)) { + if (!CWalletDB::VerifyEnvironment(wallet_path, strError)) { return InitError(strError); } if (gArgs.GetBoolArg("-salvagewallet", false)) { // Recover readable keypairs: - CWallet dummyWallet; + CWallet dummyWallet("dummy", CWalletDBWrapper::CreateDummy()); std::string backup_filename; - if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { + if (!CWalletDB::Recover(wallet_path, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { return false; } } std::string strWarning; - bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetWalletDir().string(), strWarning, strError); + bool dbV = CWalletDB::VerifyDatabaseFile(wallet_path, strWarning, strError); if (!strWarning.empty()) { InitWarning(strWarning); } @@ -284,7 +288,7 @@ bool OpenWallets() } for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile); + CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir())); if (!pwallet) { return false; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 930e8bbbb4..0edc8d8d66 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <base58.h> #include <chain.h> +#include <key_io.h> #include <rpc/safemode.h> #include <rpc/server.h> #include <wallet/init.h> @@ -147,13 +147,8 @@ UniValue importprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strSecret); - - if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - - CKey key = vchSecret.GetKey(); - if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + CKey key = DecodeSecret(strSecret); + if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); CPubKey pubkey = key.GetPubKey(); assert(key.VerifyPubKey(pubkey)); @@ -554,9 +549,8 @@ UniValue importwallet(const JSONRPCRequest& request) boost::split(vstr, line, boost::is_any_of(" ")); if (vstr.size() < 2) continue; - CBitcoinSecret vchSecret; - if (vchSecret.SetString(vstr[0])) { - CKey key = vchSecret.GetKey(); + CKey key = DecodeSecret(vstr[0]); + if (key.IsValid()) { CPubKey pubkey = key.GetPubKey(); assert(key.VerifyPubKey(pubkey)); CKeyID keyid = pubkey.GetID(); @@ -659,7 +653,7 @@ UniValue dumpprivkey(const JSONRPCRequest& request) if (!pwallet->GetKey(keyid, vchSecret)) { throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } - return CBitcoinSecret(vchSecret).ToString(); + return EncodeSecret(vchSecret); } @@ -742,10 +736,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) CExtKey masterKey; masterKey.SetMaster(key.begin(), key.size()); - CBitcoinExtKey b58extkey; - b58extkey.SetKey(masterKey); - - file << "# extended private masterkey: " << b58extkey.ToString() << "\n\n"; + file << "# extended private masterkey: " << EncodeExtKey(masterKey) << "\n\n"; } } for (std::vector<std::pair<int64_t, CKeyID> >::const_iterator it = vKeyBirth.begin(); it != vKeyBirth.end(); it++) { @@ -755,7 +746,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) std::string strLabel; CKey key; if (pwallet->GetKey(keyid, key)) { - file << strprintf("%s %s ", CBitcoinSecret(key).ToString(), strTime); + file << strprintf("%s %s ", EncodeSecret(key), strTime); if (GetWalletAddressesForKey(pwallet, keyid, strAddr, strLabel)) { file << strprintf("label=%s", strLabel); } else if (keyid == masterKeyID) { @@ -911,17 +902,10 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6 for (size_t i = 0; i < keys.size(); i++) { const std::string& privkey = keys[i].get_str(); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(privkey); - - if (!fGood) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - } - - CKey key = vchSecret.GetKey(); + CKey key = DecodeSecret(privkey); if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); } CPubKey pubkey = key.GetPubKey(); @@ -1018,16 +1002,10 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6 const std::string& strPrivkey = keys[0].get_str(); // Checks. - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strPrivkey); - - if (!fGood) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - } + CKey key = DecodeSecret(strPrivkey); - CKey key = vchSecret.GetKey(); if (!key.IsValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); } CPubKey pubKey = key.GetPubKey(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8b95c56a5f..457abec1bc 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4,12 +4,12 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <amount.h> -#include <base58.h> #include <chain.h> #include <consensus/validation.h> #include <core_io.h> #include <httpserver.h> #include <validation.h> +#include <key_io.h> #include <net.h> #include <policy/feerate.h> #include <policy/fees.h> @@ -1403,6 +1403,16 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool fByA if(params[2].get_bool()) filter = filter | ISMINE_WATCH_ONLY; + bool has_filtered_address = false; + CTxDestination filtered_address = CNoDestination(); + if (!fByAccounts && params.size() > 3) { + if (!IsValidDestinationString(params[3].get_str())) { + throw JSONRPCError(RPC_WALLET_ERROR, "address_filter parameter was invalid"); + } + filtered_address = DecodeDestination(params[3].get_str()); + has_filtered_address = true; + } + // Tally std::map<CTxDestination, tallyitem> mapTally; for (const std::pair<uint256, CWalletTx>& pairWtx : pwallet->mapWallet) { @@ -1421,6 +1431,10 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool fByA if (!ExtractDestination(txout.scriptPubKey, address)) continue; + if (has_filtered_address && !(filtered_address == address)) { + continue; + } + isminefilter mine = IsMine(*pwallet, address); if(!(mine & filter)) continue; @@ -1437,10 +1451,24 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool fByA // Reply UniValue ret(UniValue::VARR); std::map<std::string, tallyitem> mapAccountTally; - for (const std::pair<CTxDestination, CAddressBookData>& item : pwallet->mapAddressBook) { - const CTxDestination& dest = item.first; - const std::string& strAccount = item.second.name; - std::map<CTxDestination, tallyitem>::iterator it = mapTally.find(dest); + + // Create mapAddressBook iterator + // If we aren't filtering, go from begin() to end() + auto start = pwallet->mapAddressBook.begin(); + auto end = pwallet->mapAddressBook.end(); + // If we are filtering, find() the applicable entry + if (has_filtered_address) { + start = pwallet->mapAddressBook.find(filtered_address); + if (start != end) { + end = std::next(start); + } + } + + for (auto item_it = start; item_it != end; ++item_it) + { + const CTxDestination& address = item_it->first; + const std::string& strAccount = item_it->second.name; + auto it = mapTally.find(address); if (it == mapTally.end() && !fIncludeEmpty) continue; @@ -1466,7 +1494,7 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool fByA UniValue obj(UniValue::VOBJ); if(fIsWatchonly) obj.pushKV("involvesWatchonly", true); - obj.pushKV("address", EncodeDestination(dest)); + obj.pushKV("address", EncodeDestination(address)); obj.pushKV("account", strAccount); obj.pushKV("amount", ValueFromAmount(nAmount)); obj.pushKV("confirmations", (nConf == std::numeric_limits<int>::max() ? 0 : nConf)); @@ -1511,15 +1539,15 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() > 3) + if (request.fHelp || request.params.size() > 4) throw std::runtime_error( - "listreceivedbyaddress ( minconf include_empty include_watchonly)\n" + "listreceivedbyaddress ( minconf include_empty include_watchonly address_filter )\n" "\nList balances by receiving address.\n" "\nArguments:\n" "1. minconf (numeric, optional, default=1) The minimum number of confirmations before payments are included.\n" "2. include_empty (bool, optional, default=false) Whether to include addresses that haven't received any payments.\n" "3. include_watchonly (bool, optional, default=false) Whether to include watch-only addresses (see 'importaddress').\n" - + "4. address_filter (string, optional) If present, only return information on this address.\n" "\nResult:\n" "[\n" " {\n" @@ -1541,6 +1569,7 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request) + HelpExampleCli("listreceivedbyaddress", "") + HelpExampleCli("listreceivedbyaddress", "6 true") + HelpExampleRpc("listreceivedbyaddress", "6, true, true") + + HelpExampleRpc("listreceivedbyaddress", "6, true, true, \"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\"") ); ObserveSafeMode(); @@ -3837,7 +3866,7 @@ static const CRPCCommand commands[] = { "wallet", "listaddressgroupings", &listaddressgroupings, {} }, { "wallet", "listlockunspent", &listlockunspent, {} }, { "wallet", "listreceivedbyaccount", &listreceivedbyaccount, {"minconf","include_empty","include_watchonly"} }, - { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly"} }, + { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} }, { "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, { "wallet", "listtransactions", &listtransactions, {"account","count","skip","include_watchonly"} }, { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index e6510cc214..7b20bd7b02 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -13,13 +13,13 @@ BOOST_FIXTURE_TEST_SUITE(accounting_tests, WalletTestingSetup) static void -GetResults(CWallet *wallet, std::map<CAmount, CAccountingEntry>& results) +GetResults(CWallet& wallet, std::map<CAmount, CAccountingEntry>& results) { std::list<CAccountingEntry> aes; results.clear(); - BOOST_CHECK(wallet->ReorderTransactions() == DB_LOAD_OK); - wallet->ListAccountCreditDebit("", aes); + BOOST_CHECK(wallet.ReorderTransactions() == DB_LOAD_OK); + wallet.ListAccountCreditDebit("", aes); for (CAccountingEntry& ae : aes) { results[ae.nOrderPos] = ae; @@ -33,28 +33,28 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) CAccountingEntry ae; std::map<CAmount, CAccountingEntry> results; - LOCK(pwalletMain->cs_wallet); + LOCK(m_wallet.cs_wallet); ae.strAccount = ""; ae.nCreditDebit = 1; ae.nTime = 1333333333; ae.strOtherAccount = "b"; ae.strComment = ""; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); wtx.mapValue["comment"] = "z"; - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; ae.nTime = 1333333336; ae.strOtherAccount = "c"; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); - BOOST_CHECK(pwalletMain->nOrderPosNext == 3); + BOOST_CHECK(m_wallet.nOrderPosNext == 3); BOOST_CHECK(2 == results.size()); BOOST_CHECK(results[0].nTime == 1333333333); BOOST_CHECK(results[0].strComment.empty()); @@ -65,13 +65,13 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ae.nTime = 1333333330; ae.strOtherAccount = "d"; - ae.nOrderPos = pwalletMain->IncOrderPosNext(); - pwalletMain->AddAccountingEntry(ae); + ae.nOrderPos = m_wallet.IncOrderPosNext(); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 3); - BOOST_CHECK(pwalletMain->nOrderPosNext == 4); + BOOST_CHECK(m_wallet.nOrderPosNext == 4); BOOST_CHECK(results[0].nTime == 1333333333); BOOST_CHECK(1 == vpwtx[0]->nOrderPos); BOOST_CHECK(results[2].nTime == 1333333336); @@ -85,8 +85,8 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ++tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -95,15 +95,15 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ++tx.nLockTime; // Just to change the hash :) wtx.SetTx(MakeTransactionRef(std::move(tx))); } - pwalletMain->AddToWallet(wtx); - vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); + m_wallet.AddToWallet(wtx); + vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 3); - BOOST_CHECK(pwalletMain->nOrderPosNext == 6); + BOOST_CHECK(m_wallet.nOrderPosNext == 6); BOOST_CHECK(0 == vpwtx[2]->nOrderPos); BOOST_CHECK(results[1].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); @@ -116,12 +116,12 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) ae.nTime = 1333333334; ae.strOtherAccount = "e"; ae.nOrderPos = -1; - pwalletMain->AddAccountingEntry(ae); + m_wallet.AddAccountingEntry(ae); - GetResults(pwalletMain.get(), results); + GetResults(m_wallet, results); BOOST_CHECK(results.size() == 4); - BOOST_CHECK(pwalletMain->nOrderPosNext == 7); + BOOST_CHECK(m_wallet.nOrderPosNext == 7); BOOST_CHECK(0 == vpwtx[2]->nOrderPos); BOOST_CHECK(results[1].nTime == 1333333333); BOOST_CHECK(2 == vpwtx[0]->nOrderPos); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 18abf9a9db..77ccd0b8d8 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -6,26 +6,21 @@ #include <rpc/server.h> #include <wallet/db.h> +#include <wallet/wallet.h> WalletTestingSetup::WalletTestingSetup(const std::string& chainName): - TestingSetup(chainName) + TestingSetup(chainName), m_wallet("mock", CWalletDBWrapper::CreateMock()) { - bitdb.MakeMock(); bool fFirstRun; g_address_type = OUTPUT_TYPE_DEFAULT; g_change_type = OUTPUT_TYPE_DEFAULT; - std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = MakeUnique<CWallet>(std::move(dbw)); - pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain.get()); + m_wallet.LoadWallet(fFirstRun); + RegisterValidationInterface(&m_wallet); RegisterWalletRPCCommands(tableRPC); } WalletTestingSetup::~WalletTestingSetup() { - UnregisterValidationInterface(pwalletMain.get()); - - bitdb.Flush(true); - bitdb.Reset(); + UnregisterValidationInterface(&m_wallet); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index c03aec7f87..663836a955 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -15,7 +15,7 @@ struct WalletTestingSetup: public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr<CWallet> pwalletMain; + CWallet m_wallet; }; #endif diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 9db5d63922..41348b50a4 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -36,7 +36,7 @@ typedef std::set<CInputCoin> CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet; +static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); static std::vector<COutput> vCoins; static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) @@ -382,7 +382,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -397,7 +397,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -409,7 +409,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); vpwallets.insert(vpwallets.begin(), &wallet); UniValue keys; keys.setArray(); @@ -471,7 +471,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); LOCK(wallet.cs_wallet); wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); @@ -486,7 +486,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); JSONRPCRequest request; request.params.setArray(); @@ -516,7 +516,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // debit functions. BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); CWalletTx wtx(&wallet, MakeTransactionRef(coinbaseTxns.back())); LOCK2(cs_main, wallet.cs_wallet); wtx.hashBlock = chainActive.Tip()->GetBlockHash(); @@ -562,27 +562,25 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 // expanded to cover more corner cases of smart time logic. BOOST_AUTO_TEST_CASE(ComputeTimeSmart) { - CWallet wallet; - // New transaction should use clock time if lower than block time. - BOOST_CHECK_EQUAL(AddTx(wallet, 1, 100, 120), 100); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100); // Test that updating existing transaction does not change smart time. - BOOST_CHECK_EQUAL(AddTx(wallet, 1, 200, 220), 100); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100); // New transaction should use clock time if there's no block time. - BOOST_CHECK_EQUAL(AddTx(wallet, 2, 300, 0), 300); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 2, 300, 0), 300); // New transaction should use block time if lower than clock time. - BOOST_CHECK_EQUAL(AddTx(wallet, 3, 420, 400), 400); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 3, 420, 400), 400); // New transaction should use latest entry time if higher than // min(block time, clock time). - BOOST_CHECK_EQUAL(AddTx(wallet, 4, 500, 390), 400); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, 500, 390), 400); // If there are future entries, new transaction should use time of the // newest entry that is no more than 300 seconds ahead of the clock time. - BOOST_CHECK_EQUAL(AddTx(wallet, 5, 50, 600), 300); + BOOST_CHECK_EQUAL(AddTx(m_wallet, 5, 50, 600), 300); // Reset mock time for other tests. SetMockTime(0); @@ -591,12 +589,12 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) BOOST_AUTO_TEST_CASE(LoadReceiveRequests) { CTxDestination dest = CKeyID(); - LOCK(pwalletMain->cs_wallet); - pwalletMain->AddDestData(dest, "misc", "val_misc"); - pwalletMain->AddDestData(dest, "rr0", "val_rr0"); - pwalletMain->AddDestData(dest, "rr1", "val_rr1"); + LOCK(m_wallet.cs_wallet); + m_wallet.AddDestData(dest, "misc", "val_misc"); + m_wallet.AddDestData(dest, "rr0", "val_rr0"); + m_wallet.AddDestData(dest, "rr1", "val_rr1"); - auto values = pwalletMain->GetDestValues("rr"); + auto values = m_wallet.GetDestValues("rr"); BOOST_CHECK_EQUAL(values.size(), 2); BOOST_CHECK_EQUAL(values[0], "val_rr0"); BOOST_CHECK_EQUAL(values[1], "val_rr1"); @@ -608,10 +606,9 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - ::bitdb.MakeMock(); g_address_type = OUTPUT_TYPE_DEFAULT; g_change_type = OUTPUT_TYPE_DEFAULT; - wallet.reset(new CWallet(std::unique_ptr<CWalletDBWrapper>(new CWalletDBWrapper(&bitdb, "wallet_test.dat")))); + wallet = MakeUnique<CWallet>("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); AddKey(*wallet, coinbaseKey); @@ -623,8 +620,6 @@ public: ~ListCoinsTestingSetup() { wallet.reset(); - ::bitdb.Flush(true); - ::bitdb.Reset(); } CWalletTx& AddTx(CRecipient recipient) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bb7be2df33..0c468878e1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5,7 +5,6 @@ #include <wallet/wallet.h> -#include <base58.h> #include <checkpoints.h> #include <chain.h> #include <wallet/coincontrol.h> @@ -14,6 +13,7 @@ #include <fs.h> #include <wallet/init.h> #include <key.h> +#include <key_io.h> #include <keystore.h> #include <validation.h> #include <net.h> @@ -45,7 +45,6 @@ OutputType g_address_type = OUTPUT_TYPE_NONE; OutputType g_change_type = OUTPUT_TYPE_NONE; bool g_wallet_allow_fallback_fee = true; //<! will be defined via chainparams -const char * DEFAULT_WALLET_DAT = "wallet.dat"; const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; /** @@ -3908,16 +3907,17 @@ std::vector<std::string> CWallet::GetDestValues(const std::string& prefix) const return values; } -CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) +CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path) { + const std::string& walletFile = name; + // needed to restore wallet transaction meta data after -zapwallettxes std::vector<CWalletTx> vWtx; if (gArgs.GetBoolArg("-zapwallettxes", false)) { uiInterface.InitMessage(_("Zapping all transactions from wallet...")); - std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile)); - std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(std::move(dbw)); + std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(name, CWalletDBWrapper::Create(path)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); @@ -3929,8 +3929,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *walletInstance = new CWallet(std::move(dbw)); + CWallet *walletInstance = new CWallet(name, CWalletDBWrapper::Create(path)); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DB_LOAD_OK) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4db45f16ef..3e2d1794d8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -15,6 +15,7 @@ #include <validationinterface.h> #include <script/ismine.h> #include <script/sign.h> +#include <util.h> #include <wallet/crypter.h> #include <wallet/walletdb.h> #include <wallet/rpcwallet.h> @@ -67,8 +68,6 @@ static const bool DEFAULT_WALLET_RBF = false; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; -extern const char * DEFAULT_WALLET_DAT; - static const int64_t TIMESTAMP_MIN = 0; class CBlockIndex; @@ -737,6 +736,14 @@ private: */ bool AddWatchOnly(const CScript& dest) override; + /** + * Wallet filename from wallet=<path> command line or config option. + * Used in debug logs and to send RPCs to the right wallet instance when + * more than one wallet is loaded. + */ + std::string m_name; + + /** Internal database handle. */ std::unique_ptr<CWalletDBWrapper> dbw; /** @@ -768,14 +775,7 @@ public: /** Get a name for this wallet for logging/debugging purposes. */ - std::string GetName() const - { - if (dbw) { - return dbw->GetName(); - } else { - return "dummy"; - } - } + const std::string& GetName() const { return m_name; } void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool); @@ -789,14 +789,8 @@ public: MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID; - // Create wallet with dummy database handle - CWallet(): dbw(new CWalletDBWrapper()) - { - SetNull(); - } - - // Create wallet with passed-in database handle - explicit CWallet(std::unique_ptr<CWalletDBWrapper> dbw_in) : dbw(std::move(dbw_in)) + /** Construct wallet with specified name and database implementation. */ + CWallet(std::string name, std::unique_ptr<CWalletDBWrapper> dbw) : m_name(std::move(name)), dbw(std::move(dbw)) { SetNull(); } @@ -1116,7 +1110,7 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static CWallet* CreateWalletFromFile(const std::string walletFile); + static CWallet* CreateWalletFromFile(const std::string& name, const fs::path& path); /** * Wallet post-init setup diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index dd6835a06f..0b0880a2ba 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -5,10 +5,10 @@ #include <wallet/walletdb.h> -#include <base58.h> #include <consensus/tx_verify.h> #include <consensus/validation.h> #include <fs.h> +#include <key_io.h> #include <protocol.h> #include <serialize.h> #include <sync.h> @@ -771,16 +771,16 @@ void MaybeCompactWalletDB() // // Try to (very carefully!) recover wallet file if there is a problem. // -bool CWalletDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) +bool CWalletDB::Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) { - return CDB::Recover(filename, callbackDataIn, recoverKVcallback, out_backup_filename); + return CDB::Recover(wallet_path, callbackDataIn, recoverKVcallback, out_backup_filename); } -bool CWalletDB::Recover(const std::string& filename, std::string& out_backup_filename) +bool CWalletDB::Recover(const fs::path& wallet_path, std::string& out_backup_filename) { // recover without a key filter callback // results in recovering all record types - return CWalletDB::Recover(filename, nullptr, nullptr, out_backup_filename); + return CWalletDB::Recover(wallet_path, nullptr, nullptr, out_backup_filename); } bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) @@ -806,14 +806,14 @@ bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDa return true; } -bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) +bool CWalletDB::VerifyEnvironment(const fs::path& wallet_path, std::string& errorStr) { - return CDB::VerifyEnvironment(walletFile, walletDir, errorStr); + return CDB::VerifyEnvironment(wallet_path, errorStr); } -bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr) +bool CWalletDB::VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr) { - return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, CWalletDB::Recover); + return CDB::VerifyDatabaseFile(wallet_path, warningStr, errorStr, CWalletDB::Recover); } bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 3691cfcb57..7d754c7284 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -218,17 +218,17 @@ public: DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut); /* Try to (very carefully!) recover wallet database (with a possible key type filter) */ - static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + static bool Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* Recover convenience-function to bypass the key filter callback, called when verify fails, recovers everything */ - static bool Recover(const std::string& filename, std::string& out_backup_filename); + static bool Recover(const fs::path& wallet_path, std::string& out_backup_filename); /* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */ static bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr); + static bool VerifyEnvironment(const fs::path& wallet_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr); + static bool VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr); //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 61abba8082..c6cec0596b 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -37,13 +37,13 @@ class ConfArgsTest(BitcoinTestFramework): os.mkdir(new_data_dir) self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) self.stop_node(0) - assert os.path.isfile(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) + assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf os.mkdir(new_data_dir_2) self.nodes[0].datadir = new_data_dir_2 self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file, '-wallet=w2']) - assert os.path.isfile(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) + assert os.path.exists(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) if __name__ == '__main__': ConfArgsTest().main() diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 3e87f6d33f..8440f13a0d 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -296,8 +296,10 @@ class RESTTest (BitcoinTestFramework): # check that there are our submitted transactions in the TX memory pool json_string = http_get_call(url.hostname, url.port, '/rest/mempool/contents'+self.FORMAT_SEPARATOR+'json') json_obj = json.loads(json_string) - for tx in txs: + for i, tx in enumerate(txs): assert_equal(tx in json_obj, True) + assert_equal(json_obj[tx]['spentby'], txs[i+1:i+2]) + assert_equal(json_obj[tx]['depends'], txs[i-1:i]) # now mine the transactions newblockhash = self.nodes[1].generate(1) diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 23797d83db..8880db8002 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -47,7 +47,7 @@ class MempoolPackagesTest(BitcoinTestFramework): value = sent_value chain.append(txid) - # Check mempool has MAX_ANCESTORS transactions in it, and descendant + # Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor # count and fees should look correct mempool = self.nodes[0].getrawmempool(True) assert_equal(len(mempool), MAX_ANCESTORS) @@ -55,6 +55,10 @@ class MempoolPackagesTest(BitcoinTestFramework): descendant_fees = 0 descendant_size = 0 + ancestor_size = sum([mempool[tx]['size'] for tx in mempool]) + ancestor_count = MAX_ANCESTORS + ancestor_fees = sum([mempool[tx]['fee'] for tx in mempool]) + descendants = [] ancestors = list(chain) for x in reversed(chain): @@ -71,14 +75,43 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(mempool[x]['descendantsize'], descendant_size) descendant_count += 1 + # Check that ancestor calculations are correct + assert_equal(mempool[x]['ancestorcount'], ancestor_count) + assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN) + assert_equal(mempool[x]['ancestorsize'], ancestor_size) + ancestor_size -= mempool[x]['size'] + ancestor_fees -= mempool[x]['fee'] + ancestor_count -= 1 + + # Check that parent/child list is correct + assert_equal(mempool[x]['spentby'], descendants[-1:]) + assert_equal(mempool[x]['depends'], ancestors[-2:-1]) + # Check that getmempooldescendants is correct assert_equal(sorted(descendants), sorted(self.nodes[0].getmempooldescendants(x))) + + # Check getmempooldescendants verbose output is correct + for descendant, dinfo in self.nodes[0].getmempooldescendants(x, True).items(): + assert_equal(dinfo['depends'], [chain[chain.index(descendant)-1]]) + if dinfo['descendantcount'] > 1: + assert_equal(dinfo['spentby'], [chain[chain.index(descendant)+1]]) + else: + assert_equal(dinfo['spentby'], []) descendants.append(x) # Check that getmempoolancestors is correct ancestors.remove(x) assert_equal(sorted(ancestors), sorted(self.nodes[0].getmempoolancestors(x))) + # Check that getmempoolancestors verbose output is correct + for ancestor, ainfo in self.nodes[0].getmempoolancestors(x, True).items(): + assert_equal(ainfo['spentby'], [chain[chain.index(ancestor)+1]]) + if ainfo['ancestorcount'] > 1: + assert_equal(ainfo['depends'], [chain[chain.index(ancestor)-1]]) + else: + assert_equal(ainfo['depends'], []) + + # Check that getmempoolancestors/getmempooldescendants correctly handle verbose=true v_ancestors = self.nodes[0].getmempoolancestors(chain[-1], True) assert_equal(len(v_ancestors), len(chain)-1) @@ -100,7 +133,7 @@ class MempoolPackagesTest(BitcoinTestFramework): for x in chain: ancestor_fees += mempool[x]['fee'] assert_equal(mempool[x]['ancestorfees'], ancestor_fees * COIN + 1000) - + # Undo the prioritisetransaction for later tests self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=-1000) @@ -149,6 +182,7 @@ class MempoolPackagesTest(BitcoinTestFramework): vout = utxo[1]['vout'] transaction_package = [] + tx_children = [] # First create one parent tx with 10 children (txid, sent_value) = self.chain_transaction(self.nodes[0], txid, vout, value, fee, 10) parent_transaction = txid @@ -159,11 +193,17 @@ class MempoolPackagesTest(BitcoinTestFramework): for i in range(MAX_DESCENDANTS - 1): utxo = transaction_package.pop(0) (txid, sent_value) = self.chain_transaction(self.nodes[0], utxo['txid'], utxo['vout'], utxo['amount'], fee, 10) + if utxo['txid'] is parent_transaction: + tx_children.append(txid) for j in range(10): transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value}) mempool = self.nodes[0].getrawmempool(True) assert_equal(mempool[parent_transaction]['descendantcount'], MAX_DESCENDANTS) + assert_equal(sorted(mempool[parent_transaction]['spentby']), sorted(tx_children)) + + for child in tx_children: + assert_equal(mempool[child]['depends'], [parent_transaction]) # Sending one more chained transaction will fail utxo = transaction_package.pop(0) @@ -232,7 +272,7 @@ class MempoolPackagesTest(BitcoinTestFramework): signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) txid = self.nodes[0].sendrawtransaction(signedtx['hex']) sync_mempools(self.nodes) - + # Now try to disconnect the tip on each node... self.nodes[1].invalidateblock(self.nodes[1].getbestblockhash()) self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 17f0967219..53748df915 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -66,7 +66,9 @@ class MempoolPersistTest(BitcoinTestFramework): self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.") self.stop_nodes() - self.start_node(1) # Give this one a head-start, so we can be "extra-sure" that it didn't load anything later + # Give this node a head-start, so we can be "extra-sure" that it didn't load anything later + # Also don't store the mempool, to keep the datadir clean + self.start_node(1, extra_args=["-persistmempool=0"]) self.start_node(0) self.start_node(2) # Give bitcoind a second to reload the mempool diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py index 05433c7e24..d43c2cd5d0 100755 --- a/test/functional/rpc_bind.py +++ b/test/functional/rpc_bind.py @@ -14,6 +14,7 @@ from test_framework.netutil import * class RPCBindTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True + self.bind_to_localhost_only = False self.num_nodes = 1 def setup_network(self): diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 96fe283347..36d1a2f856 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -9,7 +9,6 @@ Roughly based on http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-l import sys import socket -import fcntl import struct import array import os @@ -90,6 +89,8 @@ def all_interfaces(): ''' Return all interfaces that are up ''' + import fcntl # Linux only, so only import when required + is_64bits = sys.maxsize > 2**32 struct_size = 40 if is_64bits else 32 s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ecb91b315e..86c1150abd 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -63,6 +63,7 @@ class BitcoinTestFramework(): self.nodes = [] self.mocktime = 0 self.supports_cli = False + self.bind_to_localhost_only = True self.set_test_params() assert hasattr(self, "num_nodes"), "Test must set self.num_nodes in set_test_params()" @@ -215,15 +216,19 @@ class BitcoinTestFramework(): def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None): """Instantiate TestNode objects""" - + if self.bind_to_localhost_only: + extra_confs = [["bind=127.0.0.1"]] * num_nodes + else: + extra_confs = [[]] * num_nodes if extra_args is None: extra_args = [[]] * num_nodes if binary is None: binary = [None] * num_nodes + assert_equal(len(extra_confs), num_nodes) assert_equal(len(extra_args), num_nodes) assert_equal(len(binary), num_nodes) for i in range(num_nodes): - self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli)) + self.nodes.append(TestNode(i, self.options.tmpdir, rpchost=rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli)) def start_node(self, i, *args, **kwargs): """Start a bitcoind""" @@ -395,7 +400,7 @@ class BitcoinTestFramework(): args = [os.getenv("BITCOIND", "bitcoind"), "-datadir=" + datadir] if i > 0: args.append("-connect=127.0.0.1:" + str(p2p_port(0))) - self.nodes.append(TestNode(i, self.options.cachedir, extra_args=[], rpchost=None, timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) + self.nodes.append(TestNode(i, self.options.cachedir, extra_conf=["bind=127.0.0.1"], extra_args=[],rpchost=None, timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) self.nodes[i].args = args self.start_node(i) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 93a052f785..86e44e4c97 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -16,6 +16,7 @@ import time from .authproxy import JSONRPCException from .util import ( + append_config, assert_equal, get_rpc_proxy, rpc_url, @@ -42,7 +43,7 @@ class TestNode(): To make things easier for the test writer, any unrecognised messages will be dispatched to the RPC connection.""" - def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir, use_cli=False): + def __init__(self, i, dirname, rpchost, timewait, binary, stderr, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False): self.index = i self.datadir = os.path.join(dirname, "node" + str(i)) self.rpchost = rpchost @@ -57,6 +58,8 @@ class TestNode(): self.binary = binary self.stderr = stderr self.coverage_dir = coverage_dir + if extra_conf != None: + append_config(dirname, i, extra_conf) # Most callers will just need to add extra args to the standard list below. # For those callers that need more flexibity, they can just set the args property directly. # Note that common args are set in the config file (see initialize_datadir) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index e9e7bbd8e4..a4b8d5af02 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -300,6 +300,12 @@ def initialize_datadir(dirname, n): def get_datadir_path(dirname, n): return os.path.join(dirname, "node" + str(n)) +def append_config(dirname, n, options): + datadir = get_datadir_path(dirname, n) + with open(os.path.join(datadir, "bitcoin.conf"), 'a', encoding='utf8') as f: + for option in options: + f.write(option + "\n") + def get_auth_cookie(datadir): user = None password = None diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 59eae78cbd..997f67ec7e 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -7,7 +7,10 @@ import os from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import (assert_equal, assert_raises_rpc_error) +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) def read_dump(file_name, addrs, script_addrs, hd_master_addr_old): @@ -84,11 +87,12 @@ class WalletDumpTest(BitcoinTestFramework): # longer than the default 30 seconds due to an expensive # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in # the test often takes even longer. - self.add_nodes(self.num_nodes, self.extra_args, timewait=60) + self.add_nodes(self.num_nodes, extra_args=self.extra_args, timewait=60) self.start_nodes() def run_test (self): - tmpdir = self.options.tmpdir + wallet_unenc_dump = os.path.join(self.nodes[0].datadir, "wallet.unencrypted.dump") + wallet_enc_dump = os.path.join(self.nodes[0].datadir, "wallet.encrypted.dump") # generate 20 addresses to compare against the dump # but since we add a p2sh-p2wpkh address for the first pubkey in the @@ -108,11 +112,11 @@ class WalletDumpTest(BitcoinTestFramework): script_addrs = [witness_addr, multisig_addr] # dump unencrypted wallet - result = self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.unencrypted.dump") - assert_equal(result['filename'], os.path.abspath(tmpdir + "/node0/wallet.unencrypted.dump")) + result = self.nodes[0].dumpwallet(wallet_unenc_dump) + assert_equal(result['filename'], wallet_unenc_dump) found_addr, found_script_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc, witness_addr_ret = \ - read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, script_addrs, None) + read_dump(wallet_unenc_dump, addrs, script_addrs, None) assert_equal(found_addr, test_addr_count) # all keys must be in the dump assert_equal(found_script_addr, 2) # all scripts must be in the dump assert_equal(found_addr_chg, 50) # 50 blocks where mined @@ -125,10 +129,10 @@ class WalletDumpTest(BitcoinTestFramework): self.nodes[0].walletpassphrase('test', 10) # Should be a no-op: self.nodes[0].keypoolrefill() - self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") + self.nodes[0].dumpwallet(wallet_enc_dump) found_addr, found_script_addr, found_addr_chg, found_addr_rsv, _, witness_addr_ret = \ - read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, script_addrs, hd_master_addr_unenc) + read_dump(wallet_enc_dump, addrs, script_addrs, hd_master_addr_unenc) assert_equal(found_addr, test_addr_count) assert_equal(found_script_addr, 2) assert_equal(found_addr_chg, 90*2 + 50) # old reserve keys are marked as change now @@ -136,7 +140,7 @@ class WalletDumpTest(BitcoinTestFramework): assert_equal(witness_addr_ret, witness_addr) # Overwriting should fail - assert_raises_rpc_error(-8, "already exists", self.nodes[0].dumpwallet, tmpdir + "/node0/wallet.unencrypted.dump") + assert_raises_rpc_error(-8, "already exists", lambda: self.nodes[0].dumpwallet(wallet_enc_dump)) # Restart node with new wallet, and test importwallet self.stop_node(0) @@ -146,11 +150,11 @@ class WalletDumpTest(BitcoinTestFramework): result = self.nodes[0].getaddressinfo(multisig_addr) assert(result['ismine'] == False) - self.nodes[0].importwallet(os.path.abspath(tmpdir + "/node0/wallet.unencrypted.dump")) + self.nodes[0].importwallet(wallet_unenc_dump) # Now check IsMine is true result = self.nodes[0].getaddressinfo(multisig_addr) assert(result['ismine'] == True) if __name__ == '__main__': - WalletDumpTest().main () + WalletDumpTest().main() diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 3288ce4b60..bfd4638481 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -124,7 +124,7 @@ class ImportRescanTest(BitcoinTestFramework): if import_node.prune: extra_args[i] += ["-prune=1"] - self.add_nodes(self.num_nodes, extra_args) + self.add_nodes(self.num_nodes, extra_args=extra_args) self.start_nodes() for i in range(1, self.num_nodes): connect_nodes(self.nodes[i], 0) diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index 1f2b3c8aa7..01c9899c71 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -45,10 +45,44 @@ class ReceivedByTest(BitcoinTestFramework): assert_array_result(self.nodes[1].listreceivedbyaddress(11), {"address": addr}, {}, True) # Empty Tx - addr = self.nodes[1].getnewaddress() + empty_addr = self.nodes[1].getnewaddress() assert_array_result(self.nodes[1].listreceivedbyaddress(0, True), - {"address": addr}, - {"address": addr, "account": "", "amount": 0, "confirmations": 0, "txids": []}) + {"address": empty_addr}, + {"address": empty_addr, "account": "", "amount": 0, "confirmations": 0, "txids": []}) + + #Test Address filtering + #Only on addr + expected = {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":10, "txids":[txid,]} + res = self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True, address_filter=addr) + assert_array_result(res, {"address":addr}, expected) + assert_equal(len(res), 1) + #Error on invalid address + assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, include_watchonly=True, address_filter="bamboozling") + #Another address receive money + res = self.nodes[1].listreceivedbyaddress(0, True, True) + assert_equal(len(res), 2) #Right now 2 entries + other_addr = self.nodes[1].getnewaddress() + txid2 = self.nodes[0].sendtoaddress(other_addr, 0.1) + self.nodes[0].generate(1) + self.sync_all() + #Same test as above should still pass + expected = {"address":addr, "account":"", "amount":Decimal("0.1"), "confirmations":11, "txids":[txid,]} + res = self.nodes[1].listreceivedbyaddress(0, True, True, addr) + assert_array_result(res, {"address":addr}, expected) + assert_equal(len(res), 1) + #Same test as above but with other_addr should still pass + expected = {"address":other_addr, "account":"", "amount":Decimal("0.1"), "confirmations":1, "txids":[txid2,]} + res = self.nodes[1].listreceivedbyaddress(0, True, True, other_addr) + assert_array_result(res, {"address":other_addr}, expected) + assert_equal(len(res), 1) + #Should be two entries though without filter + res = self.nodes[1].listreceivedbyaddress(0, True, True) + assert_equal(len(res), 3) #Became 3 entries + + #Not on random addr + other_addr = self.nodes[0].getnewaddress() # note on node[0]! just a random addr + res = self.nodes[1].listreceivedbyaddress(0, True, True, other_addr) + assert_equal(len(res), 0) self.log.info("getreceivedbyaddress Test") diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index b07e451667..378c06ee59 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -16,7 +16,6 @@ class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []] self.supports_cli = True def run_test(self): @@ -26,9 +25,42 @@ class MultiWalletTest(BitcoinTestFramework): wallet_dir = lambda *p: data_dir('wallets', *p) wallet = lambda name: node.get_wallet_rpc(name) - assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) - + # check wallet.dat is created self.stop_nodes() + assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) + + # create symlink to verify wallet directory path can be referenced + # through symlink + os.mkdir(wallet_dir('w7')) + os.symlink('w7', wallet_dir('w7_symlink')) + + # rename wallet.dat to make sure plain wallet file paths (as opposed to + # directory paths) can be loaded + os.rename(wallet_dir("wallet.dat"), wallet_dir("w8")) + + # restart node with a mix of wallet names: + # w1, w2, w3 - to verify new wallets created when non-existing paths specified + # w - to verify wallet name matching works when one wallet path is prefix of another + # sub/w5 - to verify relative wallet path is created correctly + # extern/w6 - to verify absolute wallet path is created correctly + # w7_symlink - to verify symlinked wallet path is initialized correctly + # w8 - to verify existing wallet file is loaded correctly + # '' - to verify default wallet file is created correctly + wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'w7_symlink', 'w8', ''] + extra_args = ['-wallet={}'.format(n) for n in wallet_names] + self.start_node(0, extra_args) + assert_equal(set(node.listwallets()), set(wallet_names)) + + # check that all requested wallets were created + self.stop_node(0) + for wallet_name in wallet_names: + if os.path.isdir(wallet_dir(wallet_name)): + assert_equal(os.path.isfile(wallet_dir(wallet_name, "wallet.dat")), True) + else: + assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) + + # should not initialize if wallet path can't be created + self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) @@ -37,17 +69,13 @@ class MultiWalletTest(BitcoinTestFramework): # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') - # should not initialize if wallet file is a directory - os.mkdir(wallet_dir('w11')) - self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.') - # should not initialize if one wallet is a copy of another - shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) - self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) + self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(wallet_dir('w1'), wallet_dir('w12')) - self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') + os.symlink('w8', wallet_dir('w8_symlink')) + self.assert_start_raises_init_error(0, ['-wallet=w8_symlink'], 'Invalid -wallet path') # should not initialize if the specified walletdir does not exist self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') @@ -77,15 +105,17 @@ class MultiWalletTest(BitcoinTestFramework): self.restart_node(0, ['-walletdir='+competing_wallet_dir]) self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') - self.restart_node(0, self.extra_args[0]) + self.restart_node(0, extra_args) - w1 = wallet("w1") - w2 = wallet("w2") - w3 = wallet("w3") - w4 = wallet("w") + wallets = [wallet(w) for w in wallet_names] wallet_bad = wallet("bad") - w1.generate(1) + # check wallet names and balances + wallets[0].generate(1) + for wallet_name, wallet in zip(wallet_names, wallets): + info = wallet.getwalletinfo() + assert_equal(info['immature_balance'], 50 if wallet is wallets[0] else 0) + assert_equal(info['walletname'], wallet_name) # accessing invalid wallet fails assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo) @@ -93,24 +123,7 @@ class MultiWalletTest(BitcoinTestFramework): # accessing wallet RPC without using wallet endpoint fails assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo) - # check w1 wallet balance - w1_info = w1.getwalletinfo() - assert_equal(w1_info['immature_balance'], 50) - w1_name = w1_info['walletname'] - assert_equal(w1_name, "w1") - - # check w2 wallet balance - w2_info = w2.getwalletinfo() - assert_equal(w2_info['immature_balance'], 0) - w2_name = w2_info['walletname'] - assert_equal(w2_name, "w2") - - w3_name = w3.getwalletinfo()['walletname'] - assert_equal(w3_name, "w3") - - w4_name = w4.getwalletinfo()['walletname'] - assert_equal(w4_name, "w") - + w1, w2, w3, w4, *_ = wallets w1.generate(101) assert_equal(w1.getbalance(), 100) assert_equal(w2.getbalance(), 0) |