From 8c8aa19b4b4fa56cd359092ef099bcfc7b26c334 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 11 Apr 2019 14:37:30 +0000 Subject: Add BroadcastTransaction utility usage in Chain interface Access through a broadcastTransaction method. Add a wait_callback flag to turn off race protection when wallet already track its tx being in mempool Standardise highfee, absurdfee variable name to max_tx_fee We drop the P2P check in BroadcastTransaction as g_connman is only called by RPCs and the wallet scheduler, both of which are initialized after g_connman is assigned and stopped before g_connman is reset. --- src/node/transaction.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'src/node/transaction.cpp') diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 5cd567a5c..0cbf64598 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; } -- cgit v1.2.3 From fb62f128bbfd8c6cd72ea8e23331a4bae23883ab Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 23 Jul 2019 11:49:53 -0400 Subject: Tidy up BroadcastTransaction() --- src/node/transaction.cpp | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) (limited to 'src/node/transaction.cpp') diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 0cbf64598..8e5649635 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -23,15 +23,17 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err { // cs_main scope LOCK(cs_main); + // If the transaction is already confirmed in the chain, don't do anything + // and return early. CCoinsViewCache &view = *pcoinsTip; - bool fHaveChain = false; - for (size_t o = 0; !fHaveChain && o < tx->vout.size(); o++) { + for (size_t o = 0; o < tx->vout.size(); o++) { const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); - fHaveChain = !existingCoin.IsSpent(); + // IsSpent doesnt mean the coin is spent, it means the output doesnt' exist. + // So if the output does exist, then this transaction exists in the chain. + if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN; } - bool fHaveMempool = mempool.exists(hashTx); - if (!fHaveMempool && !fHaveChain) { - // push to local node and sync with wallets + if (!mempool.exists(hashTx)) { + // Transaction is not already in the mempool. Submit it. CValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, @@ -46,24 +48,31 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err err_string = FormatStateMessage(state); return TransactionError::MEMPOOL_ERROR; } - } 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 - // to/from their wallet, immediately call some wallet RPC, and get - // a stale result because callbacks have not yet been processed. + } + + // Transaction was accepted to the mempool. + + 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; } - } else if (fHaveChain) { - return TransactionError::ALREADY_IN_CHAIN; } } // cs_main if (callback_set) { + // Wait until Validation Interface clients have been notified of the + // transaction entering the mempool. promise.get_future().wait(); } -- cgit v1.2.3