Merge #15638: Move-only: Pull wallet code out of libbitcoin_server

4d074e8 [build] Move AnalyzePSBT from psbt.cpp to node/psbt.cpp (Russell Yanofsky)
fd509bd [docs] Document src subdirectories and different libraries (John Newbery)
9eaeb7f [build] Move wallet load functions to wallet/load unit (John Newbery)
91a25d1 [build] Add several util units (John Newbery)
9951786 [build] Move several units into common libraries (John Newbery)
0509465 [build] Move rpc rawtransaction util functions to rpc/rawtransaction_util.cpp (John Newbery)
1acc61f [build] Move rpc utility methods to rpc/util (John Newbery)
4a75c9d [build] Move policy settings to new src/policy/settings unit (John Newbery)
fdf8888 [build] Move CheckTransaction from lib_server to lib_consensus (John Newbery)

Pull request description:

  This is a move-only commit. No code is changing and the moves can be easily verified with:

  ```sh
  git log -p -n1 --color-moved=dimmed_zebra
  ```

  This commit moves functions and variables that wallet code depends on out of libbitcoin_server.a, so the bitcoin-wallet tool can be built without libbitcoin_server.a in #15639, and attempting to access server state from wallet code will result in link errors instead of silently broken code.

  List of moves:

  - `CheckTransaction` moves from `consensus/tx_verify.cpp` to `consensus/tx_check.cpp`
  - `urlDecode` moves from `httpserver.cpp` to `util/url.cpp`
  - `TransactionErrorString` moves from `node/transaction.cpp` to `util/error.cpp`
  - `StringForFeeReason` and `FeeModeFromString` move from `policy/fees.cpp` to `util/fees.cpp`
  - `incrementalRelayFee` `dustRelayFee` and `nBytesPerSigOp` move from `policy/policy.cpp` to `policy/settings.cpp`
  - `SignalsOptInRBF` moves from `policy/rbf.cpp` to `util/rbf.cpp`
  - `fIsBareMultisigStd` moves from `validation.cpp` to `policy/settings.cpp`
  - `ConstructTransaction` `TxInErrorToJSON` and `SignTransaction` move from `rpc/rawtransaction.cpp` to `rpc/rawtransaction_util.cpp`
  - `RPCTypeCheck` `RPCTypeCheckArgument` `RPCTypeCheckObj` `AmountFromValue` `ParseHashV``ParseHashO` `ParseHexV` `ParseHexO` `HelpExampleCli` and `HelpExampleRpc` move from `rpc/server.cpp` to `rpc/util.cpp`
  - `AmountHighWarn` and `AmountErrMsg` move from `ui_interface.cpp` to `util/error.cpp`
  - `FormatStateMessage` and `strMessageMagic` move from `validation.cpp` to `util/validation.cpp`
  - `VerifyWallets` `LoadWallets` `StartWallets` `FlushWallets` `StopWallets` and `UnloadWallets` move from `wallet/init.cpp` to `wallet/node.cpp`

ACKs for commit 4d074e:
  jnewbery:
    utACK 4d074e8 (checked by doing the rebase myself and verifying no difference between my branch and 4d074e8)

Tree-SHA512: 5e1604a9fb06475f2b96da0de0baa8330f4dda834dc20a0183ef11e1e4c27631d1d1bbb9abf0054efc03d56945fdf9920f63366b6a4f200f665b742a479ff75c

Author: laanwj

build_msvc/libbitcoin_server/libbitcoin_server.vcxproj.in

@@ -151,6 +151,9 @@
   </ItemDefinitionGroup>
   <ItemGroup>
 @SOURCE_FILES@
+    <ClCompile Include="..\..\src\wallet\init.cpp">
+      <ObjectFileName>$(IntDir)wallet_init.obj</ObjectFileName>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />

build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj

@@ -22,6 +22,7 @@
   <ItemGroup>
     <ClCompile Include="..\..\src\arith_uint256.cpp" />
     <ClCompile Include="..\..\src\consensus\merkle.cpp" />
+    <ClCompile Include="..\..\src\consensus\tx_check.cpp" />
     <ClCompile Include="..\..\src\crypto\aes.cpp" />
     <ClCompile Include="..\..\src\crypto\chacha20.cpp" />
     <ClCompile Include="..\..\src\crypto\hmac_sha256.cpp" />

src/Makefile.am

@@ -124,6 +124,7 @@ BITCOIN_CORE_H = \
   compat/sanity.h \
   compressor.h \
   consensus/consensus.h \
+  consensus/tx_check.h \
   consensus/tx_verify.h \
   core_io.h \
   core_memusage.h \
@@ -155,6 +156,7 @@ BITCOIN_CORE_H = \
   netbase.h \
   netmessagemaker.h \
   node/coin.h \
+  node/psbt.h \
   node/transaction.h \
   noui.h \
   optional.h \
@@ -163,6 +165,7 @@ BITCOIN_CORE_H = \
   policy/fees.h \
   policy/policy.h \
   policy/rbf.h \
+  policy/settings.h \
   pow.h \
   protocol.h \
   psbt.h \
@@ -174,7 +177,7 @@ BITCOIN_CORE_H = \
   rpc/mining.h \
   rpc/protocol.h \
   rpc/server.h \
-  rpc/rawtransaction.h \
+  rpc/rawtransaction_util.h \
   rpc/register.h \
   rpc/util.h \
   scheduler.h \
@@ -201,10 +204,15 @@ BITCOIN_CORE_H = \
   undo.h \
   util/bip32.h \
   util/bytevectorhash.h \
+  util/error.h \
+  util/fees.h \
   util/system.h \
   util/memory.h \
   util/moneystr.h \
+  util/rbf.h \
   util/time.h \
+  util/url.h \
+  util/validation.h \
   validation.h \
   validationinterface.h \
   versionbits.h \
@@ -215,6 +223,7 @@ BITCOIN_CORE_H = \
   wallet/db.h \
   wallet/feebumper.h \
   wallet/fees.h \
+  wallet/load.h \
   wallet/psbtwallet.h \
   wallet/rpcwallet.h \
   wallet/wallet.h \
@@ -237,13 +246,15 @@ obj/build.h: FORCE
 libbitcoin_util_a-clientversion.$(OBJEXT): obj/build.h
 
 # server: shared between bitcoind and bitcoin-qt
+# Contains code accessing mempool and chain state that is meant to be separated
+# from wallet and gui code (see node/README.md). Shared code should go in
+# libbitcoin_common or libbitcoin_util libraries, instead.
 libbitcoin_server_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS)
 libbitcoin_server_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 libbitcoin_server_a_SOURCES = \
   addrdb.cpp \
   addrman.cpp \
   banman.cpp \
-  bloom.cpp \
   blockencodings.cpp \
   blockfilter.cpp \
   chain.cpp \
@@ -255,21 +266,19 @@ libbitcoin_server_a_SOURCES = \
   index/base.cpp \
   index/txindex.cpp \
   interfaces/chain.cpp \
-  interfaces/handler.cpp \
   interfaces/node.cpp \
   init.cpp \
   dbwrapper.cpp \
-  merkleblock.cpp \
   miner.cpp \
   net.cpp \
   net_processing.cpp \
   node/coin.cpp \
+  node/psbt.cpp \
   node/transaction.cpp \
   noui.cpp \
-  outputtype.cpp \
   policy/fees.cpp \
-  policy/policy.cpp \
   policy/rbf.cpp \
+  policy/settings.cpp \
   pow.cpp \
   rest.cpp \
   rpc/blockchain.cpp \
@@ -278,7 +287,6 @@ libbitcoin_server_a_SOURCES = \
   rpc/net.cpp \
   rpc/rawtransaction.cpp \
   rpc/server.cpp \
-  rpc/util.cpp \
   script/sigcache.cpp \
   shutdown.cpp \
   timedata.cpp \
@@ -291,6 +299,9 @@ libbitcoin_server_a_SOURCES = \
   versionbits.cpp \
   $(BITCOIN_CORE_H)
 
+if ENABLE_WALLET
+libbitcoin_server_a_SOURCES += wallet/init.cpp
+endif
 if !ENABLE_WALLET
 libbitcoin_server_a_SOURCES += dummywallet.cpp
 endif
@@ -317,7 +328,7 @@ libbitcoin_wallet_a_SOURCES = \
   wallet/db.cpp \
   wallet/feebumper.cpp \
   wallet/fees.cpp \
-  wallet/init.cpp \
+  wallet/load.cpp \
   wallet/psbtwallet.cpp \
   wallet/rpcdump.cpp \
   wallet/rpcwallet.cpp \
@@ -391,6 +402,7 @@ libbitcoin_consensus_a_SOURCES = \
   consensus/merkle.cpp \
   consensus/merkle.h \
   consensus/params.h \
+  consensus/tx_check.cpp \
   consensus/validation.h \
   hash.cpp \
   hash.h \
@@ -423,6 +435,7 @@ libbitcoin_common_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 libbitcoin_common_a_SOURCES = \
   base58.cpp \
   bech32.cpp \
+  bloom.cpp \
   chainparams.cpp \
   coins.cpp \
   compressor.cpp \
@@ -431,11 +444,16 @@ libbitcoin_common_a_SOURCES = \
   key.cpp \
   key_io.cpp \
   keystore.cpp \
+  merkleblock.cpp \
   netaddress.cpp \
   netbase.cpp \
+  outputtype.cpp \
   policy/feerate.cpp \
+  policy/policy.cpp \
   protocol.cpp \
   psbt.cpp \
+  rpc/rawtransaction_util.cpp \
+  rpc/util.cpp \
   scheduler.cpp \
   script/descriptor.cpp \
   script/ismine.cpp \
@@ -458,6 +476,7 @@ libbitcoin_util_a_SOURCES = \
   compat/glibcxx_sanity.cpp \
   compat/strnlen.cpp \
   fs.cpp \
+  interfaces/handler.cpp \
   logging.cpp \
   random.cpp \
   rpc/protocol.cpp \
@@ -466,10 +485,15 @@ libbitcoin_util_a_SOURCES = \
   threadinterrupt.cpp \
   util/bip32.cpp \
   util/bytevectorhash.cpp \
+  util/error.cpp \
+  util/fees.cpp \
   util/system.cpp \
   util/moneystr.cpp \
+  util/rbf.cpp \
   util/strencodings.cpp \
   util/time.cpp \
+  util/url.cpp \
+  util/validation.cpp \
   $(BITCOIN_CORE_H)
 
 if GLIBC_BACK_COMPAT

src/bitcoin-tx.cpp

@@ -18,6 +18,7 @@
 #include <script/script.h>
 #include <script/sign.h>
 #include <univalue.h>
+#include <util/rbf.h>
 #include <util/system.h>
 #include <util/moneystr.h>
 #include <util/strencodings.h>

src/consensus/tx_check.cpp

@@ -0,0 +1,57 @@
+// Copyright (c) 2017-2018 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 <consensus/tx_check.h>
+
+#include <primitives/transaction.h>
+#include <consensus/validation.h>
+
+bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
+{
+    // Basic checks that don't depend on any context
+    if (tx.vin.empty())
+        return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty");
+    if (tx.vout.empty())
+        return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty");
+    // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability)
+    if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT)
+        return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize");
+
+    // Check for negative or overflow output values
+    CAmount nValueOut = 0;
+    for (const auto& txout : tx.vout)
+    {
+        if (txout.nValue < 0)
+            return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative");
+        if (txout.nValue > MAX_MONEY)
+            return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge");
+        nValueOut += txout.nValue;
+        if (!MoneyRange(nValueOut))
+            return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge");
+    }
+
+    // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
+    if (fCheckDuplicateInputs) {
+        std::set<COutPoint> vInOutPoints;
+        for (const auto& txin : tx.vin)
+        {
+            if (!vInOutPoints.insert(txin.prevout).second)
+                return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
+        }
+    }
+
+    if (tx.IsCoinBase())
+    {
+        if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100)
+            return state.DoS(100, false, REJECT_INVALID, "bad-cb-length");
+    }
+    else
+    {
+        for (const auto& txin : tx.vin)
+            if (txin.prevout.IsNull())
+                return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null");
+    }
+
+    return true;
+}

src/consensus/tx_check.h

@@ -0,0 +1,20 @@
+// Copyright (c) 2017-2018 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_CONSENSUS_TX_CHECK_H
+#define BITCOIN_CONSENSUS_TX_CHECK_H
+
+/**
+ * Context-independent transaction checking code that can be called outside the
+ * bitcoin server and doesn't depend on chain or mempool state. Transaction
+ * verification code that does call server functions or depend on server state
+ * belongs in tx_verify.h/cpp instead.
+ */
+
+class CTransaction;
+class CValidationState;
+
+bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
+
+#endif // BITCOIN_CONSENSUS_TX_CHECK_H

src/consensus/tx_verify.cpp

@@ -156,55 +156,6 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
     return nSigOps;
 }
 
-bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
-{
-    // Basic checks that don't depend on any context
-    if (tx.vin.empty())
-        return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty");
-    if (tx.vout.empty())
-        return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty");
-    // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability)
-    if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT)
-        return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize");
-
-    // Check for negative or overflow output values
-    CAmount nValueOut = 0;
-    for (const auto& txout : tx.vout)
-    {
-        if (txout.nValue < 0)
-            return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative");
-        if (txout.nValue > MAX_MONEY)
-            return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge");
-        nValueOut += txout.nValue;
-        if (!MoneyRange(nValueOut))
-            return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge");
-    }
-
-    // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
-    if (fCheckDuplicateInputs) {
-        std::set<COutPoint> vInOutPoints;
-        for (const auto& txin : tx.vin)
-        {
-            if (!vInOutPoints.insert(txin.prevout).second)
-                return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
-        }
-    }
-
-    if (tx.IsCoinBase())
-    {
-        if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100)
-            return state.DoS(100, false, REJECT_INVALID, "bad-cb-length");
-    }
-    else
-    {
-        for (const auto& txin : tx.vin)
-            if (txin.prevout.IsNull())
-                return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null");
-    }
-
-    return true;
-}
-
 bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee)
 {
     // are the actual inputs available?

src/consensus/tx_verify.h

@@ -17,9 +17,6 @@ class CValidationState;
 
 /** Transaction validation functions */
 
-/** Context-independent validity checks */
-bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
-
 namespace Consensus {
 /**
  * Check whether all inputs of this transaction are valid (no double spends and amounts)

src/httpserver.cpp

@@ -655,15 +655,3 @@ void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch)
         pathHandlers.erase(i);
     }
 }
-
-std::string urlDecode(const std::string &urlEncoded) {
-    std::string res;
-    if (!urlEncoded.empty()) {
-        char *decoded = evhttp_uridecode(urlEncoded.c_str(), false, nullptr);
-        if (decoded) {
-            res = std::string(decoded);
-            free(decoded);
-        }
-    }
-    return res;
-}

src/httpserver.h

@@ -148,6 +148,4 @@ class HTTPEvent
     struct event* ev;
 };
 
-std::string urlDecode(const std::string &urlEncoded);
-
 #endif // BITCOIN_HTTPSERVER_H