From 0cc07f82f0d4043e93d8cea49356cd2bc5ffcb27 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Wed, 7 Dec 2016 15:20:40 +0100 Subject: [PATCH 1/7] [QA] add fundrawtransaction test on a locked wallet with empty keypool Github-Pull: #9295 Rebased-From: 1a6eacbf3b7e3d5941fec1154079bbc4678ce861 --- qa/rpc-tests/fundrawtransaction.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py index 8c45578fc..0dcca4cb5 100755 --- a/qa/rpc-tests/fundrawtransaction.py +++ b/qa/rpc-tests/fundrawtransaction.py @@ -484,6 +484,23 @@ class RawTransactionsTest(BitcoinTestFramework): self.is_network_split=False self.sync_all() + # drain the keypool + self.nodes[1].getnewaddress() + inputs = [] + outputs = {self.nodes[0].getnewaddress():1.1} + rawTx = self.nodes[1].createrawtransaction(inputs, outputs) + # fund a transaction that requires a new key for the change output + # creating the key must be impossible because the wallet is locked + try: + fundedTx = self.nodes[1].fundrawtransaction(rawTx) + raise AssertionError("Wallet unlocked without passphrase") + except JSONRPCException as e: + assert('Keypool ran out' in e.error['message']) + + #refill the keypool + self.nodes[1].walletpassphrase("test", 100) + self.nodes[1].walletlock() + try: self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.2) raise AssertionError("Wallet unlocked without passphrase") From 43bcfca48970b65d4eb215f20047bb99495f3727 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 6 Dec 2016 13:45:56 +0100 Subject: [PATCH 2/7] [Wallet] Bugfix: FRT: don't terminate when keypool is empty Github-Pull: #9295 Rebased-From: c24a4f5981d47d55aa9e4eb40294832a4d38fb80 --- src/wallet/wallet.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6ce8d19bf..5b35e49a0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2290,7 +2290,11 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt CPubKey vchPubKey; bool ret; ret = reservekey.GetReservedKey(vchPubKey); - assert(ret); // should never fail, as we just unlocked + if (!ret) + { + strFailReason = _("Keypool ran out, please call keypoolrefill first"); + return false; + } scriptChange = GetScriptForDestination(vchPubKey.GetID()); } From a0f7ececfd096df398fc5e4aad07f1a43760afb4 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 12 Dec 2016 08:10:27 +0000 Subject: [PATCH 3/7] Update for OpenSSL 1.1 API Github-Pull: #9326 Rebased-From: bae1eef752dcecfd85fa482881e1dbe4d7e9f74c b05b1af10b9a5298bd90bea439f0fd6c636e0cfa --- src/qt/paymentrequestplus.cpp | 20 +++++++++++++++----- src/wallet/test/crypto_tests.cpp | 32 ++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/qt/paymentrequestplus.cpp b/src/qt/paymentrequestplus.cpp index 20e1f79ff..82be4d831 100644 --- a/src/qt/paymentrequestplus.cpp +++ b/src/qt/paymentrequestplus.cpp @@ -159,14 +159,24 @@ bool PaymentRequestPlus::getMerchant(X509_STORE* certStore, QString& merchant) c std::string data_to_verify; // Everything but the signature rcopy.SerializeToString(&data_to_verify); - EVP_MD_CTX ctx; +#if OPENSSL_VERSION_NUMBER >= 0x10100000L + EVP_MD_CTX *ctx = EVP_MD_CTX_new(); + if (!ctx) throw SSLVerifyError("Error allocating OpenSSL context."); +#else + EVP_MD_CTX _ctx; + EVP_MD_CTX *ctx; + ctx = &_ctx; +#endif EVP_PKEY *pubkey = X509_get_pubkey(signing_cert); - EVP_MD_CTX_init(&ctx); - if (!EVP_VerifyInit_ex(&ctx, digestAlgorithm, NULL) || - !EVP_VerifyUpdate(&ctx, data_to_verify.data(), data_to_verify.size()) || - !EVP_VerifyFinal(&ctx, (const unsigned char*)paymentRequest.signature().data(), (unsigned int)paymentRequest.signature().size(), pubkey)) { + EVP_MD_CTX_init(ctx); + if (!EVP_VerifyInit_ex(ctx, digestAlgorithm, NULL) || + !EVP_VerifyUpdate(ctx, data_to_verify.data(), data_to_verify.size()) || + !EVP_VerifyFinal(ctx, (const unsigned char*)paymentRequest.signature().data(), (unsigned int)paymentRequest.signature().size(), pubkey)) { throw SSLVerifyError("Bad signature, invalid payment request."); } +#if OPENSSL_VERSION_NUMBER >= 0x10100000L + EVP_MD_CTX_free(ctx); +#endif // OpenSSL API for getting human printable strings from certs is baroque. int textlen = X509_NAME_get_text_by_NID(certname, NID_commonName, NULL, 0); diff --git a/src/wallet/test/crypto_tests.cpp b/src/wallet/test/crypto_tests.cpp index 05387f5f2..b235b9609 100644 --- a/src/wallet/test/crypto_tests.cpp +++ b/src/wallet/test/crypto_tests.cpp @@ -42,15 +42,19 @@ bool OldEncrypt(const CKeyingMaterial& vchPlaintext, std::vector int nCLen = nLen + AES_BLOCK_SIZE, nFLen = 0; vchCiphertext = std::vector (nCLen); - EVP_CIPHER_CTX ctx; + EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); + + if (!ctx) return false; bool fOk = true; - EVP_CIPHER_CTX_init(&ctx); - if (fOk) fOk = EVP_EncryptInit_ex(&ctx, EVP_aes_256_cbc(), NULL, chKey, chIV) != 0; - if (fOk) fOk = EVP_EncryptUpdate(&ctx, &vchCiphertext[0], &nCLen, &vchPlaintext[0], nLen) != 0; - if (fOk) fOk = EVP_EncryptFinal_ex(&ctx, (&vchCiphertext[0]) + nCLen, &nFLen) != 0; - EVP_CIPHER_CTX_cleanup(&ctx); + EVP_CIPHER_CTX_init(ctx); + if (fOk) fOk = EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, chKey, chIV) != 0; + if (fOk) fOk = EVP_EncryptUpdate(ctx, &vchCiphertext[0], &nCLen, &vchPlaintext[0], nLen) != 0; + if (fOk) fOk = EVP_EncryptFinal_ex(ctx, (&vchCiphertext[0]) + nCLen, &nFLen) != 0; + EVP_CIPHER_CTX_cleanup(ctx); + + EVP_CIPHER_CTX_free(ctx); if (!fOk) return false; @@ -66,15 +70,19 @@ bool OldDecrypt(const std::vector& vchCiphertext, CKeyingMaterial vchPlaintext = CKeyingMaterial(nPLen); - EVP_CIPHER_CTX ctx; + EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new(); + + if (!ctx) return false; bool fOk = true; - EVP_CIPHER_CTX_init(&ctx); - if (fOk) fOk = EVP_DecryptInit_ex(&ctx, EVP_aes_256_cbc(), NULL, chKey, chIV) != 0; - if (fOk) fOk = EVP_DecryptUpdate(&ctx, &vchPlaintext[0], &nPLen, &vchCiphertext[0], nLen) != 0; - if (fOk) fOk = EVP_DecryptFinal_ex(&ctx, (&vchPlaintext[0]) + nPLen, &nFLen) != 0; - EVP_CIPHER_CTX_cleanup(&ctx); + EVP_CIPHER_CTX_init(ctx); + if (fOk) fOk = EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, chKey, chIV) != 0; + if (fOk) fOk = EVP_DecryptUpdate(ctx, &vchPlaintext[0], &nPLen, &vchCiphertext[0], nLen) != 0; + if (fOk) fOk = EVP_DecryptFinal_ex(ctx, (&vchPlaintext[0]) + nPLen, &nFLen) != 0; + EVP_CIPHER_CTX_cleanup(ctx); + + EVP_CIPHER_CTX_free(ctx); if (!fOk) return false; From 35174a0280845e9583a3af1fcf57ee1f98238539 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Tue, 6 Dec 2016 06:39:14 +0000 Subject: [PATCH 4/7] Make RelayWalletTransaction attempt to AcceptToMemoryPool. This resolves an issue where a wallet transaction which failed to relay previously because it couldn't make it into the mempool will not try again until restart, even though mempool conditions may have changed. Abandoned and known-conflicted transactions are skipped. Some concern was expressed that there may be users with many unknown conflicts would waste a lot of CPU time trying to add them to their memory pools over and over again. But I am doubtful these users exist in any number, if they do exist they have worse problems, and they can mitigate any performance issue this might have by abandoning the transactions in question. Github-Pull: #9290 Rebased-From: f692fce8a49e05e25f1c767aae1e50db419caebe --- src/wallet/wallet.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5b35e49a0..0403b3558 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1449,9 +1449,10 @@ void CWallet::ReacceptWalletTransactions() bool CWalletTx::RelayWalletTransaction() { assert(pwallet->GetBroadcastTransactions()); - if (!IsCoinBase()) + if (!IsCoinBase() && !isAbandoned() && GetDepthInMainChain() == 0) { - if (GetDepthInMainChain() == 0 && !isAbandoned() && InMempool()) { + /* GetDepthInMainChain already catches known conflicts. */ + if (InMempool() || AcceptToMemoryPool(false, maxTxFee)) { LogPrintf("Relaying wtx %s\n", GetHash().ToString()); RelayTransaction((CTransaction)*this); return true; From f5d606e5ab0b20881cc0d58d63e6db065c59b411 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 8 Dec 2016 11:49:28 -0800 Subject: [PATCH 5/7] Return txid even if ATMP fails for new transaction Github-Pull: #9302 Rebased-From: b3a74100b86423c553ac327f3ea6fdbc2c50890a --- src/wallet/wallet.cpp | 21 +++++++++++---------- src/wallet/wallet.h | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0403b3558..7f6240262 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1442,7 +1442,8 @@ void CWallet::ReacceptWalletTransactions() CWalletTx& wtx = *(item.second); LOCK(mempool.cs); - wtx.AcceptToMemoryPool(false, maxTxFee); + CValidationState state; + wtx.AcceptToMemoryPool(false, maxTxFee, state); } } @@ -1451,8 +1452,9 @@ bool CWalletTx::RelayWalletTransaction() assert(pwallet->GetBroadcastTransactions()); if (!IsCoinBase() && !isAbandoned() && GetDepthInMainChain() == 0) { + CValidationState state; /* GetDepthInMainChain already catches known conflicts. */ - if (InMempool() || AcceptToMemoryPool(false, maxTxFee)) { + if (InMempool() || AcceptToMemoryPool(false, maxTxFee, state)) { LogPrintf("Relaying wtx %s\n", GetHash().ToString()); RelayTransaction((CTransaction)*this); return true; @@ -2482,14 +2484,14 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) if (fBroadcastTransactions) { + CValidationState state; // Broadcast - if (!wtxNew.AcceptToMemoryPool(false, maxTxFee)) - { - // This must not fail. The transaction has already been signed and recorded. - LogPrintf("CommitTransaction(): Error: Transaction not valid\n"); - return false; + if (!wtxNew.AcceptToMemoryPool(false, maxTxFee, state)) { + LogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", state.GetRejectReason()); + // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. + } else { + wtxNew.RelayWalletTransaction(); } - wtxNew.RelayWalletTransaction(); } } return true; @@ -3595,8 +3597,7 @@ int CMerkleTx::GetBlocksToMaturity() const } -bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, CAmount nAbsurdFee) +bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, CAmount nAbsurdFee, CValidationState& state) { - CValidationState state; return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, false, nAbsurdFee); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0c95fdf4b..056dcc5da 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -213,7 +213,7 @@ public: bool IsInMainChain() const { const CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet) > 0; } int GetBlocksToMaturity() const; /** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */ - bool AcceptToMemoryPool(bool fLimitFree, const CAmount nAbsurdFee); + bool AcceptToMemoryPool(bool fLimitFree, const CAmount nAbsurdFee, CValidationState& state); bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } void setAbandoned() { hashBlock = ABANDON_HASH; } From c365556185d89122a04344d2dc640cffc67f155c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 5 Dec 2016 23:26:32 -0800 Subject: [PATCH 6/7] Complain when unknown rpcserialversion is specified Github-Pull: #9292 Rebased-From: 80d073c9bca4521d512ea14ff7919d4609647b6d --- src/init.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 8e40c4388..f2dbf5751 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -364,7 +364,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-port=", strprintf(_("Listen for connections on (default: %u or testnet: %u)"), Params(CBaseChainParams::MAIN).GetDefaultPort(), Params(CBaseChainParams::TESTNET).GetDefaultPort())); strUsage += HelpMessageOpt("-proxy=", _("Connect through SOCKS5 proxy")); strUsage += HelpMessageOpt("-proxyrandomize", strprintf(_("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)"), DEFAULT_PROXYRANDOMIZE)); - strUsage += HelpMessageOpt("-rpcserialversion", strprintf(_("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) or segwit(>0) (default: %d)"), DEFAULT_RPC_SERIALIZE_VERSION)); + strUsage += HelpMessageOpt("-rpcserialversion", strprintf(_("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) or segwit(1) (default: %d)"), DEFAULT_RPC_SERIALIZE_VERSION)); strUsage += HelpMessageOpt("-seednode=", _("Connect to a node to retrieve peer addresses, and disconnect")); strUsage += HelpMessageOpt("-timeout=", strprintf(_("Specify connection timeout in milliseconds (minimum: 1, default: %d)"), DEFAULT_CONNECT_TIMEOUT)); strUsage += HelpMessageOpt("-torcontrol=:", strprintf(_("Tor control port to use if onion listening enabled (default: %s)"), DEFAULT_TOR_CONTROL)); @@ -986,6 +986,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) < 0) return InitError("rpcserialversion must be non-negative."); + if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1) + return InitError("unknown rpcserialversion requested."); + nMaxTipAge = GetArg("-maxtipage", DEFAULT_MAX_TIP_AGE); fEnableReplacement = GetBoolArg("-mempoolreplacement", DEFAULT_ENABLE_REPLACEMENT); From 49a612f347be150b03eb6ef8fa438d2b06e26993 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 11 Dec 2016 14:12:47 +0100 Subject: [PATCH 7/7] [qa] Don't set unknown rpcserialversion Github-Pull: #9322 Rebased-From: fa615d39b53a9309ef480c41ec073354c4371f40 --- qa/rpc-tests/segwit.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/rpc-tests/segwit.py b/qa/rpc-tests/segwit.py index 4b4fdf8b1..31a48955e 100755 --- a/qa/rpc-tests/segwit.py +++ b/qa/rpc-tests/segwit.py @@ -85,7 +85,7 @@ class SegWitTest(BitcoinTestFramework): def setup_network(self): self.nodes = [] self.nodes.append(start_node(0, self.options.tmpdir, ["-logtimemicros", "-debug", "-walletprematurewitness", "-rpcserialversion=0"])) - self.nodes.append(start_node(1, self.options.tmpdir, ["-logtimemicros", "-debug", "-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=2"])) + self.nodes.append(start_node(1, self.options.tmpdir, ["-logtimemicros", "-debug", "-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1"])) self.nodes.append(start_node(2, self.options.tmpdir, ["-logtimemicros", "-debug", "-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness"])) connect_nodes(self.nodes[1], 0) connect_nodes(self.nodes[2], 1) @@ -215,7 +215,6 @@ class SegWitTest(BitcoinTestFramework): assert_equal(len(segwit_tx_list), 5) print("Verify block and transaction serialization rpcs return differing serializations depending on rpc serialization flag") - # Note: node1 has version 2, which is simply >0 and will catch future upgrades in tests 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)):