diff options
58 files changed, 1339 insertions, 897 deletions
diff --git a/contrib/builder-keys/keys.txt b/contrib/builder-keys/keys.txt index c70069b440..f8377cce33 100644 --- a/contrib/builder-keys/keys.txt +++ b/contrib/builder-keys/keys.txt @@ -19,6 +19,7 @@ BF6273FAEF7CC0BA1F562E50989F6B3048A116B5 Dev Random (devrandom) D35176BE9264832E4ACA8986BF0792FBE95DC863 fivepiece (fivepiece) 6F993B250557E7B016ADE5713BDCDA2D87A881D9 Fuzzbawls (Fuzzbawls) 01CDF4627A3B88AAE4A571C87588242FBE38D3A8 Gavin Andresen (gavinandresen) +6B002C6EA3F91B1B0DF0C9BC8F617F1200A6D25C Gloria Zhao (glozow) D1DBF2C4B96F2DEBF4C16654410108112E7EA81F Hennadii Stepanov (hebasto) A2FD494D0021AA9B4FA58F759102B7AE654A4A5A Ilyas Ridhuan (IlyasRidhuan) 2688F5A9A4BE0F295E921E8A25F27A38A47AD566 James O'Beirne (jamesob) diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 6c6dd0c5ee..12b563eac2 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -147,7 +147,10 @@ desirable for building Bitcoin Core release binaries." base-gcc)) (define (make-gcc-with-pthreads gcc) - (package-with-extra-configure-variable gcc "--enable-threads" "posix")) + (package-with-extra-configure-variable + (package-with-extra-patches gcc + (search-our-patches "gcc-10-remap-guix-store.patch")) + "--enable-threads" "posix")) (define (make-mingw-w64-cross-gcc cross-gcc) (package-with-extra-patches cross-gcc diff --git a/contrib/guix/patches/gcc-10-remap-guix-store.patch b/contrib/guix/patches/gcc-10-remap-guix-store.patch new file mode 100644 index 0000000000..a47ef7a2df --- /dev/null +++ b/contrib/guix/patches/gcc-10-remap-guix-store.patch @@ -0,0 +1,25 @@ +From aad25427e74f387412e8bc9a9d7bbc6c496c792f Mon Sep 17 00:00:00 2001 +From: Andrew Chow <achow101-github@achow101.com> +Date: Wed, 6 Jul 2022 16:49:41 -0400 +Subject: [PATCH] guix: remap guix store paths to /usr + +--- + libgcc/Makefile.in | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in +index 851e7657d07..476c2becd1c 100644 +--- a/libgcc/Makefile.in ++++ b/libgcc/Makefile.in +@@ -854,7 +854,7 @@ endif + # libgcc_eh.a, only LIB2ADDEH matters. If we do, only LIB2ADDEHSTATIC and + # LIB2ADDEHSHARED matter. (Usually all three are identical.) + +-c_flags := -fexceptions ++c_flags := -fexceptions $(shell find /gnu/store -maxdepth 1 -mindepth 1 -type d -exec echo -n " -ffile-prefix-map={}=/usr" \;) + + ifeq ($(enable_shared),yes) + +-- +2.37.0 + diff --git a/contrib/verify-commits/trusted-keys b/contrib/verify-commits/trusted-keys index 2f8a21009a..5ca65e7b0d 100644 --- a/contrib/verify-commits/trusted-keys +++ b/contrib/verify-commits/trusted-keys @@ -1,6 +1,6 @@ 71A3B16735405025D447E8F274810B012346C9A6 -133EAC179436F14A5CF1B794860FEB804E669320 B8B3F1C0E58C15DB6A81D30C3648A882F4316B9B E777299FC265DD04793070EB944D35F9AC3DB76A D1DBF2C4B96F2DEBF4C16654410108112E7EA81F 152812300785C96444D3334D17565732E08E5E41 +6B002C6EA3F91B1B0DF0C9BC8F617F1200A6D25C diff --git a/doc/build-netbsd.md b/doc/build-netbsd.md index 9cec201faf..0f05cdcba7 100644 --- a/doc/build-netbsd.md +++ b/doc/build-netbsd.md @@ -1,92 +1,116 @@ -NetBSD Build Guide -====================== -**Updated for NetBSD [8.0](https://www.netbsd.org/releases/formal-8/NetBSD-8.0.html)** +# NetBSD Build Guide -This guide describes how to build bitcoind and command-line utilities on NetBSD. +Updated for NetBSD [9.2](https://netbsd.org/releases/formal-9/NetBSD-9.2.html). -This guide does not contain instructions for building the GUI. +This guide describes how to build bitcoind, command-line utilities, and GUI on NetBSD. -Preparation -------------- +## Preparation -You will need the following modules, which can be installed via pkgsrc or pkgin: +### 1. Install Required Dependencies + +Install the required dependencies the usual way you [install software on NetBSD](https://www.netbsd.org/docs/guide/en/chap-boot.html#chap-boot-pkgsrc). +The example commands below use `pkgin`. + +```bash +pkgin install autoconf automake libtool pkg-config git gmake boost libevent ``` -autoconf -automake -boost -git -gmake -libevent -libtool -pkg-config -python37 -git clone https://github.com/bitcoin/bitcoin.git +NetBSD currently ships with an older version of `gcc` than is needed to build. You should upgrade your `gcc` and then pass this new version to the configure script. + +For example, grab `gcc9`: +``` +pkgin install gcc9 +``` + +Then, when configuring, pass the following: +```bash +./configure + ... + CC="/usr/pkg/gcc9/bin/gcc" \ + CXX="/usr/pkg/gcc9/bin/g++" \ + ... ``` See [dependencies.md](dependencies.md) for a complete overview. -### Building Bitcoin Core +### 2. Clone Bitcoin Repo -**Important**: Use `gmake` (the non-GNU `make` will exit with an error). +Clone the Bitcoin Core repository to a directory. All build scripts and commands will run from this directory. -#### With descriptor wallet: +```bash +git clone https://github.com/bitcoin/bitcoin.git +``` + +### 3. Install Optional Dependencies + +#### Wallet Dependencies + +It is not necessary to build wallet functionality to run bitcoind or the GUI. + +###### Descriptor Wallet Support + +`sqlite3` is required to enable support for [descriptor wallets](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md). -The descriptor wallet uses `sqlite3`. You can install it using: ```bash pkgin install sqlite3 ``` +###### Legacy Wallet Support + +`db4` is required to enable support for legacy wallets. + ```bash -./autogen.sh -./configure --with-gui=no --without-bdb \ - CPPFLAGS="-I/usr/pkg/include" \ - LDFLAGS="-L/usr/pkg/lib" \ - BOOST_CPPFLAGS="-I/usr/pkg/include" \ - MAKE=gmake +pkgin install db4 ``` -#### With legacy wallet: - -BerkeleyDB is use for legacy wallet functionality. +#### GUI Dependencies -It is recommended to use Berkeley DB 4.8. You cannot use the BerkeleyDB library -from ports. -You can use [the installation script included in contrib/](/contrib/install_db4.sh) like so: +Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install `qt5`. ```bash -./contrib/install_db4.sh `pwd` +pkgin install qt5 ``` -from the root of the repository. Then set `BDB_PREFIX` for the next section: +The GUI can encode addresses in a QR Code. To build in QR support for the GUI, install `qrencode`. ```bash -export BDB_PREFIX="$PWD/db4" +pkgin install qrencode ``` +#### Test Suite Dependencies + +There is an included test suite that is useful for testing code changes when developing. +To run the test suite (recommended), you will need to have Python 3 installed: + ```bash -./autogen.sh -./configure --with-gui=no CPPFLAGS="-I/usr/pkg/include" \ - LDFLAGS="-L/usr/pkg/lib" \ - BOOST_CPPFLAGS="-I/usr/pkg/include" \ - BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" \ - BDB_CFLAGS="-I${BDB_PREFIX}/include" \ - MAKE=gmake +pkgin install python37 ``` -#### Without wallet: +### Building Bitcoin Core + +**Note**: Use `gmake` (the non-GNU `make` will exit with an error). + + +### 1. Configuration + +There are many ways to configure Bitcoin Core. Here is an example that +explicitly disables the wallet and GUI: + ```bash ./autogen.sh -./configure --with-gui=no --disable-wallet \ +./configure --without-wallet --with-gui=no \ CPPFLAGS="-I/usr/pkg/include" \ - LDFLAGS="-L/usr/pkg/lib" \ - BOOST_CPPFLAGS="-I/usr/pkg/include" \ MAKE=gmake ``` +For a full list of configuration options, see the output of `./configure --help` + +### 2. Compile + Build and run the tests: + ```bash gmake # use "-j N" here for N parallel jobs -gmake check +gmake check # Run tests if Python 3 is available ``` diff --git a/doc/policy/mempool-replacements.md b/doc/policy/mempool-replacements.md index 18f08daf88..fea0143757 100644 --- a/doc/policy/mempool-replacements.md +++ b/doc/policy/mempool-replacements.md @@ -15,6 +15,8 @@ other consensus and policy rules, each of the following conditions are met: *Rationale*: See [BIP125 explanation](https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki#motivation). + The Bitcoin Core implementation offers a node setting (`mempoolfullrbf`) to allow transaction + replacement without enforcement of the opt-in signaling rule. 2. The replacement transaction only include an unconfirmed input if that input was included in one of the directly conflicting transactions. An unconfirmed input spends an output from a @@ -74,3 +76,6 @@ This set of rules is similar but distinct from BIP125. * RBF enabled by default in the wallet GUI as of **v0.18.1** ([PR #11605](https://github.com/bitcoin/bitcoin/pull/11605)). + +* Full replace-by-fee enabled as a configurable mempool policy as of **v24.0** ([PR + #25353](https://github.com/bitcoin/bitcoin/pull/25353)). diff --git a/doc/release-notes.md b/doc/release-notes.md index 35f0713879..2c3bb27935 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -86,6 +86,10 @@ Changes to GUI or wallet related settings can be found in the GUI or Wallet sect New settings ------------ +- A new `mempoolfullrbf` option has been added, which enables the mempool to + accept transaction replacement without enforcing the opt-in replaceability + signal. (#25353) + Tools and Utilities ------------------- diff --git a/doc/release-notes/release-notes-471.md b/doc/release-notes/release-notes-471.md new file mode 100644 index 0000000000..7cebedd8b3 --- /dev/null +++ b/doc/release-notes/release-notes-471.md @@ -0,0 +1,4 @@ +GUI changes +-------- + +- A new menu item to restore a wallet from a backup file has been added (#471).
\ No newline at end of file diff --git a/src/Makefile.am b/src/Makefile.am index bc0982f74d..3fbbe180fc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -139,6 +139,7 @@ BITCOIN_CORE_H = \ compat/cpuid.h \ compat/endian.h \ compressor.h \ + node/connection_types.h \ consensus/consensus.h \ consensus/tx_check.h \ consensus/tx_verify.h \ @@ -148,6 +149,7 @@ BITCOIN_CORE_H = \ dbwrapper.h \ deploymentinfo.h \ deploymentstatus.h \ + node/eviction.h \ external_signer.h \ flatfile.h \ fs.h \ @@ -373,7 +375,9 @@ libbitcoin_node_a_SOURCES = \ node/caches.cpp \ node/chainstate.cpp \ node/coin.cpp \ + node/connection_types.cpp \ node/context.cpp \ + node/eviction.cpp \ node/interfaces.cpp \ node/miner.cpp \ node/minisketchwrapper.cpp \ diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index b2958bcc9f..eaefb9b63a 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -58,7 +58,8 @@ static void CoinSelection(benchmark::Bench& bench) // Create coins std::vector<COutput> coins; for (const auto& wtx : wtxs) { - coins.emplace_back(COutPoint(wtx->GetHash(), 0), wtx->tx->vout.at(0), /*depth=*/6 * 24, GetTxSpendSize(wallet, *wtx, 0), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); + const auto txout = wtx->tx->vout.at(0); + coins.emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0); } const CoinEligibilityFilter filter_standard(1, 6, 0); diff --git a/src/bench/gcs_filter.cpp b/src/bench/gcs_filter.cpp index 607e4392b7..80babb213b 100644 --- a/src/bench/gcs_filter.cpp +++ b/src/bench/gcs_filter.cpp @@ -5,39 +5,84 @@ #include <bench/bench.h> #include <blockfilter.h> -static void ConstructGCSFilter(benchmark::Bench& bench) +static const GCSFilter::ElementSet GenerateGCSTestElements() { GCSFilter::ElementSet elements; - for (int i = 0; i < 10000; ++i) { + + // Testing the benchmarks with different number of elements show that a filter + // with at least 100,000 elements results in benchmarks that have the same + // ns/op. This makes it easy to reason about how long (in nanoseconds) a single + // filter element takes to process. + for (int i = 0; i < 100000; ++i) { GCSFilter::Element element(32); element[0] = static_cast<unsigned char>(i); element[1] = static_cast<unsigned char>(i >> 8); elements.insert(std::move(element)); } + return elements; +} + +static void GCSBlockFilterGetHash(benchmark::Bench& bench) +{ + auto elements = GenerateGCSTestElements(); + + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements); + BlockFilter block_filter(BlockFilterType::BASIC, {}, filter.GetEncoded(), /*skip_decode_check=*/false); + + bench.run([&] { + block_filter.GetHash(); + }); +} + +static void GCSFilterConstruct(benchmark::Bench& bench) +{ + auto elements = GenerateGCSTestElements(); + uint64_t siphash_k0 = 0; - bench.batch(elements.size()).unit("elem").run([&] { - GCSFilter filter({siphash_k0, 0, 20, 1 << 20}, elements); + bench.run([&]{ + GCSFilter filter({siphash_k0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements); siphash_k0++; }); } -static void MatchGCSFilter(benchmark::Bench& bench) +static void GCSFilterDecode(benchmark::Bench& bench) { - GCSFilter::ElementSet elements; - for (int i = 0; i < 10000; ++i) { - GCSFilter::Element element(32); - element[0] = static_cast<unsigned char>(i); - element[1] = static_cast<unsigned char>(i >> 8); - elements.insert(std::move(element)); - } - GCSFilter filter({0, 0, 20, 1 << 20}, elements); + auto elements = GenerateGCSTestElements(); - bench.unit("elem").run([&] { - filter.Match(GCSFilter::Element()); + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements); + auto encoded = filter.GetEncoded(); + + bench.run([&] { + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/false); }); } -BENCHMARK(ConstructGCSFilter); -BENCHMARK(MatchGCSFilter); +static void GCSFilterDecodeSkipCheck(benchmark::Bench& bench) +{ + auto elements = GenerateGCSTestElements(); + + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements); + auto encoded = filter.GetEncoded(); + + bench.run([&] { + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/true); + }); +} + +static void GCSFilterMatch(benchmark::Bench& bench) +{ + auto elements = GenerateGCSTestElements(); + + GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements); + + bench.run([&] { + filter.Match(GCSFilter::Element()); + }); +} +BENCHMARK(GCSBlockFilterGetHash); +BENCHMARK(GCSFilterConstruct); +BENCHMARK(GCSFilterDecode); +BENCHMARK(GCSFilterDecodeSkipCheck); +BENCHMARK(GCSFilterMatch); diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index 63a9ba498f..1ad6872143 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -47,7 +47,7 @@ GCSFilter::GCSFilter(const Params& params) : m_params(params), m_N(0), m_F(0), m_encoded{0} {} -GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter) +GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check) : m_params(params), m_encoded(std::move(encoded_filter)) { SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded}; @@ -59,6 +59,8 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi } m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M); + if (skip_decode_check) return; + // Verify that the encoded filter contains exactly N elements. If it has too much or too little // data, a std::ios_base::failure exception will be raised. BitStreamReader<SpanReader> bitreader{stream}; @@ -219,14 +221,14 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block, } BlockFilter::BlockFilter(BlockFilterType filter_type, const uint256& block_hash, - std::vector<unsigned char> filter) + std::vector<unsigned char> filter, bool skip_decode_check) : m_filter_type(filter_type), m_block_hash(block_hash) { GCSFilter::Params params; if (!BuildParams(params)) { throw std::invalid_argument("unknown filter_type"); } - m_filter = GCSFilter(params, std::move(filter)); + m_filter = GCSFilter(params, std::move(filter), skip_decode_check); } BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo) diff --git a/src/blockfilter.h b/src/blockfilter.h index 96cefbf3b2..d6a51e95c2 100644 --- a/src/blockfilter.h +++ b/src/blockfilter.h @@ -59,7 +59,7 @@ public: explicit GCSFilter(const Params& params = Params()); /** Reconstructs an already-created filter from an encoding. */ - GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter); + GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check); /** Builds a new filter from the params and set of elements. */ GCSFilter(const Params& params, const ElementSet& elements); @@ -122,7 +122,7 @@ public: //! Reconstruct a BlockFilter from parts. BlockFilter(BlockFilterType filter_type, const uint256& block_hash, - std::vector<unsigned char> filter); + std::vector<unsigned char> filter, bool skip_decode_check); //! Construct a new BlockFilter of the specified type from a block. BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo); @@ -164,7 +164,7 @@ public: if (!BuildParams(params)) { throw std::ios_base::failure("unknown filter_type"); } - m_filter = GCSFilter(params, std::move(encoded_filter)); + m_filter = GCSFilter(params, std::move(encoded_filter), /*skip_decode_check=*/false); } }; diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index c92b8c7e19..e7fad8eb64 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -5,6 +5,7 @@ #include <map> #include <dbwrapper.h> +#include <hash.h> #include <index/blockfilterindex.h> #include <node/blockstorage.h> #include <util/system.h> @@ -143,18 +144,22 @@ bool BlockFilterIndex::CommitInternal(CDBBatch& batch) return BaseIndex::CommitInternal(batch); } -bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const +bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const { CAutoFile filein(m_filter_fileseq->Open(pos, true), SER_DISK, CLIENT_VERSION); if (filein.IsNull()) { return false; } + // Check that the hash of the encoded_filter matches the one stored in the db. uint256 block_hash; std::vector<uint8_t> encoded_filter; try { filein >> block_hash >> encoded_filter; - filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter)); + uint256 result; + CHash256().Write(encoded_filter).Finalize(result); + if (result != hash) return error("Checksum mismatch in filter decode."); + filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true); } catch (const std::exception& e) { return error("%s: Failed to deserialize block filter from disk: %s", __func__, e.what()); @@ -381,7 +386,7 @@ bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter& return false; } - return ReadFilterFromDisk(entry.pos, filter_out); + return ReadFilterFromDisk(entry.pos, entry.hash, filter_out); } bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out) @@ -425,7 +430,7 @@ bool BlockFilterIndex::LookupFilterRange(int start_height, const CBlockIndex* st filters_out.resize(entries.size()); auto filter_pos_it = filters_out.begin(); for (const auto& entry : entries) { - if (!ReadFilterFromDisk(entry.pos, *filter_pos_it)) { + if (!ReadFilterFromDisk(entry.pos, entry.hash, *filter_pos_it)) { return false; } ++filter_pos_it; diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 6deff59000..fef8b573e8 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -31,7 +31,7 @@ private: FlatFilePos m_next_filter_pos; std::unique_ptr<FlatFileSeq> m_filter_fileseq; - bool ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const; + bool ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const; size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter); Mutex m_cs_headers_cache; diff --git a/src/init.cpp b/src/init.cpp index d844e9b169..eff37e1a83 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -558,6 +558,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted inbound peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index ddfb4bda95..df9e55874f 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -111,6 +111,10 @@ public: //! Get locator for the current chain tip. virtual CBlockLocator getTipLocator() = 0; + //! Return a locator that refers to a block in the active chain. + //! If specified block is not in the active chain, return locator for the latest ancestor that is in the chain. + virtual CBlockLocator getActiveChainLocator(const uint256& block_hash) = 0; + //! Return height of the highest block on chain in common with the locator, //! which will either be the original block used to create the locator, //! or one of its ancestors. diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index f26ac866dc..b3cb0ae387 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -114,7 +114,7 @@ public: std::string* purpose) = 0; //! Get wallet address list. - virtual std::vector<WalletAddress> getAddresses() = 0; + virtual std::vector<WalletAddress> getAddresses() const = 0; //! Get receive requests. virtual std::vector<std::string> getAddressReceiveRequests() = 0; diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index a14abb6628..07953b443b 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -15,6 +15,8 @@ class CBlockPolicyEstimator; static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300}; /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336}; +/** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */ +static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false}; namespace kernel { /** @@ -31,6 +33,7 @@ struct MemPoolOptions { int check_ratio{0}; int64_t max_size_bytes{DEFAULT_MAX_MEMPOOL_SIZE_MB * 1'000'000}; std::chrono::seconds expiry{std::chrono::hours{DEFAULT_MEMPOOL_EXPIRY_HOURS}}; + bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF}; MemPoolLimits limits{}; }; } // namespace kernel diff --git a/src/mempool_args.cpp b/src/mempool_args.cpp index e26cbe0275..77caa127e9 100644 --- a/src/mempool_args.cpp +++ b/src/mempool_args.cpp @@ -33,5 +33,7 @@ void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolOptions& mempool_opt if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours}; + mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf); + ApplyArgsManOptions(argsman, mempool_opts.limits); } diff --git a/src/net.cpp b/src/net.cpp index 7f4e571c8d..c37d90519c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -16,6 +16,7 @@ #include <compat.h> #include <consensus/consensus.h> #include <crypto/sha256.h> +#include <node/eviction.h> #include <fs.h> #include <i2p.h> #include <net_permissions.h> @@ -576,26 +577,6 @@ void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNet } } -std::string ConnectionTypeAsString(ConnectionType conn_type) -{ - switch (conn_type) { - case ConnectionType::INBOUND: - return "inbound"; - case ConnectionType::MANUAL: - return "manual"; - case ConnectionType::FEELER: - return "feeler"; - case ConnectionType::OUTBOUND_FULL_RELAY: - return "outbound-full-relay"; - case ConnectionType::BLOCK_RELAY: - return "block-relay-only"; - case ConnectionType::ADDR_FETCH: - return "addr-fetch"; - } // no default case, so the compiler can warn about missing cases - - assert(false); -} - CService CNode::GetAddrLocal() const { AssertLockNotHeld(m_addr_local_mutex); @@ -877,210 +858,6 @@ size_t CConnman::SocketSendData(CNode& node) const return nSentSize; } -static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - return a.m_min_ping_time > b.m_min_ping_time; -} - -static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - return a.m_connected > b.m_connected; -} - -static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { - return a.nKeyedNetGroup < b.nKeyedNetGroup; -} - -static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - // There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block. - if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time; - if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; - return a.m_connected > b.m_connected; -} - -static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn. - if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time; - if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs; - if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter; - return a.m_connected > b.m_connected; -} - -// Pick out the potential block-relay only peers, and sort them by last block time. -static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) -{ - if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs; - if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time; - if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; - return a.m_connected > b.m_connected; -} - -/** - * Sort eviction candidates by network/localhost and connection uptime. - * Candidates near the beginning are more likely to be evicted, and those - * near the end are more likely to be protected, e.g. less likely to be evicted. - * - First, nodes that are not `is_local` and that do not belong to `network`, - * sorted by increasing uptime (from most recently connected to connected longer). - * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime. - */ -struct CompareNodeNetworkTime { - const bool m_is_local; - const Network m_network; - CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {} - bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const - { - if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local; - if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network; - return a.m_connected > b.m_connected; - }; -}; - -//! Sort an array by the specified comparator, then erase the last K elements where predicate is true. -template <typename T, typename Comparator> -static void EraseLastKElements( - std::vector<T>& elements, Comparator comparator, size_t k, - std::function<bool(const NodeEvictionCandidate&)> predicate = [](const NodeEvictionCandidate& n) { return true; }) -{ - std::sort(elements.begin(), elements.end(), comparator); - size_t eraseSize = std::min(k, elements.size()); - elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end()); -} - -void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& eviction_candidates) -{ - // Protect the half of the remaining nodes which have been connected the longest. - // This replicates the non-eviction implicit behavior, and precludes attacks that start later. - // To favorise the diversity of our peer connections, reserve up to half of these protected - // spots for Tor/onion, localhost, I2P, and CJDNS peers, even if they're not longest uptime - // overall. This helps protect these higher-latency peers that tend to be otherwise - // disadvantaged under our eviction criteria. - const size_t initial_size = eviction_candidates.size(); - const size_t total_protect_size{initial_size / 2}; - - // Disadvantaged networks to protect. In the case of equal counts, earlier array members - // have the first opportunity to recover unused slots from the previous iteration. - struct Net { bool is_local; Network id; size_t count; }; - std::array<Net, 4> networks{ - {{false, NET_CJDNS, 0}, {false, NET_I2P, 0}, {/*localhost=*/true, NET_MAX, 0}, {false, NET_ONION, 0}}}; - - // Count and store the number of eviction candidates per network. - for (Net& n : networks) { - n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(), - [&n](const NodeEvictionCandidate& c) { - return n.is_local ? c.m_is_local : c.m_network == n.id; - }); - } - // Sort `networks` by ascending candidate count, to give networks having fewer candidates - // the first opportunity to recover unused protected slots from the previous iteration. - std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; }); - - // Protect up to 25% of the eviction candidates by disadvantaged network. - const size_t max_protect_by_network{total_protect_size / 2}; - size_t num_protected{0}; - - while (num_protected < max_protect_by_network) { - // Count the number of disadvantaged networks from which we have peers to protect. - auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; }); - if (num_networks == 0) { - break; - } - const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; - const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast<size_t>(1))}; - // Early exit flag if there are no remaining candidates by disadvantaged network. - bool protected_at_least_one{false}; - - for (Net& n : networks) { - if (n.count == 0) continue; - const size_t before = eviction_candidates.size(); - EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id), - protect_per_network, [&n](const NodeEvictionCandidate& c) { - return n.is_local ? c.m_is_local : c.m_network == n.id; - }); - const size_t after = eviction_candidates.size(); - if (before > after) { - protected_at_least_one = true; - const size_t delta{before - after}; - num_protected += delta; - if (num_protected >= max_protect_by_network) { - break; - } - n.count -= delta; - } - } - if (!protected_at_least_one) { - break; - } - } - - // Calculate how many we removed, and update our total number of peers that - // we want to protect based on uptime accordingly. - assert(num_protected == initial_size - eviction_candidates.size()); - const size_t remaining_to_protect{total_protect_size - num_protected}; - EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect); -} - -[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates) -{ - // Protect connections with certain characteristics - - // Deterministically select 4 peers to protect by netgroup. - // An attacker cannot predict which netgroups will be protected - EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4); - // Protect the 8 nodes with the lowest minimum ping time. - // An attacker cannot manipulate this metric without physically moving nodes closer to the target. - EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); - // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool. - // An attacker cannot manipulate this metric without performing useful work. - EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); - // Protect up to 8 non-tx-relay peers that have sent us novel blocks. - EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8, - [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; }); - - // Protect 4 nodes that most recently sent us novel blocks. - // An attacker cannot manipulate this metric without performing useful work. - EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); - - // Protect some of the remaining eviction candidates by ratios of desirable - // or disadvantaged characteristics. - ProtectEvictionCandidatesByRatio(vEvictionCandidates); - - if (vEvictionCandidates.empty()) return std::nullopt; - - // If any remaining peers are preferred for eviction consider only them. - // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks) - // then we probably don't want to evict it no matter what. - if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) { - vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(), - [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end()); - } - - // Identify the network group with the most connections and youngest member. - // (vEvictionCandidates is already sorted by reverse connect time) - uint64_t naMostConnections; - unsigned int nMostConnections = 0; - std::chrono::seconds nMostConnectionsTime{0}; - std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes; - for (const NodeEvictionCandidate &node : vEvictionCandidates) { - std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup]; - group.push_back(node); - const auto grouptime{group[0].m_connected}; - - if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) { - nMostConnections = group.size(); - nMostConnectionsTime = grouptime; - naMostConnections = node.nKeyedNetGroup; - } - } - - // Reduce to the network group with the most connections - vEvictionCandidates = std::move(mapNetGroupNodes[naMostConnections]); - - // Disconnect from the network group with the most connections - return vEvictionCandidates.front().id; -} - /** Try to find a connection to evict when the node is full. * Extreme care must be taken to avoid opening the node to attacker * triggered network partitioning. @@ -1096,10 +873,6 @@ bool CConnman::AttemptToEvictConnection() LOCK(m_nodes_mutex); for (const CNode* node : m_nodes) { - if (node->HasPermission(NetPermissionFlags::NoBan)) - continue; - if (!node->IsInboundConn()) - continue; if (node->fDisconnect) continue; NodeEvictionCandidate candidate{ @@ -1115,6 +888,8 @@ bool CConnman::AttemptToEvictConnection() Desig(prefer_evict) node->m_prefer_evict, Desig(m_is_local) node->addr.IsLocal(), Desig(m_network) node->ConnectedThroughNetwork(), + Desig(m_noban) node->HasPermission(NetPermissionFlags::NoBan), + Desig(m_conn_type) node->m_conn_type, }; vEvictionCandidates.push_back(candidate); } @@ -9,6 +9,7 @@ #include <chainparams.h> #include <common/bloom.h> #include <compat.h> +#include <node/connection_types.h> #include <consensus/amount.h> #include <crypto/siphash.h> #include <hash.h> @@ -121,78 +122,6 @@ struct CSerializedNetMsg { std::string m_type; }; -/** Different types of connections to a peer. This enum encapsulates the - * information we have available at the time of opening or accepting the - * connection. Aside from INBOUND, all types are initiated by us. - * - * If adding or removing types, please update CONNECTION_TYPE_DOC in - * src/rpc/net.cpp and src/qt/rpcconsole.cpp, as well as the descriptions in - * src/qt/guiutil.cpp and src/bitcoin-cli.cpp::NetinfoRequestHandler. */ -enum class ConnectionType { - /** - * Inbound connections are those initiated by a peer. This is the only - * property we know at the time of connection, until P2P messages are - * exchanged. - */ - INBOUND, - - /** - * These are the default connections that we use to connect with the - * network. There is no restriction on what is relayed; by default we relay - * blocks, addresses & transactions. We automatically attempt to open - * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan. - */ - OUTBOUND_FULL_RELAY, - - - /** - * We open manual connections to addresses that users explicitly requested - * via the addnode RPC or the -addnode/-connect configuration options. Even if a - * manual connection is misbehaving, we do not automatically disconnect or - * add it to our discouragement filter. - */ - MANUAL, - - /** - * Feeler connections are short-lived connections made to check that a node - * is alive. They can be useful for: - * - test-before-evict: if one of the peers is considered for eviction from - * our AddrMan because another peer is mapped to the same slot in the tried table, - * evict only if this longer-known peer is offline. - * - move node addresses from New to Tried table, so that we have more - * connectable addresses in our AddrMan. - * Note that in the literature ("Eclipse Attacks on Bitcoin’s Peer-to-Peer Network") - * only the latter feature is referred to as "feeler connections", - * although in our codebase feeler connections encompass test-before-evict as well. - * We make these connections approximately every FEELER_INTERVAL: - * first we resolve previously found collisions if they exist (test-before-evict), - * otherwise we connect to a node from the new table. - */ - FEELER, - - /** - * We use block-relay-only connections to help prevent against partition - * attacks. By not relaying transactions or addresses, these connections - * are harder to detect by a third party, thus helping obfuscate the - * network topology. We automatically attempt to open - * MAX_BLOCK_RELAY_ONLY_ANCHORS using addresses from our anchors.dat. Then - * addresses from our AddrMan if MAX_BLOCK_RELAY_ONLY_CONNECTIONS - * isn't reached yet. - */ - BLOCK_RELAY, - - /** - * AddrFetch connections are short lived connections used to solicit - * addresses from peers. These are initiated to addresses submitted via the - * -seednode command line argument, or under certain conditions when the - * AddrMan is empty. - */ - ADDR_FETCH, -}; - -/** Convert ConnectionType enum to a string value */ -std::string ConnectionTypeAsString(ConnectionType conn_type); - /** * Look up IP addresses from all interfaces on the machine and add them to the * list of local addresses to self-advertise. @@ -1247,54 +1176,4 @@ extern std::function<void(const CAddress& addr, bool is_incoming)> CaptureMessage; -struct NodeEvictionCandidate -{ - NodeId id; - std::chrono::seconds m_connected; - std::chrono::microseconds m_min_ping_time; - std::chrono::seconds m_last_block_time; - std::chrono::seconds m_last_tx_time; - bool fRelevantServices; - bool m_relay_txs; - bool fBloomFilter; - uint64_t nKeyedNetGroup; - bool prefer_evict; - bool m_is_local; - Network m_network; -}; - -/** - * Select an inbound peer to evict after filtering out (protecting) peers having - * distinct, difficult-to-forge characteristics. The protection logic picks out - * fixed numbers of desirable peers per various criteria, followed by (mostly) - * ratios of desirable or disadvantaged peers. If any eviction candidates - * remain, the selection logic chooses a peer to evict. - */ -[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates); - -/** Protect desirable or disadvantaged inbound peers from eviction by ratio. - * - * This function protects half of the peers which have been connected the - * longest, to replicate the non-eviction implicit behavior and preclude attacks - * that start later. - * - * Half of these protected spots (1/4 of the total) are reserved for the - * following categories of peers, sorted by longest uptime, even if they're not - * longest uptime overall: - * - * - onion peers connected via our tor control service - * - * - localhost peers, as manually configured hidden services not using - * `-bind=addr[:port]=onion` will not be detected as inbound onion connections - * - * - I2P peers - * - * - CJDNS peers - * - * This helps protect these privacy network peers, which tend to be otherwise - * disadvantaged under our eviction criteria for their higher min ping times - * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections. - */ -void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates); - #endif // BITCOIN_NET_H diff --git a/src/node/connection_types.cpp b/src/node/connection_types.cpp new file mode 100644 index 0000000000..904f4371aa --- /dev/null +++ b/src/node/connection_types.cpp @@ -0,0 +1,26 @@ +// Copyright (c) 2022 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 <node/connection_types.h> +#include <cassert> + +std::string ConnectionTypeAsString(ConnectionType conn_type) +{ + switch (conn_type) { + case ConnectionType::INBOUND: + return "inbound"; + case ConnectionType::MANUAL: + return "manual"; + case ConnectionType::FEELER: + return "feeler"; + case ConnectionType::OUTBOUND_FULL_RELAY: + return "outbound-full-relay"; + case ConnectionType::BLOCK_RELAY: + return "block-relay-only"; + case ConnectionType::ADDR_FETCH: + return "addr-fetch"; + } // no default case, so the compiler can warn about missing cases + + assert(false); +} diff --git a/src/node/connection_types.h b/src/node/connection_types.h new file mode 100644 index 0000000000..5e1abcace6 --- /dev/null +++ b/src/node/connection_types.h @@ -0,0 +1,82 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_CONNECTION_TYPES_H +#define BITCOIN_NODE_CONNECTION_TYPES_H + +#include <string> + +/** Different types of connections to a peer. This enum encapsulates the + * information we have available at the time of opening or accepting the + * connection. Aside from INBOUND, all types are initiated by us. + * + * If adding or removing types, please update CONNECTION_TYPE_DOC in + * src/rpc/net.cpp and src/qt/rpcconsole.cpp, as well as the descriptions in + * src/qt/guiutil.cpp and src/bitcoin-cli.cpp::NetinfoRequestHandler. */ +enum class ConnectionType { + /** + * Inbound connections are those initiated by a peer. This is the only + * property we know at the time of connection, until P2P messages are + * exchanged. + */ + INBOUND, + + /** + * These are the default connections that we use to connect with the + * network. There is no restriction on what is relayed; by default we relay + * blocks, addresses & transactions. We automatically attempt to open + * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan. + */ + OUTBOUND_FULL_RELAY, + + + /** + * We open manual connections to addresses that users explicitly requested + * via the addnode RPC or the -addnode/-connect configuration options. Even if a + * manual connection is misbehaving, we do not automatically disconnect or + * add it to our discouragement filter. + */ + MANUAL, + + /** + * Feeler connections are short-lived connections made to check that a node + * is alive. They can be useful for: + * - test-before-evict: if one of the peers is considered for eviction from + * our AddrMan because another peer is mapped to the same slot in the tried table, + * evict only if this longer-known peer is offline. + * - move node addresses from New to Tried table, so that we have more + * connectable addresses in our AddrMan. + * Note that in the literature ("Eclipse Attacks on Bitcoin’s Peer-to-Peer Network") + * only the latter feature is referred to as "feeler connections", + * although in our codebase feeler connections encompass test-before-evict as well. + * We make these connections approximately every FEELER_INTERVAL: + * first we resolve previously found collisions if they exist (test-before-evict), + * otherwise we connect to a node from the new table. + */ + FEELER, + + /** + * We use block-relay-only connections to help prevent against partition + * attacks. By not relaying transactions or addresses, these connections + * are harder to detect by a third party, thus helping obfuscate the + * network topology. We automatically attempt to open + * MAX_BLOCK_RELAY_ONLY_ANCHORS using addresses from our anchors.dat. Then + * addresses from our AddrMan if MAX_BLOCK_RELAY_ONLY_CONNECTIONS + * isn't reached yet. + */ + BLOCK_RELAY, + + /** + * AddrFetch connections are short lived connections used to solicit + * addresses from peers. These are initiated to addresses submitted via the + * -seednode command line argument, or under certain conditions when the + * AddrMan is empty. + */ + ADDR_FETCH, +}; + +/** Convert ConnectionType enum to a string value */ +std::string ConnectionTypeAsString(ConnectionType conn_type); + +#endif // BITCOIN_NODE_CONNECTION_TYPES_H diff --git a/src/node/eviction.cpp b/src/node/eviction.cpp new file mode 100644 index 0000000000..33406931d4 --- /dev/null +++ b/src/node/eviction.cpp @@ -0,0 +1,240 @@ +// Copyright (c) 2022 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 <node/eviction.h> + +#include <algorithm> +#include <array> +#include <chrono> +#include <cstdint> +#include <functional> +#include <map> +#include <vector> + + +static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + return a.m_min_ping_time > b.m_min_ping_time; +} + +static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + return a.m_connected > b.m_connected; +} + +static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { + return a.nKeyedNetGroup < b.nKeyedNetGroup; +} + +static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + // There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block. + if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time; + if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; + return a.m_connected > b.m_connected; +} + +static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn. + if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time; + if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs; + if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter; + return a.m_connected > b.m_connected; +} + +// Pick out the potential block-relay only peers, and sort them by last block time. +static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs; + if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time; + if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; + return a.m_connected > b.m_connected; +} + +/** + * Sort eviction candidates by network/localhost and connection uptime. + * Candidates near the beginning are more likely to be evicted, and those + * near the end are more likely to be protected, e.g. less likely to be evicted. + * - First, nodes that are not `is_local` and that do not belong to `network`, + * sorted by increasing uptime (from most recently connected to connected longer). + * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime. + */ +struct CompareNodeNetworkTime { + const bool m_is_local; + const Network m_network; + CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {} + bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const + { + if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local; + if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network; + return a.m_connected > b.m_connected; + }; +}; + +//! Sort an array by the specified comparator, then erase the last K elements where predicate is true. +template <typename T, typename Comparator> +static void EraseLastKElements( + std::vector<T>& elements, Comparator comparator, size_t k, + std::function<bool(const NodeEvictionCandidate&)> predicate = [](const NodeEvictionCandidate& n) { return true; }) +{ + std::sort(elements.begin(), elements.end(), comparator); + size_t eraseSize = std::min(k, elements.size()); + elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end()); +} + +void ProtectNoBanConnections(std::vector<NodeEvictionCandidate>& eviction_candidates) +{ + eviction_candidates.erase(std::remove_if(eviction_candidates.begin(), eviction_candidates.end(), + [](NodeEvictionCandidate const& n) { + return n.m_noban; + }), + eviction_candidates.end()); +} + +void ProtectOutboundConnections(std::vector<NodeEvictionCandidate>& eviction_candidates) +{ + eviction_candidates.erase(std::remove_if(eviction_candidates.begin(), eviction_candidates.end(), + [](NodeEvictionCandidate const& n) { + return n.m_conn_type != ConnectionType::INBOUND; + }), + eviction_candidates.end()); +} + +void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& eviction_candidates) +{ + // Protect the half of the remaining nodes which have been connected the longest. + // This replicates the non-eviction implicit behavior, and precludes attacks that start later. + // To favorise the diversity of our peer connections, reserve up to half of these protected + // spots for Tor/onion, localhost, I2P, and CJDNS peers, even if they're not longest uptime + // overall. This helps protect these higher-latency peers that tend to be otherwise + // disadvantaged under our eviction criteria. + const size_t initial_size = eviction_candidates.size(); + const size_t total_protect_size{initial_size / 2}; + + // Disadvantaged networks to protect. In the case of equal counts, earlier array members + // have the first opportunity to recover unused slots from the previous iteration. + struct Net { bool is_local; Network id; size_t count; }; + std::array<Net, 4> networks{ + {{false, NET_CJDNS, 0}, {false, NET_I2P, 0}, {/*localhost=*/true, NET_MAX, 0}, {false, NET_ONION, 0}}}; + + // Count and store the number of eviction candidates per network. + for (Net& n : networks) { + n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(), + [&n](const NodeEvictionCandidate& c) { + return n.is_local ? c.m_is_local : c.m_network == n.id; + }); + } + // Sort `networks` by ascending candidate count, to give networks having fewer candidates + // the first opportunity to recover unused protected slots from the previous iteration. + std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; }); + + // Protect up to 25% of the eviction candidates by disadvantaged network. + const size_t max_protect_by_network{total_protect_size / 2}; + size_t num_protected{0}; + + while (num_protected < max_protect_by_network) { + // Count the number of disadvantaged networks from which we have peers to protect. + auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; }); + if (num_networks == 0) { + break; + } + const size_t disadvantaged_to_protect{max_protect_by_network - num_protected}; + const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast<size_t>(1))}; + // Early exit flag if there are no remaining candidates by disadvantaged network. + bool protected_at_least_one{false}; + + for (Net& n : networks) { + if (n.count == 0) continue; + const size_t before = eviction_candidates.size(); + EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id), + protect_per_network, [&n](const NodeEvictionCandidate& c) { + return n.is_local ? c.m_is_local : c.m_network == n.id; + }); + const size_t after = eviction_candidates.size(); + if (before > after) { + protected_at_least_one = true; + const size_t delta{before - after}; + num_protected += delta; + if (num_protected >= max_protect_by_network) { + break; + } + n.count -= delta; + } + } + if (!protected_at_least_one) { + break; + } + } + + // Calculate how many we removed, and update our total number of peers that + // we want to protect based on uptime accordingly. + assert(num_protected == initial_size - eviction_candidates.size()); + const size_t remaining_to_protect{total_protect_size - num_protected}; + EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect); +} + +[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates) +{ + // Protect connections with certain characteristics + + ProtectNoBanConnections(vEvictionCandidates); + + ProtectOutboundConnections(vEvictionCandidates); + + // Deterministically select 4 peers to protect by netgroup. + // An attacker cannot predict which netgroups will be protected + EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4); + // Protect the 8 nodes with the lowest minimum ping time. + // An attacker cannot manipulate this metric without physically moving nodes closer to the target. + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); + // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool. + // An attacker cannot manipulate this metric without performing useful work. + EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); + // Protect up to 8 non-tx-relay peers that have sent us novel blocks. + EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8, + [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; }); + + // Protect 4 nodes that most recently sent us novel blocks. + // An attacker cannot manipulate this metric without performing useful work. + EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); + + // Protect some of the remaining eviction candidates by ratios of desirable + // or disadvantaged characteristics. + ProtectEvictionCandidatesByRatio(vEvictionCandidates); + + if (vEvictionCandidates.empty()) return std::nullopt; + + // If any remaining peers are preferred for eviction consider only them. + // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks) + // then we probably don't want to evict it no matter what. + if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) { + vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(), + [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end()); + } + + // Identify the network group with the most connections and youngest member. + // (vEvictionCandidates is already sorted by reverse connect time) + uint64_t naMostConnections; + unsigned int nMostConnections = 0; + std::chrono::seconds nMostConnectionsTime{0}; + std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes; + for (const NodeEvictionCandidate &node : vEvictionCandidates) { + std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup]; + group.push_back(node); + const auto grouptime{group[0].m_connected}; + + if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) { + nMostConnections = group.size(); + nMostConnectionsTime = grouptime; + naMostConnections = node.nKeyedNetGroup; + } + } + + // Reduce to the network group with the most connections + vEvictionCandidates = std::move(mapNetGroupNodes[naMostConnections]); + + // Disconnect from the network group with the most connections + return vEvictionCandidates.front().id; +} diff --git a/src/node/eviction.h b/src/node/eviction.h new file mode 100644 index 0000000000..1bb32e5327 --- /dev/null +++ b/src/node/eviction.h @@ -0,0 +1,69 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_EVICTION_H +#define BITCOIN_NODE_EVICTION_H + +#include <node/connection_types.h> +#include <net_permissions.h> + +#include <chrono> +#include <cstdint> +#include <optional> +#include <vector> + +typedef int64_t NodeId; + +struct NodeEvictionCandidate { + NodeId id; + std::chrono::seconds m_connected; + std::chrono::microseconds m_min_ping_time; + std::chrono::seconds m_last_block_time; + std::chrono::seconds m_last_tx_time; + bool fRelevantServices; + bool m_relay_txs; + bool fBloomFilter; + uint64_t nKeyedNetGroup; + bool prefer_evict; + bool m_is_local; + Network m_network; + bool m_noban; + ConnectionType m_conn_type; +}; + +/** + * Select an inbound peer to evict after filtering out (protecting) peers having + * distinct, difficult-to-forge characteristics. The protection logic picks out + * fixed numbers of desirable peers per various criteria, followed by (mostly) + * ratios of desirable or disadvantaged peers. If any eviction candidates + * remain, the selection logic chooses a peer to evict. + */ +[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates); + +/** Protect desirable or disadvantaged inbound peers from eviction by ratio. + * + * This function protects half of the peers which have been connected the + * longest, to replicate the non-eviction implicit behavior and preclude attacks + * that start later. + * + * Half of these protected spots (1/4 of the total) are reserved for the + * following categories of peers, sorted by longest uptime, even if they're not + * longest uptime overall: + * + * - onion peers connected via our tor control service + * + * - localhost peers, as manually configured hidden services not using + * `-bind=addr[:port]=onion` will not be detected as inbound onion connections + * + * - I2P peers + * + * - CJDNS peers + * + * This helps protect these privacy network peers, which tend to be otherwise + * disadvantaged under our eviction criteria for their higher min ping times + * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections. + */ +void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates); + +#endif // BITCOIN_NODE_EVICTION_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 3c085ae6fb..93aa9e526b 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -524,20 +524,27 @@ public: } bool haveBlockOnDisk(int height) override { - LOCK(cs_main); + LOCK(::cs_main); const CChain& active = Assert(m_node.chainman)->ActiveChain(); CBlockIndex* block = active[height]; return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0; } CBlockLocator getTipLocator() override { - LOCK(cs_main); + LOCK(::cs_main); const CChain& active = Assert(m_node.chainman)->ActiveChain(); return active.GetLocator(); } + CBlockLocator getActiveChainLocator(const uint256& block_hash) override + { + LOCK(::cs_main); + const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash); + if (!index) return {}; + return chainman().ActiveChain().GetLocator(index); + } std::optional<int> findLocatorFork(const CBlockLocator& locator) override { - LOCK(cs_main); + LOCK(::cs_main); const CChainState& active = Assert(m_node.chainman)->ActiveChainstate(); if (const CBlockIndex* fork = active.FindForkInGlobalIndex(locator)) { return fork->nHeight; @@ -593,7 +600,7 @@ public: void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); } double guessVerificationProgress(const uint256& block_hash) override { - LOCK(cs_main); + LOCK(::cs_main); return GuessVerificationProgress(chainman().GetParams().TxData(), chainman().m_blockman.LookupBlockIndex(block_hash)); } bool hasBlocks(const uint256& block_hash, int min_height, std::optional<int> max_height) override @@ -693,7 +700,7 @@ public: CFeeRate relayDustFee() override { return ::dustRelayFee; } bool havePruned() override { - LOCK(cs_main); + LOCK(::cs_main); return m_node.chainman->m_blockman.m_have_pruned; } bool isReadyToBroadcast() override { return !node::fImporting && !node::fReindex && !isInitialBlockDownload(); } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index d65fc58865..35e32f515b 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -47,6 +47,7 @@ #include <QCursor> #include <QDateTime> #include <QDragEnterEvent> +#include <QInputDialog> #include <QKeySequence> #include <QListWidget> #include <QMenu> @@ -348,6 +349,12 @@ void BitcoinGUI::createActions() m_create_wallet_action->setEnabled(false); m_create_wallet_action->setStatusTip(tr("Create a new wallet")); + //: Name of the menu item that restores wallet from a backup file. + m_restore_wallet_action = new QAction(tr("Restore Wallet…"), this); + m_restore_wallet_action->setEnabled(false); + //: Status tip for Restore Wallet menu item + m_restore_wallet_action->setStatusTip(tr("Restore a wallet from a backup file")); + m_close_all_wallets_action = new QAction(tr("Close All Wallets…"), this); m_close_all_wallets_action->setStatusTip(tr("Close all wallets")); @@ -412,6 +419,27 @@ void BitcoinGUI::createActions() action->setEnabled(false); } }); + connect(m_restore_wallet_action, &QAction::triggered, [this] { + //: Name of the wallet data file format. + QString name_data_file = tr("Wallet Data"); + + //: The title for Restore Wallet File Windows + QString title_windows = tr("Load Wallet Backup"); + + QString backup_file = GUIUtil::getOpenFileName(this, title_windows, QString(), name_data_file + QLatin1String(" (*.dat)"), nullptr); + if (backup_file.isEmpty()) return; + + bool wallet_name_ok; + //: Title of the Restore Wallet input dialog (where the wallet name is entered) + QString wallet_name = QInputDialog::getText(this, tr("Restore Name"), tr("Wallet Name:"), QLineEdit::Normal, "", &wallet_name_ok); + if (!wallet_name_ok || wallet_name.isEmpty()) return; + + auto activity = new RestoreWalletActivity(m_wallet_controller, this); + connect(activity, &RestoreWalletActivity::restored, this, &BitcoinGUI::setCurrentWallet, Qt::QueuedConnection); + + auto backup_file_path = fs::PathFromString(backup_file.toStdString()); + activity->restore(backup_file_path, wallet_name.toStdString()); + }); connect(m_close_wallet_action, &QAction::triggered, [this] { m_wallet_controller->closeWallet(walletFrame->currentWalletModel(), this); }); @@ -450,8 +478,10 @@ void BitcoinGUI::createMenuBar() file->addAction(m_close_wallet_action); file->addAction(m_close_all_wallets_action); file->addSeparator(); - file->addAction(openAction); file->addAction(backupWalletAction); + file->addAction(m_restore_wallet_action); + file->addSeparator(); + file->addAction(openAction); file->addAction(signMessageAction); file->addAction(verifyMessageAction); file->addAction(m_load_psbt_action); @@ -642,6 +672,7 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) m_create_wallet_action->setEnabled(true); m_open_wallet_action->setEnabled(true); m_open_wallet_action->setMenu(m_open_wallet_menu); + m_restore_wallet_action->setEnabled(true); GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 6272d5d947..b2e13245e1 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -157,6 +157,7 @@ private: QAction* m_create_wallet_action{nullptr}; QAction* m_open_wallet_action{nullptr}; QMenu* m_open_wallet_menu{nullptr}; + QAction* m_restore_wallet_action{nullptr}; QAction* m_close_wallet_action{nullptr}; QAction* m_close_all_wallets_action{nullptr}; QAction* m_wallet_selector_label_action = nullptr; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 7671bfc739..b8ae62ecab 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -173,7 +173,7 @@ void TestGUI(interfaces::Node& node) { WalletRescanReserver reserver(*wallet); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, 0 /* block height */, {} /* max height */, reserver, true /* fUpdate */); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/false); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); QCOMPARE(result.last_scanned_block, node.context()->chainman->ActiveChain().Tip()->GetBlockHash()); QVERIFY(result.last_failed_block.IsNull()); diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index d27ddf1aba..11140c5da9 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -373,3 +373,46 @@ void LoadWalletsActivity::load() QTimer::singleShot(0, this, [this] { Q_EMIT finished(); }); }); } + +RestoreWalletActivity::RestoreWalletActivity(WalletController* wallet_controller, QWidget* parent_widget) + : WalletControllerActivity(wallet_controller, parent_widget) +{ +} + +void RestoreWalletActivity::restore(const fs::path& backup_file, const std::string& wallet_name) +{ + QString name = QString::fromStdString(wallet_name); + + showProgressDialog( + //: Title of progress window which is displayed when wallets are being restored. + tr("Restore Wallet"), + /*: Descriptive text of the restore wallets progress window which indicates to + the user that wallets are currently being restored.*/ + tr("Restoring Wallet <b>%1</b>…").arg(name.toHtmlEscaped())); + + QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] { + std::unique_ptr<interfaces::Wallet> wallet = node().walletLoader().restoreWallet(backup_file, wallet_name, m_error_message, m_warning_message); + + if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); + + QTimer::singleShot(0, this, &RestoreWalletActivity::finish); + }); +} + +void RestoreWalletActivity::finish() +{ + if (!m_error_message.empty()) { + //: Title of message box which is displayed when the wallet could not be restored. + QMessageBox::critical(m_parent_widget, tr("Restore wallet failed"), QString::fromStdString(m_error_message.translated)); + } else if (!m_warning_message.empty()) { + //: Title of message box which is displayed when the wallet is restored with some warning. + QMessageBox::warning(m_parent_widget, tr("Restore wallet warning"), QString::fromStdString(Join(m_warning_message, Untranslated("\n")).translated)); + } else { + //: Title of message box which is displayed when the wallet is successfully restored. + QMessageBox::information(m_parent_widget, tr("Restore wallet message"), QString::fromStdString(Untranslated("Wallet restored successfully \n").translated)); + } + + if (m_wallet_model) Q_EMIT restored(m_wallet_model); + + Q_EMIT finished(); +} diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index 24f85c258c..fcd65756c6 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -33,6 +33,10 @@ class Node; class Wallet; } // namespace interfaces +namespace fs { +class path; +} + class AskPassphraseDialog; class CreateWalletActivity; class CreateWalletDialog; @@ -155,4 +159,20 @@ public: void load(); }; +class RestoreWalletActivity : public WalletControllerActivity +{ + Q_OBJECT + +public: + RestoreWalletActivity(WalletController* wallet_controller, QWidget* parent_widget); + + void restore(const fs::path& backup_file, const std::string& wallet_name); + +Q_SIGNALS: + void restored(WalletModel* wallet_model); + +private: + void finish(); +}; + #endif // BITCOIN_QT_WALLETCONTROLLER_H diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index fbb40ab861..84d43e7818 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -663,6 +663,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) ret.pushKV("minrelaytxfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())); ret.pushKV("incrementalrelayfee", ValueFromAmount(::incrementalRelayFee.GetFeePerK())); ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()}); + ret.pushKV("fullrbf", pool.m_full_rbf); return ret; } @@ -684,6 +685,7 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::STR_AMOUNT, "minrelaytxfee", "Current minimum relay fee for transactions"}, {RPCResult::Type::NUM, "incrementalrelayfee", "minimum fee rate increment for mempool limiting or BIP 125 replacement in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::NUM, "unbroadcastcount", "Current number of transactions that haven't passed initial broadcast yet"}, + {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection"}, }}, RPCExamples{ HelpExampleCli("getmempoolinfo", "") diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp index 6a363f00f7..e27b254580 100644 --- a/src/test/fuzz/node_eviction.cpp +++ b/src/test/fuzz/node_eviction.cpp @@ -32,6 +32,8 @@ FUZZ_TARGET(node_eviction) /*prefer_evict=*/fuzzed_data_provider.ConsumeBool(), /*m_is_local=*/fuzzed_data_provider.ConsumeBool(), /*m_network=*/fuzzed_data_provider.PickValueInArray(ALL_NETWORKS), + /*m_noban=*/fuzzed_data_provider.ConsumeBool(), + /*m_conn_type=*/fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES), }); } // Make a copy since eviction_candidates may be in some valid but otherwise diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 62b770753a..071193b550 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -5,6 +5,7 @@ #include <test/util/net.h> #include <chainparams.h> +#include <node/eviction.h> #include <net.h> #include <span.h> @@ -58,6 +59,8 @@ std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candida /*prefer_evict=*/random_context.randbool(), /*m_is_local=*/random_context.randbool(), /*m_network=*/ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())], + /*m_noban=*/false, + /*m_conn_type=*/ConnectionType::INBOUND, }); } return candidates; diff --git a/src/test/util/net.h b/src/test/util/net.h index c5dbaeca3e..34ab9958c6 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -6,6 +6,7 @@ #define BITCOIN_TEST_UTIL_NET_H #include <compat.h> +#include <node/eviction.h> #include <netaddress.h> #include <net.h> #include <util/sock.h> diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 69ae9fed99..aeaa10034e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -458,6 +458,7 @@ CTxMemPool::CTxMemPool(const Options& opts) minerPolicyEstimator{opts.estimator}, m_max_size_bytes{opts.max_size_bytes}, m_expiry{opts.expiry}, + m_full_rbf{opts.full_rbf}, m_limits{opts.limits} { _clear(); //lock free clear diff --git a/src/txmempool.h b/src/txmempool.h index f44e78fde5..6e37f59f2e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -568,6 +568,7 @@ public: const int64_t m_max_size_bytes; const std::chrono::seconds m_expiry; + const bool m_full_rbf; using Limits = kernel::MemPoolLimits; diff --git a/src/validation.cpp b/src/validation.cpp index 6b21d33871..4c694a2c21 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -740,7 +740,10 @@ 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. - if (!SignalsOptInRBF(*ptxConflicting)) { + // + // If replaceability signaling is ignored due to node setting, + // replacement is always allowed. + if (!m_pool.m_full_rbf && !SignalsOptInRBF(*ptxConflicting)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index f54b2c83d2..327276f2ed 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -191,29 +191,27 @@ public: std::string* purpose) override { LOCK(m_wallet->cs_wallet); - auto it = m_wallet->m_address_book.find(dest); - if (it == m_wallet->m_address_book.end() || it->second.IsChange()) { - return false; - } + const auto& entry = m_wallet->FindAddressBookEntry(dest, /*allow_change=*/false); + if (!entry) return false; // addr not found if (name) { - *name = it->second.GetLabel(); + *name = entry->GetLabel(); } if (is_mine) { *is_mine = m_wallet->IsMine(dest); } if (purpose) { - *purpose = it->second.purpose; + *purpose = entry->purpose; } return true; } - std::vector<WalletAddress> getAddresses() override + std::vector<WalletAddress> getAddresses() const override { LOCK(m_wallet->cs_wallet); std::vector<WalletAddress> result; - for (const auto& item : m_wallet->m_address_book) { - if (item.second.IsChange()) continue; - result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.GetLabel(), item.second.purpose); - } + m_wallet->ForEachAddrBookEntry([&](const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change) EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet) { + if (is_change) return; + result.emplace_back(dest, m_wallet->IsMine(dest), label, purpose); + }); return result; } std::vector<std::string> getAddressReceiveRequests() override { diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index f25ad59528..da4cc44ee6 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -637,17 +637,6 @@ RPCHelpMan getaddressinfo() }; } -/** Convert CAddressBookData to JSON record. */ -static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool verbose) -{ - UniValue ret(UniValue::VOBJ); - if (verbose) { - ret.pushKV("name", data.GetLabel()); - } - ret.pushKV("purpose", data.purpose); - return ret; -} - RPCHelpMan getaddressesbylabel() { return RPCHelpMan{"getaddressesbylabel", @@ -680,10 +669,10 @@ RPCHelpMan getaddressesbylabel() // Find all addresses that have the given label UniValue ret(UniValue::VOBJ); std::set<std::string> addresses; - for (const std::pair<const CTxDestination, CAddressBookData>& item : pwallet->m_address_book) { - if (item.second.IsChange()) continue; - if (item.second.GetLabel() == label) { - std::string address = EncodeDestination(item.first); + pwallet->ForEachAddrBookEntry([&](const CTxDestination& _dest, const std::string& _label, const std::string& _purpose, bool _is_change) { + if (_is_change) return; + if (_label == label) { + std::string address = EncodeDestination(_dest); // CWallet::m_address_book is not expected to contain duplicate // address strings, but build a separate set as a precaution just in // case it does. @@ -693,9 +682,11 @@ RPCHelpMan getaddressesbylabel() // and since duplicate addresses are unexpected (checked with // std::set in O(log(N))), UniValue::__pushKV is used instead, // which currently is O(1). - ret.__pushKV(address, AddressBookDataToJSON(item.second, false)); + UniValue value(UniValue::VOBJ); + value.pushKV("purpose", _purpose); + ret.__pushKV(address, value); } - } + }); if (ret.empty()) { throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, std::string("No addresses with label " + label)); @@ -742,13 +733,7 @@ RPCHelpMan listlabels() } // Add to a set to sort by label name, then insert into Univalue array - std::set<std::string> label_set; - for (const std::pair<const CTxDestination, CAddressBookData>& entry : pwallet->m_address_book) { - if (entry.second.IsChange()) continue; - if (purpose.empty() || entry.second.purpose == purpose) { - label_set.insert(entry.second.GetLabel()); - } - } + std::set<std::string> label_set = pwallet->ListAddrBookLabels(purpose); UniValue ret(UniValue::VARR); for (const std::string& name : label_set) { diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index ad59cc94ff..a9fff95882 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -18,10 +18,10 @@ namespace wallet { static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { - std::set<CTxDestination> addresses; + std::vector<CTxDestination> addresses; if (by_label) { // Get the set of addresses assigned to label - addresses = wallet.GetLabelAddresses(LabelFromValue(params[0])); + addresses = wallet.ListAddrBookAddresses(CWallet::AddrBookFilter{LabelFromValue(params[0])}); if (addresses.empty()) throw JSONRPCError(RPC_WALLET_ERROR, "Label not found in wallet"); } else { // Get the address @@ -29,7 +29,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b if (!IsValidDestination(dest)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); } - addresses.insert(dest); + addresses.emplace_back(dest); } // Filter by own scripts only diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index fae9bf3ea5..d44b62c934 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -85,14 +85,12 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons filter |= ISMINE_WATCH_ONLY; } - bool has_filtered_address = false; - CTxDestination filtered_address = CNoDestination(); + std::optional<CTxDestination> filtered_address{std::nullopt}; if (!by_label && !params[3].isNull() && !params[3].get_str().empty()) { if (!IsValidDestinationString(params[3].get_str())) { throw JSONRPCError(RPC_WALLET_ERROR, "address_filter parameter was invalid"); } filtered_address = DecodeDestination(params[3].get_str()); - has_filtered_address = true; } // Tally @@ -106,23 +104,21 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons // Coinbase with less than 1 confirmation is no longer in the main chain if ((wtx.IsCoinBase() && (nDepth < 1)) - || (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase)) - { + || (wallet.IsTxImmatureCoinBase(wtx) && !include_immature_coinbase)) { continue; } - for (const CTxOut& txout : wtx.tx->vout) - { + for (const CTxOut& txout : wtx.tx->vout) { CTxDestination address; if (!ExtractDestination(txout.scriptPubKey, address)) continue; - if (has_filtered_address && !(filtered_address == address)) { + if (filtered_address && !(filtered_address == address)) { continue; } isminefilter mine = wallet.IsMine(address); - if(!(mine & filter)) + if (!(mine & filter)) continue; tallyitem& item = mapTally[address]; @@ -138,70 +134,55 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons UniValue ret(UniValue::VARR); std::map<std::string, tallyitem> label_tally; - // Create m_address_book iterator - // If we aren't filtering, go from begin() to end() - auto start = wallet.m_address_book.begin(); - auto end = wallet.m_address_book.end(); - // If we are filtering, find() the applicable entry - if (has_filtered_address) { - start = wallet.m_address_book.find(filtered_address); - if (start != end) { - end = std::next(start); - } - } + const auto& func = [&](const CTxDestination& address, const std::string& label, const std::string& purpose, bool is_change) { + if (is_change) return; // no change addresses - for (auto item_it = start; item_it != end; ++item_it) - { - if (item_it->second.IsChange()) continue; - const CTxDestination& address = item_it->first; - const std::string& label = item_it->second.GetLabel(); auto it = mapTally.find(address); if (it == mapTally.end() && !fIncludeEmpty) - continue; + return; CAmount nAmount = 0; int nConf = std::numeric_limits<int>::max(); bool fIsWatchonly = false; - if (it != mapTally.end()) - { + if (it != mapTally.end()) { nAmount = (*it).second.nAmount; nConf = (*it).second.nConf; fIsWatchonly = (*it).second.fIsWatchonly; } - if (by_label) - { + if (by_label) { tallyitem& _item = label_tally[label]; _item.nAmount += nAmount; _item.nConf = std::min(_item.nConf, nConf); _item.fIsWatchonly = fIsWatchonly; - } - else - { + } else { UniValue obj(UniValue::VOBJ); - if(fIsWatchonly) - obj.pushKV("involvesWatchonly", true); + if (fIsWatchonly) obj.pushKV("involvesWatchonly", true); obj.pushKV("address", EncodeDestination(address)); obj.pushKV("amount", ValueFromAmount(nAmount)); obj.pushKV("confirmations", (nConf == std::numeric_limits<int>::max() ? 0 : nConf)); obj.pushKV("label", label); UniValue transactions(UniValue::VARR); - if (it != mapTally.end()) - { - for (const uint256& _item : (*it).second.txids) - { + if (it != mapTally.end()) { + for (const uint256& _item : (*it).second.txids) { transactions.push_back(_item.GetHex()); } } obj.pushKV("txids", transactions); ret.push_back(obj); } + }; + + if (filtered_address) { + const auto& entry = wallet.FindAddressBookEntry(*filtered_address, /*allow_change=*/false); + if (entry) func(*filtered_address, entry->GetLabel(), entry->purpose, /*is_change=*/false); + } else { + // No filtered addr, walk-through the addressbook entry + wallet.ForEachAddrBookEntry(func); } - if (by_label) - { - for (const auto& entry : label_tally) - { + if (by_label) { + for (const auto& entry : label_tally) { CAmount nAmount = entry.second.nAmount; int nConf = entry.second.nConf; UniValue obj(UniValue::VOBJ); @@ -908,7 +889,7 @@ RPCHelpMan rescanblockchain() } CWallet::ScanResult result = - pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, true /* fUpdate */); + pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, /*fUpdate=*/true, /*save_progress=*/false); switch (result.status) { case CWallet::ScanResult::SUCCESS: break; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 1d22d0993e..21007f5a92 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -27,25 +27,20 @@ using interfaces::FoundBlock; namespace wallet { static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100}; -int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out, bool use_max_sig) -{ - return CalculateMaximumSignedInputSize(wtx.tx->vout[out], &wallet, use_max_sig); -} - -int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* provider, const CCoinControl* coin_control) { CMutableTransaction txn; - txn.vin.push_back(CTxIn(COutPoint())); - if (!provider || !DummySignInput(*provider, txn.vin[0], txout, use_max_sig)) { + txn.vin.push_back(CTxIn(outpoint)); + if (!provider || !DummySignInput(*provider, txn.vin[0], txout, coin_control)) { return -1; } return GetVirtualTransactionInputSize(txn.vin[0]); } -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, const CCoinControl* coin_control) { const std::unique_ptr<SigningProvider> provider = wallet->GetSolvingProvider(txout.scriptPubKey); - return CalculateMaximumSignedInputSize(txout, provider.get(), use_max_sig); + return CalculateMaximumSignedInputSize(txout, COutPoint(), provider.get(), coin_control); } // txouts needs to be in the order of tx.vin @@ -198,7 +193,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, // Filter by spendable outputs only if (!spendable && only_spendable) continue; - int input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly)); + int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), coinControl); result.coins.emplace_back(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate); result.total_amount += output.nValue; @@ -289,8 +284,9 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) ) { CTxDestination address; if (ExtractDestination(FindNonChangeParentOutput(wallet, *wtx.tx, output.n).scriptPubKey, address)) { + const auto out = wtx.tx->vout.at(output.n); result[address].emplace_back( - COutPoint(wtx.GetHash(), output.n), wtx.tx->vout.at(output.n), depth, GetTxSpendSize(wallet, wtx, output.n), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); + COutPoint(wtx.GetHash(), output.n), out, depth, CalculateMaximumSignedInputSize(out, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); } } } @@ -450,14 +446,14 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec if (ptr_wtx->tx->vout.size() <= outpoint.n) { return std::nullopt; } - input_bytes = GetTxSpendSize(wallet, *ptr_wtx, outpoint.n, false); txout = ptr_wtx->tx->vout.at(outpoint.n); + input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control); } else { // The input is external. We did not find the tx in mapWallet. if (!coin_control.GetExternalOutput(outpoint, txout)) { return std::nullopt; } - input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /*use_max_sig=*/true); + input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); } // If available, override calculated size with coin control specified size if (coin_control.HasInputWeight(outpoint)) { diff --git a/src/wallet/spend.h b/src/wallet/spend.h index cba42d6fae..21f0095e77 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -14,23 +14,16 @@ namespace wallet { /** Get the marginal bytes if spending the specified output from this transaction. - * use_max_sig indicates whether to use the maximum sized, 72 byte signature when calculating the - * size of the input spend. This should only be set when watch-only outputs are allowed */ -int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out, bool use_max_sig = false); - -//Get the marginal bytes of spending the specified output -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, bool use_max_sig = false); -int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* pwallet, bool use_max_sig = false); - + * Use CoinControl to determine whether to expect signature grinding when calculating the size of the input spend. */ +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, const CCoinControl* coin_control = nullptr); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* pwallet, const CCoinControl* coin_control = nullptr); struct TxSize { int64_t vsize{-1}; int64_t weight{-1}; }; -/** Calculate the size of the transaction assuming all signatures are max size -* Use DummySignatureCreator, which inserts 71 byte signatures everywhere. -* NOTE: this requires that all inputs must be in mapWallet (eg the tx should -* be AllInputsMine). */ +/** Calculate the size of the transaction using CoinControl to determine + * whether to expect signature grinding when calculating the size of the input spend. */ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control = nullptr); TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* wallet, const CCoinControl* coin_control = nullptr) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 27202cd7f3..9dd17c8e48 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -86,7 +86,8 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{})); assert(ret.second); CWalletTx& wtx = (*ret.first).second; - coins.emplace_back(COutPoint(wtx.GetHash(), nInput), wtx.tx->vout.at(nInput), nAge, GetTxSpendSize(wallet, wtx, nInput), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); + const auto& txout = wtx.tx->vout.at(nInput); + coins.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); } /** Check if SelectionResult a is equivalent to SelectionResult b. @@ -866,7 +867,23 @@ BOOST_AUTO_TEST_CASE(waste_test) const CAmount new_target{in_amt - fee * 2 - fee_diff * 2}; add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /* change cost */ 0, new_target)); + BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/ 0, new_target)); + selection.clear(); + + // Negative waste when the long term fee is greater than the current fee and the selected value == target + const CAmount exact_target1{3 * COIN - 2 * fee}; + const CAmount target_waste1{-2 * fee_diff}; // = (2 * fee) - (2 * (fee + fee_diff)) + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + BOOST_CHECK_EQUAL(target_waste1, GetSelectionWaste(selection, /*change_cost=*/ 0, exact_target1)); + selection.clear(); + + // Negative waste when the long term fee is greater than the current fee and change_cost < - (inputs * (fee - long_term_fee)) + const CAmount large_fee_diff{90}; + const CAmount target_waste2{-2 * large_fee_diff + change_cost}; // = (2 * fee) - (2 * (fee + large_fee_diff)) + change_cost + add_coin(1 * COIN, 1, selection, fee, fee + large_fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + large_fee_diff); + BOOST_CHECK_EQUAL(target_waste2, GetSelectionWaste(selection, change_cost, target)); } BOOST_AUTO_TEST_CASE(effective_value_test) diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index aa3121511d..ab72721f9d 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -38,7 +38,7 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc } WalletRescanReserver reserver(*wallet); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), 0 /* start_height */, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height()); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 0c03b2f4a2..4093392ff9 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -112,7 +112,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* start_block */, 0 /* start_height */, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/{}, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK(result.last_scanned_block.IsNull()); @@ -123,7 +123,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -131,13 +131,28 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); + std::chrono::steady_clock::time_point fake_time; + reserver.setNow([&] { fake_time += 60s; return fake_time; }); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + + { + CBlockLocator locator; + BOOST_CHECK(!WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator)); + BOOST_CHECK(locator.IsNull()); + } + + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/true); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 100 * COIN); + + { + CBlockLocator locator; + BOOST_CHECK(WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator)); + BOOST_CHECK(!locator.IsNull()); + } } // Prune the older block file. @@ -161,7 +176,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); @@ -188,7 +203,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash()); BOOST_CHECK(result.last_scanned_block.IsNull()); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 041481559b..79ec29f19a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1504,13 +1504,16 @@ bool CWallet::AddWalletFlags(uint64_t flags) } // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) -// or a max-sized low-S signature (e.g. 72 bytes) if use_max_sig is true -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig) +// or a max-sized low-S signature (e.g. 72 bytes) depending on coin_control +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control) { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; + // Use max sig if watch only inputs were used or if this particular input is an external input + // to ensure a sufficient fee is attained for the requested feerate. + const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(tx_in.prevout)); if (!ProduceSignature(provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; } @@ -1577,12 +1580,9 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> nIn++; continue; } - // Use max sig if watch only inputs were used or if this particular input is an external input - // to ensure a sufficient fee is attained for the requested feerate. - const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(txin.prevout)); const std::unique_ptr<SigningProvider> provider = GetSolvingProvider(txout.scriptPubKey); - if (!provider || !DummySignInput(*provider, txin, txout, use_max_sig)) { - if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, use_max_sig)) { + if (!provider || !DummySignInput(*provider, txin, txout, coin_control)) { + if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, coin_control)) { return false; } } @@ -1665,7 +1665,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r if (start) { // TODO: this should take into account failure by ScanResult::USER_ABORT - ScanResult result = ScanForWalletTransactions(start_block, start_height, {} /* max_height */, reserver, update); + ScanResult result = ScanForWalletTransactions(start_block, start_height, /*max_height=*/{}, reserver, /*fUpdate=*/update, /*save_progress=*/false); if (result.status == ScanResult::FAILURE) { int64_t time_max; CHECK_NONFATAL(chain().findBlock(result.last_failed_block, FoundBlock().maxTime(time_max))); @@ -1696,12 +1696,11 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r * the main chain after to the addition of any new keys you want to detect * transactions for. */ -CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate) +CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress) { - using Clock = std::chrono::steady_clock; - constexpr auto LOG_INTERVAL{60s}; - auto current_time{Clock::now()}; - auto start_time{Clock::now()}; + constexpr auto INTERVAL_TIME{60s}; + auto current_time{reserver.now()}; + auto start_time{reserver.now()}; assert(reserver.isReserved()); @@ -1728,8 +1727,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) { ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); } - if (Clock::now() >= current_time + LOG_INTERVAL) { - current_time = Clock::now(); + + bool next_interval = reserver.now() >= current_time + INTERVAL_TIME; + if (next_interval) { + current_time = reserver.now(); WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", block_height, progress_current); } @@ -1759,6 +1760,16 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; result.last_scanned_height = block_height; + + if (save_progress && next_interval) { + CBlockLocator loc = m_chain->getActiveChainLocator(block_hash); + + if (!loc.IsNull()) { + WalletLogPrintf("Saving scan progress %d.\n", block_height); + WalletBatch batch(GetDatabase()); + batch.WriteBestBlock(loc); + } + } } else { // could not scan block, keep scanning but record this block as the most recent failure result.last_failed_block = block_hash; @@ -1796,7 +1807,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", block_height, progress_current); result.status = ScanResult::USER_ABORT; } else { - auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - start_time); + auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(reserver.now() - start_time); WalletLogPrintf("Rescan completed in %15dms\n", duration_milliseconds.count()); } return result; @@ -2368,21 +2379,45 @@ void CWallet::MarkDestinationsDirty(const std::set<CTxDestination>& destinations } } -std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) const +void CWallet::ForEachAddrBookEntry(const ListAddrBookFunc& func) const { AssertLockHeld(cs_wallet); - std::set<CTxDestination> result; - for (const std::pair<const CTxDestination, CAddressBookData>& item : m_address_book) - { - if (item.second.IsChange()) continue; - const CTxDestination& address = item.first; - const std::string& strName = item.second.GetLabel(); - if (strName == label) - result.insert(address); + for (const std::pair<const CTxDestination, CAddressBookData>& item : m_address_book) { + const auto& entry = item.second; + func(item.first, entry.GetLabel(), entry.purpose, entry.IsChange()); } +} + +std::vector<CTxDestination> CWallet::ListAddrBookAddresses(const std::optional<AddrBookFilter>& _filter) const +{ + AssertLockHeld(cs_wallet); + std::vector<CTxDestination> result; + AddrBookFilter filter = _filter ? *_filter : AddrBookFilter(); + ForEachAddrBookEntry([&result, &filter](const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change) { + // Filter by change + if (filter.ignore_change && is_change) return; + // Filter by label + if (filter.m_op_label && *filter.m_op_label != label) return; + // All good + result.emplace_back(dest); + }); return result; } +std::set<std::string> CWallet::ListAddrBookLabels(const std::string& purpose) const +{ + AssertLockHeld(cs_wallet); + std::set<std::string> label_set; + ForEachAddrBookEntry([&](const CTxDestination& _dest, const std::string& _label, + const std::string& _purpose, bool _is_change) { + if (_is_change) return; + if (purpose.empty() || _purpose == purpose) { + label_set.insert(_label); + } + }); + return label_set; +} + bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, bilingual_str& error) { m_spk_man = pwallet->GetScriptPubKeyMan(type, internal); @@ -3045,7 +3080,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf { WalletRescanReserver reserver(*walletInstance); - if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { + if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/true).status)) { error = _("Failed to rescan the wallet during initialization"); return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bf42b3da9e..865ce92311 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -528,7 +528,7 @@ public: //! USER_ABORT. uint256 last_failed_block; }; - ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate); + ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); @@ -635,7 +635,30 @@ public: std::optional<int64_t> GetOldestKeyPoolTime() const; - std::set<CTxDestination> GetLabelAddresses(const std::string& label) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + // Filter struct for 'ListAddrBookAddresses' + struct AddrBookFilter { + // Fetch addresses with the provided label + std::optional<std::string> m_op_label{std::nullopt}; + // Don't include change addresses by default + bool ignore_change{true}; + }; + + /** + * Filter and retrieve destinations stored in the addressbook + */ + std::vector<CTxDestination> ListAddrBookAddresses(const std::optional<AddrBookFilter>& filter) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + /** + * Retrieve all the known labels in the address book + */ + std::set<std::string> ListAddrBookLabels(const std::string& purpose) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + /** + * Walk-through the address book entries. + * Stops when the provided 'ListAddrBookFunc' returns false. + */ + using ListAddrBookFunc = std::function<void(const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change)>; + void ForEachAddrBookEntry(const ListAddrBookFunc& func) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Marks all outputs in each one of the destinations dirty, so their cache is @@ -896,8 +919,11 @@ void MaybeResendWalletTxs(WalletContext& context); class WalletRescanReserver { private: + using Clock = std::chrono::steady_clock; + using NowFn = std::function<Clock::time_point()>; CWallet& m_wallet; bool m_could_reserve; + NowFn m_now; public: explicit WalletRescanReserver(CWallet& w) : m_wallet(w), m_could_reserve(false) {} @@ -918,6 +944,10 @@ public: return (m_could_reserve && m_wallet.fScanningWallet); } + Clock::time_point now() const { return m_now ? m_now() : Clock::now(); }; + + void setNow(NowFn now) { m_now = std::move(now); } + ~WalletRescanReserver() { if (m_could_reserve) { @@ -932,7 +962,7 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); //! Remove wallet name from persistent configuration so it will not be loaded on startup. bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool use_max_sig); +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control = nullptr); bool FillInputToWeight(CTxIn& txin, int64_t target_weight); } // namespace wallet diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 91dc222bab..8e5cbba01a 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -4,28 +4,18 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the RBF code.""" -from copy import deepcopy from decimal import Decimal from test_framework.messages import ( BIP125_SEQUENCE_NUMBER, COIN, - COutPoint, - CTransaction, - CTxIn, - CTxOut, SEQUENCE_FINAL, ) -from test_framework.script import CScript, OP_DROP from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, ) -from test_framework.script_util import ( - DUMMY_P2WPKH_SCRIPT, - DUMMY_2_P2WPKH_SCRIPT, -) from test_framework.wallet import MiniWallet from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE @@ -35,7 +25,6 @@ class ReplaceByFeeTest(BitcoinTestFramework): self.num_nodes = 2 self.extra_args = [ [ - "-acceptnonstdtxn=1", "-maxorphantx=1000", "-limitancestorcount=50", "-limitancestorsize=101", @@ -94,17 +83,19 @@ class ReplaceByFeeTest(BitcoinTestFramework): self.log.info("Running test replacement relay fee...") self.test_replacement_relay_fee() + self.log.info("Running test full replace by fee...") + self.test_fullrbf() + self.log.info("Passed") - def make_utxo(self, node, amount, confirmed=True, scriptPubKey=DUMMY_P2WPKH_SCRIPT): + def make_utxo(self, node, amount, *, confirmed=True, scriptPubKey=None): """Create a txout with a given amount and scriptPubKey - confirmed - txouts created will be confirmed in the blockchain; + confirmed - txout created will be confirmed in the blockchain; unconfirmed otherwise. """ - txid, n = self.wallet.send_to(from_node=node, scriptPubKey=scriptPubKey, amount=amount) + txid, n = self.wallet.send_to(from_node=node, scriptPubKey=scriptPubKey or self.wallet.get_scriptPubKey(), amount=amount) - # If requested, ensure txouts are confirmed. if confirmed: mempool_size = len(node.getrawmempool()) while mempool_size > 0: @@ -115,30 +106,24 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert new_size < mempool_size mempool_size = new_size - return COutPoint(int(txid, 16), n) + return self.wallet.get_utxo(txid=txid, vout=n) def test_simple_doublespend(self): """Simple doublespend""" # we use MiniWallet to create a transaction template with inputs correctly set, # and modify the output (amount, scriptPubKey) according to our needs - tx_template = self.wallet.create_self_transfer()['tx'] - - tx1a = deepcopy(tx_template) - tx1a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1a_hex = tx1a.serialize().hex() - tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, 0) + tx = self.wallet.create_self_transfer()["tx"] + tx1a_txid = self.nodes[0].sendrawtransaction(tx.serialize().hex()) # Should fail because we haven't changed the fee - tx1b = deepcopy(tx_template) - tx1b.vout = [CTxOut(1 * COIN, DUMMY_2_P2WPKH_SCRIPT)] - tx1b_hex = tx1b.serialize().hex() + tx.vout[0].scriptPubKey[-1] ^= 1 # This will raise an exception due to insufficient fee - assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, 0) + assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex(), 0) # Extra 0.1 BTC fee - tx1b.vout[0].nValue -= int(0.1 * COIN) - tx1b_hex = tx1b.serialize().hex() + tx.vout[0].nValue -= int(0.1 * COIN) + tx1b_hex = tx.serialize().hex() # Works when enabled tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, 0) @@ -160,28 +145,28 @@ class ReplaceByFeeTest(BitcoinTestFramework): chain_txids = [] while remaining_value > 1 * COIN: remaining_value -= int(0.1 * COIN) - tx = CTransaction() - tx.vin = [CTxIn(prevout, nSequence=0)] - tx.vout = [CTxOut(remaining_value, CScript([1, OP_DROP] * 15 + [1]))] - tx_hex = tx.serialize().hex() - txid = self.nodes[0].sendrawtransaction(tx_hex, 0) - chain_txids.append(txid) - prevout = COutPoint(int(txid, 16), 0) + prevout = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=prevout, + sequence=0, + fee=Decimal("0.1"), + )["new_utxo"] + chain_txids.append(prevout["txid"]) # Whether the double-spend is allowed is evaluated by including all # child fees - 4 BTC - so this attempt is rejected. - dbl_tx = CTransaction() - dbl_tx.vin = [CTxIn(tx0_outpoint, nSequence=0)] - dbl_tx.vout = [CTxOut(initial_nValue - 3 * COIN, DUMMY_P2WPKH_SCRIPT)] + dbl_tx = self.wallet.create_self_transfer( + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=Decimal("3"), + )["tx"] dbl_tx_hex = dbl_tx.serialize().hex() # This will raise an exception due to insufficient fee assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0) # Accepted with sufficient fee - dbl_tx = CTransaction() - dbl_tx.vin = [CTxIn(tx0_outpoint, nSequence=0)] - dbl_tx.vout = [CTxOut(int(0.1 * COIN), DUMMY_P2WPKH_SCRIPT)] + dbl_tx.vout[0].nValue = int(0.1 * COIN) dbl_tx_hex = dbl_tx.serialize().hex() self.nodes[0].sendrawtransaction(dbl_tx_hex, 0) @@ -205,22 +190,19 @@ class ReplaceByFeeTest(BitcoinTestFramework): if txout_value < fee: return - vout = [CTxOut(txout_value, CScript([i+1])) - for i in range(tree_width)] - tx = CTransaction() - tx.vin = [CTxIn(prevout, nSequence=0)] - tx.vout = vout - tx_hex = tx.serialize().hex() + tx = self.wallet.send_self_transfer_multi( + utxos_to_spend=[prevout], + from_node=self.nodes[0], + sequence=0, + num_outputs=tree_width, + amount_per_output=txout_value, + ) - assert len(tx.serialize()) < 100000 - txid = self.nodes[0].sendrawtransaction(tx_hex, 0) - yield tx + yield tx["txid"] _total_txs[0] += 1 - txid = int(txid, 16) - - for i, txout in enumerate(tx.vout): - for x in branch(COutPoint(txid, i), txout_value, + for utxo in tx["new_utxos"]: + for x in branch(utxo, txout_value, max_txs, tree_width=tree_width, fee=fee, _total_txs=_total_txs): @@ -232,25 +214,26 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert_equal(len(tree_txs), n) # Attempt double-spend, will fail because too little fee paid - dbl_tx = CTransaction() - dbl_tx.vin = [CTxIn(tx0_outpoint, nSequence=0)] - dbl_tx.vout = [CTxOut(initial_nValue - fee * n, DUMMY_P2WPKH_SCRIPT)] - dbl_tx_hex = dbl_tx.serialize().hex() + dbl_tx_hex = self.wallet.create_self_transfer( + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=(Decimal(fee) / COIN) * n, + )["hex"] # This will raise an exception due to insufficient fee assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0) # 0.1 BTC fee is enough - dbl_tx = CTransaction() - dbl_tx.vin = [CTxIn(tx0_outpoint, nSequence=0)] - dbl_tx.vout = [CTxOut(initial_nValue - fee * n - int(0.1 * COIN), DUMMY_P2WPKH_SCRIPT)] - dbl_tx_hex = dbl_tx.serialize().hex() + dbl_tx_hex = self.wallet.create_self_transfer( + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=(Decimal(fee) / COIN) * n + Decimal("0.1"), + )["hex"] self.nodes[0].sendrawtransaction(dbl_tx_hex, 0) mempool = self.nodes[0].getrawmempool() - for tx in tree_txs: - tx.rehash() - assert tx.hash not in mempool + for txid in tree_txs: + assert txid not in mempool # Try again, but with more total transactions than the "max txs # double-spent at once" anti-DoS limit. @@ -260,33 +243,36 @@ class ReplaceByFeeTest(BitcoinTestFramework): tree_txs = list(branch(tx0_outpoint, initial_nValue, n, fee=fee)) assert_equal(len(tree_txs), n) - dbl_tx = CTransaction() - dbl_tx.vin = [CTxIn(tx0_outpoint, nSequence=0)] - dbl_tx.vout = [CTxOut(initial_nValue - 2 * fee * n, DUMMY_P2WPKH_SCRIPT)] - dbl_tx_hex = dbl_tx.serialize().hex() + dbl_tx_hex = self.wallet.create_self_transfer( + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=2 * (Decimal(fee) / COIN) * n, + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "too many potential replacements", self.nodes[0].sendrawtransaction, dbl_tx_hex, 0) - for tx in tree_txs: - tx.rehash() - self.nodes[0].getrawtransaction(tx.hash) + for txid in tree_txs: + self.nodes[0].getrawtransaction(txid) def test_replacement_feeperkb(self): """Replacement requires fee-per-KB to be higher""" tx0_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN)) - tx1a = CTransaction() - tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1a_hex = tx1a.serialize().hex() - self.nodes[0].sendrawtransaction(tx1a_hex, 0) + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=Decimal("0.1"), + ) # Higher fee, but the fee per KB is much lower, so the replacement is # rejected. - tx1b = CTransaction() - tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1b.vout = [CTxOut(int(0.001 * COIN), CScript([b'a' * 999000]))] - tx1b_hex = tx1b.serialize().hex() + tx1b_hex = self.wallet.create_self_transfer_multi( + utxos_to_spend=[tx0_outpoint], + sequence=0, + num_outputs=100, + amount_per_output=1000, + )["hex"] # This will raise an exception due to insufficient fee assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, 0) @@ -296,37 +282,36 @@ class ReplaceByFeeTest(BitcoinTestFramework): utxo1 = self.make_utxo(self.nodes[0], int(1.2 * COIN)) utxo2 = self.make_utxo(self.nodes[0], 3 * COIN) - tx1a = CTransaction() - tx1a.vin = [CTxIn(utxo1, nSequence=0)] - tx1a.vout = [CTxOut(int(1.1 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx1a_hex = tx1a.serialize().hex() - tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, 0) - - tx1a_txid = int(tx1a_txid, 16) + tx1a_utxo = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=utxo1, + sequence=0, + fee=Decimal("0.1"), + )["new_utxo"] # Direct spend an output of the transaction we're replacing. - tx2 = CTransaction() - tx2.vin = [CTxIn(utxo1, nSequence=0), CTxIn(utxo2, nSequence=0)] - tx2.vin.append(CTxIn(COutPoint(tx1a_txid, 0), nSequence=0)) - tx2.vout = tx1a.vout - tx2_hex = tx2.serialize().hex() + tx2_hex = self.wallet.create_self_transfer_multi( + utxos_to_spend=[utxo1, utxo2, tx1a_utxo], + sequence=0, + amount_per_output=int(COIN * tx1a_utxo["value"]), + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, tx2_hex, 0) # Spend tx1a's output to test the indirect case. - tx1b = CTransaction() - tx1b.vin = [CTxIn(COutPoint(tx1a_txid, 0), nSequence=0)] - tx1b.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1b_hex = tx1b.serialize().hex() - tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, 0) - tx1b_txid = int(tx1b_txid, 16) + tx1b_utxo = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx1a_utxo, + sequence=0, + fee=Decimal("0.1"), + )["new_utxo"] - tx2 = CTransaction() - tx2.vin = [CTxIn(utxo1, nSequence=0), CTxIn(utxo2, nSequence=0), - CTxIn(COutPoint(tx1b_txid, 0))] - tx2.vout = tx1a.vout - tx2_hex = tx2.serialize().hex() + tx2_hex = self.wallet.create_self_transfer_multi( + utxos_to_spend=[utxo1, utxo2, tx1b_utxo], + sequence=0, + amount_per_output=int(COIN * tx1a_utxo["value"]), + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "bad-txns-spends-conflicting-tx", self.nodes[0].sendrawtransaction, tx2_hex, 0) @@ -334,18 +319,20 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_new_unconfirmed_inputs(self): """Replacements that add new unconfirmed inputs are rejected""" confirmed_utxo = self.make_utxo(self.nodes[0], int(1.1 * COIN)) - unconfirmed_utxo = self.make_utxo(self.nodes[0], int(0.1 * COIN), False) + unconfirmed_utxo = self.make_utxo(self.nodes[0], int(0.1 * COIN), confirmed=False) - tx1 = CTransaction() - tx1.vin = [CTxIn(confirmed_utxo)] - tx1.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1_hex = tx1.serialize().hex() - self.nodes[0].sendrawtransaction(tx1_hex, 0) + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=confirmed_utxo, + sequence=0, + fee=Decimal("0.1"), + ) - tx2 = CTransaction() - tx2.vin = [CTxIn(confirmed_utxo), CTxIn(unconfirmed_utxo)] - tx2.vout = tx1.vout - tx2_hex = tx2.serialize().hex() + tx2_hex = self.wallet.create_self_transfer_multi( + utxos_to_spend=[confirmed_utxo, unconfirmed_utxo], + sequence=0, + amount_per_output=1 * COIN, + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "replacement-adds-unconfirmed", self.nodes[0].sendrawtransaction, tx2_hex, 0) @@ -361,45 +348,39 @@ class ReplaceByFeeTest(BitcoinTestFramework): fee = int(0.0001 * COIN) split_value = int((initial_nValue - fee) / (MAX_REPLACEMENT_LIMIT + 1)) - outputs = [] - for _ in range(MAX_REPLACEMENT_LIMIT + 1): - outputs.append(CTxOut(split_value, CScript([1]))) - - splitting_tx = CTransaction() - splitting_tx.vin = [CTxIn(utxo, nSequence=0)] - splitting_tx.vout = outputs - splitting_tx_hex = splitting_tx.serialize().hex() - - txid = self.nodes[0].sendrawtransaction(splitting_tx_hex, 0) - txid = int(txid, 16) + splitting_tx_utxos = self.wallet.send_self_transfer_multi( + from_node=self.nodes[0], + utxos_to_spend=[utxo], + sequence=0, + num_outputs=MAX_REPLACEMENT_LIMIT + 1, + amount_per_output=split_value, + )["new_utxos"] # Now spend each of those outputs individually - for i in range(MAX_REPLACEMENT_LIMIT + 1): - tx_i = CTransaction() - tx_i.vin = [CTxIn(COutPoint(txid, i), nSequence=0)] - tx_i.vout = [CTxOut(split_value - fee, DUMMY_P2WPKH_SCRIPT)] - tx_i_hex = tx_i.serialize().hex() - self.nodes[0].sendrawtransaction(tx_i_hex, 0) + for utxo in splitting_tx_utxos: + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=utxo, + sequence=0, + fee=Decimal(fee) / COIN, + ) # Now create doublespend of the whole lot; should fail. # Need a big enough fee to cover all spending transactions and have # a higher fee rate double_spend_value = (split_value - 100 * fee) * (MAX_REPLACEMENT_LIMIT + 1) - inputs = [] - for i in range(MAX_REPLACEMENT_LIMIT + 1): - inputs.append(CTxIn(COutPoint(txid, i), nSequence=0)) - double_tx = CTransaction() - double_tx.vin = inputs - double_tx.vout = [CTxOut(double_spend_value, CScript([b'a']))] + double_tx = self.wallet.create_self_transfer_multi( + utxos_to_spend=splitting_tx_utxos, + sequence=0, + amount_per_output=double_spend_value, + )["tx"] double_tx_hex = double_tx.serialize().hex() # This will raise an exception assert_raises_rpc_error(-26, "too many potential replacements", self.nodes[0].sendrawtransaction, double_tx_hex, 0) # If we remove an input, it should pass - double_tx = CTransaction() - double_tx.vin = inputs[0:-1] - double_tx.vout = [CTxOut(double_spend_value, CScript([b'a']))] + double_tx.vin.pop() double_tx_hex = double_tx.serialize().hex() self.nodes[0].sendrawtransaction(double_tx_hex, 0) @@ -494,20 +475,22 @@ class ReplaceByFeeTest(BitcoinTestFramework): tx0_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN)) # Create a non-opting in transaction - tx1a = CTransaction() - tx1a.vin = [CTxIn(tx0_outpoint, nSequence=SEQUENCE_FINAL)] - tx1a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1a_hex = tx1a.serialize().hex() - tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, 0) + tx1a_utxo = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx0_outpoint, + sequence=SEQUENCE_FINAL, + fee=Decimal("0.1"), + )["new_utxo"] # This transaction isn't shown as replaceable - assert_equal(self.nodes[0].getmempoolentry(tx1a_txid)['bip125-replaceable'], False) + assert_equal(self.nodes[0].getmempoolentry(tx1a_utxo["txid"])['bip125-replaceable'], False) # Shouldn't be able to double-spend - tx1b = CTransaction() - tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1b.vout = [CTxOut(int(0.9 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx1b_hex = tx1b.serialize().hex() + tx1b_hex = self.wallet.create_self_transfer( + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=Decimal("0.2"), + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, tx1b_hex, 0) @@ -515,17 +498,19 @@ class ReplaceByFeeTest(BitcoinTestFramework): tx1_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN)) # Create a different non-opting in transaction - tx2a = CTransaction() - tx2a.vin = [CTxIn(tx1_outpoint, nSequence=0xfffffffe)] - tx2a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx2a_hex = tx2a.serialize().hex() - tx2a_txid = self.nodes[0].sendrawtransaction(tx2a_hex, 0) + tx2a_utxo = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx1_outpoint, + sequence=0xfffffffe, + fee=Decimal("0.1"), + )["new_utxo"] # Still shouldn't be able to double-spend - tx2b = CTransaction() - tx2b.vin = [CTxIn(tx1_outpoint, nSequence=0)] - tx2b.vout = [CTxOut(int(0.9 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx2b_hex = tx2b.serialize().hex() + tx2b_hex = self.wallet.create_self_transfer( + utxo_to_spend=tx1_outpoint, + sequence=0, + fee=Decimal("0.2"), + )["hex"] # This will raise an exception assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, tx2b_hex, 0) @@ -534,34 +519,31 @@ class ReplaceByFeeTest(BitcoinTestFramework): # opt-in on one of the inputs # Transaction should be replaceable on either input - tx1a_txid = int(tx1a_txid, 16) - tx2a_txid = int(tx2a_txid, 16) - - tx3a = CTransaction() - tx3a.vin = [CTxIn(COutPoint(tx1a_txid, 0), nSequence=SEQUENCE_FINAL), - CTxIn(COutPoint(tx2a_txid, 0), nSequence=0xfffffffd)] - tx3a.vout = [CTxOut(int(0.9 * COIN), CScript([b'c'])), CTxOut(int(0.9 * COIN), CScript([b'd']))] - tx3a_hex = tx3a.serialize().hex() - - tx3a_txid = self.nodes[0].sendrawtransaction(tx3a_hex, 0) + tx3a_txid = self.wallet.send_self_transfer_multi( + from_node=self.nodes[0], + utxos_to_spend=[tx1a_utxo, tx2a_utxo], + sequence=[SEQUENCE_FINAL, 0xfffffffd], + fee_per_output=int(0.1 * COIN), + )["txid"] # This transaction is shown as replaceable assert_equal(self.nodes[0].getmempoolentry(tx3a_txid)['bip125-replaceable'], True) - tx3b = CTransaction() - tx3b.vin = [CTxIn(COutPoint(tx1a_txid, 0), nSequence=0)] - tx3b.vout = [CTxOut(int(0.5 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx3b_hex = tx3b.serialize().hex() - - tx3c = CTransaction() - tx3c.vin = [CTxIn(COutPoint(tx2a_txid, 0), nSequence=0)] - tx3c.vout = [CTxOut(int(0.5 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx3c_hex = tx3c.serialize().hex() + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx1a_utxo, + sequence=0, + fee=Decimal("0.4"), + ) - self.nodes[0].sendrawtransaction(tx3b_hex, 0) # If tx3b was accepted, tx3c won't look like a replacement, # but make sure it is accepted anyway - self.nodes[0].sendrawtransaction(tx3c_hex, 0) + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx2a_utxo, + sequence=0, + fee=Decimal("0.4"), + ) def test_prioritised_transactions(self): # Ensure that fee deltas used via prioritisetransaction are @@ -570,17 +552,20 @@ class ReplaceByFeeTest(BitcoinTestFramework): # 1. Check that feeperkb uses modified fees tx0_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN)) - tx1a = CTransaction() - tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx1a_hex = tx1a.serialize().hex() - tx1a_txid = self.nodes[0].sendrawtransaction(tx1a_hex, 0) + tx1a_txid = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx0_outpoint, + sequence=0, + fee=Decimal("0.1"), + )["txid"] # Higher fee, but the actual fee per KB is much lower. - tx1b = CTransaction() - tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1b.vout = [CTxOut(int(0.001 * COIN), CScript([b'a' * 740000]))] - tx1b_hex = tx1b.serialize().hex() + tx1b_hex = self.wallet.create_self_transfer_multi( + utxos_to_spend=[tx0_outpoint], + sequence=0, + num_outputs=100, + amount_per_output=int(0.00001 * COIN), + )["hex"] # Verify tx1b cannot replace tx1a. assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, 0) @@ -596,27 +581,29 @@ class ReplaceByFeeTest(BitcoinTestFramework): # 2. Check that absolute fee checks use modified fee. tx1_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN)) - tx2a = CTransaction() - tx2a.vin = [CTxIn(tx1_outpoint, nSequence=0)] - tx2a.vout = [CTxOut(1 * COIN, DUMMY_P2WPKH_SCRIPT)] - tx2a_hex = tx2a.serialize().hex() - self.nodes[0].sendrawtransaction(tx2a_hex, 0) + # tx2a + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=tx1_outpoint, + sequence=0, + fee=Decimal("0.1"), + ) # Lower fee, but we'll prioritise it - tx2b = CTransaction() - tx2b.vin = [CTxIn(tx1_outpoint, nSequence=0)] - tx2b.vout = [CTxOut(int(1.01 * COIN), DUMMY_P2WPKH_SCRIPT)] - tx2b.rehash() - tx2b_hex = tx2b.serialize().hex() + tx2b = self.wallet.create_self_transfer( + utxo_to_spend=tx1_outpoint, + sequence=0, + fee=Decimal("0.09"), + ) # Verify tx2b cannot replace tx2a. - assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx2b_hex, 0) + assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx2b["hex"], 0) # Now prioritise tx2b to have a higher modified fee - self.nodes[0].prioritisetransaction(txid=tx2b.hash, fee_delta=int(0.1 * COIN)) + self.nodes[0].prioritisetransaction(txid=tx2b["txid"], fee_delta=int(0.1 * COIN)) # tx2b should now be accepted - tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, 0) + tx2b_txid = self.nodes[0].sendrawtransaction(tx2b["hex"], 0) assert tx2b_txid in self.nodes[0].getrawmempool() @@ -714,5 +701,33 @@ class ReplaceByFeeTest(BitcoinTestFramework): tx.vout[0].nValue -= 1 assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex()) + def test_fullrbf(self): + txid = self.wallet.send_self_transfer(from_node=self.nodes[0])['txid'] + self.generate(self.nodes[0], 1) + confirmed_utxo = self.wallet.get_utxo(txid=txid) + + self.restart_node(0, extra_args=["-mempoolfullrbf=1"]) + + # Create an explicitly opt-out transaction + optout_tx = self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=confirmed_utxo, + sequence=SEQUENCE_FINAL, + fee_rate=Decimal('0.01'), + ) + assert_equal(False, self.nodes[0].getmempoolentry(optout_tx['txid'])['bip125-replaceable']) + + conflicting_tx = self.wallet.create_self_transfer( + utxo_to_spend=confirmed_utxo, + sequence=SEQUENCE_FINAL, + fee_rate=Decimal('0.02'), + ) + + # Send the replacement transaction, conflicting with the optout_tx. + self.nodes[0].sendrawtransaction(conflicting_tx['hex'], 0) + + # Optout_tx is not anymore in the mempool. + assert optout_tx['txid'] not in self.nodes[0].getrawmempool() + if __name__ == '__main__': ReplaceByFeeTest().main() diff --git a/test/functional/rpc_signrawtransactionwithkey.py b/test/functional/rpc_signrawtransactionwithkey.py new file mode 100755 index 0000000000..0da5a99fdb --- /dev/null +++ b/test/functional/rpc_signrawtransactionwithkey.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test transaction signing using the signrawtransactionwithkey RPC.""" + +from test_framework.blocktools import ( + COINBASE_MATURITY, +) +from test_framework.address import ( + script_to_p2sh, +) +from test_framework.key import ECKey +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + find_vout_for_address, +) +from test_framework.script_util import ( + key_to_p2pk_script, + key_to_p2pkh_script, + script_to_p2sh_p2wsh_script, + script_to_p2wsh_script, +) +from test_framework.wallet_util import ( + bytes_to_wif, +) + +from decimal import ( + Decimal, +) +from test_framework.wallet import ( + getnewdestination, +) + + +class SignRawTransactionWithKeyTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 2 + + def send_to_address(self, addr, amount): + input = {"txid": self.nodes[0].getblock(self.block_hash[self.blk_idx])["tx"][0], "vout": 0} + output = {addr: amount} + self.blk_idx += 1 + rawtx = self.nodes[0].createrawtransaction([input], output) + txid = self.nodes[0].sendrawtransaction(self.nodes[0].signrawtransactionwithkey(rawtx, [self.nodes[0].get_deterministic_priv_key().key])["hex"], 0) + return txid + + def successful_signing_test(self): + """Create and sign a valid raw transaction with one input. + + Expected results: + + 1) The transaction has a complete set of signatures + 2) No script verification error occurred""" + self.log.info("Test valid raw transaction with one input") + privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] + + inputs = [ + # Valid pay-to-pubkey scripts + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, + 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, + {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, + 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, + ] + + outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} + + rawTx = self.nodes[0].createrawtransaction(inputs, outputs) + rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, inputs) + + # 1) The transaction has a complete set of signatures + assert rawTxSigned['complete'] + + # 2) No script verification error occurred + assert 'errors' not in rawTxSigned + + def witness_script_test(self): + self.log.info("Test signing transaction to P2SH-P2WSH addresses without wallet") + # Create a new P2SH-P2WSH 1-of-1 multisig address: + eckey = ECKey() + eckey.generate() + embedded_privkey = bytes_to_wif(eckey.get_bytes()) + embedded_pubkey = eckey.get_pubkey().get_bytes().hex() + p2sh_p2wsh_address = self.nodes[1].createmultisig(1, [embedded_pubkey], "p2sh-segwit") + # send transaction to P2SH-P2WSH 1-of-1 multisig address + self.block_hash = self.generate(self.nodes[0], COINBASE_MATURITY + 1) + self.blk_idx = 0 + self.send_to_address(p2sh_p2wsh_address["address"], 49.999) + self.generate(self.nodes[0], 1) + # Get the UTXO info from scantxoutset + unspent_output = self.nodes[1].scantxoutset('start', [p2sh_p2wsh_address['descriptor']])['unspents'][0] + spk = script_to_p2sh_p2wsh_script(p2sh_p2wsh_address['redeemScript']).hex() + unspent_output['witnessScript'] = p2sh_p2wsh_address['redeemScript'] + unspent_output['redeemScript'] = script_to_p2wsh_script(unspent_output['witnessScript']).hex() + assert_equal(spk, unspent_output['scriptPubKey']) + # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys + spending_tx = self.nodes[0].createrawtransaction([unspent_output], {getnewdestination()[2]: Decimal("49.998")}) + spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [unspent_output]) + # Check the signing completed successfully + assert 'complete' in spending_tx_signed + assert_equal(spending_tx_signed['complete'], True) + + # Now test with P2PKH and P2PK scripts as the witnessScript + for tx_type in ['P2PKH', 'P2PK']: # these tests are order-independent + self.verify_txn_with_witness_script(tx_type) + + def verify_txn_with_witness_script(self, tx_type): + self.log.info("Test with a {} script as the witnessScript".format(tx_type)) + eckey = ECKey() + eckey.generate() + embedded_privkey = bytes_to_wif(eckey.get_bytes()) + embedded_pubkey = eckey.get_pubkey().get_bytes().hex() + witness_script = { + 'P2PKH': key_to_p2pkh_script(embedded_pubkey).hex(), + 'P2PK': key_to_p2pk_script(embedded_pubkey).hex() + }.get(tx_type, "Invalid tx_type") + redeem_script = script_to_p2wsh_script(witness_script).hex() + addr = script_to_p2sh(redeem_script) + script_pub_key = self.nodes[1].validateaddress(addr)['scriptPubKey'] + # Fund that address + txid = self.send_to_address(addr, 10) + vout = find_vout_for_address(self.nodes[0], txid, addr) + self.generate(self.nodes[0], 1) + # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys + spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {getnewdestination()[2]: Decimal("9.999")}) + spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}]) + # Check the signing completed successfully + assert 'complete' in spending_tx_signed + assert_equal(spending_tx_signed['complete'], True) + self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + + def run_test(self): + self.successful_signing_test() + self.witness_script_test() + + +if __name__ == '__main__': + SignRawTransactionWithKeyTest().main() diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 1ee23f7574..58528b7858 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -512,16 +512,13 @@ def gen_return_txouts(): # Create a spend of each passed-in utxo, splicing in "txouts" to each raw # transaction to make it large. See gen_return_txouts() above. def create_lots_of_big_transactions(mini_wallet, node, fee, tx_batch_size, txouts, utxos=None): - from .messages import COIN - fee_sats = int(fee * COIN) txids = [] use_internal_utxos = utxos is None for _ in range(tx_batch_size): tx = mini_wallet.create_self_transfer( utxo_to_spend=None if use_internal_utxos else utxos.pop(), - fee_rate=0, + fee=fee, )["tx"] - tx.vout[0].nValue -= fee_sats tx.vout.extend(txouts) res = node.testmempoolaccept([tx.serialize().hex()])[0] assert_equal(res['fees']['base'], fee) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 68d5dfa880..2164627781 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -197,7 +197,7 @@ class MiniWallet: return utxos def send_self_transfer(self, *, from_node, **kwargs): - """Create and send a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" + """Call create_self_transfer and send the transaction.""" tx = self.create_self_transfer(**kwargs) self.sendrawtransaction(from_node=from_node, tx_hex=tx['hex']) return tx @@ -232,21 +232,27 @@ class MiniWallet: *, utxos_to_spend: Optional[List[dict]] = None, num_outputs=1, + amount_per_output=0, sequence=0, fee_per_output=1000, ): """ Create and return a transaction that spends the given UTXOs and creates a - certain number of outputs with equal amounts. + certain number of outputs with equal amounts. The output amounts can be + set by amount_per_output or automatically calculated with a fee_per_output. """ utxos_to_spend = utxos_to_spend or [self.get_utxo()] + sequence = [sequence] * len(utxos_to_spend) if type(sequence) is int else sequence + assert_equal(len(utxos_to_spend), len(sequence)) # create simple tx template (1 input, 1 output) tx = self.create_self_transfer( fee_rate=0, - utxo_to_spend=utxos_to_spend[0], sequence=sequence)["tx"] + utxo_to_spend=utxos_to_spend[0])["tx"] # duplicate inputs, witnesses and outputs tx.vin = [deepcopy(tx.vin[0]) for _ in range(len(utxos_to_spend))] + for txin, seq in zip(tx.vin, sequence): + txin.nSequence = seq tx.wit.vtxinwit = [deepcopy(tx.wit.vtxinwit[0]) for _ in range(len(utxos_to_spend))] tx.vout = [deepcopy(tx.vout[0]) for _ in range(num_outputs)] @@ -258,7 +264,7 @@ class MiniWallet: inputs_value_total = sum([int(COIN * utxo['value']) for utxo in utxos_to_spend]) outputs_value_total = inputs_value_total - fee_per_output * num_outputs for o in tx.vout: - o.nValue = outputs_value_total // num_outputs + o.nValue = amount_per_output or (outputs_value_total // num_outputs) txid = tx.rehash() return { "new_utxos": [self._create_utxo( @@ -272,21 +278,23 @@ class MiniWallet: "tx": tx, } - def create_self_transfer(self, *, fee_rate=Decimal("0.003"), utxo_to_spend=None, locktime=0, sequence=0): - """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" + def create_self_transfer(self, *, fee_rate=Decimal("0.003"), fee=Decimal("0"), utxo_to_spend=None, locktime=0, sequence=0): + """Create and return a tx with the specified fee. If fee is 0, use fee_rate, where the resulting fee may be exact or at most one satoshi higher than needed.""" utxo_to_spend = utxo_to_spend or self.get_utxo() + assert fee_rate >= 0 + assert fee >= 0 if self._mode in (MiniWalletMode.RAW_OP_TRUE, MiniWalletMode.ADDRESS_OP_TRUE): vsize = Decimal(104) # anyone-can-spend elif self._mode == MiniWalletMode.RAW_P2PK: vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) else: assert False - send_value = utxo_to_spend["value"] - (fee_rate * vsize / 1000) + send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000)) assert send_value > 0 tx = CTransaction() tx.vin = [CTxIn(COutPoint(int(utxo_to_spend['txid'], 16), utxo_to_spend['vout']), nSequence=sequence)] - tx.vout = [CTxOut(int(COIN * send_value), self._scriptPubKey)] + tx.vout = [CTxOut(int(COIN * send_value), bytearray(self._scriptPubKey))] tx.nLockTime = locktime if self._mode == MiniWalletMode.RAW_P2PK: self.sign_tx(tx) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 08229de57a..7f4758af43 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -182,8 +182,9 @@ BASE_SCRIPTS = [ 'rpc_whitelist.py', 'feature_proxy.py', 'feature_syscall_sandbox.py', - 'rpc_signrawtransaction.py --legacy-wallet', - 'rpc_signrawtransaction.py --descriptors', + 'wallet_signrawtransactionwithwallet.py --legacy-wallet', + 'wallet_signrawtransactionwithwallet.py --descriptors', + 'rpc_signrawtransactionwithkey.py', 'rpc_rawtransaction.py --legacy-wallet', 'wallet_groups.py --legacy-wallet', 'wallet_transactiontime_rescan.py --descriptors', diff --git a/test/functional/wallet_coinbase_category.py b/test/functional/wallet_coinbase_category.py index 5a6b6cee59..c2a8e612cf 100755 --- a/test/functional/wallet_coinbase_category.py +++ b/test/functional/wallet_coinbase_category.py @@ -15,6 +15,7 @@ from test_framework.util import ( class CoinbaseCategoryTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + self.setup_clean_chain = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index db1d8eb54a..7ae3a40eaf 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -57,6 +57,11 @@ class ReceivedByTest(BitcoinTestFramework): {"address": empty_addr}, {"address": empty_addr, "label": "", "amount": 0, "confirmations": 0, "txids": []}) + # No returned addy should be a change addr + for node in self.nodes: + for addr_obj in node.listreceivedbyaddress(): + assert_equal(node.getaddressinfo(addr_obj["address"])["ischange"], False) + # Test Address filtering # Only on addr expected = {"address": addr, "label": "", "amount": Decimal("0.1"), "confirmations": 10, "txids": [txid, ]} diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/wallet_signrawtransactionwithwallet.py index 8da2cfa72b..6b30386b7e 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -2,16 +2,14 @@ # Copyright (c) 2015-2021 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test transaction signing using the signrawtransaction* RPCs.""" +"""Test transaction signing using the signrawtransactionwithwallet RPC.""" from test_framework.blocktools import ( COINBASE_MATURITY, ) from test_framework.address import ( - script_to_p2sh, script_to_p2wsh, ) -from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -29,20 +27,13 @@ from test_framework.script import ( OP_DROP, OP_TRUE, ) -from test_framework.script_util import ( - key_to_p2pk_script, - key_to_p2pkh_script, - script_to_p2sh_p2wsh_script, - script_to_p2wsh_script, -) -from test_framework.wallet_util import bytes_to_wif from decimal import ( Decimal, getcontext, ) -class SignRawTransactionsTest(BitcoinTestFramework): +class SignRawTransactionWithWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 @@ -50,35 +41,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() - def successful_signing_test(self): - """Create and sign a valid raw transaction with one input. - - Expected results: - - 1) The transaction has a complete set of signatures - 2) No script verification error occurred""" - self.log.info("Test valid raw transaction with one input") - privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] - - inputs = [ - # Valid pay-to-pubkey scripts - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, - 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, - {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, - 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, - ] - - outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} - - rawTx = self.nodes[0].createrawtransaction(inputs, outputs) - rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, inputs) - - # 1) The transaction has a complete set of signatures - assert rawTxSigned['complete'] - - # 2) No script verification error occurred - assert 'errors' not in rawTxSigned - def test_with_lock_outputs(self): self.log.info("Test correct error reporting when trying to sign a locked output") self.nodes[0].encryptwallet("password") @@ -191,60 +153,6 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(signedtx["hex"], signedtx2["hex"]) self.nodes[0].walletlock() - def witness_script_test(self): - self.log.info("Test signing transaction to P2SH-P2WSH addresses without wallet") - # Create a new P2SH-P2WSH 1-of-1 multisig address: - eckey = ECKey() - eckey.generate() - embedded_privkey = bytes_to_wif(eckey.get_bytes()) - embedded_pubkey = eckey.get_pubkey().get_bytes().hex() - p2sh_p2wsh_address = self.nodes[1].createmultisig(1, [embedded_pubkey], "p2sh-segwit") - # send transaction to P2SH-P2WSH 1-of-1 multisig address - self.generate(self.nodes[0], COINBASE_MATURITY + 1) - self.nodes[0].sendtoaddress(p2sh_p2wsh_address["address"], 49.999) - self.generate(self.nodes[0], 1) - # Get the UTXO info from scantxoutset - unspent_output = self.nodes[1].scantxoutset('start', [p2sh_p2wsh_address['descriptor']])['unspents'][0] - spk = script_to_p2sh_p2wsh_script(p2sh_p2wsh_address['redeemScript']).hex() - unspent_output['witnessScript'] = p2sh_p2wsh_address['redeemScript'] - unspent_output['redeemScript'] = script_to_p2wsh_script(unspent_output['witnessScript']).hex() - assert_equal(spk, unspent_output['scriptPubKey']) - # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys - spending_tx = self.nodes[0].createrawtransaction([unspent_output], {self.nodes[1].get_wallet_rpc(self.default_wallet_name).getnewaddress(): Decimal("49.998")}) - spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [unspent_output]) - # Check the signing completed successfully - assert 'complete' in spending_tx_signed - assert_equal(spending_tx_signed['complete'], True) - - # Now test with P2PKH and P2PK scripts as the witnessScript - for tx_type in ['P2PKH', 'P2PK']: # these tests are order-independent - self.verify_txn_with_witness_script(tx_type) - - def verify_txn_with_witness_script(self, tx_type): - self.log.info("Test with a {} script as the witnessScript".format(tx_type)) - eckey = ECKey() - eckey.generate() - embedded_privkey = bytes_to_wif(eckey.get_bytes()) - embedded_pubkey = eckey.get_pubkey().get_bytes().hex() - witness_script = { - 'P2PKH': key_to_p2pkh_script(embedded_pubkey).hex(), - 'P2PK': key_to_p2pk_script(embedded_pubkey).hex() - }.get(tx_type, "Invalid tx_type") - redeem_script = script_to_p2wsh_script(witness_script).hex() - addr = script_to_p2sh(redeem_script) - script_pub_key = self.nodes[1].validateaddress(addr)['scriptPubKey'] - # Fund that address - txid = self.nodes[0].sendtoaddress(addr, 10) - vout = find_vout_for_address(self.nodes[0], txid, addr) - self.generate(self.nodes[0], 1) - # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys - spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): Decimal("9.999")}) - spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}]) - # Check the signing completed successfully - assert 'complete' in spending_tx_signed - assert_equal(spending_tx_signed['complete'], True) - self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) - def OP_1NEGATE_test(self): self.log.info("Test OP_1NEGATE (0x4f) satisfies BIP62 minimal push standardness rule") hex_str = ( @@ -385,9 +293,7 @@ class SignRawTransactionsTest(BitcoinTestFramework): ]) def run_test(self): - self.successful_signing_test() self.script_verification_error_test() - self.witness_script_test() self.OP_1NEGATE_test() self.test_with_lock_outputs() self.test_fully_signed_tx() @@ -397,4 +303,4 @@ class SignRawTransactionsTest(BitcoinTestFramework): if __name__ == '__main__': - SignRawTransactionsTest().main() + SignRawTransactionWithWalletTest().main() |