diff options
83 files changed, 1836 insertions, 773 deletions
diff --git a/.gitignore b/.gitignore index 60c26dae8b..ff297fbeca 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,7 @@ src/qt/test/moc*.cpp Makefile bitcoin-qt Bitcoin-Qt.app +background.tiff* # Unit-tests Makefile.test diff --git a/.travis.yml b/.travis.yml index 0de7ca6f75..4bb0b1f9fb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: required dist: trusty os: linux -language: generic +language: minimal cache: directories: - depends/built @@ -40,7 +40,6 @@ env: before_install: - export PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/python/d' | tr "\n" ":" | sed "s|::|:|g") - - export PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/pyenv/d' | tr "\n" ":" | sed "s|::|:|g") install: - if [ -n "$DPKG_ADD_ARCH" ]; then sudo dpkg --add-architecture "$DPKG_ADD_ARCH" ; fi - if [ -n "$PACKAGES" ]; then travis_retry sudo apt-get update; fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aed6d79542..9f95d3f818 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,9 +24,9 @@ facilitates social contribution, easy testing and peer review. To contribute a patch, the workflow is as follows: - - Fork repository - - Create topic branch - - Commit patches + 1. Fork repository + 1. Create topic branch + 1. Commit patches The project coding conventions in the [developer notes](doc/developer-notes.md) must be adhered to. @@ -42,8 +42,8 @@ in init.cpp") in which case a single title line is sufficient. Commit messages s helpful to people reading your code in the future, so explain the reasoning for your decisions. Further explanation [here](http://chris.beams.io/posts/git-commit/). -If a particular commit references another issue, please add the reference, for -example `refs #1234`, or `fixes #4321`. Using the `fixes` or `closes` keywords +If a particular commit references another issue, please add the reference. For +example: `refs #1234` or `fixes #4321`. Using the `fixes` or `closes` keywords will cause the corresponding issue to be closed when the pull request is merged. Please refer to the [Git manual](https://git-scm.com/doc) for more information @@ -85,7 +85,7 @@ Note that translations should not be submitted as pull requests, please see [Translation Process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) for more information on helping with translations. -If a pull request is specifically not to be considered for merging (yet) please +If a pull request is not to be considered for merging (yet), please prefix the title with [WIP] or use [Tasks Lists](https://help.github.com/articles/basic-writing-and-formatting-syntax/#task-lists) in the body of the pull request to indicate tasks are pending. diff --git a/Makefile.am b/Makefile.am index 3b62a10603..a7092bb334 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,7 +9,6 @@ SUBDIRS += doc/man endif .PHONY: deploy FORCE -GZIP_ENV="-9n" export PYTHONPATH if BUILD_BITCOIN_LIBS @@ -44,6 +43,9 @@ DIST_CONTRIB = $(top_srcdir)/contrib/bitcoin-cli.bash-completion \ $(top_srcdir)/contrib/bitcoind.bash-completion \ $(top_srcdir)/contrib/init \ $(top_srcdir)/contrib/rpm +DIST_SHARE = \ + $(top_srcdir)/share/genbuild.sh \ + $(top_srcdir)/share/rpcuser BIN_CHECKS=$(top_srcdir)/contrib/devtools/symbol-check.py \ $(top_srcdir)/contrib/devtools/security-check.py @@ -213,7 +215,7 @@ endif dist_noinst_SCRIPTS = autogen.sh -EXTRA_DIST = $(top_srcdir)/share/genbuild.sh test/functional/test_runner.py test/functional $(DIST_CONTRIB) $(DIST_DOCS) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) +EXTRA_DIST = $(DIST_SHARE) test/functional/test_runner.py test/functional $(DIST_CONTRIB) $(DIST_DOCS) $(WINDOWS_PACKAGING) $(OSX_PACKAGING) $(BIN_CHECKS) EXTRA_DIST += \ test/util/bitcoin-util-test.py \ diff --git a/configure.ac b/configure.ac index 81c84a8af4..8e5561243a 100644 --- a/configure.ac +++ b/configure.ac @@ -162,7 +162,7 @@ AC_ARG_ENABLE([ccache], AC_ARG_ENABLE([lcov], [AS_HELP_STRING([--enable-lcov], [enable lcov testing (default is no)])], - [use_lcov=yes], + [use_lcov=$enableval], [use_lcov=no]) AC_ARG_ENABLE([lcov-branch-coverage], @@ -241,6 +241,7 @@ if test "x$enable_werror" = "xyes"; then AC_MSG_ERROR("enable-werror set but -Werror is not usable") fi AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Werror=thread-safety-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety-analysis"],,[[$CXXFLAG_WERROR]]) fi if test "x$CXXFLAGS_overridden" = "xno"; then @@ -249,6 +250,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wformat],[CXXFLAGS="$CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wvla],[CXXFLAGS="$CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat-security],[CXXFLAGS="$CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Wthread-safety-analysis],[CXXFLAGS="$CXXFLAGS -Wthread-safety-analysis"],,[[$CXXFLAG_WERROR]]) ## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all ## unknown options if any other warning is produced. Test the -Wfoo case, and diff --git a/contrib/gitian-keys/sjors-key.pgp b/contrib/gitian-keys/sjors-key.pgp new file mode 100644 index 0000000000..2b5acc82aa --- /dev/null +++ b/contrib/gitian-keys/sjors-key.pgp @@ -0,0 +1,76 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQINBFWSwMoBEADG31O8+ex+xpgzVKQgF4iVRE5uBPT0+GM6FnwqIIhXVKiBLQh8 +YDhhgk6joh+vsLrFzKZ9kXwoiHN8y/AiNCQ0xjAUdpznD5xvHAaGIAlT/sodRNT+ +869WgT9G1uiVp0P4ucEeilmhCn9o51LqkS3roXkj0ec52b1pslUl2WKdu1ZD+Bj4 +3/oVZm7mmjkDwl0RHJQmqlK0bunq0jlVlgH5sdQfmLbCZaq3LhVPf73zt5qHH+J6 +ZbU7A4cqm2eN5SyH+Nno+cq3+vXmvVI+x/jPe/dPDCXaGWf5fWI/Lbk/mMP7JAl1 +6X44CN+hZHUnNuzeZt2/ROWZ0s0JJcjQkSe9noUQedjBAHX82s886vsFzOHvDtul +EuV/XAjUlkhMbhZkZaIq9ucqHmUBI4+OcFEIbbKc9TrKtJe+CYuWTNlomVk/iFr8 +zSm/S64NiqKi/BeQGgcsDZIaJDYfDP83esOOaaxFswHnJNtHnU1PwntrJtXft0dK +ydtlQZ6r96SYxLDTeGfC2SNk0zbnKAGvjj04vzQeN+JSRZ75tNKmgdbJdNL8wvPh +879TpCwMhNDvSRG+YqCe6whaJV76a+Doxg48HCJYaj6bnRn41/QGJEyL31I8l/7S +YsLLmAEbqwG7erYi7WZS3cRrGJI8RwohGMZf7yraqoaOgMKmtE/Sq0tLtwARAQAB +tCNTam9ycyBQcm92b29zdCA8c2pvcnNAc3Byb3Zvb3N0Lm5sPokCQAQTAQoAKgIb +AwUJB4YfgAULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAUCWWXcoAIZAQAKCRBX/5vb +zDAQCeYJD/47XDMfEMg4g4spo7k92XsNkvjlAhWvvxd+kxow/V8c64WQXody32FZ +HRSmK8dVjf9mIJMKkX4lpKpim7cQxsdTcorcdu+yk4TK+Wah61vsMhbSSllfHs1U ++q8jYMGnXTD+CY0aeTMrTfJcR2yN98jmNSWIL1qWmJ51RSTL6BQKb6eYtR7pWRkW +uMR6oFC09Db4fiKa4zhH81+/t0g+6pMY391gSluaS+OfNqGORCo+/IdG5IDzh5Vp +f19qXjd5oMsZQf6/P4b4XUktgl8RVRcNzdYGoXpcd8LpeHtEOh5I93ODmCwqd67b +YDlhDNN7iGhPndPEF6P4CNO/rXLPCZyMhRyt1dflu0KPCr+0AgR31cdhH/p7eCyj +FTE9gUgUHOG9OHdRoVXrwHYXwAiDBr2pp2giLpBsAwa4d2hXNDJ6wfMMCSOXKQlS +lHq06y/v/049DammkqW0XnEsU4qvsdteZ0jQu7Ob3LyGoytBIj8fn1OioT21W7wc +ns3/Tt4cQsn2ICBYB4PzqwkvGUp7fDwwHYw7rq6kvCEVDUDWMtVgQ8kjsh2OoU75 +eeteM1Q1fV06Wfn2Qct9bn0NKRGrA8mm3lrCWYCeGqJeBvC6kna1QgV53vYRLJod +w3Ql4+M9tUIi9uiGLvVaGZWO9wU1EwL+EAO+6D85h6QiJN7H8gcwUokCPQQTAQoA +JwUCVZLAygIbAwUJB4YfgAULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAAKCRBX/5vb +zDAQCauuD/9IDWhf/fTseA1Rt5i4gwK+8dCQjTlRS2cZtGc2aMX8w5XruDWnna1P +Mj/aVUncDrprRx9rxgEqIDyPheuJ6r7v6D8GjrpAjcG/BPNFtPaxQccbZbAYdzoj +Rrs+ttVIqS+wO7qLmQkKA4oGRMmgYh3VX8EBZNcvxaGCcJx0PfoqS8cPXTnCRHcg +Wx6kaFyuWtrTX+kCpDraB1KGtxedR4rzuOtUOLoqFOOfsQuOxPlKNNr9Zjc8x2o4 +5TtwbuoEog8FIEttY6NOywpsSsvYvNB4gq1fxO49H0pQopmJlOMatMH6IRT7BJJZ +cOoHOh4X/zItOJZtuCOT4u+Y2XOuyLcW83X5ymIR3ZCxedsLzjyiCWm61/znJVON +Ws8I+gShbvauahBCB9rOHqwM0QioJMc36hUPB21KghQS8RJpGwmtk1WhFFMtAsSJ +w+wRfy2d6u+lSGdlA+2hEyKVm/DNQMDCQVFx3lQ6YBwAwkSiLMylrPKvs56fUjRr +74qoPyDxuRMC+q+TThHsy5O9r31G+Dc3+H5k4iTk354Jshjltx/k2O732e9Vxyar +/U5P7UZqHHuJKXDihUFrcJZq+gk8sGEWzGG/wocce7ezrTnHqR8YA04BTA4PXQqZ +4N42f422YYGIH/3Nm6drQkbigekLw6wx+NrxtTsYg4eCtSsaUd/RjLQhU2pvcnMg +UHJvdm9vc3QgPHNqb3JzQGZyZWVkb20ubmw+iQI9BBMBCgAnBQJZZdyfAhsDBQkH +hh+ABQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJEFf/m9vMMBAJEsIQAK4ihgRB +05QqETpWNeV/XSGBHQINuwwEDz/k8dAJ5Uo6OoSpDULa16fs/EgAV46wTSxfWuci +n2Fc1AWLeLDWOax/NlycL00VDHEwT2PCjcc5uMuwR4RUTciKyByT1u7BFToZ6PyL +mbU6u6whcQejl6Ci2kw0Mu4n4bKTS7OL4/w/EbdfMSpRi8wWmTPMB/aMjtS2Mxi/ +N+yQhJ9pReHADeCBoAjq0cUy+QbzvBwDCK4XWRzF7kiFuA7UW2r7/dX6l31mPfi/ +GLA5+ftPxJ6EH8cxToF70OWiSfhOTleaqZaHUOG0V7wV2lr/bwAYzpVlxeZSCIta +lAA9ZLzUD2hiHYcei6kc/YjIhmlml7O0FK1eBk7+bt5wr0nvWt4Lbha4y5LxBX8C +d7InvB3xUYHz+S5Ul4vp0Rzx97MBL4oX2ltBEDpc1CcOgzv4dcWMG9bbh9/SaI/G +RehAzwkbpVUl9AEUNKO0dNlZUdu8CkehHdPdz5sJyS/9zE0A7yIECDFP9Nrht0nK +MahBijm4K+jOiLOZ2xyfOX1pVWLqIXGQHKjfcD3oI3qvGrQYtxB5Dffb9ACFMpZO +z3jM8h2UAa2/KqA4MZiZG9N6uWHKkIAMMuXWs1s439WePvbQ+5aw/qPUAMyqA3XZ +dkfn8QWaJPR4nRM+McYBYuS4fKK9HRJWQgcQuQINBFWSwMoBEACzmkabZ8oHWJUE +beU7rJF/TMbwV1IFtFxJ/QlY8rE4VnHekPMvkLi/gjx3WY5nmMe+d4JYoK/uPNdt +y5u0QYgH2MB/jebk4gYXCAHIPpU38h9UgHRb6qV8OaqHhmoXvKwyz+1QPzyJpmgg +oCUN+OAroNjl7zhunE7w7EEddFQftfPoGKEUnTjv84QOCuAb46JsYyiNAc3h6okq +74hY7PKCv8IRGclMPjemhBT2LEenn1t4yi7a8W/hjIe44PmQiqQEXR17keqcP/ls +EH9xSST1v/70ieiPqb6zbHGWzjQxqpFUJxRU6OluBCy5pHVd8wfFGYrrbTpoxaUC +jyA2SLr1oZZ9gaGprt6X7FC5gpE5LV9essq3O5wwvoPbyMe1F5uFaxIPhlt55oEu +rwVWecFJ8tSjniF/WSkTcILrOmiQZ4mylXfOP9Wk38seZReCs799KEfKFlXHk89a +Sj3ZvaJQxwVCnvsAsbVKmmHZ5wPt+G2KfhOkkv2A1I/UyeTT7aXvt2vxDqGuG0su +Eo6QknM/2Sr5Uv7BwBeSIQ6llH5ZnqKz34+HjriP8YPWzvsC959GXsxS01dCSvUM +92j5PvTZzf5dt1CWHMeufAY5XIH+nftkRniuScRhJ7xK3tJ7wngg7UvdeZwJWqmK +lJ7GI38V8HIMnd2x28yiGpj1ue6T+QARAQABiQIlBBgBCgAPBQJVksDKAhsMBQkH +hh+AAAoJEFf/m9vMMBAJjeIP/1UBCi6gSXzpGJBLD2u4PcZJjXBJAImZdf1aCqfS +YZBCaA65UrM3uaVa7h8MGAJc9kDjpqHurjDmG3YWf33KvHWYmReQvX43pZmfF12s +X7FZgcCfgZJKKj+ri6oHQonZzUMrecEcAJLLaQoD3Du3iZpETiyRLL7sJ1lZSaCJ +gYKnN4WV5GypvdFvb8vSUBST2h0D6AewGKMNh8ruRlkIxI+YSlywgYIH+O0qNKqW +wBlZc/5f+JZ3hu+cjx/+Zn+w+saIb6SgySg0UzN35b2WM2YzrfQep4ah3NIxuC7e +qzmfV6GnRtuUrBLVJ8qyjif1JSM9tZfinnmAB4/U5Qfc+YYViIXMTljmHWvbokas +tTBfVAw74yWnkv4ZuXf5SkTmGwEMJUOat0TSr085Ck5y394bRepdI1Y+1cdqpwMQ +QmkKyvcBlREQ7Xk1UnDDR3o/2ieVuGGHRp8jmoWBWGq4Cm43fYOlVe+PcaX0tDns +Tmmh2uwEU/TXe5qGil51OlSM7qhAMqhWUIYphSOcdvApNXuiWMfnTdjsNygE4HVh +Jq4efJ/nlx5N+PNAK2GpzeUJQGyxiVsXybq+h8UlvytBsdz1X6ZYzBv1yYwANThU +rMB1s4tMaEugX0aNByLcsxuS4ixd2qzwkYVz25Aeko/U1v2/j2cIRtrTNgja3BKE +N5Ug +=80Es +-----END PGP PUBLIC KEY BLOCK----- diff --git a/contrib/verify-commits/gpg.sh b/contrib/verify-commits/gpg.sh index b01e2a6d39..abd8f5fd9f 100755 --- a/contrib/verify-commits/gpg.sh +++ b/contrib/verify-commits/gpg.sh @@ -46,6 +46,11 @@ for LINE in $(echo "$GPG_RES"); do REVSIG=true GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}" ;; + "[GNUPG:] EXPKEYSIG "*) + [ "$BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG" != 1 ] && exit 1 + REVSIG=true + GOODREVSIG="[GNUPG:] GOODSIG ${LINE#* * *}" + ;; esac done if ! $VALID; then diff --git a/contrib/verify-commits/verify-commits.sh b/contrib/verify-commits/verify-commits.sh index 74b7f38375..7194b040eb 100755 --- a/contrib/verify-commits/verify-commits.sh +++ b/contrib/verify-commits/verify-commits.sh @@ -39,7 +39,7 @@ PREV_COMMIT="" while true; do if [ "$CURRENT_COMMIT" = $VERIFIED_ROOT ]; then echo "There is a valid path from "$CURRENT_COMMIT" to $VERIFIED_ROOT where all commits are signed!" - exit 0; + exit 0 fi if [ "$CURRENT_COMMIT" = $VERIFIED_SHA512_ROOT ]; then diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index 4ea8e30b53..760bb69b15 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -1,10 +1,10 @@ OpenBSD build guide ====================== -(updated for OpenBSD 6.1) +(updated for OpenBSD 6.2) This guide describes how to build bitcoind and command-line utilities on OpenBSD. -As OpenBSD is most common as a server OS, we will not bother with the GUI. +OpenBSD is most commonly used as a server OS, so this guide does not contain instructions for building the GUI. Preparation ------------- @@ -12,10 +12,13 @@ Preparation Run the following as root to install the base dependencies for building: ```bash -pkg_add gmake libtool libevent +pkg_add git gmake libevent libtool pkg_add autoconf # (select highest version, e.g. 2.69) pkg_add automake # (select highest version, e.g. 1.15) -pkg_add python # (select highest version, e.g. 3.5) +pkg_add python # (select highest version, e.g. 3.6) +pkg_add boost + +git clone https://github.com/bitcoin/bitcoin.git ``` See [dependencies.md](dependencies.md) for a complete overview. @@ -23,54 +26,19 @@ See [dependencies.md](dependencies.md) for a complete overview. GCC ------- -The default C++ compiler that comes with OpenBSD 5.9 is g++ 4.2. This version is old (from 2007), and is not able to compile the current version of Bitcoin Core, primarily as it has no C++11 support, but even before there were issues. So here we will be installing a newer compiler: +The default C++ compiler that comes with OpenBSD 6.2 is g++ 4.2.1. This version is old (from 2007), and is not able to compile the current version of Bitcoin Core because it has no C++11 support. We'll install a newer version of GCC: ```bash -pkg_add g++ # (select newest 4.x version, e.g. 4.9.3) -``` - -This compiler will not overwrite the system compiler, it will be installed as `egcc` and `eg++` in `/usr/local/bin`. - -### Building boost - -Do not use `pkg_add boost`! The boost version installed thus is compiled using the `g++` compiler not `eg++`, which will result in a conflict between `/usr/local/lib/libestdc++.so.XX.0` and `/usr/lib/libstdc++.so.XX.0`, resulting in a test crash: - - test_bitcoin:/usr/lib/libstdc++.so.57.0: /usr/local/lib/libestdc++.so.17.0 : WARNING: symbol(_ZN11__gnu_debug17_S_debug_me ssagesE) size mismatch, relink your program - ... - Segmentation fault (core dumped) + pkg_add g++ + ``` -This makes it necessary to build boost, or at least the parts used by Bitcoin Core, manually: - -``` -# Pick some path to install boost to, here we create a directory within the bitcoin directory -BITCOIN_ROOT=$(pwd) -BOOST_PREFIX="${BITCOIN_ROOT}/boost" -mkdir -p $BOOST_PREFIX - -# Fetch the source and verify that it is not tampered with -curl -o boost_1_64_0.tar.bz2 https://netcologne.dl.sourceforge.net/project/boost/boost/1.64.0/boost_1_64_0.tar.bz2 -echo '7bcc5caace97baa948931d712ea5f37038dbb1c5d89b43ad4def4ed7cb683332 boost_1_64_0.tar.bz2' | sha256 -c -# MUST output: (SHA256) boost_1_64_0.tar.bz2: OK -tar -xjf boost_1_64_0.tar.bz2 - -# Boost 1.64 needs one small patch for OpenBSD -cd boost_1_64_0 -# Also here: https://gist.githubusercontent.com/laanwj/bf359281dc319b8ff2e1/raw/92250de8404b97bb99d72ab898f4a8cb35ae1ea3/patch-boost_test_impl_execution_monitor_ipp.patch -patch -p0 < /usr/ports/devel/boost/patches/patch-boost_test_impl_execution_monitor_ipp - -# Build w/ minimum configuration necessary for bitcoin -echo 'using gcc : : eg++ : <cxxflags>"-fvisibility=hidden -fPIC" <linkflags>"" <archiver>"ar" <striper>"strip" <ranlib>"ranlib" <rc>"" : ;' > user-config.jam -config_opts="runtime-link=shared threadapi=pthread threading=multi link=static variant=release --layout=tagged --build-type=complete --user-config=user-config.jam -sNO_BZIP2=1" -./bootstrap.sh --without-icu --with-libraries=chrono,filesystem,program_options,system,thread,test -./b2 -d2 -j2 -d1 ${config_opts} --prefix=${BOOST_PREFIX} stage -./b2 -d0 -j4 ${config_opts} --prefix=${BOOST_PREFIX} install -``` + This compiler will not overwrite the system compiler, it will be installed as `egcc` and `eg++` in `/usr/local/bin`. ### Building BerkeleyDB BerkeleyDB is only necessary for the wallet functionality. To skip this, pass `--disable-wallet` to `./configure`. -See "Berkeley DB" in [build_unix.md](build_unix.md) for instructions on how to build BerkeleyDB 4.8. +See "Berkeley DB" in [build-unix.md](build-unix.md#berkeley-db) for instructions on how to build BerkeleyDB 4.8. You cannot use the BerkeleyDB library from ports, for the same reason as boost above (g++/libstd++ incompatibility). ```bash @@ -98,8 +66,8 @@ The standard ulimit restrictions in OpenBSD are very strict: data(kbytes) 1572864 -This is, unfortunately, no longer enough to compile some `.cpp` files in the project, -at least with gcc 4.9.3 (see issue https://github.com/bitcoin/bitcoin/issues/6658). +This, unfortunately, may no longer be enough to compile some `.cpp` files in the project, +at least with GCC 4.9.4 (see issue [#6658](https://github.com/bitcoin/bitcoin/issues/6658)). If your user is in the `staff` group the limit can be raised with: ulimit -d 3000000 @@ -118,59 +86,32 @@ export AUTOCONF_VERSION=2.69 # replace this with the autoconf version that you i export AUTOMAKE_VERSION=1.15 # replace this with the automake version that you installed ./autogen.sh ``` -Make sure `BDB_PREFIX` and `BOOST_PREFIX` are set to the appropriate paths from the above steps. +Make sure `BDB_PREFIX` is set to the appropriate path from the above steps. To configure with wallet: ```bash -./configure --with-gui=no --with-boost=$BOOST_PREFIX \ - CC=egcc CXX=eg++ CPP=ecpp \ +./configure --with-gui=no CC=egcc CXX=eg++ CPP=ecpp \ BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" BDB_CFLAGS="-I${BDB_PREFIX}/include" ``` To configure without wallet: ```bash -./configure --disable-wallet --with-gui=no --with-boost=$BOOST_PREFIX \ - CC=egcc CXX=eg++ CPP=ecpp +./configure --disable-wallet --with-gui=no CC=egcc CXX=eg++ CPP=ecpp ``` Build and run the tests: ```bash -gmake # can use -jX here for parallelism +gmake # use -jX here for parallelism gmake check ``` -Clang (not currently working) +Clang ------------------------------ -WARNING: This is outdated, needs to be updated for OpenBSD 6.0 and re-tried. - -Using a newer g++ results in linking the new code to a new libstdc++. -Libraries built with the old g++, will still import the old library. -This gives conflicts, necessitating rebuild of all C++ dependencies of the application. - -With clang this can - at least theoretically - be avoided because it uses the -base system's libstdc++. - ```bash -pkg_add llvm boost -``` +pkg_add llvm -```bash ./configure --disable-wallet --with-gui=no CC=clang CXX=clang++ -gmake +gmake # use -jX here for parallelism +gmake check ``` - -However, this does not appear to work. Compilation succeeds, but link fails -with many 'local symbol discarded' errors: - - local symbol 150: discarded in section `.text._ZN10tinyformat6detail14FormatIterator6finishEv' from libbitcoin_util.a(libbitcoin_util_a-random.o) - local symbol 151: discarded in section `.text._ZN10tinyformat6detail14FormatIterator21streamStateFromFormatERSoRjPKcii' from libbitcoin_util.a(libbitcoin_util_a-random.o) - local symbol 152: discarded in section `.text._ZN10tinyformat6detail12convertToIntIA13_cLb0EE6invokeERA13_Kc' from libbitcoin_util.a(libbitcoin_util_a-random.o) - -According to similar reported errors this is a binutils (ld) issue in 2.15, the -version installed by OpenBSD 5.7: - -- http://openbsd-archive.7691.n7.nabble.com/UPDATE-cppcheck-1-65-td248900.html -- https://llvm.org/bugs/show_bug.cgi?id=9758 - -There is no known workaround for this. diff --git a/doc/release-notes.md b/doc/release-notes.md index 4ecca7897c..23414666ce 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -76,6 +76,9 @@ will only create hierarchical deterministic (HD) wallets. Low-level RPC changes ---------------------- +- `listsinceblock` will now throw an error if an unknown `blockhash` argument + value is passed, instead of returning a list of all wallet transactions since + the genesis block. - The "currentblocksize" value in getmininginfo has been removed. - The deprecated RPC `getinfo` was removed. It is recommended that the more specific RPCs are used: * `getblockchaininfo` diff --git a/src/Makefile.am b/src/Makefile.am index 90deff48b0..3e43076878 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -500,10 +500,6 @@ clean-local: ## FIXME: How to get the appropriate modulename_CPPFLAGS in here? $(AM_V_GEN) $(WINDRES) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(CPPFLAGS) -DWINDRES_PREPROC -i $< -o $@ -.mm.o: - $(AM_V_CXX) $(OBJCXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \ - $(CPPFLAGS) $(AM_CXXFLAGS) $(QT_INCLUDES) $(AM_CXXFLAGS) $(PIE_FLAGS) $(CXXFLAGS) -c -o $@ $< - check-symbols: $(bin_PROGRAMS) if GLIBC_BACK_COMPAT @echo "Checking glibc back compat..." diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index e4b64c1ca7..0767ee1302 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -368,6 +368,7 @@ BITCOIN_QT_INCLUDES = -I$(builddir)/qt -I$(srcdir)/qt -I$(srcdir)/qt/forms \ qt_libbitcoinqt_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_INCLUDES) \ $(QT_INCLUDES) $(QT_DBUS_INCLUDES) $(PROTOBUF_CFLAGS) $(QR_CFLAGS) qt_libbitcoinqt_a_CXXFLAGS = $(AM_CXXFLAGS) $(QT_PIE_FLAGS) +qt_libbitcoinqt_a_OBJCXXFLAGS = $(AM_OBJCXXFLAGS) $(QT_PIE_FLAGS) qt_libbitcoinqt_a_SOURCES = $(BITCOIN_QT_CPP) $(BITCOIN_QT_H) $(QT_FORMS_UI) \ $(QT_QRC) $(QT_QRC_LOCALE) $(QT_TS) $(PROTOBUF_PROTO) $(RES_ICONS) $(RES_IMAGES) $(RES_MOVIES) diff --git a/src/addrman.h b/src/addrman.h index 18f3062287..f347cba6ca 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -455,6 +455,7 @@ public: void Clear() { + LOCK(cs); std::vector<int>().swap(vRandom); nKey = GetRandHash(); for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 7b307d6f42..dd4ba5ab0e 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -8,29 +8,22 @@ #include <assert.h> #include <iostream> #include <iomanip> -#include <sys/time.h> benchmark::BenchRunner::BenchmarkMap &benchmark::BenchRunner::benchmarks() { static std::map<std::string, benchmark::BenchFunction> benchmarks_map; return benchmarks_map; } -static double gettimedouble(void) { - struct timeval tv; - gettimeofday(&tv, nullptr); - return tv.tv_usec * 0.000001 + tv.tv_sec; -} - benchmark::BenchRunner::BenchRunner(std::string name, benchmark::BenchFunction func) { benchmarks().insert(std::make_pair(name, func)); } void -benchmark::BenchRunner::RunAll(double elapsedTimeForOne) +benchmark::BenchRunner::RunAll(benchmark::duration elapsedTimeForOne) { perf_init(); - std::cout << "#Benchmark" << "," << "count" << "," << "min" << "," << "max" << "," << "average" << "," + std::cout << "#Benchmark" << "," << "count" << "," << "min(ns)" << "," << "max(ns)" << "," << "average(ns)" << "," << "min_cycles" << "," << "max_cycles" << "," << "average_cycles" << "\n"; for (const auto &p: benchmarks()) { @@ -46,16 +39,17 @@ bool benchmark::State::KeepRunning() ++count; return true; } - double now; + time_point now; + uint64_t nowCycles; if (count == 0) { - lastTime = beginTime = now = gettimedouble(); + lastTime = beginTime = now = clock::now(); lastCycles = beginCycles = nowCycles = perf_cpucycles(); } else { - now = gettimedouble(); - double elapsed = now - lastTime; - double elapsedOne = elapsed / (countMask + 1); + now = clock::now(); + auto elapsed = now - lastTime; + auto elapsedOne = elapsed / (countMask + 1); if (elapsedOne < minTime) minTime = elapsedOne; if (elapsedOne > maxTime) maxTime = elapsedOne; @@ -70,8 +64,8 @@ bool benchmark::State::KeepRunning() // The restart avoids including the overhead of this code in the measurement. countMask = ((countMask<<3)|7) & ((1LL<<60)-1); count = 0; - minTime = std::numeric_limits<double>::max(); - maxTime = std::numeric_limits<double>::min(); + minTime = duration::max(); + maxTime = duration::zero(); minCycles = std::numeric_limits<uint64_t>::max(); maxCycles = std::numeric_limits<uint64_t>::min(); return true; @@ -94,9 +88,13 @@ bool benchmark::State::KeepRunning() assert(count != 0 && "count == 0 => (now == 0 && beginTime == 0) => return above"); // Output results - double average = (now-beginTime)/count; + // Duration casts are only necessary here because hardware with sub-nanosecond clocks + // will lose precision. + int64_t min_elapsed = std::chrono::duration_cast<std::chrono::nanoseconds>(minTime).count(); + int64_t max_elapsed = std::chrono::duration_cast<std::chrono::nanoseconds>(maxTime).count(); + int64_t avg_elapsed = std::chrono::duration_cast<std::chrono::nanoseconds>((now-beginTime)/count).count(); int64_t averageCycles = (nowCycles-beginCycles)/count; - std::cout << std::fixed << std::setprecision(15) << name << "," << count << "," << minTime << "," << maxTime << "," << average << "," + std::cout << std::fixed << std::setprecision(15) << name << "," << count << "," << min_elapsed << "," << max_elapsed << "," << avg_elapsed << "," << minCycles << "," << maxCycles << "," << averageCycles << "\n"; std::cout.copyfmt(std::ios(nullptr)); diff --git a/src/bench/bench.h b/src/bench/bench.h index 79109eaa56..d276f4ee91 100644 --- a/src/bench/bench.h +++ b/src/bench/bench.h @@ -9,6 +9,7 @@ #include <limits> #include <map> #include <string> +#include <chrono> #include <boost/preprocessor/cat.hpp> #include <boost/preprocessor/stringize.hpp> @@ -36,12 +37,23 @@ BENCHMARK(CODE_TO_TIME); */ namespace benchmark { + // On many systems, the high_resolution_clock offers no better resolution than the steady_clock. + // If that's the case, prefer the steady_clock. + struct best_clock { + using hi_res_clock = std::chrono::high_resolution_clock; + using steady_clock = std::chrono::steady_clock; + static constexpr bool steady_is_high_res = std::ratio_less_equal<steady_clock::period, hi_res_clock::period>::value; + using type = std::conditional<steady_is_high_res, steady_clock, hi_res_clock>::type; + }; + using clock = best_clock::type; + using time_point = clock::time_point; + using duration = clock::duration; class State { std::string name; - double maxElapsed; - double beginTime; - double lastTime, minTime, maxTime; + duration maxElapsed; + time_point beginTime, lastTime; + duration minTime, maxTime; uint64_t count; uint64_t countMask; uint64_t beginCycles; @@ -49,9 +61,9 @@ namespace benchmark { uint64_t minCycles; uint64_t maxCycles; public: - State(std::string _name, double _maxElapsed) : name(_name), maxElapsed(_maxElapsed), count(0) { - minTime = std::numeric_limits<double>::max(); - maxTime = std::numeric_limits<double>::min(); + State(std::string _name, duration _maxElapsed) : name(_name), maxElapsed(_maxElapsed), count(0) { + minTime = duration::max(); + maxTime = duration::zero(); minCycles = std::numeric_limits<uint64_t>::max(); maxCycles = std::numeric_limits<uint64_t>::min(); countMask = 1; @@ -69,7 +81,7 @@ namespace benchmark { public: BenchRunner(std::string name, BenchFunction func); - static void RunAll(double elapsedTimeForOne=1.0); + static void RunAll(duration elapsedTimeForOne = std::chrono::seconds(1)); }; } diff --git a/src/bench/rollingbloom.cpp b/src/bench/rollingbloom.cpp index 73c02cf718..a93d0fb0a5 100644 --- a/src/bench/rollingbloom.cpp +++ b/src/bench/rollingbloom.cpp @@ -6,7 +6,6 @@ #include "bench.h" #include "bloom.h" -#include "utiltime.h" static void RollingBloom(benchmark::State& state) { @@ -23,10 +22,10 @@ static void RollingBloom(benchmark::State& state) data[2] = count >> 16; data[3] = count >> 24; if (countnow == nEntriesPerGeneration) { - int64_t b = GetTimeMicros(); + auto b = benchmark::clock::now(); filter.insert(data); - int64_t e = GetTimeMicros(); - std::cout << "RollingBloom-refresh,1," << (e-b)*0.000001 << "," << (e-b)*0.000001 << "," << (e-b)*0.000001 << "\n"; + auto total = std::chrono::duration_cast<std::chrono::nanoseconds>(benchmark::clock::now() - b).count(); + std::cout << "RollingBloom-refresh,1," << total << "," << total << "," << total << "\n"; countnow = 0; } else { filter.insert(data); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index a20222d05c..b499b15507 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -387,6 +387,10 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s scriptPubKey = GetScriptForWitness(scriptPubKey); } if (bScriptHash) { + if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) { + throw std::runtime_error(strprintf( + "redeemScript exceeds size limit: %d > %d", scriptPubKey.size(), MAX_SCRIPT_ELEMENT_SIZE)); + } // Get the ID for the script, and then construct a P2SH destination for it. scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey)); } @@ -447,10 +451,19 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str bScriptHash = (flags.find("S") != std::string::npos); } + if (scriptPubKey.size() > MAX_SCRIPT_SIZE) { + throw std::runtime_error(strprintf( + "script exceeds size limit: %d > %d", scriptPubKey.size(), MAX_SCRIPT_SIZE)); + } + if (bSegWit) { scriptPubKey = GetScriptForWitness(scriptPubKey); } if (bScriptHash) { + if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) { + throw std::runtime_error(strprintf( + "redeemScript exceeds size limit: %d > %d", scriptPubKey.size(), MAX_SCRIPT_ELEMENT_SIZE)); + } scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey)); } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 543eba0e69..5f88c35dbd 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -120,7 +120,7 @@ bool AppInit(int argc, char* argv[]) for (int i = 1; i < argc; i++) { if (!IsSwitchChar(argv[i][0])) { fprintf(stderr, "Error: Command line contains unexpected token '%s', see bitcoind -h for a list of options.\n", argv[i]); - exit(EXIT_FAILURE); + return false; } } @@ -132,17 +132,17 @@ bool AppInit(int argc, char* argv[]) if (!AppInitBasicSetup()) { // InitError will have been called with detailed error, which ends up on console - exit(EXIT_FAILURE); + return false; } if (!AppInitParameterInteraction()) { // InitError will have been called with detailed error, which ends up on console - exit(EXIT_FAILURE); + return false; } if (!AppInitSanityChecks()) { // InitError will have been called with detailed error, which ends up on console - exit(EXIT_FAILURE); + return false; } if (gArgs.GetBoolArg("-daemon", false)) { @@ -163,7 +163,7 @@ bool AppInit(int argc, char* argv[]) if (!AppInitLockDataDirectory()) { // If locking the data directory failed, exit immediately - exit(EXIT_FAILURE); + return false; } fRet = AppInitMain(threadGroup, scheduler); } diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 85c9cd6934..950bdd945c 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -75,6 +75,7 @@ public: CMainParams() { strNetworkID = "main"; consensus.nSubsidyHalvingInterval = 210000; + consensus.BIP16Height = 173805; // 00000000000000ce80a7e057163a4db1d5ad7b20fb6f598c9597b9665c8fb0d4 - April 1, 2012 consensus.BIP34Height = 227931; consensus.BIP34Hash = uint256S("0x000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8"); consensus.BIP65Height = 388381; // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0 @@ -181,6 +182,7 @@ public: CTestNetParams() { strNetworkID = "test"; consensus.nSubsidyHalvingInterval = 210000; + consensus.BIP16Height = 514; // 00000000040b4e986385315e14bee30ad876d8b47f748025b26683116d21aa65 consensus.BIP34Height = 21111; consensus.BIP34Hash = uint256S("0x0000000023b3a96d3484e5abb3755c413e7d41500f8e2a5c3f0dd01299cd8ef8"); consensus.BIP65Height = 581885; // 00000000007f6655f22f98e72ed80d8b06dc761d5da09df0fa1dc4be4f861eb6 @@ -230,7 +232,6 @@ public: vSeeds.emplace_back("testnet-seed.bitcoin.jonasschnelli.ch", true); vSeeds.emplace_back("seed.tbtc.petertodd.org", true); vSeeds.emplace_back("testnet-seed.bluematt.me", false); - vSeeds.emplace_back("testnet-seed.bitcoin.schildbach.de", false); base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,111); base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,196); @@ -271,6 +272,7 @@ public: CRegTestParams() { strNetworkID = "regtest"; consensus.nSubsidyHalvingInterval = 150; + consensus.BIP16Height = 0; // always enforce P2SH BIP16 on regtest consensus.BIP34Height = 100000000; // BIP34 has not activated on regtest (far in the future so block v1 are not rejected in tests) consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in rpc activation tests) @@ -284,13 +286,13 @@ public: consensus.nMinerConfirmationWindow = 144; // Faster than normal for regtest (144 instead of 2016) consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28; consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 0; - consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 999999999999ULL; + consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; consensus.vDeployments[Consensus::DEPLOYMENT_CSV].bit = 0; consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nStartTime = 0; - consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nTimeout = 999999999999ULL; + consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].bit = 1; - consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nStartTime = 0; - consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nTimeout = 999999999999ULL; + consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; // The best chain should have at least this much work. consensus.nMinimumChainWork = uint256S("0x00"); diff --git a/src/consensus/params.h b/src/consensus/params.h index 6240e82857..fd0946a612 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -7,6 +7,7 @@ #define BITCOIN_CONSENSUS_PARAMS_H #include "uint256.h" +#include <limits> #include <map> #include <string> @@ -31,6 +32,15 @@ struct BIP9Deployment { int64_t nStartTime; /** Timeout/expiry MedianTime for the deployment attempt. */ int64_t nTimeout; + + /** Constant for nTimeout very far in the future. */ + static constexpr int64_t NO_TIMEOUT = std::numeric_limits<int64_t>::max(); + + /** Special value for nStartTime indicating that the deployment is always active. + * This is useful for testing, as it means tests don't need to deal with the activation + * process (which takes at least 3 BIP9 intervals). Only tests that specifically test the + * behaviour during activation cannot use this. */ + static constexpr int64_t ALWAYS_ACTIVE = -1; }; /** @@ -39,6 +49,8 @@ struct BIP9Deployment { struct Params { uint256 hashGenesisBlock; int nSubsidyHalvingInterval; + /** Block height at which BIP16 becomes active */ + int BIP16Height; /** Block height and hash at which BIP34 becomes active */ int BIP34Height; uint256 BIP34Hash; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 5923871691..f6cbaa20b7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -24,6 +24,7 @@ #include <event2/thread.h> #include <event2/buffer.h> +#include <event2/bufferevent.h> #include <event2/util.h> #include <event2/keyvalq_struct.h> @@ -239,6 +240,16 @@ static std::string RequestMethodString(HTTPRequest::RequestMethod m) /** HTTP request callback */ static void http_request_cb(struct evhttp_request* req, void* arg) { + // Disable reading to work around a libevent bug, fixed in 2.2.0. + if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02020001) { + evhttp_connection* conn = evhttp_request_get_connection(req); + if (conn) { + bufferevent* bev = evhttp_connection_get_bufferevent(conn); + if (bev) { + bufferevent_disable(bev, EV_READ); + } + } + } std::unique_ptr<HTTPRequest> hreq(new HTTPRequest(req)); LogPrint(BCLog::HTTP, "Received a %s request for %s from %s\n", @@ -481,6 +492,8 @@ void StopHTTPServer() } if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); + // Exit the event loop as soon as there are no active events. + event_base_loopexit(eventBase, nullptr); // Give event loop a few seconds to exit (to send back last RPC responses), then break it // Before this was solved with event_base_loopexit, but that didn't work as expected in // at least libevent 2.0.21 and always introduced a delay. In libevent @@ -599,8 +612,21 @@ void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) struct evbuffer* evb = evhttp_request_get_output_buffer(req); assert(evb); evbuffer_add(evb, strReply.data(), strReply.size()); - HTTPEvent* ev = new HTTPEvent(eventBase, true, - std::bind(evhttp_send_reply, req, nStatus, (const char*)nullptr, (struct evbuffer *)nullptr)); + auto req_copy = req; + HTTPEvent* ev = new HTTPEvent(eventBase, true, [req_copy, nStatus]{ + evhttp_send_reply(req_copy, nStatus, nullptr, nullptr); + // Re-enable reading from the socket. This is the second part of the libevent + // workaround above. + if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02020001) { + evhttp_connection* conn = evhttp_request_get_connection(req_copy); + if (conn) { + bufferevent* bev = evhttp_connection_get_bufferevent(conn); + if (bev) { + bufferevent_enable(bev, EV_READ | EV_WRITE); + } + } + } + }); ev->trigger(nullptr); replySent = true; req = nullptr; // transferred back to main thread diff --git a/src/init.cpp b/src/init.cpp index f63ad7f5d3..ddac606a39 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -544,14 +544,14 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex } static bool fHaveGenesis = false; -static boost::mutex cs_GenesisWait; +static CWaitableCriticalSection cs_GenesisWait; static CConditionVariable condvar_GenesisWait; static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex) { if (pBlockIndex != nullptr) { { - boost::unique_lock<boost::mutex> lock_GenesisWait(cs_GenesisWait); + WaitableLock lock_GenesisWait(cs_GenesisWait); fHaveGenesis = true; } condvar_GenesisWait.notify_all(); @@ -588,7 +588,7 @@ void CleanupBlockRevFiles() LogPrintf("Removing unusable blk?????.dat and rev?????.dat files for -reindex with -prune\n"); fs::path blocksdir = GetDataDir() / "blocks"; for (fs::directory_iterator it(blocksdir); it != fs::directory_iterator(); it++) { - if (is_regular_file(*it) && + if (fs::is_regular_file(*it) && it->path().filename().string().length() == 12 && it->path().filename().string().substr(8,4) == ".dat") { @@ -1270,7 +1270,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) g_connman = std::unique_ptr<CConnman>(new CConnman(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()))); CConnman& connman = *g_connman; - peerLogic.reset(new PeerLogicValidation(&connman)); + peerLogic.reset(new PeerLogicValidation(&connman, scheduler)); RegisterValidationInterface(peerLogic.get()); // sanitize comments per BIP-0014, format user agent and check total size @@ -1630,7 +1630,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // Wait for genesis block to be processed { - boost::unique_lock<boost::mutex> lock(cs_GenesisWait); + WaitableLock lock(cs_GenesisWait); while (!fHaveGenesis) { condvar_GenesisWait.wait(lock); } diff --git a/src/net.cpp b/src/net.cpp index 258599747a..935cb81901 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -961,6 +961,16 @@ static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEviction return a.nTimeConnected > b.nTimeConnected; } + +//! Sort an array by the specified comparator, then erase the last K elements. +template<typename T, typename Comparator> +static void EraseLastKElements(std::vector<T> &elements, Comparator comparator, size_t k) +{ + std::sort(elements.begin(), elements.end(), comparator); + size_t eraseSize = std::min(k, elements.size()); + elements.erase(elements.end() - eraseSize, elements.end()); +} + /** Try to find a connection to evict when the node is full. * Extreme care must be taken to avoid opening the node to attacker * triggered network partitioning. @@ -990,42 +1000,23 @@ bool CConnman::AttemptToEvictConnection() } } - if (vEvictionCandidates.empty()) return false; - // Protect connections with certain characteristics // Deterministically select 4 peers to protect by netgroup. // An attacker cannot predict which netgroups will be protected - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNetGroupKeyed); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4); // Protect the 8 nodes with the lowest minimum ping time. // An attacker cannot manipulate this metric without physically moving nodes closer to the target. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeMinPingTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(8, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); // Protect 4 nodes that most recently sent us transactions. // An attacker cannot manipulate this metric without performing useful work. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeTXTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); // Protect 4 nodes that most recently sent us blocks. // An attacker cannot manipulate this metric without performing useful work. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeBlockTime); - vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); - - if (vEvictionCandidates.empty()) return false; - + EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); // Protect the half of the remaining nodes which have been connected the longest. // This replicates the non-eviction implicit behavior, and precludes attacks that start later. - std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeTimeConnected); - vEvictionCandidates.erase(vEvictionCandidates.end() - static_cast<int>(vEvictionCandidates.size() / 2), vEvictionCandidates.end()); + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, vEvictionCandidates.size() / 2); if (vEvictionCandidates.empty()) return false; @@ -1036,12 +1027,12 @@ bool CConnman::AttemptToEvictConnection() int64_t nMostConnectionsTime = 0; std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes; for (const NodeEvictionCandidate &node : vEvictionCandidates) { - mapNetGroupNodes[node.nKeyedNetGroup].push_back(node); - int64_t grouptime = mapNetGroupNodes[node.nKeyedNetGroup][0].nTimeConnected; - size_t groupsize = mapNetGroupNodes[node.nKeyedNetGroup].size(); + std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup]; + group.push_back(node); + int64_t grouptime = group[0].nTimeConnected; - if (groupsize > nMostConnections || (groupsize == nMostConnections && grouptime > nMostConnectionsTime)) { - nMostConnections = groupsize; + if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) { + nMostConnections = group.size(); nMostConnectionsTime = grouptime; naMostConnections = node.nKeyedNetGroup; } @@ -1693,6 +1684,37 @@ void CConnman::ProcessOneShot() } } +bool CConnman::GetTryNewOutboundPeer() +{ + return m_try_another_outbound_peer; +} + +void CConnman::SetTryNewOutboundPeer(bool flag) +{ + m_try_another_outbound_peer = flag; + LogPrint(BCLog::NET, "net: setting try another outbound peer=%s\n", flag ? "true" : "false"); +} + +// Return the number of peers we have over our outbound connection limit +// Exclude peers that are marked for disconnect, or are going to be +// disconnected soon (eg one-shots and feelers) +// Also exclude peers that haven't finished initial connection handshake yet +// (so that we don't decide we're over our desired connection limit, and then +// evict some peer that has finished the handshake) +int CConnman::GetExtraOutboundCount() +{ + int nOutbound = 0; + { + LOCK(cs_vNodes); + for (CNode* pnode : vNodes) { + if (!pnode->fInbound && !pnode->m_manual_connection && !pnode->fFeeler && !pnode->fDisconnect && !pnode->fOneShot && pnode->fSuccessfullyConnected) { + ++nOutbound; + } + } + } + return std::max(nOutbound - nMaxOutbound, 0); +} + void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) { // Connect to specific addresses @@ -1781,7 +1803,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // * Only make a feeler connection once every few minutes. // bool fFeeler = false; - if (nOutbound >= nMaxOutbound) { + + if (nOutbound >= nMaxOutbound && !GetTryNewOutboundPeer()) { int64_t nTime = GetTimeMicros(); // The current time right now (in microseconds). if (nTime > nNextFeeler) { nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); @@ -2204,6 +2227,7 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe semOutbound = nullptr; semAddnode = nullptr; flagInterruptMsgProc = false; + SetTryNewOutboundPeer(false); Options connOptions; Init(connOptions); @@ -251,6 +251,19 @@ public: void GetBanned(banmap_t &banmap); void SetBanned(const banmap_t &banmap); + // This allows temporarily exceeding nMaxOutbound, with the goal of finding + // a peer that is better than all our current peers. + void SetTryNewOutboundPeer(bool flag); + bool GetTryNewOutboundPeer(); + + // Return the number of outbound peers we have in excess of our target (eg, + // if we previously called SetTryNewOutboundPeer(true), and have since set + // to false, we may have extra peers that we wish to disconnect). This may + // return a value less than (num_outbound_connections - num_outbound_slots) + // in cases where some outbound connections are not yet fully connected, or + // not yet fully disconnected. + int GetExtraOutboundCount(); + bool AddNode(const std::string& node); bool RemoveAddedNode(const std::string& node); std::vector<AddedNodeInfo> GetAddedNodeInfo(); @@ -413,6 +426,13 @@ private: std::thread threadOpenAddedConnections; std::thread threadOpenConnections; std::thread threadMessageHandler; + + /** flag for deciding to connect to an extra outbound peer, + * in excess of nMaxOutbound + * This takes the place of a feeler connection */ + std::atomic_bool m_try_another_outbound_peer; + + friend struct CConnmanTest; }; extern std::unique_ptr<CConnman> g_connman; void Discover(boost::thread_group& threadGroup); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6c26cd4cee..6866cd3409 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -23,6 +23,7 @@ #include "primitives/transaction.h" #include "random.h" #include "reverse_iterator.h" +#include "scheduler.h" #include "tinyformat.h" #include "txmempool.h" #include "ui_interface.h" @@ -124,6 +125,12 @@ namespace { /** Number of peers from which we're downloading blocks. */ int nPeersWithValidatedDownloads = 0; + /** Number of outbound peers with m_chain_sync.m_protect. */ + int g_outbound_peers_with_protect_from_disconnect = 0; + + /** When our tip was last updated. */ + int64_t g_last_tip_update = 0; + /** Relay map, protected by cs_main. */ typedef std::map<uint256, CTransactionRef> MapRelay; MapRelay mapRelay; @@ -201,6 +208,36 @@ struct CNodeState { */ bool fSupportsDesiredCmpctVersion; + /** State used to enforce CHAIN_SYNC_TIMEOUT + * Only in effect for outbound, non-manual connections, with + * m_protect == false + * Algorithm: if a peer's best known block has less work than our tip, + * set a timeout CHAIN_SYNC_TIMEOUT seconds in the future: + * - If at timeout their best known block now has more work than our tip + * when the timeout was set, then either reset the timeout or clear it + * (after comparing against our current tip's work) + * - If at timeout their best known block still has less work than our + * tip did when the timeout was set, then send a getheaders message, + * and set a shorter timeout, HEADERS_RESPONSE_TIME seconds in future. + * If their best known block is still behind when that new timeout is + * reached, disconnect. + */ + struct ChainSyncTimeoutState { + //! A timeout used for checking whether our peer has sufficiently synced + int64_t m_timeout; + //! A header with the work we require on our peer's chain + const CBlockIndex * m_work_header; + //! After timeout is reached, set to true after sending getheaders + bool m_sent_getheaders; + //! Whether this peer is protected from disconnection due to a bad/slow chain + bool m_protect; + }; + + ChainSyncTimeoutState m_chain_sync; + + //! Time of last new block announcement + int64_t m_last_block_announcement; + CNodeState(CAddress addrIn, std::string addrNameIn) : address(addrIn), name(addrNameIn) { fCurrentlyConnected = false; nMisbehavior = 0; @@ -223,6 +260,8 @@ struct CNodeState { fHaveWitness = false; fWantsCmpctWitness = false; fSupportsDesiredCmpctVersion = false; + m_chain_sync = { 0, nullptr, false, false }; + m_last_block_announcement = 0; } }; @@ -396,6 +435,15 @@ void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) { } } +bool TipMayBeStale(const Consensus::Params &consensusParams) +{ + AssertLockHeld(cs_main); + if (g_last_tip_update == 0) { + g_last_tip_update = GetTime(); + } + return g_last_tip_update < GetTime() - consensusParams.nPowTargetSpacing * 3 && mapBlocksInFlight.empty(); +} + // Requires cs_main bool CanDirectFetch(const Consensus::Params &consensusParams) { @@ -502,6 +550,22 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<con } // namespace +// This function is used for testing the stale tip eviction logic, see +// DoS_tests.cpp +void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) +{ + LOCK(cs_main); + CNodeState *state = State(node); + if (state) state->m_last_block_announcement = time_in_seconds; +} + +// Returns true for outbound peers, excluding manual connections, feelers, and +// one-shots +bool IsOutboundDisconnectionCandidate(const CNode *node) +{ + return !(node->fInbound || node->m_manual_connection || node->fFeeler || node->fOneShot); +} + void PeerLogicValidation::InitializeNode(CNode *pnode) { CAddress addr = pnode->addr; std::string addrName = pnode->GetAddrName(); @@ -534,6 +598,8 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim nPreferredDownload -= state->fPreferredDownload; nPeersWithValidatedDownloads -= (state->nBlocksInFlightValidHeaders != 0); assert(nPeersWithValidatedDownloads >= 0); + g_outbound_peers_with_protect_from_disconnect -= state->m_chain_sync.m_protect; + assert(g_outbound_peers_with_protect_from_disconnect >= 0); mapNodeState.erase(nodeid); @@ -542,6 +608,7 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim assert(mapBlocksInFlight.empty()); assert(nPreferredDownload == 0); assert(nPeersWithValidatedDownloads == 0); + assert(g_outbound_peers_with_protect_from_disconnect == 0); } LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } @@ -566,7 +633,7 @@ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) { // mapOrphanTransactions // -void AddToCompactExtraTransactions(const CTransactionRef& tx) +void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { size_t max_extra_txn = gArgs.GetArg("-blockreconstructionextratxn", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN); if (max_extra_txn <= 0) @@ -723,9 +790,17 @@ static bool StaleBlockRequestAllowed(const CBlockIndex* pindex, const Consensus: (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); } -PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) { +PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, CScheduler &scheduler) : connman(connmanIn), m_stale_tip_check_time(0) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); + + const Consensus::Params& consensusParams = Params().GetConsensus(); + // Stale tip checking and peer eviction are on two different timers, but we + // don't want them to get out of sync due to drift in the scheduler, so we + // combine them in one function and schedule at the quicker (peer-eviction) + // timer. + static_assert(EXTRA_PEER_CHECK_INTERVAL < STALE_CHECK_INTERVAL, "peer eviction timer should be less than stale tip check timer"); + scheduler.scheduleEvery(std::bind(&PeerLogicValidation::CheckForStaleTipAndEvictPeers, this, consensusParams), EXTRA_PEER_CHECK_INTERVAL * 1000); } void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex, const std::vector<CTransactionRef>& vtxConflicted) { @@ -756,6 +831,8 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb } LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased); } + + g_last_tip_update = GetTime(); } // All of the following cache a recent block, and are protected by cs_most_recent_block @@ -1164,6 +1241,225 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); } +bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool punish_duplicate_invalid) +{ + const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + size_t nCount = headers.size(); + + if (nCount == 0) { + // Nothing interesting. Stop asking this peers for more headers. + return true; + } + + bool received_new_header = false; + const CBlockIndex *pindexLast = nullptr; + { + LOCK(cs_main); + CNodeState *nodestate = State(pfrom->GetId()); + + // If this looks like it could be a block announcement (nCount < + // MAX_BLOCKS_TO_ANNOUNCE), use special logic for handling headers that + // don't connect: + // - Send a getheaders message in response to try to connect the chain. + // - The peer can send up to MAX_UNCONNECTING_HEADERS in a row that + // don't connect before giving DoS points + // - Once a headers message is received that is valid and does connect, + // nUnconnectingHeaders gets reset back to 0. + if (mapBlockIndex.find(headers[0].hashPrevBlock) == mapBlockIndex.end() && nCount < MAX_BLOCKS_TO_ANNOUNCE) { + nodestate->nUnconnectingHeaders++; + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), uint256())); + LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", + headers[0].GetHash().ToString(), + headers[0].hashPrevBlock.ToString(), + pindexBestHeader->nHeight, + pfrom->GetId(), nodestate->nUnconnectingHeaders); + // Set hashLastUnknownBlock for this peer, so that if we + // eventually get the headers - even from a different peer - + // we can use this peer to download. + UpdateBlockAvailability(pfrom->GetId(), headers.back().GetHash()); + + if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { + Misbehaving(pfrom->GetId(), 20); + } + return true; + } + + uint256 hashLastBlock; + for (const CBlockHeader& header : headers) { + if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) { + Misbehaving(pfrom->GetId(), 20); + return error("non-continuous headers sequence"); + } + hashLastBlock = header.GetHash(); + } + + // If we don't have the last header, then they'll have given us + // something new (if these headers are valid). + if (mapBlockIndex.find(hashLastBlock) == mapBlockIndex.end()) { + received_new_header = true; + } + } + + CValidationState state; + CBlockHeader first_invalid_header; + if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) { + int nDoS; + if (state.IsInvalid(nDoS)) { + LOCK(cs_main); + if (nDoS > 0) { + Misbehaving(pfrom->GetId(), nDoS); + } + if (punish_duplicate_invalid && mapBlockIndex.find(first_invalid_header.GetHash()) != mapBlockIndex.end()) { + // Goal: don't allow outbound peers to use up our outbound + // connection slots if they are on incompatible chains. + // + // We ask the caller to set punish_invalid appropriately based + // on the peer and the method of header delivery (compact + // blocks are allowed to be invalid in some circumstances, + // under BIP 152). + // Here, we try to detect the narrow situation that we have a + // valid block header (ie it was valid at the time the header + // was received, and hence stored in mapBlockIndex) but know the + // block is invalid, and that a peer has announced that same + // block as being on its active chain. + // Disconnect the peer in such a situation. + // + // Note: if the header that is invalid was not accepted to our + // mapBlockIndex at all, that may also be grounds for + // disconnecting the peer, as the chain they are on is likely + // to be incompatible. However, there is a circumstance where + // that does not hold: if the header's timestamp is more than + // 2 hours ahead of our current time. In that case, the header + // may become valid in the future, and we don't want to + // disconnect a peer merely for serving us one too-far-ahead + // block header, to prevent an attacker from splitting the + // network by mining a block right at the 2 hour boundary. + // + // TODO: update the DoS logic (or, rather, rewrite the + // DoS-interface between validation and net_processing) so that + // the interface is cleaner, and so that we disconnect on all the + // reasons that a peer's headers chain is incompatible + // with ours (eg block->nVersion softforks, MTP violations, + // etc), and not just the duplicate-invalid case. + pfrom->fDisconnect = true; + } + return error("invalid header received"); + } + } + + { + LOCK(cs_main); + CNodeState *nodestate = State(pfrom->GetId()); + if (nodestate->nUnconnectingHeaders > 0) { + LogPrint(BCLog::NET, "peer=%d: resetting nUnconnectingHeaders (%d -> 0)\n", pfrom->GetId(), nodestate->nUnconnectingHeaders); + } + nodestate->nUnconnectingHeaders = 0; + + assert(pindexLast); + UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash()); + + // From here, pindexBestKnownBlock should be guaranteed to be non-null, + // because it is set in UpdateBlockAvailability. Some nullptr checks + // are still present, however, as belt-and-suspenders. + + if (received_new_header && pindexLast->nChainWork > chainActive.Tip()->nChainWork) { + nodestate->m_last_block_announcement = GetTime(); + } + + if (nCount == MAX_HEADERS_RESULTS) { + // Headers message had its maximum size; the peer may have more headers. + // TODO: optimize: if pindexLast is an ancestor of chainActive.Tip or pindexBestHeader, continue + // from there instead. + LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom->GetId(), pfrom->nStartingHeight); + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexLast), uint256())); + } + + bool fCanDirectFetch = CanDirectFetch(chainparams.GetConsensus()); + // If this set of headers is valid and ends in a block with at least as + // much work as our tip, download as much as possible. + if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->nChainWork <= pindexLast->nChainWork) { + std::vector<const CBlockIndex*> vToFetch; + const CBlockIndex *pindexWalk = pindexLast; + // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. + while (pindexWalk && !chainActive.Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && + !mapBlocksInFlight.count(pindexWalk->GetBlockHash()) && + (!IsWitnessEnabled(pindexWalk->pprev, chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) { + // We don't have this block, and it's not yet in flight. + vToFetch.push_back(pindexWalk); + } + pindexWalk = pindexWalk->pprev; + } + // If pindexWalk still isn't on our main chain, we're looking at a + // very large reorg at a time we think we're close to caught up to + // the main chain -- this shouldn't really happen. Bail out on the + // direct fetch and rely on parallel download instead. + if (!chainActive.Contains(pindexWalk)) { + LogPrint(BCLog::NET, "Large reorg, won't direct fetch to %s (%d)\n", + pindexLast->GetBlockHash().ToString(), + pindexLast->nHeight); + } else { + std::vector<CInv> vGetData; + // Download as much as possible, from earliest to latest. + for (const CBlockIndex *pindex : reverse_iterate(vToFetch)) { + if (nodestate->nBlocksInFlight >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + // Can't download any more from this peer + break; + } + uint32_t nFetchFlags = GetFetchFlags(pfrom); + vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); + MarkBlockAsInFlight(pfrom->GetId(), pindex->GetBlockHash(), pindex); + LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", + pindex->GetBlockHash().ToString(), pfrom->GetId()); + } + if (vGetData.size() > 1) { + LogPrint(BCLog::NET, "Downloading blocks toward %s (%d) via headers direct fetch\n", + pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); + } + if (vGetData.size() > 0) { + if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + // In any case, we want to download using a compact block, not a regular one + vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); + } + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + } + } + } + // If we're in IBD, we want outbound peers that will serve us a useful + // chain. Disconnect peers that are on chains with insufficient work. + if (IsInitialBlockDownload() && nCount != MAX_HEADERS_RESULTS) { + // When nCount < MAX_HEADERS_RESULTS, we know we have no more + // headers to fetch from this peer. + if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { + // This peer has too little work on their headers chain to help + // us sync -- disconnect if using an outbound slot (unless + // whitelisted or addnode). + // Note: We compare their tip to nMinimumChainWork (rather than + // chainActive.Tip()) because we won't start block download + // until we have a headers chain that has at least + // nMinimumChainWork, even if a peer has a chain past our tip, + // as an anti-DoS measure. + if (IsOutboundDisconnectionCandidate(pfrom)) { + LogPrintf("Disconnecting outbound peer %d -- headers chain has insufficient work\n", pfrom->GetId()); + pfrom->fDisconnect = true; + } + } + } + + if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr) { + // If this is an outbound peer, check to see if we should protect + // it from the bad/lagging chain logic. + if (g_outbound_peers_with_protect_from_disconnect < MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT && nodestate->pindexBestKnownBlock->nChainWork >= chainActive.Tip()->nChainWork && !nodestate->m_chain_sync.m_protect) { + LogPrint(BCLog::NET, "Protecting outbound peer=%d from eviction\n", pfrom->GetId()); + nodestate->m_chain_sync.m_protect = true; + ++g_outbound_peers_with_protect_from_disconnect; + } + } + } + + return true; +} + bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc) { LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->GetId()); @@ -1971,6 +2267,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr CBlockHeaderAndShortTxIDs cmpctblock; vRecv >> cmpctblock; + bool received_new_header = false; + { LOCK(cs_main); @@ -1980,6 +2278,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), uint256())); return true; } + + if (mapBlockIndex.find(cmpctblock.header.GetHash()) == mapBlockIndex.end()) { + received_new_header = true; + } } const CBlockIndex *pindex = nullptr; @@ -2006,7 +2308,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // If we end up treating this as a plain headers message, call that as well // without cs_main. bool fRevertToHeaderProcessing = false; - CDataStream vHeadersMsg(SER_NETWORK, PROTOCOL_VERSION); // Keep a CBlock for "optimistic" compactblock reconstructions (see // below) @@ -2019,6 +2320,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr assert(pindex); UpdateBlockAvailability(pfrom->GetId(), pindex->GetBlockHash()); + CNodeState *nodestate = State(pfrom->GetId()); + + // If this was a new header with more work than our tip, update the + // peer's last block announcement time + if (received_new_header && pindex->nChainWork > chainActive.Tip()->nChainWork) { + nodestate->m_last_block_announcement = GetTime(); + } + std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> >::iterator blockInFlightIt = mapBlocksInFlight.find(pindex->GetBlockHash()); bool fAlreadyInFlight = blockInFlightIt != mapBlocksInFlight.end(); @@ -2041,8 +2350,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (!fAlreadyInFlight && !CanDirectFetch(chainparams.GetConsensus())) return true; - CNodeState *nodestate = State(pfrom->GetId()); - if (IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) { // Don't bother trying to process compact blocks from v1 peers // after segwit activates. @@ -2123,10 +2430,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } else { // If this was an announce-cmpctblock, we want the same treatment as a header message - // Dirty hack to process as if it were just a headers message (TODO: move message handling into their own functions) - std::vector<CBlock> headers; - headers.push_back(cmpctblock.header); - vHeadersMsg << headers; fRevertToHeaderProcessing = true; } } @@ -2135,8 +2438,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (fProcessBLOCKTXN) return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc); - if (fRevertToHeaderProcessing) - return ProcessMessage(pfrom, NetMsgType::HEADERS, vHeadersMsg, nTimeReceived, chainparams, connman, interruptMsgProc); + if (fRevertToHeaderProcessing) { + // Headers received from HB compact block peers are permitted to be + // relayed before full validation (see BIP 152), so we don't want to disconnect + // the peer if the header turns out to be for an invalid block. + // Note that if a peer tries to build on an invalid chain, that + // will be detected and the peer will be banned. + return ProcessHeadersMessage(pfrom, connman, {cmpctblock.header}, chainparams, /*punish_duplicate_invalid=*/false); + } if (fBlockReconstructed) { // If we got here, we were able to optimistically reconstruct a @@ -2146,7 +2455,16 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom->GetId(), false)); } bool fNewBlock = false; - ProcessNewBlock(chainparams, pblock, true, &fNewBlock); + // Setting fForceProcessing to true means that we bypass some of + // our anti-DoS protections in AcceptBlock, which filters + // unrequested blocks that might be trying to waste our resources + // (eg disk space). Because we only try to reconstruct blocks when + // we're close to caught up (via the CanDirectFetch() requirement + // above, combined with the behavior of not requesting blocks until + // we have a chain with at least nMinimumChainWork), and we ignore + // compact blocks with less work than our tip, it is safe to treat + // reconstructed compact blocks as having been requested. + ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock); if (fNewBlock) { pfrom->nLastBlockTime = GetTime(); } else { @@ -2226,7 +2544,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool fNewBlock = false; // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) - ProcessNewBlock(chainparams, pblock, true, &fNewBlock); + // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent + // disk-space attacks), but this should be safe due to the + // protections in the compact block handler -- see related comment + // in compact block optimistic reconstruction handling. + ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock); if (fNewBlock) { pfrom->nLastBlockTime = GetTime(); } else { @@ -2254,136 +2576,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadCompactSize(vRecv); // ignore tx count; assume it is 0. } - if (nCount == 0) { - // Nothing interesting. Stop asking this peers for more headers. - return true; - } - - const CBlockIndex *pindexLast = nullptr; - { - LOCK(cs_main); - CNodeState *nodestate = State(pfrom->GetId()); - - // If this looks like it could be a block announcement (nCount < - // MAX_BLOCKS_TO_ANNOUNCE), use special logic for handling headers that - // don't connect: - // - Send a getheaders message in response to try to connect the chain. - // - The peer can send up to MAX_UNCONNECTING_HEADERS in a row that - // don't connect before giving DoS points - // - Once a headers message is received that is valid and does connect, - // nUnconnectingHeaders gets reset back to 0. - if (mapBlockIndex.find(headers[0].hashPrevBlock) == mapBlockIndex.end() && nCount < MAX_BLOCKS_TO_ANNOUNCE) { - nodestate->nUnconnectingHeaders++; - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), uint256())); - LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", - headers[0].GetHash().ToString(), - headers[0].hashPrevBlock.ToString(), - pindexBestHeader->nHeight, - pfrom->GetId(), nodestate->nUnconnectingHeaders); - // Set hashLastUnknownBlock for this peer, so that if we - // eventually get the headers - even from a different peer - - // we can use this peer to download. - UpdateBlockAvailability(pfrom->GetId(), headers.back().GetHash()); - - if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { - Misbehaving(pfrom->GetId(), 20); - } - return true; - } - - uint256 hashLastBlock; - for (const CBlockHeader& header : headers) { - if (!hashLastBlock.IsNull() && header.hashPrevBlock != hashLastBlock) { - Misbehaving(pfrom->GetId(), 20); - return error("non-continuous headers sequence"); - } - hashLastBlock = header.GetHash(); - } - } - - CValidationState state; - if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) { - int nDoS; - if (state.IsInvalid(nDoS)) { - if (nDoS > 0) { - LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS); - } - return error("invalid header received"); - } - } - - { - LOCK(cs_main); - CNodeState *nodestate = State(pfrom->GetId()); - if (nodestate->nUnconnectingHeaders > 0) { - LogPrint(BCLog::NET, "peer=%d: resetting nUnconnectingHeaders (%d -> 0)\n", pfrom->GetId(), nodestate->nUnconnectingHeaders); - } - nodestate->nUnconnectingHeaders = 0; - - assert(pindexLast); - UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash()); - - if (nCount == MAX_HEADERS_RESULTS) { - // Headers message had its maximum size; the peer may have more headers. - // TODO: optimize: if pindexLast is an ancestor of chainActive.Tip or pindexBestHeader, continue - // from there instead. - LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom->GetId(), pfrom->nStartingHeight); - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexLast), uint256())); - } - - bool fCanDirectFetch = CanDirectFetch(chainparams.GetConsensus()); - // If this set of headers is valid and ends in a block with at least as - // much work as our tip, download as much as possible. - if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->nChainWork <= pindexLast->nChainWork) { - std::vector<const CBlockIndex*> vToFetch; - const CBlockIndex *pindexWalk = pindexLast; - // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. - while (pindexWalk && !chainActive.Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { - if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && - !mapBlocksInFlight.count(pindexWalk->GetBlockHash()) && - (!IsWitnessEnabled(pindexWalk->pprev, chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) { - // We don't have this block, and it's not yet in flight. - vToFetch.push_back(pindexWalk); - } - pindexWalk = pindexWalk->pprev; - } - // If pindexWalk still isn't on our main chain, we're looking at a - // very large reorg at a time we think we're close to caught up to - // the main chain -- this shouldn't really happen. Bail out on the - // direct fetch and rely on parallel download instead. - if (!chainActive.Contains(pindexWalk)) { - LogPrint(BCLog::NET, "Large reorg, won't direct fetch to %s (%d)\n", - pindexLast->GetBlockHash().ToString(), - pindexLast->nHeight); - } else { - std::vector<CInv> vGetData; - // Download as much as possible, from earliest to latest. - for (const CBlockIndex *pindex : reverse_iterate(vToFetch)) { - if (nodestate->nBlocksInFlight >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { - // Can't download any more from this peer - break; - } - uint32_t nFetchFlags = GetFetchFlags(pfrom); - vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); - MarkBlockAsInFlight(pfrom->GetId(), pindex->GetBlockHash(), pindex); - LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", - pindex->GetBlockHash().ToString(), pfrom->GetId()); - } - if (vGetData.size() > 1) { - LogPrint(BCLog::NET, "Downloading blocks toward %s (%d) via headers direct fetch\n", - pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); - } - if (vGetData.size() > 0) { - if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { - // In any case, we want to download using a compact block, not a regular one - vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); - } - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETDATA, vGetData)); - } - } - } - } + // Headers received via a HEADERS message should be valid, and reflect + // the chain the peer is on. If we receive a known-invalid header, + // disconnect the peer if it is using one of our outbound connection + // slots. + bool should_punish = !pfrom->fInbound && !pfrom->m_manual_connection; + return ProcessHeadersMessage(pfrom, connman, headers, chainparams, should_punish); } else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing @@ -2393,11 +2591,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint(BCLog::NET, "received block %s peer=%d\n", pblock->GetHash().ToString(), pfrom->GetId()); - // Process all blocks from whitelisted peers, even if not requested, - // unless we're still syncing with the network. - // Such an unrequested block may still be processed, subject to the - // conditions in AcceptBlock(). - bool forceProcessing = pfrom->fWhitelisted && !IsInitialBlockDownload(); + bool forceProcessing = false; const uint256 hash(pblock->GetHash()); { LOCK(cs_main); @@ -2781,6 +2975,135 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter return fMoreWork; } +void PeerLogicValidation::ConsiderEviction(CNode *pto, int64_t time_in_seconds) +{ + AssertLockHeld(cs_main); + + CNodeState &state = *State(pto->GetId()); + const CNetMsgMaker msgMaker(pto->GetSendVersion()); + + if (!state.m_chain_sync.m_protect && IsOutboundDisconnectionCandidate(pto) && state.fSyncStarted) { + // This is an outbound peer subject to disconnection if they don't + // announce a block with as much work as the current tip within + // CHAIN_SYNC_TIMEOUT + HEADERS_RESPONSE_TIME seconds (note: if + // their chain has more work than ours, we should sync to it, + // unless it's invalid, in which case we should find that out and + // disconnect from them elsewhere). + if (state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= chainActive.Tip()->nChainWork) { + if (state.m_chain_sync.m_timeout != 0) { + state.m_chain_sync.m_timeout = 0; + state.m_chain_sync.m_work_header = nullptr; + state.m_chain_sync.m_sent_getheaders = false; + } + } else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && state.pindexBestKnownBlock != nullptr && state.pindexBestKnownBlock->nChainWork >= state.m_chain_sync.m_work_header->nChainWork)) { + // Our best block known by this peer is behind our tip, and we're either noticing + // that for the first time, OR this peer was able to catch up to some earlier point + // where we checked against our tip. + // Either way, set a new timeout based on current tip. + state.m_chain_sync.m_timeout = time_in_seconds + CHAIN_SYNC_TIMEOUT; + state.m_chain_sync.m_work_header = chainActive.Tip(); + state.m_chain_sync.m_sent_getheaders = false; + } else if (state.m_chain_sync.m_timeout > 0 && time_in_seconds > state.m_chain_sync.m_timeout) { + // No evidence yet that our peer has synced to a chain with work equal to that + // of our tip, when we first detected it was behind. Send a single getheaders + // message to give the peer a chance to update us. + if (state.m_chain_sync.m_sent_getheaders) { + // They've run out of time to catch up! + LogPrintf("Disconnecting outbound peer %d for old chain, best known block = %s\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>"); + pto->fDisconnect = true; + } else { + LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>", state.m_chain_sync.m_work_header->GetBlockHash().ToString()); + connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(state.m_chain_sync.m_work_header->pprev), uint256())); + state.m_chain_sync.m_sent_getheaders = true; + constexpr int64_t HEADERS_RESPONSE_TIME = 120; // 2 minutes + // Bump the timeout to allow a response, which could clear the timeout + // (if the response shows the peer has synced), reset the timeout (if + // the peer syncs to the required work but not to our tip), or result + // in disconnect (if we advance to the timeout and pindexBestKnownBlock + // has not sufficiently progressed) + state.m_chain_sync.m_timeout = time_in_seconds + HEADERS_RESPONSE_TIME; + } + } + } +} + +void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) +{ + // Check whether we have too many outbound peers + int extra_peers = connman->GetExtraOutboundCount(); + if (extra_peers > 0) { + // If we have more outbound peers than we target, disconnect one. + // Pick the outbound peer that least recently announced + // us a new block, with ties broken by choosing the more recent + // connection (higher node id) + NodeId worst_peer = -1; + int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max(); + + LOCK(cs_main); + + connman->ForEachNode([&](CNode* pnode) { + // Ignore non-outbound peers, or nodes marked for disconnect already + if (!IsOutboundDisconnectionCandidate(pnode) || pnode->fDisconnect) return; + CNodeState *state = State(pnode->GetId()); + if (state == nullptr) return; // shouldn't be possible, but just in case + // Don't evict our protected peers + if (state->m_chain_sync.m_protect) return; + if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { + worst_peer = pnode->GetId(); + oldest_block_announcement = state->m_last_block_announcement; + } + }); + if (worst_peer != -1) { + bool disconnected = connman->ForNode(worst_peer, [&](CNode *pnode) { + // Only disconnect a peer that has been connected to us for + // some reasonable fraction of our check-frequency, to give + // it time for new information to have arrived. + // Also don't disconnect any peer we're trying to download a + // block from. + CNodeState &state = *State(pnode->GetId()); + if (time_in_seconds - pnode->nTimeConnected > MINIMUM_CONNECT_TIME && state.nBlocksInFlight == 0) { + LogPrint(BCLog::NET, "disconnecting extra outbound peer=%d (last block announcement received at time %d)\n", pnode->GetId(), oldest_block_announcement); + pnode->fDisconnect = true; + return true; + } else { + LogPrint(BCLog::NET, "keeping outbound peer=%d chosen for eviction (connect time: %d, blocks_in_flight: %d)\n", pnode->GetId(), pnode->nTimeConnected, state.nBlocksInFlight); + return false; + } + }); + if (disconnected) { + // If we disconnected an extra peer, that means we successfully + // connected to at least one peer after the last time we + // detected a stale tip. Don't try any more extra peers until + // we next detect a stale tip, to limit the load we put on the + // network from these extra connections. + connman->SetTryNewOutboundPeer(false); + } + } + } +} + +void PeerLogicValidation::CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams) +{ + if (connman == nullptr) return; + + int64_t time_in_seconds = GetTime(); + + EvictExtraOutboundPeers(time_in_seconds); + + if (time_in_seconds > m_stale_tip_check_time) { + LOCK(cs_main); + // Check whether our tip is stale, and if so, allow using an extra + // outbound peer + if (TipMayBeStale(consensusParams)) { + LogPrintf("Potential stale tip detected, will try using extra outbound peer (last tip update: %d seconds ago)\n", time_in_seconds - g_last_tip_update); + connman->SetTryNewOutboundPeer(true); + } else if (connman->GetTryNewOutboundPeer()) { + connman->SetTryNewOutboundPeer(false); + } + m_stale_tip_check_time = time_in_seconds + STALE_CHECK_INTERVAL; + } +} + class CompareInvMempoolOrder { CTxMemPool *mp; @@ -3247,6 +3570,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM } } + // Check that outbound peers have reasonable chains + // GetTime() is used by this anti-DoS logic so we can test this using mocktime + ConsiderEviction(pto, GetTime()); // // Message: getdata (blocks) diff --git a/src/net_processing.h b/src/net_processing.h index 79745cdd42..0a49972eed 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -8,6 +8,7 @@ #include "net.h" #include "validationinterface.h" +#include "consensus/params.h" /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; @@ -21,13 +22,25 @@ static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; * Timeout = base + per_header * (expected number of headers) */ static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 1000; // 1ms/header +/** Protect at least this many outbound peers from disconnection due to slow/ + * behind headers chain. + */ +static constexpr int32_t MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT = 4; +/** Timeout for (unprotected) outbound peers to sync to our chainwork, in seconds */ +static constexpr int64_t CHAIN_SYNC_TIMEOUT = 20 * 60; // 20 minutes +/** How frequently to check for stale tips, in seconds */ +static constexpr int64_t STALE_CHECK_INTERVAL = 10 * 60; // 10 minutes +/** How frequently to check for extra outbound peers and disconnect, in seconds */ +static constexpr int64_t EXTRA_PEER_CHECK_INTERVAL = 45; +/** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict, in seconds */ +static constexpr int64_t MINIMUM_CONNECT_TIME = 30; class PeerLogicValidation : public CValidationInterface, public NetEventsInterface { private: - CConnman* connman; + CConnman* const connman; public: - explicit PeerLogicValidation(CConnman* connman); + explicit PeerLogicValidation(CConnman* connman, CScheduler &scheduler); void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected, const std::vector<CTransactionRef>& vtxConflicted) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; @@ -47,6 +60,13 @@ public: * @return True if there is more work to be done */ bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override; + + void ConsiderEviction(CNode *pto, int64_t time_in_seconds); + void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams); + void EvictExtraOutboundPeers(int64_t time_in_seconds); + +private: + int64_t m_stale_tip_check_time; //! Next time to check for stale tip }; struct CNodeStateStats { diff --git a/src/netbase.cpp b/src/netbase.cpp index 5a560bc95a..82040605c5 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -291,7 +291,7 @@ struct ProxyCredentials std::string password; }; -/** Convert SOCKS5 reply to a an error message */ +/** Convert SOCKS5 reply to an error message */ std::string Socks5ErrorString(uint8_t err) { switch(err) { diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index dcf49de5f1..c7e57671c0 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -180,6 +180,7 @@ TxConfirmStats::TxConfirmStats(const std::vector<double>& defaultBuckets, : buckets(defaultBuckets), bucketMap(defaultBucketMap) { decay = _decay; + assert(_scale != 0 && "_scale must be non-zero"); scale = _scale; confAvg.resize(maxPeriods); for (unsigned int i = 0; i < maxPeriods; i++) { @@ -418,6 +419,9 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); } filein >> scale; + if (scale == 0) { + throw std::runtime_error("Corrupt estimates file. Scale must be non-zero"); + } } filein >> avg; diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index e9f5c77a5b..d6cce09e8d 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -70,6 +70,7 @@ AskPassphraseDialog::AskPassphraseDialog(Mode _mode, QWidget *parent) : break; } textChanged(); + connect(ui->toggleShowPasswordButton, SIGNAL(toggled(bool)), this, SLOT(toggleShowPassword(bool))); connect(ui->passEdit1, SIGNAL(textChanged(QString)), this, SLOT(textChanged())); connect(ui->passEdit2, SIGNAL(textChanged(QString)), this, SLOT(textChanged())); connect(ui->passEdit3, SIGNAL(textChanged(QString)), this, SLOT(textChanged())); @@ -234,6 +235,15 @@ bool AskPassphraseDialog::event(QEvent *event) return QWidget::event(event); } +void AskPassphraseDialog::toggleShowPassword(bool show) +{ + ui->toggleShowPasswordButton->setDown(show); + const auto mode = show ? QLineEdit::Normal : QLineEdit::Password; + ui->passEdit1->setEchoMode(mode); + ui->passEdit2->setEchoMode(mode); + ui->passEdit3->setEchoMode(mode); +} + bool AskPassphraseDialog::eventFilter(QObject *object, QEvent *event) { /* Detect Caps Lock. diff --git a/src/qt/askpassphrasedialog.h b/src/qt/askpassphrasedialog.h index 34bf7ccb31..7c6acc4650 100644 --- a/src/qt/askpassphrasedialog.h +++ b/src/qt/askpassphrasedialog.h @@ -43,6 +43,7 @@ private: private Q_SLOTS: void textChanged(); void secureClearPassFields(); + void toggleShowPassword(bool); protected: bool event(QEvent *event); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 6952eb5064..207e441b6b 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -582,8 +582,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) QString toolTipDust = tr("This label turns red if any recipient receives an amount smaller than the current dust threshold."); // how many satoshis the estimated fee can vary per byte we guess wrong - assert(nBytes != 0); - double dFeeVary = (double)nPayFee / nBytes; + double dFeeVary = (nBytes != 0) ? (double)nPayFee / nBytes : 0; QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary); diff --git a/src/qt/forms/askpassphrasedialog.ui b/src/qt/forms/askpassphrasedialog.ui index a2105ecd0a..69803989cd 100644 --- a/src/qt/forms/askpassphrasedialog.ui +++ b/src/qt/forms/askpassphrasedialog.ui @@ -93,6 +93,13 @@ </widget> </item> <item row="3" column="1"> + <widget class="QCheckBox" name="toggleShowPasswordButton"> + <property name="text"> + <string>Show password</string> + </property> + </widget> + </item> + <item row="4" column="1"> <widget class="QLabel" name="capsLabel"> <property name="font"> <font> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index d520d7d4be..4bd63f4649 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -984,6 +984,18 @@ QString formatNiceTimeOffset(qint64 secs) return timeBehindText; } +QString formatBytes(uint64_t bytes) +{ + if(bytes < 1024) + return QString(QObject::tr("%1 B")).arg(bytes); + if(bytes < 1024 * 1024) + return QString(QObject::tr("%1 KB")).arg(bytes / 1024); + if(bytes < 1024 * 1024 * 1024) + return QString(QObject::tr("%1 MB")).arg(bytes / 1024 / 1024); + + return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); +} + void ClickableLabel::mouseReleaseEvent(QMouseEvent *event) { Q_EMIT clicked(event->pos()); diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index d10818d0c8..7622816f7f 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -199,6 +199,8 @@ namespace GUIUtil QString formatNiceTimeOffset(qint64 secs); + QString formatBytes(uint64_t bytes); + class ClickableLabel : public QLabel { Q_OBJECT diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 42934f8055..8b2a7e7047 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -33,6 +33,10 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombine return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0; case PeerTableModel::Ping: return pLeft->dMinPing < pRight->dMinPing; + case PeerTableModel::Sent: + return pLeft->nSendBytes < pRight->nSendBytes; + case PeerTableModel::Received: + return pLeft->nRecvBytes < pRight->nRecvBytes; } return false; @@ -114,7 +118,7 @@ PeerTableModel::PeerTableModel(ClientModel *parent) : clientModel(parent), timer(0) { - columns << tr("NodeId") << tr("Node/Service") << tr("User Agent") << tr("Ping"); + columns << tr("NodeId") << tr("Node/Service") << tr("Ping") << tr("Sent") << tr("Received") << tr("User Agent"); priv.reset(new PeerTablePriv()); // default to unsorted priv->sortColumn = -1; @@ -173,10 +177,20 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const return QString::fromStdString(rec->nodeStats.cleanSubVer); case Ping: return GUIUtil::formatPingTime(rec->nodeStats.dMinPing); + case Sent: + return GUIUtil::formatBytes(rec->nodeStats.nSendBytes); + case Received: + return GUIUtil::formatBytes(rec->nodeStats.nRecvBytes); } } else if (role == Qt::TextAlignmentRole) { - if (index.column() == Ping) - return (QVariant)(Qt::AlignRight | Qt::AlignVCenter); + switch (index.column()) { + case Ping: + case Sent: + case Received: + return QVariant(Qt::AlignRight | Qt::AlignVCenter); + default: + return QVariant(); + } } return QVariant(); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index cc47b67ec9..ec91d07127 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -55,8 +55,10 @@ public: enum ColumnIndex { NetNodeId = 0, Address = 1, - Subversion = 2, - Ping = 3 + Ping = 2, + Sent = 3, + Received = 4, + Subversion = 5 }; /** @name Methods overridden from QAbstractTableModel diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index d895fc1663..068c40e1e6 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -935,18 +935,6 @@ void RPCConsole::on_sldGraphRange_valueChanged(int value) setTrafficGraphRange(mins); } -QString RPCConsole::FormatBytes(quint64 bytes) -{ - if(bytes < 1024) - return QString(tr("%1 B")).arg(bytes); - if(bytes < 1024 * 1024) - return QString(tr("%1 KB")).arg(bytes / 1024); - if(bytes < 1024 * 1024 * 1024) - return QString(tr("%1 MB")).arg(bytes / 1024 / 1024); - - return QString(tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); -} - void RPCConsole::setTrafficGraphRange(int mins) { ui->trafficGraph->setGraphRangeMins(mins); @@ -955,8 +943,8 @@ void RPCConsole::setTrafficGraphRange(int mins) void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut) { - ui->lblBytesIn->setText(FormatBytes(totalBytesIn)); - ui->lblBytesOut->setText(FormatBytes(totalBytesOut)); + ui->lblBytesIn->setText(GUIUtil::formatBytes(totalBytesIn)); + ui->lblBytesOut->setText(GUIUtil::formatBytes(totalBytesOut)); } void RPCConsole::peerSelected(const QItemSelection &selected, const QItemSelection &deselected) @@ -1050,8 +1038,8 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); ui->peerLastSend->setText(stats->nodeStats.nLastSend ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastSend) : tr("never")); ui->peerLastRecv->setText(stats->nodeStats.nLastRecv ? GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nLastRecv) : tr("never")); - ui->peerBytesSent->setText(FormatBytes(stats->nodeStats.nSendBytes)); - ui->peerBytesRecv->setText(FormatBytes(stats->nodeStats.nRecvBytes)); + ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes)); + ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerConnTime->setText(GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nTimeConnected)); ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingTime)); ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingWait)); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index da06818f87..ad6e84a44a 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -123,7 +123,6 @@ Q_SIGNALS: void cmdRequest(const QString &command); private: - static QString FormatBytes(quint64 bytes); void startExecutor(); void setTrafficGraphRange(int mins); /** show detailed information on ui about selected node */ diff --git a/src/rest.cpp b/src/rest.cpp index 4d2cdfdf08..b1fc96bdf5 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -178,8 +178,11 @@ static bool rest_headers(HTTPRequest* req, } case RF_JSON: { UniValue jsonHeaders(UniValue::VARR); - for (const CBlockIndex *pindex : headers) { - jsonHeaders.push_back(blockheaderToJSON(pindex)); + { + LOCK(cs_main); + for (const CBlockIndex *pindex : headers) { + jsonHeaders.push_back(blockheaderToJSON(pindex)); + } } std::string strJSON = jsonHeaders.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); @@ -239,7 +242,11 @@ static bool rest_block(HTTPRequest* req, } case RF_JSON: { - UniValue objBlock = blockToJSON(block, pblockindex, showTxDetails); + UniValue objBlock; + { + LOCK(cs_main); + objBlock = blockToJSON(block, pblockindex, showTxDetails); + } std::string strJSON = objBlock.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); req->WriteReply(HTTP_OK, strJSON); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 68af376f35..8d01d8ba9c 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -78,6 +78,7 @@ double GetDifficulty(const CBlockIndex* blockindex) UniValue blockheaderToJSON(const CBlockIndex* blockindex) { + AssertLockHeld(cs_main); UniValue result(UniValue::VOBJ); result.push_back(Pair("hash", blockindex->GetBlockHash().GetHex())); int confirmations = -1; @@ -106,6 +107,7 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex) UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails) { + AssertLockHeld(cs_main); UniValue result(UniValue::VOBJ); result.push_back(Pair("hash", blockindex->GetBlockHash().GetHex())); int confirmations = -1; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f79439f038..0ba0e968a7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -455,7 +455,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request) { // Wait to respond until either the best block changes, OR a minute has passed and there are more transactions uint256 hashWatchedChain; - boost::system_time checktxtime; + std::chrono::steady_clock::time_point checktxtime; unsigned int nTransactionsUpdatedLastLP; if (lpval.isStr()) @@ -476,17 +476,17 @@ UniValue getblocktemplate(const JSONRPCRequest& request) // Release the wallet and main lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { - checktxtime = boost::get_system_time() + boost::posix_time::minutes(1); + checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); - boost::unique_lock<boost::mutex> lock(csBestBlock); + WaitableLock lock(csBestBlock); while (chainActive.Tip()->GetBlockHash() == hashWatchedChain && IsRPCRunning()) { - if (!cvBlockChange.timed_wait(lock, checktxtime)) + if (cvBlockChange.wait_until(lock, checktxtime) == std::cv_status::timeout) { // Timeout: Check transactions for update if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; - checktxtime += boost::posix_time::seconds(10); + checktxtime += std::chrono::seconds(10); } } } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 521b49e2a7..d042fa31d5 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -608,6 +608,7 @@ static const CRPCCommand commands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- { "control", "getmemoryinfo", &getmemoryinfo, {"mode"} }, + { "control", "logging", &logging, {"include", "exclude"}}, { "util", "validateaddress", &validateaddress, {"address"} }, /* uses wallet if enabled */ { "util", "createmultisig", &createmultisig, {"nrequired","keys"} }, { "util", "verifymessage", &verifymessage, {"address","signature","message"} }, @@ -617,7 +618,6 @@ static const CRPCCommand commands[] = { "hidden", "setmocktime", &setmocktime, {"timestamp"}}, { "hidden", "echo", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, { "hidden", "echojson", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, - { "hidden", "logging", &logging, {"include", "exclude"}}, }; void RegisterMiscRPCCommands(CRPCTable &t) diff --git a/src/sync.h b/src/sync.h index 0871c5fb4d..20556af890 100644 --- a/src/sync.h +++ b/src/sync.h @@ -10,7 +10,9 @@ #include <boost/thread/condition_variable.hpp> #include <boost/thread/mutex.hpp> -#include <boost/thread/recursive_mutex.hpp> +#include <condition_variable> +#include <thread> +#include <mutex> //////////////////////////////////////////////// @@ -21,17 +23,17 @@ /* CCriticalSection mutex; - boost::recursive_mutex mutex; + std::recursive_mutex mutex; LOCK(mutex); - boost::unique_lock<boost::recursive_mutex> criticalblock(mutex); + std::unique_lock<std::recursive_mutex> criticalblock(mutex); LOCK2(mutex1, mutex2); - boost::unique_lock<boost::recursive_mutex> criticalblock1(mutex1); - boost::unique_lock<boost::recursive_mutex> criticalblock2(mutex2); + std::unique_lock<std::recursive_mutex> criticalblock1(mutex1); + std::unique_lock<std::recursive_mutex> criticalblock2(mutex2); TRY_LOCK(mutex, name); - boost::unique_lock<boost::recursive_mutex> name(mutex, boost::try_to_lock_t); + std::unique_lock<std::recursive_mutex> name(mutex, std::try_to_lock_t); ENTER_CRITICAL_SECTION(mutex); // no RAII mutex.lock(); @@ -85,10 +87,10 @@ void static inline DeleteLock(void* cs) {} #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) /** - * Wrapped boost mutex: supports recursive locking, but no waiting + * Wrapped mutex: supports recursive locking, but no waiting * TODO: We should move away from using the recursive lock by default. */ -class CCriticalSection : public AnnotatedMixin<boost::recursive_mutex> +class CCriticalSection : public AnnotatedMixin<std::recursive_mutex> { public: ~CCriticalSection() { @@ -96,22 +98,24 @@ public: } }; -/** Wrapped boost mutex: supports waiting but not recursive locking */ -typedef AnnotatedMixin<boost::mutex> CWaitableCriticalSection; +/** Wrapped mutex: supports waiting but not recursive locking */ +typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection; -/** Just a typedef for boost::condition_variable, can be wrapped later if desired */ -typedef boost::condition_variable CConditionVariable; +/** Just a typedef for std::condition_variable, can be wrapped later if desired */ +typedef std::condition_variable CConditionVariable; + +/** Just a typedef for std::unique_lock, can be wrapped later if desired */ +typedef std::unique_lock<std::mutex> WaitableLock; #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char* pszName, const char* pszFile, int nLine); #endif -/** Wrapper around boost::unique_lock<Mutex> */ -template <typename Mutex> -class SCOPED_LOCKABLE CMutexLock +/** Wrapper around std::unique_lock<CCriticalSection> */ +class SCOPED_LOCKABLE CCriticalBlock { private: - boost::unique_lock<Mutex> lock; + std::unique_lock<CCriticalSection> lock; void Enter(const char* pszName, const char* pszFile, int nLine) { @@ -136,7 +140,7 @@ private: } public: - CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, boost::defer_lock) + CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock) { if (fTry) TryEnter(pszName, pszFile, nLine); @@ -144,18 +148,18 @@ public: Enter(pszName, pszFile, nLine); } - CMutexLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) + CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) { if (!pmutexIn) return; - lock = boost::unique_lock<Mutex>(*pmutexIn, boost::defer_lock); + lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock); if (fTry) TryEnter(pszName, pszFile, nLine); else Enter(pszName, pszFile, nLine); } - ~CMutexLock() UNLOCK_FUNCTION() + ~CCriticalBlock() UNLOCK_FUNCTION() { if (lock.owns_lock()) LeaveCritical(); @@ -167,8 +171,6 @@ public: } }; -typedef CMutexLock<CCriticalSection> CCriticalBlock; - #define PASTE(x, y) x ## y #define PASTE2(x, y) PASTE(x, y) diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index b88ad5ed1b..d1f9e63ecf 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -40,8 +40,138 @@ CService ip(uint32_t i) static NodeId id = 0; +void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds); + BOOST_FIXTURE_TEST_SUITE(DoS_tests, TestingSetup) +// Test eviction of an outbound peer whose chain never advances +// Mock a node connection, and use mocktime to simulate a peer +// which never sends any headers messages. PeerLogic should +// decide to evict that outbound peer, after the appropriate timeouts. +// Note that we protect 4 outbound nodes from being subject to +// this logic; this test takes advantage of that protection only +// being applied to nodes which send headers with sufficient +// work. +BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) +{ + std::atomic<bool> interruptDummy(false); + + // Mock an outbound peer + CAddress addr1(ip(0xa0b0c001), NODE_NONE); + CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", /*fInboundIn=*/ false); + dummyNode1.SetSendVersion(PROTOCOL_VERSION); + + peerLogic->InitializeNode(&dummyNode1); + dummyNode1.nVersion = 1; + dummyNode1.fSuccessfullyConnected = true; + + // This test requires that we have a chain with non-zero work. + BOOST_CHECK(chainActive.Tip() != nullptr); + BOOST_CHECK(chainActive.Tip()->nChainWork > 0); + + // Test starts here + peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders + BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); + dummyNode1.vSendMsg.clear(); + + int64_t nStartTime = GetTime(); + // Wait 21 minutes + SetMockTime(nStartTime+21*60); + peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders + BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); + // Wait 3 more minutes + SetMockTime(nStartTime+24*60); + peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect + BOOST_CHECK(dummyNode1.fDisconnect == true); + SetMockTime(0); + + bool dummy; + peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); +} + +void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic) +{ + CAddress addr(ip(GetRandInt(0xffffffff)), NODE_NONE); + vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", /*fInboundIn=*/ false)); + CNode &node = *vNodes.back(); + node.SetSendVersion(PROTOCOL_VERSION); + + peerLogic.InitializeNode(&node); + node.nVersion = 1; + node.fSuccessfullyConnected = true; + + CConnmanTest::AddNode(node); +} + +BOOST_AUTO_TEST_CASE(stale_tip_peer_management) +{ + const Consensus::Params& consensusParams = Params().GetConsensus(); + constexpr int nMaxOutbound = 8; + CConnman::Options options; + options.nMaxConnections = 125; + options.nMaxOutbound = nMaxOutbound; + options.nMaxFeeler = 1; + + connman->Init(options); + std::vector<CNode *> vNodes; + + // Mock some outbound peers + for (int i=0; i<nMaxOutbound; ++i) { + AddRandomOutboundPeer(vNodes, *peerLogic); + } + + peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); + + // No nodes should be marked for disconnection while we have no extra peers + for (const CNode *node : vNodes) { + BOOST_CHECK(node->fDisconnect == false); + } + + SetMockTime(GetTime() + 3*consensusParams.nPowTargetSpacing + 1); + + // Now tip should definitely be stale, and we should look for an extra + // outbound peer + peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); + BOOST_CHECK(connman->GetTryNewOutboundPeer()); + + // Still no peers should be marked for disconnection + for (const CNode *node : vNodes) { + BOOST_CHECK(node->fDisconnect == false); + } + + // If we add one more peer, something should get marked for eviction + // on the next check (since we're mocking the time to be in the future, the + // required time connected check should be satisfied). + AddRandomOutboundPeer(vNodes, *peerLogic); + + peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); + for (int i=0; i<nMaxOutbound; ++i) { + BOOST_CHECK(vNodes[i]->fDisconnect == false); + } + // Last added node should get marked for eviction + BOOST_CHECK(vNodes.back()->fDisconnect == true); + + vNodes.back()->fDisconnect = false; + + // Update the last announced block time for the last + // peer, and check that the next newest node gets evicted. + UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime()); + + peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); + for (int i=0; i<nMaxOutbound-1; ++i) { + BOOST_CHECK(vNodes[i]->fDisconnect == false); + } + BOOST_CHECK(vNodes[nMaxOutbound-1]->fDisconnect == true); + BOOST_CHECK(vNodes.back()->fDisconnect == false); + + bool dummy; + for (const CNode *node : vNodes) { + peerLogic->FinalizeNode(node->GetId(), dummy); + } + + CConnmanTest::ClearNodes(); +} + BOOST_AUTO_TEST_CASE(DoS_banning) { std::atomic<bool> interruptDummy(false); @@ -71,6 +201,10 @@ BOOST_AUTO_TEST_CASE(DoS_banning) Misbehaving(dummyNode2.GetId(), 50); peerLogic->SendMessages(&dummyNode2, interruptDummy); BOOST_CHECK(connman->IsBanned(addr2)); + + bool dummy; + peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); + peerLogic->FinalizeNode(dummyNode2.GetId(), dummy); } BOOST_AUTO_TEST_CASE(DoS_banscore) @@ -95,6 +229,9 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) peerLogic->SendMessages(&dummyNode1, interruptDummy); BOOST_CHECK(connman->IsBanned(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); + + bool dummy; + peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); } BOOST_AUTO_TEST_CASE(DoS_bantime) @@ -121,6 +258,9 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) SetMockTime(nStartTime+60*60*24+1); BOOST_CHECK(!connman->IsBanned(addr)); + + bool dummy; + peerLogic->FinalizeNode(dummyNode.GetId(), dummy); } CTransactionRef RandomOrphan() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 41e0626eb9..2851808cf4 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -335,23 +335,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_CHECK_THROW(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error); mempool.clear(); - // invalid (pre-p2sh) txn in mempool, template creation fails - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].prevout.n = 0; - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vout[0].nValue = BLOCKSUBSIDY-LOWFEE; - script = CScript() << OP_0; - tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script)); - hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - tx.vin[0].prevout.hash = hash; - tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end()); - tx.vout[0].nValue -= LOWFEE; - hash = tx.GetHash(); - mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); - BOOST_CHECK_THROW(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error); - mempool.clear(); - // double spend txn pair in mempool, template creation fails tx.vin[0].prevout.hash = txFirst[0]->GetHash(); tx.vin[0].scriptSig = CScript() << OP_1; @@ -391,6 +374,24 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) chainActive.SetTip(next); } BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + + // invalid p2sh txn in mempool, template creation fails + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].prevout.n = 0; + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout[0].nValue = BLOCKSUBSIDY-LOWFEE; + script = CScript() << OP_0; + tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script)); + hash = tx.GetHash(); + mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end()); + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + mempool.addUnchecked(hash, entry.Fee(LOWFEE).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); + BOOST_CHECK_THROW(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error); + mempool.clear(); + // Delete the dummy blocks again. while (chainActive.Tip()->nHeight > nHeight) { CBlockIndex* del = chainActive.Tip(); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 79bc48a118..f9ce52c594 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -25,6 +25,18 @@ #include <memory> +void CConnmanTest::AddNode(CNode& node) +{ + LOCK(g_connman->cs_vNodes); + g_connman->vNodes.push_back(&node); +} + +void CConnmanTest::ClearNodes() +{ + LOCK(g_connman->cs_vNodes); + g_connman->vNodes.clear(); +} + uint256 insecure_rand_seed = GetRandHash(); FastRandomContext insecure_rand_ctx(insecure_rand_seed); @@ -86,7 +98,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha threadGroup.create_thread(&ThreadScriptCheck); g_connman = std::unique_ptr<CConnman>(new CConnman(0x1337, 0x1337)); // Deterministic randomness for tests. connman = g_connman.get(); - peerLogic.reset(new PeerLogicValidation(connman)); + peerLogic.reset(new PeerLogicValidation(connman, scheduler)); } TestingSetup::~TestingSetup() @@ -106,6 +118,9 @@ TestingSetup::~TestingSetup() TestChain100Setup::TestChain100Setup() : TestingSetup(CBaseChainParams::REGTEST) { + // CreateAndProcessBlock() does not support building SegWit blocks, so don't activate in these tests. + // TODO: fix the code to support SegWit blocks. + UpdateVersionBitsParameters(Consensus::DEPLOYMENT_SEGWIT, 0, Consensus::BIP9Deployment::NO_TIMEOUT); // Generate a 100-block chain: coinbaseKey.MakeNewKey(true); CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index 2390aca342..62ded2aaf5 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -49,6 +49,12 @@ struct BasicTestingSetup { * Included are data directory, coins database, script check threads setup. */ class CConnman; +class CNode; +struct CConnmanTest { + static void AddNode(CNode& node); + static void ClearNodes(); +}; + class PeerLogicValidation; struct TestingSetup: public BasicTestingSetup { CCoinsViewDB *pcoinsdbview; diff --git a/src/test/test_bitcoin_fuzzy.cpp b/src/test/test_bitcoin_fuzzy.cpp index 581ad2ffa0..6694c5caa8 100644 --- a/src/test/test_bitcoin_fuzzy.cpp +++ b/src/test/test_bitcoin_fuzzy.cpp @@ -19,6 +19,7 @@ #include "undo.h" #include "version.h" #include "pubkey.h" +#include "blockencodings.h" #include <stdint.h> #include <unistd.h> @@ -45,6 +46,8 @@ enum TEST_ID { CBLOOMFILTER_DESERIALIZE, CDISKBLOCKINDEX_DESERIALIZE, CTXOUTCOMPRESSOR_DESERIALIZE, + BLOCKTRANSACTIONS_DESERIALIZE, + BLOCKTRANSACTIONSREQUEST_DESERIALIZE, TEST_ID_END }; @@ -245,6 +248,26 @@ int test_one_input(std::vector<uint8_t> buffer) { break; } + case BLOCKTRANSACTIONS_DESERIALIZE: + { + try + { + BlockTransactions bt; + ds >> bt; + } catch (const std::ios_base::failure& e) {return 0;} + + break; + } + case BLOCKTRANSACTIONSREQUEST_DESERIALIZE: + { + try + { + BlockTransactionsRequest btr; + ds >> btr; + } catch (const std::ios_base::failure& e) {return 0;} + + break; + } default: return 0; } diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 882afb2e20..db537d3932 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -32,6 +32,12 @@ public: int GetStateSinceHeightFor(const CBlockIndex* pindexPrev) const { return AbstractThresholdConditionChecker::GetStateSinceHeightFor(pindexPrev, paramsDummy, cache); } }; +class TestAlwaysActiveConditionChecker : public TestConditionChecker +{ +public: + int64_t BeginTime(const Consensus::Params& params) const override { return Consensus::BIP9Deployment::ALWAYS_ACTIVE; } +}; + #define CHECKERS 6 class VersionBitsTester @@ -43,6 +49,8 @@ class VersionBitsTester // The first one performs all checks, the second only 50%, the third only 25%, etc... // This is to test whether lack of cached information leads to the same results. TestConditionChecker checker[CHECKERS]; + // Another 6 that assume always active activation + TestAlwaysActiveConditionChecker checker_always[CHECKERS]; // Test counter (to identify failures) int num; @@ -56,6 +64,7 @@ public: } for (unsigned int i = 0; i < CHECKERS; i++) { checker[i] = TestConditionChecker(); + checker_always[i] = TestAlwaysActiveConditionChecker(); } vpblock.clear(); return *this; @@ -82,6 +91,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateSinceHeightFor(vpblock.empty() ? nullptr : vpblock.back()) == height, strprintf("Test %i for StateSinceHeight", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateSinceHeightFor(vpblock.empty() ? nullptr : vpblock.back()) == 0, strprintf("Test %i for StateSinceHeight (always active)", num)); } } num++; @@ -92,6 +102,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_DEFINED, strprintf("Test %i for DEFINED", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE (always active)", num)); } } num++; @@ -102,6 +113,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_STARTED, strprintf("Test %i for STARTED", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE (always active)", num)); } } num++; @@ -112,6 +124,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_LOCKED_IN, strprintf("Test %i for LOCKED_IN", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE (always active)", num)); } } num++; @@ -122,6 +135,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE (always active)", num)); } } num++; @@ -132,6 +146,7 @@ public: for (int i = 0; i < CHECKERS; i++) { if (InsecureRandBits(i) == 0) { BOOST_CHECK_MESSAGE(checker[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_FAILED, strprintf("Test %i for FAILED", num)); + BOOST_CHECK_MESSAGE(checker_always[i].GetStateFor(vpblock.empty() ? nullptr : vpblock.back()) == THRESHOLD_ACTIVE, strprintf("Test %i for ACTIVE (always active)", num)); } } num++; diff --git a/src/tinyformat.h b/src/tinyformat.h index 2e453e56bb..d34cfaa94f 100644 --- a/src/tinyformat.h +++ b/src/tinyformat.h @@ -495,7 +495,11 @@ namespace detail { class FormatArg { public: - FormatArg() {} + FormatArg() + : m_value(nullptr), + m_formatImpl(nullptr), + m_toIntImpl(nullptr) + { } template<typename T> explicit FormatArg(const T& value) @@ -507,11 +511,15 @@ class FormatArg void format(std::ostream& out, const char* fmtBegin, const char* fmtEnd, int ntrunc) const { + assert(m_value); + assert(m_formatImpl); m_formatImpl(out, fmtBegin, fmtEnd, ntrunc, m_value); } int toInt() const { + assert(m_value); + assert(m_toIntImpl); return m_toIntImpl(m_value); } @@ -712,23 +720,27 @@ inline const char* streamStateFromFormat(std::ostream& out, bool& spacePadPositi break; case 'X': out.setf(std::ios::uppercase); + // Falls through case 'x': case 'p': out.setf(std::ios::hex, std::ios::basefield); intConversion = true; break; case 'E': out.setf(std::ios::uppercase); + // Falls through case 'e': out.setf(std::ios::scientific, std::ios::floatfield); out.setf(std::ios::dec, std::ios::basefield); break; case 'F': out.setf(std::ios::uppercase); + // Falls through case 'f': out.setf(std::ios::fixed, std::ios::floatfield); break; case 'G': out.setf(std::ios::uppercase); + // Falls through case 'g': out.setf(std::ios::dec, std::ios::basefield); // As in boost::format, let stream decide float format. diff --git a/src/validation.cpp b/src/validation.cpp index 7f6697c780..a98036f03c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -156,6 +156,26 @@ namespace { /** chainwork for the last block that preciousblock has been applied to. */ arith_uint256 nLastPreciousChainwork = 0; + /** In order to efficiently track invalidity of headers, we keep the set of + * blocks which we tried to connect and found to be invalid here (ie which + * were set to BLOCK_FAILED_VALID since the last restart). We can then + * walk this set and check if a new header is a descendant of something in + * this set, preventing us from having to walk mapBlockIndex when we try + * to connect a bad block and fail. + * + * While this is more complicated than marking everything which descends + * from an invalid block as invalid at the time we discover it to be + * invalid, doing so would require walking all of mapBlockIndex to find all + * descendants. Since this case should be very rare, keeping track of all + * BLOCK_FAILED_VALID blocks in a set should be just fine and work just as + * well. + * + * Because we alreardy walk mapBlockIndex in height-order at startup, we go + * ahead and mark descendants of invalid blocks as FAILED_CHILD at that time, + * instead of putting things in this set. + */ + std::set<CBlockIndex*> g_failed_blocks; + /** Dirty block index entries. */ std::set<CBlockIndex*> setDirtyBlockIndex; @@ -939,6 +959,9 @@ bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus return error("%s: txid mismatch", __func__); return true; } + + // transaction not found in index, nothing more can be done + return false; } if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it @@ -1177,6 +1200,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) void static InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { if (!state.CorruptionPossible()) { pindex->nStatus |= BLOCK_FAILED_VALID; + g_failed_blocks.insert(pindex); setDirtyBlockIndex.insert(pindex); setBlockIndexCandidates.erase(pindex); InvalidChainFound(pindex); @@ -1587,11 +1611,12 @@ static ThresholdConditionCache warningcache[VERSIONBITS_NUM_BITS]; static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& consensusparams) { AssertLockHeld(cs_main); - // BIP16 didn't become active until Apr 1 2012 - int64_t nBIP16SwitchTime = 1333238400; - bool fStrictPayToScriptHash = (pindex->GetBlockTime() >= nBIP16SwitchTime); + unsigned int flags = SCRIPT_VERIFY_NONE; - unsigned int flags = fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE; + // Start enforcing P2SH (BIP16) + if (pindex->nHeight >= consensusparams.BIP16Height) { + flags |= SCRIPT_VERIFY_P2SH; + } // Start enforcing the DERSIG (BIP66) rule if (pindex->nHeight >= consensusparams.BIP66Height) { @@ -2530,17 +2555,18 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C { AssertLockHeld(cs_main); - // Mark the block itself as invalid. - pindex->nStatus |= BLOCK_FAILED_VALID; - setDirtyBlockIndex.insert(pindex); - setBlockIndexCandidates.erase(pindex); + // We first disconnect backwards and then mark the blocks as invalid. + // This prevents a case where pruned nodes may fail to invalidateblock + // and be left unable to start as they have no tip candidates (as there + // are no blocks that meet the "have data and are not invalid per + // nStatus" criteria for inclusion in setBlockIndexCandidates). + + bool pindex_was_in_chain = false; + CBlockIndex *invalid_walk_tip = chainActive.Tip(); DisconnectedBlockTransactions disconnectpool; while (chainActive.Contains(pindex)) { - CBlockIndex *pindexWalk = chainActive.Tip(); - pindexWalk->nStatus |= BLOCK_FAILED_CHILD; - setDirtyBlockIndex.insert(pindexWalk); - setBlockIndexCandidates.erase(pindexWalk); + pindex_was_in_chain = true; // ActivateBestChain considers blocks already in chainActive // unconditionally valid already, so force disconnect away from it. if (!DisconnectTip(state, chainparams, &disconnectpool)) { @@ -2551,6 +2577,21 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C } } + // Now mark the blocks we just disconnected as descendants invalid + // (note this may not be all descendants). + while (pindex_was_in_chain && invalid_walk_tip != pindex) { + invalid_walk_tip->nStatus |= BLOCK_FAILED_CHILD; + setDirtyBlockIndex.insert(invalid_walk_tip); + setBlockIndexCandidates.erase(invalid_walk_tip); + invalid_walk_tip = invalid_walk_tip->pprev; + } + + // Mark the block itself as invalid. + pindex->nStatus |= BLOCK_FAILED_VALID; + setDirtyBlockIndex.insert(pindex); + setBlockIndexCandidates.erase(pindex); + g_failed_blocks.insert(pindex); + // DisconnectTip will add transactions to disconnectpool; try to add these // back to the mempool. UpdateMempoolForReorg(disconnectpool, true); @@ -2588,6 +2629,7 @@ bool ResetBlockFailureFlags(CBlockIndex *pindex) { // Reset invalid block marker if it was pointing to one of those. pindexBestInvalid = nullptr; } + g_failed_blocks.erase(it->second); } it++; } @@ -3063,6 +3105,21 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state return state.DoS(100, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); + + if (!pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) { + for (const CBlockIndex* failedit : g_failed_blocks) { + if (pindexPrev->GetAncestor(failedit->nHeight) == failedit) { + assert(failedit->nStatus & BLOCK_FAILED_VALID); + CBlockIndex* invalid_walk = pindexPrev; + while (invalid_walk != failedit) { + invalid_walk->nStatus |= BLOCK_FAILED_CHILD; + setDirtyBlockIndex.insert(invalid_walk); + invalid_walk = invalid_walk->pprev; + } + return state.DoS(100, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); + } + } + } } if (pindex == nullptr) pindex = AddToBlockIndex(block); @@ -3076,13 +3133,15 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state } // Exposed wrapper for AcceptBlockHeader -bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) +bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, CBlockHeader *first_invalid) { + if (first_invalid != nullptr) first_invalid->SetNull(); { LOCK(cs_main); for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast if (!AcceptBlockHeader(header, state, chainparams, &pindex)) { + if (first_invalid) *first_invalid = header; return false; } if (ppindex) { @@ -3112,7 +3171,7 @@ static bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidation // process an unrequested block if it's new and has enough work to // advance our tip, and isn't too many blocks ahead. bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA; - bool fHasMoreWork = (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true); + bool fHasMoreOrSameWork = (chainActive.Tip() ? pindex->nChainWork >= chainActive.Tip()->nChainWork : true); // Blocks that are too out-of-order needlessly limit the effectiveness of // pruning, because pruning will not delete block files that contain any // blocks which are too close in height to the tip. Apply this test @@ -3129,9 +3188,15 @@ static bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidation // and unrequested blocks. if (fAlreadyHave) return true; if (!fRequested) { // If we didn't ask for it: - if (pindex->nTx != 0) return true; // This is a previously-processed block that was pruned - if (!fHasMoreWork) return true; // Don't process less-work chains - if (fTooFarAhead) return true; // Block height is too high + if (pindex->nTx != 0) return true; // This is a previously-processed block that was pruned + if (!fHasMoreOrSameWork) return true; // Don't process less-work chains + if (fTooFarAhead) return true; // Block height is too high + + // Protect against DoS attacks from low-work chains. + // If our tip is behind, a peer could try to send us + // low-work blocks on a fake chain that we would never + // request; don't process these. + if (pindex->nChainWork < nMinimumChainWork) return true; } if (fNewBlock) *fNewBlock = true; @@ -3483,6 +3548,10 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) pindex->nChainTx = pindex->nTx; } } + if (!(pindex->nStatus & BLOCK_FAILED_MASK) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_MASK)) { + pindex->nStatus |= BLOCK_FAILED_CHILD; + setDirtyBlockIndex.insert(pindex); + } if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->nChainTx || pindex->pprev == nullptr)) setBlockIndexCandidates.insert(pindex); if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) @@ -3873,6 +3942,7 @@ void UnloadBlockIndex() nLastBlockFile = 0; nBlockSequenceId = 1; setDirtyBlockIndex.clear(); + g_failed_blocks.clear(); setDirtyFileInfo.clear(); versionbitscache.Clear(); for (int b = 0; b < VERSIONBITS_NUM_BITS; b++) { @@ -4287,8 +4357,9 @@ bool LoadMempool(void) } int64_t count = 0; - int64_t skipped = 0; + int64_t expired = 0; int64_t failed = 0; + int64_t already_there = 0; int64_t nNow = GetTime(); try { @@ -4319,10 +4390,18 @@ bool LoadMempool(void) if (state.IsValid()) { ++count; } else { - ++failed; + // mempool may contain the transaction already, e.g. from + // wallet(s) having loaded it while we were processing + // mempool transactions; consider these as valid, instead of + // failed, but mark them as 'already there' + if (mempool.exists(tx->GetHash())) { + ++already_there; + } else { + ++failed; + } } } else { - ++skipped; + ++expired; } if (ShutdownRequested()) return false; @@ -4338,7 +4417,7 @@ bool LoadMempool(void) return false; } - LogPrintf("Imported mempool transactions from disk: %i successes, %i failed, %i expired\n", count, failed, skipped); + LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there\n", count, failed, expired, already_there); return true; } diff --git a/src/validation.h b/src/validation.h index 6bc52753c5..93669de6c4 100644 --- a/src/validation.h +++ b/src/validation.h @@ -247,8 +247,9 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<cons * @param[out] state This may be set to an Error state if any error occurred processing them * @param[in] chainparams The params for the chain we want to connect to * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers + * @param[out] first_invalid First header that fails validation, if one exists */ -bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex=nullptr); +bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex=nullptr, CBlockHeader *first_invalid=nullptr); /** Check whether enough disk space is available for an incoming block */ bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 64ae939672..fc1acb3258 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -27,6 +27,11 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* int64_t nTimeStart = BeginTime(params); int64_t nTimeTimeout = EndTime(params); + // Check if this deployment is always active. + if (nTimeStart == Consensus::BIP9Deployment::ALWAYS_ACTIVE) { + return THRESHOLD_ACTIVE; + } + // A block's state is always the same as that of the first of its period, so it is computed based on a pindexPrev whose height equals a multiple of nPeriod - 1. if (pindexPrev != nullptr) { pindexPrev = pindexPrev->GetAncestor(pindexPrev->nHeight - ((pindexPrev->nHeight + 1) % nPeriod)); @@ -136,6 +141,11 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const { + int64_t start_time = BeginTime(params); + if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE) { + return 0; + } + const ThresholdState initialState = GetStateFor(pindexPrev, params, cache); // BIP 9 about state DEFINED: "The genesis block is by definition in this state for each deployment." diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index d66ba48421..5d48b01c2e 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -20,6 +20,40 @@ #include <boost/thread.hpp> +namespace { +//! Make sure database has a unique fileid within the environment. If it +//! doesn't, throw an error. BDB caches do not work properly when more than one +//! open database has the same fileid (values written to one database may show +//! up in reads to other databases). +//! +//! BerkeleyDB generates unique fileids by default +//! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html), +//! so bitcoin should never create different databases with the same fileid, but +//! this error can be triggered if users manually copy database files. +void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) +{ + if (env.IsMock()) return; + + u_int8_t fileid[DB_FILE_ID_LEN]; + int ret = db.get_mpf()->get_fileid(fileid); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Can't open database %s (get_fileid failed with %d)", filename, ret)); + } + + for (const auto& item : env.mapDb) { + u_int8_t item_fileid[DB_FILE_ID_LEN]; + if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 && + memcmp(fileid, item_fileid, sizeof(fileid)) == 0) { + const char* item_filename = nullptr; + item.second->get_dbname(&item_filename, nullptr); + throw std::runtime_error(strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", filename, + HexStr(std::begin(item_fileid), std::end(item_fileid)), + item_filename ? item_filename : "(unknown database)")); + } + } +} +} // namespace + // // CDB // @@ -379,35 +413,34 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (!env->Open(GetDataDir())) throw std::runtime_error("CDB: Failed to open database environment."); - strFile = strFilename; - ++env->mapFileUseCount[strFile]; - pdb = env->mapDb[strFile]; + pdb = env->mapDb[strFilename]; if (pdb == nullptr) { int ret; - pdb = new Db(env->dbenv, 0); + std::unique_ptr<Db> pdb_temp(new Db(env->dbenv, 0)); bool fMockDb = env->IsMock(); if (fMockDb) { - DbMpoolFile* mpf = pdb->get_mpf(); + DbMpoolFile* mpf = pdb_temp->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); - if (ret != 0) - throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFile)); + if (ret != 0) { + throw std::runtime_error(strprintf("CDB: Failed to configure for no temp file backing for database %s", strFilename)); + } } - ret = pdb->open(nullptr, // Txn pointer - fMockDb ? nullptr : strFile.c_str(), // Filename - fMockDb ? strFile.c_str() : "main", // Logical db name - DB_BTREE, // Database type - nFlags, // Flags + ret = pdb_temp->open(nullptr, // Txn pointer + fMockDb ? nullptr : strFilename.c_str(), // Filename + fMockDb ? strFilename.c_str() : "main", // Logical db name + DB_BTREE, // Database type + nFlags, // Flags 0); if (ret != 0) { - delete pdb; - pdb = nullptr; - --env->mapFileUseCount[strFile]; - strFile = ""; throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } + CheckUniqueFileid(*env, strFilename, *pdb_temp); + + pdb = pdb_temp.release(); + env->mapDb[strFilename] = pdb; if (fCreate && !Exists(std::string("version"))) { bool fTmp = fReadOnly; @@ -415,9 +448,9 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb WriteVersion(CLIENT_VERSION); fReadOnly = fTmp; } - - env->mapDb[strFile] = pdb; } + ++env->mapFileUseCount[strFilename]; + strFile = strFilename; } } @@ -672,6 +705,11 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) pathDest /= strFile; try { + if (fs::equivalent(pathSrc, pathDest)) { + LogPrintf("cannot backup to wallet source file %s\n", pathDest.string()); + return false; + } + fs::copy_file(pathSrc, pathDest, fs::copy_option::overwrite_if_exists); LogPrintf("copied %s to %s\n", strFile, pathDest.string()); return true; diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 6abd060714..b5c5709ec9 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -196,7 +196,13 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin // moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment. CFeeRate minMempoolFeeRate = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { - vErrors.push_back(strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK()))); + vErrors.push_back(strprintf( + "New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- " + "the totalFee value should be at least %s or the settxfee value should be at least %s to add transaction", + FormatMoney(nNewFeeRate.GetFeePerK()), + FormatMoney(minMempoolFeeRate.GetFeePerK()), + FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), + FormatMoney(minMempoolFeeRate.GetFeePerK()))); currentResult = BumpFeeResult::WALLET_ERROR; return; } @@ -267,7 +273,7 @@ bool CFeeBumper::commit(CWallet *pWallet) CValidationState state; if (!pWallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. - vErrors.push_back(strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason())); + vErrors.push_back(strprintf("The transaction was rejected: %s", state.GetRejectReason())); return false; } @@ -275,7 +281,7 @@ bool CFeeBumper::commit(CWallet *pWallet) if (state.IsInvalid()) { // This can happen if the mempool rejected the transaction. Report // what happened in the "errors" response. - vErrors.push_back(strprintf("Error: The transaction was rejected: %s", FormatStateMessage(state))); + vErrors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); } // mark the original tx as bumped @@ -284,7 +290,7 @@ bool CFeeBumper::commit(CWallet *pWallet) // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction // replaced does not succeed for some reason. - vErrors.push_back("Error: Created new bumpfee transaction but could not mark the original transaction as replaced."); + vErrors.push_back("Created new bumpfee transaction but could not mark the original transaction as replaced"); } return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 67ef4edd5b..25b5327431 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -964,7 +964,7 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6 pwallet->SetAddressBook(vchAddress, label, "receive"); if (pwallet->HaveKey(vchAddress)) { - return false; + throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script"); } pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a8090add4c..9854ace67a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1893,19 +1893,20 @@ UniValue listsinceblock(const JSONRPCRequest& request) int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; - if (!request.params[0].isNull()) { + if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { uint256 blockId; blockId.SetHex(request.params[0].get_str()); BlockMap::iterator it = mapBlockIndex.find(blockId); - if (it != mapBlockIndex.end()) { - paltindex = pindex = it->second; - if (chainActive[pindex->nHeight] != pindex) { - // the block being asked for is a part of a deactivated chain; - // we don't want to depend on its perceived height in the block - // chain, we want to instead use the last common ancestor - pindex = chainActive.FindFork(pindex); - } + if (it == mapBlockIndex.end()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } + paltindex = pindex = it->second; + if (chainActive[pindex->nHeight] != pindex) { + // the block being asked for is a part of a deactivated chain; + // we don't want to depend on its perceived height in the block + // chain, we want to instead use the last common ancestor + pindex = chainActive.FindFork(pindex); } } @@ -2179,7 +2180,7 @@ UniValue walletpassphrase(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 2)) { + if (request.fHelp || request.params.size() != 2) { throw std::runtime_error( "walletpassphrase \"passphrase\" timeout\n" "\nStores the wallet decryption key in memory for 'timeout' seconds.\n" @@ -2243,7 +2244,7 @@ UniValue walletpassphrasechange(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 2)) { + if (request.fHelp || request.params.size() != 2) { throw std::runtime_error( "walletpassphrasechange \"oldpassphrase\" \"newpassphrase\"\n" "\nChanges the wallet passphrase from 'oldpassphrase' to 'newpassphrase'.\n" @@ -2294,7 +2295,7 @@ UniValue walletlock(const JSONRPCRequest& request) return NullUniValue; } - if (pwallet->IsCrypted() && (request.fHelp || request.params.size() != 0)) { + if (request.fHelp || request.params.size() != 0) { throw std::runtime_error( "walletlock\n" "\nRemoves the wallet encryption key from memory, locking the wallet.\n" @@ -2334,7 +2335,7 @@ UniValue encryptwallet(const JSONRPCRequest& request) return NullUniValue; } - if (!pwallet->IsCrypted() && (request.fHelp || request.params.size() != 1)) { + if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( "encryptwallet \"passphrase\"\n" "\nEncrypts the wallet with 'passphrase'. This is for first time encryption.\n" @@ -3233,7 +3234,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) "}\n" "\nExamples:\n" + HelpExampleCli("rescanblockchain", "100000 120000") - + HelpExampleRpc("rescanblockchain", "100000 120000") + + HelpExampleRpc("rescanblockchain", "100000, 120000") ); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3672ecea5b..543bef32ad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3881,7 +3881,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) // Top up the keypool if (!walletInstance->TopUpKeyPool()) { InitError(_("Unable to generate initial keys") += "\n"); - return NULL; + return nullptr; } walletInstance->SetBestChain(chainActive.GetLocator()); diff --git a/test/functional/importmulti.py b/test/functional/importmulti.py index c1a42870ec..a691595f15 100755 --- a/test/functional/importmulti.py +++ b/test/functional/importmulti.py @@ -160,6 +160,18 @@ class ImportMultiTest (BitcoinTestFramework): assert_equal(address_assert['ismine'], True) assert_equal(address_assert['timestamp'], timestamp) + self.log.info("Should not import an address with private key if is already imported") + result = self.nodes[1].importmulti([{ + "scriptPubKey": { + "address": address['address'] + }, + "timestamp": "now", + "keys": [ self.nodes[0].dumpprivkey(address['address']) ] + }]) + assert_equal(result[0]['success'], False) + assert_equal(result[0]['error']['code'], -4) + assert_equal(result[0]['error']['message'], 'The wallet already contains the private key for this address or script') + # Address + Private key + watchonly self.log.info("Should not import an address with private key and with watchonly") address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) diff --git a/test/functional/listsinceblock.py b/test/functional/listsinceblock.py index 6f428388ec..67e7744bf8 100755 --- a/test/functional/listsinceblock.py +++ b/test/functional/listsinceblock.py @@ -5,7 +5,7 @@ """Test the listsincelast RPC.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_array_result, assert_raises_rpc_error class ListSinceBlockTest (BitcoinTestFramework): def set_test_params(self): @@ -16,10 +16,43 @@ class ListSinceBlockTest (BitcoinTestFramework): self.nodes[2].generate(101) self.sync_all() + self.test_no_blockhash() + self.test_invalid_blockhash() self.test_reorg() self.test_double_spend() self.test_double_send() + def test_no_blockhash(self): + txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) + blockhash, = self.nodes[2].generate(1) + self.sync_all() + + txs = self.nodes[0].listtransactions() + assert_array_result(txs, {"txid": txid}, { + "category": "receive", + "amount": 1, + "blockhash": blockhash, + "confirmations": 1, + }) + assert_equal( + self.nodes[0].listsinceblock(), + {"lastblock": blockhash, + "removed": [], + "transactions": txs}) + assert_equal( + self.nodes[0].listsinceblock(""), + {"lastblock": blockhash, + "removed": [], + "transactions": txs}) + + def test_invalid_blockhash(self): + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "42759cde25462784395a337460bde75f58e73d3f08bd31fdc3507cbac856a2c4") + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "0000000000000000000000000000000000000000000000000000000000000000") + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "invalid-hex") + def test_reorg(self): ''' `listsinceblock` did not behave correctly when handed a block that was diff --git a/test/functional/minchainwork.py b/test/functional/minchainwork.py index c7579d2548..35cd7ad141 100755 --- a/test/functional/minchainwork.py +++ b/test/functional/minchainwork.py @@ -27,6 +27,7 @@ class MinimumChainWorkTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 + self.extra_args = [[], ["-minimumchainwork=0x65"], ["-minimumchainwork=0x65"]] self.node_min_work = [0, 101, 101] @@ -74,6 +75,13 @@ class MinimumChainWorkTest(BitcoinTestFramework): self.nodes[0].generate(1) self.log.info("Verifying nodes are all synced") + + # Because nodes in regtest are all manual connections (eg using + # addnode), node1 should not have disconnected node0. If not for that, + # we'd expect node1 to have disconnected node0 for serving an + # insufficient work chain, in which case we'd need to reconnect them to + # continue the test. + self.sync_all() self.log.info("Blockcounts: %s", [n.getblockcount() for n in self.nodes]) diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py index f55da76819..7a0fbce477 100755 --- a/test/functional/multiwallet.py +++ b/test/functional/multiwallet.py @@ -7,6 +7,7 @@ Verify that a bitcoind node can load multiple wallet files """ import os +import shutil from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error @@ -29,6 +30,11 @@ class MultiWalletTest(BitcoinTestFramework): os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', '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(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'), + os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22')) + self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + # should not initialize if wallet file is a symlink os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12')) self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') diff --git a/test/functional/nulldummy.py b/test/functional/nulldummy.py index 91c4550653..7bc7c168f4 100755 --- a/test/functional/nulldummy.py +++ b/test/functional/nulldummy.py @@ -40,7 +40,9 @@ class NULLDUMMYTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness']] + # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through + # normal segwit activation here (and don't use the default always-on behaviour). + self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999']] def run_test(self): self.address = self.nodes[0].getnewaddress() diff --git a/test/functional/p2p-acceptblock.py b/test/functional/p2p-acceptblock.py index 5b6429b410..220b776369 100755 --- a/test/functional/p2p-acceptblock.py +++ b/test/functional/p2p-acceptblock.py @@ -4,37 +4,32 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test processing of unrequested blocks. -Since behavior differs when receiving unrequested blocks from whitelisted peers -versus non-whitelisted peers, this tests the behavior of both (effectively two -separate tests running in parallel). +Setup: two nodes, node0+node1, not connected to each other. Node1 will have +nMinimumChainWork set to 0x10, so it won't process low-work unrequested blocks. -Setup: two nodes, node0 and node1, not connected to each other. Node0 does not -whitelist localhost, but node1 does. They will each be on their own chain for -this test. - -We have one NodeConn connection to each, test_node and white_node respectively. +We have one NodeConn connection to node0 called test_node, and one to node1 +called min_work_node. The test: 1. Generate one block on each node, to leave IBD. 2. Mine a new block on each tip, and deliver to each node from node's peer. - The tip should advance. + The tip should advance for node0, but node1 should skip processing due to + nMinimumChainWork. + +Node1 is unused in tests 3-7: -3. Mine a block that forks the previous block, and deliver to each node from - corresponding peer. - Node0 should not process this block (just accept the header), because it is - unrequested and doesn't have more work than the tip. - Node1 should process because this is coming from a whitelisted peer. +3. Mine a block that forks from the genesis block, and deliver to test_node. + Node0 should not process this block (just accept the header), because it + is unrequested and doesn't have more or equal work to the tip. -4. Send another block that builds on the forking block. - Node0 should process this block but be stuck on the shorter chain, because - it's missing an intermediate block. - Node1 should reorg to this longer chain. +4a,b. Send another two blocks that build on the forking block. + Node0 should process the second block but be stuck on the shorter chain, + because it's missing an intermediate block. -4b.Send 288 more blocks on the longer chain. +4c.Send 288 more blocks on the longer chain (the number of blocks ahead + we currently store). Node0 should process all but the last block (too far ahead in height). - Send all headers to Node1, and then send the last block in that chain. - Node1 should accept the block because it's coming from a whitelisted peer. 5. Send a duplicate of the block in #3 to Node0. Node0 should not process the block because it is unrequested, and stay on @@ -46,13 +41,21 @@ The test: 7. Send Node0 the missing block again. Node0 should process and the tip should advance. + +8. Create a fork which is invalid at a height longer than the current chain + (ie to which the node will try to reorg) but which has headers built on top + of the invalid block. Check that we get disconnected if we send more headers + on the chain the node now knows to be invalid. + +9. Test Node1 is able to sync when connected to node0 (which should have sufficient + work on its chain). """ from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * import time -from test_framework.blocktools import create_block, create_coinbase +from test_framework.blocktools import create_block, create_coinbase, create_transaction class AcceptBlockTest(BitcoinTestFramework): def add_options(self, parser): @@ -63,37 +66,39 @@ class AcceptBlockTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.extra_args = [[], ["-whitelist=127.0.0.1"]] + self.extra_args = [[], ["-minimumchainwork=0x10"]] def setup_network(self): # Node0 will be used to test behavior of processing unrequested blocks # from peers which are not whitelisted, while Node1 will be used for # the whitelisted case. + # Node2 will be used for non-whitelisted peers to test the interaction + # with nMinimumChainWork. self.setup_nodes() def run_test(self): # Setup the p2p connections and start up the network thread. - test_node = NodeConnCB() # connects to node0 (not whitelisted) - white_node = NodeConnCB() # connects to node1 (whitelisted) + test_node = NodeConnCB() # connects to node0 + min_work_node = NodeConnCB() # connects to node1 connections = [] connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node)) - connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], white_node)) + connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], min_work_node)) test_node.add_connection(connections[0]) - white_node.add_connection(connections[1]) + min_work_node.add_connection(connections[1]) NetworkThread().start() # Start up network handling in another thread # Test logic begins here test_node.wait_for_verack() - white_node.wait_for_verack() + min_work_node.wait_for_verack() - # 1. Have both nodes mine a block (leave IBD) + # 1. Have nodes mine a block (leave IBD) [ n.generate(1) for n in self.nodes ] tips = [ int("0x" + n.getbestblockhash(), 0) for n in self.nodes ] # 2. Send one block that builds on each tip. - # This should be accepted. + # This should be accepted by node0 blocks_h2 = [] # the height 2 blocks on each node's chain block_time = int(time.time()) + 1 for i in range(2): @@ -101,95 +106,119 @@ class AcceptBlockTest(BitcoinTestFramework): blocks_h2[i].solve() block_time += 1 test_node.send_message(msg_block(blocks_h2[0])) - white_node.send_message(msg_block(blocks_h2[1])) + min_work_node.send_message(msg_block(blocks_h2[1])) - [ x.sync_with_ping() for x in [test_node, white_node] ] + for x in [test_node, min_work_node]: + x.sync_with_ping() assert_equal(self.nodes[0].getblockcount(), 2) - assert_equal(self.nodes[1].getblockcount(), 2) - self.log.info("First height 2 block accepted by both nodes") + assert_equal(self.nodes[1].getblockcount(), 1) + self.log.info("First height 2 block accepted by node0; correctly rejected by node1") - # 3. Send another block that builds on the original tip. - blocks_h2f = [] # Blocks at height 2 that fork off the main chain - for i in range(2): - blocks_h2f.append(create_block(tips[i], create_coinbase(2), blocks_h2[i].nTime+1)) - blocks_h2f[i].solve() - test_node.send_message(msg_block(blocks_h2f[0])) - white_node.send_message(msg_block(blocks_h2f[1])) + # 3. Send another block that builds on genesis. + block_h1f = create_block(int("0x" + self.nodes[0].getblockhash(0), 0), create_coinbase(1), block_time) + block_time += 1 + block_h1f.solve() + test_node.send_message(msg_block(block_h1f)) - [ x.sync_with_ping() for x in [test_node, white_node] ] + test_node.sync_with_ping() + tip_entry_found = False for x in self.nodes[0].getchaintips(): - if x['hash'] == blocks_h2f[0].hash: + if x['hash'] == block_h1f.hash: assert_equal(x['status'], "headers-only") + tip_entry_found = True + assert(tip_entry_found) + assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_h1f.hash) - for x in self.nodes[1].getchaintips(): - if x['hash'] == blocks_h2f[1].hash: - assert_equal(x['status'], "valid-headers") + # 4. Send another two block that build on the fork. + block_h2f = create_block(block_h1f.sha256, create_coinbase(2), block_time) + block_time += 1 + block_h2f.solve() + test_node.send_message(msg_block(block_h2f)) - self.log.info("Second height 2 block accepted only from whitelisted peer") + test_node.sync_with_ping() + # Since the earlier block was not processed by node, the new block + # can't be fully validated. + tip_entry_found = False + for x in self.nodes[0].getchaintips(): + if x['hash'] == block_h2f.hash: + assert_equal(x['status'], "headers-only") + tip_entry_found = True + assert(tip_entry_found) - # 4. Now send another block that builds on the forking chain. - blocks_h3 = [] - for i in range(2): - blocks_h3.append(create_block(blocks_h2f[i].sha256, create_coinbase(3), blocks_h2f[i].nTime+1)) - blocks_h3[i].solve() - test_node.send_message(msg_block(blocks_h3[0])) - white_node.send_message(msg_block(blocks_h3[1])) + # But this block should be accepted by node since it has equal work. + self.nodes[0].getblock(block_h2f.hash) + self.log.info("Second height 2 block accepted, but not reorg'ed to") - [ x.sync_with_ping() for x in [test_node, white_node] ] - # Since the earlier block was not processed by node0, the new block + # 4b. Now send another block that builds on the forking chain. + block_h3 = create_block(block_h2f.sha256, create_coinbase(3), block_h2f.nTime+1) + block_h3.solve() + test_node.send_message(msg_block(block_h3)) + + test_node.sync_with_ping() + # Since the earlier block was not processed by node, the new block # can't be fully validated. + tip_entry_found = False for x in self.nodes[0].getchaintips(): - if x['hash'] == blocks_h3[0].hash: + if x['hash'] == block_h3.hash: assert_equal(x['status'], "headers-only") + tip_entry_found = True + assert(tip_entry_found) + self.nodes[0].getblock(block_h3.hash) + + # But this block should be accepted by node since it has more work. + self.nodes[0].getblock(block_h3.hash) + self.log.info("Unrequested more-work block accepted") + + # 4c. Now mine 288 more blocks and deliver; all should be processed but + # the last (height-too-high) on node (as long as its not missing any headers) + tip = block_h3 + all_blocks = [] + for i in range(288): + next_block = create_block(tip.sha256, create_coinbase(i + 4), tip.nTime+1) + next_block.solve() + all_blocks.append(next_block) + tip = next_block + + # Now send the block at height 5 and check that it wasn't accepted (missing header) + test_node.send_message(msg_block(all_blocks[1])) + test_node.sync_with_ping() + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, all_blocks[1].hash) + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblockheader, all_blocks[1].hash) - # But this block should be accepted by node0 since it has more work. - self.nodes[0].getblock(blocks_h3[0].hash) - self.log.info("Unrequested more-work block accepted from non-whitelisted peer") + # The block at height 5 should be accepted if we provide the missing header, though + headers_message = msg_headers() + headers_message.headers.append(CBlockHeader(all_blocks[0])) + test_node.send_message(headers_message) + test_node.send_message(msg_block(all_blocks[1])) + test_node.sync_with_ping() + self.nodes[0].getblock(all_blocks[1].hash) - # Node1 should have accepted and reorged. - assert_equal(self.nodes[1].getblockcount(), 3) - self.log.info("Successfully reorged to length 3 chain from whitelisted peer") + # Now send the blocks in all_blocks + for i in range(288): + test_node.send_message(msg_block(all_blocks[i])) + test_node.sync_with_ping() - # 4b. Now mine 288 more blocks and deliver; all should be processed but - # the last (height-too-high) on node0. Node1 should process the tip if - # we give it the headers chain leading to the tip. - tips = blocks_h3 - headers_message = msg_headers() - all_blocks = [] # node0's blocks - for j in range(2): - for i in range(288): - next_block = create_block(tips[j].sha256, create_coinbase(i + 4), tips[j].nTime+1) - next_block.solve() - if j==0: - test_node.send_message(msg_block(next_block)) - all_blocks.append(next_block) - else: - headers_message.headers.append(CBlockHeader(next_block)) - tips[j] = next_block - - time.sleep(2) # Blocks 1-287 should be accepted, block 288 should be ignored because it's too far ahead for x in all_blocks[:-1]: self.nodes[0].getblock(x.hash) assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[-1].hash) - headers_message.headers.pop() # Ensure the last block is unrequested - white_node.send_message(headers_message) # Send headers leading to tip - white_node.send_message(msg_block(tips[1])) # Now deliver the tip - white_node.sync_with_ping() - self.nodes[1].getblock(tips[1].hash) - self.log.info("Unrequested block far ahead of tip accepted from whitelisted peer") - # 5. Test handling of unrequested block on the node that didn't process # Should still not be processed (even though it has a child that has more # work). - test_node.send_message(msg_block(blocks_h2f[0])) - # Here, if the sleep is too short, the test could falsely succeed (if the - # node hasn't processed the block by the time the sleep returns, and then - # the node processes it and incorrectly advances the tip). - # But this would be caught later on, when we verify that an inv triggers - # a getdata request for this block. + # The node should have requested the blocks at some point, so + # disconnect/reconnect first + connections[0].disconnect_node() + test_node.wait_for_disconnect() + + test_node = NodeConnCB() # connects to node (not whitelisted) + connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node) + test_node.add_connection(connections[0]) + + test_node.wait_for_verack() + test_node.send_message(msg_block(block_h1f)) + test_node.sync_with_ping() assert_equal(self.nodes[0].getblockcount(), 2) self.log.info("Unrequested block that would complete more-work chain was ignored") @@ -200,23 +229,100 @@ class AcceptBlockTest(BitcoinTestFramework): with mininode_lock: # Clear state so we can check the getdata request test_node.last_message.pop("getdata", None) - test_node.send_message(msg_inv([CInv(2, blocks_h3[0].sha256)])) + test_node.send_message(msg_inv([CInv(2, block_h3.sha256)])) test_node.sync_with_ping() with mininode_lock: getdata = test_node.last_message["getdata"] # Check that the getdata includes the right block - assert_equal(getdata.inv[0].hash, blocks_h2f[0].sha256) + assert_equal(getdata.inv[0].hash, block_h1f.sha256) self.log.info("Inv at tip triggered getdata for unprocessed block") # 7. Send the missing block for the third time (now it is requested) - test_node.send_message(msg_block(blocks_h2f[0])) + test_node.send_message(msg_block(block_h1f)) test_node.sync_with_ping() assert_equal(self.nodes[0].getblockcount(), 290) + self.nodes[0].getblock(all_blocks[286].hash) + assert_equal(self.nodes[0].getbestblockhash(), all_blocks[286].hash) + assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[287].hash) self.log.info("Successfully reorged to longer chain from non-whitelisted peer") + # 8. Create a chain which is invalid at a height longer than the + # current chain, but which has more blocks on top of that + block_289f = create_block(all_blocks[284].sha256, create_coinbase(289), all_blocks[284].nTime+1) + block_289f.solve() + block_290f = create_block(block_289f.sha256, create_coinbase(290), block_289f.nTime+1) + block_290f.solve() + block_291 = create_block(block_290f.sha256, create_coinbase(291), block_290f.nTime+1) + # block_291 spends a coinbase below maturity! + block_291.vtx.append(create_transaction(block_290f.vtx[0], 0, b"42", 1)) + block_291.hashMerkleRoot = block_291.calc_merkle_root() + block_291.solve() + block_292 = create_block(block_291.sha256, create_coinbase(292), block_291.nTime+1) + block_292.solve() + + # Now send all the headers on the chain and enough blocks to trigger reorg + headers_message = msg_headers() + headers_message.headers.append(CBlockHeader(block_289f)) + headers_message.headers.append(CBlockHeader(block_290f)) + headers_message.headers.append(CBlockHeader(block_291)) + headers_message.headers.append(CBlockHeader(block_292)) + test_node.send_message(headers_message) + + test_node.sync_with_ping() + tip_entry_found = False + for x in self.nodes[0].getchaintips(): + if x['hash'] == block_292.hash: + assert_equal(x['status'], "headers-only") + tip_entry_found = True + assert(tip_entry_found) + assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_292.hash) + + test_node.send_message(msg_block(block_289f)) + test_node.send_message(msg_block(block_290f)) + + test_node.sync_with_ping() + self.nodes[0].getblock(block_289f.hash) + self.nodes[0].getblock(block_290f.hash) + + test_node.send_message(msg_block(block_291)) + + # At this point we've sent an obviously-bogus block, wait for full processing + # without assuming whether we will be disconnected or not + try: + # Only wait a short while so the test doesn't take forever if we do get + # disconnected + test_node.sync_with_ping(timeout=1) + except AssertionError: + test_node.wait_for_disconnect() + + test_node = NodeConnCB() # connects to node (not whitelisted) + connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node) + test_node.add_connection(connections[0]) + + NetworkThread().start() # Start up network handling in another thread + test_node.wait_for_verack() + + # We should have failed reorg and switched back to 290 (but have block 291) + assert_equal(self.nodes[0].getblockcount(), 290) + assert_equal(self.nodes[0].getbestblockhash(), all_blocks[286].hash) + assert_equal(self.nodes[0].getblock(block_291.hash)["confirmations"], -1) + + # Now send a new header on the invalid chain, indicating we're forked off, and expect to get disconnected + block_293 = create_block(block_292.sha256, create_coinbase(293), block_292.nTime+1) + block_293.solve() + headers_message = msg_headers() + headers_message.headers.append(CBlockHeader(block_293)) + test_node.send_message(headers_message) + test_node.wait_for_disconnect() + + # 9. Connect node1 to node0 and ensure it is able to sync + connect_nodes(self.nodes[0], 1) + sync_blocks([self.nodes[0], self.nodes[1]]) + self.log.info("Successfully synced nodes 1 and 0") + [ c.disconnect_node() for c in connections ] if __name__ == '__main__': diff --git a/test/functional/p2p-compactblocks.py b/test/functional/p2p-compactblocks.py index 94513d3f43..e3f986d80b 100755 --- a/test/functional/p2p-compactblocks.py +++ b/test/functional/p2p-compactblocks.py @@ -93,7 +93,9 @@ class CompactBlocksTest(BitcoinTestFramework): self.setup_clean_chain = True # Node0 = pre-segwit, node1 = segwit-aware self.num_nodes = 2 - self.extra_args = [["-vbparams=segwit:0:0"], ["-txindex"]] + # This test was written assuming SegWit is activated using BIP9 at height 432 (3x confirmation window). + # TODO: Rewrite this test to support SegWit being always active. + self.extra_args = [["-vbparams=segwit:0:0"], ["-vbparams=segwit:0:999999999999", "-txindex"]] self.utxos = [] def build_block_on_tip(self, node, segwit=False): diff --git a/test/functional/p2p-fullblocktest.py b/test/functional/p2p-fullblocktest.py index 1d969fc7c1..f19b845a32 100755 --- a/test/functional/p2p-fullblocktest.py +++ b/test/functional/p2p-fullblocktest.py @@ -20,7 +20,7 @@ from test_framework.key import CECKey from test_framework.script import * import struct -class PreviousSpendableOutput(object): +class PreviousSpendableOutput(): def __init__(self, tx = CTransaction(), n = -1): self.tx = tx self.n = n # the output we're spending diff --git a/test/functional/p2p-segwit.py b/test/functional/p2p-segwit.py index a9ef47559b..49de1a6e87 100755 --- a/test/functional/p2p-segwit.py +++ b/test/functional/p2p-segwit.py @@ -89,7 +89,7 @@ class TestNode(NodeConnCB): assert_equal(self.connection.rpc.getbestblockhash() == block.hash, accepted) # Used to keep track of anyone-can-spend outputs that we can use in the tests -class UTXO(object): +class UTXO(): def __init__(self, sha256, n, nValue): self.sha256 = sha256 self.n = n @@ -111,7 +111,8 @@ class SegWitTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 - self.extra_args = [["-whitelist=127.0.0.1"], ["-whitelist=127.0.0.1", "-acceptnonstdtxn=0"], ["-whitelist=127.0.0.1", "-vbparams=segwit:0:0"]] + # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. + self.extra_args = [["-whitelist=127.0.0.1", "-vbparams=segwit:0:999999999999"], ["-whitelist=127.0.0.1", "-acceptnonstdtxn=0", "-vbparams=segwit:0:999999999999"], ["-whitelist=127.0.0.1", "-vbparams=segwit:0:0"]] def setup_network(self): self.setup_nodes() @@ -1493,7 +1494,7 @@ class SegWitTest(BitcoinTestFramework): # Restart with the new binary self.stop_node(node_id) - self.start_node(node_id, extra_args=[]) + self.start_node(node_id, extra_args=["-vbparams=segwit:0:999999999999"]) connect_nodes(self.nodes[0], node_id) sync_blocks(self.nodes) diff --git a/test/functional/replace-by-fee.py b/test/functional/replace-by-fee.py index 269d57775c..815e964848 100755 --- a/test/functional/replace-by-fee.py +++ b/test/functional/replace-by-fee.py @@ -72,8 +72,14 @@ class ReplaceByFeeTest(BitcoinTestFramework): ["-mempoolreplacement=0"]] def run_test(self): + # Leave IBD + self.nodes[0].generate(1) + make_utxo(self.nodes[0], 1*COIN) + # Ensure nodes are synced + self.sync_all() + self.log.info("Running test simple doublespend...") self.test_simple_doublespend() @@ -110,13 +116,18 @@ class ReplaceByFeeTest(BitcoinTestFramework): """Simple doublespend""" tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + # make_utxo may have generated a bunch of blocks, so we need to sync + # before we can spend the coins generated, or else the resulting + # transactions might not be accepted by our peers. + self.sync_all() + tx1a = CTransaction() tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] tx1a.vout = [CTxOut(1*COIN, CScript([b'a']))] tx1a_hex = txToHex(tx1a) tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, True) - self.sync_all([self.nodes]) + self.sync_all() # Should fail because we haven't changed the fee tx1b = CTransaction() diff --git a/test/functional/segwit.py b/test/functional/segwit.py index 6ecade7cb6..338fa1bc52 100755 --- a/test/functional/segwit.py +++ b/test/functional/segwit.py @@ -77,9 +77,10 @@ class SegWitTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 - self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0"], - ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1"], - ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness"]] + # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. + self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999"], + ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999"], + ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999"]] def setup_network(self): super().setup_network() diff --git a/test/functional/sendheaders.py b/test/functional/sendheaders.py index fe577dc20a..08de2ffa03 100755 --- a/test/functional/sendheaders.py +++ b/test/functional/sendheaders.py @@ -215,7 +215,7 @@ class SendHeadersTest(BitcoinTestFramework): connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], inv_node)) # Set nServices to 0 for test_node, so no block download will occur outside of # direct fetching - connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node, services=0)) + connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node, services=NODE_WITNESS)) inv_node.add_connection(connections[0]) test_node.add_connection(connections[1]) @@ -225,6 +225,10 @@ class SendHeadersTest(BitcoinTestFramework): inv_node.wait_for_verack() test_node.wait_for_verack() + # Ensure verack's have been processed by our peer + inv_node.sync_with_ping() + test_node.sync_with_ping() + tip = int(self.nodes[0].getbestblockhash(), 16) # PART 1 diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index 747bda309c..bd3a3b3fab 100644 --- a/test/functional/test_framework/authproxy.py +++ b/test/functional/test_framework/authproxy.py @@ -33,24 +33,17 @@ ServiceProxy class: - uses standard Python json lib """ -try: - import http.client as httplib -except ImportError: - import httplib import base64 import decimal +import http.client import json import logging import socket import time -try: - import urllib.parse as urlparse -except ImportError: - import urlparse - -USER_AGENT = "AuthServiceProxy/0.1" +import urllib.parse HTTP_TIMEOUT = 30 +USER_AGENT = "AuthServiceProxy/0.1" log = logging.getLogger("BitcoinRPC") @@ -60,7 +53,7 @@ class JSONRPCException(Exception): errmsg = '%(message)s (%(code)i)' % rpc_error except (KeyError, TypeError): errmsg = '' - Exception.__init__(self, errmsg) + super().__init__(errmsg) self.error = rpc_error @@ -69,28 +62,18 @@ def EncodeDecimal(o): return str(o) raise TypeError(repr(o) + " is not JSON serializable") -class AuthServiceProxy(object): +class AuthServiceProxy(): __id_count = 0 # ensure_ascii: escape unicode as \uXXXX, passed to json.dumps def __init__(self, service_url, service_name=None, timeout=HTTP_TIMEOUT, connection=None, ensure_ascii=True): self.__service_url = service_url self._service_name = service_name - self.ensure_ascii = ensure_ascii # can be toggled on the fly by tests - self.__url = urlparse.urlparse(service_url) - if self.__url.port is None: - port = 80 - else: - port = self.__url.port - (user, passwd) = (self.__url.username, self.__url.password) - try: - user = user.encode('utf8') - except AttributeError: - pass - try: - passwd = passwd.encode('utf8') - except AttributeError: - pass + self.ensure_ascii = ensure_ascii # can be toggled on the fly by tests + self.__url = urllib.parse.urlparse(service_url) + port = 80 if self.__url.port is None else self.__url.port + user = None if self.__url.username is None else self.__url.username.encode('utf8') + passwd = None if self.__url.password is None else self.__url.password.encode('utf8') authpair = user + b':' + passwd self.__auth_header = b'Basic ' + base64.b64encode(authpair) @@ -98,11 +81,9 @@ class AuthServiceProxy(object): # Callables re-use the connection of the original proxy self.__conn = connection elif self.__url.scheme == 'https': - self.__conn = httplib.HTTPSConnection(self.__url.hostname, port, - timeout=timeout) + self.__conn = http.client.HTTPSConnection(self.__url.hostname, port, timeout=timeout) else: - self.__conn = httplib.HTTPConnection(self.__url.hostname, port, - timeout=timeout) + self.__conn = http.client.HTTPConnection(self.__url.hostname, port, timeout=timeout) def __getattr__(self, name): if name.startswith('__') and name.endswith('__'): @@ -124,14 +105,14 @@ class AuthServiceProxy(object): try: self.__conn.request(method, path, postdata, headers) return self._get_response() - except httplib.BadStatusLine as e: - if e.line == "''": # if connection was closed, try again + except http.client.BadStatusLine as e: + if e.line == "''": # if connection was closed, try again self.__conn.close() self.__conn.request(method, path, postdata, headers) return self._get_response() else: raise - except (BrokenPipeError,ConnectionResetError): + except (BrokenPipeError, ConnectionResetError): # Python 3.5+ raises BrokenPipeError instead of BadStatusLine when the connection was reset # ConnectionResetError happens on FreeBSD with Python 3.4 self.__conn.close() @@ -141,8 +122,8 @@ class AuthServiceProxy(object): def get_request(self, *args, **argsn): AuthServiceProxy.__id_count += 1 - log.debug("-%s-> %s %s"%(AuthServiceProxy.__id_count, self._service_name, - json.dumps(args, default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) + log.debug("-%s-> %s %s" % (AuthServiceProxy.__id_count, self._service_name, + json.dumps(args, default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) if args and argsn: raise ValueError('Cannot handle both named and positional arguments') return {'version': '1.1', @@ -163,7 +144,7 @@ class AuthServiceProxy(object): def batch(self, rpc_call_list): postdata = json.dumps(list(rpc_call_list), default=EncodeDecimal, ensure_ascii=self.ensure_ascii) - log.debug("--> "+postdata) + log.debug("--> " + postdata) return self._request('POST', self.__url.path, postdata.encode('utf-8')) def _get_response(self): @@ -190,9 +171,9 @@ class AuthServiceProxy(object): response = json.loads(responsedata, parse_float=decimal.Decimal) elapsed = time.time() - req_start_time if "error" in response and response["error"] is None: - log.debug("<-%s- [%.6f] %s"%(response["id"], elapsed, json.dumps(response["result"], default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) + log.debug("<-%s- [%.6f] %s" % (response["id"], elapsed, json.dumps(response["result"], default=EncodeDecimal, ensure_ascii=self.ensure_ascii))) else: - log.debug("<-- [%.6f] %s"%(elapsed,responsedata)) + log.debug("<-- [%.6f] %s" % (elapsed, responsedata)) return response def __truediv__(self, relative_uri): diff --git a/test/functional/test_framework/blockstore.py b/test/functional/test_framework/blockstore.py index 4b2170a03f..051c57a6c7 100644 --- a/test/functional/test_framework/blockstore.py +++ b/test/functional/test_framework/blockstore.py @@ -10,7 +10,7 @@ import dbm.dumb as dbmd logger = logging.getLogger("TestFramework.blockstore") -class BlockStore(object): +class BlockStore(): """BlockStore helper class. BlockStore keeps a map of blocks and implements helper functions for @@ -100,7 +100,7 @@ class BlockStore(object): def get_blocks(self, inv): responses = [] for i in inv: - if (i.type == 2): # MSG_BLOCK + if (i.type == 2 or i.type == (2 | (1 << 30))): # MSG_BLOCK or MSG_WITNESS_BLOCK data = self.get(i.hash) if data is not None: # Use msg_generic to avoid re-serialization @@ -127,7 +127,7 @@ class BlockStore(object): locator.vHave = r return locator -class TxStore(object): +class TxStore(): def __init__(self, datadir): self.txDB = dbmd.open(datadir + "/transactions", 'c') @@ -153,7 +153,7 @@ class TxStore(object): def get_transactions(self, inv): responses = [] for i in inv: - if (i.type == 1): # MSG_TX + if (i.type == 1 or i.type == (1 | (1 << 30))): # MSG_TX or MSG_WITNESS_TX tx = self.get(i.hash) if tx is not None: responses.append(msg_generic(b"tx", tx)) diff --git a/test/functional/test_framework/comptool.py b/test/functional/test_framework/comptool.py index bfbc0c3b03..39a81a267f 100755 --- a/test/functional/test_framework/comptool.py +++ b/test/functional/test_framework/comptool.py @@ -27,7 +27,7 @@ logger=logging.getLogger("TestFramework.comptool") global mininode_lock -class RejectResult(object): +class RejectResult(): """Outcome that expects rejection of a transaction or block.""" def __init__(self, code, reason=b''): self.code = code @@ -80,9 +80,9 @@ class TestNode(NodeConnCB): [conn.send_message(r) for r in self.tx_store.get_transactions(message.inv)] for i in message.inv: - if i.type == 1: + if i.type == 1 or i.type == 1 | (1 << 30): # MSG_TX or MSG_WITNESS_TX self.tx_request_map[i.hash] = True - elif i.type == 2: + elif i.type == 2 or i.type == 2 | (1 << 30): # MSG_BLOCK or MSG_WITNESS_BLOCK self.block_request_map[i.hash] = True def on_inv(self, conn, message): @@ -156,13 +156,13 @@ class TestNode(NodeConnCB): # across all connections. (If outcome of final tx is specified as true # or false, then only the last tx is tested against outcome.) -class TestInstance(object): +class TestInstance(): def __init__(self, objects=None, sync_every_block=True, sync_every_tx=False): self.blocks_and_transactions = objects if objects else [] self.sync_every_block = sync_every_block self.sync_every_tx = sync_every_tx -class TestManager(object): +class TestManager(): def __init__(self, testgen, datadir): self.test_generator = testgen diff --git a/test/functional/test_framework/coverage.py b/test/functional/test_framework/coverage.py index 84049e76bc..ddc3c515b2 100644 --- a/test/functional/test_framework/coverage.py +++ b/test/functional/test_framework/coverage.py @@ -14,7 +14,7 @@ import os REFERENCE_FILENAME = 'rpc_interface.txt' -class AuthServiceProxyWrapper(object): +class AuthServiceProxyWrapper(): """ An object that wraps AuthServiceProxy to record specific RPC calls. diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index 85a6158a2f..aa91fb5b0d 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -84,7 +84,7 @@ def _check_result(val, func, args): ssl.EC_KEY_new_by_curve_name.restype = ctypes.c_void_p ssl.EC_KEY_new_by_curve_name.errcheck = _check_result -class CECKey(object): +class CECKey(): """Wrapper around OpenSSL's EC_KEY""" POINT_CONVERSION_COMPRESSED = 2 diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index c6f596156a..3e751f0f32 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -219,7 +219,7 @@ def ToHex(obj): # Objects that map to bitcoind objects, which can be serialized/deserialized -class CAddress(object): +class CAddress(): def __init__(self): self.nServices = 1 self.pchReserved = b"\x00" * 10 + b"\xff" * 2 @@ -246,7 +246,7 @@ class CAddress(object): MSG_WITNESS_FLAG = 1<<30 -class CInv(object): +class CInv(): typemap = { 0: "Error", 1: "TX", @@ -275,7 +275,7 @@ class CInv(object): % (self.typemap[self.type], self.hash) -class CBlockLocator(object): +class CBlockLocator(): def __init__(self): self.nVersion = MY_VERSION self.vHave = [] @@ -295,7 +295,7 @@ class CBlockLocator(object): % (self.nVersion, repr(self.vHave)) -class COutPoint(object): +class COutPoint(): def __init__(self, hash=0, n=0): self.hash = hash self.n = n @@ -314,7 +314,7 @@ class COutPoint(object): return "COutPoint(hash=%064x n=%i)" % (self.hash, self.n) -class CTxIn(object): +class CTxIn(): def __init__(self, outpoint=None, scriptSig=b"", nSequence=0): if outpoint is None: self.prevout = COutPoint() @@ -342,7 +342,7 @@ class CTxIn(object): self.nSequence) -class CTxOut(object): +class CTxOut(): def __init__(self, nValue=0, scriptPubKey=b""): self.nValue = nValue self.scriptPubKey = scriptPubKey @@ -363,7 +363,7 @@ class CTxOut(object): bytes_to_hex_str(self.scriptPubKey)) -class CScriptWitness(object): +class CScriptWitness(): def __init__(self): # stack is a vector of strings self.stack = [] @@ -378,7 +378,7 @@ class CScriptWitness(object): return True -class CTxInWitness(object): +class CTxInWitness(): def __init__(self): self.scriptWitness = CScriptWitness() @@ -395,7 +395,7 @@ class CTxInWitness(object): return self.scriptWitness.is_null() -class CTxWitness(object): +class CTxWitness(): def __init__(self): self.vtxinwit = [] @@ -423,7 +423,7 @@ class CTxWitness(object): return True -class CTransaction(object): +class CTransaction(): def __init__(self, tx=None): if tx is None: self.nVersion = 1 @@ -526,7 +526,7 @@ class CTransaction(object): % (self.nVersion, repr(self.vin), repr(self.vout), repr(self.wit), self.nLockTime) -class CBlockHeader(object): +class CBlockHeader(): def __init__(self, header=None): if header is None: self.set_null() @@ -666,7 +666,7 @@ class CBlock(CBlockHeader): time.ctime(self.nTime), self.nBits, self.nNonce, repr(self.vtx)) -class CUnsignedAlert(object): +class CUnsignedAlert(): def __init__(self): self.nVersion = 1 self.nRelayUntil = 0 @@ -721,7 +721,7 @@ class CUnsignedAlert(object): self.strComment, self.strStatusBar, self.strReserved) -class CAlert(object): +class CAlert(): def __init__(self): self.vchMsg = b"" self.vchSig = b"" @@ -741,7 +741,7 @@ class CAlert(object): % (len(self.vchMsg), len(self.vchSig)) -class PrefilledTransaction(object): +class PrefilledTransaction(): def __init__(self, index=0, tx = None): self.index = index self.tx = tx @@ -767,7 +767,7 @@ class PrefilledTransaction(object): return "PrefilledTransaction(index=%d, tx=%s)" % (self.index, repr(self.tx)) # This is what we send on the wire, in a cmpctblock message. -class P2PHeaderAndShortIDs(object): +class P2PHeaderAndShortIDs(): def __init__(self): self.header = CBlockHeader() self.nonce = 0 @@ -819,7 +819,7 @@ def calculate_shortid(k0, k1, tx_hash): # This version gets rid of the array lengths, and reinterprets the differential # encoding into indices that can be used for lookup. -class HeaderAndShortIDs(object): +class HeaderAndShortIDs(): def __init__(self, p2pheaders_and_shortids = None): self.header = CBlockHeader() self.nonce = 0 @@ -880,7 +880,7 @@ class HeaderAndShortIDs(object): return "HeaderAndShortIDs(header=%s, nonce=%d, shortids=%s, prefilledtxn=%s" % (repr(self.header), self.nonce, repr(self.shortids), repr(self.prefilled_txn)) -class BlockTransactionsRequest(object): +class BlockTransactionsRequest(): def __init__(self, blockhash=0, indexes = None): self.blockhash = blockhash @@ -920,7 +920,7 @@ class BlockTransactionsRequest(object): return "BlockTransactionsRequest(hash=%064x indexes=%s)" % (self.blockhash, repr(self.indexes)) -class BlockTransactions(object): +class BlockTransactions(): def __init__(self, blockhash=0, transactions = None): self.blockhash = blockhash @@ -944,12 +944,12 @@ class BlockTransactions(object): # Objects that correspond to messages on the wire -class msg_version(object): +class msg_version(): command = b"version" def __init__(self): self.nVersion = MY_VERSION - self.nServices = 1 + self.nServices = NODE_NETWORK | NODE_WITNESS self.nTime = int(time.time()) self.addrTo = CAddress() self.addrFrom = CAddress() @@ -1012,7 +1012,7 @@ class msg_version(object): self.strSubVer, self.nStartingHeight, self.nRelay) -class msg_verack(object): +class msg_verack(): command = b"verack" def __init__(self): @@ -1028,7 +1028,7 @@ class msg_verack(object): return "msg_verack()" -class msg_addr(object): +class msg_addr(): command = b"addr" def __init__(self): @@ -1044,7 +1044,7 @@ class msg_addr(object): return "msg_addr(addrs=%s)" % (repr(self.addrs)) -class msg_alert(object): +class msg_alert(): command = b"alert" def __init__(self): @@ -1063,7 +1063,7 @@ class msg_alert(object): return "msg_alert(alert=%s)" % (repr(self.alert), ) -class msg_inv(object): +class msg_inv(): command = b"inv" def __init__(self, inv=None): @@ -1082,7 +1082,7 @@ class msg_inv(object): return "msg_inv(inv=%s)" % (repr(self.inv)) -class msg_getdata(object): +class msg_getdata(): command = b"getdata" def __init__(self, inv=None): @@ -1098,7 +1098,7 @@ class msg_getdata(object): return "msg_getdata(inv=%s)" % (repr(self.inv)) -class msg_getblocks(object): +class msg_getblocks(): command = b"getblocks" def __init__(self): @@ -1121,7 +1121,7 @@ class msg_getblocks(object): % (repr(self.locator), self.hashstop) -class msg_tx(object): +class msg_tx(): command = b"tx" def __init__(self, tx=CTransaction()): @@ -1142,7 +1142,7 @@ class msg_witness_tx(msg_tx): return self.tx.serialize_with_witness() -class msg_block(object): +class msg_block(): command = b"block" def __init__(self, block=None): @@ -1162,7 +1162,7 @@ class msg_block(object): # for cases where a user needs tighter control over what is sent over the wire # note that the user must supply the name of the command, and the data -class msg_generic(object): +class msg_generic(): def __init__(self, command, data=None): self.command = command self.data = data @@ -1179,7 +1179,7 @@ class msg_witness_block(msg_block): r = self.block.serialize(with_witness=True) return r -class msg_getaddr(object): +class msg_getaddr(): command = b"getaddr" def __init__(self): @@ -1195,7 +1195,7 @@ class msg_getaddr(object): return "msg_getaddr()" -class msg_ping_prebip31(object): +class msg_ping_prebip31(): command = b"ping" def __init__(self): @@ -1211,7 +1211,7 @@ class msg_ping_prebip31(object): return "msg_ping() (pre-bip31)" -class msg_ping(object): +class msg_ping(): command = b"ping" def __init__(self, nonce=0): @@ -1229,7 +1229,7 @@ class msg_ping(object): return "msg_ping(nonce=%08x)" % self.nonce -class msg_pong(object): +class msg_pong(): command = b"pong" def __init__(self, nonce=0): @@ -1247,7 +1247,7 @@ class msg_pong(object): return "msg_pong(nonce=%08x)" % self.nonce -class msg_mempool(object): +class msg_mempool(): command = b"mempool" def __init__(self): @@ -1262,7 +1262,7 @@ class msg_mempool(object): def __repr__(self): return "msg_mempool()" -class msg_sendheaders(object): +class msg_sendheaders(): command = b"sendheaders" def __init__(self): @@ -1282,7 +1282,7 @@ class msg_sendheaders(object): # number of entries # vector of hashes # hash_stop (hash of last desired block header, 0 to get as many as possible) -class msg_getheaders(object): +class msg_getheaders(): command = b"getheaders" def __init__(self): @@ -1307,7 +1307,7 @@ class msg_getheaders(object): # headers message has # <count> <vector of block headers> -class msg_headers(object): +class msg_headers(): command = b"headers" def __init__(self, headers=None): @@ -1327,7 +1327,7 @@ class msg_headers(object): return "msg_headers(headers=%s)" % repr(self.headers) -class msg_reject(object): +class msg_reject(): command = b"reject" REJECT_MALFORMED = 1 @@ -1358,7 +1358,7 @@ class msg_reject(object): return "msg_reject: %s %d %s [%064x]" \ % (self.message, self.code, self.reason, self.data) -class msg_feefilter(object): +class msg_feefilter(): command = b"feefilter" def __init__(self, feerate=0): @@ -1375,7 +1375,7 @@ class msg_feefilter(object): def __repr__(self): return "msg_feefilter(feerate=%08x)" % self.feerate -class msg_sendcmpct(object): +class msg_sendcmpct(): command = b"sendcmpct" def __init__(self): @@ -1395,7 +1395,7 @@ class msg_sendcmpct(object): def __repr__(self): return "msg_sendcmpct(announce=%s, version=%lu)" % (self.announce, self.version) -class msg_cmpctblock(object): +class msg_cmpctblock(): command = b"cmpctblock" def __init__(self, header_and_shortids = None): @@ -1413,7 +1413,7 @@ class msg_cmpctblock(object): def __repr__(self): return "msg_cmpctblock(HeaderAndShortIDs=%s)" % repr(self.header_and_shortids) -class msg_getblocktxn(object): +class msg_getblocktxn(): command = b"getblocktxn" def __init__(self): @@ -1431,7 +1431,7 @@ class msg_getblocktxn(object): def __repr__(self): return "msg_getblocktxn(block_txn_request=%s)" % (repr(self.block_txn_request)) -class msg_blocktxn(object): +class msg_blocktxn(): command = b"blocktxn" def __init__(self): @@ -1454,7 +1454,7 @@ class msg_witness_blocktxn(msg_blocktxn): r += self.block_transactions.serialize(with_witness=True) return r -class NodeConnCB(object): +class NodeConnCB(): """Callback and helper functions for P2P connection to a bitcoind node. Individual testcases should subclass this and override the on_* methods @@ -1615,7 +1615,6 @@ class NodeConnCB(object): test_function = lambda: self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter wait_until(test_function, timeout=timeout, lock=mininode_lock) self.ping_counter += 1 - return True # The actual NodeConn class # This class provides an interface for a p2p connection to a specified node @@ -1650,7 +1649,7 @@ class NodeConn(asyncore.dispatcher): "regtest": b"\xfa\xbf\xb5\xda", # regtest } - def __init__(self, dstaddr, dstport, rpc, callback, net="regtest", services=NODE_NETWORK, send_version=True): + def __init__(self, dstaddr, dstport, rpc, callback, net="regtest", services=NODE_NETWORK|NODE_WITNESS, send_version=True): asyncore.dispatcher.__init__(self, map=mininode_socket_map) self.dstaddr = dstaddr self.dstport = dstport diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 8f5339a02a..a4c046bd3d 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -370,7 +370,7 @@ class CScriptTruncatedPushDataError(CScriptInvalidError): super(CScriptTruncatedPushDataError, self).__init__(msg) # This is used, eg, for blockchain heights in coinbase scripts (bip34) -class CScriptNum(object): +class CScriptNum(): def __init__(self, d=0): self.value = d diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py index 0070844168..7b40c47fbf 100644 --- a/test/functional/test_framework/socks5.py +++ b/test/functional/test_framework/socks5.py @@ -31,7 +31,7 @@ def recvall(s, n): return rv ### Implementation classes -class Socks5Configuration(object): +class Socks5Configuration(): """Proxy configuration.""" def __init__(self): self.addr = None # Bind address (must be set) @@ -39,7 +39,7 @@ class Socks5Configuration(object): self.unauth = False # Support unauthenticated self.auth = False # Support authentication -class Socks5Command(object): +class Socks5Command(): """Information about an incoming socks5 command.""" def __init__(self, cmd, atyp, addr, port, username, password): self.cmd = cmd # Command (one of Command.*) @@ -51,7 +51,7 @@ class Socks5Command(object): def __repr__(self): return 'Socks5Command(%s,%s,%s,%s,%s,%s)' % (self.cmd, self.atyp, self.addr, self.port, self.username, self.password) -class Socks5Connection(object): +class Socks5Connection(): def __init__(self, serv, conn, peer): self.serv = serv self.conn = conn @@ -122,7 +122,7 @@ class Socks5Connection(object): finally: self.conn.close() -class Socks5Server(object): +class Socks5Server(): def __init__(self, conf): self.conf = conf self.s = socket.socket(conf.af) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 381513ab9e..8df50474f3 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -43,7 +43,7 @@ TEST_EXIT_PASSED = 0 TEST_EXIT_FAILED = 1 TEST_EXIT_SKIPPED = 77 -class BitcoinTestFramework(object): +class BitcoinTestFramework(): """Base class for a bitcoin test script. Individual bitcoin test scripts should subclass this class and override the set_test_params() and run_test() methods. @@ -102,8 +102,11 @@ class BitcoinTestFramework(object): check_json_precision() + self.options.cachedir = os.path.abspath(self.options.cachedir) + # Set up temp directory and start logging if self.options.tmpdir: + self.options.tmpdir = os.path.abspath(self.options.tmpdir) os.makedirs(self.options.tmpdir, exist_ok=False) else: self.options.tmpdir = tempfile.mkdtemp(prefix="test") diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 8c4651f6e0..ca36426a0a 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -124,6 +124,8 @@ BASE_SCRIPTS= [ 'resendwallettransactions.py', 'minchainwork.py', 'p2p-fingerprint.py', + 'uacomment.py', + 'p2p-acceptblock.py', ] EXTENDED_SCRIPTS = [ @@ -151,7 +153,6 @@ EXTENDED_SCRIPTS = [ 'txn_clone.py --mineblock', 'notifications.py', 'invalidateblock.py', - 'p2p-acceptblock.py', 'replace-by-fee.py', ] @@ -459,7 +460,7 @@ def check_script_list(src_dir): # On travis this warning is an error to prevent merging incomplete commits into master sys.exit(1) -class RPCCoverage(object): +class RPCCoverage(): """ Coverage reporting utilities for test_runner. diff --git a/test/functional/uacomment.py b/test/functional/uacomment.py new file mode 100755 index 0000000000..0b2c64ab69 --- /dev/null +++ b/test/functional/uacomment.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the -uacomment option.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +class UacommentTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + self.log.info("test multiple -uacomment") + test_uacomment = self.nodes[0].getnetworkinfo()["subversion"][-12:-1] + assert_equal(test_uacomment, "(testnode0)") + + self.restart_node(0, ["-uacomment=foo"]) + foo_uacomment = self.nodes[0].getnetworkinfo()["subversion"][-17:-1] + assert_equal(foo_uacomment, "(testnode0; foo)") + + self.log.info("test -uacomment max length") + self.stop_node(0) + expected = "Total length of network version string (286) exceeds maximum length (256). Reduce the number or size of uacomments." + self.assert_start_raises_init_error(0, ["-uacomment=" + 'a' * 256], expected) + + self.log.info("test -uacomment unsafe characters") + for unsafe_char in ['/', ':', '(', ')']: + expected = "User Agent comment (" + unsafe_char + ") contains unsafe characters" + self.assert_start_raises_init_error(0, ["-uacomment=" + unsafe_char], expected) + +if __name__ == '__main__': + UacommentTest().main() diff --git a/test/functional/walletbackup.py b/test/functional/walletbackup.py index 15ea26afa1..85a149793e 100755 --- a/test/functional/walletbackup.py +++ b/test/functional/walletbackup.py @@ -190,6 +190,16 @@ class WalletBackupTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), balance1) assert_equal(self.nodes[2].getbalance(), balance2) + # Backup to source wallet file must fail + sourcePaths = [ + tmpdir + "/node0/regtest/wallet.dat", + tmpdir + "/node0/./regtest/wallet.dat", + tmpdir + "/node0/regtest/", + tmpdir + "/node0/regtest"] + + for sourcePath in sourcePaths: + assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath) + if __name__ == '__main__': WalletBackupTest().main() diff --git a/test/functional/zmq_test.py b/test/functional/zmq_test.py index 382ef5bae2..fa30318416 100755 --- a/test/functional/zmq_test.py +++ b/test/functional/zmq_test.py @@ -2,16 +2,37 @@ # Copyright (c) 2015-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. -"""Test the ZMQ API.""" +"""Test the ZMQ notification interface.""" import configparser import os import struct from test_framework.test_framework import BitcoinTestFramework, SkipTest +from test_framework.mininode import CTransaction from test_framework.util import (assert_equal, bytes_to_hex_str, hash256, ) +from io import BytesIO + +class ZMQSubscriber: + def __init__(self, socket, topic): + self.sequence = 0 + self.socket = socket + self.topic = topic + + import zmq + self.socket.setsockopt(zmq.SUBSCRIBE, self.topic) + + def receive(self): + topic, body, seq = self.socket.recv_multipart() + # Topic should match the subscriber topic. + assert_equal(topic, self.topic) + # Sequence should be incremental. + assert_equal(struct.unpack('<I', seq)[-1], self.sequence) + self.sequence += 1 + return body + class ZMQTest (BitcoinTestFramework): def set_test_params(self): @@ -24,26 +45,33 @@ class ZMQTest (BitcoinTestFramework): except ImportError: raise SkipTest("python3-zmq module not available.") - # Check that bitcoin has been built with ZMQ enabled + # Check that bitcoin has been built with ZMQ enabled. config = configparser.ConfigParser() if not self.options.configfile: - self.options.configfile = os.path.dirname(__file__) + "/../config.ini" + self.options.configfile = os.path.abspath(os.path.join(os.path.dirname(__file__), "../config.ini")) config.read_file(open(self.options.configfile)) if not config["components"].getboolean("ENABLE_ZMQ"): raise SkipTest("bitcoind has not been built with zmq enabled.") - self.zmqContext = zmq.Context() - self.zmqSubSocket = self.zmqContext.socket(zmq.SUB) - self.zmqSubSocket.set(zmq.RCVTIMEO, 60000) - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashblock") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashtx") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"rawblock") - self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"rawtx") - ip_address = "tcp://127.0.0.1:28332" - self.zmqSubSocket.connect(ip_address) - self.extra_args = [['-zmqpubhashblock=%s' % ip_address, '-zmqpubhashtx=%s' % ip_address, - '-zmqpubrawblock=%s' % ip_address, '-zmqpubrawtx=%s' % ip_address], []] + # Initialize ZMQ context and socket. + # All messages are received in the same socket which means + # that this test fails if the publishing order changes. + # Note that the publishing order is not defined in the documentation and + # is subject to change. + address = "tcp://127.0.0.1:28332" + self.zmq_context = zmq.Context() + socket = self.zmq_context.socket(zmq.SUB) + socket.set(zmq.RCVTIMEO, 60000) + socket.connect(address) + + # Subscribe to all available topics. + self.hashblock = ZMQSubscriber(socket, b"hashblock") + self.hashtx = ZMQSubscriber(socket, b"hashtx") + self.rawblock = ZMQSubscriber(socket, b"rawblock") + self.rawtx = ZMQSubscriber(socket, b"rawtx") + + self.extra_args = [["-zmqpub%s=%s" % (sub.topic.decode(), address) for sub in [self.hashblock, self.hashtx, self.rawblock, self.rawtx]], []] self.add_nodes(self.num_nodes, self.extra_args) self.start_nodes() @@ -51,103 +79,48 @@ class ZMQTest (BitcoinTestFramework): try: self._zmq_test() finally: - # Destroy the zmq context - self.log.debug("Destroying zmq context") - self.zmqContext.destroy(linger=None) + # Destroy the ZMQ context. + self.log.debug("Destroying ZMQ context") + self.zmq_context.destroy(linger=None) def _zmq_test(self): - genhashes = self.nodes[0].generate(1) + num_blocks = 5 + self.log.info("Generate %(n)d blocks (and %(n)d coinbase txes)" % {"n": num_blocks}) + genhashes = self.nodes[0].generate(num_blocks) self.sync_all() - self.log.info("Wait for tx") - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashtx") - txhash = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on hashtx - - # rawtx - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawtx") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on rawtx - - # Check that the rawtx hashes to the hashtx - assert_equal(hash256(body), txhash) - - self.log.info("Wait for block") - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashblock") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) # must be sequence 0 on hashblock - blkhash = bytes_to_hex_str(body) - assert_equal(genhashes[0], blkhash) # blockhash from generate must be equal to the hash received over zmq - - # rawblock - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawblock") - body = msg[1] - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) #must be sequence 0 on rawblock - - # Check the hash of the rawblock's header matches generate - assert_equal(genhashes[0], bytes_to_hex_str(hash256(body[:80]))) - - self.log.info("Generate 10 blocks (and 10 coinbase txes)") - n = 10 - genhashes = self.nodes[1].generate(n) - self.sync_all() + for x in range(num_blocks): + # Should receive the coinbase txid. + txid = self.hashtx.receive() + + # Should receive the coinbase raw transaction. + hex = self.rawtx.receive() + tx = CTransaction() + tx.deserialize(BytesIO(hex)) + tx.calc_sha256() + assert_equal(tx.hash, bytes_to_hex_str(txid)) - zmqHashes = [] - zmqRawHashed = [] - blockcount = 0 - for x in range(n * 4): - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - body = msg[1] - if topic == b"hashblock": - zmqHashes.append(bytes_to_hex_str(body)) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount + 1) - blockcount += 1 - if topic == b"rawblock": - zmqRawHashed.append(bytes_to_hex_str(hash256(body[:80]))) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount) - - for x in range(n): - assert_equal(genhashes[x], zmqHashes[x]) # blockhash from generate must be equal to the hash received over zmq - assert_equal(genhashes[x], zmqRawHashed[x]) + # Should receive the generated block hash. + hash = bytes_to_hex_str(self.hashblock.receive()) + assert_equal(genhashes[x], hash) + # The block should only have the coinbase txid. + assert_equal([bytes_to_hex_str(txid)], self.nodes[1].getblock(hash)["tx"]) + + # Should receive the generated raw block. + block = self.rawblock.receive() + assert_equal(genhashes[x], bytes_to_hex_str(hash256(block[:80]))) self.log.info("Wait for tx from second node") - # test tx from a second node - hashRPC = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) + payment_txid = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) self.sync_all() - # now we should receive a zmq msg because the tx was broadcast - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"hashtx") - body = msg[1] - hashZMQ = bytes_to_hex_str(body) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount + 1) - - msg = self.zmqSubSocket.recv_multipart() - topic = msg[0] - assert_equal(topic, b"rawtx") - body = msg[1] - hashedZMQ = bytes_to_hex_str(hash256(body)) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount+1) - assert_equal(hashRPC, hashZMQ) # txid from sendtoaddress must be equal to the hash received over zmq - assert_equal(hashRPC, hashedZMQ) + # Should receive the broadcasted txid. + txid = self.hashtx.receive() + assert_equal(payment_txid, bytes_to_hex_str(txid)) + + # Should receive the broadcasted raw transaction. + hex = self.rawtx.receive() + assert_equal(payment_txid, bytes_to_hex_str(hash256(hex))) if __name__ == '__main__': ZMQTest().main() |