diff options
| -rw-r--r-- | .travis.yml | 6 | ||||
| -rw-r--r-- | depends/Makefile | 1 | ||||
| -rw-r--r-- | depends/config.site.in | 11 | ||||
| -rw-r--r-- | doc/developer-notes.md | 6 | ||||
| -rw-r--r-- | src/compat.h | 2 | ||||
| -rw-r--r-- | src/net.cpp | 11 | ||||
| -rw-r--r-- | src/netbase.cpp | 12 | ||||
| -rw-r--r-- | src/netbase.h | 4 | ||||
| -rw-r--r-- | src/qt/rpcconsole.cpp | 9 | ||||
| -rw-r--r-- | src/qt/test/test_main.cpp | 6 | ||||
| -rw-r--r-- | src/rpc/client.cpp | 1 | ||||
| -rw-r--r-- | src/scheduler.cpp | 1 | ||||
| -rw-r--r-- | src/validation.cpp | 7 | ||||
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 98 | ||||
| -rw-r--r-- | src/wallet/test/crypto_tests.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 2 | ||||
| -rwxr-xr-x | test/functional/listsinceblock.py | 190 | ||||
| -rwxr-xr-x | test/functional/test_runner.py | 1 |
19 files changed, 305 insertions, 71 deletions
diff --git a/.travis.yml b/.travis.yml index a79428fc17..12f91096cc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,9 +29,9 @@ env: - HOST=i686-pc-linux-gnu PACKAGES="g++-multilib bc python3-zmq" DEP_OPTS="NO_QT=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports LDFLAGS=-static-libstdc++" USE_SHELL="/bin/dash" # Win64 - HOST=x86_64-w64-mingw32 DPKG_ADD_ARCH="i386" DEP_OPTS="NO_QT=1" PACKAGES="python3 nsis g++-mingw-w64-x86-64 wine1.6 bc" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-reduce-exports" -# bitcoind - - HOST=x86_64-unknown-linux-gnu PACKAGES="bc python3-zmq" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports CPPFLAGS=-DDEBUG_LOCKORDER" -# No wallet +# x86_64 Linux (uses qt5 dev package instead of depends Qt to speed up build and avoid timeout) + - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools protobuf-compiler libdbus-1-dev libharfbuzz-dev" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports CPPFLAGS=-DDEBUG_LOCKORDER" +# x86_64 Linux, No wallet - HOST=x86_64-unknown-linux-gnu PACKAGES="python3" DEP_OPTS="NO_WALLET=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports" # Cross-Mac - HOST=x86_64-apple-darwin11 PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python-dev" BITCOIN_CONFIG="--enable-gui --enable-reduce-exports" OSX_SDK=10.11 GOAL="deploy" diff --git a/depends/Makefile b/depends/Makefile index dedb0674cf..0ddd348e53 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -134,6 +134,7 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_ -e 's|@CXXFLAGS@|$(strip $(host_CXXFLAGS) $(host_$(release_type)_CXXFLAGS))|' \ -e 's|@CPPFLAGS@|$(strip $(host_CPPFLAGS) $(host_$(release_type)_CPPFLAGS))|' \ -e 's|@LDFLAGS@|$(strip $(host_LDFLAGS) $(host_$(release_type)_LDFLAGS))|' \ + -e 's|@allow_host_packages@|$(ALLOW_HOST_PACKAGES)|' \ -e 's|@no_qt@|$(NO_QT)|' \ -e 's|@no_wallet@|$(NO_WALLET)|' \ -e 's|@no_upnp@|$(NO_UPNP)|' \ diff --git a/depends/config.site.in b/depends/config.site.in index 3d7c9fd43c..0a4a9c327e 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -13,10 +13,10 @@ fi if test -z $with_qt_translationdir; then with_qt_translationdir=$depends_prefix/translations fi -if test -z $with_qt_bindir; then +if test -z $with_qt_bindir && test -z "@no_qt@"; then with_qt_bindir=$depends_prefix/native/bin fi -if test -z $with_protoc_bindir; then +if test -z $with_protoc_bindir && test -z "@no_qt@"; then with_protoc_bindir=$depends_prefix/native/bin fi @@ -53,9 +53,10 @@ PKG_CONFIG="`which pkg-config` --static" # These two need to remain exported because pkg-config does not see them # otherwise. That means they must be unexported at the end of configure.ac to # avoid ruining the cache. Sigh. - -export PKG_CONFIG_LIBDIR=$depends_prefix/lib/pkgconfig -export PKG_CONFIG_PATH=$depends_prefix/share/pkgconfig +export PKG_CONFIG_PATH=$depends_prefix/share/pkgconfig:$depends_prefix/lib/pkgconfig +if test -z "@allow_host_packages@"; then + export PKGCONFIG_LIBDIR= +fi CPPFLAGS="-I$depends_prefix/include/ $CPPFLAGS" LDFLAGS="-L$depends_prefix/lib $LDFLAGS" diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 81bdcc9fdb..458e7bcbff 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -28,12 +28,12 @@ tool to clean up patches automatically before submission. required when doing so would need changes to significant pieces of existing code. - Variable and namespace names are all lowercase, and may use `_` to - separate words. + separate words (snake_case). - Class member variables have a `m_` prefix. - Global variables have a `g_` prefix. - Constant names are all uppercase, and use `_` to separate words. - - Class names, function names and method names are CamelCase. Do not prefix - class names with `C`. + - Class names, function names and method names are UpperCamelCase + (PascalCase). Do not prefix class names with `C`. - **Miscellaneous** - `++i` is preferred over `i++`. diff --git a/src/compat.h b/src/compat.h index e76ab94c82..e022659c01 100644 --- a/src/compat.h +++ b/src/compat.h @@ -76,7 +76,7 @@ typedef unsigned int SOCKET; size_t strnlen( const char *start, size_t max_len); #endif // HAVE_DECL_STRNLEN -bool static inline IsSelectableSocket(SOCKET s) { +bool static inline IsSelectableSocket(const SOCKET& s) { #ifdef WIN32 return true; #else diff --git a/src/net.cpp b/src/net.cpp index b4b2de09f6..d4a36a0306 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2076,6 +2076,7 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b // Set to non-blocking, incoming connections will also inherit this if (!SetSocketNonBlocking(hListenSocket, true)) { + CloseSocket(hListenSocket); strError = strprintf("BindListenPort: Setting listening socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError())); LogPrintf("%s\n", strError); return false; @@ -2182,16 +2183,18 @@ void CConnman::SetNetworkActive(bool active) { LogPrint(BCLog::NET, "SetNetworkActive: %s\n", active); - if (!active) { - fNetworkActive = false; + if (fNetworkActive == active) { + return; + } + + fNetworkActive = active; + if (!fNetworkActive) { LOCK(cs_vNodes); // Close sockets to all nodes for (CNode* pnode : vNodes) { pnode->CloseSocketDisconnect(); } - } else { - fNetworkActive = true; } uiInterface.NotifyNetworkActiveChanged(fNetworkActive); diff --git a/src/netbase.cpp b/src/netbase.cpp index 1f668a5d4c..8952468ecd 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -203,7 +203,7 @@ enum class IntrRecvError { * * @note This function requires that hSocket is in non-blocking mode. */ -static IntrRecvError InterruptibleRecv(char* data, size_t len, int timeout, SOCKET& hSocket) +static IntrRecvError InterruptibleRecv(char* data, size_t len, int timeout, const SOCKET& hSocket) { int64_t curTime = GetTimeMillis(); int64_t endTime = curTime + timeout; @@ -424,8 +424,10 @@ bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRe SetSocketNoDelay(hSocket); // Set to non-blocking - if (!SetSocketNonBlocking(hSocket, true)) + if (!SetSocketNonBlocking(hSocket, true)) { + CloseSocket(hSocket); return error("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError())); + } if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR) { @@ -682,7 +684,7 @@ bool CloseSocket(SOCKET& hSocket) return ret != SOCKET_ERROR; } -bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking) +bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking) { if (fNonBlocking) { #ifdef WIN32 @@ -692,7 +694,6 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking) int fFlags = fcntl(hSocket, F_GETFL, 0); if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) { #endif - CloseSocket(hSocket); return false; } } else { @@ -703,7 +704,6 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking) int fFlags = fcntl(hSocket, F_GETFL, 0); if (fcntl(hSocket, F_SETFL, fFlags & ~O_NONBLOCK) == SOCKET_ERROR) { #endif - CloseSocket(hSocket); return false; } } @@ -711,7 +711,7 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking) return true; } -bool SetSocketNoDelay(SOCKET& hSocket) +bool SetSocketNoDelay(const SOCKET& hSocket) { int set = 1; int rc = setsockopt(hSocket, IPPROTO_TCP, TCP_NODELAY, (const char*)&set, sizeof(int)); diff --git a/src/netbase.h b/src/netbase.h index fd4b34c8f1..941da31f9c 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -57,9 +57,9 @@ std::string NetworkErrorString(int err); /** Close socket and set hSocket to INVALID_SOCKET */ bool CloseSocket(SOCKET& hSocket); /** Disable or enable blocking-mode for a socket */ -bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking); +bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking); /** Set the TCP_NODELAY flag on a socket */ -bool SetSocketNoDelay(SOCKET& hSocket); +bool SetSocketNoDelay(const SOCKET& hSocket); /** * Convert milliseconds to a struct timeval for e.g. select. */ diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index ec0580b81c..232068bf45 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -25,6 +25,7 @@ #ifdef ENABLE_WALLET #include <db_cxx.h> +#include <wallet/wallet.h> #endif #include <QKeyEvent> @@ -301,6 +302,14 @@ bool RPCConsole::RPCParseCommandLine(std::string &strResult, const std::string & JSONRPCRequest req; req.params = RPCConvertValues(stack.back()[0], std::vector<std::string>(stack.back().begin() + 1, stack.back().end())); req.strMethod = stack.back()[0]; +#ifdef ENABLE_WALLET + // TODO: Move this logic to WalletModel + if (!vpwallets.empty()) { + // in Qt, use always the wallet with index 0 when running with multiple wallets + QByteArray encodedName = QUrl::toPercentEncoding(QString::fromStdString(vpwallets[0]->GetName())); + req.URI = "/wallet/"+std::string(encodedName.constData(), encodedName.length()); + } +#endif lastResult = tableRPC.execute(req); } diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index cae18f41a5..1b28a285f1 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -59,7 +59,11 @@ int main(int argc, char *argv[]) // Prefer the "minimal" platform for the test instead of the normal default // platform ("xcb", "windows", or "cocoa") so tests can't unintentially // interfere with any background GUIs and don't require extra resources. - setenv("QT_QPA_PLATFORM", "minimal", 0); + #if defined(WIN32) + _putenv_s("QT_QPA_PLATFORM", "minimal"); + #else + setenv("QT_QPA_PLATFORM", "minimal", 0); + #endif // Don't remove this, it's needed to access // QApplication:: and QCoreApplication:: in the tests diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 7c75586d03..4179453782 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -68,6 +68,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getblocktemplate", 0, "template_request" }, { "listsinceblock", 1, "target_confirmations" }, { "listsinceblock", 2, "include_watchonly" }, + { "listsinceblock", 3, "include_removed" }, { "sendmany", 1, "amounts" }, { "sendmany", 2, "minconf" }, { "sendmany", 4, "subtractfeefrom" }, diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 36a6d5110d..1d3fb1f6ea 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -141,6 +141,7 @@ size_t CScheduler::getQueueInfo(boost::chrono::system_clock::time_point &first, } bool CScheduler::AreThreadsServicingQueue() const { + boost::unique_lock<boost::mutex> lock(newTaskMutex); return nThreadsServicingQueue; } diff --git a/src/validation.cpp b/src/validation.cpp index 9a00203e8f..d223715c46 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -20,6 +20,7 @@ #include "init.h" #include "policy/fees.h" #include "policy/policy.h" +#include "policy/rbf.h" #include "pow.h" #include "primitives/block.h" #include "primitives/transaction.h" @@ -488,9 +489,9 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (!setConflicts.count(ptxConflicting->GetHash())) { // Allow opt-out of transaction replacement by setting - // nSequence >= maxint-1 on all inputs. + // nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. // - // maxint-1 is picked to still allow use of nLockTime by + // SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by // non-replaceable transactions. All inputs rather than just one // is for the sake of multi-party protocols, where we don't // want a single party to be able to disable replacement. @@ -504,7 +505,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool { for (const CTxIn &_txin : ptxConflicting->vin) { - if (_txin.nSequence < std::numeric_limits<unsigned int>::max()-1) + if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE) { fReplacementOptOut = false; break; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2b7b4085ee..e956adaf89 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -622,7 +622,7 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) + HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\"") + "\nThe amount including unconfirmed transactions, zero confirmations\n" + HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\" 0") + - "\nThe amount with at least 6 confirmation, very safe\n" + "\nThe amount with at least 6 confirmations\n" + HelpExampleCli("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\" 6") + "\nAs a json rpc call\n" + HelpExampleRpc("getreceivedbyaddress", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\", 6") @@ -682,7 +682,7 @@ UniValue getreceivedbyaccount(const JSONRPCRequest& request) + HelpExampleCli("getreceivedbyaccount", "\"\"") + "\nAmount received at the tabby account including unconfirmed amounts with zero confirmations\n" + HelpExampleCli("getreceivedbyaccount", "\"tabby\" 0") + - "\nThe amount with at least 6 confirmation, very safe\n" + "\nThe amount with at least 6 confirmations\n" + HelpExampleCli("getreceivedbyaccount", "\"tabby\" 6") + "\nAs a json rpc call\n" + HelpExampleRpc("getreceivedbyaccount", "\"tabby\", 6") @@ -1426,6 +1426,17 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) entry.push_back(Pair("address", addr.ToString())); } +/** + * List transactions based on the given criteria. + * + * @param pwallet The wallet. + * @param wtx The wallet transaction. + * @param strAccount The account, if any, or "*" for all. + * @param nMinDepth The minimum confirmation depth. + * @param fLong Whether to include the JSON version of the transaction. + * @param ret The UniValue into which the result is stored. + * @param filter The "is mine" filter bool. + */ void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) { CAmount nFee; @@ -1742,14 +1753,18 @@ UniValue listsinceblock(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() > 3) + if (request.fHelp || request.params.size() > 4) throw std::runtime_error( - "listsinceblock ( \"blockhash\" target_confirmations include_watchonly)\n" - "\nGet all transactions in blocks since block [blockhash], or all transactions if omitted\n" + "listsinceblock ( \"blockhash\" target_confirmations include_watchonly include_removed )\n" + "\nGet all transactions in blocks since block [blockhash], or all transactions if omitted.\n" + "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n" + "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n" "\nArguments:\n" "1. \"blockhash\" (string, optional) The block hash to list transactions since\n" - "2. target_confirmations: (numeric, optional) The confirmations required, must be 1 or more\n" - "3. include_watchonly: (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')" + "2. target_confirmations: (numeric, optional, default=1) Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value\n" + "3. include_watchonly: (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')\n" + "4. include_removed: (bool, optional, default=true) Show transactions that were removed due to a reorg in the \"removed\" array\n" + " (not guaranteed to work on pruned nodes)\n" "\nResult:\n" "{\n" " \"transactions\": [\n" @@ -1774,8 +1789,12 @@ UniValue listsinceblock(const JSONRPCRequest& request) " \"comment\": \"...\", (string) If a comment is associated with the transaction.\n" " \"label\" : \"label\" (string) A comment for the address/transaction, if any\n" " \"to\": \"...\", (string) If a comment to is associated with the transaction.\n" - " ],\n" - " \"lastblock\": \"lastblockhash\" (string) The hash of the last block\n" + " ],\n" + " \"removed\": [\n" + " <structure is the same as \"transactions\" above, only present if include_removed=true>\n" + " Note: transactions that were readded in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n" + " ],\n" + " \"lastblock\": \"lastblockhash\" (string) The hash of the block (target_confirmations-1) from the best block on the main chain. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones\n" "}\n" "\nExamples:\n" + HelpExampleCli("listsinceblock", "") @@ -1785,21 +1804,19 @@ UniValue listsinceblock(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - const CBlockIndex *pindex = NULL; + const CBlockIndex* pindex = NULL; // Block index of the specified block or the common ancestor, if the block provided was in a deactivated chain. + const CBlockIndex* paltindex = NULL; // Block index of the specified block, even if it's in a deactivated chain. int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; - if (!request.params[0].isNull()) - { + if (!request.params[0].isNull()) { uint256 blockId; blockId.SetHex(request.params[0].get_str()); BlockMap::iterator it = mapBlockIndex.find(blockId); - if (it != mapBlockIndex.end()) - { - pindex = it->second; - if (chainActive[pindex->nHeight] != pindex) - { + if (it != mapBlockIndex.end()) { + paltindex = pindex = it->second; + if (chainActive[pindex->nHeight] != pindex) { // the block being asked for is a part of a deactivated chain; // we don't want to depend on its perceived height in the block // chain, we want to instead use the last common ancestor @@ -1808,19 +1825,20 @@ UniValue listsinceblock(const JSONRPCRequest& request) } } - if (!request.params[1].isNull()) - { + if (!request.params[1].isNull()) { target_confirms = request.params[1].get_int(); - if (target_confirms < 1) + if (target_confirms < 1) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter"); + } } - if (request.params.size() > 2 && request.params[2].get_bool()) - { + if (!request.params[2].isNull() && request.params[2].get_bool()) { filter = filter | ISMINE_WATCH_ONLY; } + bool include_removed = (request.params[3].isNull() || request.params[3].get_bool()); + int depth = pindex ? (1 + chainActive.Height() - pindex->nHeight) : -1; UniValue transactions(UniValue::VARR); @@ -1828,8 +1846,27 @@ UniValue listsinceblock(const JSONRPCRequest& request) for (const std::pair<uint256, CWalletTx>& pairWtx : pwallet->mapWallet) { CWalletTx tx = pairWtx.second; - if (depth == -1 || tx.GetDepthInMainChain() < depth) + if (depth == -1 || tx.GetDepthInMainChain() < depth) { ListTransactions(pwallet, tx, "*", 0, true, transactions, filter); + } + } + + // when a reorg'd block is requested, we also list any relevant transactions + // in the blocks of the chain that was detached + UniValue removed(UniValue::VARR); + while (include_removed && paltindex && paltindex != pindex) { + CBlock block; + if (!ReadBlockFromDisk(block, paltindex, Params().GetConsensus())) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); + } + for (const CTransactionRef& tx : block.vtx) { + if (pwallet->mapWallet.count(tx->GetHash()) > 0) { + // We want all transactions regardless of confirmation count to appear here, + // even negative confirmation ones, hence the big negative. + ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter); + } + } + paltindex = paltindex->pprev; } CBlockIndex *pblockLast = chainActive[chainActive.Height() + 1 - target_confirms]; @@ -1837,6 +1874,7 @@ UniValue listsinceblock(const JSONRPCRequest& request) UniValue ret(UniValue::VOBJ); ret.push_back(Pair("transactions", transactions)); + if (include_removed) ret.push_back(Pair("removed", removed)); ret.push_back(Pair("lastblock", lastblock.GetHex())); return ret; @@ -2065,7 +2103,7 @@ UniValue walletpassphrase(const JSONRPCRequest& request) "Issuing the walletpassphrase command while the wallet is already unlocked will set a new unlock\n" "time that overrides the old one.\n" "\nExamples:\n" - "\nunlock the wallet for 60 seconds\n" + "\nUnlock the wallet for 60 seconds\n" + HelpExampleCli("walletpassphrase", "\"my pass phrase\" 60") + "\nLock the wallet again (before 60 seconds)\n" + HelpExampleCli("walletlock", "") + @@ -2220,11 +2258,11 @@ UniValue encryptwallet(const JSONRPCRequest& request) "\nArguments:\n" "1. \"passphrase\" (string) The pass phrase to encrypt the wallet with. It must be at least 1 character, but should be long.\n" "\nExamples:\n" - "\nEncrypt you wallet\n" + "\nEncrypt your wallet\n" + HelpExampleCli("encryptwallet", "\"my pass phrase\"") + "\nNow set the passphrase to use the wallet, such as for signing or sending bitcoin\n" + HelpExampleCli("walletpassphrase", "\"my pass phrase\"") + - "\nNow we can so something like sign\n" + "\nNow we can do something like sign\n" + HelpExampleCli("signmessage", "\"address\" \"test message\"") + "\nNow lock the wallet again by removing the passphrase\n" + HelpExampleCli("walletlock", "") + @@ -2837,9 +2875,15 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) coinControl.signalRbf = options["replaceable"].get_bool(); } if (options.exists("conf_target")) { + if (options.exists("feeRate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate"); + } coinControl.m_confirm_target = ParseConfirmTarget(options["conf_target"]); } if (options.exists("estimate_mode")) { + if (options.exists("feeRate")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and feeRate"); + } if (!FeeModeFromString(options["estimate_mode"].get_str(), coinControl.m_fee_mode)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); } @@ -3117,7 +3161,7 @@ static const CRPCCommand commands[] = { "wallet", "listlockunspent", &listlockunspent, false, {} }, { "wallet", "listreceivedbyaccount", &listreceivedbyaccount, false, {"minconf","include_empty","include_watchonly"} }, { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, false, {"minconf","include_empty","include_watchonly"} }, - { "wallet", "listsinceblock", &listsinceblock, false, {"blockhash","target_confirmations","include_watchonly"} }, + { "wallet", "listsinceblock", &listsinceblock, false, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, { "wallet", "listtransactions", &listtransactions, false, {"account","count","skip","include_watchonly"} }, { "wallet", "listunspent", &listunspent, false, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, { "wallet", "listwallets", &listwallets, true, {} }, diff --git a/src/wallet/test/crypto_tests.cpp b/src/wallet/test/crypto_tests.cpp index 524a72c303..744063624c 100644 --- a/src/wallet/test/crypto_tests.cpp +++ b/src/wallet/test/crypto_tests.cpp @@ -26,8 +26,8 @@ bool OldSetKeyFromPassphrase(const SecureString& strKeyData, const std::vector<u if (i != (int)WALLET_CRYPTO_KEY_SIZE) { - memory_cleanse(chKey, sizeof(chKey)); - memory_cleanse(chIV, sizeof(chIV)); + memory_cleanse(chKey, WALLET_CRYPTO_KEY_SIZE); + memory_cleanse(chIV, WALLET_CRYPTO_IV_SIZE); return false; } return true; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 8176a0017c..4a2cc9a139 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -469,7 +469,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) JSONRPCRequest request; request.params.setArray(); - request.params.push_back("wallet.backup"); + request.params.push_back((pathTemp / "wallet.backup").string()); vpwallets.insert(vpwallets.begin(), &wallet); ::dumpwallet(request); } @@ -481,7 +481,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) JSONRPCRequest request; request.params.setArray(); - request.params.push_back("wallet.backup"); + request.params.push_back((pathTemp / "wallet.backup").string()); vpwallets[0] = &wallet; ::importwallet(request); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 223790aa40..3e9c531f00 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2739,7 +2739,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // to avoid conflicting with other possible uses of nSequence, // and in the spirit of "smallest possible change from prior // behavior." - const uint32_t nSequence = coin_control.signalRbf ? MAX_BIP125_RBF_SEQUENCE : (std::numeric_limits<unsigned int>::max() - 1); + const uint32_t nSequence = coin_control.signalRbf ? MAX_BIP125_RBF_SEQUENCE : (CTxIn::SEQUENCE_FINAL - 1); for (const auto& coin : setCoins) txNew.vin.push_back(CTxIn(coin.outpoint,CScript(), nSequence)); diff --git a/test/functional/listsinceblock.py b/test/functional/listsinceblock.py index f3d41e573e..ce2d556ef0 100755 --- a/test/functional/listsinceblock.py +++ b/test/functional/listsinceblock.py @@ -14,7 +14,15 @@ class ListSinceBlockTest (BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 4 - def run_test (self): + def run_test(self): + self.nodes[2].generate(101) + self.sync_all() + + self.test_reorg() + self.test_double_spend() + self.test_double_send() + + def test_reorg(self): ''' `listsinceblock` did not behave correctly when handed a block that was no longer in the main chain: @@ -43,14 +51,6 @@ class ListSinceBlockTest (BitcoinTestFramework): This test only checks that [tx0] is present. ''' - self.nodes[2].generate(101) - self.sync_all() - - assert_equal(self.nodes[0].getbalance(), 0) - assert_equal(self.nodes[1].getbalance(), 0) - assert_equal(self.nodes[2].getbalance(), 50) - assert_equal(self.nodes[3].getbalance(), 0) - # Split network into two self.split_network() @@ -73,7 +73,177 @@ class ListSinceBlockTest (BitcoinTestFramework): if tx['txid'] == senttx: found = True break - assert_equal(found, True) + assert found + + def test_double_spend(self): + ''' + This tests the case where the same UTXO is spent twice on two separate + blocks as part of a reorg. + + ab0 + / \ + aa1 [tx1] bb1 [tx2] + | | + aa2 bb2 + | | + aa3 bb3 + | + bb4 + + Problematic case: + + 1. User 1 receives BTC in tx1 from utxo1 in block aa1. + 2. User 2 receives BTC in tx2 from utxo1 (same) in block bb1 + 3. User 1 sees 2 confirmations at block aa3. + 4. Reorg into bb chain. + 5. User 1 asks `listsinceblock aa3` and does not see that tx1 is now + invalidated. + + Currently the solution to this is to detect that a reorg'd block is + asked for in listsinceblock, and to iterate back over existing blocks up + until the fork point, and to include all transactions that relate to the + node wallet. + ''' + + self.sync_all() + + # Split network into two + self.split_network() + + # share utxo between nodes[1] and nodes[2] + utxos = self.nodes[2].listunspent() + utxo = utxos[0] + privkey = self.nodes[2].dumpprivkey(utxo['address']) + self.nodes[1].importprivkey(privkey) + + # send from nodes[1] using utxo to nodes[0] + change = '%.8f' % (float(utxo['amount']) - 1.0003) + recipientDict = { + self.nodes[0].getnewaddress(): 1, + self.nodes[1].getnewaddress(): change, + } + utxoDicts = [{ + 'txid': utxo['txid'], + 'vout': utxo['vout'], + }] + txid1 = self.nodes[1].sendrawtransaction( + self.nodes[1].signrawtransaction( + self.nodes[1].createrawtransaction(utxoDicts, recipientDict))['hex']) + + # send from nodes[2] using utxo to nodes[3] + recipientDict2 = { + self.nodes[3].getnewaddress(): 1, + self.nodes[2].getnewaddress(): change, + } + self.nodes[2].sendrawtransaction( + self.nodes[2].signrawtransaction( + self.nodes[2].createrawtransaction(utxoDicts, recipientDict2))['hex']) + + # generate on both sides + lastblockhash = self.nodes[1].generate(3)[2] + self.nodes[2].generate(4) + + self.join_network() + + self.sync_all() + + # gettransaction should work for txid1 + assert self.nodes[0].gettransaction(txid1)['txid'] == txid1, "gettransaction failed to find txid1" + + # listsinceblock(lastblockhash) should now include txid1, as seen from nodes[0] + lsbres = self.nodes[0].listsinceblock(lastblockhash) + assert any(tx['txid'] == txid1 for tx in lsbres['removed']) + + # but it should not include 'removed' if include_removed=false + lsbres2 = self.nodes[0].listsinceblock(blockhash=lastblockhash, include_removed=False) + assert 'removed' not in lsbres2 + + def test_double_send(self): + ''' + This tests the case where the same transaction is submitted twice on two + separate blocks as part of a reorg. The former will vanish and the + latter will appear as the true transaction (with confirmations dropping + as a result). + + ab0 + / \ + aa1 [tx1] bb1 + | | + aa2 bb2 + | | + aa3 bb3 [tx1] + | + bb4 + + Asserted: + + 1. tx1 is listed in listsinceblock. + 2. It is included in 'removed' as it was removed, even though it is now + present in a different block. + 3. It is listed with a confirmations count of 2 (bb3, bb4), not + 3 (aa1, aa2, aa3). + ''' + + self.sync_all() + + # Split network into two + self.split_network() + + # create and sign a transaction + utxos = self.nodes[2].listunspent() + utxo = utxos[0] + change = '%.8f' % (float(utxo['amount']) - 1.0003) + recipientDict = { + self.nodes[0].getnewaddress(): 1, + self.nodes[2].getnewaddress(): change, + } + utxoDicts = [{ + 'txid': utxo['txid'], + 'vout': utxo['vout'], + }] + signedtxres = self.nodes[2].signrawtransaction( + self.nodes[2].createrawtransaction(utxoDicts, recipientDict)) + assert signedtxres['complete'] + + signedtx = signedtxres['hex'] + + # send from nodes[1]; this will end up in aa1 + txid1 = self.nodes[1].sendrawtransaction(signedtx) + + # generate bb1-bb2 on right side + self.nodes[2].generate(2) + + # send from nodes[2]; this will end up in bb3 + txid2 = self.nodes[2].sendrawtransaction(signedtx) + + assert_equal(txid1, txid2) + + # generate on both sides + lastblockhash = self.nodes[1].generate(3)[2] + self.nodes[2].generate(2) + + self.join_network() + + self.sync_all() + + # gettransaction should work for txid1 + self.nodes[0].gettransaction(txid1) + + # listsinceblock(lastblockhash) should now include txid1 in transactions + # as well as in removed + lsbres = self.nodes[0].listsinceblock(lastblockhash) + assert any(tx['txid'] == txid1 for tx in lsbres['transactions']) + assert any(tx['txid'] == txid1 for tx in lsbres['removed']) + + # find transaction and ensure confirmations is valid + for tx in lsbres['transactions']: + if tx['txid'] == txid1: + assert_equal(tx['confirmations'], 2) + + # the same check for the removed array; confirmations should STILL be 2 + for tx in lsbres['removed']: + if tx['txid'] == txid1: + assert_equal(tx['confirmations'], 2) if __name__ == '__main__': ListSinceBlockTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c158eaa34e..a043560ea8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -63,7 +63,6 @@ BASE_SCRIPTS= [ 'segwit.py', # vv Tests less than 2m vv 'wallet.py', - 'multiwallet.py', 'wallet-accounts.py', 'p2p-segwit.py', 'wallet-dump.py', |