Remove p2pEnabled from Chain interface

Antoine Riard · Antoine Riard · commit b7b9f6e4cee2 · 2019-08-08T22:57:35.000-04:00
RPC server starts in warmup mode, it can't
process yet calls, then follows connection manager
initialization and finally RPC server get out of
warmup mode. RPC calls shouldn't be able to get
P2P disabled errors because once we initialize
g_connman it's not unset until shutdown, after
RPC server has been stopped.
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
@@ -332,7 +332,6 @@ class ChainImpl : public Chain
         LOCK(cs_main);
         return ::fHavePruned;
     }
-    bool p2pEnabled() override { return g_connman != nullptr; }
     bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !isInitialBlockDownload(); }
     bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); }
     bool shutdownRequested() override { return ShutdownRequested(); }
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -187,9 +187,6 @@ class Chain
     //! Check if any block has been pruned.
     virtual bool havePruned() = 0;
 
-    //! Check if p2p enabled.
-    virtual bool p2pEnabled() = 0;
-
     //! Check if the node is ready to broadcast transactions.
     virtual bool isReadyToBroadcast() = 0;
 
diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
@@ -16,6 +16,9 @@
 
 TransactionError BroadcastTransaction(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);
     std::promise<void> promise;
     uint256 hashTx = tx->GetHash();
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -309,10 +309,6 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
     if (nValue > curBalance)
         throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds");
 
-    if (pwallet->GetBroadcastTransactions() && !pwallet->chain().p2pEnabled()) {
-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
-    }
-
     // Parse Bitcoin address
     CScript scriptPubKey = GetScriptForDestination(address);
 
@@ -845,10 +841,6 @@ static UniValue sendmany(const JSONRPCRequest& request)
     auto locked_chain = pwallet->chain().lock();
     LOCK(pwallet->cs_wallet);
 
-    if (pwallet->GetBroadcastTransactions() && !pwallet->chain().p2pEnabled()) {
-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
-    }
-
     if (!request.params[0].isNull() && !request.params[0].get_str().empty()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Dummy value must be set to \"\"");
     }

Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,6 @@ class ChainImpl : public Chain`
`332`	`332`	`LOCK(cs_main);`
`333`	`333`	`return ::fHavePruned;`
`334`	`334`	`}`
`335`		`- bool p2pEnabled() override { return g_connman != nullptr; }`
`336`	`335`	`bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !isInitialBlockDownload(); }`
`337`	`336`	`bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); }`
`338`	`337`	`bool shutdownRequested() override { return ShutdownRequested(); }`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,9 @@`
`16`	`16`
`17`	`17`	`TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback)`
`18`	`18`	`{`
	`19`	`+ // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs.`
	`20`	`+ // g_connman is assigned both before chain clients and before RPC server is accepting calls,`
	`21`	`+ // and reset after chain clients and RPC sever are stopped. g_connman should never be null here.`
`19`	`22`	`assert(g_connman);`
`20`	`23`	`std::promise<void> promise;`
`21`	`24`	`uint256 hashTx = tx->GetHash();`