From 162ffefd2f562169725559906601c25c579aa91c Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Mon, 28 Jan 2019 22:44:11 -0800 Subject: [PATCH 1/7] Add pf_invalid arg to std::string DecodeBase{32,64} Add support for the optional "pf_invalid" out parameter (which allows the caller to detect decoding failures) to the std::string versions of DecodeBase32 and DecodeBase64. The char* versions already have this feature. Also, rename all uses of pfInvalid to pf_invalid to match style guidelines. --- src/util/strencodings.cpp | 16 ++++++++-------- src/util/strencodings.h | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index fedeeac39..b55547bc6 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -141,7 +141,7 @@ std::string EncodeBase64(const std::string& str) return EncodeBase64((const unsigned char*)str.c_str(), str.size()); } -std::vector DecodeBase64(const char* p, bool* pfInvalid) +std::vector DecodeBase64(const char* p, bool* pf_invalid) { static const int decode64_table[256] = { @@ -183,14 +183,14 @@ std::vector DecodeBase64(const char* p, bool* pfInvalid) ++p; } valid = valid && (p - e) % 4 == 0 && p - q < 4; - if (pfInvalid) *pfInvalid = !valid; + if (pf_invalid) *pf_invalid = !valid; return ret; } -std::string DecodeBase64(const std::string& str) +std::string DecodeBase64(const std::string& str, bool* pf_invalid) { - std::vector vchRet = DecodeBase64(str.c_str()); + std::vector vchRet = DecodeBase64(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } @@ -210,7 +210,7 @@ std::string EncodeBase32(const std::string& str) return EncodeBase32((const unsigned char*)str.c_str(), str.size()); } -std::vector DecodeBase32(const char* p, bool* pfInvalid) +std::vector DecodeBase32(const char* p, bool* pf_invalid) { static const int decode32_table[256] = { @@ -252,14 +252,14 @@ std::vector DecodeBase32(const char* p, bool* pfInvalid) ++p; } valid = valid && (p - e) % 8 == 0 && p - q < 8; - if (pfInvalid) *pfInvalid = !valid; + if (pf_invalid) *pf_invalid = !valid; return ret; } -std::string DecodeBase32(const std::string& str) +std::string DecodeBase32(const std::string& str, bool* pf_invalid) { - std::vector vchRet = DecodeBase32(str.c_str()); + std::vector vchRet = DecodeBase32(str.c_str(), pf_invalid); return std::string((const char*)vchRet.data(), vchRet.size()); } diff --git a/src/util/strencodings.h b/src/util/strencodings.h index e392055f2..59eefff56 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -44,12 +44,12 @@ bool IsHex(const std::string& str); * Return true if the string is a hex number, optionally prefixed with "0x" */ bool IsHexNumber(const std::string& str); -std::vector DecodeBase64(const char* p, bool* pfInvalid = nullptr); -std::string DecodeBase64(const std::string& str); +std::vector DecodeBase64(const char* p, bool* pf_invalid = nullptr); +std::string DecodeBase64(const std::string& str, bool* pf_invalid = nullptr); std::string EncodeBase64(const unsigned char* pch, size_t len); std::string EncodeBase64(const std::string& str); -std::vector DecodeBase32(const char* p, bool* pfInvalid = nullptr); -std::string DecodeBase32(const std::string& str); +std::vector DecodeBase32(const char* p, bool* pf_invalid = nullptr); +std::string DecodeBase32(const std::string& str, bool* pf_invalid = nullptr); std::string EncodeBase32(const unsigned char* pch, size_t len); std::string EncodeBase32(const std::string& str); From c734aaa15d924470cec0f17b00ad2e47472b471f Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Tue, 29 Jan 2019 21:32:38 -0800 Subject: [PATCH 2/7] Split DecodePSBT into Base64 and Raw versions Split up DecodePSBT, which both decodes base64 and then deserializes a PartiallySignedTransaction, into two functions: DecodeBase64PSBT, which retains the old behavior, and DecodeRawPSBT, which only performs the deserialization. Add a test for base64 decoding failure. --- src/core_io.h | 6 +++++- src/core_read.cpp | 16 +++++++++++++--- src/rpc/rawtransaction.cpp | 6 +++--- src/wallet/rpcwallet.cpp | 2 +- test/functional/rpc_psbt.py | 3 +++ 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/core_io.h b/src/core_io.h index 6f87161f4..ae377eb6e 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -37,7 +37,11 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); */ bool ParseHashStr(const std::string& strHex, uint256& result); std::vector ParseHexUV(const UniValue& v, const std::string& strName); -NODISCARD bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error); + +//! Decode a base64ed PSBT into a PartiallySignedTransaction +NODISCARD bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error); +//! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction +NODISCARD bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error); int ParseSighashString(const UniValue& sighash); // core_write.cpp diff --git a/src/core_read.cpp b/src/core_read.cpp index 3b82b2853..037909871 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -176,10 +176,20 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk) return true; } -bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error) +bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error) { - std::vector tx_data = DecodeBase64(base64_tx.c_str()); - CDataStream ss_data(tx_data, SER_NETWORK, PROTOCOL_VERSION); + bool invalid; + std::string tx_data = DecodeBase64(base64_tx, &invalid); + if (invalid) { + error = "invalid base64"; + return false; + } + return DecodeRawPSBT(psbt, tx_data, error); +} + +bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error) +{ + CDataStream ss_data(tx_data.data(), tx_data.data() + tx_data.size(), SER_NETWORK, PROTOCOL_VERSION); try { ss_data >> psbt; if (!ss_data.empty()) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index cce62bace..9dac989b9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1323,7 +1323,7 @@ UniValue decodepsbt(const JSONRPCRequest& request) // Unserialize the transactions PartiallySignedTransaction psbtx; std::string error; - if (!DecodePSBT(psbtx, request.params[0].get_str(), error)) { + if (!DecodeBase64PSBT(psbtx, request.params[0].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } @@ -1524,7 +1524,7 @@ UniValue combinepsbt(const JSONRPCRequest& request) for (unsigned int i = 0; i < txs.size(); ++i) { PartiallySignedTransaction psbtx; std::string error; - if (!DecodePSBT(psbtx, txs[i].get_str(), error)) { + if (!DecodeBase64PSBT(psbtx, txs[i].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } psbtxs.push_back(psbtx); @@ -1581,7 +1581,7 @@ UniValue finalizepsbt(const JSONRPCRequest& request) // Unserialize the transactions PartiallySignedTransaction psbtx; std::string error; - if (!DecodePSBT(psbtx, request.params[0].get_str(), error)) { + if (!DecodeBase64PSBT(psbtx, request.params[0].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9bbbdc613..b01f40b12 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4046,7 +4046,7 @@ UniValue walletprocesspsbt(const JSONRPCRequest& request) // Unserialize the transaction PartiallySignedTransaction psbtx; std::string error; - if (!DecodePSBT(psbtx, request.params[0].get_str(), error)) { + if (!DecodeBase64PSBT(psbtx, request.params[0].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index a82a5d020..c98f10582 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -293,5 +293,8 @@ class PSBTTest(BitcoinTestFramework): psbt = self.nodes[1].walletcreatefundedpsbt([], [{p2pkh : 1}], 0, {"includeWatching" : True}, True) self.nodes[0].decodepsbt(psbt['psbt']) + # Test decoding error: invalid base64 + assert_raises_rpc_error(-22, "TX decode failed invalid base64", self.nodes[0].decodepsbt, ";definitely not base64;") + if __name__ == '__main__': PSBTTest().main() From 81cd9588484cb4f4050ea4e239da0681111795db Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Tue, 8 Jan 2019 22:16:50 -0800 Subject: [PATCH 3/7] Factor BroadcastTransaction out of sendrawtransaction Factor out a new BroadcastTransaction function, performing the core work of the sendrawtransaction rpc, so that it can be used from the GUI code. Move it from src/rpc/ to src/node/. --- src/Makefile.am | 2 + src/node/transaction.cpp | 79 ++++++++++++++++++++++++++++++++++++++ src/node/transaction.h | 14 +++++++ src/rpc/rawtransaction.cpp | 69 ++------------------------------- 4 files changed, 99 insertions(+), 65 deletions(-) create mode 100644 src/node/transaction.cpp create mode 100644 src/node/transaction.h diff --git a/src/Makefile.am b/src/Makefile.am index a57fcb071..9a87af2ef 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -152,6 +152,7 @@ BITCOIN_CORE_H = \ netaddress.h \ netbase.h \ netmessagemaker.h \ + node/transaction.h \ noui.h \ optional.h \ outputtype.h \ @@ -255,6 +256,7 @@ libbitcoin_server_a_SOURCES = \ miner.cpp \ net.cpp \ net_processing.cpp \ + node/transaction.cpp \ noui.cpp \ outputtype.cpp \ policy/fees.cpp \ diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp new file mode 100644 index 000000000..47c0323f1 --- /dev/null +++ b/src/node/transaction.cpp @@ -0,0 +1,79 @@ +// Copyright (c) 2010 Satoshi Nakamoto +// Copyright (c) 2009-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include + +#include + +uint256 BroadcastTransaction(const CTransactionRef tx, const bool allowhighfees) { + std::promise promise; + const uint256& hashTx = tx->GetHash(); + + CAmount nMaxRawTxFee = maxTxFee; + if (allowhighfees) + nMaxRawTxFee = 0; + + { // cs_main scope + LOCK(cs_main); + CCoinsViewCache &view = *pcoinsTip; + bool fHaveChain = false; + for (size_t o = 0; !fHaveChain && o < tx->vout.size(); o++) { + const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); + fHaveChain = !existingCoin.IsSpent(); + } + bool fHaveMempool = mempool.exists(hashTx); + if (!fHaveMempool && !fHaveChain) { + // push to local node and sync with wallets + CValidationState state; + bool fMissingInputs; + if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, + nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) { + if (state.IsInvalid()) { + throw JSONRPCError(RPC_TRANSACTION_REJECTED, FormatStateMessage(state)); + } else { + if (fMissingInputs) { + throw JSONRPCError(RPC_TRANSACTION_ERROR, "Missing inputs"); + } + throw JSONRPCError(RPC_TRANSACTION_ERROR, FormatStateMessage(state)); + } + } else { + // If wallet is enabled, ensure that the wallet has been made aware + // of the new transaction prior to returning. This prevents a race + // where a user might call sendrawtransaction with a transaction + // to/from their wallet, immediately call some wallet RPC, and get + // a stale result because callbacks have not yet been processed. + CallFunctionInValidationInterfaceQueue([&promise] { + promise.set_value(); + }); + } + } else if (fHaveChain) { + throw JSONRPCError(RPC_TRANSACTION_ALREADY_IN_CHAIN, "transaction already in block chain"); + } else { + // Make sure we don't block forever if re-sending + // a transaction already in mempool. + promise.set_value(); + } + + } // cs_main + + promise.get_future().wait(); + + if(!g_connman) + throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + + CInv inv(MSG_TX, hashTx); + g_connman->ForEachNode([&inv](CNode* pnode) + { + pnode->PushInventory(inv); + }); + + return hashTx; +} diff --git a/src/node/transaction.h b/src/node/transaction.h new file mode 100644 index 000000000..1916c6db2 --- /dev/null +++ b/src/node/transaction.h @@ -0,0 +1,14 @@ +// Copyright (c) 2017-2018 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 +#include + +/** Broadcast a transaction */ +uint256 BroadcastTransaction(CTransactionRef tx, bool allowhighfees = false); + +#endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 9dac989b9..8e561c618 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,13 +24,11 @@ #include