diff options
39 files changed, 1066 insertions, 936 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index fe75403261..4a7c4eaf55 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -21,11 +21,13 @@ persistent_worker_template: &PERSISTENT_WORKER_TEMPLATE base_template: &BASE_TEMPLATE skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == "" # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution merge_base_script: + # Unconditionally install git (used in fingerprint_script) and set the + # default git author name (used in verify-commits.py) - bash -c "$PACKAGE_MANAGER_INSTALL git" - - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi - - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH - git config --global user.email "ci@ci.ci" - git config --global user.name "ci" + - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi + - git fetch $CIRRUS_REPO_CLONE_URL $CIRRUS_BASE_BRANCH - git merge FETCH_HEAD # Merge base to detect silent merge conflicts stateful: false # https://cirrus-ci.org/guide/writing-tasks/#stateful-tasks diff --git a/build_msvc/bitcoin_config.h b/build_msvc/bitcoin_config.h index e987aa64cb..e2930f3ea9 100644 --- a/build_msvc/bitcoin_config.h +++ b/build_msvc/bitcoin_config.h @@ -5,9 +5,6 @@ #ifndef BITCOIN_BITCOIN_CONFIG_H #define BITCOIN_BITCOIN_CONFIG_H -/* Define if building universal (internal helper macro) */ -/* #undef AC_APPLE_UNIVERSAL_BUILD */ - /* Version Build */ #define CLIENT_VERSION_BUILD 0 @@ -59,14 +56,11 @@ /* define if the Boost::Unit_Test_Framework library is available */ #define HAVE_BOOST_UNIT_TEST_FRAMEWORK /**/ -/* Define to 1 if you have the <byteswap.h> header file. */ -/* #undef HAVE_BYTESWAP_H */ - /* Define this symbol if the consensus lib has been built */ #define HAVE_CONSENSUS_LIB 1 -/* define if the compiler supports basic C++11 syntax */ -#define HAVE_CXX11 1 +/* define if the compiler supports basic C++17 syntax */ +#define HAVE_CXX17 1 /* Define to 1 if you have the declaration of `be16toh', and to 0 if you don't. */ @@ -144,37 +138,12 @@ don't. */ #define HAVE_DECL_STRNLEN 1 -/* Define to 1 if you have the <dlfcn.h> header file. */ -/* #undef HAVE_DLFCN_H */ - -/* Define to 1 if you have the <endian.h> header file. */ -/* #undef HAVE_ENDIAN_H */ - -/* Define to 1 if the system has the `dllexport' function attribute */ -#define HAVE_FUNC_ATTRIBUTE_DLLEXPORT 1 - -/* Define to 1 if the system has the `dllimport' function attribute */ -#define HAVE_FUNC_ATTRIBUTE_DLLIMPORT 1 - -/* Define to 1 if the system has the `visibility' function attribute */ -#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1 - -/* Define this symbol if the BSD getentropy system call is available */ -/* #undef HAVE_GETENTROPY */ - -/* Define this symbol if the BSD getentropy system call is available with - sys/random.h */ -/* #undef HAVE_GETENTROPY_RAND */ +/* Define if the dllexport attribute is supported. */ +#define HAVE_DLLEXPORT_ATTRIBUTE 1 /* Define to 1 if you have the <inttypes.h> header file. */ #define HAVE_INTTYPES_H 1 -/* Define this symbol if you have malloc_info */ -/* #undef HAVE_MALLOC_INFO */ - -/* Define this symbol if you have mallopt with M_ARENA_MAX */ -/* #undef HAVE_MALLOPT_ARENA_MAX */ - /* Define to 1 if you have the <memory.h> header file. */ #define HAVE_MEMORY_H 1 @@ -187,18 +156,6 @@ /* Define to 1 if you have the <miniupnpc/upnperrors.h> header file. */ #define HAVE_MINIUPNPC_UPNPERRORS_H 1 -/* Define this symbol if you have MSG_DONTWAIT */ -/* #undef HAVE_MSG_DONTWAIT */ - -/* Define this symbol if you have MSG_NOSIGNAL */ -/* #undef HAVE_MSG_NOSIGNAL */ - -/* Define if you have POSIX threads libraries and header files. */ -//#define HAVE_PTHREAD 1 - -/* Have PTHREAD_PRIO_INHERIT. */ -//#define HAVE_PTHREAD_PRIO_INHERIT 1 - /* Define to 1 if you have the <stdint.h> header file. */ #define HAVE_STDINT_H 1 @@ -208,45 +165,18 @@ /* Define to 1 if you have the <stdlib.h> header file. */ #define HAVE_STDLIB_H 1 -/* Define to 1 if you have the `strerror_r' function. */ -/* #undef HAVE_STRERROR_R */ - /* Define to 1 if you have the <strings.h> header file. */ #define HAVE_STRINGS_H 1 /* Define to 1 if you have the <string.h> header file. */ #define HAVE_STRING_H 1 -/* Define this symbol if the BSD sysctl(KERN_ARND) is available */ -/* #undef HAVE_SYSCTL_ARND */ - -/* Define to 1 if you have the <sys/endian.h> header file. */ -/* #undef HAVE_SYS_ENDIAN_H */ - -/* Define this symbol if the Linux getrandom system call is available */ -/* #undef HAVE_SYS_GETRANDOM */ - -/* Define to 1 if you have the <sys/prctl.h> header file. */ -/* #undef HAVE_SYS_PRCTL_H */ - -/* Define to 1 if you have the <sys/select.h> header file. */ -/* #undef HAVE_SYS_SELECT_H */ - /* Define to 1 if you have the <sys/stat.h> header file. */ #define HAVE_SYS_STAT_H 1 /* Define to 1 if you have the <sys/types.h> header file. */ #define HAVE_SYS_TYPES_H 1 -/* Define to 1 if you have the <unistd.h> header file. */ -//#define HAVE_UNISTD_H 1 - -/* Define if the visibility attribute is supported. */ -#define HAVE_VISIBILITY_ATTRIBUTE 1 - -/* Define to the sub-directory where libtool stores uninstalled libraries. */ -#define LT_OBJDIR ".libs/" - /* Define to the address where bug reports for this package should be sent. */ #define PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues" @@ -256,76 +186,21 @@ /* Define to the full name and version of this package. */ #define PACKAGE_STRING "Bitcoin Core 22.99.0" -/* Define to the one symbol short name of this package. */ -#define PACKAGE_TARNAME "bitcoin" - /* Define to the home page for this package. */ #define PACKAGE_URL "https://bitcoincore.org/" /* Define to the version of this package. */ #define PACKAGE_VERSION "22.99.0" -/* Define to necessary symbol if this constant uses a non-standard name on - your system. */ -/* #undef PTHREAD_CREATE_JOINABLE */ - -/* Define this symbol if the qt platform is cocoa */ -/* #undef QT_QPA_PLATFORM_COCOA */ - /* Define this symbol if the minimal qt platform exists */ #define QT_QPA_PLATFORM_MINIMAL 1 /* Define this symbol if the qt platform is windows */ #define QT_QPA_PLATFORM_WINDOWS 1 -/* Define this symbol if the qt platform is xcb */ -/* #undef QT_QPA_PLATFORM_XCB */ - /* Define this symbol if qt plugins are static */ #define QT_STATICPLUGIN 1 -/* Define to 1 if you have the ANSI C header files. */ -#define STDC_HEADERS 1 - -/* Define to 1 if strerror_r returns char *. */ -/* #undef STRERROR_R_CHAR_P */ - -/* Define this symbol to build in assembly routines */ -//#define USE_ASM 1 - -/* Define if dbus support should be compiled in */ -/* #undef USE_DBUS */ - -/* Define if QR support should be compiled in */ -//#define USE_QRCODE 1 - -/* UPnP support not compiled if undefined, otherwise value (0 or 1) determines - default state */ -//#define USE_UPNP 0 - -/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most - significant byte first (like Motorola and SPARC, unlike Intel). */ -#if defined AC_APPLE_UNIVERSAL_BUILD -# if defined __BIG_ENDIAN__ -# define WORDS_BIGENDIAN 1 -# endif -#else -# ifndef WORDS_BIGENDIAN -/* # undef WORDS_BIGENDIAN */ -# endif -#endif - -/* Enable large inode numbers on Mac OS X 10.5. */ -#ifndef _DARWIN_USE_64_BIT_INODE -# define _DARWIN_USE_64_BIT_INODE 1 -#endif - -/* Number of bits in a file offset, on hosts where this is settable. */ -#define _FILE_OFFSET_BITS 64 - -/* Define for large files, on AIX-style hosts. */ -/* #undef _LARGE_FILES */ - /* Windows Universal Platform constraints */ #if !defined(WINAPI_FAMILY) || (WINAPI_FAMILY == WINAPI_FAMILY_DESKTOP_APP) /* Either a desktop application without API restrictions, or and older system diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index c3c7619ef7..f7dacd8512 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -23,10 +23,15 @@ test/lint/git-subtree-check.sh src/crc32c test/lint/check-doc.py test/lint/lint-all.sh -if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ -n "$CIRRUS_CRON" ]; then - git log --merges --before="2 days ago" -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit +if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ "$CIRRUS_PR" = "" ] ; then + # Sanity check only the last few commits to get notified of missing sigs, + # missing keys, or expired keys. Usually there is only one new merge commit + # per push on the master branch and a few commits on release branches, so + # sanity checking only a few (10) commits seems sufficient and cheap. + git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit + git log HEAD~10 -1 --format='%H' > ./contrib/verify-commits/trusted-git-root ${CI_RETRY_EXE} gpg --keyserver hkps://keys.openpgp.org --recv-keys $(<contrib/verify-commits/trusted-keys) && - ./contrib/verify-commits/verify-commits.py --clean-merge=2; + ./contrib/verify-commits/verify-commits.py; fi echo diff --git a/contrib/builder-keys/keys.txt b/contrib/builder-keys/keys.txt index 890406c745..e8032f66ee 100644 --- a/contrib/builder-keys/keys.txt +++ b/contrib/builder-keys/keys.txt @@ -28,6 +28,7 @@ D3CC177286005BB8FF673294C5242A1AB3936517 jl2012 (jl2012) 32EE5C4C3FA15CCADB46ABE529D4BCB6416F53EC Jonas Schnelli (jonasschnelli) 4B4E840451149DD7FB0D633477DFAB5C3108B9A8 Jorge Timon (jtimon) C42AFF7C61B3E44A1454CD3557AF762DB3353322 Karl-Johan Alm (kallewoof) +70A1D47DD44F59DF8B22244333E472FE870C7E5D Kristaps Kaupe (kristapsk) 30DE693AE0DE9E37B3E7EB6BBFF0F67810C1EED1 Lisa Neigut (niftynei) E463A93F5F3117EEDE6C7316BD02942421F4889F Luke Dashjr (luke-jr) B8B3F1C0E58C15DB6A81D30C3648A882F4316B9B Marco Falke (marco) diff --git a/contrib/guix/libexec/prelude.bash b/contrib/guix/libexec/prelude.bash index 9705607119..40ae4b5208 100644 --- a/contrib/guix/libexec/prelude.bash +++ b/contrib/guix/libexec/prelude.bash @@ -49,7 +49,7 @@ fi # Set common variables ################ -VERSION="${VERSION:-$(git_head_version)}" +VERSION="${FORCE_VERSION:-$(git_head_version)}" DISTNAME="${DISTNAME:-bitcoin-${VERSION}}" version_base_prefix="${PWD}/guix-build-" diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index d45ac3d03f..8a3116bb3b 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -12,7 +12,7 @@ $(package)_config_opts_mingw32=--enable-mingw $(package)_config_opts_linux=--with-pic $(package)_config_opts_android=--with-pic $(package)_cflags+=-Wno-error=implicit-function-declaration -$(package)_cxxflags=-std=c++17 +$(package)_cxxflags+=-std=c++17 $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index f879d176f5..21df50b040 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -23,7 +23,7 @@ else $(package)_toolset_$(host_os)=gcc endif $(package)_config_libraries=filesystem,system,test -$(package)_cxxflags=-std=c++17 -fvisibility=hidden +$(package)_cxxflags+=-std=c++17 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC $(package)_cxxflags_android=-fPIC $(package)_cxxflags_x86_64_darwin=-fcf-protection=full diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index 3b7f3690a4..9798248c61 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -12,7 +12,7 @@ define $(package)_set_vars $(package)_config_opts += --disable-Werror --disable-drafts --enable-option-checking $(package)_config_opts_linux=--with-pic $(package)_config_opts_android=--with-pic - $(package)_cxxflags=-std=c++17 + $(package)_cxxflags+=-std=c++17 endef define $(package)_preprocess_cmds diff --git a/doc/build-unix.md b/doc/build-unix.md index 4a56114109..44b6ad5968 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -122,6 +122,10 @@ To build with Qt 5 you need the following: sudo apt-get install libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools +Additionally, to support Wayland protocol for modern desktop environments: + + sudo apt install qtwayland5 + libqrencode (optional) can be installed with: sudo apt-get install libqrencode-dev @@ -181,6 +185,10 @@ To build with Qt 5 you need the following: sudo dnf install qt5-qttools-devel qt5-qtbase-devel +Additionally, to support Wayland protocol for modern desktop environments: + + sudo dnf install qt5-qtwayland + libqrencode (optional) can be installed with: sudo dnf install qrencode-devel diff --git a/doc/descriptors.md b/doc/descriptors.md index e27ff87546..3bbb626a42 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -99,7 +99,7 @@ Descriptors consist of several types of expressions. The top level expression is `ADDR` expressions are any type of supported address: - P2PKH addresses (base58, of the form `1...` for mainnet or `[nm]...` for testnet). Note that P2PKH addresses in descriptors cannot be used for P2PK outputs (use the `pk` function instead). - P2SH addresses (base58, of the form `3...` for mainnet or `2...` for testnet, defined in [BIP 13](https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki)). -- Segwit addresses (bech32, of the form `bc1...` for mainnet or `tb1...` for testnet, defined in [BIP 173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki)). +- Segwit addresses (bech32 and bech32m, of the form `bc1...` for mainnet or `tb1...` for testnet, defined in [BIP 173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki) and [BIP 350](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki)). ## Explanation diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 40d44aaa2e..6af5ead443 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -207,7 +207,6 @@ test_fuzz_fuzz_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_fuzz_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) $(RUNTIME_LDFLAGS) test_fuzz_fuzz_SOURCES = \ test/fuzz/addition_overflow.cpp \ - test/fuzz/addrdb.cpp \ test/fuzz/addrman.cpp \ test/fuzz/asmap.cpp \ test/fuzz/asmap_direct.cpp \ diff --git a/src/addrdb.h b/src/addrdb.h index 1e0ccb1f60..26400ee0b6 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -8,10 +8,8 @@ #include <fs.h> #include <net_types.h> // For banmap_t -#include <serialize.h> #include <univalue.h> -#include <string> #include <vector> class CAddress; @@ -21,21 +19,15 @@ class CDataStream; class CBanEntry { public: - static const int CURRENT_VERSION=1; - int nVersion; - int64_t nCreateTime; - int64_t nBanUntil; + static constexpr int CURRENT_VERSION{1}; + int nVersion{CBanEntry::CURRENT_VERSION}; + int64_t nCreateTime{0}; + int64_t nBanUntil{0}; - CBanEntry() - { - SetNull(); - } + CBanEntry() {} explicit CBanEntry(int64_t nCreateTimeIn) - { - SetNull(); - nCreateTime = nCreateTimeIn; - } + : nCreateTime{nCreateTimeIn} {} /** * Create a ban entry from JSON. @@ -44,19 +36,6 @@ public: */ explicit CBanEntry(const UniValue& json); - SERIALIZE_METHODS(CBanEntry, obj) - { - uint8_t ban_reason = 2; //! For backward compatibility - READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, ban_reason); - } - - void SetNull() - { - nVersion = CBanEntry::CURRENT_VERSION; - nCreateTime = 0; - nBanUntil = 0; - } - /** * Generate a JSON representation of this ban entry. * @return JSON suitable for passing to the `CBanEntry(const UniValue&)` constructor. diff --git a/src/addrman.cpp b/src/addrman.cpp index edcf97f846..48e79c64ed 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -15,6 +15,27 @@ #include <unordered_map> #include <unordered_set> +/** Over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread */ +static constexpr uint32_t ADDRMAN_TRIED_BUCKETS_PER_GROUP{8}; +/** Over how many buckets entries with new addresses originating from a single group are spread */ +static constexpr uint32_t ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP{64}; +/** Maximum number of times an address can be added to the new table */ +static constexpr int32_t ADDRMAN_NEW_BUCKETS_PER_ADDRESS{8}; +/** How old addresses can maximally be */ +static constexpr int64_t ADDRMAN_HORIZON_DAYS{30}; +/** After how many failed attempts we give up on a new node */ +static constexpr int32_t ADDRMAN_RETRIES{3}; +/** How many successive failures are allowed ... */ +static constexpr int32_t ADDRMAN_MAX_FAILURES{10}; +/** ... in at least this many days */ +static constexpr int64_t ADDRMAN_MIN_FAIL_DAYS{7}; +/** How recent a successful connection should be before we allow an address to be evicted from tried */ +static constexpr int64_t ADDRMAN_REPLACEMENT_HOURS{4}; +/** The maximum number of tried addr collisions to store */ +static constexpr size_t ADDRMAN_SET_TRIED_COLLISION_SIZE{10}; +/** The maximum time we'll spend trying to resolve a tried table collision, in seconds */ +static constexpr int64_t ADDRMAN_TEST_WINDOW{40*60}; // 40 minutes + int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const { uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash(); @@ -94,6 +115,285 @@ CAddrMan::CAddrMan(bool deterministic, int32_t consistency_check_ratio) } } +template <typename Stream> +void CAddrMan::Serialize(Stream& s_) const +{ + LOCK(cs); + + /** + * Serialized format. + * * format version byte (@see `Format`) + * * lowest compatible format version byte. This is used to help old software decide + * whether to parse the file. For example: + * * Bitcoin Core version N knows how to parse up to format=3. If a new format=4 is + * introduced in version N+1 that is compatible with format=3 and it is known that + * version N will be able to parse it, then version N+1 will write + * (format=4, lowest_compatible=3) in the first two bytes of the file, and so + * version N will still try to parse it. + * * Bitcoin Core version N+2 introduces a new incompatible format=5. It will write + * (format=5, lowest_compatible=5) and so any versions that do not know how to parse + * format=5 will not try to read the file. + * * nKey + * * nNew + * * nTried + * * number of "new" buckets XOR 2**30 + * * all new addresses (total count: nNew) + * * all tried addresses (total count: nTried) + * * for each new bucket: + * * number of elements + * * for each element: index in the serialized "all new addresses" + * * asmap checksum + * + * 2**30 is xorred with the number of buckets to make addrman deserializer v0 detect it + * as incompatible. This is necessary because it did not check the version number on + * deserialization. + * + * vvNew, vvTried, mapInfo, mapAddr and vRandom are never encoded explicitly; + * they are instead reconstructed from the other information. + * + * This format is more complex, but significantly smaller (at most 1.5 MiB), and supports + * changes to the ADDRMAN_ parameters without breaking the on-disk structure. + * + * We don't use SERIALIZE_METHODS since the serialization and deserialization code has + * very little in common. + */ + + // Always serialize in the latest version (FILE_FORMAT). + + OverrideStream<Stream> s(&s_, s_.GetType(), s_.GetVersion() | ADDRV2_FORMAT); + + s << static_cast<uint8_t>(FILE_FORMAT); + + // Increment `lowest_compatible` iff a newly introduced format is incompatible with + // the previous one. + static constexpr uint8_t lowest_compatible = Format::V3_BIP155; + s << static_cast<uint8_t>(INCOMPATIBILITY_BASE + lowest_compatible); + + s << nKey; + s << nNew; + s << nTried; + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + std::unordered_map<int, int> mapUnkIds; + int nIds = 0; + for (const auto& entry : mapInfo) { + mapUnkIds[entry.first] = nIds; + const CAddrInfo &info = entry.second; + if (info.nRefCount) { + assert(nIds != nNew); // this means nNew was wrong, oh ow + s << info; + nIds++; + } + } + nIds = 0; + for (const auto& entry : mapInfo) { + const CAddrInfo &info = entry.second; + if (info.fInTried) { + assert(nIds != nTried); // this means nTried was wrong, oh ow + s << info; + nIds++; + } + } + for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { + int nSize = 0; + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[bucket][i] != -1) + nSize++; + } + s << nSize; + for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { + if (vvNew[bucket][i] != -1) { + int nIndex = mapUnkIds[vvNew[bucket][i]]; + s << nIndex; + } + } + } + // Store asmap checksum after bucket entries so that it + // can be ignored by older clients for backward compatibility. + uint256 asmap_checksum; + if (m_asmap.size() != 0) { + asmap_checksum = SerializeHash(m_asmap); + } + s << asmap_checksum; +} + +template <typename Stream> +void CAddrMan::Unserialize(Stream& s_) +{ + LOCK(cs); + + assert(vRandom.empty()); + + Format format; + s_ >> Using<CustomUintFormatter<1>>(format); + + int stream_version = s_.GetVersion(); + if (format >= Format::V3_BIP155) { + // Add ADDRV2_FORMAT to the version so that the CNetAddr and CAddress + // unserialize methods know that an address in addrv2 format is coming. + stream_version |= ADDRV2_FORMAT; + } + + OverrideStream<Stream> s(&s_, s_.GetType(), stream_version); + + uint8_t compat; + s >> compat; + const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE; + if (lowest_compatible > FILE_FORMAT) { + throw std::ios_base::failure(strprintf( + "Unsupported format of addrman database: %u. It is compatible with formats >=%u, " + "but the maximum supported by this version of %s is %u.", + format, lowest_compatible, PACKAGE_NAME, static_cast<uint8_t>(FILE_FORMAT))); + } + + s >> nKey; + s >> nNew; + s >> nTried; + int nUBuckets = 0; + s >> nUBuckets; + if (format >= Format::V1_DETERMINISTIC) { + nUBuckets ^= (1 << 30); + } + + if (nNew > ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nNew < 0) { + throw std::ios_base::failure( + strprintf("Corrupt CAddrMan serialization: nNew=%d, should be in [0, %d]", + nNew, + ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); + } + + if (nTried > ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nTried < 0) { + throw std::ios_base::failure( + strprintf("Corrupt CAddrMan serialization: nTried=%d, should be in [0, %d]", + nTried, + ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); + } + + // Deserialize entries from the new table. + for (int n = 0; n < nNew; n++) { + CAddrInfo &info = mapInfo[n]; + s >> info; + mapAddr[info] = n; + info.nRandomPos = vRandom.size(); + vRandom.push_back(n); + } + nIdCount = nNew; + + // Deserialize entries from the tried table. + int nLost = 0; + for (int n = 0; n < nTried; n++) { + CAddrInfo info; + s >> info; + int nKBucket = info.GetTriedBucket(nKey, m_asmap); + int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); + if (info.IsValid() + && vvTried[nKBucket][nKBucketPos] == -1) { + info.nRandomPos = vRandom.size(); + info.fInTried = true; + vRandom.push_back(nIdCount); + mapInfo[nIdCount] = info; + mapAddr[info] = nIdCount; + vvTried[nKBucket][nKBucketPos] = nIdCount; + nIdCount++; + } else { + nLost++; + } + } + nTried -= nLost; + + // Store positions in the new table buckets to apply later (if possible). + // An entry may appear in up to ADDRMAN_NEW_BUCKETS_PER_ADDRESS buckets, + // so we store all bucket-entry_index pairs to iterate through later. + std::vector<std::pair<int, int>> bucket_entries; + + for (int bucket = 0; bucket < nUBuckets; ++bucket) { + int num_entries{0}; + s >> num_entries; + for (int n = 0; n < num_entries; ++n) { + int entry_index{0}; + s >> entry_index; + if (entry_index >= 0 && entry_index < nNew) { + bucket_entries.emplace_back(bucket, entry_index); + } + } + } + + // If the bucket count and asmap checksum haven't changed, then attempt + // to restore the entries to the buckets/positions they were in before + // serialization. + uint256 supplied_asmap_checksum; + if (m_asmap.size() != 0) { + supplied_asmap_checksum = SerializeHash(m_asmap); + } + uint256 serialized_asmap_checksum; + if (format >= Format::V2_ASMAP) { + s >> serialized_asmap_checksum; + } + const bool restore_bucketing{nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && + serialized_asmap_checksum == supplied_asmap_checksum}; + + if (!restore_bucketing) { + LogPrint(BCLog::ADDRMAN, "Bucketing method was updated, re-bucketing addrman entries from disk\n"); + } + + for (auto bucket_entry : bucket_entries) { + int bucket{bucket_entry.first}; + const int entry_index{bucket_entry.second}; + CAddrInfo& info = mapInfo[entry_index]; + + // Don't store the entry in the new bucket if it's not a valid address for our addrman + if (!info.IsValid()) continue; + + // The entry shouldn't appear in more than + // ADDRMAN_NEW_BUCKETS_PER_ADDRESS. If it has already, just skip + // this bucket_entry. + if (info.nRefCount >= ADDRMAN_NEW_BUCKETS_PER_ADDRESS) continue; + + int bucket_position = info.GetBucketPosition(nKey, true, bucket); + if (restore_bucketing && vvNew[bucket][bucket_position] == -1) { + // Bucketing has not changed, using existing bucket positions for the new table + vvNew[bucket][bucket_position] = entry_index; + ++info.nRefCount; + } else { + // In case the new table data cannot be used (bucket count wrong or new asmap), + // try to give them a reference based on their primary source address. + bucket = info.GetNewBucket(nKey, m_asmap); + bucket_position = info.GetBucketPosition(nKey, true, bucket); + if (vvNew[bucket][bucket_position] == -1) { + vvNew[bucket][bucket_position] = entry_index; + ++info.nRefCount; + } + } + } + + // Prune new entries with refcount 0 (as a result of collisions or invalid address). + int nLostUnk = 0; + for (auto it = mapInfo.cbegin(); it != mapInfo.cend(); ) { + if (it->second.fInTried == false && it->second.nRefCount == 0) { + const auto itCopy = it++; + Delete(itCopy->first); + ++nLostUnk; + } else { + ++it; + } + } + if (nLost + nLostUnk > 0) { + LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost); + } + + Check(); +} + +// explicit instantiation +template void CAddrMan::Serialize(CHashWriter& s) const; +template void CAddrMan::Serialize(CAutoFile& s) const; +template void CAddrMan::Serialize(CDataStream& s) const; +template void CAddrMan::Unserialize(CAutoFile& s); +template void CAddrMan::Unserialize(CHashVerifier<CAutoFile>& s); +template void CAddrMan::Unserialize(CDataStream& s); +template void CAddrMan::Unserialize(CHashVerifier<CDataStream>& s); + CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId) { AssertLockHeld(cs); diff --git a/src/addrman.h b/src/addrman.h index e2cb60b061..2548b891ba 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -131,49 +131,17 @@ public: * configuration option will introduce (expensive) consistency checks for the entire data structure. */ -//! total number of buckets for tried addresses -#define ADDRMAN_TRIED_BUCKET_COUNT_LOG2 8 +/** Total number of buckets for tried addresses */ +static constexpr int32_t ADDRMAN_TRIED_BUCKET_COUNT_LOG2{8}; +static constexpr int ADDRMAN_TRIED_BUCKET_COUNT{1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2}; -//! total number of buckets for new addresses -#define ADDRMAN_NEW_BUCKET_COUNT_LOG2 10 +/** Total number of buckets for new addresses */ +static constexpr int32_t ADDRMAN_NEW_BUCKET_COUNT_LOG2{10}; +static constexpr int ADDRMAN_NEW_BUCKET_COUNT{1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2}; -//! maximum allowed number of entries in buckets for new and tried addresses -#define ADDRMAN_BUCKET_SIZE_LOG2 6 - -//! over how many buckets entries with tried addresses from a single group (/16 for IPv4) are spread -#define ADDRMAN_TRIED_BUCKETS_PER_GROUP 8 - -//! over how many buckets entries with new addresses originating from a single group are spread -#define ADDRMAN_NEW_BUCKETS_PER_SOURCE_GROUP 64 - -//! in how many buckets for entries with new addresses a single address may occur -#define ADDRMAN_NEW_BUCKETS_PER_ADDRESS 8 - -//! how old addresses can maximally be -#define ADDRMAN_HORIZON_DAYS 30 - -//! after how many failed attempts we give up on a new node -#define ADDRMAN_RETRIES 3 - -//! how many successive failures are allowed ... -#define ADDRMAN_MAX_FAILURES 10 - -//! ... in at least this many days -#define ADDRMAN_MIN_FAIL_DAYS 7 - -//! how recent a successful connection should be before we allow an address to be evicted from tried -#define ADDRMAN_REPLACEMENT_HOURS 4 - -//! Convenience -#define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) -#define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2) -#define ADDRMAN_BUCKET_SIZE (1 << ADDRMAN_BUCKET_SIZE_LOG2) - -//! the maximum number of tried addr collisions to store -#define ADDRMAN_SET_TRIED_COLLISION_SIZE 10 - -//! the maximum time we'll spend trying to resolve a tried table collision, in seconds -static const int64_t ADDRMAN_TEST_WINDOW = 40*60; // 40 minutes +/** Maximum allowed number of entries in buckets for new and tried addresses */ +static constexpr int32_t ADDRMAN_BUCKET_SIZE_LOG2{6}; +static constexpr int ADDRMAN_BUCKET_SIZE{1 << ADDRMAN_BUCKET_SIZE_LOG2}; /** * Stochastical (IP) address manager @@ -200,276 +168,11 @@ public: // Read asmap from provided binary file static std::vector<bool> DecodeAsmap(fs::path path); - /** - * Serialized format. - * * format version byte (@see `Format`) - * * lowest compatible format version byte. This is used to help old software decide - * whether to parse the file. For example: - * * Bitcoin Core version N knows how to parse up to format=3. If a new format=4 is - * introduced in version N+1 that is compatible with format=3 and it is known that - * version N will be able to parse it, then version N+1 will write - * (format=4, lowest_compatible=3) in the first two bytes of the file, and so - * version N will still try to parse it. - * * Bitcoin Core version N+2 introduces a new incompatible format=5. It will write - * (format=5, lowest_compatible=5) and so any versions that do not know how to parse - * format=5 will not try to read the file. - * * nKey - * * nNew - * * nTried - * * number of "new" buckets XOR 2**30 - * * all new addresses (total count: nNew) - * * all tried addresses (total count: nTried) - * * for each new bucket: - * * number of elements - * * for each element: index in the serialized "all new addresses" - * * asmap checksum - * - * 2**30 is xorred with the number of buckets to make addrman deserializer v0 detect it - * as incompatible. This is necessary because it did not check the version number on - * deserialization. - * - * vvNew, vvTried, mapInfo, mapAddr and vRandom are never encoded explicitly; - * they are instead reconstructed from the other information. - * - * This format is more complex, but significantly smaller (at most 1.5 MiB), and supports - * changes to the ADDRMAN_ parameters without breaking the on-disk structure. - * - * We don't use SERIALIZE_METHODS since the serialization and deserialization code has - * very little in common. - */ template <typename Stream> - void Serialize(Stream& s_) const - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - - // Always serialize in the latest version (FILE_FORMAT). - - OverrideStream<Stream> s(&s_, s_.GetType(), s_.GetVersion() | ADDRV2_FORMAT); - - s << static_cast<uint8_t>(FILE_FORMAT); - - // Increment `lowest_compatible` iff a newly introduced format is incompatible with - // the previous one. - static constexpr uint8_t lowest_compatible = Format::V3_BIP155; - s << static_cast<uint8_t>(INCOMPATIBILITY_BASE + lowest_compatible); - - s << nKey; - s << nNew; - s << nTried; - - int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); - s << nUBuckets; - std::unordered_map<int, int> mapUnkIds; - int nIds = 0; - for (const auto& entry : mapInfo) { - mapUnkIds[entry.first] = nIds; - const CAddrInfo &info = entry.second; - if (info.nRefCount) { - assert(nIds != nNew); // this means nNew was wrong, oh ow - s << info; - nIds++; - } - } - nIds = 0; - for (const auto& entry : mapInfo) { - const CAddrInfo &info = entry.second; - if (info.fInTried) { - assert(nIds != nTried); // this means nTried was wrong, oh ow - s << info; - nIds++; - } - } - for (int bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { - int nSize = 0; - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvNew[bucket][i] != -1) - nSize++; - } - s << nSize; - for (int i = 0; i < ADDRMAN_BUCKET_SIZE; i++) { - if (vvNew[bucket][i] != -1) { - int nIndex = mapUnkIds[vvNew[bucket][i]]; - s << nIndex; - } - } - } - // Store asmap checksum after bucket entries so that it - // can be ignored by older clients for backward compatibility. - uint256 asmap_checksum; - if (m_asmap.size() != 0) { - asmap_checksum = SerializeHash(m_asmap); - } - s << asmap_checksum; - } + void Serialize(Stream& s_) const EXCLUSIVE_LOCKS_REQUIRED(!cs); template <typename Stream> - void Unserialize(Stream& s_) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - - assert(vRandom.empty()); - - Format format; - s_ >> Using<CustomUintFormatter<1>>(format); - - int stream_version = s_.GetVersion(); - if (format >= Format::V3_BIP155) { - // Add ADDRV2_FORMAT to the version so that the CNetAddr and CAddress - // unserialize methods know that an address in addrv2 format is coming. - stream_version |= ADDRV2_FORMAT; - } - - OverrideStream<Stream> s(&s_, s_.GetType(), stream_version); - - uint8_t compat; - s >> compat; - const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE; - if (lowest_compatible > FILE_FORMAT) { - throw std::ios_base::failure(strprintf( - "Unsupported format of addrman database: %u. It is compatible with formats >=%u, " - "but the maximum supported by this version of %s is %u.", - format, lowest_compatible, PACKAGE_NAME, static_cast<uint8_t>(FILE_FORMAT))); - } - - s >> nKey; - s >> nNew; - s >> nTried; - int nUBuckets = 0; - s >> nUBuckets; - if (format >= Format::V1_DETERMINISTIC) { - nUBuckets ^= (1 << 30); - } - - if (nNew > ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nNew < 0) { - throw std::ios_base::failure( - strprintf("Corrupt CAddrMan serialization: nNew=%d, should be in [0, %u]", - nNew, - ADDRMAN_NEW_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); - } - - if (nTried > ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE || nTried < 0) { - throw std::ios_base::failure( - strprintf("Corrupt CAddrMan serialization: nTried=%d, should be in [0, %u]", - nTried, - ADDRMAN_TRIED_BUCKET_COUNT * ADDRMAN_BUCKET_SIZE)); - } - - // Deserialize entries from the new table. - for (int n = 0; n < nNew; n++) { - CAddrInfo &info = mapInfo[n]; - s >> info; - mapAddr[info] = n; - info.nRandomPos = vRandom.size(); - vRandom.push_back(n); - } - nIdCount = nNew; - - // Deserialize entries from the tried table. - int nLost = 0; - for (int n = 0; n < nTried; n++) { - CAddrInfo info; - s >> info; - int nKBucket = info.GetTriedBucket(nKey, m_asmap); - int nKBucketPos = info.GetBucketPosition(nKey, false, nKBucket); - if (info.IsValid() - && vvTried[nKBucket][nKBucketPos] == -1) { - info.nRandomPos = vRandom.size(); - info.fInTried = true; - vRandom.push_back(nIdCount); - mapInfo[nIdCount] = info; - mapAddr[info] = nIdCount; - vvTried[nKBucket][nKBucketPos] = nIdCount; - nIdCount++; - } else { - nLost++; - } - } - nTried -= nLost; - - // Store positions in the new table buckets to apply later (if possible). - // An entry may appear in up to ADDRMAN_NEW_BUCKETS_PER_ADDRESS buckets, - // so we store all bucket-entry_index pairs to iterate through later. - std::vector<std::pair<int, int>> bucket_entries; - - for (int bucket = 0; bucket < nUBuckets; ++bucket) { - int num_entries{0}; - s >> num_entries; - for (int n = 0; n < num_entries; ++n) { - int entry_index{0}; - s >> entry_index; - if (entry_index >= 0 && entry_index < nNew) { - bucket_entries.emplace_back(bucket, entry_index); - } - } - } - - // If the bucket count and asmap checksum haven't changed, then attempt - // to restore the entries to the buckets/positions they were in before - // serialization. - uint256 supplied_asmap_checksum; - if (m_asmap.size() != 0) { - supplied_asmap_checksum = SerializeHash(m_asmap); - } - uint256 serialized_asmap_checksum; - if (format >= Format::V2_ASMAP) { - s >> serialized_asmap_checksum; - } - const bool restore_bucketing{nUBuckets == ADDRMAN_NEW_BUCKET_COUNT && - serialized_asmap_checksum == supplied_asmap_checksum}; - - if (!restore_bucketing) { - LogPrint(BCLog::ADDRMAN, "Bucketing method was updated, re-bucketing addrman entries from disk\n"); - } - - for (auto bucket_entry : bucket_entries) { - int bucket{bucket_entry.first}; - const int entry_index{bucket_entry.second}; - CAddrInfo& info = mapInfo[entry_index]; - - // Don't store the entry in the new bucket if it's not a valid address for our addrman - if (!info.IsValid()) continue; - - // The entry shouldn't appear in more than - // ADDRMAN_NEW_BUCKETS_PER_ADDRESS. If it has already, just skip - // this bucket_entry. - if (info.nRefCount >= ADDRMAN_NEW_BUCKETS_PER_ADDRESS) continue; - - int bucket_position = info.GetBucketPosition(nKey, true, bucket); - if (restore_bucketing && vvNew[bucket][bucket_position] == -1) { - // Bucketing has not changed, using existing bucket positions for the new table - vvNew[bucket][bucket_position] = entry_index; - ++info.nRefCount; - } else { - // In case the new table data cannot be used (bucket count wrong or new asmap), - // try to give them a reference based on their primary source address. - bucket = info.GetNewBucket(nKey, m_asmap); - bucket_position = info.GetBucketPosition(nKey, true, bucket); - if (vvNew[bucket][bucket_position] == -1) { - vvNew[bucket][bucket_position] = entry_index; - ++info.nRefCount; - } - } - } - - // Prune new entries with refcount 0 (as a result of collisions or invalid address). - int nLostUnk = 0; - for (auto it = mapInfo.cbegin(); it != mapInfo.cend(); ) { - if (it->second.fInTried == false && it->second.nRefCount == 0) { - const auto itCopy = it++; - Delete(itCopy->first); - ++nLostUnk; - } else { - ++it; - } - } - if (nLost + nLostUnk > 0) { - LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost); - } - - Check(); - } + void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs); explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio); diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 95886d3138..2d897f4c40 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -28,6 +28,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-addresstype", "-avoidpartialspends", "-changetype", + "-consolidatefeerate=<amt>", "-disablewallet", "-discardfee=<amt>", "-fallbackfee=<amt>", diff --git a/src/key.cpp b/src/key.cpp index 7bef3d529b..40df248e02 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -357,6 +357,7 @@ void CExtKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8]; memcpy(chaincode.begin(), code+9, 32); key.Set(code+42, code+BIP32_EXTKEY_SIZE, true); + if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || code[41] != 0) key = CKey(); } bool ECC_InitSanityCheck() { diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 8125b41c41..43624c7993 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -3,6 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <policy/rbf.h> + +#include <policy/settings.h> +#include <tinyformat.h> +#include <util/moneystr.h> #include <util/rbf.h> RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) @@ -42,3 +46,34 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx) // If we don't have a local mempool we can only check the transaction itself. return SignalsOptInRBF(tx) ? RBFTransactionState::REPLACEABLE_BIP125 : RBFTransactionState::UNKNOWN; } + +bool GetEntriesForConflicts(const CTransaction& tx, + CTxMemPool& m_pool, + const CTxMemPool::setEntries& setIterConflicting, + CTxMemPool::setEntries& allConflicting, + std::string& err_string) +{ + AssertLockHeld(m_pool.cs); + const uint256 hash = tx.GetHash(); + uint64_t nConflictingCount = 0; + for (const auto& mi : setIterConflicting) { + nConflictingCount += mi->GetCountWithDescendants(); + // This potentially overestimates the number of actual descendants + // but we just want to be conservative to avoid doing too much + // work. + if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) { + err_string = strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", + hash.ToString(), + nConflictingCount, + MAX_BIP125_REPLACEMENT_CANDIDATES); + return false; + } + } + // If not too many to replace, then calculate the set of + // transactions that would have to be evicted + for (CTxMemPool::txiter it : setIterConflicting) { + m_pool.CalculateDescendants(it, allConflicting); + } + return true; +} + diff --git a/src/policy/rbf.h b/src/policy/rbf.h index e078070c1c..a67e9058df 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -7,6 +7,10 @@ #include <txmempool.h> +/** Maximum number of transactions that can be replaced by BIP125 RBF (Rule #5). This includes all + * mempool conflicts and their descendants. */ +static constexpr uint32_t MAX_BIP125_REPLACEMENT_CANDIDATES{100}; + /** The rbf state of unconfirmed transactions */ enum class RBFTransactionState { /** Unconfirmed tx that does not signal rbf and is not in the mempool */ @@ -31,4 +35,19 @@ enum class RBFTransactionState { RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs); RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx); +/** Get all descendants of setIterConflicting. Also enforce BIP125 Rule #5, "The number of original + * transactions to be replaced and their descendant transactions which will be evicted from the + * mempool must not exceed a total of 100 transactions." Quit as early as possible. There cannot be + * more than MAX_BIP125_REPLACEMENT_CANDIDATES potential entries. + * @param[in] setIterConflicting The set of iterators to mempool entries. + * @param[out] err_string Used to return errors, if any. + * @param[out] allConflicting Populated with all the mempool entries that would be replaced, + * which includes descendants of setIterConflicting. Not cleared at + * the start; any existing mempool entries will remain in the set. + * @returns false if Rule 5 is broken. + */ +bool GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& m_pool, + const CTxMemPool::setEntries& setIterConflicting, + CTxMemPool::setEntries& allConflicting, + std::string& err_string) EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs); #endif // BITCOIN_POLICY_RBF_H diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 75202e7cf4..d14a20b870 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -180,6 +180,23 @@ XOnlyPubKey::XOnlyPubKey(Span<const unsigned char> bytes) std::copy(bytes.begin(), bytes.end(), m_keydata.begin()); } +std::vector<CKeyID> XOnlyPubKey::GetKeyIDs() const +{ + std::vector<CKeyID> out; + // For now, use the old full pubkey-based key derivation logic. As it is indexed by + // Hash160(full pubkey), we need to return both a version prefixed with 0x02, and one + // with 0x03. + unsigned char b[33] = {0x02}; + std::copy(m_keydata.begin(), m_keydata.end(), b + 1); + CPubKey fullpubkey; + fullpubkey.Set(b, b + 33); + out.push_back(fullpubkey.GetID()); + b[0] = 0x03; + fullpubkey.Set(b, b + 33); + out.push_back(fullpubkey.GetID()); + return out; +} + bool XOnlyPubKey::IsFullyValid() const { secp256k1_xonly_pubkey pubkey; @@ -333,6 +350,7 @@ void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { nChild = (code[5] << 24) | (code[6] << 16) | (code[7] << 8) | code[8]; memcpy(chaincode.begin(), code+9, 32); pubkey.Set(code+41, code+BIP32_EXTKEY_SIZE); + if ((nDepth == 0 && (nChild != 0 || ReadLE32(vchFingerprint) != 0)) || !pubkey.IsFullyValid()) pubkey = CPubKey(); } bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const { diff --git a/src/pubkey.h b/src/pubkey.h index eec34a89c2..861a2cf500 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -267,6 +267,11 @@ public: /** Construct a Taproot tweaked output point with this point as internal key. */ std::optional<std::pair<XOnlyPubKey, bool>> CreateTapTweak(const uint256* merkle_root) const; + /** Returns a list of CKeyIDs for the CPubKeys that could have been used to create this XOnlyPubKey. + * This is needed for key lookups since keys are indexed by CKeyID. + */ + std::vector<CKeyID> GetKeyIDs() const; + const unsigned char& operator[](int pos) const { return *(m_keydata.begin() + pos); } const unsigned char* data() const { return m_keydata.begin(); } static constexpr size_t size() { return decltype(m_keydata)::size(); } diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 682b55742a..621a1b9fd6 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1242,14 +1242,8 @@ std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseS CPubKey pubkey(full_key); std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true); KeyOriginInfo info; - if (provider.GetKeyOrigin(pubkey.GetID(), info)) { + if (provider.GetKeyOriginByXOnly(xkey, info)) { return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); - } else { - full_key[0] = 0x03; - pubkey = CPubKey(full_key); - if (provider.GetKeyOrigin(pubkey.GetID(), info)) { - return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); - } } return key_provider; } diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 4714d0ef11..b912b00365 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -60,22 +60,7 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider& assert(sigversion == SigVersion::TAPROOT || sigversion == SigVersion::TAPSCRIPT); CKey key; - { - // For now, use the old full pubkey-based key derivation logic. As it is indexed by - // Hash160(full pubkey), we need to try both a version prefixed with 0x02, and one - // with 0x03. - unsigned char b[33] = {0x02}; - std::copy(pubkey.begin(), pubkey.end(), b + 1); - CPubKey fullpubkey; - fullpubkey.Set(b, b + 33); - CKeyID keyid = fullpubkey.GetID(); - if (!provider.GetKey(keyid, key)) { - b[0] = 0x03; - fullpubkey.Set(b, b + 33); - CKeyID keyid = fullpubkey.GetID(); - if (!provider.GetKey(keyid, key)) return false; - } - } + if (!provider.GetKeyByXOnly(pubkey, key)) return false; // BIP341/BIP342 signing needs lots of precomputed transaction data. While some // (non-SIGHASH_DEFAULT) sighash modes exist that can work with just some subset diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h index 939ae10622..fbce61c6a9 100644 --- a/src/script/signingprovider.h +++ b/src/script/signingprovider.h @@ -26,6 +26,30 @@ public: virtual bool HaveKey(const CKeyID &address) const { return false; } virtual bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const { return false; } virtual bool GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const { return false; } + + bool GetKeyByXOnly(const XOnlyPubKey& pubkey, CKey& key) const + { + for (const auto& id : pubkey.GetKeyIDs()) { + if (GetKey(id, key)) return true; + } + return false; + } + + bool GetPubKeyByXOnly(const XOnlyPubKey& pubkey, CPubKey& out) const + { + for (const auto& id : pubkey.GetKeyIDs()) { + if (GetPubKey(id, out)) return true; + } + return false; + } + + bool GetKeyOriginByXOnly(const XOnlyPubKey& pubkey, KeyOriginInfo& info) const + { + for (const auto& id : pubkey.GetKeyIDs()) { + if (GetKeyOrigin(id, info)) return true; + } + return false; + } }; extern const SigningProvider& DUMMY_SIGNING_PROVIDER; diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index fb16c92647..a89868e1ef 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -14,6 +14,8 @@ #include <string> #include <vector> +namespace { + struct TestDerivation { std::string pub; std::string prv; @@ -99,7 +101,26 @@ TestVector test4 = "xprv9xJocDuwtYCMNAo3Zw76WENQeAS6WGXQ55RCy7tDJ8oALr4FWkuVoHJeHVAcAqiZLE7Je3vZJHxspZdFHfnBEjHqU5hG1Jaj32dVoS6XLT1", 0); -static void RunTest(const TestVector &test) { +const std::vector<std::string> TEST5 = { + "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6LBpB85b3D2yc8sfvZU521AAwdZafEz7mnzBBsz4wKY5fTtTQBm", + "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGTQQD3dC4H2D5GBj7vWvSQaaBv5cxi9gafk7NF3pnBju6dwKvH", + "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Txnt3siSujt9RCVYsx4qHZGc62TG4McvMGcAUjeuwZdduYEvFn", + "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFGpWnsj83BHtEy5Zt8CcDr1UiRXuWCmTQLxEK9vbz5gPstX92JQ", + "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6N8ZMMXctdiCjxTNq964yKkwrkBJJwpzZS4HS2fxvyYUA4q2Xe4", + "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD9y5gkZ6Eq3Rjuahrv17fEQ3Qen6J", + "xprv9s2SPatNQ9Vc6GTbVMFPFo7jsaZySyzk7L8n2uqKXJen3KUmvQNTuLh3fhZMBoG3G4ZW1N2kZuHEPY53qmbZzCHshoQnNf4GvELZfqTUrcv", + "xpub661no6RGEX3uJkY4bNnPcw4URcQTrSibUZ4NqJEw5eBkv7ovTwgiT91XX27VbEXGENhYRCf7hyEbWrR3FewATdCEebj6znwMfQkhRYHRLpJ", + "xprv9s21ZrQH4r4TsiLvyLXqM9P7k1K3EYhA1kkD6xuquB5i39AU8KF42acDyL3qsDbU9NmZn6MsGSUYZEsuoePmjzsB3eFKSUEh3Gu1N3cqVUN", + "xpub661MyMwAuDcm6CRQ5N4qiHKrJ39Xe1R1NyfouMKTTWcguwVcfrZJaNvhpebzGerh7gucBvzEQWRugZDuDXjNDRmXzSZe4c7mnTK97pTvGS8", + "DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHGMQzT7ayAmfo4z3gY5KfbrZWZ6St24UVf2Qgo6oujFktLHdHY4", + "DMwo58pR1QLEFihHiXPVykYB6fJmsTeHvyTp7hRThAtCX8CvYzgPcn8XnmdfHPmHJiEDXkTiJTVV9rHEBUem2mwVbbNfvT2MTcAqj3nesx8uBf9", + "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21Teqt1VvEHx", + "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD5SDKr24z3aiUvKr9bJpdrcLg1y3G", + "xpub661MyMwAqRbcEYS8w7XLSVeEsBXy79zSzH1J8vCdxAZningWLdN3zgtU6Q5JXayek4PRsn35jii4veMimro1xefsM58PgBMrvdYre8QyULY", + "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHL" +}; + +void RunTest(const TestVector &test) { std::vector<unsigned char> seed = ParseHex(test.strHexMaster); CExtKey key; CExtPubKey pubkey; @@ -133,6 +154,8 @@ static void RunTest(const TestVector &test) { } } +} // namespace + BOOST_FIXTURE_TEST_SUITE(bip32_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(bip32_test1) { @@ -151,4 +174,13 @@ BOOST_AUTO_TEST_CASE(bip32_test4) { RunTest(test4); } +BOOST_AUTO_TEST_CASE(bip32_test5) { + for (const auto& str : TEST5) { + auto dec_extkey = DecodeExtKey(str); + auto dec_extpubkey = DecodeExtPubKey(str); + BOOST_CHECK_MESSAGE(!dec_extkey.key.IsValid(), "Decoding '" + str + "' as xprv should fail"); + BOOST_CHECK_MESSAGE(!dec_extpubkey.pubkey.IsValid(), "Decoding '" + str + "' as xpub should fail"); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/addrdb.cpp b/src/test/fuzz/addrdb.cpp deleted file mode 100644 index d15c785673..0000000000 --- a/src/test/fuzz/addrdb.cpp +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) 2020 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <addrdb.h> -#include <test/fuzz/FuzzedDataProvider.h> -#include <test/fuzz/fuzz.h> -#include <test/fuzz/util.h> - -#include <cassert> -#include <cstdint> -#include <optional> -#include <string> -#include <vector> - -FUZZ_TARGET(addrdb) -{ - FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - - // The point of this code is to exercise all CBanEntry constructors. - const CBanEntry ban_entry = [&] { - switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 2)) { - case 0: - return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; - break; - case 1: { - const std::optional<CBanEntry> ban_entry = ConsumeDeserializable<CBanEntry>(fuzzed_data_provider); - if (ban_entry) { - return *ban_entry; - } - break; - } - } - return CBanEntry{}; - }(); - (void)ban_entry; // currently unused -} diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 49503e8dc6..cfbbe77311 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -195,10 +195,6 @@ FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, { CBlockHeader bh; DeserializeFromFuzzingInput(buffer, bh); }) -FUZZ_TARGET_DESERIALIZE(banentry_deserialize, { - CBanEntry be; - DeserializeFromFuzzingInput(buffer, be); -}) FUZZ_TARGET_DESERIALIZE(txundo_deserialize, { CTxUndo tu; DeserializeFromFuzzingInput(buffer, tu); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 97fd0600fa..20e26d1c35 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -955,6 +955,33 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); BOOST_CHECK_EQUAL(reason, "bare-multisig"); fIsBareMultisigStd = DEFAULT_PERMIT_BAREMULTISIG; + + // Check P2WPKH outputs dust threshold + t.vout[0].scriptPubKey = CScript() << OP_0 << ParseHex("ffffffffffffffffffffffffffffffffffffffff"); + t.vout[0].nValue = 294; + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + t.vout[0].nValue = 293; + BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK_EQUAL(reason, "dust"); + + // Check P2WSH outputs dust threshold + t.vout[0].scriptPubKey = CScript() << OP_0 << ParseHex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + t.vout[0].nValue = 330; + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + t.vout[0].nValue = 329; + BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK_EQUAL(reason, "dust"); + + // Check future Witness Program versions dust threshold + for (int op = OP_2; op <= OP_16; op += 1) { + t.vout[0].scriptPubKey = CScript() << (opcodetype)op << ParseHex("ffff"); + t.vout[0].nValue = 240; + BOOST_CHECK(IsStandardTx(CTransaction(t), reason)); + + t.vout[0].nValue = 239; + BOOST_CHECK(!IsStandardTx(CTransaction(t), reason)); + BOOST_CHECK_EQUAL(reason, "dust"); + } } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/rbf.h b/src/util/rbf.h index 6a20b37de5..4eb44b904f 100644 --- a/src/util/rbf.h +++ b/src/util/rbf.h @@ -11,8 +11,15 @@ class CTransaction; static const uint32_t MAX_BIP125_RBF_SEQUENCE = 0xfffffffd; -// Check whether the sequence numbers on this transaction are signaling -// opt-in to replace-by-fee, according to BIP 125 +/** Check whether the sequence numbers on this transaction are signaling +* opt-in to replace-by-fee, according to BIP 125. +* Allow opt-out of transaction replacement by setting +* nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. +* +* SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by +* non-replaceable transactions. All inputs rather than just one +* is for the sake of multi-party protocols, where we don't +* want a single party to be able to disable replacement. */ bool SignalsOptInRBF(const CTransaction &tx); #endif // BITCOIN_UTIL_RBF_H diff --git a/src/validation.cpp b/src/validation.cpp index 9944e31557..753b824167 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -25,6 +25,7 @@ #include <node/coinstats.h> #include <node/ui_interface.h> #include <policy/policy.h> +#include <policy/rbf.h> #include <policy/settings.h> #include <pow.h> #include <primitives/block.h> @@ -474,8 +475,10 @@ private: bool m_replacement_transaction; CAmount m_base_fees; CAmount m_modified_fees; - CAmount m_conflicting_fees; - size_t m_conflicting_size; + /** Total modified fees of all transactions being replaced. */ + CAmount m_conflicting_fees{0}; + /** Total virtual size of all transactions being replaced. */ + size_t m_conflicting_size{0}; const CTransactionRef& m_ptx; const uint256& m_hash; @@ -602,14 +605,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } if (!setConflicts.count(ptxConflicting->GetHash())) { - // Allow opt-out of transaction replacement by setting - // nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. - // - // SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by - // non-replaceable transactions. All inputs rather than just one - // is for the sake of multi-party protocols, where we don't - // want a single party to be able to disable replacement. - // // Transactions that don't explicitly signal replaceability are // *not* replaceable with the current logic, even if one of their // unconfirmed ancestors signals replaceability. This diverges @@ -617,16 +612,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Applications relying on first-seen mempool behavior should // check all unconfirmed ancestors; otherwise an opt-in ancestor // might be replaced, causing removal of this descendant. - bool fReplacementOptOut = true; - for (const CTxIn &_txin : ptxConflicting->vin) - { - if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE) - { - fReplacementOptOut = false; - break; - } - } - if (fReplacementOptOut) { + if (!SignalsOptInRBF(*ptxConflicting)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } @@ -796,11 +782,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } - // Check if it's economically rational to mine this transaction rather - // than the ones it replaces. - nConflictingFees = 0; - nConflictingSize = 0; - uint64_t nConflictingCount = 0; // If we don't hold the lock allConflicting might be incomplete; the // subsequent RemoveStaged() and addUnchecked() calls don't guarantee @@ -808,9 +789,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) fReplacementTransaction = setConflicts.size(); if (fReplacementTransaction) { + std::string err_string; CFeeRate newFeeRate(nModifiedFees, nSize); - std::set<uint256> setConflictsParents; - const int maxDescendantsToVisit = 100; for (const auto& mi : setIterConflicting) { // Don't allow the replacement to reduce the feerate of the // mempool. @@ -835,33 +815,26 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) newFeeRate.ToString(), oldFeeRate.ToString())); } + } + + // Calculate all conflicting entries and enforce Rule #5. + if (!GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting, err_string)) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", err_string); + } + + // Check if it's economically rational to mine this transaction rather + // than the ones it replaces. + for (CTxMemPool::txiter it : allConflicting) { + nConflictingFees += it->GetModifiedFee(); + nConflictingSize += it->GetTxSize(); + } + std::set<uint256> setConflictsParents; + for (const auto& mi : setIterConflicting) { for (const CTxIn &txin : mi->GetTx().vin) { setConflictsParents.insert(txin.prevout.hash); } - - nConflictingCount += mi->GetCountWithDescendants(); - } - // This potentially overestimates the number of actual descendants - // but we just want to be conservative to avoid doing too much - // work. - if (nConflictingCount <= maxDescendantsToVisit) { - // If not too many to replace, then calculate the set of - // transactions that would have to be evicted - for (CTxMemPool::txiter it : setIterConflicting) { - m_pool.CalculateDescendants(it, allConflicting); - } - for (CTxMemPool::txiter it : allConflicting) { - nConflictingFees += it->GetModifiedFee(); - nConflictingSize += it->GetTxSize(); - } - } else { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", - strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", - hash.ToString(), - nConflictingCount, - maxDescendantsToVisit)); } for (unsigned int j = 0; j < tx.vin.size(); j++) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 25b1ee07e4..1699424657 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -341,3 +341,30 @@ CAmount OutputGroup::GetSelectionAmount() const { return m_subtract_fee_outputs ? m_value : effective_value; } + +CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) +{ + // This function should not be called with empty inputs as that would mean the selection failed + assert(!inputs.empty()); + + // Always consider the cost of spending an input now vs in the future. + CAmount waste = 0; + CAmount selected_effective_value = 0; + for (const CInputCoin& coin : inputs) { + waste += coin.m_fee - coin.m_long_term_fee; + selected_effective_value += use_effective_value ? coin.effective_value : coin.txout.nValue; + } + + if (change_cost) { + // Consider the cost of making change and spending it in the future + // If we aren't making change, the caller should've set change_cost to 0 + assert(change_cost > 0); + waste += change_cost; + } else { + // When we are not making change (change_cost == 0), consider the excess we are throwing away to fees + assert(selected_effective_value >= target); + waste += selected_effective_value - target; + } + + return waste; +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 7a3fb82139..35617d455b 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -166,6 +166,21 @@ struct OutputGroup CAmount GetSelectionAmount() const; }; +/** Compute the waste for this result given the cost of change + * and the opportunity cost of spending these inputs now vs in the future. + * If change exists, waste = change_cost + inputs * (effective_feerate - long_term_feerate) + * If no change, waste = excess + inputs * (effective_feerate - long_term_feerate) + * where excess = selected_effective_value - target + * change_cost = effective_feerate * change_output_size + long_term_feerate * change_spend_size + * + * @param[in] inputs The selected inputs + * @param[in] change_cost The cost of creating change and spending it in the future. Only used if there is change. Must be 0 if there is no change. + * @param[in] target The amount targeted by the coin selection algorithm. + * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false). + * @return The waste + */ +[[nodiscard]] CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); + bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret); // Original coin selection algorithm as a fallback diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index eb0d6316c0..bb5f0cceff 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -45,6 +45,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(DEFAULT_ADDRESS_TYPE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-avoidpartialspends", strprintf("Group outputs by address, selecting many (possibly all) or none, instead of selecting on a per-output basis. Privacy is improved as addresses are mostly swept with fewer transactions and outputs are aggregated in clean change addresses. It may result in higher fees due to less optimal coin selection caused by this added limitation and possibly a larger-than-necessary number of inputs being used. Always enabled for wallets with \"avoid_reuse\" enabled, otherwise default: %u.", DEFAULT_AVOIDPARTIALSPENDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-changetype", "What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype, except when -addresstype=p2sh-segwit a native segwit output is used when sending to a native segwit address)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-consolidatefeerate=<amt>", strprintf("The maximum feerate (in %s/kvB) at which transaction building may use more inputs than strictly necessary so that the wallet's UTXO pool can be reduced (default: %s).", CURRENCY_UNIT, FormatMoney(DEFAULT_CONSOLIDATE_FEERATE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-disablewallet", "Do not load the wallet and disable wallet RPC calls", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-discardfee=<amt>", strprintf("The fee rate (in %s/kvB) that indicates your tolerance for discarding change by adding it to the fee (default: %s). " "Note: An output is discarded if it is dust at this rate, but we will always discard up to the dust relay fee and a discard fee above that is limited by the fee estimate for the longest target", diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 3bb7134b24..928335da2b 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -357,17 +357,44 @@ bool CWallet::AttemptSelection(const CAmount& nTargetValue, const CoinEligibilit { setCoinsRet.clear(); nValueRet = 0; + // Vector of results for use with waste calculation + // In order: calculated waste, selected inputs, selected input value (sum of input values) + // TODO: Use a struct representing the selection result + std::vector<std::tuple<CAmount, std::set<CInputCoin>, CAmount>> results; // Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output. std::vector<OutputGroup> positive_groups = GroupOutputs(coins, coin_selection_params, eligibility_filter, true /* positive_only */); - if (SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, setCoinsRet, nValueRet)) { - return true; + std::set<CInputCoin> bnb_coins; + CAmount bnb_value; + if (SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, bnb_coins, bnb_value)) { + const auto waste = GetSelectionWaste(bnb_coins, /* cost of change */ CAmount(0), nTargetValue, !coin_selection_params.m_subtract_fee_outputs); + results.emplace_back(std::make_tuple(waste, std::move(bnb_coins), bnb_value)); } + // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. std::vector<OutputGroup> all_groups = GroupOutputs(coins, coin_selection_params, eligibility_filter, false /* positive_only */); // While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output. // So we need to include that for KnapsackSolver as well, as we are expecting to create a change output. - return KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, setCoinsRet, nValueRet); + std::set<CInputCoin> knapsack_coins; + CAmount knapsack_value; + if (KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, knapsack_coins, knapsack_value)) { + const auto waste = GetSelectionWaste(knapsack_coins, coin_selection_params.m_cost_of_change, nTargetValue + coin_selection_params.m_change_fee, !coin_selection_params.m_subtract_fee_outputs); + results.emplace_back(std::make_tuple(waste, std::move(knapsack_coins), knapsack_value)); + } + + if (results.size() == 0) { + // No solution found + return false; + } + + // Choose the result with the least waste + // If the waste is the same, choose the one which spends more inputs. + const auto& best_result = std::min_element(results.begin(), results.end(), [](const auto& a, const auto& b) { + return std::get<0>(a) < std::get<0>(b) || (std::get<0>(a) == std::get<0>(b) && std::get<1>(a).size() > std::get<1>(b).size()); + }); + setCoinsRet = std::get<1>(*best_result); + nValueRet = std::get<2>(*best_result); + return true; } bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params) const @@ -586,6 +613,9 @@ bool CWallet::CreateTransactionInternal( CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; + // Set the long term feerate estimate to the wallet's consolidate feerate + coin_selection_params.m_long_term_feerate = m_consolidate_feerate; + CAmount recipients_sum = 0; const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); ReserveDestination reservedest(this, change_type); @@ -659,11 +689,6 @@ bool CWallet::CreateTransactionInternal( return false; } - // Get long term estimate - CCoinControl cc_temp; - cc_temp.m_confirm_target = chain().estimateMaxBlocks(); - coin_selection_params.m_long_term_feerate = GetMinimumFeeRate(*this, cc_temp, nullptr); - // Calculate the cost of change // Cost of change is the cost of creating the change output + cost of spending the change output in the future. // For creating the change output now, we use the effective feerate. diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 3488ae3526..7b2169a5b6 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -49,12 +49,16 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set.emplace_back(MakeTransactionRef(tx), nInput); } -static void add_coin(const CAmount& nValue, int nInput, CoinSet& set) +static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fee = 0, CAmount long_term_fee = 0) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; - set.emplace(MakeTransactionRef(tx), nInput); + CInputCoin coin(MakeTransactionRef(tx), nInput); + coin.effective_value = nValue - fee; + coin.m_fee = fee; + coin.m_long_term_fee = long_term_fee; + set.insert(coin); } static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) @@ -137,6 +141,13 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) return static_groups; } +inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinEligibilityFilter& filter) +{ + static std::vector<OutputGroup> static_groups; + static_groups = testWallet.GroupOutputs(vCoins, coin_selection_params, filter, /* positive_only */false); + return static_groups; +} + // Branch and bound coin selection tests BOOST_AUTO_TEST_CASE(bnb_search_test) { @@ -281,14 +292,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) empty_wallet(); add_coin(1); vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!testWallet.AttemptSelection( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb)); + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(vCoins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); // Test fees subtracted from output: empty_wallet(); add_coin(1 * CENT); vCoins.at(0).nInputBytes = 40; coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(testWallet.AttemptSelection( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb)); + BOOST_CHECK(SelectCoinsBnB(GroupCoins(vCoins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // Make sure that can use BnB when there are preset inputs @@ -323,24 +334,24 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.AttemptSelection( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); add_coin(1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.AttemptSelection( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.AttemptSelection( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); add_coin(2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.AttemptSelection( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(!KnapsackSolver(3 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.AttemptSelection( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(3 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -350,33 +361,33 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!testWallet.AttemptSelection(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.AttemptSelection(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_standard_extra), setCoinsRet, nValueRet)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.AttemptSelection(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(37 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.AttemptSelection(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.AttemptSelection(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(34 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK( testWallet.AttemptSelection( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(7 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.AttemptSelection( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(8 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.AttemptSelection( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(9 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -390,30 +401,30 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK( testWallet.AttemptSelection(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); - BOOST_CHECK(!testWallet.AttemptSelection(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(71 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(72 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK( testWallet.AttemptSelection(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK( testWallet.AttemptSelection(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK( testWallet.AttemptSelection(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.AttemptSelection(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(11 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -422,11 +433,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin( 2*COIN); add_coin( 3*COIN); add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.AttemptSelection(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(95 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK( testWallet.AttemptSelection(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(195 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -441,14 +452,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.AttemptSelection(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.AttemptSelection(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -456,7 +467,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.AttemptSelection(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -465,7 +476,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.AttemptSelection(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(500000 * COIN, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -478,7 +489,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.AttemptSelection(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -488,7 +499,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.AttemptSelection(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 @@ -499,12 +510,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.AttemptSelection(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 10001 / 100, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.AttemptSelection(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 9990 / 100, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); } @@ -518,7 +529,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(testWallet.AttemptSelection(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(2000, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: @@ -603,7 +614,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.AttemptSelection(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)); + BOOST_CHECK(KnapsackSolver(1003 * COIN, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -651,4 +662,73 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) } } +BOOST_AUTO_TEST_CASE(waste_test) +{ + CoinSet selection; + const CAmount fee{100}; + const CAmount change_cost{125}; + const CAmount fee_diff{40}; + const CAmount in_amt{3 * COIN}; + const CAmount target{2 * COIN}; + const CAmount excess{in_amt - fee * 2 - target}; + + // Waste with change is the change cost and difference between fee and long term fee + add_coin(1 * COIN, 1, selection, fee, fee - fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee - fee_diff); + const CAmount waste1 = GetSelectionWaste(selection, change_cost, target); + BOOST_CHECK_EQUAL(fee_diff * 2 + change_cost, waste1); + selection.clear(); + + // Waste without change is the excess and difference between fee and long term fee + add_coin(1 * COIN, 1, selection, fee, fee - fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee - fee_diff); + const CAmount waste_nochange1 = GetSelectionWaste(selection, 0, target); + BOOST_CHECK_EQUAL(fee_diff * 2 + excess, waste_nochange1); + selection.clear(); + + // Waste with change and fee == long term fee is just cost of change + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + BOOST_CHECK_EQUAL(change_cost, GetSelectionWaste(selection, change_cost, target)); + selection.clear(); + + // Waste without change and fee == long term fee is just the excess + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + BOOST_CHECK_EQUAL(excess, GetSelectionWaste(selection, 0, target)); + selection.clear(); + + // Waste will be greater when fee is greater, but long term fee is the same + add_coin(1 * COIN, 1, selection, fee * 2, fee - fee_diff); + add_coin(2 * COIN, 2, selection, fee * 2, fee - fee_diff); + const CAmount waste2 = GetSelectionWaste(selection, change_cost, target); + BOOST_CHECK_GT(waste2, waste1); + selection.clear(); + + // Waste with change is the change cost and difference between fee and long term fee + // With long term fee greater than fee, waste should be less than when long term fee is less than fee + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + const CAmount waste3 = GetSelectionWaste(selection, change_cost, target); + BOOST_CHECK_EQUAL(fee_diff * -2 + change_cost, waste3); + BOOST_CHECK_LT(waste3, waste1); + selection.clear(); + + // Waste without change is the excess and difference between fee and long term fee + // With long term fee greater than fee, waste should be less than when long term fee is less than fee + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + const CAmount waste_nochange2 = GetSelectionWaste(selection, 0, target); + BOOST_CHECK_EQUAL(fee_diff * -2 + excess, waste_nochange2); + BOOST_CHECK_LT(waste_nochange2, waste_nochange1); + selection.clear(); + + // 0 Waste only when fee == long term fee, no change, and no excess + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + const CAmount exact_target = in_amt - 2 * fee; + BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, 0, exact_target)); + +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 59bdf0405a..6f3dcf2afa 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2703,6 +2703,15 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri walletInstance->m_default_max_tx_fee = max_fee.value(); } + if (gArgs.IsArgSet("-consolidatefeerate")) { + if (std::optional<CAmount> consolidate_feerate = ParseMoney(gArgs.GetArg("-consolidatefeerate", ""))) { + walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate); + } else { + error = AmountErrMsg("consolidatefeerate", gArgs.GetArg("-consolidatefeerate", "")); + return nullptr; + } + } + if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + _("The wallet will avoid paying less than the minimum relay fee.")); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a1bbf66136..607af3efb0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -73,6 +73,8 @@ static const CAmount DEFAULT_FALLBACK_FEE = 0; static const CAmount DEFAULT_DISCARD_FEE = 10000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; +//! -consolidatefeerate default +static const CAmount DEFAULT_CONSOLIDATE_FEERATE{10000}; // 10 sat/vbyte /** * maximum fee increase allowed to do partial spend avoidance, even for nodes with this feature disabled by default * @@ -638,6 +640,12 @@ public: * output itself, just drop it to fees. */ CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE}; + /** When the actual feerate is less than the consolidate feerate, we will tend to make transactions which + * consolidate inputs. When the actual feerate is greater than the consolidate feerate, we will tend to make + * transactions which have the lowest fees. + */ + CFeeRate m_consolidate_feerate{DEFAULT_CONSOLIDATE_FEERATE}; + /** The maximum fee amount we're willing to pay to prioritize partial spend avoidance. */ CAmount m_max_aps_fee{DEFAULT_MAX_AVOIDPARTIALSPEND_FEE}; //!< note: this is absolute fee, not fee rate OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 2369ad25d5..0ce58efbcf 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -543,7 +543,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.nodes[1].getnewaddress() self.nodes[1].getrawchangeaddress() inputs = [] - outputs = {self.nodes[0].getnewaddress():1.09999500} + outputs = {self.nodes[0].getnewaddress():1.19999500} rawtx = self.nodes[1].createrawtransaction(inputs, outputs) # fund a transaction that does not require a new key for the change output self.nodes[1].fundrawtransaction(rawtx) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 9d4a5525d1..fbf8c6ef15 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -5,11 +5,11 @@ """Test the rawtransaction RPCs. Test the following RPCs: + - getrawtransaction - createrawtransaction - signrawtransactionwithwallet - sendrawtransaction - decoderawtransaction - - getrawtransaction """ from collections import OrderedDict @@ -28,6 +28,9 @@ from test_framework.util import ( ) +TXID = "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000" + + class multidict(dict): """Dictionary that allows duplicate keys. @@ -46,15 +49,15 @@ class multidict(dict): return self.x -# Create one-input, one-output, no-fee transaction: class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 3 + self.num_nodes = 4 self.extra_args = [ ["-txindex"], ["-txindex"], ["-txindex"], + [], ] # whitelist all peers to speed up tx relay / mempool sync for args in self.extra_args: @@ -70,23 +73,112 @@ class RawTransactionsTest(BitcoinTestFramework): self.connect_nodes(0, 2) def run_test(self): - self.log.info('prepare some coins for multiple *rawtransaction commands') + self.log.info("Prepare some coins for multiple *rawtransaction commands") self.nodes[2].generate(1) self.sync_all() self.nodes[0].generate(COINBASE_MATURITY + 1) self.sync_all() - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.5) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.0) - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),5.0) + for amount in [1.5, 1.0, 5.0]: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), amount) self.sync_all() self.nodes[0].generate(5) self.sync_all() - self.log.info('Test getrawtransaction on genesis block coinbase returns an error') + self.getrawtransaction_tests() + self.createrawtransaction_tests() + self.signrawtransactionwithwallet_tests() + self.sendrawtransaction_tests() + self.sendrawtransaction_testmempoolaccept_tests() + self.decoderawtransaction_tests() + self.transaction_version_number_tests() + if not self.options.descriptors: + self.raw_multisig_transaction_legacy_tests() + + def getrawtransaction_tests(self): + addr = self.nodes[1].getnewaddress() + txid = self.nodes[0].sendtoaddress(addr, 10) + self.nodes[0].generate(1) + self.sync_all() + vout = find_vout_for_address(self.nodes[1], txid, addr) + rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999}) + rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx) + txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex']) + self.nodes[0].generate(1) + self.sync_all() + + for n in [0, 3]: + self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex") + # 1. valid parameters - only supply txid + assert_equal(self.nodes[n].getrawtransaction(txId), rawTxSigned['hex']) + + # 2. valid parameters - supply txid and 0 for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, 0), rawTxSigned['hex']) + + # 3. valid parameters - supply txid and False for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, False), rawTxSigned['hex']) + + # 4. valid parameters - supply txid and 1 for verbose. + # We only check the "hex" field of the output so we don't need to update this test every time the output format changes. + assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex']) + + # 5. valid parameters - supply txid and True for non-verbose + assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], rawTxSigned['hex']) + + # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose + for value in ["True", "False"]: + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value) + + # 7. invalid parameters - supply txid and empty array + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, []) + + # 8. invalid parameters - supply txid and empty dict + assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {}) + + # Make a tx by sending, then generate 2 blocks; block1 has the tx in it + tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1) + block1, block2 = self.nodes[2].generate(2) + self.sync_all() + for n in [0, 3]: + self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash") + # We should be able to get the raw transaction by providing the correct block + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1) + assert_equal(gottx['txid'], tx) + assert_equal(gottx['in_active_chain'], True) + if n == 0: + self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent") + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True) + assert_equal(gottx['txid'], tx) + assert 'in_active_chain' not in gottx + else: + self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise") + err_msg = ( + "No such mempool transaction. Use -txindex or provide a block hash to enable" + " blockchain transaction queries. Use gettransaction for wallet transactions." + ) + assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True) + # We should not get the tx if we provide an unrelated block + assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2) + # An invalid block hash should raise the correct errors + assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True) + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar") + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234") + foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000" + assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo) + bar = "0000000000000000000000000000000000000000000000000000000000000000" + assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar) + # Undo the blocks and verify that "in_active_chain" is false. + self.nodes[n].invalidateblock(block1) + gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1) + assert_equal(gottx['in_active_chain'], False) + self.nodes[n].reconsiderblock(block1) + assert_equal(self.nodes[n].getbestblockhash(), block2) + + self.log.info("Test getrawtransaction on genesis block coinbase returns an error") block = self.nodes[0].getblock(self.nodes[0].getblockhash(0)) assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot']) - self.log.info('Check parameter types and required parameters of createrawtransaction') + def createrawtransaction_tests(self): + self.log.info("Test createrawtransaction") # Test `createrawtransaction` required parameters assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction) assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, []) @@ -95,16 +187,28 @@ class RawTransactionsTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [], {}, 0, False, 'foo') # Test `createrawtransaction` invalid `inputs` - txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {}) assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {}) assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {}) assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {}) - assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {}) - assert_raises_rpc_error(-8, "Invalid parameter, sequence number is out of range", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 0, 'sequence': -1}], {}) + txid = "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844" + assert_raises_rpc_error(-8, f"txid must be hexadecimal string (not '{txid}')", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': 'foo'}], {}) + assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': -1}], {}) + # sequence number out of range + for invalid_seq in [-1, 4294967296]: + inputs = [{'txid': TXID, 'vout': 1, 'sequence': invalid_seq}] + outputs = {self.nodes[0].getnewaddress(): 1} + assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', + self.nodes[0].createrawtransaction, inputs, outputs) + # with valid sequence number + for valid_seq in [1000, 4294967294]: + inputs = [{'txid': TXID, 'vout': 1, 'sequence': valid_seq}] + outputs = {self.nodes[0].getnewaddress(): 1} + rawtx = self.nodes[0].createrawtransaction(inputs, outputs) + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['vin'][0]['sequence'], valid_seq) # Test `createrawtransaction` invalid `outputs` address = self.nodes[0].getnewaddress() @@ -131,53 +235,51 @@ class RawTransactionsTest(BitcoinTestFramework): # Test `createrawtransaction` invalid `replaceable` assert_raises_rpc_error(-3, "Expected type bool", self.nodes[0].createrawtransaction, [], {}, 0, 'foo') - self.log.info('Check that createrawtransaction accepts an array and object as outputs') + # Test that createrawtransaction accepts an array and object as outputs # One output - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs={address: 99})) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs={address: 99})) assert_equal(len(tx.vout), 1) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}]), ) # Two outputs - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)]))) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)]))) assert_equal(len(tx.vout), 2) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]), ) # Multiple mixed outputs - tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')]))) + tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')]))) assert_equal(len(tx.vout), 3) assert_equal( tx.serialize().hex(), - self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]), + self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]), ) + def signrawtransactionwithwallet_tests(self): for type in ["bech32", "p2sh-segwit", "legacy"]: + self.log.info(f"Test signrawtransactionwithwallet with missing prevtx info ({type})") addr = self.nodes[0].getnewaddress("", type) addrinfo = self.nodes[0].getaddressinfo(addr) pubkey = addrinfo["scriptPubKey"] + inputs = [{'txid': TXID, 'vout': 3, 'sequence': 1000}] + outputs = {self.nodes[0].getnewaddress(): 1} + rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - self.log.info('sendrawtransaction with missing prevtx info (%s)' %(type)) - - # Test `signrawtransactionwithwallet` invalid `prevtxs` - inputs = [ {'txid' : txid, 'vout' : 3, 'sequence' : 1000}] - outputs = { self.nodes[0].getnewaddress() : 1 } - rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - - prevtx = dict(txid=txid, scriptPubKey=pubkey, vout=3, amount=1) + prevtx = dict(txid=TXID, scriptPubKey=pubkey, vout=3, amount=1) succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx]) assert succ["complete"] + if type == "legacy": del prevtx["amount"] succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx]) assert succ["complete"] - - if type != "legacy": + else: assert_raises_rpc_error(-3, "Missing amount", self.nodes[0].signrawtransactionwithwallet, rawtx, [ { - "txid": txid, + "txid": TXID, "scriptPubKey": pubkey, "vout": 3, } @@ -185,7 +287,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_raises_rpc_error(-3, "Missing vout", self.nodes[0].signrawtransactionwithwallet, rawtx, [ { - "txid": txid, + "txid": TXID, "scriptPubKey": pubkey, "amount": 1, } @@ -199,273 +301,23 @@ class RawTransactionsTest(BitcoinTestFramework): ]) assert_raises_rpc_error(-3, "Missing scriptPubKey", self.nodes[0].signrawtransactionwithwallet, rawtx, [ { - "txid": txid, + "txid": TXID, "vout": 3, "amount": 1 } ]) - ######################################### - # sendrawtransaction with missing input # - ######################################### - - self.log.info('sendrawtransaction with missing input') - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1}] #won't exists - outputs = { self.nodes[0].getnewaddress() : 4.998 } - rawtx = self.nodes[2].createrawtransaction(inputs, outputs) - rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx) - - # This will raise an exception since there are missing inputs + def sendrawtransaction_tests(self): + self.log.info("Test sendrawtransaction with missing input") + inputs = [{'txid': TXID, 'vout': 1}] # won't exist + outputs = {self.nodes[0].getnewaddress(): 4.998} + rawtx = self.nodes[2].createrawtransaction(inputs, outputs) + rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx) assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex']) - ##################################### - # getrawtransaction with block hash # - ##################################### - - # make a tx by sending then generate 2 blocks; block1 has the tx in it - tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1) - block1, block2 = self.nodes[2].generate(2) - self.sync_all() - # We should be able to get the raw transaction by providing the correct block - gottx = self.nodes[0].getrawtransaction(tx, True, block1) - assert_equal(gottx['txid'], tx) - assert_equal(gottx['in_active_chain'], True) - # We should not have the 'in_active_chain' flag when we don't provide a block - gottx = self.nodes[0].getrawtransaction(tx, True) - assert_equal(gottx['txid'], tx) - assert 'in_active_chain' not in gottx - # We should not get the tx if we provide an unrelated block - assert_raises_rpc_error(-5, "No such transaction found", self.nodes[0].getrawtransaction, tx, True, block2) - # An invalid block hash should raise the correct errors - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getrawtransaction, tx, True, True) - assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[0].getrawtransaction, tx, True, "foobar") - assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[0].getrawtransaction, tx, True, "abcd1234") - assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getrawtransaction, tx, True, "ZZZ0000000000000000000000000000000000000000000000000000000000000") - assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000") - # Undo the blocks and check in_active_chain - self.nodes[0].invalidateblock(block1) - gottx = self.nodes[0].getrawtransaction(txid=tx, verbose=True, blockhash=block1) - assert_equal(gottx['in_active_chain'], False) - self.nodes[0].reconsiderblock(block1) - assert_equal(self.nodes[0].getbestblockhash(), block2) - - if not self.options.descriptors: - # The traditional multisig workflow does not work with descriptor wallets so these are legacy only. - # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here. - ######################### - # RAW TX MULTISIG TESTS # - ######################### - # 2of2 test - addr1 = self.nodes[2].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[2].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - - # Tests for createmultisig and addmultisigaddress - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) - self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here. - - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] - - #use balance deltas instead of absolute values - bal = self.nodes[2].getbalance() - - # send 1.2 BTC to msig adr - txId = self.nodes[0].sendtoaddress(mSigObj, 1.2) - self.sync_all() - self.nodes[0].generate(1) - self.sync_all() - assert_equal(self.nodes[2].getbalance(), bal+Decimal('1.20000000')) #node2 has both keys of the 2of2 ms addr., tx should affect the balance - - - # 2of3 test from different nodes - bal = self.nodes[2].getbalance() - addr1 = self.nodes[1].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - addr3 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[1].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - addr3Obj = self.nodes[2].getaddressinfo(addr3) - - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address'] - - txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) - decTx = self.nodes[0].gettransaction(txId) - rawTx = self.nodes[0].decoderawtransaction(decTx['hex']) - self.sync_all() - self.nodes[0].generate(1) - self.sync_all() - - #THIS IS AN INCOMPLETE FEATURE - #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION - assert_equal(self.nodes[2].getbalance(), bal) #for now, assume the funds of a 2of3 multisig tx are not marked as spendable - - txDetails = self.nodes[0].gettransaction(txId, True) - rawTx = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000')) - - bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "amount" : vout['value']}] - outputs = { self.nodes[0].getnewaddress() : 2.19 } - rawTx = self.nodes[2].createrawtransaction(inputs, outputs) - rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs) - assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx - - rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs) - assert_equal(rawTxSigned['complete'], True) #node2 can sign the tx compl., own two of three keys - self.nodes[2].sendrawtransaction(rawTxSigned['hex']) - rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex']) - self.sync_all() - self.nodes[0].generate(1) - self.sync_all() - assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx - - # 2of2 test for combining transactions - bal = self.nodes[2].getbalance() - addr1 = self.nodes[1].getnewaddress() - addr2 = self.nodes[2].getnewaddress() - - addr1Obj = self.nodes[1].getaddressinfo(addr1) - addr2Obj = self.nodes[2].getaddressinfo(addr2) - - self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] - mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] - mSigObjValid = self.nodes[2].getaddressinfo(mSigObj) - - txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) - decTx = self.nodes[0].gettransaction(txId) - rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex']) - self.sync_all() - self.nodes[0].generate(1) - self.sync_all() - - assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable - - txDetails = self.nodes[0].gettransaction(txId, True) - rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex']) - vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000')) - - bal = self.nodes[0].getbalance() - inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}] - outputs = { self.nodes[0].getnewaddress() : 2.19 } - rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) - rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs) - self.log.debug(rawTxPartialSigned1) - assert_equal(rawTxPartialSigned1['complete'], False) #node1 only has one key, can't comp. sign the tx - - rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs) - self.log.debug(rawTxPartialSigned2) - assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx - rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) - self.log.debug(rawTxComb) - self.nodes[2].sendrawtransaction(rawTxComb) - rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb) - self.sync_all() - self.nodes[0].generate(1) - self.sync_all() - assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx - - # decoderawtransaction tests - # witness transaction - encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000" - decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction - assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000')) - assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # force decode as non-witness transaction - # non-witness transaction - encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000" - decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction - assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000')) - # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579) - encrawtx = "020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000" - decrawtx = self.nodes[0].decoderawtransaction(encrawtx) - decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True) - assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction - assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen - assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000") - - # Basic signrawtransaction test - addr = self.nodes[1].getnewaddress() - txid = self.nodes[0].sendtoaddress(addr, 10) - self.nodes[0].generate(1) - self.sync_all() - vout = find_vout_for_address(self.nodes[1], txid, addr) - rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999}) - rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx) - txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex']) - self.nodes[0].generate(1) - self.sync_all() - - # getrawtransaction tests - # 1. valid parameters - only supply txid - assert_equal(self.nodes[0].getrawtransaction(txId), rawTxSigned['hex']) - - # 2. valid parameters - supply txid and 0 for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, 0), rawTxSigned['hex']) - - # 3. valid parameters - supply txid and False for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, False), rawTxSigned['hex']) - - # 4. valid parameters - supply txid and 1 for verbose. - # We only check the "hex" field of the output so we don't need to update this test every time the output format changes. - assert_equal(self.nodes[0].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex']) - - # 5. valid parameters - supply txid and True for non-verbose - assert_equal(self.nodes[0].getrawtransaction(txId, True)["hex"], rawTxSigned['hex']) - - # 6. invalid parameters - supply txid and string "Flase" - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, "Flase") - - # 7. invalid parameters - supply txid and empty array - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, []) - - # 8. invalid parameters - supply txid and empty dict - assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, {}) - - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}] - outputs = { self.nodes[0].getnewaddress() : 1 } - rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - decrawtx= self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['vin'][0]['sequence'], 1000) - - # 9. invalid parameters - sequence number out of range - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : -1}] - outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) - - # 10. invalid parameters - sequence number out of range - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967296}] - outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) - - inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967294}] - outputs = { self.nodes[0].getnewaddress() : 1 } - rawtx = self.nodes[0].createrawtransaction(inputs, outputs) - decrawtx= self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['vin'][0]['sequence'], 4294967294) - - #################################### - # TRANSACTION VERSION NUMBER TESTS # - #################################### - - # Test the minimum transaction version number that fits in a signed 32-bit integer. - # As transaction version is unsigned, this should convert to its unsigned equivalent. - tx = CTransaction() - tx.nVersion = -0x80000000 - rawtx = tx.serialize().hex() - decrawtx = self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['version'], 0x80000000) - - # Test the maximum transaction version number that fits in a signed 32-bit integer. - tx = CTransaction() - tx.nVersion = 0x7fffffff - rawtx = tx.serialize().hex() - decrawtx = self.nodes[0].decoderawtransaction(rawtx) - assert_equal(decrawtx['version'], 0x7fffffff) - - self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate') + def sendrawtransaction_testmempoolaccept_tests(self): + self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate") + fee_exceeds_max = "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)" # Test a transaction with a small fee. txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) @@ -473,9 +325,9 @@ class RawTransactionsTest(BitcoinTestFramework): vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() - inputs = [{ "txid" : txId, "vout" : vout['n'] }] + inputs = [{"txid": txId, "vout": vout['n']}] # Fee 10,000 satoshis, (1 - (10000 sat * 0.00000001 BTC/sat)) = 0.9999 - outputs = { self.nodes[0].getnewaddress() : Decimal("0.99990000") } + outputs = {self.nodes[0].getnewaddress(): Decimal("0.99990000")} rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) @@ -485,7 +337,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(testres['allowed'], False) assert_equal(testres['reject-reason'], 'max-fee-exceeded') # and sendrawtransaction should throw - assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) + assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000) # and the following calls should both succeed testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0] assert_equal(testres['allowed'], True) @@ -497,9 +349,9 @@ class RawTransactionsTest(BitcoinTestFramework): vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000')) self.sync_all() - inputs = [{ "txid" : txId, "vout" : vout['n'] }] + inputs = [{"txid": txId, "vout": vout['n']}] # Fee 2,000,000 satoshis, (1 - (2000000 sat * 0.00000001 BTC/sat)) = 0.98 - outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") } + outputs = {self.nodes[0].getnewaddress() : Decimal("0.98000000")} rawTx = self.nodes[2].createrawtransaction(inputs, outputs) rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx) assert_equal(rawTxSigned['complete'], True) @@ -509,13 +361,13 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(testres['allowed'], False) assert_equal(testres['reject-reason'], 'max-fee-exceeded') # and sendrawtransaction should throw - assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex']) + assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex']) # and the following calls should both succeed testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0] assert_equal(testres['allowed'], True) self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000') - self.log.info('sendrawtransaction/testmempoolaccept with tx that is already in the chain') + self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain") self.nodes[2].generate(1) self.sync_blocks() for node in self.nodes: @@ -524,6 +376,166 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(testres['reject-reason'], 'txn-already-known') assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, rawTxSigned['hex']) + def decoderawtransaction_tests(self): + self.log.info("Test decoderawtransaction") + # witness transaction + encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000" + decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction + assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000')) + assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # force decode as non-witness transaction + # non-witness transaction + encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000" + decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction + assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000')) + # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579) + coinbase = "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000" + encrawtx = f"020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b{coinbase}" \ + "ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000" + decrawtx = self.nodes[0].decoderawtransaction(encrawtx) + decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True) + assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction + assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen + assert_equal(decrawtx['vin'][0]['coinbase'], coinbase) + + def transaction_version_number_tests(self): + self.log.info("Test transaction version numbers") + + # Test the minimum transaction version number that fits in a signed 32-bit integer. + # As transaction version is unsigned, this should convert to its unsigned equivalent. + tx = CTransaction() + tx.nVersion = -0x80000000 + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0x80000000) + + # Test the maximum transaction version number that fits in a signed 32-bit integer. + tx = CTransaction() + tx.nVersion = 0x7fffffff + rawtx = tx.serialize().hex() + decrawtx = self.nodes[0].decoderawtransaction(rawtx) + assert_equal(decrawtx['version'], 0x7fffffff) + + def raw_multisig_transaction_legacy_tests(self): + self.log.info("Test raw multisig transactions (legacy)") + # The traditional multisig workflow does not work with descriptor wallets so these are legacy only. + # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here. + + # 2of2 test + addr1 = self.nodes[2].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[2].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + + # Tests for createmultisig and addmultisigaddress + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + # createmultisig can only take public keys + self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) + # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here + assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) + + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] + + # use balance deltas instead of absolute values + bal = self.nodes[2].getbalance() + + # send 1.2 BTC to msig adr + txId = self.nodes[0].sendtoaddress(mSigObj, 1.2) + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + # node2 has both keys of the 2of2 ms addr, tx should affect the balance + assert_equal(self.nodes[2].getbalance(), bal + Decimal('1.20000000')) + + + # 2of3 test from different nodes + bal = self.nodes[2].getbalance() + addr1 = self.nodes[1].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + addr3 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[1].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + addr3Obj = self.nodes[2].getaddressinfo(addr3) + + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address'] + + txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) + decTx = self.nodes[0].gettransaction(txId) + rawTx = self.nodes[0].decoderawtransaction(decTx['hex']) + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + # THIS IS AN INCOMPLETE FEATURE + # NODE2 HAS TWO OF THREE KEYS AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION + assert_equal(self.nodes[2].getbalance(), bal) # for now, assume the funds of a 2of3 multisig tx are not marked as spendable + + txDetails = self.nodes[0].gettransaction(txId, True) + rawTx = self.nodes[0].decoderawtransaction(txDetails['hex']) + vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000')) + + bal = self.nodes[0].getbalance() + inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex'], "amount": vout['value']}] + outputs = {self.nodes[0].getnewaddress(): 2.19} + rawTx = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs) + assert_equal(rawTxPartialSigned['complete'], False) # node1 only has one key, can't comp. sign the tx + + rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs) + assert_equal(rawTxSigned['complete'], True) # node2 can sign the tx compl., own two of three keys + self.nodes[2].sendrawtransaction(rawTxSigned['hex']) + rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex']) + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + assert_equal(self.nodes[0].getbalance(), bal + Decimal('50.00000000') + Decimal('2.19000000')) # block reward + tx + + # 2of2 test for combining transactions + bal = self.nodes[2].getbalance() + addr1 = self.nodes[1].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[1].getaddressinfo(addr1) + addr2Obj = self.nodes[2].getaddressinfo(addr2) + + self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] + mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address'] + mSigObjValid = self.nodes[2].getaddressinfo(mSigObj) + + txId = self.nodes[0].sendtoaddress(mSigObj, 2.2) + decTx = self.nodes[0].gettransaction(txId) + rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex']) + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable + + txDetails = self.nodes[0].gettransaction(txId, True) + rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex']) + vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000')) + + bal = self.nodes[0].getbalance() + inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex'], "redeemScript": mSigObjValid['hex'], "amount": vout['value']}] + outputs = {self.nodes[0].getnewaddress(): 2.19} + rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs) + rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs) + self.log.debug(rawTxPartialSigned1) + assert_equal(rawTxPartialSigned1['complete'], False) # node1 only has one key, can't comp. sign the tx + + rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs) + self.log.debug(rawTxPartialSigned2) + assert_equal(rawTxPartialSigned2['complete'], False) # node2 only has one key, can't comp. sign the tx + rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) + self.log.debug(rawTxComb) + self.nodes[2].sendrawtransaction(rawTxComb) + rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb) + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + assert_equal(self.nodes[0].getbalance(), bal + Decimal('50.00000000') + Decimal('2.19000000')) # block reward + tx + if __name__ == '__main__': RawTransactionsTest().main() diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index df5051720b..233381f2d9 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -15,6 +15,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "index/base -> validation -> index/blockfilterindex -> index/base" "index/coinstatsindex -> node/coinstats -> index/coinstatsindex" "policy/fees -> txmempool -> policy/fees" + "policy/rbf -> txmempool -> validation -> policy/rbf" "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel" "qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel" "qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog" |