4 files changed, 74 insertions, 15 deletions
diff --git a/src/node/context.cpp b/src/node/context.cpp
new file mode 100644
index 0000000000..26a01420c8
--- /dev/null
+++ b/src/node/context.cpp
@@ -0,0 +1,13 @@
+// Copyright (c) 2019 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 <node/context.h>
+
+#include <banman.h>
+#include <interfaces/chain.h>
+#include <net.h>
+#include <net_processing.h>
+
+NodeContext::NodeContext() {}
+NodeContext::~NodeContext() {}
diff --git a/src/node/context.h b/src/node/context.h
new file mode 100644
index 0000000000..2b124af4db
--- /dev/null
+++ b/src/node/context.h
@@ -0,0 +1,44 @@
+// Copyright (c) 2019 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_CONTEXT_H
+#define BITCOIN_NODE_CONTEXT_H
+
+#include <memory>
+#include <vector>
+
+class BanMan;
+class CConnman;
+class PeerLogicValidation;
+namespace interfaces {
+class Chain;
+class ChainClient;
+} // namespace interfaces
+
+//! NodeContext struct containing references to chain state and connection
+//! state.
+//!
+//! This is used by init, rpc, and test code to pass object references around
+//! without needing to declare the same variables and parameters repeatedly, or
+//! to use globals. More variables could be added to this struct (particularly
+//! references to validation and mempool objects) to eliminate use of globals
+//! and make code more modular and testable. The struct isn't intended to have
+//! any member functions. It should just be a collection of references that can
+//! be used without pulling in unwanted dependencies or functionality.
+struct NodeContext
+{
+    std::unique_ptr<CConnman> connman;
+    std::unique_ptr<PeerLogicValidation> peer_logic;
+    std::unique_ptr<BanMan> banman;
+    std::unique_ptr<interfaces::Chain> chain;
+    std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients;
+
+    //! Declare default constructor and destructor that are not inline, so code
+    //! instantiating the NodeContext struct doesn't need to #include class
+    //! definitions for all the unique_ptr members.
+    NodeContext();
+    ~NodeContext();
+};
+
+#endif // BITCOIN_NODE_CONTEXT_H
diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 7783671a6c..3c0df2b26e 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -6,6 +6,7 @@
 #include <consensus/validation.h>
 #include <net.h>
 #include <net_processing.h>
+#include <node/context.h>
 #include <util/validation.h>
 #include <validation.h>
 #include <validationinterface.h>
@@ -13,12 +14,12 @@
 
 #include <future>
 
-TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback)
+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.
-    // g_connman is assigned both before chain clients and before RPC server is accepting calls,
-    // and reset after chain clients and RPC sever are stopped. g_connman should never be null here.
-    assert(g_connman);
+    // node.connman is assigned both before chain clients and before RPC server is accepting calls,
+    // and reset after chain clients and RPC sever are stopped. node.connman should never be null here.
+    assert(node.connman);
     std::promise<void> promise;
     uint256 hashTx = tx->GetHash();
     bool callback_set = false;
@@ -30,24 +31,22 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err
     CCoinsViewCache &view = ::ChainstateActive().CoinsTip();
     for (size_t o = 0; o < tx->vout.size(); o++) {
         const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o));
-        // IsSpent doesnt mean the coin is spent, it means the output doesnt' exist.
+        // 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 (!mempool.exists(hashTx)) {
         // Transaction is not already in the mempool. Submit it.
-        CValidationState state;
-        bool fMissingInputs;
-        if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs,
+        TxValidationState state;
+        if (!AcceptToMemoryPool(mempool, state, std::move(tx),
                 nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) {
+            err_string = FormatStateMessage(state);
             if (state.IsInvalid()) {
-                err_string = FormatStateMessage(state);
-                return TransactionError::MEMPOOL_REJECTED;
-            } else {
-                if (fMissingInputs) {
+                if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
                     return TransactionError::MISSING_INPUTS;
                 }
-                err_string = FormatStateMessage(state);
+                return TransactionError::MEMPOOL_REJECTED;
+            } else {
                 return TransactionError::MEMPOOL_ERROR;
             }
         }
@@ -79,7 +78,7 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err
     }
 
     if (relay) {
-        RelayTransaction(hashTx, *g_connman);
+        RelayTransaction(hashTx, *node.connman);
     }
 
     return TransactionError::OK;
diff --git a/src/node/transaction.h b/src/node/transaction.h
index a3e56544a7..35873d8376 100644
--- a/src/node/transaction.h
+++ b/src/node/transaction.h
@@ -9,6 +9,8 @@
 #include <primitives/transaction.h>
 #include <util/error.h>
 
+struct NodeContext;
+
 /**
  * Submit a transaction to the mempool and (optionally) relay it to all P2P peers.
  *
@@ -18,6 +20,7 @@
  * NOT be set while cs_main, cs_mempool or cs_wallet are held to avoid
  * deadlock.
  *
+ * @param[in]  node reference to node context
  * @param[in]  tx the transaction to broadcast
  * @param[out] &err_string reference to std::string to fill with error string if available
  * @param[in]  max_tx_fee reject txs with fees higher than this (if 0, accept any fee)
@@ -25,6 +28,6 @@
  * @param[in]  wait_callback, wait until callbacks have been processed to avoid stale result due to a sequentially RPC.
  * return error
  */
-NODISCARD TransactionError BroadcastTransaction(CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback);
+NODISCARD TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback);
 
 #endif // BITCOIN_NODE_TRANSACTION_H