diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 22e4aaedaf..10cf82acdc 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -295,6 +296,14 @@ public: { RelayTransaction(txid, *g_connman); } + bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) override + { + const TransactionError err = BroadcastTransaction(tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); + // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. + // Note: this will need to be updated if BroadcastTransactions() is updated to return other non-mempool failures + // that Chain clients do not need to know about. + return TransactionError::OK == err; + } void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override { ::mempool.GetTransactionAncestry(txid, ancestors, descendants); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index e675defd47..2341506854 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -167,6 +167,11 @@ public: //! Relay transaction. virtual void relayTransaction(const uint256& txid) = 0; + //! Transaction is added to memory pool, if the transaction fee is below the + //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true. + //! Return false if the transaction could not be added due to the fee or for another reason. + virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) = 0; + //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 5cd567a5c4..0cbf645984 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -14,10 +14,12 @@ #include -TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, std::string& err_string, const CAmount& highfee) +TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback) { + assert(g_connman); std::promise promise; - hashTx = tx->GetHash(); + uint256 hashTx = tx->GetHash(); + bool callback_set = false; { // cs_main scope LOCK(cs_main); @@ -33,7 +35,7 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, CValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, - nullptr /* plTxnReplaced */, false /* bypass_limits */, highfee)) { + nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { if (state.IsInvalid()) { err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_REJECTED; @@ -44,7 +46,7 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_ERROR; } - } else { + } else if (wait_callback) { // 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 @@ -53,24 +55,21 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, uint256& hashTx, CallFunctionInValidationInterfaceQueue([&promise] { promise.set_value(); }); + callback_set = true; } } else if (fHaveChain) { return TransactionError::ALREADY_IN_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) { - return TransactionError::P2P_DISABLED; + if (callback_set) { + promise.get_future().wait(); } - RelayTransaction(hashTx, *g_connman); + if (relay) { + RelayTransaction(hashTx, *g_connman); + } return TransactionError::OK; } diff --git a/src/node/transaction.h b/src/node/transaction.h index 51033f94e5..08ceace7f8 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -14,11 +14,13 @@ * Broadcast a transaction * * @param[in] tx the transaction to broadcast - * @param[out] &txid the txid of the transaction, if successfully broadcast * @param[out] &err_string reference to std::string to fill with error string if available - * @param[in] highfee Reject txs with fees higher than this (if 0, accept any fee) + * @param[in] max_tx_fee reject txs with fees higher than this (if 0, accept any fee) + * @param[in] relay flag if both mempool insertion and p2p relay are requested + * @param[in] wait_callback, wait until callbacks have been processed to avoid stale result due to a sequentially RPC. + * It MUST NOT be set while cs_main, cs_mempool or cs_wallet are held to avoid deadlock * return error */ -NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, uint256& txid, std::string& err_string, const CAmount& highfee); +NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0ab504de06..b8ec2178d2 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -810,14 +810,14 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request) max_raw_tx_fee = fr.GetFee((weight+3)/4); } - uint256 txid; std::string err_string; - const TransactionError err = BroadcastTransaction(tx, txid, err_string, max_raw_tx_fee); + AssertLockNotHeld(cs_main); + const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true); if (TransactionError::OK != err) { throw JSONRPCTransactionError(err, err_string); } - return txid.GetHex(); + return tx->GetHash().GetHex(); } static UniValue testmempoolaccept(const JSONRPCRequest& request)