diff options
120 files changed, 1363 insertions, 835 deletions
diff --git a/SECURITY.md b/SECURITY.md index 7ed96c7cea..25b6175c95 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,7 +14,7 @@ The following keys may be used to communicate sensitive information to developer | Name | Fingerprint | |------|-------------| | Wladimir van der Laan | 71A3 B167 3540 5025 D447 E8F2 7481 0B01 2346 C9A6 | -| Jonas Schnelli | 32EE 5C4C 3FA1 5CCA DB46 ABE5 29D4 BCB6 416F 53EC | | Pieter Wuille | 133E AC17 9436 F14A 5CF1 B794 860F EB80 4E66 9320 | +| Michael Ford | E777 299F C265 DD04 7930 70EB 944D 35F9 AC3D B76A | -You can import a key by running the following command with that individual’s fingerprint: `gpg --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces. +You can import a key by running the following command with that individual’s fingerprint: `gpg --keyserver hkps://keys.openpgp.org --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces. diff --git a/build_msvc/bitcoin.sln b/build_msvc/bitcoin.sln index 1b1f27a8a9..5095617812 100644 --- a/build_msvc/bitcoin.sln +++ b/build_msvc/bitcoin.sln @@ -4,8 +4,6 @@ VisualStudioVersion = 16.0.28803.452 MinimumVisualStudioVersion = 10.0.40219.1 Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libbitcoinconsensus", "libbitcoinconsensus\libbitcoinconsensus.vcxproj", "{2B384FA8-9EE1-4544-93CB-0D733C25E8CE}" EndProject -Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "testconsensus", "testconsensus\testconsensus.vcxproj", "{E78473E9-B850-456C-9120-276301E04C06}" -EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bitcoind", "bitcoind\bitcoind.vcxproj", "{D4513DDF-6013-44DC-ADCC-12EAF6D1F038}" EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libbitcoin_util", "libbitcoin_util\libbitcoin_util.vcxproj", "{B53A5535-EE9D-4C6F-9A26-F79EE3BC3754}" @@ -60,10 +58,6 @@ Global {2B384FA8-9EE1-4544-93CB-0D733C25E8CE}.Debug|x64.Build.0 = Debug|x64 {2B384FA8-9EE1-4544-93CB-0D733C25E8CE}.Release|x64.ActiveCfg = Release|x64 {2B384FA8-9EE1-4544-93CB-0D733C25E8CE}.Release|x64.Build.0 = Release|x64 - {E78473E9-B850-456C-9120-276301E04C06}.Debug|x64.ActiveCfg = Debug|x64 - {E78473E9-B850-456C-9120-276301E04C06}.Debug|x64.Build.0 = Debug|x64 - {E78473E9-B850-456C-9120-276301E04C06}.Release|x64.ActiveCfg = Release|x64 - {E78473E9-B850-456C-9120-276301E04C06}.Release|x64.Build.0 = Release|x64 {D4513DDF-6013-44DC-ADCC-12EAF6D1F038}.Debug|x64.ActiveCfg = Debug|x64 {D4513DDF-6013-44DC-ADCC-12EAF6D1F038}.Debug|x64.Build.0 = Debug|x64 {D4513DDF-6013-44DC-ADCC-12EAF6D1F038}.Release|x64.ActiveCfg = Release|x64 diff --git a/build_msvc/testconsensus/testconsensus.cpp b/build_msvc/testconsensus/testconsensus.cpp deleted file mode 100644 index f3c8517130..0000000000 --- a/build_msvc/testconsensus/testconsensus.cpp +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) 2018-2020 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <iostream> - -// bitcoin includes. -#include <..\src\script\bitcoinconsensus.h> -#include <..\src\primitives\transaction.h> -#include <..\src\script\script.h> -#include <..\src\streams.h> -#include <..\src\version.h> - -CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, int nValue = 0) -{ - CMutableTransaction txSpend; - txSpend.nVersion = 1; - txSpend.nLockTime = 0; - txSpend.vin.resize(1); - txSpend.vout.resize(1); - txSpend.vin[0].scriptWitness = scriptWitness; - txSpend.vin[0].prevout.hash = uint256(); - txSpend.vin[0].prevout.n = 0; - txSpend.vin[0].scriptSig = scriptSig; - txSpend.vin[0].nSequence = CTxIn::SEQUENCE_FINAL; - txSpend.vout[0].scriptPubKey = CScript(); - txSpend.vout[0].nValue = nValue; - - return txSpend; -} - -int main() -{ - std::cout << "bitcoinconsensus version: " << bitcoinconsensus_version() << std::endl; - - CScript pubKeyScript; - pubKeyScript << OP_1 << OP_0 << OP_1; - - int amount = 0; // 600000000; - - CScript scriptSig; - CScriptWitness scriptWitness; - CTransaction vanillaSpendTx = BuildSpendingTransaction(scriptSig, scriptWitness, amount); - CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); - stream << vanillaSpendTx; - - bitcoinconsensus_error err; - auto op0Result = bitcoinconsensus_verify_script_with_amount(pubKeyScript.data(), pubKeyScript.size(), amount, stream.data(), stream.size(), 0, bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL, &err); - std::cout << "Op0 result: " << op0Result << ", error code " << err << std::endl; - - getchar(); - - return 0; -} diff --git a/build_msvc/testconsensus/testconsensus.vcxproj b/build_msvc/testconsensus/testconsensus.vcxproj deleted file mode 100644 index 776c40920a..0000000000 --- a/build_msvc/testconsensus/testconsensus.vcxproj +++ /dev/null @@ -1,28 +0,0 @@ -<?xml version="1.0" encoding="utf-8"?> -<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> - <Import Project="..\common.init.vcxproj" /> - <PropertyGroup Label="Globals"> - <ProjectGuid>{E78473E9-B850-456C-9120-276301E04C06}</ProjectGuid> - </PropertyGroup> - <PropertyGroup Label="Configuration"> - <ConfigurationType>Application</ConfigurationType> - <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> - </PropertyGroup> - <ItemGroup> - <ClCompile Include="testconsensus.cpp" /> - </ItemGroup> - <ItemGroup> - <ProjectReference Include="..\libbitcoinconsensus\libbitcoinconsensus.vcxproj"> - <Project>{2B384FA8-9EE1-4544-93CB-0D733C25E8CE}</Project> - </ProjectReference> - <ProjectReference Include="..\libbitcoin_util\libbitcoin_util.vcxproj"> - <Project>{B53A5535-EE9D-4C6F-9A26-F79EE3BC3754}</Project> - </ProjectReference> - <ProjectReference Include="..\libsecp256k1\libsecp256k1.vcxproj"> - <Project>{BB493552-3B8C-4A8C-BF69-A6E7A51D2EA6}</Project> - </ProjectReference> - </ItemGroup> - <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> - <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> - <Import Project="..\common.vcxproj" /> -</Project> diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index f7dacd8512..11dda51d2d 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -8,8 +8,8 @@ export LC_ALL=C GIT_HEAD=$(git rev-parse HEAD) if [ -n "$CIRRUS_PR" ]; then - COMMIT_RANGE="$CIRRUS_BASE_SHA..$GIT_HEAD" - test/lint/commit-script-check.sh $COMMIT_RANGE + COMMIT_RANGE="${CIRRUS_BASE_SHA}..$GIT_HEAD" + test/lint/commit-script-check.sh "$COMMIT_RANGE" fi export COMMIT_RANGE @@ -34,5 +34,7 @@ if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ "$CIRRUS_PR" = "" ] ; t ./contrib/verify-commits/verify-commits.py; fi -echo -git log --no-merges --oneline $COMMIT_RANGE +if [ -n "$COMMIT_RANGE" ]; then + echo + git log --no-merges --oneline "$COMMIT_RANGE" +fi diff --git a/ci/test/00_setup_env_android.sh b/ci/test/00_setup_env_android.sh index 2f9d1f2a9f..6faf60bd66 100755 --- a/ci/test/00_setup_env_android.sh +++ b/ci/test/00_setup_env_android.sh @@ -22,4 +22,4 @@ export ANDROID_HOME="${DEPENDS_DIR}/SDKs/android" export ANDROID_NDK_HOME="${ANDROID_HOME}/ndk/${ANDROID_NDK_VERSION}" export DEP_OPTS="ANDROID_SDK=${ANDROID_HOME} ANDROID_NDK=${ANDROID_NDK_HOME} ANDROID_API_LEVEL=${ANDROID_API_LEVEL} ANDROID_TOOLCHAIN_BIN=${ANDROID_NDK_HOME}/toolchains/llvm/prebuilt/linux-x86_64/bin/" -export BITCOIN_CONFIG="--disable-ccache --disable-tests --enable-gui-tests --disable-bench --disable-fuzz-binary --without-utils --without-libs --without-daemon" +export BITCOIN_CONFIG="--disable-tests --enable-gui-tests --disable-bench --disable-fuzz-binary --without-utils --without-libs --without-daemon" diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index b8ac691346..9c34cfdf50 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8 export DOCKER_NAME_TAG="ubuntu:20.04" export CONTAINER_NAME=ci_native_fuzz -export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev" +export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index 2cf672b91e..673326ded7 100755 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8 export DOCKER_NAME_TAG="ubuntu:20.04" export CONTAINER_NAME=ci_native_fuzz_valgrind -export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev valgrind" +export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev libsqlite3-dev valgrind" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false diff --git a/configure.ac b/configure.ac index dc3b07e4bb..7e8b124b02 100644 --- a/configure.ac +++ b/configure.ac @@ -1244,7 +1244,6 @@ if test "x$enable_fuzz" = "xyes"; then bitcoin_enable_qt=no bitcoin_enable_qt_test=no bitcoin_enable_qt_dbus=no - enable_wallet=no use_bench=no use_external_signer=no use_upnp=no diff --git a/contrib/seeds/generate-seeds.py b/contrib/seeds/generate-seeds.py index dbecba7d1d..44345e3987 100755 --- a/contrib/seeds/generate-seeds.py +++ b/contrib/seeds/generate-seeds.py @@ -61,7 +61,7 @@ def name_to_bip155(addr): raise ValueError(f'Invalid I2P {vchAddr}') elif '.' in addr: # IPv4 return (BIP155Network.IPV4, bytes((int(x) for x in addr.split('.')))) - elif ':' in addr: # IPv6 + elif ':' in addr: # IPv6 or CJDNS sub = [[], []] # prefix, suffix x = 0 addr = addr.split(':') @@ -77,7 +77,14 @@ def name_to_bip155(addr): sub[x].append(val & 0xff) nullbytes = 16 - len(sub[0]) - len(sub[1]) assert((x == 0 and nullbytes == 0) or (x == 1 and nullbytes > 0)) - return (BIP155Network.IPV6, bytes(sub[0] + ([0] * nullbytes) + sub[1])) + addr_bytes = bytes(sub[0] + ([0] * nullbytes) + sub[1]) + if addr_bytes[0] == 0xfc: + # Assume that seeds with fc00::/8 addresses belong to CJDNS, + # not to the publicly unroutable "Unique Local Unicast" network, see + # RFC4193: https://datatracker.ietf.org/doc/html/rfc4193#section-8 + return (BIP155Network.CJDNS, addr_bytes) + else: + return (BIP155Network.IPV6, addr_bytes) else: raise ValueError('Could not parse address %s' % addr) diff --git a/doc/README.md b/doc/README.md index aabfe220bc..4845f00ade 100644 --- a/doc/README.md +++ b/doc/README.md @@ -71,6 +71,7 @@ The Bitcoin repo's [root README](/README.md) contains relevant information on th ### Miscellaneous - [Assets Attribution](assets-attribution.md) +- [Assumeutxo design](assumeutxo.md) - [bitcoin.conf Configuration File](bitcoin-conf.md) - [Files](files.md) - [Fuzz-testing](fuzzing.md) diff --git a/doc/assumeutxo.md b/doc/assumeutxo.md new file mode 100644 index 0000000000..2726cf779b --- /dev/null +++ b/doc/assumeutxo.md @@ -0,0 +1,138 @@ +# assumeutxo + +Assumeutxo is a feature that allows fast bootstrapping of a validating bitcoind +instance with a very similar security model to assumevalid. + +The RPC commands `dumptxoutset` and `loadtxoutset` are used to respectively generate +and load UTXO snapshots. The utility script `./contrib/devtools/utxo_snapshot.sh` may +be of use. + +## General background + +- [assumeutxo proposal](https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal) +- [Github issue](https://github.com/bitcoin/bitcoin/issues/15605) +- [draft PR](https://github.com/bitcoin/bitcoin/pull/15606) + +## Design notes + +- A new block index `nStatus` flag is introduced, `BLOCK_ASSUMED_VALID`, to mark block + index entries that are required to be assumed-valid by a chainstate created + from a UTXO snapshot. This flag is mostly used as a way to modify certain + CheckBlockIndex() logic to account for index entries that are pending validation by a + chainstate running asynchronously in the background. We also use this flag to control + which index entries are added to setBlockIndexCandidates during LoadBlockIndex(). + +- Indexing implementations via BaseIndex can no longer assume that indexation happens + sequentially, since background validation chainstates can submit BlockConnected + events out of order with the active chain. + +- The concept of UTXO snapshots is treated as an implementation detail that lives + behind the ChainstateManager interface. The external presentation of the changes + required to facilitate the use of UTXO snapshots is the understanding that there are + now certain regions of the chain that can be temporarily assumed to be valid (using + the nStatus flag mentioned above). In certain cases, e.g. wallet rescanning, this is + very similar to dealing with a pruned chain. + + Logic outside ChainstateManager should try not to know about snapshots, instead + preferring to work in terms of more general states like assumed-valid. + + +## Chainstate phases + +Chainstate within the system goes through a number of phases when UTXO snapshots are +used, as managed by `ChainstateManager`. At various points there can be multiple +`CChainState` objects in existence to facilitate both maintaining the network tip and +performing historical validation of the assumed-valid chain. + +It is worth noting that though there are multiple separate chainstates, those +chainstates share use of a common block index (i.e. they hold the same `BlockManager` +reference). + +The subheadings below outline the phases and the corresponding changes to chainstate +data. + +### "Normal" operation via initial block download + +`ChainstateManager` manages a single CChainState object, for which +`m_snapshot_blockhash` is null. This chainstate is (maybe obviously) +considered active. This is the "traditional" mode of operation for bitcoind. + +| | | +| ---------- | ----------- | +| number of chainstates | 1 | +| active chainstate | ibd | + +### User loads a UTXO snapshot via `loadtxoutset` RPC + +`ChainstateManager` initializes a new chainstate (see `ActivateSnapshot()`) to load the +snapshot contents into. During snapshot load and validation (see +`PopulateAndValidateSnapshot()`), the new chainstate is not considered active and the +original chainstate remains in use as active. + +| | | +| ---------- | ----------- | +| number of chainstates | 2 | +| active chainstate | ibd | + +Once the snapshot chainstate is loaded and validated, it is promoted to active +chainstate and a sync to tip begins. A new chainstate directory is created in the +datadir for the snapshot chainstate called +`chainstate_[SHA256 blockhash of snapshot base block]`. + +| | | +| ---------- | ----------- | +| number of chainstates | 2 | +| active chainstate | snapshot | + +The snapshot begins to sync to tip from its base block, technically in parallel with +the original chainstate, but it is given priority during block download and is +allocated most of the cache (see `MaybeRebalanceCaches()` and usages) as our chief +consideration is getting to network tip. + +**Failure consideration:** if shutdown happens at any point during this phase, both +chainstates will be detected during the next init and the process will resume. + +### Snapshot chainstate hits network tip + +Once the snapshot chainstate leaves IBD, caches are rebalanced +(via `MaybeRebalanceCaches()` in `ActivateBestChain()`) and more cache is given +to the background chainstate, which is responsible for doing full validation of the +assumed-valid parts of the chain. + +**Note:** at this point, ValidationInterface callbacks will be coming in from both +chainstates. Considerations here must be made for indexing, which may no longer be happening +sequentially. + +### Background chainstate hits snapshot base block + +Once the tip of the background chainstate hits the base block of the snapshot +chainstate, we stop use of the background chainstate by setting `m_stop_use` (not yet +committed - see #15606), in `CompleteSnapshotValidation()`, which is checked in +`ActivateBestChain()`). We hash the background chainstate's UTXO set contents and +ensure it matches the compiled value in `CMainParams::m_assumeutxo_data`. + +The background chainstate data lingers on disk until shutdown, when in +`ChainstateManager::Reset()`, the background chainstate is cleaned up with +`ValidatedSnapshotShutdownCleanup()`, which renames the `chainstate_[hash]` datadir as +`chainstate`. + +| | | +| ---------- | ----------- | +| number of chainstates | 2 (ibd has `m_stop_use=true`) | +| active chainstate | snapshot | + +**Failure consideration:** if bitcoind unexpectedly halts after `m_stop_use` is set on +the background chainstate but before `CompleteSnapshotValidation()` can finish, the +need to complete snapshot validation will be detected on subsequent init by +`ChainstateManager::CheckForUncleanShutdown()`. + +### Bitcoind restarts sometime after snapshot validation has completed + +When bitcoind initializes again, what began as the snapshot chainstate is now +indistinguishable from a chainstate that has been built from the traditional IBD +process, and will be initialized as such. + +| | | +| ---------- | ----------- | +| number of chainstates | 1 | +| active chainstate | ibd | diff --git a/doc/bips.md b/doc/bips.md index 45954bcfd8..b5fa9315d3 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -57,3 +57,11 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v22.0**): with mainnet activation as of **v0.21.1** ([PR 21377](https://github.com/bitcoin/bitcoin/pull/21377), [PR 21686](https://github.com/bitcoin/bitcoin/pull/21686)). * [`BIP 350`](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki): Addresses for native v1+ segregated Witness outputs use Bech32m instead of Bech32 as of **v22.0** ([PR 20861](https://github.com/bitcoin/bitcoin/pull/20861)). +* [`BIP 380`](https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki) + [`381`](https://github.com/bitcoin/bips/blob/master/bip-0381.mediawiki) + [`382`](https://github.com/bitcoin/bips/blob/master/bip-0382.mediawiki) + [`383`](https://github.com/bitcoin/bips/blob/master/bip-0383.mediawiki) + [`384`](https://github.com/bitcoin/bips/blob/master/bip-0384.mediawiki) + [`385`](https://github.com/bitcoin/bips/blob/master/bip-0385.mediawiki): + Output Script Descriptors, and most of Script Expressions are implemented as of **v0.17.0** ([PR 13697](https://github.com/bitcoin/bitcoin/pull/13697)). +* [`BIP 386`](https://github.com/bitcoin/bips/blob/master/bip-0386.mediawiki): tr() Output Script Descriptors are implemented as of **v22.0** ([PR 22051](https://github.com/bitcoin/bitcoin/pull/22051)). diff --git a/doc/build-unix.md b/doc/build-unix.md index 02c36eea7c..5d9e5ec2f4 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -42,13 +42,12 @@ Optional dependencies: ------------|------------------|---------------------- miniupnpc | UPnP Support | Firewall-jumping support libnatpmp | NAT-PMP Support | Firewall-jumping support - libdb4.8 | Berkeley DB | Optional, wallet storage (only needed when wallet enabled) + libdb4.8 | Berkeley DB | Wallet storage (only needed when legacy wallet enabled) qt | GUI | GUI toolkit (only needed when GUI enabled) - libqrencode | QR codes in GUI | Optional for generating QR codes (only needed when GUI enabled) - univalue | Utility | JSON parsing and encoding (bundled version will be used unless --with-system-univalue passed to configure) - libzmq3 | ZMQ notification | Optional, allows generating ZMQ notifications (requires ZMQ version >= 4.0.0) - sqlite3 | SQLite DB | Optional, wallet storage (only needed when wallet enabled) - systemtap | Tracing (USDT) | Optional, statically defined tracepoints + libqrencode | QR codes in GUI | QR code generation (only needed when GUI enabled) + libzmq3 | ZMQ notification | ZMQ notifications (requires ZMQ version >= 4.0.0) + sqlite3 | SQLite DB | Wallet storage (only needed when descriptor wallet enabled) + systemtap | Tracing (USDT) | Statically defined tracepoints For the versions used, see [dependencies.md](dependencies.md) @@ -85,19 +84,15 @@ Now, you can either build from self-compiled [depends](/depends/README.md) or in sudo apt-get install libevent-dev libboost-dev libboost-system-dev libboost-filesystem-dev libboost-test-dev -Berkeley DB is required for the wallet. - -Ubuntu and Debian have their own `libdb-dev` and `libdb++-dev` packages, but these will install -Berkeley DB 5.1 or later. This will break binary wallet compatibility with the distributed executables, which -are based on BerkeleyDB 4.8. If you do not care about wallet compatibility, -pass `--with-incompatible-bdb` to configure. - -Otherwise, you can build Berkeley DB [yourself](#berkeley-db). - SQLite is required for the descriptor wallet: sudo apt install libsqlite3-dev +Berkeley DB is required for the legacy wallet. Ubuntu and Debian have their own `libdb-dev` and `libdb++-dev` packages, +but these will install Berkeley DB 5.1 or later. This will break binary wallet compatibility with the distributed +executables, which are based on BerkeleyDB 4.8. If you do not care about wallet compatibility, pass +`--with-incompatible-bdb` to configure. Otherwise, you can build Berkeley DB [yourself](#berkeley-db). + To build Bitcoin Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode) Optional port mapping libraries (see: `--with-miniupnpc`, `--enable-upnp-default`, and `--with-natpmp`, `--enable-natpmp-default`): @@ -146,20 +141,18 @@ Now, you can either build from self-compiled [depends](/depends/README.md) or in sudo dnf install libevent-devel boost-devel -Berkeley DB is required for the wallet: +SQLite is required for the descriptor wallet: + + sudo dnf install sqlite-devel + +Berkeley DB is required for the legacy wallet: sudo dnf install libdb4-devel libdb4-cxx-devel Newer Fedora releases, since Fedora 33, have only `libdb-devel` and `libdb-cxx-devel` packages, but these will install Berkeley DB 5.3 or later. This will break binary wallet compatibility with the distributed executables, which are based on Berkeley DB 4.8. If you do not care about wallet compatibility, -pass `--with-incompatible-bdb` to configure. - -Otherwise, you can build Berkeley DB [yourself](#berkeley-db). - -SQLite is required for the descriptor wallet: - - sudo dnf install sqlite-devel +pass `--with-incompatible-bdb` to configure. Otherwise, you can build Berkeley DB [yourself](#berkeley-db). To build Bitcoin Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode) @@ -225,8 +218,10 @@ turned off by default. See the configure options for NAT-PMP behavior desired: Berkeley DB ----------- -It is recommended to use Berkeley DB 4.8. If you have to build it yourself, -you can use [the installation script included in contrib/](/contrib/install_db4.sh) + +The legacy wallet uses Berkeley DB. To ensure backwards compatibility it is +recommended to use Berkeley DB 4.8. If you have to build it yourself, you can +use [the installation script included in contrib/](/contrib/install_db4.sh) like so: ```shell @@ -239,15 +234,6 @@ Otherwise, you can build Bitcoin Core from self-compiled [depends](/depends/READ **Note**: You only need Berkeley DB if the wallet is enabled (see [*Disable-wallet mode*](#disable-wallet-mode)). -Boost ------ -If you need to build Boost yourself: - - sudo su - ./bootstrap.sh - ./bjam install - - Security -------- To help make your Bitcoin Core installation more secure by making certain attacks impossible to diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index e6ea8bdf61..35d5b0004a 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -391,7 +391,7 @@ QT_BASE_TLD = $(shell tar tf $(QT_BASE_PATH) --exclude='*/*') bitcoin_qt_apk: FORCE mkdir -p $(APK_LIB_DIR) - cp $(dir $(CC))../sysroot/usr/lib/$(host_alias)/libc++_shared.so $(APK_LIB_DIR) + cp $(dir $(lastword $(CC)))../sysroot/usr/lib/$(host_alias)/libc++_shared.so $(APK_LIB_DIR) tar xf $(QT_BASE_PATH) -C qt/android/src/ $(QT_BASE_TLD)src/android/jar/src --strip-components=5 tar xf $(QT_BASE_PATH) -C qt/android/src/ $(QT_BASE_TLD)src/android/java/src --strip-components=5 tar xf $(QT_BASE_PATH) -C qt/android/res/ $(QT_BASE_TLD)src/android/java/res --strip-components=5 diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 27f9382631..7df1ecb25d 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -139,6 +139,7 @@ BITCOIN_TESTS =\ test/transaction_tests.cpp \ test/txindex_tests.cpp \ test/txrequest_tests.cpp \ + test/txpackage_tests.cpp \ test/txvalidation_tests.cpp \ test/txvalidationcache_tests.cpp \ test/uint256_tests.cpp \ @@ -170,6 +171,10 @@ if USE_BDB BITCOIN_TESTS += wallet/test/db_tests.cpp endif +if USE_SQLITE +FUZZ_WALLET_SRC = \ + wallet/test/fuzz/notifications.cpp +endif # USE_SQLITE BITCOIN_TEST_SUITE += \ wallet/test/util.cpp \ @@ -178,7 +183,7 @@ BITCOIN_TEST_SUITE += \ wallet/test/wallet_test_fixture.h \ wallet/test/init_test_fixture.cpp \ wallet/test/init_test_fixture.h -endif +endif # ENABLE_WALLET test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(EVENT_CFLAGS) @@ -199,14 +204,13 @@ test_test_bitcoin_LDADD += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS) FUZZ_SUITE_LD_COMMON += $(LIBBITCOIN_ZMQ) $(ZMQ_LIBS) endif -FUZZ_SUITE_LDFLAGS_COMMON = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) - if ENABLE_FUZZ_BINARY test_fuzz_fuzz_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_fuzz_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_fuzz_LDADD = $(FUZZ_SUITE_LD_COMMON) -test_fuzz_fuzz_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) $(RUNTIME_LDFLAGS) +test_fuzz_fuzz_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS) $(RUNTIME_LDFLAGS) test_fuzz_fuzz_SOURCES = \ + $(FUZZ_WALLET_SRC) \ test/fuzz/addition_overflow.cpp \ test/fuzz/addrman.cpp \ test/fuzz/asmap.cpp \ diff --git a/src/addrman.cpp b/src/addrman.cpp index b579995a2c..825b5239c8 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -537,69 +537,13 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId) info.fInTried = true; } -void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) -{ - AssertLockHeld(cs); - - int nId; - - nLastGood = nTime; - - AddrInfo* pinfo = Find(addr, &nId); - - // if not found, bail out - if (!pinfo) - return; - - AddrInfo& info = *pinfo; - - // update info - info.nLastSuccess = nTime; - info.nLastTry = nTime; - info.nAttempts = 0; - // nTime is not updated here, to avoid leaking information about - // currently-connected peers. - - // if it is already in the tried set, don't do anything else - if (info.fInTried) - return; - - // if it is not in new, something bad happened - if (!Assume(info.nRefCount > 0)) { - return; - } - - // which tried bucket to move the entry to - int tried_bucket = info.GetTriedBucket(nKey, m_asmap); - int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); - - // Will moving this address into tried evict another entry? - if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) { - if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) { - m_tried_collisions.insert(nId); - } - // Output the entry we'd be colliding with, for debugging purposes - auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]); - LogPrint(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n", - colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "", - addr.ToString(), - m_tried_collisions.size()); - } else { - // move nId to the tried tables - MakeTried(info, nId); - LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", - addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos); - } -} - -bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) +bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) { AssertLockHeld(cs); if (!addr.IsRoutable()) return false; - bool fNew = false; int nId; AddrInfo* pinfo = Find(addr, &nId); @@ -640,13 +584,12 @@ bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTi pinfo = Create(addr, source, &nId); pinfo->nTime = std::max((int64_t)0, (int64_t)pinfo->nTime - nTimePenalty); nNew++; - fNew = true; } int nUBucket = pinfo->GetNewBucket(nKey, source, m_asmap); int nUBucketPos = pinfo->GetBucketPosition(nKey, true, nUBucket); + bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (vvNew[nUBucket][nUBucketPos] != nId) { - bool fInsert = vvNew[nUBucket][nUBucketPos] == -1; if (!fInsert) { AddrInfo& infoExisting = mapInfo[vvNew[nUBucket][nUBucketPos]]; if (infoExisting.IsTerrible() || (infoExisting.nRefCount > 1 && pinfo->nRefCount == 0)) { @@ -666,7 +609,74 @@ bool AddrManImpl::Add_(const CAddress& addr, const CNetAddr& source, int64_t nTi } } } - return fNew; + return fInsert; +} + +void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nTime) +{ + AssertLockHeld(cs); + + int nId; + + nLastGood = nTime; + + AddrInfo* pinfo = Find(addr, &nId); + + // if not found, bail out + if (!pinfo) + return; + + AddrInfo& info = *pinfo; + + // update info + info.nLastSuccess = nTime; + info.nLastTry = nTime; + info.nAttempts = 0; + // nTime is not updated here, to avoid leaking information about + // currently-connected peers. + + // if it is already in the tried set, don't do anything else + if (info.fInTried) + return; + + // if it is not in new, something bad happened + if (!Assume(info.nRefCount > 0)) { + return; + } + + // which tried bucket to move the entry to + int tried_bucket = info.GetTriedBucket(nKey, m_asmap); + int tried_bucket_pos = info.GetBucketPosition(nKey, false, tried_bucket); + + // Will moving this address into tried evict another entry? + if (test_before_evict && (vvTried[tried_bucket][tried_bucket_pos] != -1)) { + if (m_tried_collisions.size() < ADDRMAN_SET_TRIED_COLLISION_SIZE) { + m_tried_collisions.insert(nId); + } + // Output the entry we'd be colliding with, for debugging purposes + auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]); + LogPrint(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n", + colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "", + addr.ToString(), + m_tried_collisions.size()); + } else { + // move nId to the tried tables + MakeTried(info, nId); + LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n", + addr.ToString(), addr.GetMappedAS(m_asmap), tried_bucket, tried_bucket_pos); + } +} + +bool AddrManImpl::Add_(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty) +{ + int added{0}; + for (std::vector<CAddress>::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) { + added += AddSingle(*it, source, nTimePenalty) ? 1 : 0; + } + if (added > 0) { + LogPrint(BCLog::ADDRMAN, "Added %i addresses (of %i) from %s: %i tried, %i new\n", added, vAddr.size(), source.ToString(), nTried, nNew); + } + return added > 0; } void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) @@ -1031,15 +1041,10 @@ size_t AddrManImpl::size() const bool AddrManImpl::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty) { LOCK(cs); - int nAdd = 0; Check(); - for (std::vector<CAddress>::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) - nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; + auto ret = Add_(vAddr, source, nTimePenalty); Check(); - if (nAdd) { - LogPrint(BCLog::ADDRMAN, "Added %i addresses from %s: %i tried, %i new\n", nAdd, source.ToString(), nTried, nNew); - } - return nAdd > 0; + return ret; } void AddrManImpl::Good(const CService& addr, int64_t nTime) diff --git a/src/addrman.h b/src/addrman.h index 174ab4f811..a9f697f66f 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -69,7 +69,15 @@ public: //! Return the number of (unique) addresses in all tables. size_t size() const; - //! Add addresses to addrman's new table. + /** + * Attempt to add one or more addresses to addrman's new table. + * + * @param[in] vAddr Address records to attempt to add. + * @param[in] source The address of the node that sent us these addr records. + * @param[in] nTimePenalty A "time penalty" to apply to the address record's nTime. If a peer + * sends us an address record with nTime=n, then we'll add it to our + * addrman with nTime=(n - nTimePenalty). + * @return true if at least one address is successfully added. */ bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty = 0); //! Mark an entry as accessible, possibly moving it from "new" to "tried". diff --git a/src/addrman_impl.h b/src/addrman_impl.h index e8e7c15a19..e097932ee8 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -243,9 +243,13 @@ private: //! Move an entry from the "new" table(s) to the "tried" table void MakeTried(AddrInfo& info, int nId) EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Attempt to add a single address to addrman's new table. + * @see AddrMan::Add() for parameters. */ + bool AddSingle(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); + void Good_(const CService& addr, bool test_before_evict, int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool Add_(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); + bool Add_(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty) EXCLUSIVE_LOCKS_REQUIRED(cs); void Attempt_(const CService& addr, bool fCountFailure, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index 0b43ea1fd5..83609d2787 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -24,8 +24,8 @@ static void SetupBenchArgs(ArgsManager& argsman) argsman.AddArg("-asymptote=<n1,n2,n3,...>", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); - argsman.AddArg("-min_time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS); + argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-min_time=<milliseconds>", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-output_csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-output_json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index b6344ec413..338d32ef6b 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -74,7 +74,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-netinfo", "Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); SetupChainParamsBaseOptions(argsman); - argsman.AddArg("-color=<when>", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS); + argsman.AddArg("-color=<when>", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcclienttimeout=<n>", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -83,7 +83,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwait", "Wait for RPC server to start", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-rpcwaittimeout=<n>", strprintf("Timeout in seconds to wait for the RPC server to start, or 0 for no timeout. (default: %d)", DEFAULT_WAIT_CLIENT_TIMEOUT), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS); + argsman.AddArg("-rpcwaittimeout=<n>", strprintf("Timeout in seconds to wait for the RPC server to start, or 0 for no timeout. (default: %d)", DEFAULT_WAIT_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwallet=<walletname>", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind). This changes the RPC endpoint used, e.g. http://127.0.0.1:8332/wallet/<walletname>", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdinrpcpass", "Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password. When combined with -stdinwalletpassphrase, -stdinrpcpass consumes the first line, and -stdinwalletpassphrase consumes the second.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 77e0bd9a16..afa5b99549 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -28,10 +28,10 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); - argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS); + argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); - argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); + argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index dc484f5c03..f24863f419 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -24,8 +24,8 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-testnet", "Use the test chain. Equivalent to -chain=test.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_STRING, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-signetseednode", "Specify a seed node for the signet network, in the hostname[:port] format, e.g. sig.net:1234 (may be used multiple times to specify multiple seed nodes; defaults to the global default signet test network seed node(s))", ArgsManager::ALLOW_STRING, OptionsCategory::CHAINPARAMS); + argsman.AddArg("-signetchallenge", "Blocks must satisfy the given script to be considered valid (only for signet networks; defaults to the global default signet test network challenge)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS); + argsman.AddArg("-signetseednode", "Specify a seed node for the signet network, in the hostname[:port] format, e.g. sig.net:1234 (may be used multiple times to specify multiple seed nodes; defaults to the global default signet test network seed node(s))", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::CHAINPARAMS); } static std::unique_ptr<CBaseChainParams> globalChainBaseParams; diff --git a/src/init.cpp b/src/init.cpp index 164b7bb55d..b0335183d6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -422,13 +422,14 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), signetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); + argsman.AddArg("-cjdnsreachable", "If set then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-dnsseed", strprintf("Query for peer addresses via DNS lookup, if low on addresses (default: %u unless -connect used)", DEFAULT_DNSSEED), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); + argsman.AddArg("-dnsseed", strprintf("Query for peer addresses via DNS lookup, if low on addresses (default: %u unless -connect used)", DEFAULT_DNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-externalip=<ip>", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); - argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); + argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -438,7 +439,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-i2psam=<ip:port>", "I2P SAM proxy to reach I2P peers and accept I2P connections (default: none)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-i2pacceptincoming", "If set and -i2psam is also set then incoming I2P connections are accepted via the SAM proxy. If this is not set but -i2psam is set then only outgoing connections will be made to the I2P network. Ignored if -i2psam is not set. Listening for incoming I2P connections is done through the SAM proxy, not by binding to a local address and port (default: 1)", ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); + argsman.AddArg("-i2pacceptincoming", "If set and -i2psam is also set then incoming I2P connections are accepted via the SAM proxy. If this is not set but -i2psam is set then only outgoing connections will be made to the I2P network. Ignored if -i2psam is not set. Listening for incoming I2P connections is done through the SAM proxy, not by binding to a local address and port (default: 1)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (" + Join(GetNetworkNames(), ", ") + "). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with non-onion networks and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -447,7 +448,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); + argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-timeout=<n>", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=<n>", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -462,7 +463,7 @@ void SetupServerArgs(ArgsManager& argsman) hidden_args.emplace_back("-upnp"); #endif #ifdef USE_NATPMP - argsman.AddArg("-natpmp", strprintf("Use NAT-PMP to map the listening port (default: %s)", DEFAULT_NATPMP ? "1 when listening and no -proxy" : "0"), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); + argsman.AddArg("-natpmp", strprintf("Use NAT-PMP to map the listening port (default: %s)", DEFAULT_NATPMP ? "1 when listening and no -proxy" : "0"), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #else hidden_args.emplace_back("-natpmp"); #endif // USE_NATPMP @@ -514,7 +515,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-limitdescendantcount=<n>", strprintf("Do not accept transactions if any ancestor would have <n> or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-addrmantest", "Allows to test address relay on localhost", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_BOOL | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-maxtipage=<n>", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); @@ -551,13 +552,13 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcwhitelist=<whitelist>", "Set a whitelist to filter incoming RPC calls for a specific user. The field <whitelist> comes in the format: <USERNAME>:<rpc 1>,<rpc 2>,...,<rpc n>. If multiple whitelists are set for a given user, they are set-intersected. See -rpcwhitelistdefault documentation for information on default whitelist behavior.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); - argsman.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_BOOL, OptionsCategory::RPC); + argsman.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); #if HAVE_DECL_FORK - argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); - argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); + argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #else hidden_args.emplace_back("-daemon"); hidden_args.emplace_back("-daemonwait"); @@ -1294,6 +1295,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } + if (!args.IsArgSet("-cjdnsreachable")) { + SetReachable(NET_CJDNS, false); + } + // Now IsReachable(NET_CJDNS) is true if: + // 1. -cjdnsreachable is given and + // 2.1. -onlynet is not given or + // 2.2. -onlynet=cjdns is given + // Check for host lookup allowed before parsing any network related parameters fNameLookup = args.GetBoolArg("-dns", DEFAULT_NAME_LOOKUP); @@ -1315,6 +1324,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) SetProxy(NET_IPV4, addrProxy); SetProxy(NET_IPV6, addrProxy); SetProxy(NET_ONION, addrProxy); + SetProxy(NET_CJDNS, addrProxy); SetNameProxy(addrProxy); SetReachable(NET_ONION, true); // by default, -proxy sets onion as reachable, unless -noonion later } diff --git a/src/key.cpp b/src/key.cpp index 39155e4311..7688254515 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -229,6 +229,12 @@ bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool gr assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_pubkey pk; + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, begin()); + assert(ret); + ret = secp256k1_ecdsa_verify(GetVerifyContext(), &sig, hash.begin(), &pk); + assert(ret); return true; } @@ -251,13 +257,21 @@ bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) return false; vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE); int rec = -1; - secp256k1_ecdsa_recoverable_signature sig; - int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); + secp256k1_ecdsa_recoverable_signature rsig; + int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); - ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig); + ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &rsig); assert(ret); assert(rec != -1); vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_pubkey epk, rpk; + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, begin()); + assert(ret); + ret = secp256k1_ecdsa_recover(GetVerifyContext(), &rpk, &rsig, hash.begin()); + assert(ret); + ret = secp256k1_ec_pubkey_cmp(GetVerifyContext(), &epk, &rpk); + assert(ret == 0); return true; } @@ -275,6 +289,13 @@ bool CKey::SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint2 if (!secp256k1_keypair_xonly_tweak_add(GetVerifyContext(), &keypair, tweak.data())) return false; } bool ret = secp256k1_schnorrsig_sign(secp256k1_context_sign, sig.data(), hash.data(), &keypair, aux ? (unsigned char*)aux->data() : nullptr); + if (ret) { + // Additional verification step to prevent using a potentially corrupted signature + secp256k1_xonly_pubkey pubkey_verify; + ret = secp256k1_keypair_xonly_pub(GetVerifyContext(), &pubkey_verify, nullptr, &keypair); + ret &= secp256k1_schnorrsig_verify(GetVerifyContext(), sig.data(), hash.begin(), 32, &pubkey_verify); + } + if (!ret) memory_cleanse(sig.data(), sig.size()); memory_cleanse(&keypair, sizeof(keypair)); return ret; } @@ -319,10 +340,11 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { return key.Derive(out.key, out.chaincode, _nChild, chaincode); } -void CExtKey::SetSeed(const unsigned char *seed, unsigned int nSeedLen) { +void CExtKey::SetSeed(Span<const uint8_t> seed) +{ static const unsigned char hashkey[] = {'B','i','t','c','o','i','n',' ','s','e','e','d'}; std::vector<unsigned char, secure_allocator<unsigned char>> vout(64); - CHMAC_SHA512(hashkey, sizeof(hashkey)).Write(seed, nSeedLen).Finalize(vout.data()); + CHMAC_SHA512{hashkey, sizeof(hashkey)}.Write(seed.data(), seed.size()).Finalize(vout.data()); key.Set(vout.data(), vout.data() + 32, true); memcpy(chaincode.begin(), vout.data() + 32, 32); nDepth = 0; @@ -85,6 +85,7 @@ public: //! Simple read-only vector-like interface. unsigned int size() const { return (fValid ? keydata.size() : 0); } + const unsigned char* data() const { return keydata.data(); } const unsigned char* begin() const { return keydata.data(); } const unsigned char* end() const { return keydata.data() + size(); } @@ -177,7 +178,7 @@ struct CExtKey { void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]); bool Derive(CExtKey& out, unsigned int nChild) const; CExtPubKey Neuter() const; - void SetSeed(const unsigned char* seed, unsigned int nSeedLen); + void SetSeed(Span<const uint8_t> seed); }; /** Initialize the elliptic curve support. May not be called twice without calling ECC_Stop first. */ diff --git a/src/net.cpp b/src/net.cpp index ad558dd598..82e55d4189 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -230,9 +230,27 @@ std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode) return std::nullopt; } +/** + * If an IPv6 address belongs to the address range used by the CJDNS network and + * the CJDNS network is reachable (-cjdnsreachable config is set), then change + * the type from NET_IPV6 to NET_CJDNS. + * @param[in] service Address to potentially convert. + * @return a copy of `service` either unmodified or changed to CJDNS. + */ +CService MaybeFlipIPv6toCJDNS(const CService& service) +{ + CService ret{service}; + if (ret.m_net == NET_IPV6 && ret.m_addr[0] == 0xfc && IsReachable(NET_CJDNS)) { + ret.m_net = NET_CJDNS; + } + return ret; +} + // learn a new local address -bool AddLocal(const CService& addr, int nScore) +bool AddLocal(const CService& addr_, int nScore) { + CService addr{MaybeFlipIPv6toCJDNS(addr_)}; + if (!addr.IsRoutable()) return false; @@ -409,7 +427,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo if (pszDest) { std::vector<CService> resolved; if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) { - addrConnect = CAddress(resolved[GetRand(resolved.size())], NODE_NONE); + const CService rnd{resolved[GetRand(resolved.size())]}; + addrConnect = CAddress{MaybeFlipIPv6toCJDNS(rnd), NODE_NONE}; if (!addrConnect.IsValid()) { LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToString(), pszDest); return nullptr; @@ -626,25 +645,26 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete) if (m_deserializer->Complete()) { // decompose a transport agnostic CNetMessage from the deserializer - uint32_t out_err_raw_size{0}; - std::optional<CNetMessage> result{m_deserializer->GetMessage(time, out_err_raw_size)}; - if (!result) { + bool reject_message{false}; + CNetMessage msg = m_deserializer->GetMessage(time, reject_message); + if (reject_message) { // Message deserialization failed. Drop the message but don't disconnect the peer. // store the size of the corrupt message - mapRecvBytesPerMsgCmd.find(NET_MESSAGE_COMMAND_OTHER)->second += out_err_raw_size; + mapRecvBytesPerMsgCmd.at(NET_MESSAGE_COMMAND_OTHER) += msg.m_raw_message_size; continue; } - //store received bytes per message command - //to prevent a memory DOS, only allow valid commands - mapMsgCmdSize::iterator i = mapRecvBytesPerMsgCmd.find(result->m_command); - if (i == mapRecvBytesPerMsgCmd.end()) + // Store received bytes per message command + // to prevent a memory DOS, only allow valid commands + auto i = mapRecvBytesPerMsgCmd.find(msg.m_command); + if (i == mapRecvBytesPerMsgCmd.end()) { i = mapRecvBytesPerMsgCmd.find(NET_MESSAGE_COMMAND_OTHER); + } assert(i != mapRecvBytesPerMsgCmd.end()); - i->second += result->m_raw_message_size; + i->second += msg.m_raw_message_size; // push the message to the process queue, - vRecvMsg.push_back(std::move(*result)); + vRecvMsg.push_back(std::move(msg)); complete = true; } @@ -718,16 +738,18 @@ const uint256& V1TransportDeserializer::GetMessageHash() const return data_hash; } -std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono::microseconds time, uint32_t& out_err_raw_size) +CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds time, bool& reject_message) { + // Initialize out parameter + reject_message = false; // decompose a single CNetMessage from the TransportDeserializer - std::optional<CNetMessage> msg(std::move(vRecv)); + CNetMessage msg(std::move(vRecv)); // store command string, time, and sizes - msg->m_command = hdr.GetCommand(); - msg->m_time = time; - msg->m_message_size = hdr.nMessageSize; - msg->m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE; + msg.m_command = hdr.GetCommand(); + msg.m_time = time; + msg.m_message_size = hdr.nMessageSize; + msg.m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE; uint256 hash = GetMessageHash(); @@ -737,17 +759,15 @@ std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono // Check checksum and header command string if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) { LogPrint(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n", - SanitizeString(msg->m_command), msg->m_message_size, + SanitizeString(msg.m_command), msg.m_message_size, HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)), HexStr(hdr.pchChecksum), m_node_id); - out_err_raw_size = msg->m_raw_message_size; - msg = std::nullopt; + reject_message = true; } else if (!hdr.IsCommandValid()) { LogPrint(BCLog::NET, "Header error: Invalid message type (%s, %u bytes), peer=%d\n", - SanitizeString(hdr.GetCommand()), msg->m_message_size, m_node_id); - out_err_raw_size = msg->m_raw_message_size; - msg.reset(); + SanitizeString(hdr.GetCommand()), msg.m_message_size, m_node_id); + reject_message = true; } // Always reset the network deserializer (prepare for the next message) @@ -1091,9 +1111,11 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr)) { LogPrintf("Warning: Unknown socket family\n"); + } else { + addr = CAddress{MaybeFlipIPv6toCJDNS(addr), NODE_NONE}; } - const CAddress addr_bind = GetBindAddress(hSocket); + const CAddress addr_bind{MaybeFlipIPv6toCJDNS(GetBindAddress(hSocket)), NODE_NONE}; NetPermissionFlags permissionFlags = NetPermissionFlags::None; hListenSocket.AddSocketPermissionFlags(permissionFlags); @@ -2459,7 +2481,10 @@ NodeId CConnman::GetNewNodeId() } -bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags permissions) { +bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlags permissions) +{ + const CService addr{MaybeFlipIPv6toCJDNS(addr_)}; + if (!(flags & BF_EXPLICIT) && !IsReachable(addr)) { return false; } @@ -2980,7 +3005,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const LogPrint(BCLog::NET, "Added connection peer=%d\n", id); } - m_deserializer = std::make_unique<V1TransportDeserializer>(V1TransportDeserializer(Params(), GetId(), SER_NETWORK, INIT_PROTO_VERSION)); + m_deserializer = std::make_unique<V1TransportDeserializer>(V1TransportDeserializer(Params(), id, SER_NETWORK, INIT_PROTO_VERSION)); m_serializer = std::make_unique<V1TransportSerializer>(V1TransportSerializer()); } @@ -308,7 +308,7 @@ public: /** read and deserialize data, advances msg_bytes data pointer */ virtual int Read(Span<const uint8_t>& msg_bytes) = 0; // decomposes a message from the context - virtual std::optional<CNetMessage> GetMessage(std::chrono::microseconds time, uint32_t& out_err) = 0; + virtual CNetMessage GetMessage(std::chrono::microseconds time, bool& reject_message) = 0; virtual ~TransportDeserializer() {} }; @@ -372,7 +372,7 @@ public: } return ret; } - std::optional<CNetMessage> GetMessage(std::chrono::microseconds time, uint32_t& out_err_raw_size) override; + CNetMessage GetMessage(std::chrono::microseconds time, bool& reject_message) override; }; /** The TransportSerializer prepares messages for the network transport diff --git a/src/netaddress.cpp b/src/netaddress.cpp index f9fff5a6d5..7f1dd698b0 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -663,7 +663,7 @@ bool CNetAddr::GetInAddr(struct in_addr* pipv4Addr) const } /** - * Try to get our IPv6 address. + * Try to get our IPv6 (or CJDNS) address. * * @param[out] pipv6Addr The in6_addr struct to which to copy. * @@ -674,7 +674,7 @@ bool CNetAddr::GetInAddr(struct in_addr* pipv4Addr) const */ bool CNetAddr::GetIn6Addr(struct in6_addr* pipv6Addr) const { - if (!IsIPv6()) { + if (!IsIPv6() && !IsCJDNS()) { return false; } assert(sizeof(*pipv6Addr) == m_addr.size()); @@ -794,8 +794,14 @@ std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &asmap) co vchRet.push_back((ipv4 >> 24) & 0xFF); vchRet.push_back((ipv4 >> 16) & 0xFF); return vchRet; - } else if (IsTor() || IsI2P() || IsCJDNS()) { + } else if (IsTor() || IsI2P()) { nBits = 4; + } else if (IsCJDNS()) { + // Treat in the same way as Tor and I2P because the address in all of + // them is "random" bytes (derived from a public key). However in CJDNS + // the first byte is a constant 0xfc, so the random bytes come after it. + // Thus skip the constant 8 bits at the start. + nBits = 12; } else if (IsHeNet()) { // for he.net, use /36 groups nBits = 36; @@ -892,6 +898,11 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const case NET_I2P: return REACH_PRIVATE; default: return REACH_DEFAULT; } + case NET_CJDNS: + switch (ourNet) { + case NET_CJDNS: return REACH_PRIVATE; + default: return REACH_DEFAULT; + } case NET_TEREDO: switch(ourNet) { default: return REACH_DEFAULT; @@ -993,7 +1004,7 @@ bool CService::GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const paddrin->sin_port = htons(port); return true; } - if (IsIPv6()) { + if (IsIPv6() || IsCJDNS()) { if (*addrlen < (socklen_t)sizeof(struct sockaddr_in6)) return false; *addrlen = sizeof(struct sockaddr_in6); diff --git a/src/netaddress.h b/src/netaddress.h index 57eb8bc72f..b0b1c5ca9e 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -224,7 +224,7 @@ public: */ bool IsRelayable() const { - return IsIPv4() || IsIPv6() || IsTor() || IsI2P(); + return IsIPv4() || IsIPv6() || IsTor() || IsI2P() || IsCJDNS(); } /** @@ -550,6 +550,7 @@ public: } friend class CServiceHash; + friend CService MaybeFlipIPv6toCJDNS(const CService& service); }; class CServiceHash diff --git a/src/netbase.cpp b/src/netbase.cpp index 64d17189a6..6191f25cd9 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -96,6 +96,9 @@ enum Network ParseNetwork(const std::string& net_in) { if (net == "i2p") { return NET_I2P; } + if (net == "cjdns") { + return NET_CJDNS; + } return NET_UNROUTABLE; } @@ -120,7 +123,7 @@ std::vector<std::string> GetNetworkNames(bool append_unroutable) std::vector<std::string> names; for (int n = 0; n < NET_MAX; ++n) { const enum Network network{static_cast<Network>(n)}; - if (network == NET_UNROUTABLE || network == NET_CJDNS || network == NET_INTERNAL) continue; + if (network == NET_UNROUTABLE || network == NET_INTERNAL) continue; names.emplace_back(GetNetworkName(network)); } if (append_unroutable) { diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp index 25b9282b4e..ce149067b7 100644 --- a/src/policy/feerate.cpp +++ b/src/policy/feerate.cpp @@ -7,6 +7,8 @@ #include <tinyformat.h> +#include <cmath> + CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes) { const int64_t nSize{num_bytes}; @@ -22,7 +24,9 @@ CAmount CFeeRate::GetFee(uint32_t num_bytes) const { const int64_t nSize{num_bytes}; - CAmount nFee = nSatoshisPerK * nSize / 1000; + // Be explicit that we're converting from a double to int64_t (CAmount) here. + // We've previously had issues with the silent double->int64_t conversion. + CAmount nFee{static_cast<CAmount>(std::ceil(nSatoshisPerK * nSize / 1000.0))}; if (nFee == 0 && nSize != 0) { if (nSatoshisPerK > 0) nFee = CAmount(1); diff --git a/src/policy/feerate.h b/src/policy/feerate.h index b16f3f8251..8ba896bb01 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -48,6 +48,7 @@ public: CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes); /** * Return the fee in satoshis for the given size in bytes. + * If the calculated fee would have fractional satoshis, then the returned fee will always be rounded up to the nearest satoshi. */ CAmount GetFee(uint32_t num_bytes) const; /** diff --git a/src/pubkey.h b/src/pubkey.h index 861a2cf500..f174ad8d85 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -230,8 +230,8 @@ public: XOnlyPubKey& operator=(const XOnlyPubKey&) = default; /** Determine if this pubkey is fully valid. This is true for approximately 50% of all - * possible 32-byte arrays. If false, VerifySchnorr and CreatePayToContract will always - * fail. */ + * possible 32-byte arrays. If false, VerifySchnorr, CheckTapTweak and CreateTapTweak + * will always fail. */ bool IsFullyValid() const; /** Test whether this is the 0 key (the result of default construction). This implies diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index a9bee33c5d..e33f1ce4a3 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -566,7 +566,7 @@ static UniValue GetNetworksInfo() UniValue networks(UniValue::VARR); for (int n = 0; n < NET_MAX; ++n) { enum Network network = static_cast<enum Network>(n); - if (network == NET_UNROUTABLE || network == NET_CJDNS || network == NET_INTERNAL) continue; + if (network == NET_UNROUTABLE || network == NET_INTERNAL) continue; proxyType proxy; UniValue obj(UniValue::VOBJ); GetProxy(network, proxy); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 483717aa7a..fd18d4c96d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -977,7 +977,7 @@ static RPCHelpMan testmempoolaccept() if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { const CAmount fee = tx_result.m_base_fees.value(); // Check that fee does not exceed maximum fee - const int64_t virtual_size = GetVirtualTransactionSize(*tx); + const int64_t virtual_size = tx_result.m_vsize.value(); const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); if (max_raw_tx_fee && fee > max_raw_tx_fee) { result_inner.pushKV("allowed", false); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 991bfa5efc..eabc11c467 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -347,7 +347,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) //Test: tried table collision! CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); uint32_t collisions{1}; - BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK(!addrman.Add({CAddress(addr1, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp index 114fe3907c..aa23d71671 100644 --- a/src/test/amount_tests.cpp +++ b/src/test/amount_tests.cpp @@ -48,13 +48,13 @@ BOOST_AUTO_TEST_CASE(GetFeeTest) BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), CAmount(-9e3)); feeRate = CFeeRate(123); - // Truncates the result, if not integer + // Rounds up the result, if not integer BOOST_CHECK_EQUAL(feeRate.GetFee(0), CAmount(0)); BOOST_CHECK_EQUAL(feeRate.GetFee(8), CAmount(1)); // Special case: returns 1 instead of 0 - BOOST_CHECK_EQUAL(feeRate.GetFee(9), CAmount(1)); - BOOST_CHECK_EQUAL(feeRate.GetFee(121), CAmount(14)); - BOOST_CHECK_EQUAL(feeRate.GetFee(122), CAmount(15)); - BOOST_CHECK_EQUAL(feeRate.GetFee(999), CAmount(122)); + BOOST_CHECK_EQUAL(feeRate.GetFee(9), CAmount(2)); + BOOST_CHECK_EQUAL(feeRate.GetFee(121), CAmount(15)); + BOOST_CHECK_EQUAL(feeRate.GetFee(122), CAmount(16)); + BOOST_CHECK_EQUAL(feeRate.GetFee(999), CAmount(123)); BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), CAmount(123)); BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), CAmount(1107)); diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index a89868e1ef..0fa6b7784f 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -124,7 +124,7 @@ void RunTest(const TestVector &test) { std::vector<unsigned char> seed = ParseHex(test.strHexMaster); CExtKey key; CExtPubKey pubkey; - key.SetSeed(seed.data(), seed.size()); + key.SetSeed(seed); pubkey = key.Neuter(); for (const TestDerivation &derive : test.vDerive) { unsigned char data[74]; diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 64c6d7f634..4d34407ca8 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -18,7 +18,17 @@ #include <utility> #include <vector> -BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, TestingSetup) +/** + * Identical to TestingSetup but excludes lock contention logging, as some of + * these tests are designed to be heavily contested to trigger race conditions + * or other issues. + */ +struct NoLockLoggingTestingSetup : public TestingSetup { + NoLockLoggingTestingSetup() + : TestingSetup{CBaseChainParams::MAIN, /*extra_args=*/{"-debugexclude=lock"}} {} +}; + +BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, NoLockLoggingTestingSetup) static const unsigned int QUEUE_BATCH_SIZE = 128; static const int SCRIPT_CHECK_THREADS = 3; diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index edee5aeef7..29b7223c90 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -68,18 +68,16 @@ FUZZ_TARGET_INIT(p2p_transport_serialization, initialize_p2p_transport_serializa } if (deserializer.Complete()) { const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()}; - uint32_t out_err_raw_size{0}; - std::optional<CNetMessage> result{deserializer.GetMessage(m_time, out_err_raw_size)}; - if (result) { - assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE); - assert(result->m_raw_message_size <= mutable_msg_bytes.size()); - assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size); - assert(result->m_time == m_time); + bool reject_message{false}; + CNetMessage msg = deserializer.GetMessage(m_time, reject_message); + assert(msg.m_command.size() <= CMessageHeader::COMMAND_SIZE); + assert(msg.m_raw_message_size <= mutable_msg_bytes.size()); + assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size); + assert(msg.m_time == m_time); - std::vector<unsigned char> header; - auto msg = CNetMsgMaker{result->m_recv.GetVersion()}.Make(result->m_command, MakeUCharSpan(result->m_recv)); - serializer.prepareForTransport(msg, header); - } + std::vector<unsigned char> header; + auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_command, MakeUCharSpan(msg.m_recv)); + serializer.prepareForTransport(msg2, header); } } } diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 17e904fcff..b0c8068ab9 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -194,8 +194,8 @@ BOOST_AUTO_TEST_CASE(boolargno) BOOST_AUTO_TEST_CASE(logargs) { - const auto okaylog_bool = std::make_pair("-okaylog-bool", ArgsManager::ALLOW_BOOL); - const auto okaylog_negbool = std::make_pair("-okaylog-negbool", ArgsManager::ALLOW_BOOL); + const auto okaylog_bool = std::make_pair("-okaylog-bool", ArgsManager::ALLOW_ANY); + const auto okaylog_negbool = std::make_pair("-okaylog-negbool", ArgsManager::ALLOW_ANY); const auto okaylog = std::make_pair("-okaylog", ArgsManager::ALLOW_ANY); const auto dontlog = std::make_pair("-dontlog", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE); SetupArgs({okaylog_bool, okaylog_negbool, okaylog, dontlog}); diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp index 8629d13840..0361618c82 100644 --- a/src/test/key_io_tests.cpp +++ b/src/test/key_io_tests.cpp @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse) privkey = DecodeSecret(exp_base58string); BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest); BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest); - BOOST_CHECK_MESSAGE(privkey.size() == exp_payload.size() && std::equal(privkey.begin(), privkey.end(), exp_payload.begin()), "key mismatch:" + strTest); + BOOST_CHECK_MESSAGE(Span<const uint8_t>{privkey} == Span<const uint8_t>{exp_payload}, "key mismatch:" + strTest); // Private key must be invalid public key destination = DecodeDestination(exp_base58string); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 687d2f6747..b6d7496cc7 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -339,11 +339,13 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork) BOOST_CHECK_EQUAL(ParseNetwork("ipv6"), NET_IPV6); BOOST_CHECK_EQUAL(ParseNetwork("onion"), NET_ONION); BOOST_CHECK_EQUAL(ParseNetwork("tor"), NET_ONION); + BOOST_CHECK_EQUAL(ParseNetwork("cjdns"), NET_CJDNS); BOOST_CHECK_EQUAL(ParseNetwork("IPv4"), NET_IPV4); BOOST_CHECK_EQUAL(ParseNetwork("IPv6"), NET_IPV6); BOOST_CHECK_EQUAL(ParseNetwork("ONION"), NET_ONION); BOOST_CHECK_EQUAL(ParseNetwork("TOR"), NET_ONION); + BOOST_CHECK_EQUAL(ParseNetwork("CJDNS"), NET_CJDNS); BOOST_CHECK_EQUAL(ParseNetwork(":)"), NET_UNROUTABLE); BOOST_CHECK_EQUAL(ParseNetwork("tÖr"), NET_UNROUTABLE); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index acd0151e1a..54f04d2e67 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -215,7 +215,9 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) BOOST_AUTO_TEST_CASE(streams_buffered_file) { - FILE* file = fsbridge::fopen("streams_test_tmp", "w+b"); + fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp"; + FILE* file = fsbridge::fopen(streams_test_filename, "w+b"); + // The value at each offset is the offset. for (uint8_t j = 0; j < 40; ++j) { fwrite(&j, 1, 1, file); @@ -343,7 +345,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) // We can explicitly close the file, or the destructor will do it. bf.fclose(); - fs::remove("streams_test_tmp"); + fs::remove(streams_test_filename); } BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) @@ -351,8 +353,9 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) // Make this test deterministic. SeedInsecureRand(SeedRand::ZEROS); + fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp"; for (int rep = 0; rep < 50; ++rep) { - FILE* file = fsbridge::fopen("streams_test_tmp", "w+b"); + FILE* file = fsbridge::fopen(streams_test_filename, "w+b"); size_t fileSize = InsecureRandRange(256); for (uint8_t i = 0; i < fileSize; ++i) { fwrite(&i, 1, 1, file); @@ -453,7 +456,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) maxPos = currentPos; } } - fs::remove("streams_test_tmp"); + fs::remove(streams_test_filename); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c813fbea32..252a85c282 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -810,10 +810,10 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) // nDustThreshold = 182 * 3702 / 1000 dustRelayFee = CFeeRate(3702); // dust: - t.vout[0].nValue = 673 - 1; + t.vout[0].nValue = 674 - 1; CheckIsNotStandard(t, "dust"); // not dust: - t.vout[0].nValue = 673; + t.vout[0].nValue = 674; CheckIsStandard(t); dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp new file mode 100644 index 0000000000..537a6ccea1 --- /dev/null +++ b/src/test/txpackage_tests.cpp @@ -0,0 +1,117 @@ +// Copyright (c) 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. + +#include <consensus/validation.h> +#include <key_io.h> +#include <policy/packages.h> +#include <policy/policy.h> +#include <primitives/transaction.h> +#include <script/script.h> +#include <script/standard.h> +#include <test/util/setup_common.h> +#include <validation.h> + +#include <boost/test/unit_test.hpp> + +BOOST_AUTO_TEST_SUITE(txpackage_tests) + +// Create placeholder transactions that have no meaning. +inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs) +{ + CMutableTransaction mtx = CMutableTransaction(); + mtx.vin.resize(num_inputs); + mtx.vout.resize(num_outputs); + auto random_script = CScript() << ToByteVector(InsecureRand256()) << ToByteVector(InsecureRand256()); + for (size_t i{0}; i < num_inputs; ++i) { + mtx.vin[i].prevout.hash = InsecureRand256(); + mtx.vin[i].prevout.n = 0; + mtx.vin[i].scriptSig = random_script; + } + for (size_t o{0}; o < num_outputs; ++o) { + mtx.vout[o].nValue = 1 * CENT; + mtx.vout[o].scriptPubKey = random_script; + } + return MakeTransactionRef(mtx); +} + +BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup) +{ + // Packages can't have more than 25 transactions. + Package package_too_many; + package_too_many.reserve(MAX_PACKAGE_COUNT + 1); + for (size_t i{0}; i < MAX_PACKAGE_COUNT + 1; ++i) { + package_too_many.emplace_back(create_placeholder_tx(1, 1)); + } + PackageValidationState state_too_many; + BOOST_CHECK(!CheckPackage(package_too_many, state_too_many)); + BOOST_CHECK_EQUAL(state_too_many.GetResult(), PackageValidationResult::PCKG_POLICY); + BOOST_CHECK_EQUAL(state_too_many.GetRejectReason(), "package-too-many-transactions"); + + // Packages can't have a total size of more than 101KvB. + CTransactionRef large_ptx = create_placeholder_tx(150, 150); + Package package_too_large; + auto size_large = GetVirtualTransactionSize(*large_ptx); + size_t total_size{0}; + while (total_size <= MAX_PACKAGE_SIZE * 1000) { + package_too_large.push_back(large_ptx); + total_size += size_large; + } + BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT); + PackageValidationState state_too_large; + BOOST_CHECK(!CheckPackage(package_too_large, state_too_large)); + BOOST_CHECK_EQUAL(state_too_large.GetResult(), PackageValidationResult::PCKG_POLICY); + BOOST_CHECK_EQUAL(state_too_large.GetRejectReason(), "package-too-large"); +} + +BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) +{ + LOCK(cs_main); + unsigned int initialPoolSize = m_node.mempool->size(); + + // Parent and Child Package + CKey parent_key; + parent_key.MakeNewKey(true); + CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey())); + auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* vout */ 0, + /* input_height */ 0, /* input_signing_key */ coinbaseKey, + /* output_destination */ parent_locking_script, + /* output_amount */ CAmount(49 * COIN), /* submit */ false); + CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); + + CKey child_key; + child_key.MakeNewKey(true); + CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); + auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* vout */ 0, + /* input_height */ 101, /* input_signing_key */ parent_key, + /* output_destination */ child_locking_script, + /* output_amount */ CAmount(48 * COIN), /* submit */ false); + CTransactionRef tx_child = MakeTransactionRef(mtx_child); + const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /* test_accept */ true); + BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(), + "Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason()); + auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); + auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); + BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end()); + BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), + "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason()); + BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); + BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), + "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); + + + // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. + CTransactionRef giant_ptx = create_placeholder_tx(999, 999); + BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000); + auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /* test_accept */ true); + BOOST_CHECK(result_single_large.m_state.IsInvalid()); + BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); + auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); + BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end()); + BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); + + // Check that mempool size hasn't changed. + BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index ade9e210f2..8d25f5331d 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -50,99 +50,4 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup) BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "coinbase"); BOOST_CHECK(result.m_state.GetResult() == TxValidationResult::TX_CONSENSUS); } - -// Create placeholder transactions that have no meaning. -inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs) -{ - CMutableTransaction mtx = CMutableTransaction(); - mtx.vin.resize(num_inputs); - mtx.vout.resize(num_outputs); - auto random_script = CScript() << ToByteVector(InsecureRand256()) << ToByteVector(InsecureRand256()); - for (size_t i{0}; i < num_inputs; ++i) { - mtx.vin[i].prevout.hash = InsecureRand256(); - mtx.vin[i].prevout.n = 0; - mtx.vin[i].scriptSig = random_script; - } - for (size_t o{0}; o < num_outputs; ++o) { - mtx.vout[o].nValue = 1 * CENT; - mtx.vout[o].scriptPubKey = random_script; - } - return MakeTransactionRef(mtx); -} - -BOOST_FIXTURE_TEST_CASE(package_tests, TestChain100Setup) -{ - LOCK(cs_main); - unsigned int initialPoolSize = m_node.mempool->size(); - - // Parent and Child Package - CKey parent_key; - parent_key.MakeNewKey(true); - CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey())); - auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* vout */ 0, - /* input_height */ 0, /* input_signing_key */ coinbaseKey, - /* output_destination */ parent_locking_script, - /* output_amount */ CAmount(49 * COIN), /* submit */ false); - CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); - - CKey child_key; - child_key.MakeNewKey(true); - CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); - auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* vout */ 0, - /* input_height */ 101, /* input_signing_key */ parent_key, - /* output_destination */ child_locking_script, - /* output_amount */ CAmount(48 * COIN), /* submit */ false); - CTransactionRef tx_child = MakeTransactionRef(mtx_child); - const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /* test_accept */ true); - BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(), - "Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason()); - auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); - auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); - BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), - "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason()); - BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), - "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); - - // Packages can't have more than 25 transactions. - Package package_too_many; - package_too_many.reserve(MAX_PACKAGE_COUNT + 1); - for (size_t i{0}; i < MAX_PACKAGE_COUNT + 1; ++i) { - package_too_many.emplace_back(create_placeholder_tx(1, 1)); - } - auto result_too_many = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_many, /* test_accept */ true); - BOOST_CHECK(result_too_many.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(result_too_many.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); - BOOST_CHECK_EQUAL(result_too_many.m_state.GetRejectReason(), "package-too-many-transactions"); - - // Packages can't have a total size of more than 101KvB. - CTransactionRef large_ptx = create_placeholder_tx(150, 150); - Package package_too_large; - auto size_large = GetVirtualTransactionSize(*large_ptx); - size_t total_size{0}; - while (total_size <= MAX_PACKAGE_SIZE * 1000) { - package_too_large.push_back(large_ptx); - total_size += size_large; - } - BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT); - auto result_too_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_large, /* test_accept */ true); - BOOST_CHECK(result_too_large.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(result_too_large.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); - BOOST_CHECK_EQUAL(result_too_large.m_state.GetRejectReason(), "package-too-large"); - - // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. - CTransactionRef giant_ptx = create_placeholder_tx(999, 999); - BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000); - auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /* test_accept */ true); - BOOST_CHECK(result_single_large.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); - auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); - BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end()); - BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); - - // Check that mempool size hasn't changed. - BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize); -} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 9bc2377c63..502a27dc6b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -22,6 +22,58 @@ #include <cmath> #include <optional> +// Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. +struct update_descendant_state +{ + update_descendant_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount) : + modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount) + {} + + void operator() (CTxMemPoolEntry &e) + { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); } + + private: + int64_t modifySize; + CAmount modifyFee; + int64_t modifyCount; +}; + +struct update_ancestor_state +{ + update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int64_t _modifySigOpsCost) : + modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOpsCost(_modifySigOpsCost) + {} + + void operator() (CTxMemPoolEntry &e) + { e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOpsCost); } + + private: + int64_t modifySize; + CAmount modifyFee; + int64_t modifyCount; + int64_t modifySigOpsCost; +}; + +struct update_fee_delta +{ + explicit update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } + + void operator() (CTxMemPoolEntry &e) { e.UpdateFeeDelta(feeDelta); } + +private: + int64_t feeDelta; +}; + +struct update_lock_points +{ + explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { } + + void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } + +private: + const LockPoints& lp; +}; + CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, int64_t time, unsigned int entry_height, bool spends_coinbase, int64_t sigops_cost, LockPoints lp) @@ -277,7 +329,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) { - CTxMemPoolEntry::Parents parents = it->GetMemPoolParents(); + const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst(); // add or remove this tx as a child of each parent for (const CTxMemPoolEntry& parent : parents) { UpdateChild(mapTx.iterator_to(parent), it, add); diff --git a/src/txmempool.h b/src/txmempool.h index 90b2aee371..85417ac3fc 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -159,58 +159,6 @@ public: mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms }; -// Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. -struct update_descendant_state -{ - update_descendant_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount) : - modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount) - {} - - void operator() (CTxMemPoolEntry &e) - { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); } - - private: - int64_t modifySize; - CAmount modifyFee; - int64_t modifyCount; -}; - -struct update_ancestor_state -{ - update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int64_t _modifySigOpsCost) : - modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOpsCost(_modifySigOpsCost) - {} - - void operator() (CTxMemPoolEntry &e) - { e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOpsCost); } - - private: - int64_t modifySize; - CAmount modifyFee; - int64_t modifyCount; - int64_t modifySigOpsCost; -}; - -struct update_fee_delta -{ - explicit update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateFeeDelta(feeDelta); } - -private: - int64_t feeDelta; -}; - -struct update_lock_points -{ - explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } - -private: - const LockPoints& lp; -}; - // extracts a transaction hash from CTxMemPoolEntry or CTransactionRef struct mempoolentry_txid { diff --git a/src/util/system.cpp b/src/util/system.cpp index 12d7dc49b2..99d111b066 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -71,6 +71,7 @@ #endif #include <boost/algorithm/string/replace.hpp> +#include <optional> #include <thread> #include <typeinfo> #include <univalue.h> @@ -182,60 +183,65 @@ static std::string SettingName(const std::string& arg) return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg; } +struct KeyInfo { + std::string name; + std::string section; + bool negated{false}; +}; + /** - * Interpret -nofoo as if the user supplied -foo=0. - * - * This method also tracks when the -no form was supplied, and if so, - * checks whether there was a double-negative (-nofoo=0 -> -foo=1). - * - * If there was not a double negative, it removes the "no" from the key - * and returns false. + * Parse "name", "section.name", "noname", "section.noname" settings keys. * - * If there was a double negative, it removes "no" from the key, and - * returns true. - * - * If there was no "no", it returns the string value untouched. - * - * Where an option was negated can be later checked using the + * @note Where an option was negated can be later checked using the * IsArgNegated() method. One use case for this is to have a way to disable * options that are not normally boolean (e.g. using -nodebuglogfile to request * that debug log output is not sent to any file at all). */ - -static util::SettingsValue InterpretOption(std::string& section, std::string& key, const std::string& value) +KeyInfo InterpretKey(std::string key) { + KeyInfo result; // Split section name from key name for keys like "testnet.foo" or "regtest.bar" size_t option_index = key.find('.'); if (option_index != std::string::npos) { - section = key.substr(0, option_index); + result.section = key.substr(0, option_index); key.erase(0, option_index + 1); } if (key.substr(0, 2) == "no") { key.erase(0, 2); - // Double negatives like -nofoo=0 are supported (but discouraged) - if (!InterpretBool(value)) { - LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key, value); - return true; - } - return false; + result.negated = true; } - return value; + result.name = key; + return result; } /** - * Check settings value validity according to flags. + * Interpret settings value based on registered flags. + * + * @param[in] key key information to know if key was negated + * @param[in] value string value of setting to be parsed + * @param[in] flags ArgsManager registered argument flags + * @param[out] error Error description if settings value is not valid * - * TODO: Add more meaningful error checks here in the future - * See "here's how the flags are meant to behave" in - * https://github.com/bitcoin/bitcoin/pull/16097#issuecomment-514627823 + * @return parsed settings value if it is valid, otherwise nullopt accompanied + * by a descriptive error string */ -static bool CheckValid(const std::string& key, const util::SettingsValue& val, unsigned int flags, std::string& error) -{ - if (val.isBool() && !(flags & ArgsManager::ALLOW_BOOL)) { - error = strprintf("Negating of -%s is meaningless and therefore forbidden", key); +static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string& value, + unsigned int flags, std::string& error) +{ + // Return negated settings as false values. + if (key.negated) { + if (flags & ArgsManager::DISALLOW_NEGATION) { + error = strprintf("Negating of -%s is meaningless and therefore forbidden", key.name); + return std::nullopt; + } + // Double negatives like -nofoo=0 are supported (but discouraged) + if (!InterpretBool(value)) { + LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key.name, value); + return true; + } return false; } - return true; + return value; } namespace { @@ -351,21 +357,21 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin // Transform -foo to foo key.erase(0, 1); - std::string section; - util::SettingsValue value = InterpretOption(section, key, val); - std::optional<unsigned int> flags = GetArgFlags('-' + key); + KeyInfo keyinfo = InterpretKey(key); + std::optional<unsigned int> flags = GetArgFlags('-' + keyinfo.name); // Unknown command line options and command line options with dot - // characters (which are returned from InterpretOption with nonempty + // characters (which are returned from InterpretKey with nonempty // section strings) are not valid. - if (!flags || !section.empty()) { + if (!flags || !keyinfo.section.empty()) { error = strprintf("Invalid parameter %s", argv[i]); return false; } - if (!CheckValid(key, value, *flags, error)) return false; + std::optional<util::SettingsValue> value = InterpretValue(keyinfo, val, *flags, error); + if (!value) return false; - m_settings.command_line_options[key].push_back(value); + m_settings.command_line_options[keyinfo.name].push_back(*value); } // we do not allow -includeconf from command line, only -noincludeconf @@ -548,10 +554,8 @@ bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors) return false; } for (const auto& setting : m_settings.rw_settings) { - std::string section; - std::string key = setting.first; - (void)InterpretOption(section, key, /* value */ {}); // Split setting key into section and argname - if (!GetArgFlags('-' + key)) { + KeyInfo key = InterpretKey(setting.first); // Split setting key into section and argname + if (!GetArgFlags('-' + key.name)) { LogPrintf("Ignoring unknown rw_settings value %s\n", setting.first); } } @@ -870,15 +874,14 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file return false; } for (const std::pair<std::string, std::string>& option : options) { - std::string section; - std::string key = option.first; - util::SettingsValue value = InterpretOption(section, key, option.second); - std::optional<unsigned int> flags = GetArgFlags('-' + key); + KeyInfo key = InterpretKey(option.first); + std::optional<unsigned int> flags = GetArgFlags('-' + key.name); if (flags) { - if (!CheckValid(key, value, *flags, error)) { + std::optional<util::SettingsValue> value = InterpretValue(key, option.second, *flags, error); + if (!value) { return false; } - m_settings.ro_config[section][key].push_back(value); + m_settings.ro_config[key.section][key.name].push_back(*value); } else { if (ignore_invalid_keys) { LogPrintf("Ignoring unknown configuration value %s\n", option.first); diff --git a/src/util/system.h b/src/util/system.h index 2e217f6f90..37d976221b 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -158,12 +158,18 @@ struct SectionInfo class ArgsManager { public: + /** + * Flags controlling how config and command line arguments are validated and + * interpreted. + */ enum Flags : uint32_t { - // Boolean options can accept negation syntax -noOPTION or -noOPTION=1 - ALLOW_BOOL = 0x01, - ALLOW_INT = 0x02, - ALLOW_STRING = 0x04, - ALLOW_ANY = ALLOW_BOOL | ALLOW_INT | ALLOW_STRING, + ALLOW_ANY = 0x01, //!< disable validation + // ALLOW_BOOL = 0x02, //!< unimplemented, draft implementation in #16545 + // ALLOW_INT = 0x04, //!< unimplemented, draft implementation in #16545 + // ALLOW_STRING = 0x08, //!< unimplemented, draft implementation in #16545 + // ALLOW_LIST = 0x10, //!< unimplemented, draft implementation in #16545 + DISALLOW_NEGATION = 0x20, //!< disallow -nofoo syntax + DEBUG_ONLY = 0x100, /* Some options would cause cross-contamination if values for * mainnet were used while running on regtest/testnet (or vice-versa). diff --git a/src/validation.cpp b/src/validation.cpp index 207cdc8233..9a3375fea9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -450,7 +450,36 @@ public: /** Whether we allow transactions to replace mempool transactions by BIP125 rules. If false, * any transaction spending the same inputs as a transaction in the mempool is considered * a conflict. */ - const bool m_allow_bip125_replacement{true}; + const bool m_allow_bip125_replacement; + + /** Parameters for single transaction mempool validation. */ + static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time, + bool bypass_limits, std::vector<COutPoint>& coins_to_uncache, + bool test_accept) { + return ATMPArgs{/* m_chainparams */ chainparams, + /* m_accept_time */ accept_time, + /* m_bypass_limits */ bypass_limits, + /* m_coins_to_uncache */ coins_to_uncache, + /* m_test_accept */ test_accept, + /* m_allow_bip125_replacement */ true, + }; + } + + /** Parameters for test package mempool validation through testmempoolaccept. */ + static ATMPArgs PackageTestAccept(const CChainParams& chainparams, int64_t accept_time, + std::vector<COutPoint>& coins_to_uncache) { + return ATMPArgs{/* m_chainparams */ chainparams, + /* m_accept_time */ accept_time, + /* m_bypass_limits */ false, + /* m_coins_to_uncache */ coins_to_uncache, + /* m_test_accept */ true, + /* m_allow_bip125_replacement */ false, + }; + } + + // No default ctor to avoid exposing details to clients and allowing the possibility of + // mixing up the order of the arguments. Use static functions above instead. + ATMPArgs() = delete; }; // Single transaction acceptance @@ -468,13 +497,29 @@ private: // of checking a given transaction. struct Workspace { explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} + /** Txids of mempool transactions that this transaction directly conflicts with. */ std::set<uint256> m_conflicts; + /** Iterators to mempool entries that this transaction directly conflicts with. */ + CTxMemPool::setEntries m_iters_conflicting; + /** Iterators to all mempool entries that would be replaced by this transaction, including + * those it directly conflicts with and their descendants. */ CTxMemPool::setEntries m_all_conflicting; + /** All mempool ancestors of this transaction. */ CTxMemPool::setEntries m_ancestors; + /** Mempool entry constructed for this transaction. Constructed in PreChecks() but not + * inserted into the mempool until Finalize(). */ std::unique_ptr<CTxMemPoolEntry> m_entry; + /** Pointers to the transactions that have been removed from the mempool and replaced by + * this transaction, used to return to the MemPoolAccept caller. Only populated if + * validation is successful and the original transactions are removed. */ std::list<CTransactionRef> m_replaced_transactions; + /** Virtual size of the transaction as used by the mempool, calculated using serialized size + * of the transaction and sigops. */ + int64_t m_vsize; + /** Fees paid by this transaction: total input amounts subtracted by total output amounts. */ CAmount m_base_fees; + /** Base fees + any fee delta set by the user with prioritisetransaction. */ CAmount m_modified_fees; /** Total modified fees of all transactions being replaced. */ CAmount m_conflicting_fees{0}; @@ -482,8 +527,12 @@ private: size_t m_conflicting_size{0}; const CTransactionRef& m_ptx; + /** Txid. */ const uint256& m_hash; TxValidationState m_state; + /** A temporary cache containing serialized transaction data for signature verification. + * Reused across PolicyScriptChecks and ConsensusScriptChecks. */ + PrecomputedTransactionData m_precomputed_txdata; }; // Run the policy checks on a given transaction, excluding any script checks. @@ -492,15 +541,23 @@ private: // only tests that are fast should be done here (to avoid CPU DoS). bool PreChecks(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + // Run checks for mempool replace-by-fee. + bool ReplacementChecks(Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + // Enforce package mempool ancestor/descendant limits (distinct from individual + // ancestor/descendant limits done in PreChecks). + bool PackageMempoolChecks(const std::vector<CTransactionRef>& txns, + PackageValidationState& package_state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + // Run the script checks using our policy flags. As this can be slow, we should // only invoke this on transactions that have otherwise passed policy checks. - bool PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + bool PolicyScriptChecks(const ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Re-run the script checks, using consensus flags, and try to cache the // result in the scriptcache. This should be done after // PolicyScriptChecks(). This requires that all inputs either be in our // utxo set or in the mempool. - bool ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData &txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + bool ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Try to add the transaction to the mempool, removing any conflicts first. // Returns true if the transaction is in the mempool after any size @@ -536,6 +593,9 @@ private: // in-mempool conflicts; see below). size_t m_limit_descendants; size_t m_limit_descendant_size; + + /** Whether the transaction(s) would replace any mempool transactions. If so, RBF rules apply. */ + bool m_rbf{false}; }; bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) @@ -551,13 +611,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Alias what we need out of ws TxValidationState& state = ws.m_state; - std::set<uint256>& setConflicts = ws.m_conflicts; - CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; - CTxMemPool::setEntries& setAncestors = ws.m_ancestors; std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; - CAmount& nModifiedFees = ws.m_modified_fees; - CAmount& nConflictingFees = ws.m_conflicting_fees; - size_t& nConflictingSize = ws.m_conflicting_size; if (!CheckTransaction(tx, state)) { return false; // state filled in by CheckTransaction @@ -603,7 +657,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Transaction conflicts with a mempool tx, but we're not allowing replacements. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed"); } - if (!setConflicts.count(ptxConflicting->GetHash())) + if (!ws.m_conflicts.count(ptxConflicting->GetHash())) { // Transactions that don't explicitly signal replaceability are // *not* replaceable with the current logic, even if one of their @@ -616,7 +670,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } - setConflicts.insert(ptxConflicting->GetHash()); + ws.m_conflicts.insert(ptxConflicting->GetHash()); } } } @@ -680,9 +734,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); - // nModifiedFees includes any fee deltas from PrioritiseTransaction - nModifiedFees = ws.m_base_fees; - m_pool.ApplyDelta(hash, nModifiedFees); + // ws.m_modified_fees includes any fee deltas from PrioritiseTransaction + ws.m_modified_fees = ws.m_base_fees; + m_pool.ApplyDelta(hash, ws.m_modified_fees); // Keep track of transactions that spend a coinbase, which we re-scan // during reorgs to ensure COINBASE_MATURITY is still met. @@ -697,7 +751,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), fSpendsCoinbase, nSigOpsCost, lp)); - unsigned int nSize = entry->GetTxSize(); + ws.m_vsize = entry->GetTxSize(); if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", @@ -705,11 +759,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // No transactions are allowed below minRelayTxFee except from disconnected // blocks - if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; + if (!bypass_limits && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state)) return false; - const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); + ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts); // Calculate in-mempool ancestors, up to a limit. - if (setConflicts.size() == 1) { + if (ws.m_conflicts.size() == 1) { // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we // would meet the chain limits after the conflicts have been removed. However, there isn't a practical // way to do this short of calculating the ancestor and descendant sets with an overlay cache of @@ -737,16 +791,16 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // the ancestor limits should be the same for both our new transaction and any conflicts). // We don't bother incrementing m_limit_descendants by the full removal count as that limit never comes // into force here (as we're only adding a single transaction). - assert(setIterConflicting.size() == 1); - CTxMemPool::txiter conflict = *setIterConflicting.begin(); + assert(ws.m_iters_conflicting.size() == 1); + CTxMemPool::txiter conflict = *ws.m_iters_conflicting.begin(); m_limit_descendants += 1; m_limit_descendant_size += conflict->GetSizeWithDescendants(); } std::string errString; - if (!m_pool.CalculateMemPoolAncestors(*entry, setAncestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { - setAncestors.clear(); + if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) { + ws.m_ancestors.clear(); // If CalculateMemPoolAncestors fails second time, we want the original error string. std::string dummy_err_string; // Contracting/payment channels CPFP carve-out: @@ -760,60 +814,85 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // to be secure by simply only having two immediately-spendable // outputs - one for each counterparty. For more info on the uses for // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html - if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || - !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { + if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || + !m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); } } // A transaction that spends outputs that would be replaced by it is invalid. Now // that we have the set of all ancestors we can detect this - // pathological case by making sure setConflicts and setAncestors don't + // pathological case by making sure ws.m_conflicts and ws.m_ancestors don't // intersect. - if (const auto err_string{EntriesAndTxidsDisjoint(setAncestors, setConflicts, hash)}) { + if (const auto err_string{EntriesAndTxidsDisjoint(ws.m_ancestors, ws.m_conflicts, hash)}) { // We classify this as a consensus error because a transaction depending on something it // conflicts with would be inconsistent. return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string); } + m_rbf = !ws.m_conflicts.empty(); + return true; +} - if (!setConflicts.empty()) { - CFeeRate newFeeRate(nModifiedFees, nSize); - // It's possible that the replacement pays more fees than its direct conflicts but not more - // than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the - // replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not - // more economically rational to mine. Before we go digging through the mempool for all - // transactions that would need to be removed (direct conflicts and all descendants), check - // that the replacement transaction pays more than its direct conflicts. - if (const auto err_string{PaysMoreThanConflicts(setIterConflicting, newFeeRate, hash)}) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); - } +bool MemPoolAccept::ReplacementChecks(Workspace& ws) +{ + AssertLockHeld(cs_main); + AssertLockHeld(m_pool.cs); - // Calculate all conflicting entries and enforce BIP125 Rule #5. - if (const auto err_string{GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting)}) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, - "too many potential replacements", *err_string); - } - // Enforce BIP125 Rule #2. - if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, setIterConflicting)}) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, - "replacement-adds-unconfirmed", *err_string); - } + const CTransaction& tx = *ws.m_ptx; + const uint256& hash = ws.m_hash; + TxValidationState& state = ws.m_state; - // Check if it's economically rational to mine this transaction rather than the ones it - // replaces and pays for its own relay fees. Enforce BIP125 Rules #3 and #4. - for (CTxMemPool::txiter it : allConflicting) { - nConflictingFees += it->GetModifiedFee(); - nConflictingSize += it->GetTxSize(); - } - if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, ::incrementalRelayFee, hash)}) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); - } + CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize); + // It's possible that the replacement pays more fees than its direct conflicts but not more + // than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the + // replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not + // more economically rational to mine. Before we go digging through the mempool for all + // transactions that would need to be removed (direct conflicts and all descendants), check + // that the replacement transaction pays more than its direct conflicts. + if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); + } + + // Calculate all conflicting entries and enforce BIP125 Rule #5. + if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, ws.m_all_conflicting)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, + "too many potential replacements", *err_string); + } + // Enforce BIP125 Rule #2. + if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, ws.m_iters_conflicting)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, + "replacement-adds-unconfirmed", *err_string); + } + // Check if it's economically rational to mine this transaction rather than the ones it + // replaces and pays for its own relay fees. Enforce BIP125 Rules #3 and #4. + for (CTxMemPool::txiter it : ws.m_all_conflicting) { + ws.m_conflicting_fees += it->GetModifiedFee(); + ws.m_conflicting_size += it->GetTxSize(); + } + if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, + ::incrementalRelayFee, hash)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } return true; } -bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txns, + PackageValidationState& package_state) +{ + AssertLockHeld(cs_main); + AssertLockHeld(m_pool.cs); + + std::string err_string; + if (!m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, + m_limit_descendant_size, err_string)) { + // This is a package-wide error, separate from an individual transaction error. + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + } + return true; +} + +bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws) { const CTransaction& tx = *ws.m_ptx; TxValidationState& state = ws.m_state; @@ -822,13 +901,13 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, Prec // Check input scripts and signatures. // This is done last to help prevent CPU exhaustion denial-of-service attacks. - if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, txdata)) { + if (!CheckInputScripts(tx, state, m_view, scriptVerifyFlags, true, false, ws.m_precomputed_txdata)) { // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we // need to turn both off, and compare against just turning off CLEANSTACK // to see if the failure is specifically due to witness validation. TxValidationState state_dummy; // Want reported failures to be from first CheckInputScripts - if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && - !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { + if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, ws.m_precomputed_txdata) && + !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, ws.m_precomputed_txdata)) { // Only the witness is missing, so the transaction itself may be fine. state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED, state.GetRejectReason(), state.GetDebugMessage()); @@ -839,7 +918,7 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, Prec return true; } -bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata) +bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) { const CTransaction& tx = *ws.m_ptx; const uint256& hash = ws.m_hash; @@ -862,7 +941,8 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws, P // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus()); - if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, txdata, m_active_chainstate.CoinsTip())) { + if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, + ws.m_precomputed_txdata, m_active_chainstate.CoinsTip())) { return error("%s: BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s", __func__, hash.ToString(), state.ToString()); } @@ -877,24 +957,19 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) TxValidationState& state = ws.m_state; const bool bypass_limits = args.m_bypass_limits; - CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; - CTxMemPool::setEntries& setAncestors = ws.m_ancestors; - const CAmount& nModifiedFees = ws.m_modified_fees; - const CAmount& nConflictingFees = ws.m_conflicting_fees; - const size_t& nConflictingSize = ws.m_conflicting_size; std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; // Remove conflicting transactions from the mempool - for (CTxMemPool::txiter it : allConflicting) + for (CTxMemPool::txiter it : ws.m_all_conflicting) { LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s additional fees, %d delta bytes\n", it->GetTx().GetHash().ToString(), hash.ToString(), - FormatMoney(nModifiedFees - nConflictingFees), - (int)entry->GetTxSize() - (int)nConflictingSize); + FormatMoney(ws.m_modified_fees - ws.m_conflicting_fees), + (int)entry->GetTxSize() - (int)ws.m_conflicting_size); ws.m_replaced_transactions.push_back(it->GetSharedTx()); } - m_pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED); + m_pool.RemoveStaged(ws.m_all_conflicting, false, MemPoolRemovalReason::REPLACED); // This transaction should only count for fee estimation if: // - it's not being re-added during a reorg which bypasses typical mempool fee limits @@ -903,7 +978,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) bool validForFeeEstimation = !bypass_limits && IsCurrentForFeeEstimation(m_active_chainstate) && m_pool.HasNoInputsOf(tx); // Store transaction in memory - m_pool.addUnchecked(*entry, setAncestors, validForFeeEstimation); + m_pool.addUnchecked(*entry, ws.m_ancestors, validForFeeEstimation); // trim mempool and check if tx was trimmed if (!bypass_limits) { @@ -923,26 +998,24 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef if (!PreChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); - // Only compute the precomputed transaction data if we need to verify - // scripts (ie, other policy checks pass). We perform the inexpensive - // checks first and avoid hashing and signature verification unless those - // checks pass, to mitigate CPU exhaustion denial-of-service attacks. - PrecomputedTransactionData txdata; + if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); - if (!PolicyScriptChecks(args, ws, txdata)) return MempoolAcceptResult::Failure(ws.m_state); + // Perform the inexpensive checks first and avoid hashing and signature verification unless + // those checks pass, to mitigate CPU exhaustion denial-of-service attacks. + if (!PolicyScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); - if (!ConsensusScriptChecks(args, ws, txdata)) return MempoolAcceptResult::Failure(ws.m_state); + if (!ConsensusScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); // Tx was accepted, but not added if (args.m_test_accept) { - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees); } if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence()); - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees); } PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args) @@ -981,18 +1054,12 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // because it's unnecessary. Also, CPFP carve out can increase the limit for individual // transactions, but this exemption is not extended to packages in CheckPackageLimits(). std::string err_string; - if (txns.size() > 1 && - !m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, err_string)) { - // All transactions must have individually passed mempool ancestor and descendant limits - // inside of PreChecks(), so this is separate from an individual transaction error. - package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + if (txns.size() > 1 && !PackageMempoolChecks(txns, package_state)) { return PackageMempoolAcceptResult(package_state, std::move(results)); } for (Workspace& ws : workspaces) { - PrecomputedTransactionData txdata; - if (!PolicyScriptChecks(args, ws, txdata)) { + if (!PolicyScriptChecks(args, ws)) { // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished. package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); @@ -1002,7 +1069,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are // no further mempool checks (passing PolicyScriptChecks implies passing ConsensusScriptChecks). results.emplace(ws.m_ptx->GetWitnessHash(), - MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees)); + MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), + ws.m_vsize, ws.m_base_fees)); } } @@ -1019,9 +1087,7 @@ static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainp EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector<COutPoint> coins_to_uncache; - MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache, - test_accept, /* m_allow_bip125_replacement */ true }; - + auto args = MemPoolAccept::ATMPArgs::SingleAccept(chainparams, nAcceptTime, bypass_limits, coins_to_uncache, test_accept); const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { // Remove coins that were not present in the coins cache before calling @@ -1054,8 +1120,7 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx std::vector<COutPoint> coins_to_uncache; const CChainParams& chainparams = Params(); - MemPoolAccept::ATMPArgs args { chainparams, GetTime(), /* bypass_limits */ false, coins_to_uncache, - test_accept, /* m_allow_bip125_replacement */ false }; + auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache); const PackageMempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args); // Uncache coins pertaining to transactions that were not submitted to the mempool. diff --git a/src/validation.h b/src/validation.h index 4da8ec8d24..256981224a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -158,14 +158,16 @@ struct MempoolAcceptResult { // The following fields are only present when m_result_type = ResultType::VALID /** Mempool transactions replaced by the tx per BIP 125 rules. */ const std::optional<std::list<CTransactionRef>> m_replaced_transactions; + /** Virtual size as used by the mempool, calculated using serialized size and sigops. */ + const std::optional<int64_t> m_vsize; /** Raw base fees in satoshis. */ const std::optional<CAmount> m_base_fees; static MempoolAcceptResult Failure(TxValidationState state) { return MempoolAcceptResult(state); } - static MempoolAcceptResult Success(std::list<CTransactionRef>&& replaced_txns, CAmount fees) { - return MempoolAcceptResult(std::move(replaced_txns), fees); + static MempoolAcceptResult Success(std::list<CTransactionRef>&& replaced_txns, int64_t vsize, CAmount fees) { + return MempoolAcceptResult(std::move(replaced_txns), vsize, fees); } // Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct. @@ -177,9 +179,9 @@ private: } /** Constructor for success case */ - explicit MempoolAcceptResult(std::list<CTransactionRef>&& replaced_txns, CAmount fees) + explicit MempoolAcceptResult(std::list<CTransactionRef>&& replaced_txns, int64_t vsize, CAmount fees) : m_result_type(ResultType::VALID), - m_replaced_transactions(std::move(replaced_txns)), m_base_fees(fees) {} + m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees) {} }; /** diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index b666a8e73a..7d0f80518a 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -113,7 +113,7 @@ public: */ bool Rewrite(const char* pszSkip=nullptr) override; - /** Indicate the a new database user has began using the database. */ + /** Indicate that a new database user has begun using the database. */ void AddRef() override; /** Indicate that database user has stopped using the database and that it could be flushed or closed. */ void RemoveRef() override; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 59a59f9794..7a5526a4cb 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -84,7 +84,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const #endif #ifdef USE_SQLITE - argsman.AddArg("-unsafesqlitesync", "Set SQLite synchronous=OFF to disable waiting for the database to sync to disk. This is unsafe and can cause data loss and corruption. This option is only used by tests to improve their performance (default: false)", ArgsManager::ALLOW_BOOL | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); + argsman.AddArg("-unsafesqlitesync", "Set SQLite synchronous=OFF to disable waiting for the database to sync to disk. This is unsafe and can cause data loss and corruption. This option is only used by tests to improve their performance (default: false)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); #else argsman.AddHiddenArgs({"-unsafesqlitesync"}); #endif diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 1f13b80f3e..5edd9f8f66 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -797,7 +797,7 @@ RPCHelpMan dumpwallet() CKey seed; if (spk_man.GetKey(seed_id, seed)) { CExtKey masterKey; - masterKey.SetSeed(seed.begin(), seed.size()); + masterKey.SetSeed(seed); file << "# extended private masterkey: " << EncodeExtKey(masterKey) << "\n\n"; } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 619ebc8b4f..9173c790d4 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -400,7 +400,7 @@ void LegacyScriptPubKeyMan::UpgradeKeyMetadata() CKey key; GetKey(meta.hd_seed_id, key); CExtKey masterKey; - masterKey.SetSeed(key.begin(), key.size()); + masterKey.SetSeed(key); // Add to map CKeyID master_id = masterKey.key.GetPubKey().GetID(); std::copy(master_id.begin(), master_id.begin() + 4, meta.key_origin.fingerprint); @@ -1085,7 +1085,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& if (!GetKey(hd_chain.seed_id, seed)) throw std::runtime_error(std::string(__func__) + ": seed not found"); - masterKey.SetSeed(seed.begin(), seed.size()); + masterKey.SetSeed(seed); // derive m/0' // use hardened derivation (child keys >= 0x80000000 are hardened after bip32) diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp new file mode 100644 index 0000000000..252832785b --- /dev/null +++ b/src/wallet/test/fuzz/notifications.cpp @@ -0,0 +1,173 @@ +// Copyright (c) 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. + +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> +#include <util/translation.h> +#include <wallet/context.h> +#include <wallet/receive.h> +#include <wallet/wallet.h> +#include <wallet/walletdb.h> +#include <wallet/walletutil.h> + +#include <cassert> +#include <cstdint> +#include <string> +#include <vector> + +namespace { +const TestingSetup* g_setup; + +void initialize_setup() +{ + static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); + g_setup = testing_setup.get(); +} + +/** + * Wraps a descriptor wallet for fuzzing. The constructor writes the sqlite db + * to disk, the destructor deletes it. + */ +struct FuzzedWallet { + ArgsManager args; + WalletContext context; + std::shared_ptr<CWallet> wallet; + FuzzedWallet(const std::string& name) + { + context.args = &args; + context.chain = g_setup->m_node.chain.get(); + + DatabaseOptions options; + options.require_create = true; + options.create_flags = WALLET_FLAG_DESCRIPTORS; + const std::optional<bool> load_on_start; + gArgs.ForceSetArg("-keypool", "0"); // Avoid timeout in TopUp() + + DatabaseStatus status; + bilingual_str error; + std::vector<bilingual_str> warnings; + wallet = CreateWallet(context, name, load_on_start, options, status, error, warnings); + assert(wallet); + assert(error.empty()); + assert(warnings.empty()); + assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + } + ~FuzzedWallet() + { + const auto name{wallet->GetName()}; + std::vector<bilingual_str> warnings; + std::optional<bool> load_on_start; + assert(RemoveWallet(context, wallet, load_on_start, warnings)); + assert(warnings.empty()); + UnloadWallet(std::move(wallet)); + fs::remove_all(GetWalletDir() / name); + } + CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) + { + auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; + if (type == OutputType::BECH32M) { + type = OutputType::BECH32; // TODO: Setup taproot descriptor and remove this line + } + CTxDestination dest; + bilingual_str error; + if (fuzzed_data_provider.ConsumeBool()) { + assert(wallet->GetNewDestination(type, "", dest, error)); + } else { + assert(wallet->GetNewChangeDestination(type, dest, error)); + } + assert(error.empty()); + return GetScriptForDestination(dest); + } +}; + +FUZZ_TARGET_INIT(wallet_notifications, initialize_setup) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + // The total amount, to be distributed to the wallets a and b in txs + // without fee. Thus, the balance of the wallets should always equal the + // total amount. + const auto total_amount{ConsumeMoney(fuzzed_data_provider)}; + FuzzedWallet a{"fuzzed_wallet_a"}; + FuzzedWallet b{"fuzzed_wallet_b"}; + + // Keep track of all coins in this test. + // Each tuple in the chain represents the coins and the block created with + // those coins. Once the block is mined, the next tuple will have an empty + // block and the freshly mined coins. + using Coins = std::set<std::tuple<CAmount, COutPoint>>; + std::vector<std::tuple<Coins, CBlock>> chain; + { + // Add the inital entry + chain.emplace_back(); + auto& [coins, block]{chain.back()}; + coins.emplace(total_amount, COutPoint{uint256::ONE, 1}); + } + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 200) + { + CallOneOf( + fuzzed_data_provider, + [&] { + auto& [coins_orig, block]{chain.back()}; + // Copy the coins for this block and consume all of them + Coins coins = coins_orig; + while (!coins.empty()) { + // Create a new tx + CMutableTransaction tx{}; + // Add some coins as inputs to it + auto num_inputs{fuzzed_data_provider.ConsumeIntegralInRange<int>(1, coins.size())}; + CAmount in{0}; + while (num_inputs-- > 0) { + const auto& [coin_amt, coin_outpoint]{*coins.begin()}; + in += coin_amt; + tx.vin.emplace_back(coin_outpoint); + coins.erase(coins.begin()); + } + // Create some outputs spending all inputs, without fee + LIMITED_WHILE(in > 0 && fuzzed_data_provider.ConsumeBool(), 100) + { + const auto out_value{ConsumeMoney(fuzzed_data_provider, in)}; + in -= out_value; + auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b}; + tx.vout.emplace_back(out_value, wallet.GetScriptPubKey(fuzzed_data_provider)); + } + // Spend the remaining input value, if any + auto& wallet{fuzzed_data_provider.ConsumeBool() ? a : b}; + tx.vout.emplace_back(in, wallet.GetScriptPubKey(fuzzed_data_provider)); + // Add tx to block + block.vtx.emplace_back(MakeTransactionRef(tx)); + } + // Mine block + a.wallet->blockConnected(block, chain.size()); + b.wallet->blockConnected(block, chain.size()); + // Store the coins for the next block + Coins coins_new; + for (const auto& tx : block.vtx) { + uint32_t i{0}; + for (const auto& out : tx->vout) { + coins_new.emplace(out.nValue, COutPoint{tx->GetHash(), i++}); + } + } + chain.emplace_back(coins_new, CBlock{}); + }, + [&] { + if (chain.size() <= 1) return; // The first entry can't be removed + auto& [coins, block]{chain.back()}; + if (block.vtx.empty()) return; // Can only disconnect if the block was submitted first + // Disconnect block + a.wallet->blockDisconnected(block, chain.size() - 1); + b.wallet->blockDisconnected(block, chain.size() - 1); + chain.pop_back(); + }); + auto& [coins, first_block]{chain.front()}; + if (!first_block.vtx.empty()) { + // Only check balance when at least one block was submitted + const auto bal_a{GetBalance(*a.wallet).m_mine_trusted}; + const auto bal_b{GetBalance(*b.wallet).m_mine_trusted}; + assert(total_amount == bal_a + bal_b); + } + } +} +} // namespace diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 120a20749e..dd24fa2c19 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -14,8 +14,9 @@ BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup) static void import_descriptor(CWallet& wallet, const std::string& descriptor) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { - LOCK(wallet.cs_wallet); + AssertLockHeld(wallet.cs_wallet); FlatSigningProvider provider; std::string error; std::unique_ptr<Descriptor> desc = Parse(descriptor, provider, error, /* require_checksum=*/ false); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index fc744ebe5b..313606cc28 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -4,6 +4,8 @@ #include <wallet/test/wallet_test_fixture.h> +#include <scheduler.h> + WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase()) @@ -12,3 +14,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_wallet_client->registerRpcs(); } + +WalletTestingSetup::~WalletTestingSetup() +{ + if (m_node.scheduler) m_node.scheduler->stop(); +} diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index ab7fb8c42b..8bf2d36227 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -19,6 +19,7 @@ */ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + ~WalletTestingSetup(); std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_node.chain, *Assert(m_node.args)); CWallet m_wallet; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4eb9d5560d..9341c6e390 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2732,11 +2732,11 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri walletInstance->m_default_max_tx_fee = max_fee.value(); } - if (gArgs.IsArgSet("-consolidatefeerate")) { - if (std::optional<CAmount> consolidate_feerate = ParseMoney(gArgs.GetArg("-consolidatefeerate", ""))) { + if (args.IsArgSet("-consolidatefeerate")) { + if (std::optional<CAmount> consolidate_feerate = ParseMoney(args.GetArg("-consolidatefeerate", ""))) { walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate); } else { - error = AmountErrMsg("consolidatefeerate", gArgs.GetArg("-consolidatefeerate", "")); + error = AmountErrMsg("consolidatefeerate", args.GetArg("-consolidatefeerate", "")); return nullptr; } } @@ -3160,7 +3160,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() // Get the extended key CExtKey master_key; - master_key.SetSeed(seed_key.begin(), seed_key.size()); + master_key.SetSeed(seed_key); for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { diff --git a/test/functional/example_test.py b/test/functional/example_test.py index d6fc2d580f..2473edcfe9 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -141,8 +141,7 @@ class ExampleTest(BitcoinTestFramework): peer_messaging = self.nodes[0].add_p2p_connection(BaseNode()) # Generating a block on one of the nodes will get us out of IBD - blocks = [int(self.generate(self.nodes[0], nblocks=1)[0], 16)] - self.sync_all(self.nodes[0:2]) + blocks = [int(self.generate(self.nodes[0], sync_fun=lambda: self.sync_all(self.nodes[0:2]), nblocks=1)[0], 16)] # Notice above how we called an RPC by calling a method with the same # name on the node object. Notice also how we used a keyword argument diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index e3cb7725bd..bb67dc88a6 100755 --- a/test/functional/feature_abortnode.py +++ b/test/functional/feature_abortnode.py @@ -26,7 +26,7 @@ class AbortNodeTest(BitcoinTestFramework): # We'll connect the nodes later def run_test(self): - self.generate(self.nodes[0], 3) + self.generate(self.nodes[0], 3, sync_fun=self.no_op) datadir = get_datadir_path(self.options.tmpdir, 0) # Deleting the undo file will result in reorg failure @@ -34,10 +34,10 @@ class AbortNodeTest(BitcoinTestFramework): # Connecting to a node with a more work chain will trigger a reorg # attempt. - self.generate(self.nodes[1], 3) + self.generate(self.nodes[1], 3, sync_fun=self.no_op) with self.nodes[0].assert_debug_log(["Failed to disconnect block"]): self.connect_nodes(0, 1) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) # Check that node0 aborted self.log.info("Waiting for crash") diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 99ac1b5884..d962b622fe 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -275,7 +275,7 @@ class BIP68Test(BitcoinTestFramework): cur_time = int(time.time()) for _ in range(10): self.nodes[0].setmocktime(cur_time + 600) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) cur_time += 600 assert tx2.hash in self.nodes[0].getrawmempool() @@ -351,7 +351,7 @@ class BIP68Test(BitcoinTestFramework): # Reset the chain and get rid of the mocktimed-blocks self.nodes[0].setmocktime(0) self.nodes[0].invalidateblock(self.nodes[0].getblockhash(cur_height+1)) - self.generate(self.nodes[0], 10) + self.generate(self.nodes[0], 10, sync_fun=self.no_op) # Make sure that BIP68 isn't being used to validate blocks prior to # activation height. If more blocks are mined prior to this test @@ -405,9 +405,9 @@ class BIP68Test(BitcoinTestFramework): min_activation_height = 432 height = self.nodes[0].getblockcount() assert_greater_than(min_activation_height - height, 2) - self.generate(self.nodes[0], min_activation_height - height - 2) + self.generate(self.nodes[0], min_activation_height - height - 2, sync_fun=self.no_op) assert not softfork_active(self.nodes[0], 'csv') - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) assert softfork_active(self.nodes[0], 'csv') self.sync_blocks() diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index c592d7bd69..b996b16b9c 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -228,7 +228,7 @@ class CoinStatsIndexTest(BitcoinTestFramework): res9 = index_node.gettxoutsetinfo('muhash') assert_equal(res8, res9) - self.generate(index_node, 1) + self.generate(index_node, 1, sync_fun=self.no_op) res10 = index_node.gettxoutsetinfo('muhash') assert(res8['txouts'] < res10['txouts']) @@ -254,7 +254,7 @@ class CoinStatsIndexTest(BitcoinTestFramework): assert_equal(index_node.gettxoutsetinfo('muhash')['height'], 110) # Add two new blocks - block = self.generate(index_node, 2)[1] + block = self.generate(index_node, 2, sync_fun=self.no_op)[1] res = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=None, use_index=False) # Test that the result of the reorged block is not returned for its old block height diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index f0766ca7c2..1bda4a29b5 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -217,7 +217,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): # Start by creating a lot of utxos on node3 initial_height = self.nodes[3].getblockcount() - utxo_list = create_confirmed_utxos(self, self.nodes[3].getnetworkinfo()['relayfee'], self.nodes[3], 5000) + utxo_list = create_confirmed_utxos(self, self.nodes[3].getnetworkinfo()['relayfee'], self.nodes[3], 5000, sync_fun=self.no_op) self.log.info(f"Prepped {len(utxo_list)} utxo entries") # Sync these blocks with the other nodes @@ -258,6 +258,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): nblocks=min(10, current_height + 1 - self.nodes[3].getblockcount()), # new address to avoid mining a block that has just been invalidated address=self.nodes[3].getnewaddress(), + sync_fun=self.no_op, )) self.log.debug(f"Syncing {len(block_hashes)} new blocks...") self.sync_node3blocks(block_hashes) diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index ac00db8ff0..2a8dd1fb7b 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -237,7 +237,7 @@ class EstimateFeeTest(BitcoinTestFramework): # Mine while len(node.getrawmempool()) > 0: - self.generate(node, 1) + self.generate(node, 1, sync_fun=self.no_op) # Repeatedly split those 2 outputs, doubling twice for each rep # Use txouts to monitor the available utxo, since these won't be tracked in wallet @@ -247,12 +247,12 @@ class EstimateFeeTest(BitcoinTestFramework): while len(self.txouts) > 0: split_inputs(node, self.txouts, self.txouts2) while len(node.getrawmempool()) > 0: - self.generate(node, 1) + self.generate(node, 1, sync_fun=self.no_op) # Double txouts2 to txouts while len(self.txouts2) > 0: split_inputs(node, self.txouts2, self.txouts) while len(node.getrawmempool()) > 0: - self.generate(node, 1) + self.generate(node, 1, sync_fun=self.no_op) reps += 1 def sanity_check_estimates_range(self): diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py index 13e6a8d6d7..079630546e 100755 --- a/test/functional/feature_loadblock.py +++ b/test/functional/feature_loadblock.py @@ -29,7 +29,7 @@ class LoadblockTest(BitcoinTestFramework): def run_test(self): self.nodes[1].setnetworkactive(state=False) - self.generate(self.nodes[0], COINBASE_MATURITY) + self.generate(self.nodes[0], COINBASE_MATURITY, sync_fun=self.no_op) # Parsing the url of our node to get settings for config file data_dir = self.nodes[0].datadir diff --git a/test/functional/feature_minchainwork.py b/test/functional/feature_minchainwork.py index 11cb4aa3cb..a77f022ddd 100755 --- a/test/functional/feature_minchainwork.py +++ b/test/functional/feature_minchainwork.py @@ -51,8 +51,7 @@ class MinimumChainWorkTest(BitcoinTestFramework): num_blocks_to_generate = int((self.node_min_work[1] - starting_chain_work) / REGTEST_WORK_PER_BLOCK) self.log.info(f"Generating {num_blocks_to_generate} blocks on node0") - hashes = self.generatetoaddress(self.nodes[0], num_blocks_to_generate, - self.nodes[0].get_deterministic_priv_key().address) + hashes = self.generate(self.nodes[0], num_blocks_to_generate, sync_fun=self.no_op) self.log.info(f"Node0 current chain work: {self.nodes[0].getblockheader(hashes[-1])['chainwork']}") @@ -73,7 +72,7 @@ class MinimumChainWorkTest(BitcoinTestFramework): assert_equal(self.nodes[2].getblockcount(), starting_blockcount) self.log.info("Generating one more block") - self.generatetoaddress(self.nodes[0], 1, self.nodes[0].get_deterministic_priv_key().address) + self.generate(self.nodes[0], 1) self.log.info("Verifying nodes are all synced") diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 2a507c75c4..612b724fa5 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -149,7 +149,7 @@ class NotificationsTest(BitcoinTestFramework): # about newly confirmed bump2 and newly conflicted tx2. self.disconnect_nodes(0, 1) bump2 = self.nodes[0].bumpfee(tx2)["txid"] - blockhash2 = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE)[0] + blockhash2 = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE, sync_fun=self.no_op)[0] blockheight2 = self.nodes[0].getblockcount() assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1) assert_equal(tx2 in self.nodes[1].getrawmempool(), True) diff --git a/test/functional/feature_proxy.py b/test/functional/feature_proxy.py index 2fb5e328f5..70b9e019c1 100755 --- a/test/functional/feature_proxy.py +++ b/test/functional/feature_proxy.py @@ -12,6 +12,7 @@ Test plan: - `-proxy` (proxy everything) - `-onion` (proxy just onions) - `-proxyrandomize` Circuit randomization + - `-cjdnsreachable` - Proxy configurations to test on proxy side, - support no authentication (other proxy) - support no authentication + user/pass authentication (Tor) @@ -26,6 +27,7 @@ addnode connect to IPv4 addnode connect to IPv6 addnode connect to onion addnode connect to generic DNS name +addnode connect to a CJDNS address - Test getnetworkinfo for each node """ @@ -50,14 +52,15 @@ NET_IPV4 = "ipv4" NET_IPV6 = "ipv6" NET_ONION = "onion" NET_I2P = "i2p" +NET_CJDNS = "cjdns" # Networks returned by RPC getnetworkinfo, defined in src/rpc/net.cpp::GetNetworksInfo() -NETWORKS = frozenset({NET_IPV4, NET_IPV6, NET_ONION, NET_I2P}) +NETWORKS = frozenset({NET_IPV4, NET_IPV6, NET_ONION, NET_I2P, NET_CJDNS}) class ProxyTest(BitcoinTestFramework): def set_test_params(self): - self.num_nodes = 4 + self.num_nodes = 5 self.setup_clean_chain = True def setup_nodes(self): @@ -101,7 +104,9 @@ class ProxyTest(BitcoinTestFramework): ['-listen', f'-proxy={self.conf1.addr[0]}:{self.conf1.addr[1]}',f'-onion={self.conf2.addr[0]}:{self.conf2.addr[1]}', f'-i2psam={self.i2p_sam[0]}:{self.i2p_sam[1]}', '-i2pacceptincoming=0', '-proxyrandomize=0'], ['-listen', f'-proxy={self.conf2.addr[0]}:{self.conf2.addr[1]}','-proxyrandomize=1'], - [] + [], + ['-listen', f'-proxy={self.conf1.addr[0]}:{self.conf1.addr[1]}','-proxyrandomize=1', + '-cjdnsreachable'] ] if self.have_ipv6: args[3] = ['-listen', f'-proxy=[{self.conf3.addr[0]}]:{self.conf3.addr[1]}','-proxyrandomize=0', '-noonion'] @@ -113,7 +118,7 @@ class ProxyTest(BitcoinTestFramework): if peer["addr"] == addr: assert_equal(peer["network"], network) - def node_test(self, node, proxies, auth, test_onion=True): + def node_test(self, node, *, proxies, auth, test_onion, test_cjdns): rv = [] addr = "15.61.23.23:1234" self.log.debug(f"Test: outgoing IPv4 connection through node for address {addr}") @@ -161,6 +166,21 @@ class ProxyTest(BitcoinTestFramework): rv.append(cmd) self.network_test(node, addr, network=NET_ONION) + if test_cjdns: + addr = "[fc00:1:2:3:4:5:6:7]:8888" + self.log.debug(f"Test: outgoing CJDNS connection through node for address {addr}") + node.addnode(addr, "onetry") + cmd = proxies[1].queue.get() + assert isinstance(cmd, Socks5Command) + assert_equal(cmd.atyp, AddressType.DOMAINNAME) + assert_equal(cmd.addr, b"fc00:1:2:3:4:5:6:7") + assert_equal(cmd.port, 8888) + if not auth: + assert_equal(cmd.username, None) + assert_equal(cmd.password, None) + rv.append(cmd) + self.network_test(node, addr, network=NET_CJDNS) + addr = "node.noumenon:8333" self.log.debug(f"Test: outgoing DNS name connection through node for address {addr}") node.addnode(addr, "onetry") @@ -179,20 +199,33 @@ class ProxyTest(BitcoinTestFramework): def run_test(self): # basic -proxy - self.node_test(self.nodes[0], [self.serv1, self.serv1, self.serv1, self.serv1], False) + self.node_test(self.nodes[0], + proxies=[self.serv1, self.serv1, self.serv1, self.serv1], + auth=False, test_onion=True, test_cjdns=False) # -proxy plus -onion - self.node_test(self.nodes[1], [self.serv1, self.serv1, self.serv2, self.serv1], False) + self.node_test(self.nodes[1], + proxies=[self.serv1, self.serv1, self.serv2, self.serv1], + auth=False, test_onion=True, test_cjdns=False) # -proxy plus -onion, -proxyrandomize - rv = self.node_test(self.nodes[2], [self.serv2, self.serv2, self.serv2, self.serv2], True) + rv = self.node_test(self.nodes[2], + proxies=[self.serv2, self.serv2, self.serv2, self.serv2], + auth=True, test_onion=True, test_cjdns=False) # Check that credentials as used for -proxyrandomize connections are unique credentials = set((x.username,x.password) for x in rv) assert_equal(len(credentials), len(rv)) if self.have_ipv6: # proxy on IPv6 localhost - self.node_test(self.nodes[3], [self.serv3, self.serv3, self.serv3, self.serv3], False, False) + self.node_test(self.nodes[3], + proxies=[self.serv3, self.serv3, self.serv3, self.serv3], + auth=False, test_onion=False, test_cjdns=False) + + # -proxy=unauth -proxyrandomize=1 -cjdnsreachable + self.node_test(self.nodes[4], + proxies=[self.serv1, self.serv1, self.serv1, self.serv1], + auth=False, test_onion=True, test_cjdns=True) def networks_dict(d): r = {} @@ -214,6 +247,7 @@ class ProxyTest(BitcoinTestFramework): assert_equal(n0[net]['proxy_randomize_credentials'], expected_randomize) assert_equal(n0['onion']['reachable'], True) assert_equal(n0['i2p']['reachable'], False) + assert_equal(n0['cjdns']['reachable'], False) n1 = networks_dict(self.nodes[1].getnetworkinfo()) assert_equal(NETWORKS, n1.keys()) @@ -240,6 +274,7 @@ class ProxyTest(BitcoinTestFramework): assert_equal(n2[net]['proxy_randomize_credentials'], expected_randomize) assert_equal(n2['onion']['reachable'], True) assert_equal(n2['i2p']['reachable'], False) + assert_equal(n2['cjdns']['reachable'], False) if self.have_ipv6: n3 = networks_dict(self.nodes[3].getnetworkinfo()) @@ -253,6 +288,22 @@ class ProxyTest(BitcoinTestFramework): assert_equal(n3[net]['proxy_randomize_credentials'], False) assert_equal(n3['onion']['reachable'], False) assert_equal(n3['i2p']['reachable'], False) + assert_equal(n3['cjdns']['reachable'], False) + + n4 = networks_dict(self.nodes[4].getnetworkinfo()) + assert_equal(NETWORKS, n4.keys()) + for net in NETWORKS: + if net == NET_I2P: + expected_proxy = '' + expected_randomize = False + else: + expected_proxy = '%s:%i' % (self.conf1.addr) + expected_randomize = True + assert_equal(n4[net]['proxy'], expected_proxy) + assert_equal(n4[net]['proxy_randomize_credentials'], expected_randomize) + assert_equal(n4['onion']['reachable'], True) + assert_equal(n4['i2p']['reachable'], False) + assert_equal(n4['cjdns']['reachable'], True) if __name__ == '__main__': diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index c2463d0bcc..125b219bd4 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -118,9 +118,8 @@ class PruneTest(BitcoinTestFramework): def create_big_chain(self): # Start by creating some coinbases we can spend later - self.generate(self.nodes[1], 200) - self.sync_blocks(self.nodes[0:2]) - self.generate(self.nodes[0], 150) + self.generate(self.nodes[1], 200, sync_fun=lambda: self.sync_blocks(self.nodes[0:2])) + self.generate(self.nodes[0], 150, sync_fun=self.no_op) # Then mine enough full blocks to create more than 550MiB of data mine_large_blocks(self.nodes[0], 645) @@ -211,7 +210,7 @@ class PruneTest(BitcoinTestFramework): self.disconnect_nodes(1, 2) self.log.info("Generating new longer chain of 300 more blocks") - self.generate(self.nodes[1], 300) + self.generate(self.nodes[1], 300, sync_fun=self.no_op) self.log.info("Reconnect nodes") self.connect_nodes(0, 1) @@ -263,7 +262,7 @@ class PruneTest(BitcoinTestFramework): self.nodes[0].invalidateblock(curchainhash) assert_equal(self.nodes[0].getblockcount(), self.mainchainheight) assert_equal(self.nodes[0].getbestblockhash(), self.mainchainhash2) - goalbesthash = self.generate(self.nodes[0], blocks_to_mine)[-1] + goalbesthash = self.generate(self.nodes[0], blocks_to_mine, sync_fun=self.no_op)[-1] goalbestheight = first_reorg_height + 1 self.log.info("Verify node 2 reorged back to the main chain, some blocks of which it had to redownload") @@ -306,7 +305,7 @@ class PruneTest(BitcoinTestFramework): assert_equal(block1_details["nTx"], len(block1_details["tx"])) # mine 6 blocks so we are at height 1001 (i.e., above PruneAfterHeight) - self.generate(node, 6) + self.generate(node, 6, sync_fun=self.no_op) assert_equal(node.getblockchaininfo()["blocks"], 1001) # Pruned block should still know the number of transactions @@ -337,7 +336,7 @@ class PruneTest(BitcoinTestFramework): assert has_block(2), "blk00002.dat is still there, should be pruned by now" # advance the tip so blk00002.dat and blk00003.dat can be pruned (the last 288 blocks should now be in blk00004.dat) - self.generate(node, 288) + self.generate(node, 288, sync_fun=self.no_op) prune(1000) assert not has_block(2), "blk00002.dat is still there, should be pruned by now" assert not has_block(3), "blk00003.dat is still there, should be pruned by now" diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 420147542e..39859d0151 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -47,8 +47,8 @@ class ReplaceByFeeTest(BitcoinTestFramework): def run_test(self): self.wallet = MiniWallet(self.nodes[0]) # the pre-mined test framework chain contains coinbase outputs to the - # MiniWallet's default address ADDRESS_BCRT1_P2WSH_OP_TRUE in blocks - # 76-100 (see method BitcoinTestFramework._initialize_chain()) + # MiniWallet's default address in blocks 76-100 (see method + # BitcoinTestFramework._initialize_chain()) self.wallet.rescan_utxos() self.log.info("Running test simple doublespend...") diff --git a/test/functional/feature_signet.py b/test/functional/feature_signet.py index 94138b0e6d..268da62faf 100755 --- a/test/functional/feature_signet.py +++ b/test/functional/feature_signet.py @@ -51,7 +51,7 @@ class SignetBasicTest(BitcoinTestFramework): assert_equal(mining_info['networkhashps'], Decimal('0')) assert_equal(mining_info['pooledtx'], 0) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) self.log.info("pregenerated signet blocks check") diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index b1b4703d37..33b7615aea 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -69,8 +69,8 @@ class UTXOSetHashTest(BitcoinTestFramework): assert_equal(finalized[::-1].hex(), node_muhash) self.log.info("Test deterministic UTXO set hash results") - assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "5b1b44097406226c0eb8e1362cd17a1f346522cf9390a8175a57a5262cb1963f") - assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "4b8803075d7151d06fad3e88b68ba726886794873fbfa841d12aefb2cc2b881b") + assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "221f245cf4c9010eeb7f5183d342c002ae6c1c27e98aa357dccb788c21d98049") + assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "7c0890c68501f7630d36aeb3999dc924e63af084ae1bbfba11dd462144637635") def run_test(self): self.test_muhash_implementation() diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index c28186cde7..ae665958b9 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -136,7 +136,7 @@ class TestBitcoinCli(BitcoinTestFramework): network_info = self.nodes[0].getnetworkinfo() cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli() cli_get_info = cli_get_info_string_to_dict(cli_get_info_string) - assert_equal(cli_get_info["Proxies"], "127.0.0.1:9050 (ipv4, ipv6, onion), 127.0.0.1:7656 (i2p)") + assert_equal(cli_get_info["Proxies"], "127.0.0.1:9050 (ipv4, ipv6, onion, cjdns), 127.0.0.1:7656 (i2p)") if self.is_wallet_compiled(): self.log.info("Test -getinfo and bitcoin-cli getwalletinfo return expected wallet info") diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 5a11a62ec4..5bf75e2064 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -82,9 +82,8 @@ class ZMQTestSetupBlock: the generated block's hash, it's (coinbase) transaction id, the raw block or raw transaction data. """ - def __init__(self, test_framework, node): - self.block_hash = test_framework.generate(node, 1)[0] + self.block_hash = test_framework.generate(node, 1, sync_fun=test_framework.no_op)[0] coinbase = node.getblock(self.block_hash, 2)['tx'][0] self.tx_hash = coinbase['txid'] self.raw_tx = coinbase['hex'] @@ -261,14 +260,14 @@ class ZMQTest (BitcoinTestFramework): # Generate 1 block in nodes[0] with 1 mempool tx and receive all notifications payment_txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) - disconnect_block = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE)[0] + disconnect_block = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE, sync_fun=self.no_op)[0] disconnect_cb = self.nodes[0].getblock(disconnect_block)["tx"][0] assert_equal(self.nodes[0].getbestblockhash(), hashblock.receive().hex()) assert_equal(hashtx.receive().hex(), payment_txid) assert_equal(hashtx.receive().hex(), disconnect_cb) # Generate 2 blocks in nodes[1] to a different address to ensure split - connect_blocks = self.generatetoaddress(self.nodes[1], 2, ADDRESS_BCRT1_P2WSH_OP_TRUE) + connect_blocks = self.generatetoaddress(self.nodes[1], 2, ADDRESS_BCRT1_P2WSH_OP_TRUE, sync_fun=self.no_op) # nodes[0] will reorg chain after connecting back nodes[1] self.connect_nodes(0, 1) @@ -312,13 +311,13 @@ class ZMQTest (BitcoinTestFramework): seq_num = 1 # Generate 1 block in nodes[0] and receive all notifications - dc_block = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE)[0] + dc_block = self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE, sync_fun=self.no_op)[0] # Note: We are not notified of any block transactions, coinbase or mined assert_equal((self.nodes[0].getbestblockhash(), "C", None), seq.receive_sequence()) # Generate 2 blocks in nodes[1] to a different address to ensure a chain split - self.generatetoaddress(self.nodes[1], 2, ADDRESS_BCRT1_P2WSH_OP_TRUE) + self.generatetoaddress(self.nodes[1], 2, ADDRESS_BCRT1_P2WSH_OP_TRUE, sync_fun=self.no_op) # nodes[0] will reorg chain after connecting back nodes[1] self.connect_nodes(0, 1) @@ -474,7 +473,7 @@ class ZMQTest (BitcoinTestFramework): # 1) Consume backlog until we get a mempool sequence number (hash_str, label, zmq_mem_seq) = seq.receive_sequence() while zmq_mem_seq is None: - (hash_str, label, zmq_mem_seq) = seq.receive_sequence() + (hash_str, label, zmq_mem_seq) = seq.receive_sequence() assert label == "A" or label == "R" assert hash_str is not None @@ -566,7 +565,7 @@ class ZMQTest (BitcoinTestFramework): ], sync_blocks=False) # Generate 1 block in nodes[0] and receive all notifications - self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE) + self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_UNSPENDABLE, sync_fun=self.no_op) # Should receive the same block hash on both subscribers assert_equal(self.nodes[0].getbestblockhash(), subscribers[0].receive().hex()) diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index a9f09b1cf8..d450b40582 100755 --- a/test/functional/mempool_compatibility.py +++ b/test/functional/mempool_compatibility.py @@ -7,14 +7,17 @@ NOTE: The test is designed to prevent cases when compatibility is broken accidentally. In case we need to break mempool compatibility we can continue to use the test by just bumping the version number. -The previous release v0.15.2 is required by this test, see test/README.md. +The previous release v0.19.1 is required by this test, see test/README.md. """ import os from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework -from test_framework.wallet import MiniWallet +from test_framework.wallet import ( + MiniWallet, + MiniWalletMode, +) class MempoolCompatibilityTest(BitcoinTestFramework): @@ -37,9 +40,9 @@ class MempoolCompatibilityTest(BitcoinTestFramework): self.log.info("Test that mempool.dat is compatible between versions") old_node, new_node = self.nodes - new_wallet = MiniWallet(new_node) - self.generate(new_wallet, 1) - self.generate(new_node, COINBASE_MATURITY) + new_wallet = MiniWallet(new_node, mode=MiniWalletMode.RAW_P2PK) + self.generate(new_wallet, 1, sync_fun=self.no_op) + self.generate(new_node, COINBASE_MATURITY, sync_fun=self.no_op) # Sync the nodes to ensure old_node has the block that contains the coinbase that new_wallet will spend. # Otherwise, because coinbases are only valid in a block and not as loose txns, if the nodes aren't synced # unbroadcasted_tx won't pass old_node's `MemPoolAccept::PreChecks`. diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 4dae2d9042..9bcd4cf6b1 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -175,7 +175,7 @@ class MempoolPersistTest(BitcoinTestFramework): self.start_node(0) # clear out mempool - self.generate(node0, 1) + self.generate(node0, 1, sync_fun=self.no_op) # ensure node0 doesn't have any connections # make a transaction that will remain in the unbroadcast set diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py index 260b41ef12..509a003746 100755 --- a/test/functional/mempool_reorg.py +++ b/test/functional/mempool_reorg.py @@ -79,9 +79,8 @@ class MempoolCoinbaseTest(BitcoinTestFramework): spend_3_1_id = self.nodes[0].sendrawtransaction(spend_3_1['hex']) self.log.info("Generate a block") last_block = self.generate(self.nodes[0], 1) - # Sync blocks, so that peer 1 gets the block before timelock_tx + # generate() implicitly syncs blocks, so that peer 1 gets the block before timelock_tx # Otherwise, peer 1 would put the timelock_tx in m_recent_rejects - self.sync_all() self.log.info("The time-locked transaction can now be spent") timelock_tx_id = self.nodes[0].sendrawtransaction(timelock_tx) diff --git a/test/functional/mempool_unbroadcast.py b/test/functional/mempool_unbroadcast.py index 4d6379fe86..88194a09b4 100755 --- a/test/functional/mempool_unbroadcast.py +++ b/test/functional/mempool_unbroadcast.py @@ -109,7 +109,8 @@ class MempoolUnbroadcastTest(BitcoinTestFramework): # a block removal_reason = "Removed {} from set of unbroadcast txns before confirmation that txn was sent out".format(txhsh) with node.assert_debug_log([removal_reason]): - self.generate(node, 1) + self.generate(node, 1, sync_fun=self.no_op) + if __name__ == "__main__": MempoolUnbroadcastTest().main() diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index f141d201eb..3d1f804ddc 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -58,7 +58,7 @@ class MiningTest(BitcoinTestFramework): self.log.info('Create some old blocks') for t in range(TIME_GENESIS_BLOCK, TIME_GENESIS_BLOCK + 200 * 600, 600): self.nodes[0].setmocktime(t) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) mining_info = self.nodes[0].getmininginfo() assert_equal(mining_info['blocks'], 200) assert_equal(mining_info['currentblocktx'], 0) diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 35274d3500..01d8501b6b 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -105,7 +105,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): # the other high fee transactions. Keep mining until our mempool has # decreased by all the high fee size that we calculated above. while (self.nodes[0].getmempoolinfo()['bytes'] > sizes[0] + sizes[1]): - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) # High fee transaction should not have been mined, but other high fee rate # transactions should have been. diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 15b90fa61f..9df74ad3a0 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -152,7 +152,6 @@ class AddrTest(BitcoinTestFramework): msg = self.setup_addr_msg(num_ipv4_addrs) with self.nodes[0].assert_debug_log( [ - 'Added {} addresses from 127.0.0.1: 0 tried'.format(num_ipv4_addrs), 'received: addr (301 bytes) peer=1', ] ): diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py index 3833c58680..f4be893d2c 100755 --- a/test/functional/p2p_addrv2_relay.py +++ b/test/functional/p2p_addrv2_relay.py @@ -72,9 +72,6 @@ class AddrTest(BitcoinTestFramework): addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) msg.addrs = ADDRS with self.nodes[0].assert_debug_log([ - # The I2P address is not added to node's own addrman because it has no - # I2P reachability (thus 10 - 1 = 9). - 'Added 9 addresses from 127.0.0.1: 0 tried', 'received: addrv2 (159 bytes) peer=0', 'sending addrv2 (159 bytes) peer=1', ]): diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 3a4fcc4549..b67a0b3f7e 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -62,11 +62,11 @@ class CompactFiltersTest(BitcoinTestFramework): # Stale blocks by disconnecting nodes 0 & 1, mining, then reconnecting self.disconnect_nodes(0, 1) - stale_block_hash = self.generate(self.nodes[0], 1)[0] + stale_block_hash = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] self.nodes[0].syncwithvalidationinterfacequeue() assert_equal(self.nodes[0].getblockcount(), 1000) - self.generate(self.nodes[1], 1001) + self.generate(self.nodes[1], 1001, sync_fun=self.no_op) assert_equal(self.nodes[1].getblockcount(), 2000) # Check that nodes have signalled NODE_COMPACT_FILTERS correctly. diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py index 5f01fa4dfe..6367eb26a3 100755 --- a/test/functional/p2p_compactblocks_blocksonly.py +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -33,7 +33,7 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): self.sync_all() def build_block_on_tip(self): - blockhash = self.generate(self.nodes[2], 1)[0] + blockhash = self.generate(self.nodes[2], 1, sync_fun=self.no_op)[0] block_hex = self.nodes[2].getblock(blockhash=blockhash, verbosity=0) block = from_hex(CBlock(), block_hex) block.rehash() diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index e491fe7e07..d70870fa56 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -59,7 +59,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.") self.connect_nodes(0, 1) - blocks = self.generatetoaddress(self.nodes[1], 292, self.nodes[1].get_deterministic_priv_key().address) + blocks = self.generate(self.nodes[1], 292, sync_fun=self.no_op) self.sync_blocks([self.nodes[0], self.nodes[1]]) self.log.info("Make sure we can max retrieve block at tip-288.") @@ -101,7 +101,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.disconnect_all() # mine 10 blocks on node 0 (pruned node) - self.generatetoaddress(self.nodes[0], 10, self.nodes[0].get_deterministic_priv_key().address) + self.generate(self.nodes[0], 10, sync_fun=self.no_op) # connect node1 (non pruned) with node0 (pruned) and check if the can sync self.connect_nodes(0, 1) diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index a9d5ed970a..e037198573 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -77,7 +77,7 @@ class AcceptBlockTest(BitcoinTestFramework): min_work_node = self.nodes[1].add_p2p_connection(P2PInterface()) # 1. Have nodes mine a block (leave IBD) - [self.generatetoaddress(n, 1, n.get_deterministic_priv_key().address) for n in self.nodes] + [self.generate(n, 1, sync_fun=self.no_op) for n in self.nodes] tips = [int("0x" + n.getbestblockhash(), 0) for n in self.nodes] # 2. Send one block that builds on each tip. diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index eea9ee26cb..14e17bada6 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -25,7 +25,6 @@ import http.client import os import subprocess -from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE from test_framework.blocktools import ( create_block, create_coinbase, @@ -64,6 +63,7 @@ class BlockchainTest(BitcoinTestFramework): self.supports_cli = False def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) self.mine_chain() self.restart_node(0, extra_args=['-stopatheight=207', '-prune=1']) # Set extra args with pruning after rescan is complete @@ -82,7 +82,7 @@ class BlockchainTest(BitcoinTestFramework): self.log.info(f"Generate {HEIGHT} blocks after the genesis block in ten-minute steps") for t in range(TIME_GENESIS_BLOCK, TIME_RANGE_END, TIME_RANGE_STEP): self.nodes[0].setmocktime(t) - self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_P2WSH_OP_TRUE) + self.generate(self.wallet, 1) assert_equal(self.nodes[0].getblockchaininfo()['blocks'], HEIGHT) def _test_getblockchaininfo(self): @@ -371,12 +371,12 @@ class BlockchainTest(BitcoinTestFramework): def _test_stopatheight(self): self.log.info("Test stopping at height") assert_equal(self.nodes[0].getblockcount(), HEIGHT) - self.generatetoaddress(self.nodes[0], 6, ADDRESS_BCRT1_P2WSH_OP_TRUE) + self.generate(self.wallet, 6) assert_equal(self.nodes[0].getblockcount(), HEIGHT + 6) self.log.debug('Node should not stop at this height') assert_raises(subprocess.TimeoutExpired, lambda: self.nodes[0].process.wait(timeout=3)) try: - self.generatetoaddress(self.nodes[0], 1, ADDRESS_BCRT1_P2WSH_OP_TRUE) + self.generatetoaddress(self.nodes[0], 1, self.wallet.get_address(), sync_fun=self.no_op) except (ConnectionError, http.client.BadStatusLine): pass # The node already shut down before response self.log.debug('Node should stop at this height...') @@ -424,14 +424,10 @@ class BlockchainTest(BitcoinTestFramework): def _test_getblock(self): node = self.nodes[0] - - miniwallet = MiniWallet(node) - miniwallet.rescan_utxos() - fee_per_byte = Decimal('0.00000010') fee_per_kb = 1000 * fee_per_byte - miniwallet.send_self_transfer(fee_rate=fee_per_kb, from_node=node) + self.wallet.send_self_transfer(fee_rate=fee_per_kb, from_node=node) blockhash = self.generate(node, 1)[0] def assert_fee_not_in_block(verbosity): diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index b0e46c6ca7..84be772895 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -136,6 +136,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_include_unsafe() self.test_external_inputs() self.test_22670() + self.test_feerate_rounding() def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" @@ -1096,7 +1097,7 @@ class RawTransactionsTest(BitcoinTestFramework): # than any single input available, and require more than 1 input. So we make 3 outputs for i in range(0, 3): funds.sendtoaddress(tester.getnewaddress(address_type="bech32"), 1) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) # Create transactions in order to calculate fees for the target bounds that can trigger this bug change_tx = tester.fundrawtransaction(tester.createrawtransaction([], [{funds.getnewaddress(): 1.5}])) @@ -1129,6 +1130,33 @@ class RawTransactionsTest(BitcoinTestFramework): do_fund_send(upper_bound) self.restart_node(0) + self.connect_nodes(0, 1) + self.connect_nodes(0, 2) + self.connect_nodes(0, 3) + + def test_feerate_rounding(self): + self.log.info("Test that rounding of GetFee does not result in an assertion") + + self.nodes[1].createwallet("roundtest") + w = self.nodes[1].get_wallet_rpc("roundtest") + + addr = w.getnewaddress(address_type="bech32") + self.nodes[0].sendtoaddress(addr, 1) + self.generate(self.nodes[0], 1) + self.sync_all() + + # A P2WPKH input costs 68 vbytes; With a single P2WPKH output, the rest of the tx is 42 vbytes for a total of 110 vbytes. + # At a feerate of 1.85 sat/vb, the input will need a fee of 125.8 sats and the rest 77.7 sats + # The entire tx fee should be 203.5 sats. + # Coin selection rounds the fee individually instead of at the end (due to how CFeeRate::GetFee works). + # If rounding down (which is the incorrect behavior), then the calculated fee will be 125 + 77 = 202. + # If rounding up, then the calculated fee will be 126 + 78 = 204. + # In the former case, the calculated needed fee is higher than the actual fee being paid, so an assertion is reached + # To test this does not happen, we subtract 202 sats from the input value. If working correctly, this should + # fail with insufficient funds rather than bitcoind asserting. + rawtx = w.createrawtransaction(inputs=[], outputs=[{self.nodes[0].getnewaddress(address_type="bech32"): 1 - 0.00000202}]) + assert_raises_rpc_error(-4, "Insufficient funds", w.fundrawtransaction, rawtx, {"fee_rate": 1.85}) + if __name__ == '__main__': RawTransactionsTest().main() diff --git a/test/functional/rpc_getblockfilter.py b/test/functional/rpc_getblockfilter.py index 4d860d0f36..1c456a5b82 100755 --- a/test/functional/rpc_getblockfilter.py +++ b/test/functional/rpc_getblockfilter.py @@ -21,8 +21,8 @@ class GetBlockFilterTest(BitcoinTestFramework): # Create two chains by disconnecting nodes 0 & 1, mining, then reconnecting self.disconnect_nodes(0, 1) - self.generate(self.nodes[0], 3) - self.generate(self.nodes[1], 4) + self.generate(self.nodes[0], 3, sync_fun=self.no_op) + self.generate(self.nodes[1], 4, sync_fun=self.no_op) assert_equal(self.nodes[0].getblockcount(), 3) chain0_hashes = [self.nodes[0].getblockhash(block_height) for block_height in range(4)] diff --git a/test/functional/rpc_getchaintips.py b/test/functional/rpc_getchaintips.py index ab0ee9142d..fb09c81cbd 100755 --- a/test/functional/rpc_getchaintips.py +++ b/test/functional/rpc_getchaintips.py @@ -26,10 +26,8 @@ class GetChainTipsTest (BitcoinTestFramework): # Split the network and build two chains of different lengths. self.split_network() - self.generatetoaddress(self.nodes[0], 10, self.nodes[0].get_deterministic_priv_key().address) - self.generatetoaddress(self.nodes[2], 20, self.nodes[2].get_deterministic_priv_key().address) - self.sync_all(self.nodes[:2]) - self.sync_all(self.nodes[2:]) + self.generate(self.nodes[0], 10, sync_fun=lambda: self.sync_all(self.nodes[:2])) + self.generate(self.nodes[2], 20, sync_fun=lambda: self.sync_all(self.nodes[2:])) tips = self.nodes[1].getchaintips () assert_equal (len (tips), 1) diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index a91ce85855..9bc02d153e 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -22,12 +22,12 @@ class InvalidateTest(BitcoinTestFramework): def run_test(self): self.log.info("Make sure we repopulate setBlockIndexCandidates after InvalidateBlock:") self.log.info("Mine 4 blocks on Node 0") - self.generatetoaddress(self.nodes[0], 4, self.nodes[0].get_deterministic_priv_key().address) + self.generate(self.nodes[0], 4, sync_fun=self.no_op) assert_equal(self.nodes[0].getblockcount(), 4) besthash_n0 = self.nodes[0].getbestblockhash() self.log.info("Mine competing 6 blocks on Node 1") - self.generatetoaddress(self.nodes[1], 6, self.nodes[1].get_deterministic_priv_key().address) + self.generate(self.nodes[1], 6, sync_fun=self.no_op) assert_equal(self.nodes[1].getblockcount(), 6) self.log.info("Connect nodes to force a reorg") @@ -53,14 +53,14 @@ class InvalidateTest(BitcoinTestFramework): self.nodes[2].invalidateblock(self.nodes[2].getblockhash(3)) assert_equal(self.nodes[2].getblockcount(), 2) self.log.info("..and then mine a block") - self.generatetoaddress(self.nodes[2], 1, self.nodes[2].get_deterministic_priv_key().address) + self.generate(self.nodes[2], 1, sync_fun=self.no_op) self.log.info("Verify all nodes are at the right height") self.wait_until(lambda: self.nodes[2].getblockcount() == 3, timeout=5) self.wait_until(lambda: self.nodes[0].getblockcount() == 4, timeout=5) self.wait_until(lambda: self.nodes[1].getblockcount() == 4, timeout=5) self.log.info("Verify that we reconsider all ancestors as well") - blocks = self.generatetodescriptor(self.nodes[1], 10, ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR) + blocks = self.generatetodescriptor(self.nodes[1], 10, ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, sync_fun=self.no_op) assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) # Invalidate the two blocks at the tip self.nodes[1].invalidateblock(blocks[-1]) @@ -72,7 +72,7 @@ class InvalidateTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) self.log.info("Verify that we reconsider all descendants") - blocks = self.generatetodescriptor(self.nodes[1], 10, ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR) + blocks = self.generatetodescriptor(self.nodes[1], 10, ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, sync_fun=self.no_op) assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) # Invalidate the two blocks at the tip self.nodes[1].invalidateblock(blocks[-2]) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 0f3bbce54c..0857f4e0ca 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -106,7 +106,7 @@ class NetTest(BitcoinTestFramework): assert_equal(peer_info[1][1]['connection_type'], 'inbound') # Check dynamically generated networks list in getpeerinfo help output. - assert "(ipv4, ipv6, onion, i2p, not_publicly_routable)" in self.nodes[0].help("getpeerinfo") + assert "(ipv4, ipv6, onion, i2p, cjdns, not_publicly_routable)" in self.nodes[0].help("getpeerinfo") def test_getnettotals(self): self.log.info("Test getnettotals") @@ -157,7 +157,7 @@ class NetTest(BitcoinTestFramework): assert_net_servicesnames(int(info["localservices"], 0x10), info["localservicesnames"]) # Check dynamically generated networks list in getnetworkinfo help output. - assert "(ipv4, ipv6, onion, i2p)" in self.nodes[0].help("getnetworkinfo") + assert "(ipv4, ipv6, onion, i2p, cjdns)" in self.nodes[0].help("getnetworkinfo") def test_getaddednodeinfo(self): self.log.info("Test getaddednodeinfo") @@ -228,8 +228,8 @@ class NetTest(BitcoinTestFramework): assert_equal(res[0]["port"], 8333) assert_equal(res[0]["services"], P2P_SERVICES) - # Test for the absence of onion and I2P addresses. - for network in ["onion", "i2p"]: + # Test for the absence of onion, I2P and CJDNS addresses. + for network in ["onion", "i2p", "cjdns"]: assert_equal(self.nodes[0].getnodeaddresses(0, network), []) # Test invalid arguments. diff --git a/test/functional/rpc_preciousblock.py b/test/functional/rpc_preciousblock.py index 3a00992ddc..2e526efd9a 100755 --- a/test/functional/rpc_preciousblock.py +++ b/test/functional/rpc_preciousblock.py @@ -42,19 +42,18 @@ class PreciousTest(BitcoinTestFramework): def run_test(self): self.log.info("Ensure submitblock can in principle reorg to a competing chain") - gen_address = lambda i: self.nodes[i].get_deterministic_priv_key().address # A non-wallet address to mine to - self.generatetoaddress(self.nodes[0], 1, gen_address(0)) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) assert_equal(self.nodes[0].getblockcount(), 1) - hashZ = self.generatetoaddress(self.nodes[1], 2, gen_address(1))[-1] + hashZ = self.generate(self.nodes[1], 2, sync_fun=self.no_op)[-1] assert_equal(self.nodes[1].getblockcount(), 2) node_sync_via_rpc(self.nodes[0:3]) assert_equal(self.nodes[0].getbestblockhash(), hashZ) self.log.info("Mine blocks A-B-C on Node 0") - hashC = self.generatetoaddress(self.nodes[0], 3, gen_address(0))[-1] + hashC = self.generate(self.nodes[0], 3, sync_fun=self.no_op)[-1] assert_equal(self.nodes[0].getblockcount(), 5) self.log.info("Mine competing blocks E-F-G on Node 1") - hashG = self.generatetoaddress(self.nodes[1], 3, gen_address(1))[-1] + hashG = self.generate(self.nodes[1], 3, sync_fun=self.no_op)[-1] assert_equal(self.nodes[1].getblockcount(), 5) assert hashC != hashG self.log.info("Connect nodes and check no reorg occurs") @@ -83,7 +82,7 @@ class PreciousTest(BitcoinTestFramework): self.nodes[1].preciousblock(hashC) assert_equal(self.nodes[1].getbestblockhash(), hashC) self.log.info("Mine another block (E-F-G-)H on Node 0 and reorg Node 1") - self.generatetoaddress(self.nodes[0], 1, gen_address(0)) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) assert_equal(self.nodes[0].getblockcount(), 6) self.sync_blocks(self.nodes[0:2]) hashH = self.nodes[0].getbestblockhash() @@ -92,7 +91,7 @@ class PreciousTest(BitcoinTestFramework): self.nodes[1].preciousblock(hashC) assert_equal(self.nodes[1].getbestblockhash(), hashH) self.log.info("Mine competing blocks I-J-K-L on Node 2") - self.generatetoaddress(self.nodes[2], 4, gen_address(2)) + self.generate(self.nodes[2], 4, sync_fun=self.no_op) assert_equal(self.nodes[2].getblockcount(), 6) hashL = self.nodes[2].getbestblockhash() self.log.info("Connect nodes and check no reorg occurs") diff --git a/test/functional/rpc_txoutproof.py b/test/functional/rpc_txoutproof.py index a45694328a..3cb11b4c18 100755 --- a/test/functional/rpc_txoutproof.py +++ b/test/functional/rpc_txoutproof.py @@ -43,7 +43,6 @@ class MerkleBlockTest(BitcoinTestFramework): self.generate(self.nodes[0], 1) blockhash = self.nodes[0].getblockhash(chain_height + 1) - self.sync_all() txlist = [] blocktxn = self.nodes[0].getblock(blockhash, True)["tx"] diff --git a/test/functional/test_framework/address.py b/test/functional/test_framework/address.py index fe733e9368..89839c9bab 100644 --- a/test/functional/test_framework/address.py +++ b/test/functional/test_framework/address.py @@ -5,12 +5,21 @@ """Encode and decode Bitcoin addresses. - base58 P2PKH and P2SH addresses. -- bech32 segwit v0 P2WPKH and P2WSH addresses.""" +- bech32 segwit v0 P2WPKH and P2WSH addresses. +- bech32m segwit v1 P2TR addresses.""" import enum import unittest -from .script import hash256, hash160, sha256, CScript, OP_0 +from .script import ( + CScript, + OP_0, + OP_TRUE, + hash160, + hash256, + sha256, + taproot_construct, +) from .segwit_addr import encode_segwit_address from .util import assert_equal @@ -29,6 +38,21 @@ class AddressType(enum.Enum): chars = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz' +def create_deterministic_address_bcrt1_p2tr_op_true(): + """ + Generates a deterministic bech32m address (segwit v1 output) that + can be spent with a witness stack of OP_TRUE and the control block + with internal public key (script-path spending). + + Returns a tuple with the generated address and the internal key. + """ + internal_key = (1).to_bytes(32, 'big') + scriptPubKey = taproot_construct(internal_key, [(None, CScript([OP_TRUE]))]).scriptPubKey + address = encode_segwit_address("bcrt", 1, scriptPubKey[2:]) + assert_equal(address, 'bcrt1p9yfmy5h72durp7zrhlw9lf7jpwjgvwdg0jr0lqmmjtgg83266lqsekaqka') + return (address, internal_key) + + def byte_to_base58(b, version): result = '' str = b.hex() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ec3561b1f2..b18c050e0a 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -19,7 +19,7 @@ import tempfile import time from typing import List -from .address import ADDRESS_BCRT1_P2WSH_OP_TRUE +from .address import create_deterministic_address_bcrt1_p2tr_op_true from .authproxy import JSONRPCException from . import coverage from .p2p import NetworkThread @@ -413,7 +413,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # To ensure that all nodes are out of IBD, the most recent block # must have a timestamp not too old (see IsInitialBlockDownload()). self.log.debug('Generate a block with current time') - block_hash = self.generate(self.nodes[0], 1)[0] + block_hash = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] block = self.nodes[0].getblock(blockhash=block_hash, verbosity=0) for n in self.nodes: n.submitblock(block) @@ -627,20 +627,27 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.connect_nodes(1, 2) self.sync_all() - def generate(self, generator, *args, **kwargs): + def no_op(self): + pass + + def generate(self, generator, *args, sync_fun=None, **kwargs): blocks = generator.generate(*args, invalid_call=False, **kwargs) + sync_fun() if sync_fun else self.sync_all() return blocks - def generateblock(self, generator, *args, **kwargs): + def generateblock(self, generator, *args, sync_fun=None, **kwargs): blocks = generator.generateblock(*args, invalid_call=False, **kwargs) + sync_fun() if sync_fun else self.sync_all() return blocks - def generatetoaddress(self, generator, *args, **kwargs): + def generatetoaddress(self, generator, *args, sync_fun=None, **kwargs): blocks = generator.generatetoaddress(*args, invalid_call=False, **kwargs) + sync_fun() if sync_fun else self.sync_all() return blocks - def generatetodescriptor(self, generator, *args, **kwargs): + def generatetodescriptor(self, generator, *args, sync_fun=None, **kwargs): blocks = generator.generatetodescriptor(*args, invalid_call=False, **kwargs) + sync_fun() if sync_fun else self.sync_all() return blocks def sync_blocks(self, nodes=None, wait=1, timeout=60): @@ -770,7 +777,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # block in the cache does not age too much (have an old tip age). # This is needed so that we are out of IBD when the test starts, # see the tip age check in IsInitialBlockDownload(). - gen_addresses = [k.address for k in TestNode.PRIV_KEYS][:3] + [ADDRESS_BCRT1_P2WSH_OP_TRUE] + gen_addresses = [k.address for k in TestNode.PRIV_KEYS][:3] + [create_deterministic_address_bcrt1_p2tr_op_true()[0]] assert_equal(len(gen_addresses), 4) for i in range(8): self.generatetoaddress( diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 9f5bca6884..ca1ffd48de 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -36,12 +36,12 @@ def assert_approx(v, vexp, vspan=0.00001): def assert_fee_amount(fee, tx_size, feerate_BTC_kvB): """Assert the fee is in range.""" - feerate_BTC_vB = feerate_BTC_kvB / 1000 - target_fee = satoshi_round(tx_size * feerate_BTC_vB) + target_fee = get_fee(tx_size, feerate_BTC_kvB) if fee < target_fee: raise AssertionError("Fee of %s BTC too low! (Should be %s BTC)" % (str(fee), str(target_fee))) # allow the wallet's estimation to be at most 2 bytes off - if fee > (tx_size + 2) * feerate_BTC_vB: + high_fee = get_fee(tx_size + 2, feerate_BTC_kvB) + if fee > high_fee: raise AssertionError("Fee of %s BTC too high! (Should be %s BTC)" % (str(fee), str(target_fee))) @@ -218,6 +218,18 @@ def str_to_b64str(string): return b64encode(string.encode('utf-8')).decode('ascii') +def ceildiv(a, b): + """Divide 2 ints and round up to next int rather than round down""" + return -(-a // b) + + +def get_fee(tx_size, feerate_btc_kvb): + """Calculate the fee in BTC given a feerate is BTC/kvB. Reflects CFeeRate::GetFee""" + feerate_sat_kvb = int(feerate_btc_kvb * Decimal(1e8)) # Fee in sat/kvb as an int to avoid float precision errors + target_fee_sat = ceildiv(feerate_sat_kvb * tx_size, 1000) # Round calculated fee up to nearest sat + return satoshi_round(target_fee_sat / Decimal(1e8)) # Truncate BTC result to nearest sat + + def satoshi_round(amount): return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN) @@ -451,10 +463,10 @@ def find_output(node, txid, amount, *, blockhash=None): # Helper to create at least "count" utxos # Pass in a fee that is sufficient for relay and mining new transactions. -def create_confirmed_utxos(test_framework, fee, node, count): +def create_confirmed_utxos(test_framework, fee, node, count, **kwargs): to_generate = int(0.5 * count) + 101 while to_generate > 0: - test_framework.generate(node, min(25, to_generate)) + test_framework.generate(node, min(25, to_generate), **kwargs) to_generate -= 25 utxos = node.listunspent() iterations = count - len(utxos) @@ -475,7 +487,7 @@ def create_confirmed_utxos(test_framework, fee, node, count): node.sendrawtransaction(signed_tx) while (node.getmempoolinfo()['size'] > 0): - test_framework.generate(node, 1) + test_framework.generate(node, 1, **kwargs) utxos = node.listunspent() assert len(utxos) >= count diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index d2c21f3525..7de6994735 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -9,7 +9,7 @@ from decimal import Decimal from enum import Enum from random import choice from typing import Optional -from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE +from test_framework.address import create_deterministic_address_bcrt1_p2tr_op_true from test_framework.descriptors import descsum_create from test_framework.key import ECKey from test_framework.messages import ( @@ -24,8 +24,9 @@ from test_framework.messages import ( from test_framework.script import ( CScript, LegacySignatureHash, - OP_TRUE, + LEAF_VERSION_TAPSCRIPT, OP_NOP, + OP_TRUE, SIGHASH_ALL, ) from test_framework.script_util import ( @@ -43,7 +44,7 @@ class MiniWalletMode(Enum): """Determines the transaction type the MiniWallet is creating and spending. For most purposes, the default mode ADDRESS_OP_TRUE should be sufficient; - it simply uses a fixed bech32 P2WSH address whose coins are spent with a + it simply uses a fixed bech32m P2TR address whose coins are spent with a witness stack of OP_TRUE, i.e. following an anyone-can-spend policy. However, if the transactions need to be modified by the user (e.g. prepending scriptSig for testing opcodes that are activated by a soft-fork), or the txs @@ -53,7 +54,7 @@ class MiniWalletMode(Enum): | output | | tx is | can modify | needs mode | description | address | standard | scriptSig | signing ----------------+-------------------+-----------+----------+------------+---------- - ADDRESS_OP_TRUE | anyone-can-spend | bech32 | yes | no | no + ADDRESS_OP_TRUE | anyone-can-spend | bech32m | yes | no | no RAW_OP_TRUE | anyone-can-spend | - (raw) | no | yes | no RAW_P2PK | pay-to-public-key | - (raw) | yes | yes | yes """ @@ -79,7 +80,7 @@ class MiniWallet: pub_key = self._priv_key.get_pubkey() self._scriptPubKey = key_to_p2pk_script(pub_key.get_bytes()) elif mode == MiniWalletMode.ADDRESS_OP_TRUE: - self._address = ADDRESS_BCRT1_P2WSH_OP_TRUE + self._address, self._internal_key = create_deterministic_address_bcrt1_p2tr_op_true() self._scriptPubKey = bytes.fromhex(self._test_node.validateaddress(self._address)['scriptPubKey']) def rescan_utxos(self): @@ -174,7 +175,7 @@ class MiniWallet: self._utxos = sorted(self._utxos, key=lambda k: (k['value'], -k['height'])) utxo_to_spend = utxo_to_spend or self._utxos.pop() # Pick the largest utxo (if none provided) and hope it covers the fee if self._priv_key is None: - vsize = Decimal(96) # anyone-can-spend + vsize = Decimal(104) # anyone-can-spend else: vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) send_value = int(COIN * (utxo_to_spend['value'] - fee_rate * (vsize / 1000))) @@ -191,10 +192,10 @@ class MiniWallet: self.sign_tx(tx) else: # anyone-can-spend - tx.vin[0].scriptSig = CScript([OP_NOP] * 35) # pad to identical size + tx.vin[0].scriptSig = CScript([OP_NOP] * 43) # pad to identical size else: tx.wit.vtxinwit = [CTxInWitness()] - tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])] + tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE]), bytes([LEAF_VERSION_TAPSCRIPT]) + self._internal_key] tx_hex = tx.serialize().hex() tx_info = from_node.testmempoolaccept([tx_hex])[0] diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 8f54e50598..edb3779f82 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -43,7 +43,6 @@ class AbandonConflictTest(BitcoinTestFramework): # Can not abandon confirmed transaction assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: self.nodes[0].abandontransaction(txid=txA)) - self.sync_blocks() newbalance = self.nodes[0].getbalance() assert balance - newbalance < Decimal("0.001") #no more than fees lost balance = newbalance @@ -167,7 +166,7 @@ class AbandonConflictTest(BitcoinTestFramework): tx = self.nodes[0].createrawtransaction(inputs, outputs) signed = self.nodes[0].signrawtransactionwithwallet(tx) self.nodes[1].sendrawtransaction(signed["hex"]) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) self.connect_nodes(0, 1) self.sync_blocks() diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 2abac7bb92..470cd9f34c 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -265,7 +265,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].invalidateblock(block_reorg) self.nodes[1].invalidateblock(block_reorg) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted - self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY) + self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY, sync_fun=self.no_op) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted # Now confirm tx_orig diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 92da54d97c..3839360eda 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -60,15 +60,14 @@ class WalletTest(BitcoinTestFramework): self.log.info("Mining blocks...") - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) walletinfo = self.nodes[0].getwalletinfo() assert_equal(walletinfo['immature_balance'], 50) assert_equal(walletinfo['balance'], 0) self.sync_all(self.nodes[0:3]) - self.generate(self.nodes[1], COINBASE_MATURITY + 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[1], COINBASE_MATURITY + 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) assert_equal(self.nodes[0].getbalance(), 50) assert_equal(self.nodes[1].getbalance(), 50) @@ -116,8 +115,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(walletinfo['immature_balance'], 0) # Have node0 mine a block, thus it will collect its own fee. - self.generate(self.nodes[0], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) # Exercise locking of unspent outputs unspent_0 = self.nodes[2].listunspent()[0] @@ -196,8 +194,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(len(self.nodes[1].listlockunspent()), 0) # Have node1 generate 100 blocks (so node0 can recover the fee) - self.generate(self.nodes[1], COINBASE_MATURITY) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[1], COINBASE_MATURITY, sync_fun=lambda: self.sync_all(self.nodes[0:3])) # node0 should end up with 100 btc in block rewards plus fees, but # minus the 21 plus fees sent to node2 @@ -225,8 +222,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[1].sendrawtransaction(hexstring=txns_to_send[1]["hex"], maxfeerate=0) # Have node1 mine a block to confirm transactions: - self.generate(self.nodes[1], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[1], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) assert_equal(self.nodes[0].getbalance(), 0) assert_equal(self.nodes[2].getbalance(), 94) @@ -240,15 +236,13 @@ class WalletTest(BitcoinTestFramework): fee_per_byte = Decimal('0.001') / 1000 self.nodes[2].settxfee(fee_per_byte * 1000) txid = self.nodes[2].sendtoaddress(address, 10, "", "", False) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(self.nodes[0].getbalance(), Decimal('10')) # Send 10 BTC with subtract fee from amount txid = self.nodes[2].sendtoaddress(address, 10, "", "", True) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) @@ -257,16 +251,14 @@ class WalletTest(BitcoinTestFramework): # Sendmany 10 BTC txid = self.nodes[2].sendmany('', {address: 10}, 0, "", []) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) node_0_bal += Decimal('10') node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(self.nodes[0].getbalance(), node_0_bal) # Sendmany 10 BTC with subtract fee from amount txid = self.nodes[2].sendmany('', {address: 10}, 0, "", [address]) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) @@ -278,8 +270,7 @@ class WalletTest(BitcoinTestFramework): # Test passing fee_rate as a string txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=str(fee_rate_sat_vb)) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) balance = self.nodes[2].getbalance() node_2_bal = self.check_fee_amount(balance, node_2_bal - Decimal('10'), explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(balance, node_2_bal) @@ -289,8 +280,7 @@ class WalletTest(BitcoinTestFramework): # Test passing fee_rate as an integer amount = Decimal("0.0001") txid = self.nodes[2].sendmany(amounts={address: amount}, fee_rate=fee_rate_sat_vb) - self.generate(self.nodes[2], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) balance = self.nodes[2].getbalance() node_2_bal = self.check_fee_amount(balance, node_2_bal - amount, explicit_fee_rate_btc_kvb, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(balance, node_2_bal) @@ -374,14 +364,12 @@ class WalletTest(BitcoinTestFramework): txid_not_broadcast = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2) tx_obj_not_broadcast = self.nodes[0].gettransaction(txid_not_broadcast) - self.generate(self.nodes[1], 1) # mine a block, tx should not be in there - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[1], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) # mine a block, tx should not be in there assert_equal(self.nodes[2].getbalance(), node_2_bal) # should not be changed because tx was not broadcasted # now broadcast from another node, mine a block, sync, and check the balance self.nodes[1].sendrawtransaction(tx_obj_not_broadcast['hex']) - self.generate(self.nodes[1], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[1], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) node_2_bal += 2 tx_obj_not_broadcast = self.nodes[0].gettransaction(txid_not_broadcast) assert_equal(self.nodes[2].getbalance(), node_2_bal) @@ -399,8 +387,7 @@ class WalletTest(BitcoinTestFramework): self.connect_nodes(0, 2) self.sync_blocks(self.nodes[0:3]) - self.generate(self.nodes[0], 1) - self.sync_blocks(self.nodes[0:3]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(self.nodes[0:3])) node_2_bal += 2 # tx should be added to balance because after restarting the nodes tx should be broadcast @@ -467,8 +454,7 @@ class WalletTest(BitcoinTestFramework): self.sync_mempools(self.nodes[0:3]) vout = find_vout_for_address(self.nodes[2], txid, address_to_import) self.nodes[2].lockunspent(False, [{"txid": txid, "vout": vout}]) - self.generate(self.nodes[0], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) self.log.info("Test sendtoaddress with fee_rate param (explicit fee rate in sat/vB)") prebalance = self.nodes[2].getbalance() @@ -480,8 +466,7 @@ class WalletTest(BitcoinTestFramework): # Test passing fee_rate as an integer txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb) tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) - self.generate(self.nodes[0], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) postbalance = self.nodes[2].getbalance() fee = prebalance - postbalance - Decimal(amount) assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb)) @@ -493,8 +478,7 @@ class WalletTest(BitcoinTestFramework): # Test passing fee_rate as a string txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=str(fee_rate_sat_vb)) tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) - self.generate(self.nodes[0], 1) - self.sync_all(self.nodes[0:3]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) postbalance = self.nodes[2].getbalance() fee = prebalance - postbalance - amount assert_fee_amount(fee, tx_size, Decimal(fee_rate_btc_kvb)) @@ -555,17 +539,15 @@ class WalletTest(BitcoinTestFramework): # Mine a block from node0 to an address from node1 coinbase_addr = self.nodes[1].getnewaddress() - block_hash = self.generatetoaddress(self.nodes[0], 1, coinbase_addr)[0] + block_hash = self.generatetoaddress(self.nodes[0], 1, coinbase_addr, sync_fun=lambda: self.sync_all(self.nodes[0:3]))[0] coinbase_txid = self.nodes[0].getblock(block_hash)['tx'][0] - self.sync_all(self.nodes[0:3]) # Check that the txid and balance is found by node1 self.nodes[1].gettransaction(coinbase_txid) # check if wallet or blockchain maintenance changes the balance self.sync_all(self.nodes[0:3]) - blocks = self.generate(self.nodes[0], 2) - self.sync_all(self.nodes[0:3]) + blocks = self.generate(self.nodes[0], 2, sync_fun=lambda: self.sync_all(self.nodes[0:3])) balance_nodes = [self.nodes[i].getbalance() for i in range(3)] block_count = self.nodes[0].getblockcount() @@ -606,13 +588,13 @@ class WalletTest(BitcoinTestFramework): # Get all non-zero utxos together chain_addrs = [self.nodes[0].getnewaddress(), self.nodes[0].getnewaddress()] singletxid = self.nodes[0].sendtoaddress(chain_addrs[0], self.nodes[0].getbalance(), "", "", True) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) node0_balance = self.nodes[0].getbalance() # Split into two chains rawtx = self.nodes[0].createrawtransaction([{"txid": singletxid, "vout": 0}], {chain_addrs[0]: node0_balance / 2 - Decimal('0.01'), chain_addrs[1]: node0_balance / 2 - Decimal('0.01')}) signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx) singletxid = self.nodes[0].sendrawtransaction(hexstring=signedtx["hex"], maxfeerate=0) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) # Make a long chain of unconfirmed payments without hitting mempool limit # Each tx we make leaves only one output of change on a chain 1 longer @@ -663,7 +645,7 @@ class WalletTest(BitcoinTestFramework): assert not address_info["ischange"] # Test getaddressinfo 'ischange' field on change address. - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) destination = self.nodes[1].getnewaddress() txid = self.nodes[0].sendtoaddress(destination, 0.123) tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 46a5df4a8e..34ee06b2fe 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -529,7 +529,7 @@ def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address): assert_equal([t for t in rbf_node.listunspent(minconf=0, include_unsafe=False) if t["txid"] == rbfid], []) # check that the main output from the rbf tx is spendable after confirmed - self.generate(rbf_node, 1) + self.generate(rbf_node, 1, sync_fun=self.no_op) assert_equal( sum(1 for t in rbf_node.listunspent(minconf=0, include_unsafe=False) if t["txid"] == rbfid and t["address"] == rbf_node_address and t["spendable"]), 1) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 27a2a42dac..6f9cd2e80f 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -181,7 +181,6 @@ class ImportRescanTest(BitcoinTestFramework): self.generate(self.nodes[0], 1) # Generate one block for each send variant.confirmation_height = self.nodes[0].getblockcount() variant.timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] - self.sync_all() # Conclude sync before calling setmocktime to avoid timeouts # Generate a block further in the future (past the rescan window). assert_equal(self.nodes[0].getrawmempool(), []) diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 4e8907bc3d..8289c6b8ce 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -62,8 +62,8 @@ class ImportMultiTest(BitcoinTestFramework): def run_test(self): self.log.info("Mining blocks...") - self.generate(self.nodes[0], 1) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] self.nodes[1].syncwithvalidationinterfacequeue() # Sync the timestamp to the wallet, so that importmulti works @@ -256,9 +256,9 @@ class ImportMultiTest(BitcoinTestFramework): # P2SH address multisig = get_multisig(self.nodes[0]) - self.generate(self.nodes[1], COINBASE_MATURITY) + self.generate(self.nodes[1], COINBASE_MATURITY, sync_fun=self.no_op) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] self.nodes[1].syncwithvalidationinterfacequeue() @@ -277,9 +277,9 @@ class ImportMultiTest(BitcoinTestFramework): # P2SH + Redeem script multisig = get_multisig(self.nodes[0]) - self.generate(self.nodes[1], COINBASE_MATURITY) + self.generate(self.nodes[1], COINBASE_MATURITY, sync_fun=self.no_op) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] self.nodes[1].syncwithvalidationinterfacequeue() @@ -298,9 +298,9 @@ class ImportMultiTest(BitcoinTestFramework): # P2SH + Redeem script + Private Keys + !Watchonly multisig = get_multisig(self.nodes[0]) - self.generate(self.nodes[1], COINBASE_MATURITY) + self.generate(self.nodes[1], COINBASE_MATURITY, sync_fun=self.no_op) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] self.nodes[1].syncwithvalidationinterfacequeue() @@ -324,9 +324,9 @@ class ImportMultiTest(BitcoinTestFramework): # P2SH + Redeem script + Private Keys + Watchonly multisig = get_multisig(self.nodes[0]) - self.generate(self.nodes[1], COINBASE_MATURITY) + self.generate(self.nodes[1], COINBASE_MATURITY, sync_fun=self.no_op) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] self.nodes[1].syncwithvalidationinterfacequeue() diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 79235646b0..5807a92b9d 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -193,7 +193,7 @@ class KeyPoolTest(BitcoinTestFramework): assert_equal("psbt" in res, True) # create a transaction without change at the maximum fee rate, such that the output is still spendable: - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.0008824}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00010000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.0008823}) assert_equal("psbt" in res, True) assert_equal(res["fee"], Decimal("0.00009706")) diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index f4a00a8ec8..815e3e6298 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -54,7 +54,6 @@ class ListSinceBlockTest(BitcoinTestFramework): blockhash, = self.generate(self.nodes[2], 1) blockheight = self.nodes[2].getblockheader(blockhash)['height'] - self.sync_all() txs = self.nodes[0].listtransactions() assert_array_result(txs, {"txid": txid}, { @@ -99,7 +98,6 @@ class ListSinceBlockTest(BitcoinTestFramework): self.log.info("Test target_confirmations") blockhash, = self.generate(self.nodes[2], 1) blockheight = self.nodes[2].getblockheader(blockhash)['height'] - self.sync_all() assert_equal( self.nodes[0].getblockhash(0), @@ -147,14 +145,11 @@ class ListSinceBlockTest(BitcoinTestFramework): senttx = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) # generate on both sides - nodes1_last_blockhash = self.generate(self.nodes[1], 6)[-1] - nodes2_first_blockhash = self.generate(self.nodes[2], 7)[0] + nodes1_last_blockhash = self.generate(self.nodes[1], 6, sync_fun=lambda: self.sync_all(self.nodes[:2]))[-1] + nodes2_first_blockhash = self.generate(self.nodes[2], 7, sync_fun=lambda: self.sync_all(self.nodes[2:]))[0] self.log.debug("nodes[1] last blockhash = {}".format(nodes1_last_blockhash)) self.log.debug("nodes[2] first blockhash = {}".format(nodes2_first_blockhash)) - self.sync_all(self.nodes[:2]) - self.sync_all(self.nodes[2:]) - self.join_network() # listsinceblock(nodes1_last_blockhash) should now include tx as seen from nodes[0] @@ -236,8 +231,8 @@ class ListSinceBlockTest(BitcoinTestFramework): self.nodes[2].createrawtransaction(utxo_dicts, recipient_dict2))['hex']) # generate on both sides - lastblockhash = self.generate(self.nodes[1], 3)[2] - self.generate(self.nodes[2], 4) + lastblockhash = self.generate(self.nodes[1], 3, sync_fun=self.no_op)[2] + self.generate(self.nodes[2], 4, sync_fun=self.no_op) self.join_network() @@ -308,7 +303,7 @@ class ListSinceBlockTest(BitcoinTestFramework): txid1 = self.nodes[1].sendrawtransaction(signedtx) # generate bb1-bb2 on right side - self.generate(self.nodes[2], 2) + self.generate(self.nodes[2], 2, sync_fun=self.no_op) # send from nodes[2]; this will end up in bb3 txid2 = self.nodes[2].sendrawtransaction(signedtx) @@ -316,8 +311,8 @@ class ListSinceBlockTest(BitcoinTestFramework): assert_equal(txid1, txid2) # generate on both sides - lastblockhash = self.generate(self.nodes[1], 3)[2] - self.generate(self.nodes[2], 2) + lastblockhash = self.generate(self.nodes[1], 3, sync_fun=self.no_op)[2] + self.generate(self.nodes[2], 2, sync_fun=self.no_op) self.join_network() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index ca6a6ab540..269ce6925d 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -38,7 +38,6 @@ class ListTransactionsTest(BitcoinTestFramework): self.log.info("Test confirmations change after mining a block") blockhash = self.generate(self.nodes[0], 1)[0] blockheight = self.nodes[0].getblockheader(blockhash)['height'] - self.sync_all() assert_array_result(self.nodes[0].listtransactions(), {"txid": txid}, {"category": "send", "amount": Decimal("-0.1"), "confirmations": 1, "blockhash": blockhash, "blockheight": blockheight}) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 68ca005649..1ed887191b 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -185,7 +185,7 @@ class MultiWalletTest(BitcoinTestFramework): self.nodes[0].createwallet("w5") assert_equal(set(node.listwallets()), {"w4", "w5"}) w5 = wallet("w5") - self.generatetoaddress(node, nblocks=1, address=w5.getnewaddress()) + self.generatetoaddress(node, nblocks=1, address=w5.getnewaddress(), sync_fun=self.no_op) # now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded os.rename(wallet_dir2, wallet_dir()) @@ -217,7 +217,7 @@ class MultiWalletTest(BitcoinTestFramework): wallet_bad = wallet("bad") # check wallet names and balances - self.generatetoaddress(node, nblocks=1, address=wallets[0].getnewaddress()) + self.generatetoaddress(node, nblocks=1, address=wallets[0].getnewaddress(), sync_fun=self.no_op) for wallet_name, wallet in zip(wallet_names, wallets): info = wallet.getwalletinfo() assert_equal(info['immature_balance'], 50 if wallet is wallets[0] else 0) @@ -230,7 +230,7 @@ class MultiWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo) w1, w2, w3, w4, *_ = wallets - self.generatetoaddress(node, nblocks=COINBASE_MATURITY + 1, address=w1.getnewaddress()) + self.generatetoaddress(node, nblocks=COINBASE_MATURITY + 1, address=w1.getnewaddress(), sync_fun=self.no_op) assert_equal(w1.getbalance(), 100) assert_equal(w2.getbalance(), 0) assert_equal(w3.getbalance(), 0) @@ -239,7 +239,7 @@ class MultiWalletTest(BitcoinTestFramework): w1.sendtoaddress(w2.getnewaddress(), 1) w1.sendtoaddress(w3.getnewaddress(), 2) w1.sendtoaddress(w4.getnewaddress(), 3) - self.generatetoaddress(node, nblocks=1, address=w1.getnewaddress()) + self.generatetoaddress(node, nblocks=1, address=w1.getnewaddress(), sync_fun=self.no_op) assert_equal(w2.getbalance(), 1) assert_equal(w3.getbalance(), 2) assert_equal(w4.getbalance(), 3) diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index 1f452f8337..9a86ede5f9 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -43,7 +43,7 @@ class ReorgsRestoreTest(BitcoinTestFramework): # Send a tx to be unconfirmed later txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) tx = self.nodes[0].gettransaction(txid) - self.generate(self.nodes[0], 4) + self.generate(self.nodes[0], 4, sync_fun=self.no_op) tx_before_reorg = self.nodes[0].gettransaction(txid) assert_equal(tx_before_reorg["confirmations"], 4) @@ -62,9 +62,9 @@ class ReorgsRestoreTest(BitcoinTestFramework): conflicting = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs_2)) conflicted_txid = self.nodes[0].sendrawtransaction(conflicted["hex"]) - self.generate(self.nodes[0], 1) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) conflicting_txid = self.nodes[2].sendrawtransaction(conflicting["hex"]) - self.generate(self.nodes[2], 9) + self.generate(self.nodes[2], 9, sync_fun=self.no_op) # Reconnect node0 and node2 and check that conflicted_txid is effectively conflicted self.connect_nodes(0, 2) @@ -78,11 +78,11 @@ class ReorgsRestoreTest(BitcoinTestFramework): self.restart_node(0) # The block chain re-orgs and the tx is included in a different block - self.generate(self.nodes[1], 9) + self.generate(self.nodes[1], 9, sync_fun=self.no_op) self.nodes[1].sendrawtransaction(tx["hex"]) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) self.nodes[1].sendrawtransaction(conflicted["hex"]) - self.generate(self.nodes[1], 1) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) # Node0 wallet file is loaded on longest sync'ed node1 self.stop_node(1) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index c9daeabeb9..ec8a4d33a3 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -448,7 +448,6 @@ class WalletSendTest(BitcoinTestFramework): res = self.nodes[0].sendrawtransaction(hex) self.generate(self.nodes[0], 1) assert_equal(self.nodes[0].gettransaction(txid)["confirmations"], 1) - self.sync_all() self.log.info("Lock unspents...") utxo1 = w0.listunspent()[0] diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index 80c125df97..b22c171374 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -272,11 +272,11 @@ class WalletTaprootTest(BitcoinTestFramework): boring_balance = int(self.boring.getbalance() * 100000000) to_amnt = random.randrange(1000000, boring_balance) self.boring.sendtoaddress(address=addr_g, amount=Decimal(to_amnt) / 100000000, subtractfeefromamount=True) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) test_balance = int(self.rpc_online.getbalance() * 100000000) ret_amnt = random.randrange(100000, test_balance) res = self.rpc_online.sendtoaddress(address=self.boring.getnewaddress(), amount=Decimal(ret_amnt) / 100000000, subtractfeefromamount=True) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert(self.rpc_online.gettransaction(res)["confirmations"] > 0) def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change): @@ -303,7 +303,7 @@ class WalletTaprootTest(BitcoinTestFramework): boring_balance = int(self.boring.getbalance() * 100000000) to_amnt = random.randrange(1000000, boring_balance) self.boring.sendtoaddress(address=addr_g, amount=Decimal(to_amnt) / 100000000, subtractfeefromamount=True) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) test_balance = int(self.psbt_online.getbalance() * 100000000) ret_amnt = random.randrange(100000, test_balance) psbt = self.psbt_online.walletcreatefundedpsbt([], [{self.boring.getnewaddress(): Decimal(ret_amnt) / 100000000}], None, {"subtractFeeFromOutputs":[0]})['psbt'] @@ -311,7 +311,7 @@ class WalletTaprootTest(BitcoinTestFramework): assert(res['complete']) rawtx = self.nodes[0].finalizepsbt(res['psbt'])['hex'] txid = self.nodes[0].sendrawtransaction(rawtx) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert(self.psbt_online.gettransaction(txid)['confirmations'] > 0) def do_test(self, comment, pattern, privmap, treefn, nkeys): @@ -343,7 +343,7 @@ class WalletTaprootTest(BitcoinTestFramework): self.log.info("Mining blocks...") gen_addr = self.boring.getnewaddress() - self.generatetoaddress(self.nodes[0], 101, gen_addr) + self.generatetoaddress(self.nodes[0], 101, gen_addr, sync_fun=self.no_op) self.do_test( "tr(XPRV)", @@ -412,7 +412,7 @@ class WalletTaprootTest(BitcoinTestFramework): self.log.info("Sending everything back...") txid = self.rpc_online.sendtoaddress(address=self.boring.getnewaddress(), amount=self.rpc_online.getbalance(), subtractfeefromamount=True) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert(self.rpc_online.gettransaction(txid)["confirmations"] > 0) psbt = self.psbt_online.walletcreatefundedpsbt([], [{self.boring.getnewaddress(): self.psbt_online.getbalance()}], None, {"subtractFeeFromOutputs": [0]})['psbt'] @@ -420,7 +420,7 @@ class WalletTaprootTest(BitcoinTestFramework): assert(res['complete']) rawtx = self.nodes[0].finalizepsbt(res['psbt'])['hex'] txid = self.nodes[0].sendrawtransaction(rawtx) - self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress()) + self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert(self.psbt_online.gettransaction(txid)['confirmations'] > 0) if __name__ == '__main__': diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index 7f178d7d46..5ca231cb76 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -93,8 +93,7 @@ class TxnMallTest(BitcoinTestFramework): # Have node0 mine a block, if requested: if (self.options.mine_block): - self.generate(self.nodes[0], 1) - self.sync_blocks(self.nodes[0:2]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(self.nodes[0:2])) tx1 = self.nodes[0].gettransaction(txid1) tx2 = self.nodes[0].gettransaction(txid2) @@ -123,7 +122,7 @@ class TxnMallTest(BitcoinTestFramework): return # ... mine a block... - self.generate(self.nodes[2], 1) + self.generate(self.nodes[2], 1, sync_fun=self.no_op) # Reconnect the split network, and sync chain: self.connect_nodes(1, 2) diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py index 150e4083b9..526e5cdd94 100755 --- a/test/functional/wallet_txn_doublespend.py +++ b/test/functional/wallet_txn_doublespend.py @@ -91,8 +91,7 @@ class TxnMallTest(BitcoinTestFramework): # Have node0 mine a block: if (self.options.mine_block): - self.generate(self.nodes[0], 1) - self.sync_blocks(self.nodes[0:2]) + self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(self.nodes[0:2])) tx1 = self.nodes[0].gettransaction(txid1) tx2 = self.nodes[0].gettransaction(txid2) @@ -120,7 +119,7 @@ class TxnMallTest(BitcoinTestFramework): self.nodes[2].sendrawtransaction(fund_bar_tx["hex"]) doublespend_txid = self.nodes[2].sendrawtransaction(doublespend["hex"]) # ... mine a block... - self.generate(self.nodes[2], 1) + self.generate(self.nodes[2], 1, sync_fun=self.no_op) # Reconnect the split network, and sync chain: self.connect_nodes(1, 2) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 5800880830..db03796df6 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -119,8 +119,7 @@ class UpgradeWalletTest(BitcoinTestFramework): assert_equal(wallet.getwalletinfo()["walletversion"], previous_version) def run_test(self): - self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 1, self.nodes[0].getnewaddress()) - self.dumb_sync_blocks() + self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 1, self.nodes[0].getnewaddress(), sync_fun=lambda: self.dumb_sync_blocks()) # # Sanity check the test framework: res = self.nodes[0].getblockchaininfo() assert_equal(res['blocks'], COINBASE_MATURITY + 1) @@ -131,8 +130,7 @@ class UpgradeWalletTest(BitcoinTestFramework): # Send coins to old wallets for later conversion checks. v16_3_wallet = v16_3_node.get_wallet_rpc('wallet.dat') v16_3_address = v16_3_wallet.getnewaddress() - self.generatetoaddress(node_master, COINBASE_MATURITY + 1, v16_3_address) - self.dumb_sync_blocks() + self.generatetoaddress(node_master, COINBASE_MATURITY + 1, v16_3_address, sync_fun=lambda: self.dumb_sync_blocks()) v16_3_balance = v16_3_wallet.getbalance() self.log.info("Test upgradewallet RPC...") diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 7db051ca37..3c5a15a0c7 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -41,3 +41,6 @@ race:CZMQAbstractPublishNotifier::SendZmqMessage # https://github.com/bitcoin/bitcoin/pull/20218, https://github.com/bitcoin/bitcoin/pull/20745 race:epoll_ctl + +# https://github.com/bitcoin/bitcoin/issues/23366 +race:std::__1::ios_base::width |