Refactor Public Transaction Creation

qa/rpc-tests/dip3-deterministicmns.py

@@ -61,6 +61,7 @@ def run_test(self):
         self.log.info("creating collateral for mn-before-dip3")
         before_dip3_mn = prepare_mn(self.nodes[0], 1, 'mn-before-dip3')
         create_mn_collateral(self.nodes[0], before_dip3_mn)
+        lock_mn_collateral(self.nodes[0], before_dip3_mn)
         mns.append(before_dip3_mn)
 
         # block 550 starts enforcing DIP3 MN payments
@@ -96,6 +97,7 @@ def run_test(self):
             else:
                 self.log.info("create_collateral %s" % mn.alias)
                 create_mn_collateral(self.nodes[0], mn)
+                lock_mn_collateral(self.nodes[0], mn)
                 self.log.info("register %s" % mn.alias)
                 register_mn(self.nodes[0], mn)
 
@@ -191,6 +193,7 @@ def run_test(self):
         # self.test_instantsend(10, 3, timeout=20)
 
     def spend_mn_collateral(self, mn, with_dummy_input_output=False):
+        unlock_mn_collateral(self.nodes[0], mn)
         return self.spend_input(mn.collateral_txid, mn.collateral_vout, 1000, with_dummy_input_output)
 
     def update_mn_payee(self, mn, payee):

qa/rpc-tests/test_framework/mn_utils.py

@@ -46,6 +46,12 @@ def create_mn_collateral(node, mn):
             break
     assert(mn.collateral_vout != -1)
 
+def lock_mn_collateral(node, mn):
+    node.lockunspent(False, [{'txid': mn.collateral_txid, 'vout': mn.collateral_vout}])
+
+def unlock_mn_collateral(node, mn):
+    node.lockunspent(True, [{'txid': mn.collateral_txid, 'vout': mn.collateral_vout}])
+
 # register a protx MN and also fund it (using collateral inside ProRegTx)
 def register_fund_mn(node, mn):
     node.sendtoaddress(mn.fundsAddr, 1000.001)

qa/rpc-tests/txn_clone.py

@@ -49,9 +49,7 @@ def run_test(self):
         # Coins are sent to node1_address
         node1_address = self.nodes[1].getnewaddress("from0")
 
-        # Send tx1, and another transaction tx2 that won't be cloned
         txid1 = self.nodes[0].sendfrom("foo", node1_address, 40, 0)
-        txid2 = self.nodes[0].sendfrom("bar", node1_address, 20, 0)
 
         # Construct a clone of tx1, to be malleated
         rawtx1 = self.nodes[0].getrawtransaction(txid1,1)
@@ -84,51 +82,43 @@ def run_test(self):
             sync_blocks(self.nodes[0:2])
 
         tx1 = self.nodes[0].gettransaction(txid1)
-        tx2 = self.nodes[0].gettransaction(txid2)
 
         # Node0's balance should be starting balance, plus 40BTC for another
         # matured block, minus tx1 and tx2 amounts, and minus transaction fees:
         expected = starting_balance + fund_foo_tx["fee"] + fund_bar_tx["fee"]
         if self.options.mine_block: expected += 40
         expected += tx1["amount"] + tx1["fee"]
-        expected += tx2["amount"] + tx2["fee"]
         assert_equal(self.nodes[0].getbalance(), expected)
 
         # foo and bar accounts should be debited:
         assert_equal(self.nodes[0].getbalance("foo", 0), 969 + tx1["amount"] + tx1["fee"])
-        assert_equal(self.nodes[0].getbalance("bar", 0), 29 + tx2["amount"] + tx2["fee"])
 
         if self.options.mine_block:
             assert_equal(tx1["confirmations"], 1)
-            assert_equal(tx2["confirmations"], 1)
             # Node1's "from0" balance should be both transaction amounts:
-            assert_equal(self.nodes[1].getbalance("from0"), -(tx1["amount"] + tx2["amount"]))
+            assert_equal(self.nodes[1].getbalance("from0"), -(tx1["amount"]))
         else:
             assert_equal(tx1["confirmations"], 0)
-            assert_equal(tx2["confirmations"], 0)
 
         # Send clone and its parent to miner
         self.nodes[2].sendrawtransaction(fund_foo_tx["hex"])
+        self.nodes[2].sendrawtransaction(fund_bar_tx["hex"])
         txid1_clone = self.nodes[2].sendrawtransaction(tx1_clone["hex"])
         # ... mine a block...
         self.nodes[2].generate(1)
 
         # Reconnect the split network, and sync chain:
         connect_nodes_bi(self.nodes, 1, 2)
-        self.nodes[2].sendrawtransaction(fund_bar_tx["hex"])
-        self.nodes[2].sendrawtransaction(tx2["hex"])
         self.nodes[2].generate(1)  # Mine another block to make sure we sync
         sync_blocks(self.nodes)
 
         # Re-fetch transaction info:
         tx1 = self.nodes[0].gettransaction(txid1)
         tx1_clone = self.nodes[0].gettransaction(txid1_clone)
-        tx2 = self.nodes[0].gettransaction(txid2)
 
         # Verify expected confirmations
         assert_equal(tx1["confirmations"], -2)
         assert_equal(tx1_clone["confirmations"], 2)
-        assert_equal(tx2["confirmations"], 1)
 
         # Check node0's total balance; should be same as before the clone, + 100 BTC for 2 matured,
         # less possible orphaned matured subsidy
@@ -141,8 +131,6 @@ def run_test(self):
         # Check node0's individual account balances.
         # "foo" should have been debited by the equivalent clone of tx1
         assert_equal(self.nodes[0].getbalance("foo"), 969 + tx1["amount"] + tx1["fee"])
-        # "bar" should have been debited by (possibly unconfirmed) tx2
-        assert_equal(self.nodes[0].getbalance("bar", 0), 29 + tx2["amount"] + tx2["fee"])
         # "" should have starting balance, less funding txes, plus subsidies
         assert_equal(self.nodes[0].getbalance("", 0), starting_balance
                                                                 - 969
@@ -152,8 +140,7 @@ def run_test(self):
                                                                 + 80)
 
         # Node1's "from0" account balance
-        assert_equal(self.nodes[1].getbalance("from0", 0), -(tx1["amount"] + tx2["amount"]))
+        assert_equal(self.nodes[1].getbalance("from0", 0), -(tx1["amount"]))
 
 if __name__ == '__main__':
     TxnMallTest().main()
-

qa/rpc-tests/txn_doublespend.py

@@ -36,10 +36,12 @@ def run_test(self):
         node0_address_foo = self.nodes[0].getnewaddress("foo")
         fund_foo_txid = self.nodes[0].sendfrom("", node0_address_foo, 969)
         fund_foo_tx = self.nodes[0].gettransaction(fund_foo_txid)
+        self.nodes[0].lockunspent(False, [{"txid": fund_foo_txid, "vout": find_output(self.nodes[0], fund_foo_txid, 969)}])
 
         node0_address_bar = self.nodes[0].getnewaddress("bar")
         fund_bar_txid = self.nodes[0].sendfrom("", node0_address_bar, 29)
         fund_bar_tx = self.nodes[0].gettransaction(fund_bar_txid)
+        self.nodes[0].lockunspent(True, [{"txid": fund_foo_txid, "vout": find_output(self.nodes[0], fund_foo_txid, 969)}])
 
         assert_equal(self.nodes[0].getbalance(""),
                      starting_balance - 969 - 29 + fund_foo_tx["fee"] + fund_bar_tx["fee"])

src/Makefile.test.include

@@ -208,7 +208,8 @@ BITCOIN_TESTS += \
   wallet/test/spark_tests.cpp \
   wallet/test/sigma_tests.cpp \
   wallet/test/mnemonic_tests.cpp \
-  wallet/test/txbuilder_tests.cpp
+  wallet/test/sigmatxbuilder_tests.cpp \
+  wallet/test/createtransaction_tests.cpp
 endif
 
 test_test_bitcoin_LDADD = $(LIBBITCOIN_SERVER) -ltor

src/llmq/quorums_instantsend.h

@@ -75,9 +75,6 @@ class CInstantSendDb
 class CInstantSendManager : public CRecoveredSigsListener
 {
 private:
-    CCriticalSection cs;
-    CInstantSendDb db;
-
     std::thread workThread;
     CThreadInterrupt workInterrupt;
 
@@ -112,6 +109,9 @@ class CInstantSendManager : public CRecoveredSigsListener
     std::unordered_set<uint256, StaticSaltedHasher> pendingRetryTxs;
     std::atomic_bool isNewInstantSendEnabled{false};
 
+    CCriticalSection cs;
+    CInstantSendDb db;
+
 public:
     CInstantSendManager(CDBWrapper& _llmqDb);
     ~CInstantSendManager();

src/primitives/transaction.h

@@ -171,7 +171,6 @@ class CTxOut
 public:
     CAmount nValue;
     CScript scriptPubKey;
-    int nRounds;
 
     CTxOut()
     {
@@ -186,15 +185,12 @@ class CTxOut
     inline void SerializationOp(Stream& s, Operation ser_action) {
         READWRITE(nValue);
         READWRITE(*(CScriptBase*)(&scriptPubKey));
-        if (ser_action.ForRead())
-            nRounds = -10;
     }
 
     void SetNull()
     {
         nValue = -1;
         scriptPubKey.clear();
-        nRounds = -10; // an initial value, should be no way to get this by calculations
     }
 
     bool IsNull() const

src/script/script.cpp

@@ -217,6 +217,16 @@ unsigned int CScript::GetSigOpCount(const CScript& scriptSig) const
     return subscript.GetSigOpCount(true);
 }
 
+bool CScript::IsPayToPublicKey() const {
+    if (size() != 35) return false;
+
+    opcodetype opcode;
+    const_iterator pc = begin();
+    GetOp(pc, opcode);
+    GetOp(pc, opcode);
+    return opcode == OP_CHECKSIG;
+}
+
 bool CScript::IsNormalPaymentScript() const
 {
     if(this->size() != 25) return false;

src/script/script.h

@@ -662,6 +662,7 @@ class CScript : public CScriptBase
     unsigned int GetSigOpCount(const CScript& scriptSig) const;
     bool IsNormalPaymentScript() const;
 
+    bool IsPayToPublicKey() const;
     bool IsPayToPublicKeyHash() const;
     bool IsPayToExchangeAddress() const;
 

src/sync.cpp

@@ -18,7 +18,7 @@
 #include "utilstrencodings.h"
 
 #include <stdio.h>
-
+#include <set>
 #include <boost/foreach.hpp>
 #include <boost/thread.hpp>
 
@@ -67,7 +67,7 @@ struct CLockLocation {
 
 typedef std::vector<std::pair<void*, CLockLocation> > LockStack;
 typedef std::map<std::pair<void*, void*>, LockStack> LockOrders;
-typedef std::set<std::pair<void*, void*> > InvLockOrders;
+typedef std::set<std::pair<void*, void*>> InvLockOrders;
 
 struct LockData {
     // Very ugly hack: as the global constructs and destructors run single
@@ -174,7 +174,7 @@ void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLi
         if (i.first == cs) {
             fprintf(stderr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str());
             abort();
-        }
+       }
     }
 }
 

src/test/test_bitcoin.cpp

@@ -109,8 +109,8 @@ TestingSetup::TestingSetup(const std::string& chainName, std::string suf) : Basi
 
         // Init HD mint
 
-        // Create new keyUser and set as default key
-        // generate a new master key
+        pwalletMain->GenerateNewMnemonic();
+
         CPubKey masterPubKey = pwalletMain->GenerateNewHDMasterKey();
         pwalletMain->SetHDMasterKey(masterPubKey);
         CPubKey newDefaultKey;

src/validation.cpp

@@ -1196,6 +1196,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
                     if (pfMissingInputs) {
                         *pfMissingInputs = true;
                     }
+                    LogPrintf("%s(): Couldn't find input %s-%d for tx %s\n", __func__, txin.prevout.hash.GetHex(), txin.prevout.n, tx.GetHash().GetHex());
                     return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid()
                 }
             }

src/wallet/coincontrol.h

@@ -6,6 +6,8 @@
 #define BITCOIN_WALLET_COINCONTROL_H
 
 #include "primitives/transaction.h"
+#include "script/standard.h"
+#include <optional>
 
 enum class CoinType
 {
@@ -16,7 +18,8 @@ enum class CoinType
     ONLY_1000 = 5, // find znode outputs including locked ones (use with caution)
     ONLY_PRIVATESEND_COLLATERAL = 6,
     ONLY_MINTS = 7,
-    WITH_MINTS = 8
+    WITH_MINTS = 8,
+    WITH_1000 = 9
 };
 
 /** Coin Control Features. */
@@ -36,12 +39,18 @@ class CCoinControl
     CAmount nMinimumTotalFee;
     //! Override estimated feerate
     bool fOverrideFeeRate;
+    // Allow inputs from ourself that haven't been confirmed yet.
+    std::optional<bool> fAllowUnconfirmed;
     //! Feerate to use if overrideFeeRate is true
     CFeeRate nFeeRate;
     //! Override the default confirmation target, 0 = use default
     int nConfirmTarget;
     //! Controls which types of coins are allowed to be used (default: ALL_COINS)
     CoinType nCoinType;
+    //! No more than this number of inputs may be used.
+    size_t nMaxInputs;
+    // The generated transaction may not be over this size.
+    size_t nMaxSize;
 
     CCoinControl()
     {
@@ -61,6 +70,8 @@ class CCoinControl
         fOverrideFeeRate = false;
         nConfirmTarget = 0;
         nCoinType = CoinType::ALL_COINS;
+        nMaxInputs = 0;
+        nMaxSize = 0;
     }
 
     bool HasSelected() const
@@ -93,6 +104,11 @@ class CCoinControl
         vOutpoints.assign(setSelected.begin(), setSelected.end());
     }
 
+    size_t GetSelectedSize() const
+    {
+        return setSelected.size();
+    }
+
 private:
     std::set<COutPoint> setSelected;
 };

src/wallet/rpcwallet.cpp

@@ -3081,6 +3081,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
         setSubtractFeeFromOutputs.insert(pos);
     }
 
+    LOCK2(cs_main, pwallet->cs_wallet);
+
     CAmount nFeeOut;
     std::string strFailReason;
 

src/wallet/sigmaspendbuilder.cpp

@@ -18,7 +18,7 @@
 #include <stdexcept>
 #include <tuple>
 
-class SigmaSpendSigner : public InputSigner
+class SigmaSpendSigner : public SigmaTxBuilderInputSigner
 {
 public:
     const sigma::PrivateCoin coin;
@@ -108,7 +108,7 @@ static std::unique_ptr<SigmaSpendSigner> CreateSigner(const CSigmaEntry& coin)
 }
 
 SigmaSpendBuilder::SigmaSpendBuilder(CWallet& wallet, CHDMintWallet& mintWallet, const CCoinControl *coinControl) :
-    TxBuilder(wallet),
+    SigmaTxBuilderSuperclass(wallet),
     mintWallet(mintWallet)
 {
     cs_main.lock();
@@ -129,7 +129,7 @@ SigmaSpendBuilder::~SigmaSpendBuilder()
     cs_main.unlock();
 }
 
-CAmount SigmaSpendBuilder::GetInputs(std::vector<std::unique_ptr<InputSigner>>& signers, CAmount required)
+CAmount SigmaSpendBuilder::GetInputs(std::vector<std::unique_ptr<SigmaTxBuilderInputSigner>>& signers, CAmount required)
 {
     // get coins to spend
 

src/wallet/sigmaspendbuilder.h

@@ -7,7 +7,7 @@
 
 #include <vector>
 
-class SigmaSpendBuilder : public TxBuilder
+class SigmaSpendBuilder : public SigmaTxBuilderSuperclass
 {
 public:
     std::vector<CSigmaEntry> selected;
@@ -19,7 +19,7 @@ class SigmaSpendBuilder : public TxBuilder
     ~SigmaSpendBuilder() override;
 
 protected:
-    CAmount GetInputs(std::vector<std::unique_ptr<InputSigner>>& signers, CAmount required) override;
+    CAmount GetInputs(std::vector<std::unique_ptr<SigmaTxBuilderInputSigner>>& signers, CAmount required) override;
     // remint change
     CAmount GetChanges(std::vector<CTxOut>& outputs, CAmount amount, CWalletDB& walletdb) override;