diff options
99 files changed, 696 insertions, 403 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 0c43e61592..ea587b78d4 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -38,17 +38,6 @@ after_build: - ps: fsutil behavior set disablelastaccess 1 # Disable Access time feature on Windows (better performance) - ps: clcache -z before_test: -- ps: ${conf_ini} = (Get-Content([IO.Path]::Combine(${env:APPVEYOR_BUILD_FOLDER}, "test", "config.ini.in"))) -- ps: ${conf_ini} = ${conf_ini}.Replace("@PACKAGE_NAME@", "Bitcoin Core") -- ps: ${conf_ini} = ${conf_ini}.Replace("@abs_top_srcdir@", ${env:APPVEYOR_BUILD_FOLDER}) -- ps: ${conf_ini} = ${conf_ini}.Replace("@abs_top_builddir@", ${env:APPVEYOR_BUILD_FOLDER}) -- ps: ${conf_ini} = ${conf_ini}.Replace("@EXEEXT@", ".exe") -- ps: ${conf_ini} = ${conf_ini}.Replace("@ENABLE_WALLET_TRUE@", "") -- ps: ${conf_ini} = ${conf_ini}.Replace("@BUILD_BITCOIN_CLI_TRUE@", "") -- ps: ${conf_ini} = ${conf_ini}.Replace("@BUILD_BITCOIND_TRUE@", "") -- ps: ${conf_ini} = ${conf_ini}.Replace("@ENABLE_ZMQ_TRUE@", "") -- ps: ${utf8} = New-Object System.Text.UTF8Encoding ${false} -- ps: '[IO.File]::WriteAllLines([IO.Path]::Combine(${env:APPVEYOR_BUILD_FOLDER}, "test", "config.ini"), ${conf_ini}, ${utf8})' - ps: move "build_msvc\${env:PLATFORM}\${env:CONFIGURATION}\*.exe" src test_script: - cmd: src\test_bitcoin.exe -k stdout -e stdout 2> NUL diff --git a/build_msvc/bitcoind/bitcoind.vcxproj b/build_msvc/bitcoind/bitcoind.vcxproj index 2c5dde3015..c5cddfdbb2 100644 --- a/build_msvc/bitcoind/bitcoind.vcxproj +++ b/build_msvc/bitcoind/bitcoind.vcxproj @@ -45,4 +45,30 @@ </ItemGroup> <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> + <Import Label="ReplaceInFile" Project="..\msbuild\tasks\replaceinfile.targets" /> + <PropertyGroup> + <ConfigIniIn>..\..\test\config.ini.in</ConfigIniIn> + <ConfigIniOut>..\..\test\config.ini</ConfigIniOut> + </PropertyGroup> + <Target Name="AfterBuild"> + <Copy SourceFiles="$(ConfigIniIn)" DestinationFiles="$(ConfigIniOut)" ></Copy> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@PACKAGE_NAME@" By="Bitcoin Core"></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@abs_top_srcdir@" By="..\.." ToFullPath="true"></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@abs_top_builddir@" By="..\.." ToFullPath="true"></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@EXEEXT@" By=".exe"></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@ENABLE_WALLET_TRUE@" By=""></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@BUILD_BITCOIN_CLI_TRUE@" By=""></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@BUILD_BITCOIND_TRUE@" By=""></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@ENABLE_FUZZ_TRUE@" By=""></ReplaceInFile> + <ReplaceInFile FilePath="$(ConfigIniOut)" + Replace="@ENABLE_ZMQ_TRUE@" By=""></ReplaceInFile> + </Target> </Project> diff --git a/build_msvc/common.init.vcxproj b/build_msvc/common.init.vcxproj index 8b6217e2b0..f08ac78b21 100644 --- a/build_msvc/common.init.vcxproj +++ b/build_msvc/common.init.vcxproj @@ -115,4 +115,5 @@ <AdditionalDependencies>crypt32.lib;Iphlpapi.lib;ws2_32.lib;Shlwapi.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies)</AdditionalDependencies> </Link> </ItemDefinitionGroup> + <Import Project="common.init.vcxproj.user" Condition="Exists('common.init.vcxproj.user')" /> </Project> diff --git a/build_msvc/msbuild/tasks/replaceinfile.targets b/build_msvc/msbuild/tasks/replaceinfile.targets new file mode 100644 index 0000000000..2ccb8b30e0 --- /dev/null +++ b/build_msvc/msbuild/tasks/replaceinfile.targets @@ -0,0 +1,35 @@ +<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <UsingTask + TaskName="ReplaceInFile" + TaskFactory="CodeTaskFactory" + AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll" > + <ParameterGroup> + <FilePath Required="true" /> + <Replace Required="true" /> + <By Required="false" /> + <ToFullPath Required="false" /> + </ParameterGroup> + <Task> + <Using Namespace="System"/> + <Using Namespace="System.IO"/> + <Code Type="Fragment" Language="cs"> +<![CDATA[ +if(File.Exists(FilePath) == false) { + Log.LogError("replaceinfile task could not locate " + FilePath + "."); +} +else { + var data = File.ReadAllText(FilePath); + var by = By; + if (ToFullPath == "true") + { + by = Path.GetFullPath(by); + } + data = data.Replace(Replace, by); + Log.LogMessage("Replace '" + Replace + "' by '" + by + "' in " + FilePath); + File.WriteAllText(FilePath, data, new System.Text.UTF8Encoding(false)); +} +]]> + </Code> + </Task> + </UsingTask> +</Project>
\ No newline at end of file diff --git a/contrib/devtools/copyright_header.py b/contrib/devtools/copyright_header.py index f2987f2260..fc01e570aa 100755 --- a/contrib/devtools/copyright_header.py +++ b/contrib/devtools/copyright_header.py @@ -34,7 +34,7 @@ EXCLUDE_DIRS = [ "src/univalue/", ] -INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.py'] +INCLUDE = ['*.h', '*.cpp', '*.cc', '*.c', '*.mm', '*.py'] INCLUDE_COMPILED = re.compile('|'.join([fnmatch.translate(m) for m in INCLUDE])) def applies_to_file(filename): @@ -90,14 +90,12 @@ def compile_copyright_regex(copyright_style, year_style, name): EXPECTED_HOLDER_NAMES = [ "Satoshi Nakamoto\n", "The Bitcoin Core developers\n", - "Bitcoin Core Developers\n", "BitPay Inc\.\n", "University of Illinois at Urbana-Champaign\.\n", "Pieter Wuille\n", "Wladimir J. van der Laan\n", "Jeff Garzik\n", "Jan-Klaas Kollhof\n", - "Sam Rushing\n", "ArtForz -- public domain half-a-node\n", "Intel Corporation", "The Zcash developers", diff --git a/contrib/devtools/github-merge.py b/contrib/devtools/github-merge.py index cd7a271e83..78ac671bfe 100755 --- a/contrib/devtools/github-merge.py +++ b/contrib/devtools/github-merge.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2016-2017 Bitcoin Core Developers +# Copyright (c) 2016-2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -411,4 +411,3 @@ def main(): if __name__ == '__main__': main() - diff --git a/depends/packages/rapidcheck.mk b/depends/packages/rapidcheck.mk index a35e091c80..fa4fb3c782 100644 --- a/depends/packages/rapidcheck.mk +++ b/depends/packages/rapidcheck.mk @@ -8,6 +8,10 @@ define $(package)_config_cmds cmake -DCMAKE_INSTALL_PREFIX=$($(package)_staging_dir)$(host_prefix) -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=true -DRC_INSTALL_ALL_EXTRAS=ON endef +define $(package)_preprocess_cmds + sed -i.old 's/ -Wall//' CMakeLists.txt +endef + define $(package)_build_cmds $(MAKE) rapidcheck endef diff --git a/doc/build-osx.md b/doc/build-osx.md index d28a3d97aa..65cfce6b15 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -1,33 +1,37 @@ -macOS Build Instructions and Notes -==================================== +# macOS Build Instructions and Notes + The commands in this guide should be executed in a Terminal application. -The built-in one is located in `/Applications/Utilities/Terminal.app`. +The built-in one is located in +``` +/Applications/Utilities/Terminal.app +``` -Preparation ------------ +## Preparation Install the macOS command line tools: -`xcode-select --install` +```shell +xcode-select --install +``` When the popup appears, click `Install`. Then install [Homebrew](https://brew.sh). -Dependencies ----------------------- - - brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python qt libevent qrencode +## Dependencies +```shell +brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python qt libevent qrencode +``` See [dependencies.md](dependencies.md) for a complete overview. If you want to build the disk image with `make deploy` (.dmg / optional), you need RSVG: +```shell +brew install librsvg +``` - brew install librsvg - -Berkeley DB ------------ +## 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) +you can use [this](/contrib/install_db4.sh) script to install it like so: ```shell @@ -38,172 +42,167 @@ from the root of the repository. **Note**: You only need Berkeley DB if the wallet is enabled (see [*Disable-wallet mode*](/doc/build-osx.md#disable-wallet-mode)). -Build Bitcoin Core ------------------------- +## Build Bitcoin Core 1. Clone the Bitcoin Core source code: - - git clone https://github.com/bitcoin/bitcoin - cd bitcoin + ```shell + git clone https://github.com/bitcoin/bitcoin + cd bitcoin + ``` 2. Build Bitcoin Core: Configure and build the headless Bitcoin Core binaries as well as the GUI (if Qt is found). You can disable the GUI build by passing `--without-gui` to configure. - - ./autogen.sh - ./configure - make + ```shell + ./autogen.sh + ./configure + make + ``` 3. It is recommended to build and run the unit tests: - - make check - -4. You can also create a .dmg that contains the .app bundle (optional): - - make deploy - -Disable-wallet mode --------------------- -When the intention is to run only a P2P node without a wallet, Bitcoin Core may be compiled in -disable-wallet mode with: - - ./configure --disable-wallet + ```shell + make check + ``` + +4. You can also create a `.dmg` that contains the `.app` bundle (optional): + ```shell + make deploy + ``` + +## `disable-wallet` mode +When the intention is to run only a P2P node without a wallet, Bitcoin Core may be +compiled in `disable-wallet` mode with: +```shell +./configure --disable-wallet +``` In this case there is no dependency on Berkeley DB 4.8. Mining is also possible in disable-wallet mode using the `getblocktemplate` RPC call. -Running -------- - +## Running Bitcoin Core is now available at `./src/bitcoind` Before running, you may create an empty configuration file: +```shell +mkdir -p "/Users/${USER}/Library/Application Support/Bitcoin" - mkdir -p "/Users/${USER}/Library/Application Support/Bitcoin" - - touch "/Users/${USER}/Library/Application Support/Bitcoin/bitcoin.conf" +touch "/Users/${USER}/Library/Application Support/Bitcoin/bitcoin.conf" - chmod 600 "/Users/${USER}/Library/Application Support/Bitcoin/bitcoin.conf" +chmod 600 "/Users/${USER}/Library/Application Support/Bitcoin/bitcoin.conf" +``` -The first time you run bitcoind, it will start downloading the blockchain. This process could take many hours, or even days on slower than average systems. +The first time you run bitcoind, it will start downloading the blockchain. This process could +take many hours, or even days on slower than average systems. You can monitor the download process by looking at the debug.log file: +```shell +tail -f $HOME/Library/Application\ Support/Bitcoin/debug.log +``` - tail -f $HOME/Library/Application\ Support/Bitcoin/debug.log - -Other commands: -------- - - ./src/bitcoind -daemon # Starts the bitcoin daemon. - ./src/bitcoin-cli --help # Outputs a list of command-line options. - ./src/bitcoin-cli help # Outputs a list of RPC commands when the daemon is running. - -Notes ------ - -* Tested on OS X 10.10 Yosemite through macOS 10.13 High Sierra on 64-bit Intel processors only. - -* Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714) +## Other commands: +```shell +./src/bitcoind -daemon # Starts the bitcoin daemon. +./src/bitcoin-cli --help # Outputs a list of command-line options. +./src/bitcoin-cli help # Outputs a list of RPC commands when the daemon is running. +``` -Deterministic macOS DMG Notes ------------------------------ +## Notes +* Tested on OS X 10.10 Yosemite through macOS 10.14 Mojave on 64-bit Intel +processors only. +* Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714). -Working macOS DMGs are created in Linux by combining a recent clang, -the Apple binutils (ld, ar, etc) and DMG authoring tools. +## Deterministic macOS DMG Notes +Working macOS DMGs are created in Linux by combining a recent `clang`, the Apple +`binutils` (`ld`, `ar`, etc) and DMG authoring tools. -Apple uses clang extensively for development and has upstreamed the necessary -functionality so that a vanilla clang can take advantage. It supports the use -of -F, -target, -mmacosx-version-min, and --sysroot, which are all necessary -when building for macOS. +Apple uses `clang` extensively for development and has upstreamed the necessary +functionality so that a vanilla clang can take advantage. It supports the use of `-F`, +`-target`, `-mmacosx-version-min`, and `--sysroot`, which are all necessary when +building for macOS. -Apple's version of binutils (called cctools) contains lots of functionality -missing in the FSF's binutils. In addition to extra linker options for -frameworks and sysroots, several other tools are needed as well such as -install_name_tool, lipo, and nmedit. These do not build under linux, so they -have been patched to do so. The work here was used as a starting point: -[mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). +Apple's version of `binutils` (called `cctools`) contains lots of functionality missing in the +FSF's `binutils`. In addition to extra linker options for frameworks and sysroots, several +other tools are needed as well such as `install_name_tool`, `lipo`, and `nmedit`. These +do not build under Linux, so they have been patched to do so. The work here was used as +a starting point: [mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). -In order to build a working toolchain, the following source packages are needed -from Apple: cctools, dyld, and ld64. +In order to build a working toolchain, the following source packages are needed from +Apple: `cctools`, `dyld`, and `ld64`. -These tools inject timestamps by default, which produce non-deterministic -binaries. The ZERO_AR_DATE environment variable is used to disable that. +These tools inject timestamps by default, which produce non-deterministic binaries. The +`ZERO_AR_DATE` environment variable is used to disable that. -This version of cctools has been patched to use the current version of clang's -headers and its libLTO.so rather than those from llvmgcc, as it was -originally done in toolchain4. +This version of `cctools` has been patched to use the current version of `clang`'s headers +and its `libLTO.so` rather than those from `llvmgcc`, as it was originally done in `toolchain4`. -To complicate things further, all builds must target an Apple SDK. These SDKs -are free to download, but not redistributable. -To obtain it, register for a developer account, then download the [Xcode 7.3.1 dmg](https://developer.apple.com/devcenter/download.action?path=/Developer_Tools/Xcode_7.3.1/Xcode_7.3.1.dmg). +To complicate things further, all builds must target an Apple SDK. These SDKs are free to +download, but not redistributable. To obtain it, register for an Apple Developer Account, +then download the [Xcode 7.3.1 dmg](https://developer.apple.com/devcenter/download.action?path=/Developer_Tools/Xcode_7.3.1/Xcode_7.3.1.dmg). -This file is several gigabytes in size, but only a single directory inside is -needed: +This file is several gigabytes in size, but only a single directory inside is needed: ``` Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk ``` -Unfortunately, the usual linux tools (7zip, hpmount, loopback mount) are incapable of opening this file. -To create a tarball suitable for Gitian input, there are two options: +Unfortunately, the usual Linux tools (7zip, hpmount, loopback mount) are incapable of +opening this file. To create a tarball suitable for Gitian input, there are two options: -Using macOS, you can mount the dmg, and then create it with: -``` - $ hdiutil attach Xcode_7.3.1.dmg - $ tar -C /Volumes/Xcode/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/ -czf MacOSX10.11.sdk.tar.gz MacOSX10.11.sdk +Using macOS, you can mount the DMG, and then create it with: +```shell +hdiutil attach Xcode_7.3.1.dmg +tar -C /Volumes/Xcode/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/ -czf MacOSX10.11.sdk.tar.gz MacOSX10.11.sdk ``` -Alternatively, you can use 7zip and SleuthKit to extract the files one by one. -The script contrib/macdeploy/extract-osx-sdk.sh automates this. First ensure -the dmg file is in the current directory, and then run the script. You may wish -to delete the intermediate 5.hfs file and MacOSX10.11.sdk (the directory) when -you've confirmed the extraction succeeded. +Alternatively, you can use 7zip and SleuthKit to extract the files one by one. The script +[`extract-osx-sdk.sh`](./../contrib/macdeploy/extract-osx-sdk.sh) automates this. First +ensure the DMG file is in the current directory, and then run the script. You may wish to +delete the `intermediate 5.hfs` file and `MacOSX10.11.sdk` (the directory) when you've +confirmed the extraction succeeded. -```bash +```shell apt-get install p7zip-full sleuthkit contrib/macdeploy/extract-osx-sdk.sh rm -rf 5.hfs MacOSX10.11.sdk ``` -The Gitian descriptors build 2 sets of files: Linux tools, then Apple binaries -which are created using these tools. The build process has been designed to -avoid including the SDK's files in Gitian's outputs. All interim tarballs are -fully deterministic and may be freely redistributed. +The Gitian descriptors build 2 sets of files: Linux tools, then Apple binaries which are +created using these tools. The build process has been designed to avoid including the +SDK's files in Gitian's outputs. All interim tarballs are fully deterministic and may be freely +redistributed. -genisoimage is used to create the initial DMG. It is not deterministic as-is, -so it has been patched. A system genisoimage will work fine, but it will not -be deterministic because the file-order will change between invocations. -The patch can be seen here: [theuni/osx-cross-depends](https://raw.githubusercontent.com/theuni/osx-cross-depends/master/patches/cdrtools/genisoimage.diff). -No effort was made to fix this cleanly, so it likely leaks memory badly. But -it's only used for a single invocation, so that's no real concern. +`genisoimage` is used to create the initial DMG. It is not deterministic as-is, so it has been +patched. A system `genisoimage` will work fine, but it will not be deterministic because +the file-order will change between invocations. The patch can be seen here: [theuni/osx-cross-depends](https://raw.githubusercontent.com/theuni/osx-cross-depends/master/patches/cdrtools/genisoimage.diff). +No effort was made to fix this cleanly, so it likely leaks memory badly. But it's only used for +a single invocation, so that's no real concern. -genisoimage cannot compress DMGs, so afterwards, the 'dmg' tool from the -libdmg-hfsplus project is used to compress it. There are several bugs in this -tool and its maintainer has seemingly abandoned the project. It has been forked -and is available (with fixes) here: [theuni/libdmg-hfsplus](https://github.com/theuni/libdmg-hfsplus). +`genisoimage` cannot compress DMGs, so afterwards, the DMG tool from the +`libdmg-hfsplus` project is used to compress it. There are several bugs in this tool and its +maintainer has seemingly abandoned the project. It has been forked and is available +(with fixes) here: [theuni/libdmg-hfsplus](https://github.com/theuni/libdmg-hfsplus). -The 'dmg' tool has the ability to create DMGs from scratch as well, but this -functionality is broken. Only the compression feature is currently used. -Ideally, the creation could be fixed and genisoimage would no longer be necessary. +The DMG tool has the ability to create DMGs from scratch as well, but this functionality is +broken. Only the compression feature is currently used. Ideally, the creation could be fixed +and `genisoimage` would no longer be necessary. Background images and other features can be added to DMG files by inserting a -.DS_Store before creation. This is generated by the script -contrib/macdeploy/custom_dsstore.py. - -As of OS X 10.9 Mavericks, using an Apple-blessed key to sign binaries is a -requirement in order to satisfy the new Gatekeeper requirements. Because this -private key cannot be shared, we'll have to be a bit creative in order for the -build process to remain somewhat deterministic. Here's how it works: - -- Builders use Gitian to create an unsigned release. This outputs an unsigned - dmg which users may choose to bless and run. It also outputs an unsigned app - structure in the form of a tarball, which also contains all of the tools - that have been previously (deterministically) built in order to create a - final dmg. -- The Apple keyholder uses this unsigned app to create a detached signature, - using the script that is also included there. Detached signatures are available from this [repository](https://github.com/bitcoin-core/bitcoin-detached-sigs). -- Builders feed the unsigned app + detached signature back into Gitian. It - uses the pre-built tools to recombine the pieces into a deterministic dmg. +`.DS_Store` before creation. This is generated by the script +`contrib/macdeploy/custom_dsstore.py`. + +As of OS X 10.9 Mavericks, using an Apple-blessed key to sign binaries is a requirement in +order to satisfy the new Gatekeeper requirements. Because this private key cannot be +shared, we'll have to be a bit creative in order for the build process to remain somewhat +deterministic. Here's how it works: + +- Builders use Gitian to create an unsigned release. This outputs an unsigned DMG which + users may choose to bless and run. It also outputs an unsigned app structure in the form + of a tarball, which also contains all of the tools that have been previously (deterministically) + built in order to create a final DMG. +- The Apple keyholder uses this unsigned app to create a detached signature, using the + script that is also included there. Detached signatures are available from this [repository](https://github.com/bitcoin-core/bitcoin-detached-sigs). +- Builders feed the unsigned app + detached signature back into Gitian. It uses the + pre-built tools to recombine the pieces into a deterministic DMG. diff --git a/doc/release-notes-0.18.1-16257.md b/doc/release-notes-0.18.1-16257.md new file mode 100644 index 0000000000..21867b7fb2 --- /dev/null +++ b/doc/release-notes-0.18.1-16257.md @@ -0,0 +1,6 @@ +Wallet changes +-------------- +When creating a transaction with a fee above `-maxtxfee` (default 0.1 BTC), +the RPC commands `walletcreatefundedpsbt` and `fundrawtransaction` will now fail +instead of rounding down the fee. Beware that the `feeRate` argument is specified +in BTC per kilobyte, not satoshi per byte. diff --git a/src/addrdb.cpp b/src/addrdb.cpp index c6083f5554..db936486b6 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -44,18 +44,30 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data fs::path pathTmp = GetDataDir() / tmpfn; FILE *file = fsbridge::fopen(pathTmp, "wb"); CAutoFile fileout(file, SER_DISK, CLIENT_VERSION); - if (fileout.IsNull()) + if (fileout.IsNull()) { + fileout.fclose(); + remove(pathTmp); return error("%s: Failed to open file %s", __func__, pathTmp.string()); + } // Serialize - if (!SerializeDB(fileout, data)) return false; - if (!FileCommit(fileout.Get())) + if (!SerializeDB(fileout, data)) { + fileout.fclose(); + remove(pathTmp); + return false; + } + if (!FileCommit(fileout.Get())) { + fileout.fclose(); + remove(pathTmp); return error("%s: Failed to flush file %s", __func__, pathTmp.string()); + } fileout.fclose(); // replace existing file, if any, with new file - if (!RenameOver(pathTmp, path)) + if (!RenameOver(pathTmp, path)) { + remove(pathTmp); return error("%s: Rename-into-place failed", __func__); + } return true; } diff --git a/src/policy/fees.h b/src/policy/fees.h index 6e61f76178..16683bf5ad 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -43,7 +43,6 @@ enum class FeeReason { PAYTXFEE, FALLBACK, REQUIRED, - MAXTXFEE, }; /* Used to determine type of fee estimation requested */ diff --git a/src/psbt.cpp b/src/psbt.cpp index d765133190..fe74002e82 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -212,6 +212,25 @@ bool PSBTInputSigned(const PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index) +{ + const CTxOut& out = psbt.tx->vout.at(index); + PSBTOutput& psbt_out = psbt.outputs.at(index); + + // Fill a SignatureData with output info + SignatureData sigdata; + psbt_out.FillSignatureData(sigdata); + + // Construct a would-be spend of this output, to update sigdata with. + // Note that ProduceSignature is used to fill in metadata (not actual signatures), + // so provider does not need to provide any private keys (it can be a HidingSigningProvider). + MutableTransactionSignatureCreator creator(psbt.tx.get_ptr(), /* index */ 0, out.nValue, SIGHASH_ALL); + ProduceSignature(provider, creator, out.scriptPubKey, sigdata); + + // Put redeem_script, witness_script, key paths, into PSBTOutput. + psbt_out.FromSignatureData(sigdata); +} + bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash, SignatureData* out_sigdata, bool use_dummy) { PSBTInput& input = psbt.inputs.at(index); diff --git a/src/psbt.h b/src/psbt.h index 1bc1e91a84..f3840b9ed3 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -565,6 +565,12 @@ bool PSBTInputSigned(const PSBTInput& input); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false); +/** Updates a PSBTOutput with information from provider. + * + * This fills in the redeem_script, witness_script, and hd_keypaths where possible. + */ +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index); + /** * Finalizes a PSBT if possible, combining partial signatures. * diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index ea1019ad1d..11a518ebd2 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -12,7 +12,6 @@ #include <key.h> #include <key_io.h> -#include <pubkey.h> #include <wallet/wallet.h> #include <QApplication> diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 22e49b06cb..49e9e072a8 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -11,6 +11,8 @@ #include <qt/networkstyle.h> #include <qt/rpcconsole.h> #include <shutdown.h> +#include <test/setup_common.h> +#include <univalue.h> #include <validation.h> #if defined(HAVE_CONFIG_H) @@ -26,9 +28,6 @@ #include <QtGlobal> #include <QtTest/QtTestWidgets> #include <QtTest/QtTestGui> -#include <new> -#include <string> -#include <univalue.h> namespace { //! Call getblockchaininfo RPC and check first field of JSON output. @@ -63,6 +62,7 @@ void AppTests::appTests() } #endif + BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference LogInstance().DisconnectTestLogger(); diff --git a/src/qt/test/paymentservertests.cpp b/src/qt/test/paymentservertests.cpp index f0eca899fc..6cafe05461 100644 --- a/src/qt/test/paymentservertests.cpp +++ b/src/qt/test/paymentservertests.cpp @@ -13,7 +13,7 @@ #include <random.h> #include <script/script.h> #include <script/standard.h> -#include <util/system.h> +#include <test/setup_common.h> #include <util/strencodings.h> #include <openssl/x509.h> @@ -66,7 +66,7 @@ static SendCoinsRecipient handleRequest(PaymentServer* server, std::vector<unsig void PaymentServerTests::paymentServerTests() { - SelectParams(CBaseChainParams::MAIN); + BasicTestingSetup testing_setup(CBaseChainParams::MAIN); auto node = interfaces::MakeNode(); OptionsModel optionsModel(*node); PaymentServer* server = new PaymentServer(nullptr, false); diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 1e40bd7209..3c2ffa6c00 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -4,7 +4,6 @@ #include <qt/test/rpcnestedtests.h> -#include <fs.h> #include <interfaces/node.h> #include <rpc/server.h> #include <qt/rpcconsole.h> @@ -35,9 +34,6 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); - ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference - LogInstance().DisconnectTestLogger(); - TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/qt/test/rpcnestedtests.h b/src/qt/test/rpcnestedtests.h index e33f4e3da1..97143ff78a 100644 --- a/src/qt/test/rpcnestedtests.h +++ b/src/qt/test/rpcnestedtests.h @@ -8,9 +8,6 @@ #include <QObject> #include <QTest> -#include <txdb.h> -#include <txmempool.h> - class RPCNestedTests : public QObject { Q_OBJECT diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 9f66c3d3a9..6bda8dc6eb 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -6,7 +6,6 @@ #include <config/bitcoin-config.h> #endif -#include <chainparams.h> #include <interfaces/node.h> #include <qt/bitcoin.h> #include <qt/test/apptests.h> @@ -43,12 +42,18 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); #endif #endif -extern void noui_connect(); - // This is all you need to run all the tests int main(int argc, char *argv[]) { - BasicTestingSetup test{CBaseChainParams::REGTEST}; + // Initialize persistent globals with the testing setup state for sanity. + // E.g. -datadir in gArgs is set to a temp directory dummy value (instead + // of defaulting to the default datadir), or globalChainParams is set to + // regtest params. + // + // All tests must use their own testing setup (if needed). + { + BasicTestingSetup dummy{CBaseChainParams::REGTEST}; + } auto node = interfaces::MakeNode(); diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index 1588be8da3..006007be63 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -104,6 +104,7 @@ void TrafficGraphWidget::paintEvent(QPaintEvent *) } } + painter.setRenderHint(QPainter::Antialiasing); if(!vSamplesIn.empty()) { QPainterPath p; paintPath(p, vSamplesIn); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index a2b295df21..c1eba61749 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -221,9 +221,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact return TransactionCreationFailed; } - // reject absurdly high fee. (This can never happen because the - // wallet caps the fee at m_default_max_tx_fee. This merely serves as a - // belt-and-suspenders check) + // Reject absurdly high fee if (nFeeRequired > m_wallet->getDefaultMaxTxFee()) return AbsurdFee; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 50c4589d9f..743efc1e65 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -14,7 +14,6 @@ #include <core_io.h> #include <hash.h> #include <index/blockfilterindex.h> -#include <key_io.h> #include <policy/feerate.h> #include <policy/policy.h> #include <policy/rbf.h> @@ -42,9 +41,9 @@ #include <boost/thread/thread.hpp> // boost::thread::interrupt +#include <condition_variable> #include <memory> #include <mutex> -#include <condition_variable> struct CUpdatedBlock { @@ -169,7 +168,8 @@ static UniValue getblockcount(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 0) throw std::runtime_error( RPCHelpMan{"getblockcount", - "\nReturns the number of blocks in the longest blockchain.\n", + "\nReturns the height of the most-work fully-validated chain.\n" + "The genesis block has height 0.\n", {}, RPCResult{ "n (numeric) The current block count\n" @@ -189,7 +189,7 @@ static UniValue getbestblockhash(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 0) throw std::runtime_error( RPCHelpMan{"getbestblockhash", - "\nReturns the hash of the best (tip) block in the longest blockchain.\n", + "\nReturns the hash of the best (tip) block in the most-work fully-validated chain.\n", {}, RPCResult{ "\"hex\" (string) the block hash, hex-encoded\n" @@ -1305,7 +1305,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) RPCResult{ "{\n" " \"chain\": \"xxxx\", (string) current network name as defined in BIP70 (main, test, regtest)\n" - " \"blocks\": xxxxxx, (numeric) the current number of blocks processed in the server\n" + " \"blocks\": xxxxxx, (numeric) the height of the most-work fully-validated chain. The genesis block has height 0\n" " \"headers\": xxxxxx, (numeric) the current number of headers we have validated\n" " \"bestblockhash\": \"...\", (string) the hash of the currently best block\n" " \"difficulty\": xxxxxx, (numeric) the current difficulty\n" @@ -2247,41 +2247,12 @@ UniValue scantxoutset(const JSONRPCRequest& request) // loop through the scan objects for (const UniValue& scanobject : request.params[1].get_array().getValues()) { - std::string desc_str; - std::pair<int64_t, int64_t> range = {0, 1000}; - if (scanobject.isStr()) { - desc_str = scanobject.get_str(); - } else if (scanobject.isObject()) { - UniValue desc_uni = find_value(scanobject, "desc"); - if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); - desc_str = desc_uni.get_str(); - UniValue range_uni = find_value(scanobject, "range"); - if (!range_uni.isNull()) { - range = ParseDescriptorRange(range_uni); - } - } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); - } - FlatSigningProvider provider; - auto desc = Parse(desc_str, provider); - if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); - } - if (!desc->IsRange()) { - range.first = 0; - range.second = 0; - } - for (int i = range.first; i <= range.second; ++i) { - std::vector<CScript> scripts; - if (!desc->Expand(i, provider, scripts, provider)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); - } - for (const auto& script : scripts) { - std::string inferred = InferDescriptor(script, provider)->ToString(); - needles.emplace(script); - descriptors.emplace(std::move(script), std::move(inferred)); - } + auto scripts = EvalDescriptorStringOrObject(scanobject, provider); + for (const auto& script : scripts) { + std::string inferred = InferDescriptor(script, provider)->ToString(); + needles.emplace(script); + descriptors.emplace(std::move(script), std::move(inferred)); } } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 477f05f46c..6636a8a29a 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -103,7 +103,6 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request) static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) { - static const int nInnerLoopCount = 0x10000; int nHeightEnd = 0; int nHeight = 0; @@ -124,14 +123,14 @@ static UniValue generateBlocks(const CScript& coinbase_script, int nGenerate, ui LOCK(cs_main); IncrementExtraNonce(pblock, ::ChainActive().Tip(), nExtraNonce); } - while (nMaxTries > 0 && pblock->nNonce < nInnerLoopCount && !CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus())) { + while (nMaxTries > 0 && pblock->nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus()) && !ShutdownRequested()) { ++pblock->nNonce; --nMaxTries; } - if (nMaxTries == 0) { + if (nMaxTries == 0 || ShutdownRequested()) { break; } - if (pblock->nNonce == nInnerLoopCount) { + if (pblock->nNonce == std::numeric_limits<uint32_t>::max()) { continue; } std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock); @@ -469,7 +468,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } - // Release the wallet and main lock while waiting + // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); @@ -480,6 +479,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) { // Timeout: Check transactions for update + // without holding ::mempool.cs to avoid deadlocks if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime += std::chrono::seconds(10); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 9da24afe79..966ff3fedc 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1495,12 +1495,19 @@ UniValue converttopsbt(const JSONRPCRequest& request) UniValue utxoupdatepsbt(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) { + if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { throw std::runtime_error( RPCHelpMan{"utxoupdatepsbt", - "\nUpdates a PSBT with witness UTXOs retrieved from the UTXO set or the mempool.\n", + "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set or the mempool.\n", { - {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"} + {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"}, + {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of either strings or objects", { + {"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with an output descriptor and extra information", { + {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, + {"range", RPCArg::Type::RANGE, "1000", "Up to what index HD chains should be explored (either end or [begin,end])"}, + }}, + }}, }, RPCResult { " \"psbt\" (string) The base64-encoded partially signed transaction with inputs updated\n" @@ -1510,7 +1517,7 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) }}.ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR}, true); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR}, true); // Unserialize the transactions PartiallySignedTransaction psbtx; @@ -1519,6 +1526,17 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + // Parse descriptors, if any. + FlatSigningProvider provider; + if (!request.params[1].isNull()) { + auto descs = request.params[1].get_array(); + for (size_t i = 0; i < descs.size(); ++i) { + EvalDescriptorStringOrObject(descs[i], provider); + } + } + // We don't actually need private keys further on; hide them as a precaution. + HidingSigningProvider public_provider(&provider, /* nosign */ true, /* nobip32derivs */ false); + // Fetch previous transactions (inputs): CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); @@ -1545,11 +1563,19 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) const Coin& coin = view.AccessCoin(psbtx.tx->vin[i].prevout); - std::vector<std::vector<unsigned char>> solutions_data; - txnouttype which_type = Solver(coin.out.scriptPubKey, solutions_data); - if (which_type == TX_WITNESS_V0_SCRIPTHASH || which_type == TX_WITNESS_V0_KEYHASH || which_type == TX_WITNESS_UNKNOWN) { + if (IsSegWitOutput(provider, coin.out.scriptPubKey)) { input.witness_utxo = coin.out; } + + // Update script/keypath information using descriptor data. + // Note that SignPSBTInput does a lot more than just constructing ECDSA signatures + // we don't actually care about those here, in fact. + SignPSBTInput(public_provider, psbtx, i, /* sighash_type */ 1); + } + + // Update script/keypath information using descriptor data. + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + UpdatePSBTOutput(public_provider, psbtx, i); } CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 9c4cdc3a90..14fcb628eb 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -221,6 +221,9 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). keystore->AddCScript(GetScriptForWitness(witnessScript)); } + if (rs.isNull() && ws.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript"); + } } } } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index ca17d379bc..6fdab2c4cd 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -232,6 +232,10 @@ static UniValue getrpcinfo(const JSONRPCRequest& request) UniValue result(UniValue::VOBJ); result.pushKV("active_commands", active_commands); + const std::string path = LogInstance().m_file_path.string(); + UniValue log_path(UniValue::VSTR, path); + result.pushKV("logpath", log_path); + return result; } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4642cf16b1..67ccb225b5 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -6,6 +6,7 @@ #include <keystore.h> #include <outputtype.h> #include <rpc/util.h> +#include <script/descriptor.h> #include <tinyformat.h> #include <util/strencodings.h> @@ -697,3 +698,40 @@ std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value) } return {low, high}; } + +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider) +{ + std::string desc_str; + std::pair<int64_t, int64_t> range = {0, 1000}; + if (scanobject.isStr()) { + desc_str = scanobject.get_str(); + } else if (scanobject.isObject()) { + UniValue desc_uni = find_value(scanobject, "desc"); + if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); + desc_str = desc_uni.get_str(); + UniValue range_uni = find_value(scanobject, "range"); + if (!range_uni.isNull()) { + range = ParseDescriptorRange(range_uni); + } + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); + } + + auto desc = Parse(desc_str, provider); + if (!desc) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); + } + if (!desc->IsRange()) { + range.first = 0; + range.second = 0; + } + std::vector<CScript> ret; + for (int i = range.first; i <= range.second; ++i) { + std::vector<CScript> scripts; + if (!desc->Expand(i, provider, scripts, provider)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); + } + std::move(scripts.begin(), scripts.end(), std::back_inserter(ret)); + } + return ret; +} diff --git a/src/rpc/util.h b/src/rpc/util.h index 0eb2fef5c3..8762281d73 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -9,6 +9,8 @@ #include <outputtype.h> #include <pubkey.h> #include <rpc/protocol.h> +#include <script/script.h> +#include <script/sign.h> #include <script/standard.h> #include <univalue.h> @@ -84,6 +86,9 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s //! Parse a JSON range specified as int64, or [int64, int64] std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value); +/** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */ +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider); + struct RPCArg { enum class Type { OBJ, diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 36dd68a3d8..5320dc0876 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -505,3 +505,19 @@ FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvide ret.origins.insert(b.origins.begin(), b.origins.end()); return ret; } + +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) +{ + std::vector<valtype> solutions; + auto whichtype = Solver(script, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + if (whichtype == TX_SCRIPTHASH) { + auto h160 = uint160(solutions[0]); + CScript subscript; + if (provider.GetCScript(h160, subscript)) { + whichtype = Solver(subscript, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + } + } + return false; +} diff --git a/src/script/sign.h b/src/script/sign.h index f746325b90..e5c0329a61 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -232,4 +232,7 @@ void UpdateInput(CTxIn& input, const SignatureData& data); * Solvability is unrelated to whether we consider this output to be ours. */ bool IsSolvable(const SigningProvider& provider, const CScript& script); +/** Check whether a scriptPubKey is known to be segwit. */ +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/support/cleanse.cpp b/src/support/cleanse.cpp index 17a4a4c2b2..ecb00510f7 100644 --- a/src/support/cleanse.cpp +++ b/src/support/cleanse.cpp @@ -11,33 +11,25 @@ #include <Windows.h> // For SecureZeroMemory. #endif -/* Compilers have a bad habit of removing "superfluous" memset calls that - * are trying to zero memory. For example, when memset()ing a buffer and - * then free()ing it, the compiler might decide that the memset is - * unobservable and thus can be removed. - * - * Previously we used OpenSSL which tried to stop this by a) implementing - * memset in assembly on x86 and b) putting the function in its own file - * for other platforms. - * - * This change removes those tricks in favour of using asm directives to - * scare the compiler away. As best as our compiler folks can tell, this is - * sufficient and will continue to be so. - * - * Adam Langley <agl@google.com> - * Commit: ad1907fe73334d6c696c8539646c21b11178f20f - * BoringSSL (LICENSE: ISC) - */ void memory_cleanse(void *ptr, size_t len) { - std::memset(ptr, 0, len); - - /* As best as we can tell, this is sufficient to break any optimisations that - might try to eliminate "superfluous" memsets. If there's an easy way to - detect memset_s, it would be better to use that. */ #if defined(_MSC_VER) + /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ SecureZeroMemory(ptr, len); #else + std::memset(ptr, 0, len); + + /* Memory barrier that scares the compiler away from optimizing out the memset. + * + * Quoting Adam Langley <agl@google.com> in commit ad1907fe73334d6c696c8539646c21b11178f20f + * in BoringSSL (ISC License): + * As best as we can tell, this is sufficient to break any optimisations that + * might try to eliminate "superfluous" memsets. + * This method is used in memzero_explicit() the Linux kernel, too. Its advantage is that it + * is pretty efficient because the compiler can still implement the memset() efficiently, + * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by + * Yang et al. (USENIX Security 2017) for more background. + */ __asm__ __volatile__("" : : "r"(ptr) : "memory"); #endif } diff --git a/src/support/cleanse.h b/src/support/cleanse.h index 5298214e44..b03520315d 100644 --- a/src/support/cleanse.h +++ b/src/support/cleanse.h @@ -8,7 +8,8 @@ #include <stdlib.h> -// Attempt to overwrite data in the specified memory span. +/** Secure overwrite a buffer (possibly containing secret data) with zero-bytes. The write + * operation will not be optimized out by the compiler. */ void memory_cleanse(void *ptr, size_t len); #endif // BITCOIN_SUPPORT_CLEANSE_H diff --git a/src/test/allocator_tests.cpp b/src/test/allocator_tests.cpp index f255691704..e333763f27 100644 --- a/src/test/allocator_tests.cpp +++ b/src/test/allocator_tests.cpp @@ -2,9 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <util/memory.h> #include <util/system.h> -#include <support/allocators/secure.h> #include <test/setup_common.h> #include <memory> diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp index 809c627d27..9ac87261b6 100644 --- a/src/test/arith_uint256_tests.cpp +++ b/src/test/arith_uint256_tests.cpp @@ -11,7 +11,6 @@ #include <uint256.h> #include <arith_uint256.h> #include <string> -#include <version.h> #include <test/setup_common.h> BOOST_FIXTURE_TEST_SUITE(arith_uint256_tests, BasicTestingSetup) diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 0c0423c0db..662878750e 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -4,9 +4,10 @@ #include <boost/test/unit_test.hpp> +#include <clientversion.h> #include <key.h> #include <key_io.h> -#include <uint256.h> +#include <streams.h> #include <util/system.h> #include <util/strencodings.h> #include <test/setup_common.h> diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index 13afcca375..ca75563ef0 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -2,6 +2,7 @@ #include <stdlib.h> +#include <chain.h> #include <rpc/blockchain.h> #include <test/setup_common.h> diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index f57e1a0ebd..dac201a35f 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -6,7 +6,7 @@ #include <consensus/merkle.h> #include <chainparams.h> #include <pow.h> -#include <random.h> +#include <streams.h> #include <test/setup_common.h> diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index a9db405477..cf87aa9303 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -10,6 +10,7 @@ #include <pow.h> #include <test/setup_common.h> #include <script/standard.h> +#include <util/time.h> #include <validation.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 408a7fbda4..d796444419 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <util/memory.h> #include <util/system.h> #include <util/time.h> #include <validation.h> @@ -17,8 +18,6 @@ #include <condition_variable> #include <unordered_set> -#include <memory> -#include <random.h> // BasicTestingSetup not sufficient because nScriptCheckThreads is not set // otherwise. diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 2c42596edc..948591196c 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -3,8 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <attributes.h> +#include <clientversion.h> #include <coins.h> #include <script/standard.h> +#include <streams.h> #include <test/setup_common.h> #include <uint256.h> #include <undo.h> diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index a518dbaf55..efcadd51fc 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -4,8 +4,8 @@ #include <dbwrapper.h> #include <uint256.h> -#include <random.h> #include <test/setup_common.h> +#include <util/memory.h> #include <memory> diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 3a2844861b..93883d1d98 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -11,7 +11,9 @@ #include <net_processing.h> #include <script/sign.h> #include <serialize.h> +#include <util/memory.h> #include <util/system.h> +#include <util/time.h> #include <validation.h> #include <test/setup_common.h> diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index ef3946a115..740d805cce 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -2,8 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <clientversion.h> #include <flatfile.h> +#include <streams.h> #include <test/setup_common.h> +#include <util/system.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 10d3bbde55..6d5a6641f0 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -4,6 +4,7 @@ // #include <fs.h> #include <test/setup_common.h> +#include <util/system.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 97d7633715..9364ac4a32 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <addrdb.h> #include <addrman.h> #include <blockencodings.h> #include <chain.h> @@ -11,8 +12,6 @@ #include <net.h> #include <primitives/block.h> #include <protocol.h> -#include <pubkey.h> -#include <script/script.h> #include <streams.h> #include <undo.h> #include <version.h> @@ -20,8 +19,6 @@ #include <stdint.h> #include <unistd.h> -#include <algorithm> -#include <memory> #include <vector> #include <test/fuzz/fuzz.h> diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h index 8b03a7e46e..4e009d9b54 100644 --- a/src/test/fuzz/fuzz.h +++ b/src/test/fuzz/fuzz.h @@ -5,7 +5,6 @@ #ifndef BITCOIN_TEST_FUZZ_FUZZ_H #define BITCOIN_TEST_FUZZ_FUZZ_H -#include <functional> #include <stdint.h> #include <vector> diff --git a/src/test/fuzz/script_flags.cpp b/src/test/fuzz/script_flags.cpp index 2c0bfa360c..9b90d66755 100644 --- a/src/test/fuzz/script_flags.cpp +++ b/src/test/fuzz/script_flags.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <script/interpreter.h> -#include <script/script.h> #include <streams.h> #include <version.h> diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp index 325b7002f2..d91fcb0034 100644 --- a/src/test/hash_tests.cpp +++ b/src/test/hash_tests.cpp @@ -2,13 +2,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <clientversion.h> #include <crypto/siphash.h> #include <hash.h> #include <util/strencodings.h> #include <test/setup_common.h> -#include <vector> - #include <boost/test/unit_test.hpp> BOOST_FIXTURE_TEST_SUITE(hash_tests, BasicTestingSetup) diff --git a/src/test/key_properties.cpp b/src/test/key_properties.cpp index 8b508ed7f7..abcfc4547b 100644 --- a/src/test/key_properties.cpp +++ b/src/test/key_properties.cpp @@ -3,13 +3,9 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <key.h> -#include <base58.h> -#include <script/script.h> #include <uint256.h> #include <util/system.h> -#include <util/strencodings.h> #include <test/setup_common.h> -#include <string> #include <vector> #include <boost/test/unit_test.hpp> diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 1b95105eab..3e99dcaa40 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -5,7 +5,6 @@ #include <key.h> #include <key_io.h> -#include <script/script.h> #include <uint256.h> #include <util/system.h> #include <util/strencodings.h> diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0f74b379c0..c6a3de2285 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -5,11 +5,11 @@ #include <policy/policy.h> #include <txmempool.h> #include <util/system.h> +#include <util/time.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> -#include <list> #include <vector> BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9a182d7bd3..4bd40687a6 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -9,12 +9,12 @@ #include <consensus/tx_verify.h> #include <miner.h> #include <policy/policy.h> -#include <pubkey.h> #include <script/standard.h> #include <txmempool.h> #include <uint256.h> #include <util/strencodings.h> #include <util/system.h> +#include <util/time.h> #include <validation.h> #include <test/setup_common.h> diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 10a732d64d..11e79937be 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -9,6 +9,7 @@ #include <script/script_error.h> #include <script/interpreter.h> #include <script/sign.h> +#include <tinyformat.h> #include <uint256.h> #include <test/setup_common.h> diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index d23a4b8fcc..fed65afdbf 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1,16 +1,19 @@ // Copyright (c) 2012-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <addrdb.h> #include <addrman.h> +#include <clientversion.h> #include <test/setup_common.h> #include <string> #include <boost/test/unit_test.hpp> -#include <hash.h> #include <serialize.h> #include <streams.h> #include <net.h> #include <netbase.h> #include <chainparams.h> +#include <util/memory.h> #include <util/system.h> #include <memory> diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 149094fc00..016a4f471b 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -7,6 +7,7 @@ #include <txmempool.h> #include <uint256.h> #include <util/system.h> +#include <util/time.h> #include <test/setup_common.h> diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp index 653433bfce..1123d4202c 100644 --- a/src/test/pow_tests.cpp +++ b/src/test/pow_tests.cpp @@ -5,7 +5,6 @@ #include <chain.h> #include <chainparams.h> #include <pow.h> -#include <random.h> #include <util/system.h> #include <test/setup_common.h> diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp index 2b01acf7fa..41ca8029e5 100644 --- a/src/test/raii_event_tests.cpp +++ b/src/test/raii_event_tests.cpp @@ -14,8 +14,6 @@ #include <test/setup_common.h> -#include <vector> - #include <boost/test/unit_test.hpp> static std::map<void*, short> tags; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 63bfe1d346..5ae0812243 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -9,8 +9,8 @@ #include <core_io.h> #include <init.h> #include <interfaces/chain.h> - #include <test/setup_common.h> +#include <util/time.h> #include <boost/algorithm/string.hpp> #include <boost/test/unit_test.hpp> diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 195283f89f..046b220e3f 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -5,7 +5,6 @@ #include <key.h> #include <keystore.h> #include <script/script.h> -#include <script/script_error.h> #include <script/standard.h> #include <test/setup_common.h> diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 4798909e2f..ae903df0ad 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -14,12 +14,12 @@ #include <util/strencodings.h> #include <test/setup_common.h> #include <rpc/util.h> +#include <streams.h> #if defined(HAVE_CONSENSUS_LIB) #include <script/bitcoinconsensus.h> #endif -#include <fstream> #include <stdint.h> #include <string> #include <vector> diff --git a/src/test/scriptnum10.h b/src/test/scriptnum10.h index e763b64275..2c89a18331 100644 --- a/src/test/scriptnum10.h +++ b/src/test/scriptnum10.h @@ -6,7 +6,6 @@ #ifndef BITCOIN_TEST_SCRIPTNUM10_H #define BITCOIN_TEST_SCRIPTNUM10_H -#include <algorithm> #include <limits> #include <stdexcept> #include <stdint.h> diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 2fab309aa4..8a8620938e 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -6,6 +6,7 @@ #include <streams.h> #include <hash.h> #include <test/setup_common.h> +#include <util/strencodings.h> #include <stdint.h> diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index aaf69b9575..24c7d51898 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -12,15 +12,20 @@ #include <crypto/sha256.h> #include <init.h> #include <miner.h> -#include <net_processing.h> +#include <net.h> #include <noui.h> #include <pow.h> #include <rpc/register.h> #include <rpc/server.h> #include <script/sigcache.h> #include <streams.h> +#include <txdb.h> +#include <util/memory.h> +#include <util/strencodings.h> +#include <util/time.h> #include <util/validation.h> #include <validation.h> +#include <validationinterface.h> const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; @@ -68,7 +73,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. - RegisterAllCoreRPCCommands(tableRPC); // We have to run a scheduler thread to prevent ActivateBestChain diff --git a/src/test/setup_common.h b/src/test/setup_common.h index b1bb5e6b25..6c9494898c 100644 --- a/src/test/setup_common.h +++ b/src/test/setup_common.h @@ -11,10 +11,8 @@ #include <pubkey.h> #include <random.h> #include <scheduler.h> -#include <txdb.h> #include <txmempool.h> -#include <memory> #include <type_traits> #include <boost/thread.hpp> diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 5c12ec13d2..a32f2cda92 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <consensus/consensus.h> #include <consensus/tx_verify.h> -#include <consensus/validation.h> #include <pubkey.h> #include <key.h> #include <script/script.h> diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 4e37199c63..b812cef801 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <streams.h> -#include <support/allocators/zeroafterfree.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index f5ff18c055..f77b77a972 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -20,6 +20,7 @@ #include <script/sign.h> #include <script/script_error.h> #include <script/standard.h> +#include <streams.h> #include <util/strencodings.h> #include <map> diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 26ae7be202..2356e0ccdc 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -3,8 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <validation.h> -#include <txmempool.h> -#include <amount.h> #include <consensus/validation.h> #include <primitives/transaction.h> #include <script/script.h> diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 352ce0295b..45c97fa2aa 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -5,14 +5,10 @@ #include <consensus/validation.h> #include <key.h> #include <validation.h> -#include <miner.h> -#include <pubkey.h> #include <txmempool.h> -#include <random.h> #include <script/standard.h> #include <script/sign.h> #include <test/setup_common.h> -#include <util/time.h> #include <keystore.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index c1749fb856..33a118c2bb 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -1,19 +1,17 @@ // Copyright (c) 2011-2019 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 <arith_uint256.h> +#include <streams.h> #include <uint256.h> #include <version.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> -#include <stdint.h> #include <sstream> #include <iomanip> -#include <limits> -#include <cmath> #include <string> -#include <stdio.h> BOOST_FIXTURE_TEST_SUITE(uint256_tests, BasicTestingSetup) diff --git a/src/test/util.cpp b/src/test/util.cpp index bc09d00b7a..a2ea648324 100644 --- a/src/test/util.cpp +++ b/src/test/util.cpp @@ -17,8 +17,6 @@ #include <wallet/wallet.h> #endif -#include <boost/thread.hpp> - const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; #ifdef ENABLE_WALLET diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index d1fcb345b4..9960573b33 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -5,14 +5,15 @@ #include <util/system.h> #include <clientversion.h> -#include <primitives/transaction.h> #include <sync.h> #include <test/util.h> #include <util/strencodings.h> #include <util/moneystr.h> +#include <util/time.h> #include <test/setup_common.h> #include <stdint.h> +#include <thread> #include <vector> #ifndef WIN32 #include <signal.h> diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 5dee034b20..b3368d44b6 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -10,14 +10,20 @@ #include <miner.h> #include <pow.h> #include <random.h> +#include <script/standard.h> #include <test/setup_common.h> +#include <util/time.h> #include <validation.h> #include <validationinterface.h> +#include <thread> + struct RegtestingSetup : public TestingSetup { RegtestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {} }; +static const std::vector<unsigned char> V_OP_TRUE{OP_TRUE}; + BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegtestingSetup) struct TestSubscriber : public CValidationInterface { @@ -59,8 +65,21 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; + pubKey.clear(); + { + WitnessV0ScriptHash witness_program; + CSHA256().Write(&V_OP_TRUE[0], V_OP_TRUE.size()).Finalize(witness_program.begin()); + pubKey << OP_0 << ToByteVector(witness_program); + } + + // Make the coinbase transaction with two outputs: + // One zero-value one that has a unique pubkey to make sure that blocks at the same height can have a different hash + // Another one that has the coinbase reward in a P2WSH with OP_TRUE as witness program to make it easy to spend CMutableTransaction txCoinbase(*pblock->vtx[0]); - txCoinbase.vout.resize(1); + txCoinbase.vout.resize(2); + txCoinbase.vout[1].scriptPubKey = pubKey; + txCoinbase.vout[1].nValue = txCoinbase.vout[0].nValue; + txCoinbase.vout[0].nValue = 0; txCoinbase.vin[0].scriptWitness.SetNull(); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); @@ -69,6 +88,9 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) { + LOCK(cs_main); // For LookupBlockIndex + GenerateCoinbaseCommitment(*pblock, LookupBlockIndex(pblock->hashPrevBlock), Params().GetConsensus()); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus())) { @@ -79,13 +101,13 @@ std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) } // construct a valid block -const std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) { return FinalizeBlock(Block(prev_hash)); } // construct an invalid block (but with a valid header) -const std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) { auto pblock = Block(prev_hash); @@ -185,4 +207,131 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash()); } +/** + * Test that mempool updates happen atomically with reorgs. + * + * This prevents RPC clients, among others, from retrieving immediately-out-of-date mempool data + * during large reorgs. + * + * The test verifies this by creating a chain of `num_txs` blocks, matures their coinbases, and then + * submits txns spending from their coinbase to the mempool. A fork chain is then processed, + * invalidating the txns and evicting them from the mempool. + * + * We verify that the mempool updates atomically by polling it continuously + * from another thread during the reorg and checking that its size only changes + * once. The size changing exactly once indicates that the polling thread's + * view of the mempool is either consistent with the chain state before reorg, + * or consistent with the chain state after the reorg, and not just consistent + * with some intermediate state during the reorg. + */ +BOOST_AUTO_TEST_CASE(mempool_locks_reorg) +{ + bool ignored; + auto ProcessBlock = [&ignored](std::shared_ptr<const CBlock> block) -> bool { + return ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored); + }; + + // Process all mined blocks + BOOST_REQUIRE(ProcessBlock(std::make_shared<CBlock>(Params().GenesisBlock()))); + auto last_mined = GoodBlock(Params().GenesisBlock().GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + + // Run the test multiple times + for (int test_runs = 3; test_runs > 0; --test_runs) { + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // Later on split from here + const uint256 split_hash{last_mined->hashPrevBlock}; + + // Create a bunch of transactions to spend the miner rewards of the + // most recent blocks + std::vector<CTransactionRef> txs; + for (int num_txs = 22; num_txs > 0; --num_txs) { + CMutableTransaction mtx; + mtx.vin.push_back(CTxIn{COutPoint{last_mined->vtx[0]->GetHash(), 1}, CScript{}}); + mtx.vin[0].scriptWitness.stack.push_back(V_OP_TRUE); + mtx.vout.push_back(last_mined->vtx[0]->vout[1]); + mtx.vout[0].nValue -= 1000; + txs.push_back(MakeTransactionRef(mtx)); + + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mature the inputs of the txs + for (int j = COINBASE_MATURITY; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mine a reorg (and hold it back) before adding the txs to the mempool + const uint256 tip_init{last_mined->GetHash()}; + + std::vector<std::shared_ptr<const CBlock>> reorg; + last_mined = GoodBlock(split_hash); + reorg.push_back(last_mined); + for (size_t j = COINBASE_MATURITY + txs.size() + 1; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + reorg.push_back(last_mined); + } + + // Add the txs to the tx pool + { + LOCK(cs_main); + CValidationState state; + std::list<CTransactionRef> plTxnReplaced; + for (const auto& tx : txs) { + BOOST_REQUIRE(AcceptToMemoryPool( + ::mempool, + state, + tx, + /* pfMissingInputs */ &ignored, + &plTxnReplaced, + /* bypass_limits */ false, + /* nAbsurdFee */ 0)); + } + } + + // Check that all txs are in the pool + { + LOCK(::mempool.cs); + BOOST_CHECK_EQUAL(::mempool.mapTx.size(), txs.size()); + } + + // Run a thread that simulates an RPC caller that is polling while + // validation is doing a reorg + std::thread rpc_thread{[&]() { + // This thread is checking that the mempool either contains all of + // the transactions invalidated by the reorg, or none of them, and + // not some intermediate amount. + while (true) { + LOCK(::mempool.cs); + if (::mempool.mapTx.size() == 0) { + // We are done with the reorg + break; + } + // Internally, we might be in the middle of the reorg, but + // externally the reorg to the most-proof-of-work chain should + // be atomic. So the caller assumes that the returned mempool + // is consistent. That is, it has all txs that were there + // before the reorg. + assert(::mempool.mapTx.size() == txs.size()); + continue; + } + LOCK(cs_main); + // We are done with the reorg, so the tip must have changed + assert(tip_init != ::ChainActive().Tip()->GetBlockHash()); + }}; + + // Submit the reorg in this thread to invalidate and remove the txs from the tx pool + for (const auto& b : reorg) { + ProcessBlock(b); + } + // Check that the reorg was eventually successful + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // We can join the other thread, which returns when the reorg was successful + rpc_thread.join(); + } +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cac7beb6a1..9257cff718 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -104,7 +104,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan // for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) { - LOCK(cs); + AssertLockHeld(cs); // For each entry in vHashesToUpdate, store the set of in-mempool, but not // in-vHashesToUpdate transactions, so that we don't have to recalculate // descendants when we come across a previously seen entry. @@ -322,8 +322,8 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, assert(int(nSigOpCostWithAncestors) >= 0); } -CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) : - nTransactionsUpdated(0), minerPolicyEstimator(estimator) +CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) + : nTransactionsUpdated(0), minerPolicyEstimator(estimator) { _clear(); //lock free clear @@ -341,13 +341,11 @@ bool CTxMemPool::isSpent(const COutPoint& outpoint) const unsigned int CTxMemPool::GetTransactionsUpdated() const { - LOCK(cs); return nTransactionsUpdated; } void CTxMemPool::AddTransactionsUpdated(unsigned int n) { - LOCK(cs); nTransactionsUpdated += n; } @@ -459,8 +457,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool - { - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { @@ -485,13 +482,12 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso } RemoveStaged(setAllRemoves, false, reason); - } } void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) { // Remove transactions spending a coinbase which are now immature and no-longer-final transactions - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); @@ -547,7 +543,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) */ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); std::vector<const CTxMemPoolEntry*> entries; for (const auto& tx : vtx) { @@ -922,7 +918,7 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool } int CTxMemPool::Expire(int64_t time) { - LOCK(cs); + AssertLockHeld(cs); indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin(); setEntries toremove; while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) { @@ -1015,7 +1011,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining) { - LOCK(cs); + AssertLockHeld(cs); unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(0); diff --git a/src/txmempool.h b/src/txmempool.h index ce0b762336..565dd61f0f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -6,12 +6,13 @@ #ifndef BITCOIN_TXMEMPOOL_H #define BITCOIN_TXMEMPOOL_H +#include <atomic> +#include <map> #include <memory> #include <set> -#include <map> -#include <vector> -#include <utility> #include <string> +#include <utility> +#include <vector> #include <amount.h> #include <coins.h> @@ -443,7 +444,7 @@ class CTxMemPool { private: uint32_t nCheckFrequency GUARDED_BY(cs); //!< Value n means that n times in 2^32 we check. - unsigned int nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation + std::atomic<unsigned int> nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation CBlockPolicyEstimator* minerPolicyEstimator; uint64_t totalTxSize; //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141. @@ -513,21 +514,12 @@ public: * `mempool.cs` whenever adding transactions to the mempool and whenever * changing the chain tip. It's necessary to keep both mutexes locked until * the mempool is consistent with the new chain tip and fully populated. - * - * @par Consistency bug - * - * The second guarantee above is not currently enforced, but - * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code - * in bitcoin currently depends on second guarantee, but it is important to - * fix for third party code that needs be able to frequently poll the - * mempool without locking `cs_main` and without encountering missing - * transactions during reorgs. */ mutable RecursiveMutex cs; indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; - std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order + std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order struct CompareIteratorByHash { bool operator()(const txiter &a, const txiter &b) const { @@ -582,10 +574,10 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); - void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight); + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free @@ -598,7 +590,7 @@ public: * Check that none of this transactions inputs are in the mempool, and thus * the tx is not dependent on other mempool transactions to be included in a block. */ - bool HasNoInputsOf(const CTransaction& tx) const; + bool HasNoInputsOf(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); @@ -632,7 +624,7 @@ public: * for). Note: vHashesToUpdate should be the set of transactions from the * disconnected block that have been accepted back into the mempool. */ - void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); /** Try to calculate all in-mempool ancestors of entry. * (these are all calculated including the tx itself) @@ -663,10 +655,10 @@ public: * pvNoSpendsRemaining, if set, will be populated with the list of outpoints * which are not in mempool which no longer have any spends in this mempool. */ - void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining=nullptr); + void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ - int Expire(int64_t time); + int Expire(int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Calculate the ancestor and descendant count for the given transaction. diff --git a/src/uint256.cpp b/src/uint256.cpp index e3bc9712e8..ea7164c1f0 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -37,16 +37,15 @@ void base_blob<BITS>::SetHex(const char* psz) psz += 2; // hex string to uint - const char* pbegin = psz; - while (::HexDigit(*psz) != -1) - psz++; - psz--; + size_t digits = 0; + while (::HexDigit(psz[digits]) != -1) + digits++; unsigned char* p1 = (unsigned char*)data; unsigned char* pend = p1 + WIDTH; - while (psz >= pbegin && p1 < pend) { - *p1 = ::HexDigit(*psz--); - if (psz >= pbegin) { - *p1 |= ((unsigned char)::HexDigit(*psz--) << 4); + while (digits > 0 && p1 < pend) { + *p1 = ::HexDigit(psz[--digits]); + if (digits > 0) { + *p1 |= ((unsigned char)::HexDigit(psz[--digits]) << 4); p1++; } } diff --git a/src/util/error.cpp b/src/util/error.cpp index 68ffd8b046..9331a92ad7 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -27,6 +27,8 @@ std::string TransactionErrorString(const TransactionError err) return "PSBTs not compatible (different transactions)"; case TransactionError::SIGHASH_MISMATCH: return "Specified sighash value does not match existing value"; + case TransactionError::MAX_FEE_EXCEEDED: + return "Fee exceeds maximum configured by -maxtxfee"; // no default case, so the compiler can warn about missing cases } assert(false); diff --git a/src/util/error.h b/src/util/error.h index d93309551b..0fd474b962 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -27,6 +27,7 @@ enum class TransactionError { INVALID_PSBT, PSBT_MISMATCH, SIGHASH_MISMATCH, + MAX_FEE_EXCEEDED, }; std::string TransactionErrorString(const TransactionError error); diff --git a/src/util/fees.cpp b/src/util/fees.cpp index 5fdaa1284c..cf16d5e44f 100644 --- a/src/util/fees.cpp +++ b/src/util/fees.cpp @@ -18,7 +18,6 @@ std::string StringForFeeReason(FeeReason reason) { {FeeReason::PAYTXFEE, "PayTxFee set"}, {FeeReason::FALLBACK, "Fallback fee"}, {FeeReason::REQUIRED, "Minimum Required Fee"}, - {FeeReason::MAXTXFEE, "MaxTxFee limit"} }; auto reason_string = fee_reason_strings.find(reason); diff --git a/src/util/system.cpp b/src/util/system.cpp index fca29a9f31..87ff6e62ba 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -705,19 +705,16 @@ fs::path GetDefaultDataDir() static fs::path g_blocks_path_cache_net_specific; static fs::path pathCached; static fs::path pathCachedNetSpecific; -static CCriticalSection csPathCached; +static RecursiveMutex csPathCached; const fs::path &GetBlocksDir() { - LOCK(csPathCached); - fs::path &path = g_blocks_path_cache_net_specific; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); @@ -737,15 +734,12 @@ const fs::path &GetBlocksDir() const fs::path &GetDataDir(bool fNetSpecific) { - LOCK(csPathCached); - fs::path &path = fNetSpecific ? pathCachedNetSpecific : pathCached; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-datadir")) { path = fs::system_complete(gArgs.GetArg("-datadir", "")); diff --git a/src/util/system.h b/src/util/system.h index 1a83cb67b1..15d7b1b402 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -20,18 +20,16 @@ #include <fs.h> #include <logging.h> #include <sync.h> -#include <util/threadnames.h> #include <tinyformat.h> #include <util/memory.h> +#include <util/threadnames.h> #include <util/time.h> -#include <atomic> #include <exception> #include <map> #include <set> #include <stdint.h> #include <string> -#include <unordered_set> #include <utility> #include <vector> @@ -85,6 +83,7 @@ fs::path GetDefaultDataDir(); // The blocks directory is always net specific. const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); +/** Tests only */ void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); #ifdef WIN32 diff --git a/src/validation.cpp b/src/validation.cpp index d39b78614c..c6995ed24b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -304,7 +304,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +{ int expired = pool.Expire(GetTime() - age); if (expired != 0) { LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); @@ -341,7 +342,7 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) { AssertLockHeld(cs_main); std::vector<uint256> vHashUpdate; @@ -2547,7 +2548,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& LimitValidationInterfaceQueue(); { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -2667,6 +2668,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c LimitValidationInterfaceQueue(); LOCK(cs_main); + LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -4100,7 +4102,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; diff --git a/src/validation.h b/src/validation.h index 638229952d..9573d62048 100644 --- a/src/validation.h +++ b/src/validation.h @@ -18,6 +18,7 @@ #include <protocol.h> // For CMessageHeader::MessageStartChars #include <script/script_error.h> #include <sync.h> +#include <txmempool.h> // For CTxMemPool::cs #include <versionbits.h> #include <algorithm> @@ -553,7 +554,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block disconnection on our pcoinsTip: - bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -572,8 +573,8 @@ public: bool IsInitialBlockDownload() const; private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index ad69e84358..2792058f2a 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -18,14 +18,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes) CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc) { - CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); - // Always obey the maximum - const CAmount max_tx_fee = wallet.m_default_max_tx_fee; - if (fee_needed > max_tx_fee) { - fee_needed = max_tx_fee; - if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; - } - return fee_needed; + return GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); } CFeeRate GetRequiredFeeRate(const CWallet& wallet) diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index ce4788dee1..721a244afb 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -44,16 +44,7 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { - const CTxOut& out = psbtx.tx->vout.at(i); - PSBTOutput& psbt_out = psbtx.outputs.at(i); - - // Fill a SignatureData with output info - SignatureData sigdata; - psbt_out.FillSignatureData(sigdata); - - MutableTransactionSignatureCreator creator(psbtx.tx.get_ptr(), 0, out.nValue, 1); - ProduceSignature(HidingSigningProvider(pwallet, true, !bip32derivs), creator, out.scriptPubKey, sigdata); - psbt_out.FromSignatureData(sigdata); + UpdatePSBTOutput(HidingSigningProvider(pwallet, true, !bip32derivs), psbtx, i); } return TransactionError::OK; diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index fcd1f3fea8..86ba0013fe 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <util/system.h> #include <wallet/test/init_test_fixture.h> diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index 67e2847963..1816fca257 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -5,6 +5,7 @@ #include <boost/test/unit_test.hpp> #include <test/setup_common.h> +#include <util/system.h> #include <wallet/test/init_test_fixture.h> BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup) diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 0cae055676..062fef7748 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -4,7 +4,6 @@ #include <key.h> #include <script/script.h> -#include <script/script_error.h> #include <script/standard.h> #include <test/setup_common.h> #include <wallet/ismine.h> diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index cdf7113203..0400f1207c 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -3,12 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <key_io.h> -#include <script/sign.h> #include <util/bip32.h> #include <util/strencodings.h> #include <wallet/psbtwallet.h> #include <wallet/wallet.h> -#include <univalue.h> #include <boost/test/unit_test.hpp> #include <test/setup_common.h> diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 7db0bc4249..ba0843f352 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -4,8 +4,6 @@ #include <wallet/test/wallet_test_fixture.h> -#include <wallet/db.h> - WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7f261a6189..ef95d0544f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -5,9 +5,7 @@ #include <wallet/wallet.h> #include <memory> -#include <set> #include <stdint.h> -#include <utility> #include <vector> #include <consensus/validation.h> diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 16366a0c23..8807acb6b7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2694,6 +2694,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC } } + if (nFeeRet > this->m_default_max_tx_fee) { + strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); + return false; + } + return true; } diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 49ae0fb1a9..e99fa22646 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests some generic aspects of the RPC interface.""" +import os from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than_or_equal @@ -31,6 +32,7 @@ class RPCInterfaceTest(BitcoinTestFramework): command = info['active_commands'][0] assert_equal(command['method'], 'getrpcinfo') assert_greater_than_or_equal(command['duration'], 0) + assert_equal(info['logpath'], os.path.join(self.nodes[0].datadir, 'regtest', 'debug.log')) def test_batch_request(self): self.log.info("Testing basic JSON-RPC batch request...") diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 58010f7c2e..62f3843756 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -129,6 +129,11 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): outval = value - decimal.Decimal("0.00001000") rawtx = node2.createrawtransaction([{"txid": txid, "vout": vout}], [{self.final: outval}]) + prevtx_err = dict(prevtxs[0]) + del prevtx_err["redeemScript"] + + assert_raises_rpc_error(-8, "Missing redeemScript/witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err]) + rawtx2 = node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs - 1], prevtxs) rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [self.priv[-1]], prevtxs) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index d89fd6461f..cdf636e200 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -662,6 +662,7 @@ class RawTransactionsTest(BitcoinTestFramework): result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee) result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}) result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10*min_relay_tx_fee}) + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1}) result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex']) assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate) assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 430be6dd37..2fffe96ebe 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -9,6 +9,7 @@ from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, connect_nodes_bi, disconnect_nodes, @@ -129,6 +130,15 @@ class PSBTTest(BitcoinTestFramework): assert_equal(walletprocesspsbt_out['complete'], True) self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) + # feeRate of 0.1 BTC / KB produces a total fee slightly below -maxtxfee (~0.05280000): + res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 0.1}) + assert_greater_than(res["fee"], 0.05) + assert_greater_than(0.06, res["fee"]) + + # feeRate of 10 BTC / KB produces a total fee well above -maxtxfee + # previously this was silenty capped at -maxtxfee + assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 10}) + # partially sign multisig things with node 1 psbtx = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wsh_pos},{"txid":txid,"vout":p2sh_pos},{"txid":txid,"vout":p2sh_p2wsh_pos}], {self.nodes[1].getnewaddress():29.99})['psbt'] walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx) @@ -315,18 +325,32 @@ class PSBTTest(BitcoinTestFramework): vout3 = find_output(self.nodes[0], txid3, 11) self.sync_all() - # Update a PSBT with UTXOs from the node - # Bech32 inputs should be filled with witness UTXO. Other inputs should not be filled because they are non-witness + def test_psbt_input_keys(psbt_input, keys): + """Check that the psbt input has only the expected keys.""" + assert_equal(set(keys), set(psbt_input.keys())) + + # Create a PSBT. None of the inputs are filled initially psbt = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1},{"txid":txid2, "vout":vout2},{"txid":txid3, "vout":vout3}], {self.nodes[0].getnewaddress():32.999}) decoded = self.nodes[1].decodepsbt(psbt) - assert "witness_utxo" not in decoded['inputs'][0] and "non_witness_utxo" not in decoded['inputs'][0] - assert "witness_utxo" not in decoded['inputs'][1] and "non_witness_utxo" not in decoded['inputs'][1] - assert "witness_utxo" not in decoded['inputs'][2] and "non_witness_utxo" not in decoded['inputs'][2] + test_psbt_input_keys(decoded['inputs'][0], []) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], []) + + # Update a PSBT with UTXOs from the node + # Bech32 inputs should be filled with witness UTXO. Other inputs should not be filled because they are non-witness updated = self.nodes[1].utxoupdatepsbt(psbt) decoded = self.nodes[1].decodepsbt(updated) - assert "witness_utxo" in decoded['inputs'][0] and "non_witness_utxo" not in decoded['inputs'][0] - assert "witness_utxo" not in decoded['inputs'][1] and "non_witness_utxo" not in decoded['inputs'][1] - assert "witness_utxo" not in decoded['inputs'][2] and "non_witness_utxo" not in decoded['inputs'][2] + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo']) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], []) + + # Try again, now while providing descriptors, making P2SH-segwit work, and causing bip32_derivs and redeem_script to be filled in + descs = [self.nodes[1].getaddressinfo(addr)['desc'] for addr in [addr1,addr2,addr3]] + updated = self.nodes[1].utxoupdatepsbt(psbt, descs) + decoded = self.nodes[1].decodepsbt(updated) + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo', 'bip32_derivs']) + test_psbt_input_keys(decoded['inputs'][1], []) + test_psbt_input_keys(decoded['inputs'][2], ['witness_utxo', 'bip32_derivs', 'redeem_script']) # Two PSBTs with a common input should not be joinable psbt1 = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1}], {self.nodes[0].getnewaddress():Decimal('10.999')}) diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 15f2195e21..137c77a51e 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -203,6 +203,8 @@ class WalletTest(BitcoinTestFramework): self.log.info('Put txs back into mempool of node 1 (not node 0)') self.nodes[0].invalidateblock(block_reorg) self.nodes[1].invalidateblock(block_reorg) + self.sync_blocks() + self.nodes[0].syncwithvalidationinterfacequeue() assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted self.nodes[0].generatetoaddress(1, ADDRESS_WATCHONLY) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 4d9bacf299..1fe029a6fb 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -304,7 +304,9 @@ def test_unconfirmed_not_spendable(rbf_node, rbf_node_address): def test_bumpfee_metadata(rbf_node, dest_address): - rbfid = rbf_node.sendtoaddress(dest_address, Decimal("0.00100000"), "comment value", "to value") + assert(rbf_node.getbalance() < 49) + rbf_node.generatetoaddress(101, rbf_node.getnewaddress()) + rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value") bumped_tx = rbf_node.bumpfee(rbfid) bumped_wtx = rbf_node.gettransaction(bumped_tx["txid"]) assert_equal(bumped_wtx["comment"], "comment value") diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index 5810e94938..91d26e9cb3 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -57,8 +57,7 @@ class ResendWalletTransactionsTest(BitcoinTestFramework): # after the last time we tried to broadcast. Use mocktime and give an extra minute to be sure. block_time = int(time.time()) + 6 * 60 node.setmocktime(block_time) - block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockchaininfo()['blocks']), block_time) - block.nVersion = 3 + block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time) block.rehash() block.solve() node.submitblock(ToHex(block)) |