From 5a77abd4e657458852875a07692898982f4b1db5 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 16 Jun 2021 12:02:00 +0100 Subject: [PATCH] [style] Clean up BroadcastTransaction() --- src/node/transaction.cpp | 124 ++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 0c1295c0b5624..d3bce069b0527 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -28,81 +28,83 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback) { - // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs. - // node.peerman is assigned both before chain clients and before RPC server is accepting calls, - // and reset after chain clients and RPC sever are stopped. node.peerman should never be null here. - assert(node.peerman); + // BroadcastTransaction can be called by either sendrawtransaction RPC or the wallet. + // chainman, mempool and peerman are initialized before the RPC server and wallet are started + // and reset after the RPC sever and wallet are stopped. + assert(node.chainman); assert(node.mempool); + assert(node.peerman); + std::promise promise; uint256 txid = tx->GetHash(); uint256 wtxid = tx->GetWitnessHash(); bool callback_set = false; - { // cs_main scope - assert(node.chainman); - LOCK(cs_main); - // If the transaction is already confirmed in the chain, don't do anything - // and return early. - CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip(); - for (size_t o = 0; o < tx->vout.size(); o++) { - const Coin& existingCoin = view.AccessCoin(COutPoint(txid, o)); - // IsSpent doesn't mean the coin is spent, it means the output doesn't exist. - // So if the output does exist, then this transaction exists in the chain. - if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; - } - if (auto mempool_tx = node.mempool->get(txid); mempool_tx) { - // There's already a transaction in the mempool with this txid. Don't - // try to submit this transaction to the mempool (since it'll be - // rejected as a TX_CONFLICT), but do attempt to reannounce the mempool - // transaction if relay=true. - // - // The mempool transaction may have the same or different witness (and - // wtxid) as this transaction. Use the mempool's wtxid for reannouncement. - wtxid = mempool_tx->GetWitnessHash(); - } else { - // Transaction is not already in the mempool. - if (max_tx_fee > 0) { - // First, call ATMP with test_accept and check the fee. If ATMP - // fails here, return error immediately. + { + LOCK(cs_main); + + // If the transaction is already confirmed in the chain, don't do anything + // and return early. + CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip(); + for (size_t o = 0; o < tx->vout.size(); o++) { + const Coin& existingCoin = view.AccessCoin(COutPoint(txid, o)); + // IsSpent doesn't mean the coin is spent, it means the output doesn't exist. + // So if the output does exist, then this transaction exists in the chain. + if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; + } + + if (auto mempool_tx = node.mempool->get(txid); mempool_tx) { + // There's already a transaction in the mempool with this txid. Don't + // try to submit this transaction to the mempool (since it'll be + // rejected as a TX_CONFLICT), but do attempt to reannounce the mempool + // transaction if relay=true. + // + // The mempool transaction may have the same or different witness (and + // wtxid) as this transaction. Use the mempool's wtxid for reannouncement. + wtxid = mempool_tx->GetWitnessHash(); + } else { + // Transaction is not already in the mempool. + if (max_tx_fee > 0) { + // First, call ATMP with test_accept and check the fee. If ATMP + // fails here, return error immediately. + const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, false /* bypass_limits */, + true /* test_accept */); + if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { + return HandleATMPError(result.m_state, err_string); + } else if (result.m_base_fees.value() > max_tx_fee) { + return TransactionError::MAX_FEE_EXCEEDED; + } + } + // Try to submit the transaction to the mempool. const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, false /* bypass_limits */, - true /* test_accept */); + false /* test_accept */); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { return HandleATMPError(result.m_state, err_string); - } else if (result.m_base_fees.value() > max_tx_fee) { - return TransactionError::MAX_FEE_EXCEEDED; } - } - // Try to submit the transaction to the mempool. - const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, false /* bypass_limits */, - false /* test_accept */); - if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { - return HandleATMPError(result.m_state, err_string); - } - // Transaction was accepted to the mempool. + // Transaction was accepted to the mempool. - if (relay) { - // the mempool tracks locally submitted transactions to make a - // best-effort of initial broadcast - node.mempool->AddUnbroadcastTx(txid); - } + if (relay) { + // the mempool tracks locally submitted transactions to make a + // best-effort of initial broadcast + node.mempool->AddUnbroadcastTx(txid); + } - if (wait_callback) { - // For transactions broadcast from outside the wallet, make sure - // that the wallet has been notified of the transaction before - // continuing. - // - // 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(); - }); - callback_set = true; + if (wait_callback) { + // For transactions broadcast from outside the wallet, make sure + // that the wallet has been notified of the transaction before + // continuing. + // + // 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(); + }); + callback_set = true; + } } - } - } // cs_main if (callback_set) {