diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -169,14 +169,23 @@ //! Check if transaction has descendants in mempool. virtual bool hasDescendantsInMempool(const TxId &txid) = 0; + //! Relay transaction. + virtual void relayTransaction(const TxId &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 Config &config, + const CTransactionRef &tx, + std::string &err_string, + const Amount &max_tx_fee, bool relay) = 0; + //! Calculate mempool ancestor and descendant counts for the given //! transaction. virtual void getTransactionAncestry(const TxId &txid, size_t &ancestors, size_t &descendants) = 0; - //! Relay transaction. - virtual void relayTransaction(const TxId &txid) = 0; - //! Check if transaction will pass the mempool's chain limits. virtual bool checkChainLimits(const CTransactionRef &tx) = 0; diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -295,6 +296,21 @@ void relayTransaction(const TxId &txid) override { RelayTransaction(txid, *g_connman); } + bool broadcastTransaction(const Config &config, + const CTransactionRef &tx, + std::string &err_string, + const Amount &max_tx_fee, + bool relay) override { + const TransactionError err = + BroadcastTransaction(config, 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 err == TransactionError::OK; + } void getTransactionAncestry(const TxId &txid, size_t &ancestors, size_t &descendants) override { ::g_mempool.GetTransactionAncestry(txid, ancestors, descendants); diff --git a/src/node/transaction.h b/src/node/transaction.h --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -16,16 +16,20 @@ * 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(const Config &config, - CTransactionRef tx, TxId &txid, + CTransactionRef tx, std::string &err_string, - Amount highfee); + Amount max_tx_fee, bool relay, + bool wait_callback); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -18,11 +18,14 @@ #include TransactionError BroadcastTransaction(const Config &config, - const CTransactionRef tx, TxId &txid, + const CTransactionRef tx, std::string &err_string, - const Amount highFee) { + const Amount max_tx_fee, bool relay, + bool wait_callback) { + assert(g_connman); std::promise promise; - txid = tx->GetId(); + TxId txid = tx->GetId(); + bool callback_set = false; { // cs_main scope LOCK(cs_main); @@ -40,7 +43,7 @@ bool fMissingInputs; if (!AcceptToMemoryPool(config, g_mempool, state, std::move(tx), &fMissingInputs, false /* bypass_limits */, - highFee)) { + max_tx_fee)) { if (state.IsInvalid()) { err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_REJECTED; @@ -52,7 +55,7 @@ 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 @@ -61,23 +64,20 @@ // have not yet been processed. 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(txid, *g_connman); + if (relay) { + RelayTransaction(txid, *g_connman); + } return TransactionError::OK; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -977,16 +977,16 @@ throw JSONRPCError(RPC_INVALID_PARAMETER, "second argument (maxfeerate) must be numeric"); } - - TxId txid; std::string err_string; - const TransactionError err = - BroadcastTransaction(config, tx, txid, err_string, max_raw_tx_fee); + AssertLockNotHeld(cs_main); + const TransactionError err = BroadcastTransaction( + config, tx, err_string, max_raw_tx_fee, /*relay*/ true, + /*wait_callback*/ true); if (err != TransactionError::OK) { throw JSONRPCTransactionError(err, err_string); } - return txid.GetHex(); + return tx->GetHash().GetHex(); } static UniValue testmempoolaccept(const Config &config,