diff options
33 files changed, 482 insertions, 373 deletions
diff --git a/contrib/verify-commits/README.md b/contrib/verify-commits/README.md index 27ca15acb4..1215962a16 100644 --- a/contrib/verify-commits/README.md +++ b/contrib/verify-commits/README.md @@ -37,7 +37,8 @@ Configuration files Import trusted keys ------------------- -In order to check the commit signatures you must add the trusted PGP keys to your machine. This can be done in Linux by running +In order to check the commit signatures, you must add the trusted PGP keys to your machine. [GnuPG](https://gnupg.org/) may be used to import the trusted keys by running the following command: + ```sh gpg --recv-keys $(<contrib/verify-commits/trusted-keys) ``` diff --git a/doc/README.md b/doc/README.md index 51950d4a13..8876ffdd72 100644 --- a/doc/README.md +++ b/doc/README.md @@ -51,6 +51,7 @@ Development The Bitcoin repo's [root README](/README.md) contains relevant information on the development process and automated testing. - [Developer Notes](developer-notes.md) +- [Productivity Notes](productivity.md) - [Release Notes](release-notes.md) - [Release Process](release-process.md) - [Source Code Documentation (External Link)](https://dev.visucore.com/bitcoin/doxygen/) diff --git a/doc/REST-interface.md b/doc/REST-interface.md index d21df36130..02a665008b 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -20,7 +20,8 @@ Supported API Given a transaction hash: returns a transaction in binary, hex-encoded binary, or JSON formats. -For full TX query capability, one must enable the transaction index via "txindex=1" command line / configuration option. +By default, this endpoint will only search the mempool. +To query for a confirmed transaction, enable the transaction index via "txindex=1" command line / configuration option. #### Blocks `GET /rest/block/<BLOCK-HASH>.<bin|hex|json>` diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index 70f5dfc882..d22b6e8383 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -1,6 +1,6 @@ FreeBSD build guide ====================== -(updated for FreeBSD 11.1) +(updated for FreeBSD 12.0) This guide describes how to build bitcoind and command-line utilities on FreeBSD. @@ -10,55 +10,51 @@ This guide does not contain instructions for building the GUI. You will need the following dependencies, which can be installed as root via pkg: -``` +```shell pkg install autoconf automake boost-libs git gmake libevent libtool openssl pkgconf + +git clone https://github.com/bitcoin/bitcoin.git ``` In order to run the test suite (recommended), you will need to have Python 3 installed: -``` +```shell pkg install python3 ``` -For the wallet (optional): -``` -./contrib/install_db4.sh `pwd` -export BDB_PREFIX="$PWD/db4" -``` - See [dependencies.md](dependencies.md) for a complete overview. -Download the source code: -``` -git clone https://github.com/bitcoin/bitcoin +### Building BerkeleyDB + +BerkeleyDB is only necessary for the wallet functionality. To skip this, pass +`--disable-wallet` to `./configure` and skip to the next section. + +```shell +./contrib/install_db4.sh `pwd` +export BDB_PREFIX="$PWD/db4" ``` ## Building Bitcoin Core **Important**: Use `gmake` (the non-GNU `make` will exit with an error): -``` +With wallet: +```shell ./autogen.sh - -./configure # to build with wallet OR -./configure --disable-wallet # to build without wallet +./configure --with-gui=no \ + BDB_LIBS="-L${BDB_PREFIX}/lib -ldb_cxx-4.8" \ + BDB_CFLAGS="-I${BDB_PREFIX}/include" ``` -followed by either: - -``` -gmake +Without wallet: +```shell +./autogen.sh +./configure --with-gui=no --disable-wallet ``` -to build without testing, or +followed by: +```shell +gmake # use -jX here for parallelism +gmake check # Run tests if Python 3 is available ``` -gmake check -``` - -to also run the test suite (recommended, if Python 3 is installed). - -*Note on debugging*: The version of `gdb` installed by default is [ancient and considered harmful](https://wiki.freebsd.org/GdbRetirement). -It is not suitable for debugging a multi-threaded C++ program, not even for getting backtraces. Please install the package `gdb` and -use the versioned gdb command (e.g. `gdb7111`). - diff --git a/doc/dependencies.md b/doc/dependencies.md index 235eeb28b6..e4be63772c 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -17,6 +17,7 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | libevent | [2.1.8-stable](https://github.com/libevent/libevent/releases) | 2.0.22 | No | | | | libjpeg | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L65) | | libpng | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L64) | +| libsrvg | | | | | | | MiniUPnPc | [2.0.20180203](http://miniupnp.free.fr/files) | | No | | | | OpenSSL | [1.0.1k](https://www.openssl.org/source) | | Yes | | | | PCRE | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L66) | @@ -28,3 +29,18 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | xkbcommon | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk#L86) (Linux only) | | ZeroMQ | [4.3.1](https://github.com/zeromq/libzmq/releases) | 4.0.0 | No | | | | zlib | [1.2.11](https://zlib.net/) | | | | No | + +Controlling dependencies +------------------------ +Some dependencies are not needed in all configurations. The following are some factors that affect the dependency list. + +#### Options passed to `./configure` +* MiniUPnPc is not needed with `--with-miniupnpc=no`. +* Berkeley DB is not needed with `--disable-wallet`. +* protobuf is not needed with `--disable-bip70`. +* Qt is not needed with `--without-gui`. +* If the qrencode dependency is absent, QR support won't be added. To force an error when that happens, pass `--with-qrencode`. +* ZeroMQ is needed only with the `--with-zmq` option. + +#### Other +* librsvg is only needed if you need to run `make deploy` on (cross-compliation to) macOS.
\ No newline at end of file diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 1deb5d791a..f765346cd8 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -844,54 +844,6 @@ would be to revert the upstream fix before applying the updates to Bitcoin's copy of LevelDB. In general you should be wary of any upstream changes affecting what data is returned from LevelDB queries. -Git and GitHub tips ---------------------- - -- For resolving merge/rebase conflicts, it can be useful to enable diff3 style using - `git config merge.conflictstyle diff3`. Instead of - - <<< - yours - === - theirs - >>> - - you will see - - <<< - yours - ||| - original - === - theirs - >>> - - This may make it much clearer what caused the conflict. In this style, you can often just look - at what changed between *original* and *theirs*, and mechanically apply that to *yours* (or the other way around). - -- When reviewing patches which change indentation in C++ files, use `git diff -w` and `git show -w`. This makes - the diff algorithm ignore whitespace changes. This feature is also available on github.com, by adding `?w=1` - at the end of any URL which shows a diff. - -- When reviewing patches that change symbol names in many places, use `git diff --word-diff`. This will instead - of showing the patch as deleted/added *lines*, show deleted/added *words*. - -- When reviewing patches that move code around, try using - `git diff --patience commit~:old/file.cpp commit:new/file/name.cpp`, and ignoring everything except the - moved body of code which should show up as neither `+` or `-` lines. In case it was not a pure move, this may - even work when combined with the `-w` or `--word-diff` options described above. - -- When looking at other's pull requests, it may make sense to add the following section to your `.git/config` - file: - - [remote "upstream-pull"] - fetch = +refs/pull/*:refs/remotes/upstream-pull/* - url = git@github.com:bitcoin/bitcoin.git - - This will add an `upstream-pull` remote to your git repository, which can be fetched using `git fetch --all` - or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`, - `git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER. - Scripted diffs -------------- diff --git a/doc/productivity.md b/doc/productivity.md new file mode 100644 index 0000000000..862017290d --- /dev/null +++ b/doc/productivity.md @@ -0,0 +1,161 @@ +Productivity Notes +================== + +Table of Contents +----------------- + +* [General](#general) + * [Cache compilations with `ccache`](#cache-compilations-with-ccache) + * [Disable features with `./configure`](#disable-features-with-configure) + * [Make use of your threads with `make -j`](#make-use-of-your-threads-with-make--j) + * [Multiple working directories with `git worktrees`](#multiple-working-directories-with-git-worktrees) +* [Writing code](#writing-code) + * [Format C/C++/Protobuf diffs with `clang-format-diff.py`](#format-ccprotobuf-diffs-with-clang-format-diffpy) + * [Format Python diffs with `yapf-diff.py`](#format-python-diffs-with-yapf-diffpy) +* [Rebasing/Merging code](#rebasingmerging-code) + * [More conflict context with `merge.conflictstyle diff3`](#more-conflict-context-with-mergeconflictstyle-diff3) +* [Reviewing code](#reviewing-code) + * [Reduce mental load with `git diff` options](#reduce-mental-load-with-git-diff-options) + * [Reference PRs easily with `refspec`s](#reference-prs-easily-with-refspecs) + * [Diff the diffs with `git range-diff`](#diff-the-diffs-with-git-range-diff) + +General +------ + +### Cache compilations with `ccache` + +The easiest way to faster compile times is to cache compiles. `ccache` is a way to do so, from its description at the time of writing: + +> ccache is a compiler cache. It speeds up recompilation by caching the result of previous compilations and detecting when the same compilation is being done again. Supported languages are C, C++, Objective-C and Objective-C++. + +Install `ccache` through your distribution's package manager, and run `./configure` with your normal flags to pick it up. + +To use ccache for all your C/C++ projects, follow the symlinks method [here](https://ccache.samba.org/manual/latest.html#_run_modes) to set it up. + +### Disable features with `./configure` + +After running `./autogen.sh`, which generates the `./configure` file, use `./configure --help` to identify features that you can disable to save on compilation time. A few common flags: + +```sh +--without-miniupnpc +--disable-bench +--disable-wallet +--without-gui +``` + +### Make use of your threads with `make -j` + +If you have multiple threads on your machine, you can tell `make` to utilize all of them with: + +```sh +make -j"$(($(nproc)+1))" +``` + +### Multiple working directories with `git worktrees` + +If you work with multiple branches or multiple copies of the repository, you should try `git worktrees`. + +To create a new branch that lives under a new working directory without disrupting your current working directory (useful for creating pull requests): +```sh +git worktree add -b my-shiny-new-branch ../living-at-my-new-working-directory based-on-my-crufty-old-commit-ish +``` + +To simply check out a commit-ish under a new working directory without disrupting your current working directory (useful for reviewing pull requests): +```sh +git worktree add --checkout ../where-my-checkout-commit-ish-will-live my-checkout-commit-ish +``` + +----- + +This synergizes well with [`ccache`](#cache-compilations-with-ccache) as objects resulting from unchanged code will most likely hit the cache and won't need to be recompiled. + +You can also set up [upstream refspecs](#reference-prs-easily-with-refspecs) to refer to pull requests easier in the above `git worktree` commands. + +Writing code +------------ + +### Format C/C++/Protobuf diffs with `clang-format-diff.py` + +See [contrib/devtools/README.md](contrib/devtools/README.md#clang-format-diff.py). + +### Format Python diffs with `yapf-diff.py` + +Usage is exactly the same as [`clang-format-diff.py`](#format-ccprotobuf-diffs-with-clang-format-diffpy). You can get it [here](https://github.com/MarcoFalke/yapf-diff). + +Rebasing/Merging code +------------- + +### More conflict context with `merge.conflictstyle diff3` + +For resolving merge/rebase conflicts, it can be useful to enable diff3 style using `git config merge.conflictstyle diff3`. Instead of + +```diff +<<< +yours +=== +theirs +>>> +``` + + you will see + +```diff +<<< +yours +||| +original +=== +theirs +>>> +``` + +This may make it much clearer what caused the conflict. In this style, you can often just look at what changed between *original* and *theirs*, and mechanically apply that to *yours* (or the other way around). + +Reviewing code +-------------- + +### Reduce mental load with `git diff` options + +When reviewing patches which change indentation in C++ files, use `git diff -w` and `git show -w`. This makes the diff algorithm ignore whitespace changes. This feature is also available on github.com, by adding `?w=1` at the end of any URL which shows a diff. + +When reviewing patches that change symbol names in many places, use `git diff --word-diff`. This will instead of showing the patch as deleted/added *lines*, show deleted/added *words*. + +When reviewing patches that move code around, try using `git diff --patience commit~:old/file.cpp commit:new/file/name.cpp`, and ignoring everything except the moved body of code which should show up as neither `+` or `-` lines. In case it was not a pure move, this may even work when combined with the `-w` or `--word-diff` options described above. `--color-moved=dimmed-zebra` will also dim the coloring of moved hunks in the diff on compatible terminals. + +### Reference PRs easily with `refspec`s + +When looking at other's pull requests, it may make sense to add the following section to your `.git/config` file: + +``` +[remote "upstream-pull"] + fetch = +refs/pull/*:refs/remotes/upstream-pull/* + url = git@github.com:bitcoin/bitcoin.git +``` + +This will add an `upstream-pull` remote to your git repository, which can be fetched using `git fetch --all` or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`, `git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER. + +### Diff the diffs with `git range-diff` + +It is very common for contributors to rebase their pull requests, or make changes to commits (perhaps in response to review) that are not at the head of their branch. This poses a problem for reviewers as when the contributor force pushes, the reviewer is no longer sure that his previous reviews of commits are still valid (as the commit hashes can now be different even though the diff is semantically the same). `git range-diff` can help solve this problem by diffing the diffs. + +For example, to identify the differences between your previously reviewed diffs P1-5, and the new diffs P1-2,N3-4 as illustrated below: +``` + P1--P2--P3--P4--P5 <-- previously-reviewed-head + / +...--m <-- master + \ + P1--P2--N3--N4--N5 <-- new-head (with P3 slightly modified) +``` + +You can do: +```sh +git range-diff master previously-reviewed-head new-head +``` + +Note that `git range-diff` also work for rebases. + +----- + +`git range-diff` also accepts normal `git diff` options, see [Reduce mental load with `git diff` options](#reduce-mental-load-with-git-diff-options) for useful `git diff` options. + +You can also set up [upstream refspecs](#reference-prs-easily-with-refspecs) to refer to pull requests easier in the above `git range-diff` commands. diff --git a/src/init.cpp b/src/init.cpp index 0013319ad5..8b831a726f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -53,6 +53,8 @@ #include <stdio.h> #ifndef WIN32 +#include <attributes.h> +#include <cerrno> #include <signal.h> #include <sys/stat.h> #endif @@ -92,6 +94,30 @@ std::unique_ptr<BanMan> g_banman; static const char* FEE_ESTIMATES_FILENAME="fee_estimates.dat"; +/** + * The PID file facilities. + */ +#ifndef WIN32 +static const char* BITCOIN_PID_FILENAME = "bitcoind.pid"; + +static fs::path GetPidFile() +{ + return AbsPathForConfigVal(fs::path(gArgs.GetArg("-pid", BITCOIN_PID_FILENAME))); +} + +NODISCARD static bool CreatePidFile() +{ + FILE* file = fsbridge::fopen(GetPidFile(), "w"); + if (file) { + fprintf(file, "%d\n", getpid()); + fclose(file); + return true; + } else { + return InitError(strprintf(_("Unable to create the PID file '%s': %s"), GetPidFile().string(), std::strerror(errno))); + } +} +#endif + ////////////////////////////////////////////////////////////////////////////// // // Shutdown @@ -262,9 +288,11 @@ void Shutdown(InitInterfaces& interfaces) #ifndef WIN32 try { - fs::remove(GetPidFile()); + if (!fs::remove(GetPidFile())) { + LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__); + } } catch (const fs::filesystem_error& e) { - LogPrintf("%s: Unable to remove pidfile: %s\n", __func__, e.what()); + LogPrintf("%s: Unable to remove PID file: %s\n", __func__, e.what()); } #endif interfaces.chain_clients.clear(); @@ -352,7 +380,7 @@ void SetupServerArgs() gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), false, OptionsCategory::OPTIONS); gArgs.AddArg("-datadir=<dir>", "Specify data directory", false, OptionsCategory::OPTIONS); gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), true, OptionsCategory::OPTIONS); - gArgs.AddArg("-dbcache=<n>", strprintf("Set database cache size in MiB (%d to %d, default: %d)", nMinDbCache, nMaxDbCache, nDefaultDbCache), false, OptionsCategory::OPTIONS); + gArgs.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (%d to %d, default: %d). In addition, unused mempool memory is shared for this cache (see -maxmempool).", nMinDbCache, nMaxDbCache, nDefaultDbCache), false, OptionsCategory::OPTIONS); gArgs.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (-nodebuglogfile to disable; default: %s)", DEFAULT_DEBUGLOGFILE), false, OptionsCategory::OPTIONS); gArgs.AddArg("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER), true, OptionsCategory::OPTIONS); gArgs.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", false, OptionsCategory::OPTIONS); @@ -1201,7 +1229,10 @@ bool AppInitMain(InitInterfaces& interfaces) const CChainParams& chainparams = Params(); // ********************************************************* Step 4a: application initialization #ifndef WIN32 - CreatePidFile(GetPidFile(), getpid()); + if (!CreatePidFile()) { + // Detailed error printed inside CreatePidFile(). + return false; + } #endif if (LogInstance().m_print_to_file) { if (gArgs.GetBoolArg("-shrinkdebugfile", LogInstance().DefaultShrinkDebugFile())) { diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 5b5430037c..0dac75834e 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -135,55 +135,55 @@ WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain, class WalletImpl : public Wallet { public: - explicit WalletImpl(const std::shared_ptr<CWallet>& wallet) : m_shared_wallet(wallet), m_wallet(*wallet.get()) {} + explicit WalletImpl(const std::shared_ptr<CWallet>& wallet) : m_wallet(wallet) {} bool encryptWallet(const SecureString& wallet_passphrase) override { - return m_wallet.EncryptWallet(wallet_passphrase); + return m_wallet->EncryptWallet(wallet_passphrase); } - bool isCrypted() override { return m_wallet.IsCrypted(); } - bool lock() override { return m_wallet.Lock(); } - bool unlock(const SecureString& wallet_passphrase) override { return m_wallet.Unlock(wallet_passphrase); } - bool isLocked() override { return m_wallet.IsLocked(); } + bool isCrypted() override { return m_wallet->IsCrypted(); } + bool lock() override { return m_wallet->Lock(); } + bool unlock(const SecureString& wallet_passphrase) override { return m_wallet->Unlock(wallet_passphrase); } + bool isLocked() override { return m_wallet->IsLocked(); } bool changeWalletPassphrase(const SecureString& old_wallet_passphrase, const SecureString& new_wallet_passphrase) override { - return m_wallet.ChangeWalletPassphrase(old_wallet_passphrase, new_wallet_passphrase); + return m_wallet->ChangeWalletPassphrase(old_wallet_passphrase, new_wallet_passphrase); } - void abortRescan() override { m_wallet.AbortRescan(); } - bool backupWallet(const std::string& filename) override { return m_wallet.BackupWallet(filename); } - std::string getWalletName() override { return m_wallet.GetName(); } + void abortRescan() override { m_wallet->AbortRescan(); } + bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } + std::string getWalletName() override { return m_wallet->GetName(); } bool getKeyFromPool(bool internal, CPubKey& pub_key) override { - return m_wallet.GetKeyFromPool(pub_key, internal); + return m_wallet->GetKeyFromPool(pub_key, internal); } - bool getPubKey(const CKeyID& address, CPubKey& pub_key) override { return m_wallet.GetPubKey(address, pub_key); } - bool getPrivKey(const CKeyID& address, CKey& key) override { return m_wallet.GetKey(address, key); } - bool isSpendable(const CTxDestination& dest) override { return IsMine(m_wallet, dest) & ISMINE_SPENDABLE; } - bool haveWatchOnly() override { return m_wallet.HaveWatchOnly(); }; + bool getPubKey(const CKeyID& address, CPubKey& pub_key) override { return m_wallet->GetPubKey(address, pub_key); } + bool getPrivKey(const CKeyID& address, CKey& key) override { return m_wallet->GetKey(address, key); } + bool isSpendable(const CTxDestination& dest) override { return IsMine(*m_wallet, dest) & ISMINE_SPENDABLE; } + bool haveWatchOnly() override { return m_wallet->HaveWatchOnly(); }; bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::string& purpose) override { - return m_wallet.SetAddressBook(dest, name, purpose); + return m_wallet->SetAddressBook(dest, name, purpose); } bool delAddressBook(const CTxDestination& dest) override { - return m_wallet.DelAddressBook(dest); + return m_wallet->DelAddressBook(dest); } bool getAddress(const CTxDestination& dest, std::string* name, isminetype* is_mine, std::string* purpose) override { - LOCK(m_wallet.cs_wallet); - auto it = m_wallet.mapAddressBook.find(dest); - if (it == m_wallet.mapAddressBook.end()) { + LOCK(m_wallet->cs_wallet); + auto it = m_wallet->mapAddressBook.find(dest); + if (it == m_wallet->mapAddressBook.end()) { return false; } if (name) { *name = it->second.name; } if (is_mine) { - *is_mine = IsMine(m_wallet, dest); + *is_mine = IsMine(*m_wallet, dest); } if (purpose) { *purpose = it->second.purpose; @@ -192,52 +192,52 @@ public: } std::vector<WalletAddress> getAddresses() override { - LOCK(m_wallet.cs_wallet); + LOCK(m_wallet->cs_wallet); std::vector<WalletAddress> result; - for (const auto& item : m_wallet.mapAddressBook) { - result.emplace_back(item.first, IsMine(m_wallet, item.first), item.second.name, item.second.purpose); + for (const auto& item : m_wallet->mapAddressBook) { + result.emplace_back(item.first, IsMine(*m_wallet, item.first), item.second.name, item.second.purpose); } return result; } - void learnRelatedScripts(const CPubKey& key, OutputType type) override { m_wallet.LearnRelatedScripts(key, type); } + void learnRelatedScripts(const CPubKey& key, OutputType type) override { m_wallet->LearnRelatedScripts(key, type); } bool addDestData(const CTxDestination& dest, const std::string& key, const std::string& value) override { - LOCK(m_wallet.cs_wallet); - return m_wallet.AddDestData(dest, key, value); + LOCK(m_wallet->cs_wallet); + return m_wallet->AddDestData(dest, key, value); } bool eraseDestData(const CTxDestination& dest, const std::string& key) override { - LOCK(m_wallet.cs_wallet); - return m_wallet.EraseDestData(dest, key); + LOCK(m_wallet->cs_wallet); + return m_wallet->EraseDestData(dest, key); } std::vector<std::string> getDestValues(const std::string& prefix) override { - LOCK(m_wallet.cs_wallet); - return m_wallet.GetDestValues(prefix); + LOCK(m_wallet->cs_wallet); + return m_wallet->GetDestValues(prefix); } void lockCoin(const COutPoint& output) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.LockCoin(output); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->LockCoin(output); } void unlockCoin(const COutPoint& output) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.UnlockCoin(output); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->UnlockCoin(output); } bool isLockedCoin(const COutPoint& output) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.IsLockedCoin(output.hash, output.n); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->IsLockedCoin(output.hash, output.n); } void listLockedCoins(std::vector<COutPoint>& outputs) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.ListLockedCoins(outputs); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->ListLockedCoins(outputs); } std::unique_ptr<PendingWalletTx> createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, @@ -246,25 +246,25 @@ public: CAmount& fee, std::string& fail_reason) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - auto pending = MakeUnique<PendingWalletTxImpl>(m_wallet); - if (!m_wallet.CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_key, fee, change_pos, + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + auto pending = MakeUnique<PendingWalletTxImpl>(*m_wallet); + if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_key, fee, change_pos, fail_reason, coin_control, sign)) { return {}; } return std::move(pending); } - bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet.TransactionCanBeAbandoned(txid); } + bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); } bool abandonTransaction(const uint256& txid) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.AbandonTransaction(*locked_chain, txid); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->AbandonTransaction(*locked_chain, txid); } bool transactionCanBeBumped(const uint256& txid) override { - return feebumper::TransactionCanBeBumped(&m_wallet, txid); + return feebumper::TransactionCanBeBumped(m_wallet.get(), txid); } bool createBumpTransaction(const uint256& txid, const CCoinControl& coin_control, @@ -274,46 +274,46 @@ public: CAmount& new_fee, CMutableTransaction& mtx) override { - return feebumper::CreateTransaction(&m_wallet, txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) == + return feebumper::CreateTransaction(m_wallet.get(), txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) == feebumper::Result::OK; } - bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(&m_wallet, mtx); } + bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid) override { - return feebumper::CommitTransaction(&m_wallet, txid, std::move(mtx), errors, bumped_txid) == + return feebumper::CommitTransaction(m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) == feebumper::Result::OK; } CTransactionRef getTx(const uint256& txid) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - auto mi = m_wallet.mapWallet.find(txid); - if (mi != m_wallet.mapWallet.end()) { + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + auto mi = m_wallet->mapWallet.find(txid); + if (mi != m_wallet->mapWallet.end()) { return mi->second.tx; } return {}; } WalletTx getWalletTx(const uint256& txid) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - auto mi = m_wallet.mapWallet.find(txid); - if (mi != m_wallet.mapWallet.end()) { - return MakeWalletTx(*locked_chain, m_wallet, mi->second); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + auto mi = m_wallet->mapWallet.find(txid); + if (mi != m_wallet->mapWallet.end()) { + return MakeWalletTx(*locked_chain, *m_wallet, mi->second); } return {}; } std::vector<WalletTx> getWalletTxs() override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); std::vector<WalletTx> result; - result.reserve(m_wallet.mapWallet.size()); - for (const auto& entry : m_wallet.mapWallet) { - result.emplace_back(MakeWalletTx(*locked_chain, m_wallet, entry.second)); + result.reserve(m_wallet->mapWallet.size()); + for (const auto& entry : m_wallet->mapWallet) { + result.emplace_back(MakeWalletTx(*locked_chain, *m_wallet, entry.second)); } return result; } @@ -322,16 +322,16 @@ public: int& num_blocks, int64_t& block_time) override { - auto locked_chain = m_wallet.chain().lock(true /* try_lock */); + auto locked_chain = m_wallet->chain().lock(true /* try_lock */); if (!locked_chain) { return false; } - TRY_LOCK(m_wallet.cs_wallet, locked_wallet); + TRY_LOCK(m_wallet->cs_wallet, locked_wallet); if (!locked_wallet) { return false; } - auto mi = m_wallet.mapWallet.find(txid); - if (mi == m_wallet.mapWallet.end()) { + auto mi = m_wallet->mapWallet.find(txid); + if (mi == m_wallet->mapWallet.end()) { return false; } if (Optional<int> height = locked_chain->getHeight()) { @@ -350,37 +350,37 @@ public: bool& in_mempool, int& num_blocks) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - auto mi = m_wallet.mapWallet.find(txid); - if (mi != m_wallet.mapWallet.end()) { + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + auto mi = m_wallet->mapWallet.find(txid); + if (mi != m_wallet->mapWallet.end()) { num_blocks = locked_chain->getHeight().get_value_or(-1); in_mempool = mi->second.InMempool(); order_form = mi->second.vOrderForm; tx_status = MakeWalletTxStatus(*locked_chain, mi->second); - return MakeWalletTx(*locked_chain, m_wallet, mi->second); + return MakeWalletTx(*locked_chain, *m_wallet, mi->second); } return {}; } WalletBalances getBalances() override { WalletBalances result; - result.balance = m_wallet.GetBalance(); - result.unconfirmed_balance = m_wallet.GetUnconfirmedBalance(); - result.immature_balance = m_wallet.GetImmatureBalance(); - result.have_watch_only = m_wallet.HaveWatchOnly(); + result.balance = m_wallet->GetBalance(); + result.unconfirmed_balance = m_wallet->GetUnconfirmedBalance(); + result.immature_balance = m_wallet->GetImmatureBalance(); + result.have_watch_only = m_wallet->HaveWatchOnly(); if (result.have_watch_only) { - result.watch_only_balance = m_wallet.GetBalance(ISMINE_WATCH_ONLY); - result.unconfirmed_watch_only_balance = m_wallet.GetUnconfirmedWatchOnlyBalance(); - result.immature_watch_only_balance = m_wallet.GetImmatureWatchOnlyBalance(); + result.watch_only_balance = m_wallet->GetBalance(ISMINE_WATCH_ONLY); + result.unconfirmed_watch_only_balance = m_wallet->GetUnconfirmedWatchOnlyBalance(); + result.immature_watch_only_balance = m_wallet->GetImmatureWatchOnlyBalance(); } return result; } bool tryGetBalances(WalletBalances& balances, int& num_blocks) override { - auto locked_chain = m_wallet.chain().lock(true /* try_lock */); + auto locked_chain = m_wallet->chain().lock(true /* try_lock */); if (!locked_chain) return false; - TRY_LOCK(m_wallet.cs_wallet, locked_wallet); + TRY_LOCK(m_wallet->cs_wallet, locked_wallet); if (!locked_wallet) { return false; } @@ -388,68 +388,68 @@ public: num_blocks = locked_chain->getHeight().get_value_or(-1); return true; } - CAmount getBalance() override { return m_wallet.GetBalance(); } + CAmount getBalance() override { return m_wallet->GetBalance(); } CAmount getAvailableBalance(const CCoinControl& coin_control) override { - return m_wallet.GetAvailableBalance(&coin_control); + return m_wallet->GetAvailableBalance(&coin_control); } isminetype txinIsMine(const CTxIn& txin) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.IsMine(txin); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->IsMine(txin); } isminetype txoutIsMine(const CTxOut& txout) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.IsMine(txout); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->IsMine(txout); } CAmount getDebit(const CTxIn& txin, isminefilter filter) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.GetDebit(txin, filter); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->GetDebit(txin, filter); } CAmount getCredit(const CTxOut& txout, isminefilter filter) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); - return m_wallet.GetCredit(txout, filter); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); + return m_wallet->GetCredit(txout, filter); } CoinsList listCoins() override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); CoinsList result; - for (const auto& entry : m_wallet.ListCoins(*locked_chain)) { + for (const auto& entry : m_wallet->ListCoins(*locked_chain)) { auto& group = result[entry.first]; for (const auto& coin : entry.second) { group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i), - MakeWalletTxOut(*locked_chain, m_wallet, *coin.tx, coin.i, coin.nDepth)); + MakeWalletTxOut(*locked_chain, *m_wallet, *coin.tx, coin.i, coin.nDepth)); } } return result; } std::vector<WalletTxOut> getCoins(const std::vector<COutPoint>& outputs) override { - auto locked_chain = m_wallet.chain().lock(); - LOCK(m_wallet.cs_wallet); + auto locked_chain = m_wallet->chain().lock(); + LOCK(m_wallet->cs_wallet); std::vector<WalletTxOut> result; result.reserve(outputs.size()); for (const auto& output : outputs) { result.emplace_back(); - auto it = m_wallet.mapWallet.find(output.hash); - if (it != m_wallet.mapWallet.end()) { + auto it = m_wallet->mapWallet.find(output.hash); + if (it != m_wallet->mapWallet.end()) { int depth = it->second.GetDepthInMainChain(*locked_chain); if (depth >= 0) { - result.back() = MakeWalletTxOut(*locked_chain, m_wallet, it->second, output.n, depth); + result.back() = MakeWalletTxOut(*locked_chain, *m_wallet, it->second, output.n, depth); } } } return result; } - CAmount getRequiredFee(unsigned int tx_bytes) override { return GetRequiredFee(m_wallet, tx_bytes); } + CAmount getRequiredFee(unsigned int tx_bytes) override { return GetRequiredFee(*m_wallet, tx_bytes); } CAmount getMinimumFee(unsigned int tx_bytes, const CCoinControl& coin_control, int* returned_target, @@ -457,55 +457,54 @@ public: { FeeCalculation fee_calc; CAmount result; - result = GetMinimumFee(m_wallet, tx_bytes, coin_control, ::mempool, ::feeEstimator, &fee_calc); + result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, ::mempool, ::feeEstimator, &fee_calc); if (returned_target) *returned_target = fee_calc.returnedTarget; if (reason) *reason = fee_calc.reason; return result; } - unsigned int getConfirmTarget() override { return m_wallet.m_confirm_target; } - bool hdEnabled() override { return m_wallet.IsHDEnabled(); } - bool canGetAddresses() override { return m_wallet.CanGetAddresses(); } - bool IsWalletFlagSet(uint64_t flag) override { return m_wallet.IsWalletFlagSet(flag); } - OutputType getDefaultAddressType() override { return m_wallet.m_default_address_type; } - OutputType getDefaultChangeType() override { return m_wallet.m_default_change_type; } + unsigned int getConfirmTarget() override { return m_wallet->m_confirm_target; } + bool hdEnabled() override { return m_wallet->IsHDEnabled(); } + bool canGetAddresses() override { return m_wallet->CanGetAddresses(); } + bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); } + OutputType getDefaultAddressType() override { return m_wallet->m_default_address_type; } + OutputType getDefaultChangeType() override { return m_wallet->m_default_change_type; } void remove() override { - RemoveWallet(m_shared_wallet); + RemoveWallet(m_wallet); } std::unique_ptr<Handler> handleUnload(UnloadFn fn) override { - return MakeHandler(m_wallet.NotifyUnload.connect(fn)); + return MakeHandler(m_wallet->NotifyUnload.connect(fn)); } std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) override { - return MakeHandler(m_wallet.ShowProgress.connect(fn)); + return MakeHandler(m_wallet->ShowProgress.connect(fn)); } std::unique_ptr<Handler> handleStatusChanged(StatusChangedFn fn) override { - return MakeHandler(m_wallet.NotifyStatusChanged.connect([fn](CCryptoKeyStore*) { fn(); })); + return MakeHandler(m_wallet->NotifyStatusChanged.connect([fn](CCryptoKeyStore*) { fn(); })); } std::unique_ptr<Handler> handleAddressBookChanged(AddressBookChangedFn fn) override { - return MakeHandler(m_wallet.NotifyAddressBookChanged.connect( + return MakeHandler(m_wallet->NotifyAddressBookChanged.connect( [fn](CWallet*, const CTxDestination& address, const std::string& label, bool is_mine, const std::string& purpose, ChangeType status) { fn(address, label, is_mine, purpose, status); })); } std::unique_ptr<Handler> handleTransactionChanged(TransactionChangedFn fn) override { - return MakeHandler(m_wallet.NotifyTransactionChanged.connect( + return MakeHandler(m_wallet->NotifyTransactionChanged.connect( [fn](CWallet*, const uint256& txid, ChangeType status) { fn(txid, status); })); } std::unique_ptr<Handler> handleWatchOnlyChanged(WatchOnlyChangedFn fn) override { - return MakeHandler(m_wallet.NotifyWatchonlyChanged.connect(fn)); + return MakeHandler(m_wallet->NotifyWatchonlyChanged.connect(fn)); } std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) override { - return MakeHandler(m_wallet.NotifyCanGetAddressesChanged.connect(fn)); + return MakeHandler(m_wallet->NotifyCanGetAddressesChanged.connect(fn)); } - std::shared_ptr<CWallet> m_shared_wallet; - CWallet& m_wallet; + std::shared_ptr<CWallet> m_wallet; }; class WalletClientImpl : public ChainClient diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index c9cdd0d1cd..7b9b4310e7 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -12,7 +12,7 @@ #include <future> -const char* TransactionErrorString(const TransactionError err) +std::string TransactionErrorString(const TransactionError err) { switch (err) { case TransactionError::OK: @@ -33,22 +33,16 @@ const char* 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::UNKNOWN_ERROR: - default: break; + // no default case, so the compiler can warn about missing cases } - return "Unknown error"; + assert(false); } -bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, TransactionError& error, std::string& err_string, const bool allowhighfees) +TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, std::string& err_string, const CAmount& highfee) { std::promise<void> promise; hashTx = tx->GetHash(); - CAmount nMaxRawTxFee = maxTxFee; - if (allowhighfees) - nMaxRawTxFee = 0; - { // cs_main scope LOCK(cs_main); CCoinsViewCache &view = *pcoinsTip; @@ -63,19 +57,16 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction CValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, - nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) { + nullptr /* plTxnReplaced */, false /* bypass_limits */, highfee)) { if (state.IsInvalid()) { err_string = FormatStateMessage(state); - error = TransactionError::MEMPOOL_REJECTED; - return false; + return TransactionError::MEMPOOL_REJECTED; } else { if (fMissingInputs) { - error = TransactionError::MISSING_INPUTS; - return false; + return TransactionError::MISSING_INPUTS; } err_string = FormatStateMessage(state); - error = TransactionError::MEMPOOL_ERROR; - return false; + return TransactionError::MEMPOOL_ERROR; } } else { // If wallet is enabled, ensure that the wallet has been made aware @@ -88,8 +79,7 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction }); } } else if (fHaveChain) { - error = TransactionError::ALREADY_IN_CHAIN; - return false; + return TransactionError::ALREADY_IN_CHAIN; } else { // Make sure we don't block forever if re-sending // a transaction already in mempool. @@ -100,16 +90,14 @@ bool BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, Transaction promise.get_future().wait(); - if(!g_connman) { - error = TransactionError::P2P_DISABLED; - return false; + if (!g_connman) { + return TransactionError::P2P_DISABLED; } CInv inv(MSG_TX, hashTx); - g_connman->ForEachNode([&inv](CNode* pnode) - { + g_connman->ForEachNode([&inv](CNode* pnode) { pnode->PushInventory(inv); }); - return true; - } + return TransactionError::OK; +} diff --git a/src/node/transaction.h b/src/node/transaction.h index 3b0cbba98b..3457ececa4 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -1,17 +1,16 @@ -// Copyright (c) 2017-2018 The Bitcoin Core developers +// Copyright (c) 2017-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. #ifndef BITCOIN_NODE_TRANSACTION_H #define BITCOIN_NODE_TRANSACTION_H +#include <attributes.h> #include <primitives/transaction.h> #include <uint256.h> enum class TransactionError { - OK = 0, - UNKNOWN_ERROR, - + OK, //!< No error MISSING_INPUTS, ALREADY_IN_CHAIN, P2P_DISABLED, @@ -20,24 +19,19 @@ enum class TransactionError { INVALID_PSBT, PSBT_MISMATCH, SIGHASH_MISMATCH, - - ERROR_COUNT }; -#define TRANSACTION_ERR_LAST TransactionError::ERROR_COUNT - -const char* TransactionErrorString(const TransactionError error); +std::string TransactionErrorString(const TransactionError error); /** * Broadcast a transaction * * @param[in] tx the transaction to broadcast * @param[out] &txid the txid of the transaction, if successfully broadcast - * @param[out] &error reference to UniValue to fill with error info on failure * @param[out] &err_string reference to std::string to fill with error string if available - * @param[in] allowhighfees whether to allow fees exceeding maxTxFee - * return true on success, false on error (and fills in `error`) + * @param[in] highfee Reject txs with fees higher than this (if 0, accept any fee) + * return error */ -bool BroadcastTransaction(CTransactionRef tx, uint256& txid, TransactionError& error, std::string& err_string, bool allowhighfees = false); +NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/psbt.cpp b/src/psbt.cpp index 32fb459dec..0fb7d49d7d 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -309,21 +309,19 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti return true; } -bool CombinePSBTs(PartiallySignedTransaction& out, TransactionError& error, const std::vector<PartiallySignedTransaction>& psbtxs) +TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs) { out = psbtxs[0]; // Copy the first one // Merge for (auto it = std::next(psbtxs.begin()); it != psbtxs.end(); ++it) { if (!out.Merge(*it)) { - error = TransactionError::PSBT_MISMATCH; - return false; + return TransactionError::PSBT_MISMATCH; } } if (!out.IsSane()) { - error = TransactionError::INVALID_PSBT; - return false; + return TransactionError::INVALID_PSBT; } - return true; + return TransactionError::OK; } diff --git a/src/psbt.h b/src/psbt.h index 27b0aedd05..c889dad361 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1,4 +1,4 @@ -// 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. @@ -575,10 +575,9 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti * Combines PSBTs with the same underlying transaction, resulting in a single PSBT with all partial signatures from each input. * * @param[out] &out the combined PSBT, if successful - * @param[out] &error reference to TransactionError to fill with error info on failure * @param[in] psbtxs the PSBTs to combine - * @return True if we successfully combined the transactions, false if they were not compatible + * @return error (OK if we successfully combined the transactions, other error if they were not compatible) */ -bool CombinePSBTs(PartiallySignedTransaction& out, TransactionError& error, const std::vector<PartiallySignedTransaction>& psbtxs); +NODISCARD TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs); #endif // BITCOIN_PSBT_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 38e2dc237e..5a714a137e 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1065,10 +1065,11 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) bool allowhighfees = false; if (!request.params[1].isNull()) allowhighfees = request.params[1].get_bool(); + const CAmount highfee{allowhighfees ? 0 : ::maxTxFee}; uint256 txid; - TransactionError err; std::string err_string; - if (!BroadcastTransaction(tx, txid, err, err_string, allowhighfees)) { + const TransactionError err = BroadcastTransaction(tx, txid, err_string, highfee); + if (TransactionError::OK != err) { throw JSONRPCTransactionError(err, err_string); } @@ -1493,8 +1494,8 @@ UniValue combinepsbt(const JSONRPCRequest& request) } PartiallySignedTransaction merged_psbt; - TransactionError error; - if (!CombinePSBTs(merged_psbt, error, psbtxs)) { + const TransactionError error = CombinePSBTs(merged_psbt, psbtxs); + if (error != TransactionError::OK) { throw JSONRPCTransactionError(error); } diff --git a/src/util/system.cpp b/src/util/system.cpp index 27ed24d012..6e82de743a 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -74,7 +74,6 @@ const int64_t nStartupTime = GetTime(); const char * const BITCOIN_CONF_FILENAME = "bitcoin.conf"; -const char * const BITCOIN_PID_FILENAME = "bitcoind.pid"; ArgsManager gArgs; @@ -965,23 +964,6 @@ std::string ArgsManager::GetChainName() const return CBaseChainParams::MAIN; } -#ifndef WIN32 -fs::path GetPidFile() -{ - return AbsPathForConfigVal(fs::path(gArgs.GetArg("-pid", BITCOIN_PID_FILENAME))); -} - -void CreatePidFile(const fs::path &path, pid_t pid) -{ - FILE* file = fsbridge::fopen(path, "w"); - if (file) - { - fprintf(file, "%d\n", pid); - fclose(file); - } -} -#endif - bool RenameOver(fs::path src, fs::path dest) { #ifdef WIN32 diff --git a/src/util/system.h b/src/util/system.h index 8867e49478..69ae11d1ed 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -40,7 +40,6 @@ int64_t GetStartupTime(); extern const char * const BITCOIN_CONF_FILENAME; -extern const char * const BITCOIN_PID_FILENAME; /** Translate a message to the native language of the user. */ const extern std::function<std::string(const char*)> G_TRANSLATION_FUN; @@ -86,10 +85,6 @@ const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); -#ifndef WIN32 -fs::path GetPidFile(); -void CreatePidFile(const fs::path &path, pid_t pid); -#endif #ifdef WIN32 fs::path GetSpecialFolderPath(int nFolder, bool fCreate = true); #endif diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index 761e7b7dd7..1b17b09763 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -4,7 +4,7 @@ #include <wallet/psbtwallet.h> -bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, TransactionError& error, bool& complete, int sighash_type, bool sign, bool bip32derivs) +TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs) { LOCK(pwallet->cs_wallet); // Get all of the previous transactions @@ -19,8 +19,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness. if (!input.IsSane()) { - error = TransactionError::INVALID_PSBT; - return false; + return TransactionError::INVALID_PSBT; } // If we have no utxo, grab it from the wallet. @@ -37,8 +36,7 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac // Get the Sighash type if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) { - error = TransactionError::SIGHASH_MISMATCH; - return false; + return TransactionError::SIGHASH_MISMATCH; } complete &= SignPSBTInput(HidingSigningProvider(pwallet, !sign, !bip32derivs), psbtx, i, sighash_type); @@ -58,5 +56,5 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, Transac psbt_out.FromSignatureData(sigdata); } - return true; + return TransactionError::OK; } diff --git a/src/wallet/psbtwallet.h b/src/wallet/psbtwallet.h index b679f5c6ba..a24a0967d2 100644 --- a/src/wallet/psbtwallet.h +++ b/src/wallet/psbtwallet.h @@ -1,4 +1,4 @@ -// 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. @@ -18,16 +18,14 @@ * * @param[in] pwallet pointer to a wallet * @param[in] &psbtx reference to PartiallySignedTransaction to fill in - * @param[out] &error reference to UniValue to fill with error info on failure * @param[out] &complete indicates whether the PSBT is now complete * @param[in] sighash_type the sighash type to use when signing (if PSBT does not specify) * @param[in] sign whether to sign or not * @param[in] bip32derivs whether to fill in bip32 derivation information if available - * return true on success, false on error (and fills in `error`) + * return error */ -bool FillPSBT(const CWallet* pwallet, +NODISCARD TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, - TransactionError& error, bool& complete, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 37fc88dfd5..97c6c38be1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4007,8 +4007,8 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? false : request.params[3].get_bool(); bool complete = true; - TransactionError err; - if (!FillPSBT(pwallet, psbtx, err, complete, nHashType, sign, bip32derivs)) { + const TransactionError err = FillPSBT(pwallet, psbtx, complete, nHashType, sign, bip32derivs); + if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -4125,8 +4125,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? false : request.params[4].get_bool(); bool complete = true; - TransactionError err; - if (!FillPSBT(pwallet, psbtx, err, complete, 1, false, bip32derivs)) { + const TransactionError err = FillPSBT(pwallet, psbtx, complete, 1, false, bip32derivs); + if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 2a3149de46..789e86e21b 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -62,9 +62,8 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) ssData >> psbtx; // Fill transaction with our data - TransactionError err; bool complete = true; - FillPSBT(&m_wallet, psbtx, err, complete, SIGHASH_ALL, false, true); + BOOST_REQUIRE_EQUAL(TransactionError::OK, FillPSBT(&m_wallet, psbtx, complete, SIGHASH_ALL, false, true)); // Get the final tx CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d174e308f0..388422bec8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -371,6 +371,8 @@ void CWallet::UpgradeKeyMetadata() return; } + std::unique_ptr<WalletBatch> batch = MakeUnique<WalletBatch>(*database); + size_t cnt = 0; for (auto& meta_pair : mapKeyMetadata) { CKeyMetadata& meta = meta_pair.second; if (!meta.hd_seed_id.IsNull() && !meta.has_key_origin && meta.hdKeypath != "s") { // If the hdKeypath is "s", that's the seed and it doesn't have a key origin @@ -392,10 +394,15 @@ void CWallet::UpgradeKeyMetadata() // Write meta to wallet CPubKey pubkey; if (GetPubKey(meta_pair.first, pubkey)) { - WriteKeyMetadata(meta, pubkey, true); + batch->WriteKeyMetadata(meta, pubkey, true); + if (++cnt % 1000 == 0) { + // avoid creating overlarge in-memory batches in case the wallet contains large amounts of keys + batch.reset(new WalletBatch(*database)); + } } } } + batch.reset(); //write before setting the flag SetWalletFlag(WALLET_FLAG_KEY_ORIGIN_METADATA); } diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 1efc50e71f..658a8cd75e 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -38,31 +38,29 @@ def find_spendable_utxo(node, min_value): raise AssertionError("Unspent output equal or higher than %s not found" % min_value) +txs_mined = {} # txindex from txid to blockhash + class SegWitTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. - # TODO: remove -txindex. Currently required for getrawtransaction call. self.extra_args = [ [ "-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", - "-txindex" ], [ "-blockversion=4", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", - "-txindex" ], [ "-blockversion=536870915", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", - "-txindex" ], ] @@ -157,10 +155,10 @@ class SegWitTest(BitcoinTestFramework): self.log.info("Verify previous witness txs skipped for mining can now be mined") assert_equal(len(self.nodes[2].getrawmempool()), 4) - block = self.nodes[2].generate(1) # block 432 (first block with new rules; 432 = 144 * 3) + blockhash = self.nodes[2].generate(1)[0] # block 432 (first block with new rules; 432 = 144 * 3) sync_blocks(self.nodes) assert_equal(len(self.nodes[2].getrawmempool()), 0) - segwit_tx_list = self.nodes[2].getblock(block[0])["tx"] + segwit_tx_list = self.nodes[2].getblock(blockhash)["tx"] assert_equal(len(segwit_tx_list), 5) self.log.info("Verify default node can't accept txs with missing witness") @@ -174,15 +172,16 @@ class SegWitTest(BitcoinTestFramework): self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V1][0], False, witness_script(True, self.pubkey[0])) self.log.info("Verify block and transaction serialization rpcs return differing serializations depending on rpc serialization flag") - assert(self.nodes[2].getblock(block[0], False) != self.nodes[0].getblock(block[0], False)) - assert(self.nodes[1].getblock(block[0], False) == self.nodes[2].getblock(block[0], False)) - for i in range(len(segwit_tx_list)): - tx = FromHex(CTransaction(), self.nodes[2].gettransaction(segwit_tx_list[i])["hex"]) - assert(self.nodes[2].getrawtransaction(segwit_tx_list[i]) != self.nodes[0].getrawtransaction(segwit_tx_list[i])) - assert(self.nodes[1].getrawtransaction(segwit_tx_list[i], 0) == self.nodes[2].getrawtransaction(segwit_tx_list[i])) - assert(self.nodes[0].getrawtransaction(segwit_tx_list[i]) != self.nodes[2].gettransaction(segwit_tx_list[i])["hex"]) - assert(self.nodes[1].getrawtransaction(segwit_tx_list[i]) == self.nodes[2].gettransaction(segwit_tx_list[i])["hex"]) - assert(self.nodes[0].getrawtransaction(segwit_tx_list[i]) == bytes_to_hex_str(tx.serialize_without_witness())) + assert(self.nodes[2].getblock(blockhash, False) != self.nodes[0].getblock(blockhash, False)) + assert(self.nodes[1].getblock(blockhash, False) == self.nodes[2].getblock(blockhash, False)) + + for tx_id in segwit_tx_list: + tx = FromHex(CTransaction(), self.nodes[2].gettransaction(tx_id)["hex"]) + assert(self.nodes[2].getrawtransaction(tx_id, False, blockhash) != self.nodes[0].getrawtransaction(tx_id, False, blockhash)) + assert(self.nodes[1].getrawtransaction(tx_id, False, blockhash) == self.nodes[2].getrawtransaction(tx_id, False, blockhash)) + assert(self.nodes[0].getrawtransaction(tx_id, False, blockhash) != self.nodes[2].gettransaction(tx_id)["hex"]) + assert(self.nodes[1].getrawtransaction(tx_id, False, blockhash) == self.nodes[2].gettransaction(tx_id)["hex"]) + assert(self.nodes[0].getrawtransaction(tx_id, False, blockhash) == bytes_to_hex_str(tx.serialize_without_witness())) self.log.info("Verify witness txs without witness data are invalid after the fork") self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)', wit_ids[NODE_2][WIT_V0][2], sign=False) @@ -538,7 +537,7 @@ class SegWitTest(BitcoinTestFramework): tx.rehash() signresults = self.nodes[0].signrawtransactionwithwallet(bytes_to_hex_str(tx.serialize_without_witness()))['hex'] txid = self.nodes[0].sendrawtransaction(signresults, True) - self.nodes[0].generate(1) + txs_mined[txid] = self.nodes[0].generate(1)[0] sync_blocks(self.nodes) watchcount = 0 spendcount = 0 @@ -581,7 +580,7 @@ class SegWitTest(BitcoinTestFramework): tx = CTransaction() for i in txids: txtmp = CTransaction() - txraw = self.nodes[0].getrawtransaction(i) + txraw = self.nodes[0].getrawtransaction(i, 0, txs_mined[i]) f = BytesIO(hex_str_to_bytes(txraw)) txtmp.deserialize(f) for j in range(len(txtmp.vout)): diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index a47a556406..f33040242c 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -43,8 +43,7 @@ class RESTTest (BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - # TODO: remove -txindex. Currently required for getrawtransaction call. - self.extra_args = [["-rest", "-txindex"], []] + self.extra_args = [["-rest"], []] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -91,15 +90,17 @@ class RESTTest (BitcoinTestFramework): txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1) self.sync_all() - self.nodes[1].generatetoaddress(1, not_related_address) - self.sync_all() - bb_hash = self.nodes[0].getbestblockhash() - assert_equal(self.nodes[1].getbalance(), Decimal("0.1")) - - self.log.info("Load the transaction using the /tx URI") + self.log.info("Test the /tx URI") json_obj = self.test_rest_request("/tx/{}".format(txid)) + assert_equal(json_obj['txid'], txid) + + # Check hex format response + hex_response = self.test_rest_request("/tx/{}".format(txid), req_type=ReqType.HEX, ret_type=RetType.OBJ) + assert_greater_than_or_equal(int(hex_response.getheader('content-length')), + json_obj['size']*2) + spent = (json_obj['vin'][0]['txid'], json_obj['vin'][0]['vout']) # get the vin to later check for utxo (should be spent by then) # get n of 0.1 outpoint n, = filter_output_indices_by_value(json_obj['vout'], Decimal('0.1')) @@ -107,9 +108,14 @@ class RESTTest (BitcoinTestFramework): self.log.info("Query an unspent TXO using the /getutxos URI") - json_obj = self.test_rest_request("/getutxos/{}-{}".format(*spending)) + self.nodes[1].generatetoaddress(1, not_related_address) + self.sync_all() + bb_hash = self.nodes[0].getbestblockhash() + + assert_equal(self.nodes[1].getbalance(), Decimal("0.1")) # Check chainTip response + json_obj = self.test_rest_request("/getutxos/{}-{}".format(*spending)) assert_equal(json_obj['chaintipHash'], bb_hash) # Make sure there is one utxo @@ -274,17 +280,6 @@ class RESTTest (BitcoinTestFramework): json_obj = self.test_rest_request("/headers/5/{}".format(bb_hash)) assert_equal(len(json_obj), 5) # now we should have 5 header objects - self.log.info("Test the /tx URI") - - tx_hash = block_json_obj['tx'][0]['txid'] - json_obj = self.test_rest_request("/tx/{}".format(tx_hash)) - assert_equal(json_obj['txid'], tx_hash) - - # Check hex format response - hex_response = self.test_rest_request("/tx/{}".format(tx_hash), req_type=ReqType.HEX, ret_type=RetType.OBJ) - assert_greater_than_or_equal(int(hex_response.getheader('content-length')), - json_obj['size']*2) - self.log.info("Test tx inclusion in the /mempool and /block URIs") # Make 3 tx and mine them on node 1 diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index df8fe23a2e..d0bef48cb6 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -184,7 +184,7 @@ class MiningTest(BitcoinTestFramework): self.log.info('submitheader tests') assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * BLOCK_HEADER_SIZE)) assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * (BLOCK_HEADER_SIZE-2))) - assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata=super(CBlock, bad_block).serialize().hex())) + assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata=b2x(super(CBlock, bad_block).serialize()))) block.nTime += 1 block.solve() diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 9a79a7ee43..e9098e4f5a 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -20,8 +20,6 @@ class PSBTTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = False self.num_nodes = 3 - # TODO: remove -txindex. Currently required for getrawtransaction call. - self.extra_args = [["-txindex"], ["-txindex"], ["-txindex"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -161,11 +159,11 @@ class PSBTTest(BitcoinTestFramework): node1_addr = self.nodes[1].getnewaddress() node2_addr = self.nodes[2].getnewaddress() txid1 = self.nodes[0].sendtoaddress(node1_addr, 13) - txid2 =self.nodes[0].sendtoaddress(node2_addr, 13) - self.nodes[0].generate(6) + txid2 = self.nodes[0].sendtoaddress(node2_addr, 13) + blockhash = self.nodes[0].generate(6)[0] self.sync_all() - vout1 = find_output(self.nodes[1], txid1, 13) - vout2 = find_output(self.nodes[2], txid2, 13) + vout1 = find_output(self.nodes[1], txid1, 13, blockhash=blockhash) + vout2 = find_output(self.nodes[2], txid2, 13, blockhash=blockhash) # Create a psbt spending outputs from nodes 1 and 2 psbt_orig = self.nodes[0].createpsbt([{"txid":txid1, "vout":vout1}, {"txid":txid2, "vout":vout2}], {self.nodes[0].getnewaddress():25.999}) @@ -344,9 +342,9 @@ class PSBTTest(BitcoinTestFramework): addr = self.nodes[1].getnewaddress("", "p2sh-segwit") txid = self.nodes[0].sendtoaddress(addr, 7) addrinfo = self.nodes[1].getaddressinfo(addr) - self.nodes[0].generate(6) + blockhash = self.nodes[0].generate(6)[0] self.sync_all() - vout = find_output(self.nodes[0], txid, 7) + vout = find_output(self.nodes[0], txid, 7, blockhash=blockhash) psbt = self.nodes[1].createpsbt([{"txid":txid, "vout":vout}], {self.nodes[0].getnewaddress("", "p2sh-segwit"):Decimal('6.999')}) analyzed = self.nodes[0].analyzepsbt(psbt) assert not analyzed['inputs'][0]['has_utxo'] and not analyzed['inputs'][0]['is_final'] and analyzed['inputs'][0]['next'] == 'updater' and analyzed['next'] == 'updater' diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index a97d753626..c793cf00de 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -42,7 +42,6 @@ class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 - # TODO: remove -txindex. Currently required for getrawtransaction call. self.extra_args = [["-addresstype=legacy", "-txindex"], ["-addresstype=legacy", "-txindex"], ["-addresstype=legacy", "-txindex"]] def skip_test_if_missing_module(self): diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8c4c0d7226..869b36673e 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -318,6 +318,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): bitcoin_cli=self.options.bitcoincli, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, + cwd=self.options.tmpdir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli, @@ -469,6 +470,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): bitcoin_cli=self.options.bitcoincli, mocktime=self.mocktime, coverage_dir=None, + cwd=self.options.tmpdir, )) self.nodes[i].args = args self.start_node(i) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 999ea68254..352774914d 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -61,7 +61,7 @@ class TestNode(): To make things easier for the test writer, any unrecognised messages will be dispatched to the RPC connection.""" - def __init__(self, i, datadir, *, rpchost, timewait, bitcoind, bitcoin_cli, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False, start_perf=False): + def __init__(self, i, datadir, *, rpchost, timewait, bitcoind, bitcoin_cli, mocktime, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False): """ Kwargs: start_perf (bool): If True, begin profiling the node with `perf` as soon as @@ -76,6 +76,7 @@ class TestNode(): self.rpc_timeout = timewait self.binary = bitcoind self.coverage_dir = coverage_dir + self.cwd = cwd if extra_conf is not None: append_config(datadir, extra_conf) # Most callers will just need to add extra args to the standard list below. @@ -171,7 +172,7 @@ class TestNode(): assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection") return getattr(self.rpc, name) - def start(self, extra_args=None, *, stdout=None, stderr=None, **kwargs): + def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, **kwargs): """Start the node.""" if extra_args is None: extra_args = self.extra_args @@ -184,6 +185,9 @@ class TestNode(): self.stderr = stderr self.stdout = stdout + if cwd is None: + cwd = self.cwd + # Delete any existing cookie file -- if such a file exists (eg due to # unclean shutdown), it will get overwritten anyway by bitcoind, and # potentially interfere with our attempt to authenticate @@ -192,7 +196,7 @@ class TestNode(): # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1") - self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, **kwargs) + self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, cwd=cwd, **kwargs) self.running = True self.log.debug("bitcoind started, waiting for RPC to come up") diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index d0a78d8dfd..fef9982412 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -410,12 +410,12 @@ def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True): # Transaction/Block functions ############################# -def find_output(node, txid, amount): +def find_output(node, txid, amount, *, blockhash=None): """ Return index to output of txid with value amount Raises exception if there is none. """ - txdata = node.getrawtransaction(txid, 1) + txdata = node.getrawtransaction(txid, 1, blockhash) for i in range(len(txdata["vout"])): if txdata["vout"][i]["value"] == amount: return i diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 0c3c247694..b0b151d2d6 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -26,8 +26,7 @@ from test_framework.util import ( class AbandonConflictTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 - # TODO: remove -txindex. Currently required for getrawtransaction call. - self.extra_args = [["-minrelaytxfee=0.00001", "-txindex"], []] + self.extra_args = [["-minrelaytxfee=0.00001"], []] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index fe1a614700..a5df763eb0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -24,8 +24,6 @@ class WalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True - # TODO: remove -txindex. Currently required for getrawtransaction call. - self.extra_args = [[], [], ["-txindex"], []] def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index 1c2e0a9cb7..a0a35bf43d 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -4,13 +4,16 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet accounts properly when there are cloned transactions with malleated scriptsigs.""" +import io from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + bytes_to_hex_str as b2x, connect_nodes, disconnect_nodes, sync_blocks, ) +from test_framework.messages import CTransaction, COIN class TxnMallTest(BitcoinTestFramework): def set_test_params(self): @@ -72,19 +75,14 @@ class TxnMallTest(BitcoinTestFramework): clone_raw = self.nodes[0].createrawtransaction(clone_inputs, clone_outputs, clone_locktime) # createrawtransaction randomizes the order of its outputs, so swap them if necessary. - # output 0 is at version+#inputs+input+sigstub+sequence+#outputs - # 40 BTC serialized is 00286bee00000000 - pos0 = 2 * (4 + 1 + 36 + 1 + 4 + 1) - hex40 = "00286bee00000000" - output_len = 16 + 2 + 2 * int("0x" + clone_raw[pos0 + 16:pos0 + 16 + 2], 0) - if (rawtx1["vout"][0]["value"] == 40 and clone_raw[pos0:pos0 + 16] != hex40 or rawtx1["vout"][0]["value"] != 40 and clone_raw[pos0:pos0 + 16] == hex40): - output0 = clone_raw[pos0:pos0 + output_len] - output1 = clone_raw[pos0 + output_len:pos0 + 2 * output_len] - clone_raw = clone_raw[:pos0] + output1 + output0 + clone_raw[pos0 + 2 * output_len:] + clone_tx = CTransaction() + clone_tx.deserialize(io.BytesIO(bytes.fromhex(clone_raw))) + if (rawtx1["vout"][0]["value"] == 40 and clone_tx.vout[0].nValue != 40*COIN or rawtx1["vout"][0]["value"] != 40 and clone_tx.vout[0].nValue == 40*COIN): + (clone_tx.vout[0], clone_tx.vout[1]) = (clone_tx.vout[1], clone_tx.vout[0]) # Use a different signature hash type to sign. This creates an equivalent but malleated clone. # Don't send the clone anywhere yet - tx1_clone = self.nodes[0].signrawtransactionwithwallet(clone_raw, None, "ALL|ANYONECANPAY") + tx1_clone = self.nodes[0].signrawtransactionwithwallet(b2x(clone_tx.serialize()), None, "ALL|ANYONECANPAY") assert_equal(tx1_clone["complete"], True) # Have node0 mine a block, if requested: diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index 1534d5ef68..2b6c78c2c8 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -8,6 +8,7 @@ KNOWN_VIOLATIONS=( "src/dbwrapper.cpp:.*vsnprintf" "src/httprpc.cpp.*trim" "src/init.cpp:.*atoi" + "src/init.cpp:.*fprintf" "src/qt/rpcconsole.cpp:.*atoi" "src/rest.cpp:.*strtol" "src/test/dbwrapper_tests.cpp:.*snprintf" @@ -18,7 +19,6 @@ KNOWN_VIOLATIONS=( "src/util/strencodings.cpp:.*strtoul" "src/util/strencodings.h:.*atoi" "src/util/system.cpp:.*atoi" - "src/util/system.cpp:.*fprintf" ) REGEXP_IGNORE_EXTERNAL_DEPENDENCIES="^src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/)" |