From 0aca3d75ae0386c2f81f6020776bdcb58dc1823d Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Sat, 11 Nov 2023 16:13:58 +0700 Subject: [PATCH 01/24] Refactor transaction creation. --- src/Makefile.test.include | 3 +- src/llmq/quorums_instantsend.h | 6 +- src/primitives/transaction.h | 2 +- src/qt/walletmodel.cpp | 2 +- src/rpc/rpcevo.cpp | 2 +- src/script/script.cpp | 10 + src/script/script.h | 1 + src/sync.cpp | 6 +- src/test/test_bitcoin.cpp | 4 +- src/validation.cpp | 1 + src/wallet/coincontrol.h | 14 +- src/wallet/rpcwallet.cpp | 4 +- src/wallet/sigmaspendbuilder.cpp | 6 +- src/wallet/sigmaspendbuilder.h | 4 +- src/wallet/test/createtransaction_tests.cpp | 1204 +++++++++++++++++ ...der_tests.cpp => sigmatxbuilder_tests.cpp} | 20 +- src/wallet/txbuilder.cpp | 17 +- src/wallet/txbuilder.h | 17 +- src/wallet/wallet.cpp | 795 ++++++----- src/wallet/wallet.h | 421 +++++- 20 files changed, 2131 insertions(+), 408 deletions(-) create mode 100644 src/wallet/test/createtransaction_tests.cpp rename src/wallet/test/{txbuilder_tests.cpp => sigmatxbuilder_tests.cpp} (90%) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 2abce15d3c..16150512b8 100644 --- a/src/Makefile.test.include +++ b/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 diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index fd73d06689..398c99511f 100644 --- a/src/llmq/quorums_instantsend.h +++ b/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; @@ -113,6 +110,9 @@ class CInstantSendManager : public CRecoveredSigsListener std::atomic_bool isNewInstantSendEnabled{false}; public: + CCriticalSection cs; + CInstantSendDb db; + CInstantSendManager(CDBWrapper& _llmqDb); ~CInstantSendManager(); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index ed9bee3465..5f8a4062c4 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -171,7 +171,7 @@ class CTxOut public: CAmount nValue; CScript scriptPubKey; - int nRounds; + int nRounds = -10; CTxOut() { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 66a8538941..898bc3e2af 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -341,7 +341,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact CWalletTx *newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); - bool fCreated = wallet->CreateTransaction(vecSend, *newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); + bool fCreated = wallet->CreateTransaction(vecSend, *newTx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && fCreated) transaction.reassignAmounts(nChangePosRet); diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index 6037b4835f..716aaf163f 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -213,7 +213,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci int nChangePos = -1; std::string strFailReason; - if (!pwallet->CreateTransaction(vecSend, wtx, reservekey, nFee, nChangePos, strFailReason, &coinControl, false, tx.vExtraPayload.size())) { + if (!pwallet->CreateTransaction(vecSend, wtx, &reservekey, nFee, nChangePos, strFailReason, &coinControl, false, tx.vExtraPayload.size())) { throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason); } diff --git a/src/script/script.cpp b/src/script/script.cpp index 5f7a5951ad..a451b62f9e 100644 --- a/src/script/script.cpp +++ b/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; diff --git a/src/script/script.h b/src/script/script.h index 3462581956..0c06889042 100644 --- a/src/script/script.h +++ b/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; diff --git a/src/sync.cpp b/src/sync.cpp index a7450a013b..f57655f9ac 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -18,7 +18,7 @@ #include "utilstrencodings.h" #include - +#include #include #include @@ -67,7 +67,7 @@ struct CLockLocation { typedef std::vector > LockStack; typedef std::map, LockStack> LockOrders; -typedef std::set > InvLockOrders; +typedef std::set> 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(); - } + } } } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 870d1be3ae..9c7a60f365 100644 --- a/src/test/test_bitcoin.cpp +++ b/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; diff --git a/src/validation.cpp b/src/validation.cpp index abcb55e0ff..b6b9396016 100644 --- a/src/validation.cpp +++ b/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() } } diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index feed073c26..218d90776d 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -16,13 +16,16 @@ 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. */ class CCoinControl { public: + std::set setSelected; + CTxDestination destChange; //! If true, don't use any change bool fNoChange; @@ -42,6 +45,10 @@ class CCoinControl 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 +68,8 @@ class CCoinControl fOverrideFeeRate = false; nConfirmTarget = 0; nCoinType = CoinType::ALL_COINS; + nMaxInputs = 0; + nMaxSize = 0; } bool HasSelected() const @@ -92,9 +101,6 @@ class CCoinControl { vOutpoints.assign(setSelected.begin(), setSelected.end()); } - -private: - std::set setSelected; }; #endif // BITCOIN_WALLET_COINCONTROL_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a5ade6b12f..8548c2bca1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -509,7 +509,7 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA int nChangePosRet = -1; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); - if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, nChangePosRet, strError)) { + if (!pwallet->CreateTransaction(vecSend, wtxNew, &reservekey, nFeeRequired, nChangePosRet, strError)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); throw JSONRPCError(RPC_WALLET_ERROR, strError); @@ -1266,7 +1266,7 @@ UniValue sendmany(const JSONRPCRequest& request) CAmount nFeeRequired = 0; int nChangePosRet = -1; std::string strFailReason; - bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason); + bool fCreated = pwallet->CreateTransaction(vecSend, wtx, &keyChange, nFeeRequired, nChangePosRet, strFailReason); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; diff --git a/src/wallet/sigmaspendbuilder.cpp b/src/wallet/sigmaspendbuilder.cpp index cda66d6986..3aac9667b5 100644 --- a/src/wallet/sigmaspendbuilder.cpp +++ b/src/wallet/sigmaspendbuilder.cpp @@ -18,7 +18,7 @@ #include #include -class SigmaSpendSigner : public InputSigner +class SigmaSpendSigner : public SigmaTxBuilderInputSigner { public: const sigma::PrivateCoin coin; @@ -108,7 +108,7 @@ static std::unique_ptr 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>& signers, CAmount required) +CAmount SigmaSpendBuilder::GetInputs(std::vector>& signers, CAmount required) { // get coins to spend diff --git a/src/wallet/sigmaspendbuilder.h b/src/wallet/sigmaspendbuilder.h index f05c49d030..933e82b7aa 100644 --- a/src/wallet/sigmaspendbuilder.h +++ b/src/wallet/sigmaspendbuilder.h @@ -7,7 +7,7 @@ #include -class SigmaSpendBuilder : public TxBuilder +class SigmaSpendBuilder : public SigmaTxBuilderSuperclass { public: std::vector selected; @@ -19,7 +19,7 @@ class SigmaSpendBuilder : public TxBuilder ~SigmaSpendBuilder() override; protected: - CAmount GetInputs(std::vector>& signers, CAmount required) override; + CAmount GetInputs(std::vector>& signers, CAmount required) override; // remint change CAmount GetChanges(std::vector& outputs, CAmount amount, CWalletDB& walletdb) override; diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp new file mode 100644 index 0000000000..2a9034778c --- /dev/null +++ b/src/wallet/test/createtransaction_tests.cpp @@ -0,0 +1,1204 @@ +#include "amount.h" +#include "base58.h" +#include "primitives/transaction.h" +#include "script/standard.h" +#include "serialize.h" +#include "util.h" +#include "wallet/wallet.h" +#include "validation.h" +#include "llmq/quorums_instantsend.h" +#include "wallet/test/wallet_test_fixture.h" +#include "wallet/coincontrol.h" +#include + +std::string GetAddress(const CTxDestination& dest) { + CBitcoinAddress addr; + addr.Set(dest); + return addr.ToString(); +} + +std::string GetAddress(const CScript& script) { + CTxDestination dest; + ExtractDestination(script, dest); + return GetAddress(dest); +} + +CTransparentTxout GetFakeTransparentTxout(const CScript& dest, CAmount value, bool isMine) { + uint256 hash; + GetRandBytes((unsigned char*)&hash, sizeof(hash)); + + CTxOut txout(value, dest); + COutPoint outpoint(hash, GetRand(128)); + + CTransparentTxout transparentTxout(outpoint, txout); + transparentTxout._mockupIsMine = isMine; + transparentTxout._mockupDepthInMainChain = 1; + return transparentTxout; +} + +CScript GetRandomDest() { + CKey key; + key.MakeNewKey(true); + + return GetScriptForDestination(key.GetPubKey().GetID()); +} + +CScript GetRandomWalletAddress() { + AssertLockHeld(pwalletMain->cs_wallet); + + CPubKey key; + pwalletMain->GetKeyFromPool(key); + return GetScriptForDestination(key.GetID()); +} + +CScript GetRandomMultisigAddress(size_t nOurs, size_t n, size_t m) { + AssertLockHeld(pwalletMain->cs_wallet); + assert(nOurs <= m); + assert(m >= n); + assert(n); + + std::vector keys; + + for (size_t i = 0; i < nOurs; i++) { + CPubKey key; + pwalletMain->GetKeyFromPool(key); + keys.emplace_back(key); + } + + for (size_t i = nOurs; i < m; i++) { + CKey key; + key.MakeNewKey(true); + keys.emplace_back(key.GetPubKey()); + } + + return GetScriptForMultisig(n, keys); +} + +CTransparentTxout GetFakeTransparentTxout(CAmount value, bool isMine=true) { + if (isMine) + return GetFakeTransparentTxout(GetRandomWalletAddress(), value, isMine); + else + return GetFakeTransparentTxout(GetRandomDest(), value, isMine); +} + +std::vector GetFakeTransparentTxouts(std::vector values) { + std::vector vTxouts; + for (CAmount value: values) { + vTxouts.emplace_back(GetFakeTransparentTxout(value)); + } + return vTxouts; +} + +std::vector GetFakeRecipients(std::vector values) { + std::vector vRecipients; + for (CAmount value: values) { + vRecipients.emplace_back(CRecipient{GetRandomDest(), value, false}); + } + return vRecipients; +} + +void AssertVinValue(const std::vector& vTxouts, const CWalletTx& wtx, uint32_t n, CAmount value) { + for (const CTransparentTxout& txout: vTxouts) { + if (txout.GetOutpoint() == COutPoint(wtx.tx->vin[n].prevout)) { + BOOST_ASSERT(txout.GetValue() == value); + return; + } + } + BOOST_ASSERT(false); +} + +void AssertVoutAddr(const std::vector& vRecipients, const CWalletTx& wtx, uint32_t voutN, uint32_t recipientN) { + BOOST_ASSERT(wtx.tx->vout.at(voutN).scriptPubKey == vRecipients.at(recipientN).scriptPubKey); +} + +void AssertVoutValue(const CWalletTx& wtx, uint32_t n, CAmount value) { + BOOST_ASSERT(wtx.tx->vout.at(n).nValue == value); +} + +void AssertHasKey(CReserveKey& reservekey, const CWalletTx& wtx, uint32_t voutN) { + AssertLockHeld(pwalletMain->cs_wallet); + + reservekey.KeepKey(); + + BOOST_ASSERT(pwalletMain->IsMine(wtx.tx->vout.at(voutN))); +} + +#define ASSERT_VIN_VALUE(vinN, value) AssertVinValue(vTxouts, wtx, vinN, value) +#define ASSERT_VOUT_ADDR(voutN, addrN) AssertVoutAddr(vRecipients, wtx, voutN, addrN) +#define ASSERT_VIN_SIZE(sizeN) BOOST_ASSERT(wtx.tx->vin.size() == sizeN) +#define ASSERT_VOUT_SIZE(sizeN) BOOST_ASSERT(wtx.tx->vout.size() == sizeN) +#define ASSERT_VOUT_VALUE(voutN, value) AssertVoutValue(wtx, voutN, value) +#define ASSERT_VOUT_ADDR_VALUE(voutN, addrN, value) \ + ASSERT_VOUT_ADDR(voutN, addrN); \ + ASSERT_VOUT_VALUE(voutN, value) +#define ASSERT_HAS_KEY(voutN) AssertHasKey(reservekey, wtx, voutN) +#define ASSERT_SUCCESS() if (!strFailReason.empty()) BOOST_FAIL(strFailReason) +#define ASSERT_FAILURE(reason) BOOST_ASSERT(strFailReason == reason) +#define ACQUIRE_LOCKS() LOCK2(cs_main, pwalletMain->cs_wallet);\ + LOCK(llmq::quorumInstantSendManager->cs); + +BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) + BOOST_AUTO_TEST_CASE(sends_money) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 20}); + std::vector vRecipients = GetFakeRecipients({1 << 19}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 20); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 19); + ASSERT_VOUT_VALUE(1, (1 << 20) - (1 << 19) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(multiple_recipients) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 20}); + std::vector vRecipients = GetFakeRecipients({1 << 19, 1 << 18, 1 << 17, 1 << 16, 1 << 15}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 20); + ASSERT_VOUT_SIZE(6); + for (uint32_t i = 0; i < 5; i++) { + ASSERT_VOUT_ADDR_VALUE(i, i, 1 << (19 - i)); + } + ASSERT_VOUT_VALUE(5, (1 << 20) - (1 << 19) - (1 << 18) - (1 << 17) - (1 << 16) - (1 << 15) - nFeeRet); + ASSERT_HAS_KEY(5); + } + } + + BOOST_AUTO_TEST_CASE(mints_change) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 20}); + std::vector vRecipients = GetFakeRecipients({1 << 19, 1 << 18, 1 << 17, 1 << 16, 1 << 15}); + + CAmount changeOutputValue = 0; + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 20); + ASSERT_VOUT_SIZE(6); + for (uint32_t i = 0; i < 5; i++) { + ASSERT_VOUT_ADDR_VALUE(i, i, 1 << (19 - i)); + } + changeOutputValue = (1 << 20) - (1 << 19) - (1 << 18) - (1 << 17) - (1 << 16) - (1 << 15) - nFeeRet; + ASSERT_VOUT_VALUE(5, changeOutputValue); + ASSERT_HAS_KEY(5); + } + + vRecipients = GetFakeRecipients({1 << 19, 1 << 18, 1 << 17, 1 << 16, 1 << 15, changeOutputValue - 30}); + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + BOOST_ASSERT(nChangePosInOut == -1); + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 20); + ASSERT_VOUT_SIZE(6); + for (uint32_t i = 0; i < 5; i++) { + ASSERT_VOUT_ADDR_VALUE(i, i, 1 << (19 - i)); + } + ASSERT_VOUT_ADDR_VALUE(5, 5, changeOutputValue - 30); + } + } + + BOOST_AUTO_TEST_CASE(selects_smallest_input_required) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15}); + std::vector vRecipients = GetFakeRecipients({1 << 15}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 16); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 15); + ASSERT_VOUT_VALUE(1, (1 << 16) - (1 << 15) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(insufficient_funds) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 15}); + std::vector vRecipients = GetFakeRecipients({1 << 15}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + vTxouts = GetFakeTransparentTxouts({1 << 16}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 16); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 15); + ASSERT_VOUT_VALUE(1, (1 << 16) - (1 << 15) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(no_unconfirmed_inputs) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15}); + std::vector vRecipients = GetFakeRecipients({1 << 17}); + + vTxouts.at(0)._mockupDepthInMainChain = 0; + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + vTxouts.at(0)._mockupDepthInMainChain = 1; + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 15); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + ASSERT_VOUT_VALUE(1, (1 << 15) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(takes_from_front_and_back) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({(1 << 17) + (1 << 16) + (1 << 14)}); + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(4); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 16); + ASSERT_VIN_VALUE(3, 1 << 14); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, (1 << 17) + (1 << 16) + (1 << 14)); + ASSERT_VOUT_VALUE(1, (1 << 13) - nFeeRet); + } + } + + BOOST_AUTO_TEST_CASE(doesnt_select_used_inputs) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 16}); + + vTxouts.at(0)._mockupIsSpent = true; + vTxouts.at(4)._mockupIsSpent = true; + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 16); + ASSERT_VIN_VALUE(1, 1 << 14); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 16); + ASSERT_VOUT_VALUE(1, (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(instantsend) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 17}); + + vTxouts.at(0)._mockupDepthInMainChain = 0; + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + vTxouts.at(0)._mockupIsLLMQInstantSendLocked = true; + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + ASSERT_VOUT_VALUE(1, (1 << 13) - nFeeRet); + ASSERT_HAS_KEY(1); + } + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, false, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + } + + BOOST_AUTO_TEST_CASE(change_position) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 17, 1 << 13, 1 << 16}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + BOOST_ASSERT(nChangePosInOut == 3); + ASSERT_VIN_SIZE(4); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 16); + ASSERT_VIN_VALUE(3, 1 << 14); + ASSERT_VOUT_SIZE(4); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + ASSERT_VOUT_ADDR_VALUE(1, 1, 1 << 13); + ASSERT_VOUT_ADDR_VALUE(2, 2, 1 << 16); + ASSERT_VOUT_VALUE(3, (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(3); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = 0; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + BOOST_ASSERT(nChangePosInOut == 0); + ASSERT_VIN_SIZE(4); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 16); + ASSERT_VIN_VALUE(3, 1 << 14); + ASSERT_VOUT_SIZE(4); + ASSERT_VOUT_VALUE(0, (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(0); + ASSERT_VOUT_ADDR_VALUE(1, 0, 1 << 17); + ASSERT_VOUT_ADDR_VALUE(2, 1, 1 << 13); + ASSERT_VOUT_ADDR_VALUE(3, 2, 1 << 16); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = 2; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + BOOST_ASSERT(nChangePosInOut == 2); + ASSERT_VIN_SIZE(4); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 16); + ASSERT_VIN_VALUE(3, 1 << 14); + ASSERT_VOUT_SIZE(4); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + ASSERT_VOUT_ADDR_VALUE(1, 1, 1 << 13); + ASSERT_VOUT_VALUE(2, (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(2); + ASSERT_VOUT_ADDR_VALUE(3, 2, 1 << 16); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = 7; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + BOOST_ASSERT(nChangePosInOut == -1); + ASSERT_FAILURE("Change index out of range"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -5; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + BOOST_ASSERT(nChangePosInOut == -1); + ASSERT_FAILURE("Change index out of range"); + } + } + + BOOST_AUTO_TEST_CASE(watch_only_address) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 16}); + + vTxouts.emplace_back(GetFakeTransparentTxout(1 << 17, false)); + vTxouts.at(3)._mockupIsMineWatchOnly = true; + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + BOOST_ASSERT(strFailReason == "Insufficient funds"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.fAllowWatchOnly = true; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, false, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 16); + ASSERT_VOUT_VALUE(1, (1 << 17) - (1 << 16) - nFeeRet); + ASSERT_HAS_KEY(1); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.fAllowWatchOnly = true; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Signing transaction failed"); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_coin_type) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1000 * COIN}); + std::vector vRecipients = GetFakeRecipients({999 * COIN}); + + + { + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + nullptr, true, 0, true, vTxouts); + + + ASSERT_FAILURE("Insufficient funds"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nCoinType = CoinType::ONLY_1000; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1000 * COIN); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 999 * COIN); + ASSERT_VOUT_VALUE(1, (1 * COIN) - nFeeRet); + ASSERT_HAS_KEY(1); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED_NOT1000IFMN; + + bool fMasternodeModeTemp = fMasternodeMode; + fMasternodeMode = false; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("fMasternode must be enabled to use CoinType::ONLY_NONDENOMINATED_NOT1000IFMN"); + + fMasternodeMode = fMasternodeModeTemp; + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED_NOT1000IFMN; + + bool fMasternodeModeTemp = fMasternodeMode; + fMasternodeMode = true; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_FAILURE("Insufficient funds"); + + fMasternodeMode = fMasternodeModeTemp; + } + + vTxouts = GetFakeTransparentTxouts({1001 * COIN, 1000 * COIN}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nCoinType = CoinType::ONLY_NONDENOMINATED_NOT1000IFMN; + + bool fMasternodeModeTemp = fMasternodeMode; + fMasternodeMode = true; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1001 * COIN); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 999 * COIN); + ASSERT_VOUT_VALUE(1, 2 * COIN - nFeeRet); + ASSERT_HAS_KEY(1); + + fMasternodeMode = fMasternodeModeTemp; + } + + vRecipients = GetFakeRecipients({2000 * COIN}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nCoinType = CoinType::WITH_1000; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1001 * COIN); + ASSERT_VIN_VALUE(1, 1000 * COIN); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 2000 * COIN); + ASSERT_VOUT_VALUE(1, 1 * COIN - nFeeRet); + ASSERT_HAS_KEY(1); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.Select(vTxouts.at(1).GetOutpoint()); + coinControl.nCoinType = CoinType::WITH_1000; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.Select(vTxouts.at(1).GetOutpoint()); + coinControl.fAllowOtherInputs = true; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Some coin control inputs could not be selected."); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_input_count_limit) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({(1 << 17) + (1 << 15)}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nFeeRate = CFeeRate(1000); + coinControl.nMaxInputs = 2; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nFeeRate = CFeeRate(1000); + coinControl.nMaxInputs = 3; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(3); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 15); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, (1 << 17) + (1 << 15)); + ASSERT_VOUT_VALUE(1, (1 << 13) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_transaction_size_limit) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({(1 << 17) + (1 << 14)}); + + size_t n3TxSize = 0; + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nFeeRate = CFeeRate(1000); + coinControl.nMaxInputs = 3; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(3); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VIN_VALUE(2, 1 << 15); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, (1 << 17) + (1 << 14)); + ASSERT_VOUT_VALUE(1, (1 << 15) + (1 << 13) - (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(1); + + // nFeeRet is based on the transaction size with maximum-length signatures, which is what is used as the + // transaction size when calculating the transaction size for input selection. It may be slightly greater + // than the final transaction size. + BOOST_ASSERT(nFeeRet >= ::GetSerializeSize(*wtx.tx, SER_NETWORK, PROTOCOL_VERSION)); + n3TxSize = nFeeRet; + } + + size_t n2TxSize = 0; + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nFeeRate = CFeeRate(1000); + coinControl.nMaxSize = n3TxSize - 1; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 15); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, (1 << 17) + (1 << 14)); + ASSERT_VOUT_VALUE(1, (1 << 15) - (1 << 14) - nFeeRet); + ASSERT_HAS_KEY(1); + + BOOST_ASSERT(nFeeRet >= ::GetSerializeSize(*wtx.tx, SER_NETWORK, PROTOCOL_VERSION)); + n2TxSize = nFeeRet; + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nFeeRate = CFeeRate(1000); + coinControl.nMaxSize = n2TxSize - 1; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_destination_address) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 16}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.destChange = GetRandomDest(); + + pwalletMain->CreateTransaction(vRecipients, wtx, nullptr, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 16); + ASSERT_VOUT_VALUE(1, (1 << 17) - (1 << 16) - nFeeRet); + BOOST_ASSERT(GetAddress(wtx.tx->vout.at(1).scriptPubKey) == GetAddress(coinControl.destChange)); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_minimum_total_fee) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 17}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nMinimumTotalFee = 1 << 13; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + BOOST_ASSERT(nFeeRet == 1 << 13); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VOUT_SIZE(1); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nMinimumTotalFee = 1 << 20; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nMinimumTotalFee = 100; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + BOOST_ASSERT(nFeeRet > 100); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 13); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 17); + ASSERT_VOUT_VALUE(1, (1 << 13) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_confirm_target) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1 << 16}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nConfirmTarget = 5; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_FAILURE("Insufficient funds"); + } + + vTxouts.at(0)._mockupDepthInMainChain = 5; + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.nConfirmTarget = 5; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1 << 16); + ASSERT_VOUT_VALUE(1, (1 << 17) - (1 << 16) - nFeeRet); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(coincontrol_require_all_inputs) { + ACQUIRE_LOCKS(); + + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 15, 1 << 14, 1 << 13}); + std::vector vRecipients = GetFakeRecipients({1}); + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.fRequireAllInputs = true; + coinControl.Select(vTxouts.at(0).GetOutpoint()); + coinControl.Select(vTxouts.at(1).GetOutpoint()); + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(2); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VIN_VALUE(1, 1 << 15); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1); + ASSERT_VOUT_VALUE(1, (1 << 17) + (1 << 15) - nFeeRet - 1); + ASSERT_HAS_KEY(1); + } + + { + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + coinControl.fRequireAllInputs = false; + coinControl.Select(vTxouts.at(0).GetOutpoint()); + coinControl.Select(vTxouts.at(1).GetOutpoint()); + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + &coinControl, true, 0, true, vTxouts); + + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, 1 << 17); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, 1); + ASSERT_VOUT_VALUE(1, (1 << 17) - nFeeRet - 1); + ASSERT_HAS_KEY(1); + } + } + + BOOST_AUTO_TEST_CASE(multisig) { + ACQUIRE_LOCKS(); + + std::vector vRecipients = GetFakeRecipients({CENT}); + + for (size_t n = 1; n < 5; n++) { + for (size_t m = n; m <= 5; m++) { + for (size_t nOurs = 0; nOurs <= m; nOurs++) { + std::vector vTxouts = + {GetFakeTransparentTxout(GetRandomMultisigAddress(nOurs, n, m), COIN, true)}; + + CCoinControl coinControl; + CWalletTx wtx; + CAmount nFeeRet = 0; + int nChangePosInOut = -1; + std::string strFailReason; + + CReserveKey reservekey(pwalletMain); + pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, + strFailReason, &coinControl, true, 0, true, vTxouts); + + if (nOurs >= n) { + ASSERT_SUCCESS(); + ASSERT_VIN_SIZE(1); + ASSERT_VIN_VALUE(0, COIN); + ASSERT_VOUT_SIZE(2); + ASSERT_VOUT_ADDR_VALUE(0, 0, CENT); + ASSERT_VOUT_VALUE(1, COIN - CENT - nFeeRet); + ASSERT_HAS_KEY(1); + } else { + ASSERT_FAILURE("Signing transaction failed"); + } + } + } + } + } +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/txbuilder_tests.cpp b/src/wallet/test/sigmatxbuilder_tests.cpp similarity index 90% rename from src/wallet/test/txbuilder_tests.cpp rename to src/wallet/test/sigmatxbuilder_tests.cpp index 9eae1c407b..439d1a699f 100644 --- a/src/wallet/test/txbuilder_tests.cpp +++ b/src/wallet/test/sigmatxbuilder_tests.cpp @@ -12,7 +12,7 @@ static const CBitcoinAddress randomAddr1("aHEog3QYDGa8wH4Go9igKLDFkpaMsi3btq"); static const CBitcoinAddress randomAddr2("aLTSv7QbTZbkgorYEhbNx2gH4hGYNLsoGv"); -class TestInputSigner : public InputSigner +class TestInputSigner : public SigmaTxBuilderInputSigner { public: CScript signature; @@ -23,7 +23,7 @@ class TestInputSigner : public InputSigner } explicit TestInputSigner(const CScript& sig, const COutPoint& output = COutPoint(), uint32_t seq = CTxIn::SEQUENCE_FINAL) : - InputSigner(output, seq), + SigmaTxBuilderInputSigner(output, seq), signature(sig) { } @@ -34,24 +34,24 @@ class TestInputSigner : public InputSigner } }; -class TestTxBuilder : public TxBuilder +class TestTxBuilder : public SigmaTxBuilderSuperclass { public: std::vector amountsRequested; std::vector changesRequested; std::vector> adjustFeeRequested; - std::function>& signers, CAmount required)> getInputs; + std::function>& signers, CAmount required)> getInputs; std::function& outputs, CAmount amount, CWalletDB& walletdb)> getChanges; std::function adjustFee; public: - explicit TestTxBuilder(CWallet& wallet) : TxBuilder(wallet) + explicit TestTxBuilder(CWallet& wallet) : SigmaTxBuilderSuperclass(wallet) { } protected: - CAmount GetInputs(std::vector>& signers, CAmount required) override + CAmount GetInputs(std::vector>& signers, CAmount required) override { amountsRequested.push_back(required); @@ -69,7 +69,7 @@ class TestTxBuilder : public TxBuilder { adjustFeeRequested.push_back(std::make_pair(needed, txSize)); - return adjustFee ? adjustFee(needed, txSize) : TxBuilder::AdjustFee(needed, txSize); + return adjustFee ? adjustFee(needed, txSize) : SigmaTxBuilderSuperclass::AdjustFee(needed, txSize); } }; @@ -184,9 +184,9 @@ BOOST_AUTO_TEST_CASE(build_with_changes) in1 << std::vector({ 0x21, 0xe3, 0xad, 0x9a, 0xec, 0x5b, 0x70, 0xcb, 0x4c, 0xc1, 0xd8, 0xe2, 0x95, 0x27, 0xe3, 0x7c }); in2 << std::vector({ 0xac, 0xd9, 0x86, 0x7d, 0xd7, 0x6e, 0xc1, 0xb7, 0x9d, 0xde, 0xdc, 0xbd, 0x91, 0xc1, 0x8e, 0xed }); - builder.getInputs = [&in1, &in2, &out1, &out2](std::vector>& signers, CAmount required) { - signers.push_back(std::unique_ptr(new TestInputSigner(in1, out1, 1))); - signers.push_back(std::unique_ptr(new TestInputSigner(in2, out2, 2))); + builder.getInputs = [&in1, &in2, &out1, &out2](std::vector>& signers, CAmount required) { + signers.push_back(std::unique_ptr(new TestInputSigner(in1, out1, 1))); + signers.push_back(std::unique_ptr(new TestInputSigner(in2, out2, 2))); return required + 5; }; diff --git a/src/wallet/txbuilder.cpp b/src/wallet/txbuilder.cpp index 637bc26a14..28ed917786 100644 --- a/src/wallet/txbuilder.cpp +++ b/src/wallet/txbuilder.cpp @@ -19,27 +19,28 @@ #include #include -InputSigner::InputSigner() : InputSigner(COutPoint()) +SigmaTxBuilderInputSigner::SigmaTxBuilderInputSigner() : SigmaTxBuilderInputSigner(COutPoint()) { } -InputSigner::InputSigner(const COutPoint& output, uint32_t seq) : output(output), sequence(seq) +SigmaTxBuilderInputSigner::SigmaTxBuilderInputSigner(const COutPoint& output, uint32_t seq) : output(output), sequence(seq) { } -InputSigner::~InputSigner() +SigmaTxBuilderInputSigner::~SigmaTxBuilderInputSigner() { } -TxBuilder::TxBuilder(CWallet& wallet) noexcept : wallet(wallet) +SigmaTxBuilderSuperclass::SigmaTxBuilderSuperclass(CWallet& wallet) noexcept : wallet(wallet) { } -TxBuilder::~TxBuilder() +SigmaTxBuilderSuperclass::~SigmaTxBuilderSuperclass() { } -CWalletTx TxBuilder::Build(const std::vector& recipients, CAmount& fee, bool& fChangeAddedToFee, CWalletDB& walletdb) +// This is NOT what is used for spending. It's only used in legacy Sigma code. +CWalletTx SigmaTxBuilderSuperclass::Build(const std::vector& recipients, CAmount& fee, bool& fChangeAddedToFee, CWalletDB& walletdb) { if (recipients.empty()) { throw std::invalid_argument(_("No recipients")); @@ -163,7 +164,7 @@ CWalletTx TxBuilder::Build(const std::vector& recipients, CAmount& f } // get inputs - std::vector> signers; + std::vector> signers; CAmount total = GetInputs(signers, required); // add changes @@ -264,7 +265,7 @@ CWalletTx TxBuilder::Build(const std::vector& recipients, CAmount& f return result; } -CAmount TxBuilder::AdjustFee(CAmount needed, unsigned txSize) +CAmount SigmaTxBuilderSuperclass::AdjustFee(CAmount needed, unsigned txSize) { return needed; } diff --git a/src/wallet/txbuilder.h b/src/wallet/txbuilder.h index 7266904a18..c970820211 100644 --- a/src/wallet/txbuilder.h +++ b/src/wallet/txbuilder.h @@ -13,34 +13,35 @@ #include -class InputSigner +class SigmaTxBuilderInputSigner { public: COutPoint output; uint32_t sequence; public: - InputSigner(); - explicit InputSigner(const COutPoint& output, uint32_t seq = CTxIn::SEQUENCE_FINAL); - virtual ~InputSigner(); + SigmaTxBuilderInputSigner(); + explicit SigmaTxBuilderInputSigner(const COutPoint& output, uint32_t seq = CTxIn::SEQUENCE_FINAL); + virtual ~SigmaTxBuilderInputSigner(); virtual CScript Sign(const CMutableTransaction& tx, const uint256& sig) = 0; }; -class TxBuilder +// This is only used for legacy Sigma code. Don't bother editing anything in it. +class SigmaTxBuilderSuperclass { public: CWallet& wallet; const CCoinControl *coinControl; public: - explicit TxBuilder(CWallet& wallet) noexcept; - virtual ~TxBuilder(); + explicit SigmaTxBuilderSuperclass(CWallet& wallet) noexcept; + virtual ~SigmaTxBuilderSuperclass(); CWalletTx Build(const std::vector& recipients, CAmount& fee, bool& fChangeAddedToFee, CWalletDB& walletdb); protected: - virtual CAmount GetInputs(std::vector>& signers, CAmount required) = 0; + virtual CAmount GetInputs(std::vector>& signers, CAmount required) = 0; virtual CAmount GetChanges(std::vector& outputs, CAmount amount, CWalletDB& walletdb) = 0; virtual CAmount AdjustFee(CAmount needed, unsigned txSize); }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 03bc7e6c52..50036cd3d9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "wallet.h" +#include "script/standard.h" #include "walletexcept.h" #include "sigmaspendbuilder.h" #include "lelantusjoinsplitbuilder.h" @@ -84,6 +85,181 @@ CFeeRate CWallet::minTxFee = CFeeRate(DEFAULT_TRANSACTION_MINFEE); */ CFeeRate CWallet::fallbackFee = CFeeRate(DEFAULT_FALLBACK_FEE); +bool CTransparentTxout::IsTransparentTxout(const CTxOut& txout) { + return txout.scriptPubKey.IsPayToPublicKey() || txout.scriptPubKey.IsPayToPublicKeyHash() || txout.scriptPubKey.IsPayToScriptHash(); +} + +uint256 CTransparentTxout::GetHash() const { + return outpoint.hash; +} + +COutPoint CTransparentTxout::GetOutpoint() const { + return outpoint; +} + +CAmount CTransparentTxout::GetValue() const { + assert(!txout.IsNull()); + return txout.nValue; +} + +CScript CTransparentTxout::GetScriptPubkey() const { + return txout.scriptPubKey; +} + +size_t CTransparentTxout::GetMarginalSpendSize(std::vector& previousInputs) const { + assert(!txout.IsNull()); + + txnouttype outType; + std::vector> vSolutions; + if (!Solver(txout.scriptPubKey, outType, vSolutions)) + return 0; + + // This is the size of scriptPubKey for the input. + size_t sigDataSize = 0; + switch (outType) { + case TX_MULTISIG: + sigDataSize = 1 + 73 * vSolutions.at(0).at(0); + break; + + case TX_PUBKEY: + sigDataSize = 101; + break; + + case TX_SCRIPTHASH: + case TX_PUBKEYHASH: + sigDataSize = 107; + break; + + case TX_NONSTANDARD: + case TX_NULL_DATA: + case TX_ZEROCOINMINT: + case TX_ZEROCOINMINTV3: + case TX_LELANTUSMINT: + case TX_LELANTUSJMINT: + case TX_WITNESS_V0_KEYHASH: + case TX_WITNESS_V0_SCRIPTHASH: + default: + throw std::runtime_error("Unsupported outType"); + } + + return + GetSizeOfCompactSize(previousInputs.size() + 1) - + GetSizeOfCompactSize(previousInputs.size()) + + 32 + // txid + 4 + // vout + GetSizeOfCompactSize(sigDataSize) + + sigDataSize + + 4; // sequence +} + +bool CTransparentTxout::IsMine(const CCoinControl* coinControl) const { + if (_isMockup) { + if (coinControl && coinControl->fAllowWatchOnly) return _mockupIsMine || _mockupIsMineWatchOnly; + else return _mockupIsMine; + } + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + bool isInvalid = false; + isminetype isMine = ::IsMine(*wallet, wallet->mapWallet.at(outpoint.hash).tx->vout.at(outpoint.n).scriptPubKey, isInvalid, SIGVERSION_BASE); + if (isInvalid) return false; + if (isMine == ISMINE_SPENDABLE) return true; + if (coinControl && coinControl->fAllowWatchOnly) + return isMine == ISMINE_WATCH_SOLVABLE; + return false; +} + +bool CTransparentTxout::IsSpendable() const { + if (_isMockup) + return !_mockupIsSpent; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + auto spendRange = wallet->mapTxSpends.equal_range(outpoint); + for (auto spendIt = spendRange.first; spendIt != spendRange.second; spendIt++) { + const auto& walletIt = wallet->mapWallet.find(spendIt->second); + if (walletIt != wallet->mapWallet.end()) { + int depth = walletIt->second.GetDepthInMainChain(); + if (depth > 0 || (depth == 0 && !walletIt->second.isAbandoned())) + return false; + } + } + + return true; +} + +bool CTransparentTxout::IsLocked() const { + if (_isMockup) + return _mockupIsLocked; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + return wallet->setLockedCoins.count(outpoint) > 0; +} + +bool CTransparentTxout::IsAbandoned() const { + if (_isMockup) + return _mockupIsAbandoned; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + return wallet->mapWallet.at(GetHash()).isAbandoned(); +} + +bool CTransparentTxout::IsCoinTypeCompatible(const CCoinControl* coinControl) const { + assert(!txout.IsNull()); + + if (!coinControl) + return GetValue() != 1000 * COIN; + else if (coinControl->nCoinType == CoinType::ONLY_MINTS) + return false; + else if (coinControl->nCoinType == CoinType::ONLY_NONDENOMINATED_NOT1000IFMN) + return !fMasternodeMode || GetValue() != 1000 * COIN; + else if (coinControl->nCoinType == CoinType::ONLY_NOT1000IFMN) + return !fMasternodeMode || GetValue() != 1000 * COIN; + else if (coinControl->nCoinType == CoinType::ONLY_1000) + return GetValue() == 1000 * COIN; + else if (coinControl->nCoinType == CoinType::WITH_1000) + return true; + else + return GetValue() != 1000 * COIN; +} + +bool CTransparentTxout::IsLLMQInstantSendLocked() const { + if (_isMockup) + return _mockupIsLLMQInstantSendLocked; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + AssertLockHeld(llmq::quorumInstantSendManager->cs); + + return llmq::quorumInstantSendManager->db.GetInstantSendLockByTxid(GetHash()) != nullptr; +} + +bool CTransparentTxout::IsCoinBase() const { + if (_isMockup) + return _mockupIsCoinBase; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + return wallet->mapWallet.at(GetHash()).IsCoinBase(); +} + +unsigned int CTransparentTxout::GetDepthInMainChain() const { + if (_isMockup) + return _mockupDepthInMainChain; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + return wallet->mapWallet.at(GetHash()).GetDepthInMainChain(); +} + const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet @@ -193,7 +369,7 @@ CPubKey CWallet::GetKeyFromKeypath(uint32_t nChange, uint32_t nChild, CKey& secr MnemonicContainer mContainer = mnemonicContainer; DecryptMnemonicContainer(mContainer); SecureVector seed = mContainer.GetSeed(); - masterKey.SetMaster(&seed[0], seed.size()); + masterKey.SetMaster(&seed.at(0), seed.size()); } else { // try to get the master key if (!GetKey(hdChain.masterKeyID, key)) @@ -255,6 +431,7 @@ CPubKey CWallet::GenerateNewKey(uint32_t nChange, bool fWriteChain) MnemonicContainer mContainer = mnemonicContainer; DecryptMnemonicContainer(mContainer); SecureVector seed = mContainer.GetSeed(); + if (seed.empty()) seed.reserve(64); masterKey.SetMaster(&seed[0], seed.size()); } else { // try to get the master key @@ -279,12 +456,12 @@ CPubKey CWallet::GenerateNewKey(uint32_t nChange, bool fWriteChain) // derive child key at next index, skip keys already known to the wallet do { - externalChainChildKey.Derive(childKey, hdChain.nExternalChainCounters[nChange]); - metadata.hdKeypath = "m/44'/" + std::to_string(nIndex) + "'/0'/" + std::to_string(nChange) + "/" + std::to_string(hdChain.nExternalChainCounters[nChange]); + externalChainChildKey.Derive(childKey, hdChain.nExternalChainCounters.at(nChange)); + metadata.hdKeypath = "m/44'/" + std::to_string(nIndex) + "'/0'/" + std::to_string(nChange) + "/" + std::to_string(hdChain.nExternalChainCounters.at(nChange)); metadata.hdMasterKeyID = hdChain.masterKeyID; - metadata.nChild = Component(hdChain.nExternalChainCounters[nChange], false); + metadata.nChild = Component(hdChain.nExternalChainCounters.at(nChange), false); // increment childkey index - hdChain.nExternalChainCounters[nChange]++; + hdChain.nExternalChainCounters.at(nChange)++; } while (HaveKey(childKey.key.GetPubKey().GetID())); secret = childKey.key; @@ -4274,7 +4451,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov CReserveKey reservekey(this); CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, nExtraPayloadSize)) + if (!CreateTransaction(vecSend, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, nExtraPayloadSize)) return false; if (nChangePosInOut != -1) @@ -4320,374 +4497,318 @@ bool CWallet::ConvertList(std::vector vecTxIn, std::vector &ve return true; } -bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, - int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, bool fUseInstantSend) -{ - CAmount nFeePay = 0; - - CAmount nValue = 0; - int nChangePosRequest = nChangePosInOut; - unsigned int nSubtractFeeFromAmount = 0; - for (const auto& recipient : vecSend) - { - if (nValue < 0 || recipient.nAmount < 0) - { - strFailReason = _("Transaction amounts must not be negative"); - return false; - } - nValue += recipient.nAmount; +CAmount CWallet::GetFee(const CCoinControl* coinControl, size_t txSize) { + AssertLockHeld(cs_main); - if (recipient.fSubtractFeeFromAmount) - nSubtractFeeFromAmount++; - } - if (vecSend.empty()) - { - strFailReason = _("Transaction must have at least one recipient"); - return false; + CAmount fee = GetRequiredFee(txSize); + if (coinControl && coinControl->fOverrideFeeRate) { + CAmount override = coinControl->nFeeRate.GetFee(txSize); + if (override < fee) + throw std::runtime_error("nFeeRate is set too low; it will lead to creation of an unrelayable tx"); } - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); - CMutableTransaction txNew; - - // Discourage fee sniping. - // - // For a large miner the value of the transactions in the best block and - // the mempool can exceed the cost of deliberately attempting to mine two - // blocks to orphan the current best block. By setting nLockTime such that - // only the next block can include the transaction, we discourage this - // practice as the height restricted and limited blocksize gives miners - // considering fee sniping fewer options for pulling off this attack. - // - // A simple way to think about this is from the wallet's point of view we - // always want the blockchain to move forward. By setting nLockTime this - // way we're basically making the statement that we only want this - // transaction to appear in the next block; we don't want to potentially - // encourage reorgs by allowing transactions to appear at lower heights - // than the next block in forks of the best chain. - // - // Of course, the subsidy is high enough, and transaction volume low - // enough, that fee sniping isn't a problem yet, but by implementing a fix - // now we ensure code won't be written that makes assumptions about - // nLockTime that preclude a fix later. - - txNew.nLockTime = chainActive.Height(); - - // Secondly occasionally randomly pick a nLockTime even further back, so - // that transactions that are delayed after signing for whatever reason, - // e.g. high-latency mix networks and some CoinJoin implementations, have - // better privacy. - if (GetRandInt(10) == 0) - txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100)); - - assert(txNew.nLockTime <= (unsigned int)chainActive.Height()); - assert(txNew.nLockTime < LOCKTIME_THRESHOLD); - - { - std::set> setCoins; - LOCK2(cs_main, cs_wallet); - { - std::vector vAvailableCoins; - AvailableCoins(vAvailableCoins, true, coinControl, false, fUseInstantSend); - int nInstantSendConfirmationsRequired = Params().GetConsensus().nInstantSendConfirmationsRequired; - - nFeeRet = 0; - if(nFeePay > 0) nFeeRet = nFeePay; - double dPriority = 0; - // Start with no fee and loop until there is enough fee - while (true) - { - nChangePosInOut = nChangePosRequest; - txNew.vin.clear(); - txNew.vout.clear(); - wtxNew.fFromMe = true; - bool fFirst = true; - - CAmount nValueToSelect = nValue; - if (nSubtractFeeFromAmount == 0) - nValueToSelect += nFeeRet; - // vouts to the payees - for (const auto& recipient : vecSend) - { - CTxOut txout(recipient.nAmount, recipient.scriptPubKey); + if (coinControl && coinControl->nMinimumTotalFee > fee) + fee = coinControl->nMinimumTotalFee; - if (recipient.fSubtractFeeFromAmount) - { - txout.nValue -= nFeeRet / nSubtractFeeFromAmount; // Subtract fee equally from each selected recipient - - if (fFirst) // first receiver pays the remainder not divisible by output count - { - fFirst = false; - txout.nValue -= nFeeRet % nSubtractFeeFromAmount; - } - } + return fee; +} - if (txout.IsDust(dustRelayFee)) - { - if (recipient.fSubtractFeeFromAmount && nFeeRet > 0) - { - if (txout.nValue < 0) - strFailReason = _("The transaction amount is too small to pay the fee"); - else - strFailReason = _("The transaction amount is too small to send after the fee has been deducted"); - } - else - strFailReason = _("Transaction amount too small"); - return false; - } - txNew.vout.push_back(txout); - } +std::vector CWallet::GetTransparentTxouts() const { + AssertLockHeld(cs_wallet); - // Choose coins to use - CAmount nValueIn = 0; - setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl, fUseInstantSend)) - { - strFailReason = _("Insufficient funds"); - return false; - } + std::vector vTransparentTxouts; + for (const auto& walletIt: mapWallet) { + size_t i = 0; + for (const CTxOut& txout: walletIt.second.tx->vout) { + if (CTransparentTxout::IsTransparentTxout(txout)) + vTransparentTxouts.emplace_back(this, COutPoint(walletIt.second.tx->GetHash(), i), txout); - const CAmount nChange = nValueIn - nValueToSelect; - CTxOut newTxOut; + i++; + } + } + return vTransparentTxouts; +} - if (nChange > 0 && !(coinControl && coinControl->fNoChange)) - { - // Fill a vout to ourself - // TODO: pass in scriptChange instead of reservekey so - // change transaction isn't always pay-to-dash-address - CScript scriptChange; +void CWallet::SignTransparentInputs(CMutableTransaction& tx, const std::vector& vInputTxs, + bool fSign) { + int nNew = -1; + for (const CTransparentTxout& txin: vInputTxs) { + nNew++; - // coin control: send change to custom address - if (coinControl && !boost::get(&coinControl->destChange)) - scriptChange = GetScriptForDestination(coinControl->destChange); + CTransaction ctx(tx); + SignatureData sigData; + bool success = false; + if (fSign) { + TransactionSignatureCreator creator(&*this, &ctx, nNew, txin.GetValue(), SIGHASH_ALL); + success = ProduceSignature(creator, txin.GetScriptPubkey(), sigData); + } else { + // ProduceSignature with DummySignatureCreator always returns false. + success = true; + ProduceSignature(DummySignatureCreator(&*this), txin.GetScriptPubkey(), sigData); + } - // no coin control: send change to newly generated address - else - { - // Note: We use a new key here to keep it from being obvious which side is the change. - // The drawback is that by not reusing a previous key, the change may be lost if a - // backup is restored, if the backup doesn't have the new private key for the change. - // If we reused the old key, it would be possible to add code to look for and - // rediscover unknown transactions that were written with keys of ours to recover - // post-backup change. + assert(nNew < tx.vin.size()); + tx.vin[nNew].scriptSig = sigData.scriptSig; + tx.vin[nNew].scriptWitness = sigData.scriptWitness; - // Reserve a new key pair from key pool - CPubKey vchPubKey; - bool ret; - ret = reservekey.GetReservedKey(vchPubKey); - if (!ret) - { - strFailReason = _("Keypool ran out, please call keypoolrefill first"); - return false; - } + std::vector> stack; + txnouttype whichType; + std::vector> vSolutions; + if (!Solver(txin.GetScriptPubkey(), whichType, vSolutions)) + throw std::runtime_error("Non-standard input"); + else if (!success) + throw std::runtime_error("Signing transaction failed"); + else if (!fSign || ctx.IsCoinBase()) + ; + else if (sigData.scriptSig.size() > 1560) + throw std::runtime_error("Produced a signature which would exceed relay limit"); + else if (!sigData.scriptSig.IsPushOnly()) + throw std::runtime_error("Produced a signature which is not push only"); + else if (!sigData.scriptSig.HasCanonicalPushes()) + throw std::runtime_error("Produced a signature which has non-canonical pushes"); + else if (!EvalScript(stack, sigData.scriptSig, SCRIPT_VERIFY_NONE, BaseSignatureChecker(), SIGVERSION_BASE)) + throw std::runtime_error("Couldn't evaluate produced signature"); + else if (stack.empty()) + throw std::runtime_error("Produced a signature with an empty sigScript stack"); + else if (whichType == TX_SCRIPTHASH && CScript(stack.back().begin(), stack.back().end()).GetSigOpCount(true) > MAX_P2SH_SIGOPS) + throw std::runtime_error("Produced a signature with too many sigops"); + } +} + +void CWallet::CheckTransparentTransactionSanity(CMutableTransaction& tx, + const std::vector& vInputTxs, + const CCoinControl* coinControl, CAmount nFee, bool fSign) { + size_t txSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); + + if (txSize * WITNESS_SCALE_FACTOR > MAX_STANDARD_TX_WEIGHT) + throw std::runtime_error("Transaction too large"); + + if (coinControl && coinControl->nMaxSize && txSize > coinControl->nMaxSize) + throw std::runtime_error("We made a transaction exceeding coinControl->nMaxSize. This is a bug."); + + if (coinControl && coinControl->nMaxInputs && tx.vin.size() > coinControl->nMaxInputs) + throw std::runtime_error("We made a transaction exceeding coinControl->nMaxInputs. This is a bug."); + + if (nFee < GetFee(coinControl, txSize)) + throw std::runtime_error("Calculated fee too low. This is a bug."); + + // Calculated signature sizes will exceed signature sizes by n bytes with a probability 1/(2*256^(n-1)). + if (fSign && nFee > GetFee(coinControl, txSize + 3000)) + throw std::runtime_error("Calculated fee too high. This is probably a bug."); + + if (GetRequiredFee(txSize) > nFee) + throw std::runtime_error("Calculated transaction fee below relay fee"); + + if (nFee > maxTxFee) + throw std::runtime_error("txFee > maxTxFee. This is probably a bug."); + + bool fHasDataOut = false; + for (CTxOut& txout: tx.vout) { + txnouttype whichType; + + if (!::IsStandard(txout.scriptPubKey, whichType, false)) + throw std::runtime_error("Created a non-standard txout"); + + if (whichType == TX_NULL_DATA) { + if (fHasDataOut) + throw std::runtime_error("Created multiple TX_NULL_DATA outputs"); + + fHasDataOut = true; + } else if ((whichType == TX_MULTISIG) && (!fIsBareMultisigStd)) { + throw std::runtime_error("Created non-standard bare multisig output"); + } + + if (txout.IsDust()) + throw std::runtime_error("Created a dust output"); + } + + assert(tx.vin.size() == vInputTxs.size()); + for (const CTxIn& txin: tx.vin) { + bool nFound = 0; + for (const CTransparentTxout& txin_: vInputTxs) { + if (txin_.GetOutpoint() == txin.prevout) + nFound += 1; + } + + assert(nFound == 1); + } + + CAmount totalInputValue = 0; + CAmount totalOutputValue = 0; + for (const CTransparentTxout& txin: vInputTxs) totalInputValue += txin.GetValue(); + for (CTxOut& txout: tx.vout) totalOutputValue += txout.nValue; + assert(totalInputValue == totalOutputValue + nFee); + assert(totalInputValue > 0); +} + +// Create a transaction to vecSend, placing it in wtxNew. reservekey is the keypool. nFeeRet will be populated with the +// calculated transaction fee. If nChangePosInOut is not -1, the change output will be placed at that position. If +// nChangePosInOut > vecSend.size(), we will fail. If creating a transaction fails, we will return false and set +// strFailReason to a human-readable description of the failure. coinControl may be set or unset; if it is unset an +// arbitrary transaction selection algorithm will be used. If coinControl->nMaxSize is set, transaction creation will +// fail if the size of the transaction with MAXIMUM length signatures would exceed the transaction size. Actual +// generated transactions may produce signatures below this size, which may result in a transaction that would be +// smaller than coinControl->nMaxSize being rejected for size reasons; this is because at the time the transaction is +// made, we do not know how long the signatures will be. If sign is false, the transaction will not be signed; if it is +// true, the wallet must be unlocked. nExtraPayloadSize should be set to the number of extra bytes in the transaction +// outside inputs/outputs (ie. LLMQ-related things). If fUseInstantSend is true, we will consider both locked and +// confirmed UTXOs to be eligible for input; if it is not, only confirmed UTXOs will be used as inputs. +bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, + const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, + bool fUseInstantSend) { + AssertLockHeld(cs_main); - scriptChange = GetScriptForDestination(vchPubKey.GetID()); - } + std::vector vTransparentTxouts = GetTransparentTxouts(); + return CreateTransaction(vecSend, wtxNew, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, sign, + nExtraPayloadSize, fUseInstantSend, vTransparentTxouts); +} - newTxOut = CTxOut(nChange, scriptChange); +bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, + const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, + bool fUseInstantSend, const std::vector& vTransparentTxouts) { + AssertLockHeld(cs_main); - // We do not move dust-change to fees, because the sender would end up paying more than requested. - // This would be against the purpose of the all-inclusive feature. - // So instead we raise the change and deduct from the recipient. - if (nSubtractFeeFromAmount > 0 && newTxOut.IsDust(dustRelayFee)) - { - CAmount nDust = newTxOut.GetDustThreshold(dustRelayFee) - newTxOut.nValue; - newTxOut.nValue += nDust; // raise change until no more dust - for (unsigned int i = 0; i < vecSend.size(); i++) // subtract from first recipient - { - if (vecSend[i].fSubtractFeeFromAmount) - { - txNew.vout[i].nValue -= nDust; - if (txNew.vout[i].IsDust(dustRelayFee)) - { - strFailReason = _("The transaction amount is too small to send after the fee has been deducted"); - return false; - } - break; - } - } - } + if (!coinControl || coinControl->destChange.which() == 0) + assert(reservekey); - // Never create dust outputs; if we would, just - // add the dust to the fee. - if (newTxOut.IsDust(dustRelayFee)) - { - nChangePosInOut = -1; - nFeeRet += nChange; - reservekey.ReturnKey(); - } - else - { - if (nChangePosInOut == -1) - { - // Insert change txn at random position: - nChangePosInOut = GetRandInt(txNew.vout.size()+1); - } - else if ((unsigned int)nChangePosInOut > txNew.vout.size()) - { - strFailReason = _("Change index out of range"); - return false; - } + nFeeRet = -1; + strFailReason = ""; + wtxNew = CWalletTx(); - std::vector::iterator position = txNew.vout.begin()+nChangePosInOut; - txNew.vout.insert(position, newTxOut); - } - } else { - reservekey.ReturnKey(); - nChangePosInOut = -1; - } + CAmount nRequired = 0; + size_t nConstantSize = 4 + // version + GetSizeOfCompactSize(vecSend.size() + 1) + // This is a varint representing the number of outputs. In the event + // that there are 0xfc inputs and a change output is not required we + // will pay the fee for one extra byte. + 4 + // locktime + (nExtraPayloadSize ? GetSizeOfCompactSize(nExtraPayloadSize) + nExtraPayloadSize : 0); - // Fill vin - // - // Note how the sequence number is set to max()-1 so that the - // nLockTime set above actually works. - // - // BIP125 defines opt-in RBF as any nSequence < maxint-1, so - // we use the highest possible value in that range (maxint-2) - // to avoid conflicting with other possible uses of nSequence, - // and in the spirit of "smallest possible change from prior - // behavior." - for (const auto& coin : setCoins) - txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(), - std::numeric_limits::max() - (fWalletRbf ? 2 : 1))); + if (vecSend.empty()) { + strFailReason = _("Transaction must have at least one recipient"); + nChangePosInOut = -1; + return false; + } - // Fill in dummy signatures for fee calculation. - if (!DummySignTx(txNew, setCoins)) { - strFailReason = _("Signing transaction failed"); - return false; - } + size_t nRecipientsToSplitFee = 0; + for (const CRecipient& recipient: vecSend) { + if (recipient.nAmount < 0) { + strFailReason = _("Transaction amounts must not be negative"); + nChangePosInOut = -1; + return false; + } - unsigned int nBytes = ::GetSerializeSize(txNew, SER_NETWORK, PROTOCOL_VERSION); + size_t nScriptSize = recipient.scriptPubKey.size(); + nConstantSize += 8 /* value */ + GetSizeOfCompactSize(nScriptSize) + nScriptSize; + nRequired += recipient.nAmount; - if (nExtraPayloadSize != 0) { - // account for extra payload in fee calculation - nBytes += GetSizeOfCompactSize(nExtraPayloadSize) + nExtraPayloadSize; - } + if (recipient.fSubtractFeeFromAmount) nRecipientsToSplitFee++; + } - if (GetTransactionWeight(txNew) >= MAX_NEW_TX_WEIGHT) { - // Do not create oversized transactions (bad-txns-oversize). - strFailReason = _("Transaction is too large (size limit: 250Kb). Select less inputs or consolidate your UTXOs"); - return false; - } + std::vector vInputTxs; + CAmount nCollected = 0; + try { + GetInputsForTx(vTransparentTxouts, vInputTxs, nFeeRet, nCollected, nRequired, nConstantSize, coinControl, + fUseInstantSend, nRecipientsToSplitFee > 0); + } catch (std::runtime_error& e) { + nChangePosInOut = -1; + strFailReason = _(e.what()); + return false; + } - CTransaction txNewConst(txNew); - dPriority = txNewConst.ComputePriority(dPriority, nBytes); + std::vector vOutputs; - // Remove scriptSigs to eliminate the fee calculation dummy signatures - for (auto& vin : txNew.vin) { - vin.scriptSig = CScript(); - vin.scriptWitness.SetNull(); - } + bool fFirstToPayFee = true; + for (const CRecipient& recipient: vecSend) { + CTxOut txout; + txout.scriptPubKey = recipient.scriptPubKey; - // Allow to override the default confirmation target over the CoinControl instance - int currentConfirmationTarget = nTxConfirmTarget; - if (coinControl && coinControl->nConfirmTarget > 0) - currentConfirmationTarget = coinControl->nConfirmTarget; + txout.nValue = recipient.nAmount; + if (recipient.fSubtractFeeFromAmount) { + txout.nValue -= nFeeRet / nRecipientsToSplitFee; + if (fFirstToPayFee) { + txout.nValue -= nFeeRet % nRecipientsToSplitFee; + fFirstToPayFee = false; + } - // Can we complete this as a free transaction? - if (fSendFreeTransactions && nBytes <= MAX_FREE_TRANSACTION_CREATE_SIZE) - { - // Not enough fee: enough priority? - double dPriorityNeeded = mempool.estimateSmartPriority(currentConfirmationTarget); - // Require at least hard-coded AllowFree. - if (dPriority >= dPriorityNeeded && AllowFree(dPriority)) - break; - } + // Note that the behaviour here differs from bitcoind. bitcoind will alter the txout list if any outputs go + // below the dust threshold, whereas we will error if amounts would go below 0, and will still send dust. + if (txout.nValue < 0) { + nFeeRet = -1; + nChangePosInOut = -1; + strFailReason = _("An output expected to pay part of the fee is unable to pay its share."); + return false; + } + } - CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, mempool); - if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) { - nFeeNeeded = coinControl->nMinimumTotalFee; - } - if (coinControl && coinControl->fOverrideFeeRate) - nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes); + vOutputs.emplace_back(txout); + } - // If we made it here and we aren't even able to meet the relay fee on the next pass, give up - // because we must be at the maximum allowed fee. - if (nFeeNeeded < ::minRelayTxFee.GetFee(nBytes)) - { - strFailReason = _("Transaction too large for fee policy"); - return false; - } + CAmount nChangeAmount = nCollected - nRequired - (nRecipientsToSplitFee ? 0 : nFeeRet); + // If the collected amount is exactly what is required, we don't need to make a change output. + if (nChangeAmount || (coinControl && coinControl->fNoChange)) { + CScript scriptChange; + if (coinControl && coinControl->destChange.which() != 0) { + scriptChange = GetScriptForDestination(coinControl->destChange); + } else { + CPubKey changeKey; + if (!reservekey->GetReservedKey(changeKey)) { + strFailReason = _("Keypool ran out, please call keypoolrefill first"); + nChangePosInOut = -1; + nFeeRet = -1; + return false; + } - if (nFeeRet >= nFeeNeeded) { - // Reduce fee to only the needed amount if we have change - // output to increase. This prevents potential overpayment - // in fees if the coins selected to meet nFeeNeeded result - // in a transaction that requires less fee than the prior - // iteration. - // TODO: The case where nSubtractFeeFromAmount > 0 remains - // to be addressed because it requires returning the fee to - // the payees and not the change output. - // TODO: The case where there is no change output remains - // to be addressed so we avoid creating too small an output. - if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) { - CAmount extraFeePaid = nFeeRet - nFeeNeeded; - std::vector::iterator change_position = txNew.vout.begin()+nChangePosInOut; - change_position->nValue += extraFeePaid; - nFeeRet -= extraFeePaid; - } - break; // Done, enough fee included. - } + CTxDestination dest(changeKey.GetID()); + scriptChange = GetScriptForDestination(dest); + } - // Try to reduce change to include necessary fee - if (nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) { - CAmount additionalFeeNeeded = nFeeNeeded - nFeeRet; - std::vector::iterator change_position = txNew.vout.begin()+nChangePosInOut; - // Only reduce change if remaining amount is still a large enough output. - if (change_position->nValue >= MIN_FINAL_CHANGE + additionalFeeNeeded) { - change_position->nValue -= additionalFeeNeeded; - nFeeRet += additionalFeeNeeded; - break; // Done, able to increase fee from change - } - } + CTxOut txout(nChangeAmount, scriptChange); - // Include more fee and try again. - nFeeRet = nFeeNeeded; - continue; - } + if (nChangePosInOut == -1) { + // Unlike bitcoind, we will always place change outputs last. + nChangePosInOut = vOutputs.size(); + vOutputs.emplace_back(txout); + } else if (nChangePosInOut > vOutputs.size() || nChangePosInOut < -1) { + strFailReason = _("Change index out of range"); + nChangePosInOut = -1; + nFeeRet = -1; + return false; + } else { + vOutputs.insert(vOutputs.begin()+nChangePosInOut, txout); } + } - if (sign) - { - CTransaction txNewConst(txNew); - int nIn = 0; - for (const auto& coin : setCoins) - { - const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey; - SignatureData sigdata; - - if (!ProduceSignature(TransactionSignatureCreator(this, &txNewConst, nIn, coin.first->tx->vout[coin.second].nValue, SIGHASH_ALL), scriptPubKey, sigdata)) - { - strFailReason = _("Signing transaction failed"); - return false; - } else { - UpdateTransaction(txNew, nIn, sigdata); - } + CMutableTransaction txNew; + txNew.vout = vOutputs; - nIn++; - } - } + // Because we use Dandelion, we want to delay nLockTime for all transactions, not just 10% of them. Fee sniping is + // not an issue due to chain locks. + if (chainActive[101]) txNew.nLockTime = chainActive.Height() - GetRandInt(100); + else txNew.nLockTime = 0; - // Embed the constructed transaction data in wtxNew. - wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); + for (CTransparentTxout& txin: vInputTxs) { + txNew.vin.emplace_back( + CTxIn(txin.GetOutpoint().hash, txin.GetOutpoint().n, CScript(), std::numeric_limits::max() - (fWalletRbf ? 2 : 1)) + ); } - if (GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { - // Lastly, ensure this tx will pass the mempool's chain limits - LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, 0, false, 0, lp); - - CTxMemPool::setEntries setAncestors; - size_t nLimitAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - size_t nLimitAncestorSize = GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; - size_t nLimitDescendants = GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); - size_t nLimitDescendantSize = GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; - std::string errString; - if (!mempool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { - strFailReason = _("Transaction has too long of a mempool chain"); - return false; - } + try { + SignTransparentInputs(txNew, vInputTxs, sign); + CheckTransparentTransactionSanity(txNew, vInputTxs, coinControl, nFeeRet, sign); + } catch (std::runtime_error& e) { + LogPrintf("%s(): %s\n", __func__, e.what()); + nFeeRet = -1; + nChangePosInOut = -1; + strFailReason = e.what(); + return false; } + + wtxNew.fFromMe = true; + wtxNew.fTimeReceivedIsTxTime = true; + wtxNew.BindWallet(this); + wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ab6e98fd7d..bc0ee3a349 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -9,6 +9,7 @@ #include "amount.h" #include "../sigma/coin.h" #include "../liblelantus/coin.h" +#include "primitives/transaction.h" #include "streams.h" #include "tinyformat.h" #include "ui_interface.h" @@ -26,14 +27,13 @@ #include "../base58.h" #include "firo_params.h" #include "univalue.h" - +#include "coincontrol.h" +#include "policy/policy.h" #include "hdmint/tracker.h" #include "hdmint/wallet.h" - #include "primitives/mint_spend.h" - #include "bip47/paymentcode.h" - +#include "../llmq/quorums_instantsend.h" #include #include @@ -647,12 +647,64 @@ class LelantusJoinSplitBuilder; //static boost::signals2::signal UnlockWallet; extern boost::signals2::signal UnlockWallet; +class CTransparentTxout; +enum AbstractTxoutType { + Transparent +}; + +class CTransparentTxout { +public: + static bool IsTransparentTxout(const CTxOut& txout); + + CTransparentTxout() = default; + CTransparentTxout(COutPoint outpoint, CTxOut txout): outpoint(outpoint), txout(txout), _isMockup(true) {} + CTransparentTxout(const CWallet* wallet, COutPoint outpoint, CTxOut txout): wallet(wallet), outpoint(outpoint), txout(txout) {}; + + uint256 GetHash() const; + COutPoint GetOutpoint() const; + CAmount GetValue() const; + CScript GetScriptPubkey() const; + size_t GetMarginalSpendSize(std::vector& previousInputs) const; + bool IsMine(const CCoinControl* coinControl) const; + bool IsSpendable() const; + bool IsLocked() const; + bool IsAbandoned() const; + bool IsCoinTypeCompatible(const CCoinControl* coinControl) const; + bool IsLLMQInstantSendLocked() const; + bool IsCoinBase() const; + unsigned int GetDepthInMainChain() const; + +private: + const CWallet* wallet = nullptr; + COutPoint outpoint; + CTxOut txout; + bool _isMockup = false; + +public: + bool _mockupIsMine = false; + bool _mockupIsMineWatchOnly = false; + bool _mockupIsSpent = false; + bool _mockupIsAbandoned = false; + bool _mockupIsLocked = false; + bool _mockupIsLLMQInstantSendLocked = false; + bool _mockupIsCoinBase = false; + unsigned int _mockupDepthInMainChain = 0; +}; + /** * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. */ class CWallet : public CCryptoKeyStore, public CValidationInterface { +public: + /** + * Used to keep track of spent outpoints, and + * detect and report conflicts (double-spends or + * mutated transactions where the mutant gets mined). + */ + typedef std::multimap TxSpends; + private: friend class CSparkWallet; @@ -665,6 +717,334 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set >& setCoinsRet, CAmount& nValueRet, const CCoinControl *coinControl = NULL, bool fForUseInInstantSend = true) const; + void AddToSpends(const COutPoint &outpoint, const uint256 &wtxid); + + void AddToSpends(const uint256 &wtxid); + + CAmount GetFee(const CCoinControl* coinControl, size_t txSize); + std::vector GetTransparentTxouts() const; + + // Get all the vRelevantTransactions that can be used. Inputs specifically selected in coinControl are put into + // vCoinControlInputs, and all other inputs compatible with coinControl and available for use are put into + // vAvailableInputs. + template + void GetAvailableInputs(const std::vector& vRelevantTransactions, + std::vector& vAvailableInputs, + std::vector& vCoinControlInputs, + const CCoinControl* coinControl, + bool fUseInstantSend) const { + AssertLockHeld(cs_wallet); + vAvailableInputs.clear(); + vCoinControlInputs.clear(); + + if (coinControl && coinControl->nCoinType == CoinType::ONLY_NONDENOMINATED_NOT1000IFMN && !fMasternodeMode) + throw std::runtime_error("fMasternode must be enabled to use CoinType::ONLY_NONDENOMINATED_NOT1000IFMN"); + + if (coinControl && coinControl->nCoinType == CoinType::WITH_MINTS) + throw std::runtime_error("CoinType::WITH_MINTS is not supported for any transactions."); + + for (const AbstractTxout& tx: vRelevantTransactions) { + bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); + + if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs && !isSelected) + continue; + if (!tx.IsSpendable()) continue; + if (!tx.IsCoinTypeCompatible(coinControl)) continue; + if (tx.IsAbandoned()) continue; + if (!tx.GetDepthInMainChain() && (!fUseInstantSend || !tx.IsLLMQInstantSendLocked())) continue; + if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) + continue; + if (tx.IsCoinBase() && tx.GetDepthInMainChain() < COINBASE_MATURITY) continue; + if (!tx.IsMine(coinControl)) continue; + + if (isSelected) vCoinControlInputs.push_back(tx); + else vAvailableInputs.push_back(tx); + } + + if (coinControl) { + if (vCoinControlInputs.size() != coinControl->setSelected.size()) + throw std::runtime_error("Some coin control inputs could not be selected."); + if (coinControl->fRequireAllInputs && coinControl->nMaxInputs && + vCoinControlInputs.size() > coinControl->nMaxInputs) + throw std::runtime_error("The number of selected inputs exceeds the maximum number of inputs."); + } + + // Sort vAvailable and vCoinControlInputs by largest first. Additionally, order it so that transaction selection + // will be deterministic; this property is not otherwise required. + for (std::vector* v: {&vAvailableInputs, &vCoinControlInputs}) { + std::sort(v->begin(), v->end(), [](const AbstractTxout& a, const AbstractTxout& b) { + if (a.GetValue() != b.GetValue()) + return a.GetValue() > b.GetValue(); + if (a.GetHash() != b.GetHash()) + return a.GetHash().Compare(b.GetHash()) == -1; + return a.GetOutpoint().n < b.GetOutpoint().n; + }); + } + } + + // This populates vInputs with AbstractTxouts from vRelevantTransactions required to produce a transaction to the + // given specification. nRequired is the total amount of value we need to produce (either including or excluding fee + // depending on the value of fSubtractFeeFromAmount); nConstantSize must be the size of the transaction minus the + // inputs, input count, and P2SH_OUTPUT_SIZE in the event that input and output amounts aren't exactly equal. If + // fUseInstantSend is not set, we will ignore UTXOs without a confirmation; if it is set, we will ignore only + // un-is-locked UTXOs. Fees will be automatically determined if not set in coinControl. If fAllowPartial is set, we + // will collect as many inputs as we can and return even if we don't have enough to fulfill nRequired. We return + // true if we were able to collect enough inputs to fulfill nRequired, and false otherwise. If fAllowPartial is set, + // vInputs may be returned with a 0-length; this will occur if there vRelevantTransactions is empty or if no + // transactions could cover the required fees. + template + bool GetInputsForTx(const std::vector& vRelevantTransactions, std::vector& vInputs, + CAmount& nFeeRet, CAmount& nCollectedRet, CAmount nRequired, size_t nConstantSize, + const CCoinControl* coinControl, bool fUseInstantSend, bool fSubtractFeeFromAmount, + bool fAllowPartial=false, size_t nChangeSize=34) { + AssertLockHeld(cs_wallet); + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + + size_t nMaxSize = coinControl && coinControl->nMaxSize ? coinControl->nMaxSize : (MAX_STANDARD_TX_WEIGHT / 4); + + if (nRequired < 0) + throw std::runtime_error("Transaction amounts must be positive"); + + if (coinControl && coinControl->nMinimumTotalFee < 0) + throw std::runtime_error("Minimum total fee must be positive"); + + if (coinControl && coinControl->nConfirmTarget && coinControl->nConfirmTarget < 0) + throw std::runtime_error("nConfirmTarget must be positive if set."); + + std::vector vCoinControlInputs; + // vAvailable contains the available inputs that are not in vCoinControlInputs. + std::vector vAvailable; + GetAvailableInputs(vRelevantTransactions, vAvailable, vCoinControlInputs, coinControl, fUseInstantSend); + + // This algorithm will first pick all the transactions selected in coinControl. If + // coinControl->fRequireAllInputs is not set, it will stop when it has enough to provide for our outputs + // otherwise it will consume all the inputs. After that, it will select the smallest UTXO in our wallet, and end + // if enough value is found. Then, if there is a UTXO which combined with the smallest that can provide for our + // entire output value, that UTXO. If there is not, it will then pick the largest UTXO, and then the next + // smallest, and so on and so forth until we can fulfill our output requirements, or until we reach nMaxSize or + // coinControl->nMaxInputs limits. Once we reach those, we will start replacing the largest small inputs we have + // chosen with the largest inputs we haven't selected yet. If that fails, it is the case that the transactions + // in our wallet are not sufficient to provide for the outputs requested within the coinControl and nMaxSize + // constraints, so we will therefore fail. + size_t txSize = nConstantSize + 1; + size_t iFront = 0; + size_t iBack = 0; + size_t iCoinControl = 0; + bool fTakeFromFront = false; + bool fTrySkipFront = true; + bool fReplace = false; + std::vector vSmallInputs; + while (true) { + // The idea here is to front-side inputs which are larger than the smallest front-side input that can + // fulfill our output value requirements. + if (fTrySkipFront && iCoinControl == vCoinControlInputs.size()) { + if (iFront) { + // If iFront is already set and we're here at another iteration of this loop, the input we last + // identified was too small to fulfill our requirements. We're therefore going to undo adding it and + // try again with the next larger input. + + assert(!vInputs.empty()); + AbstractTxout oldTxout = vInputs.back(); + vInputs.pop_back(); + + size_t inputSize = oldTxout.GetMarginalSpendSize(vInputs); + // If inputSize here is 0, nCollectedRet and txSize will have never been mutated. + if (inputSize) { + nCollectedRet -= oldTxout.GetValue(); + txSize -= inputSize; + } + + iFront -= 1; + } else { + // If we're at the first iteration, we'll identify the smallest input that's larger than nRequired + // and start looking for inputs from that. + for (const AbstractTxout& txo: vAvailable) { + if (txo.GetValue() > nRequired) iFront++; + else break; + } + if (iFront) iFront--; + + // If we got to the last element of vAvailable, which we have already added as an input element + // previously, we don't have any UTXOs which will fulfill our requirements. Therefore, we'll stop + // using the fTrySkipFront logic and pick the largest UTXOs we have. + if (iFront + 1 == vAvailable.size()) iFront = 0; + } + + // If iFront here is at 0, we've reached the end and want to try our normal logic. + if (!iFront) fTrySkipFront = false; + + // If we've done an iteration of fTrySkipFront logic before, fTakeFromFront will be false, but we need + // it to be true as this is a do-over. + fTakeFromFront = true; + } + + if (coinControl && coinControl->nMaxInputs && coinControl->nMaxInputs == vInputs.size()) + fReplace = true; + + AbstractTxout txout; + bool hasReplacedTxout = false; + // If hasReplacedTxout is false, the value of iToReplace is meaningless. + size_t iToReplace = 0; + if (iCoinControl != vCoinControlInputs.size()) { + // Select coin control inputs before dealing with other inputs. + + txout = vCoinControlInputs.at(iCoinControl++); + } else if (fReplace) { + // If this code is reached, the value of iBack and fTakeFromFront no longer carries any significance. + iBack = SIZE_MAX; + fTakeFromFront = false; + + // If we have reached the nMaxInputs or nMaxSize limit, we will replace the largest input that we've + // selected from the back of vAvailable with an input from the front of vAvailable. + + if (vSmallInputs.empty()) break; + if (iFront == vAvailable.size()) break; + + iToReplace = vSmallInputs.back(); + vSmallInputs.pop_back(); + + AbstractTxout oldTxout = vInputs.at(iToReplace); + vInputs.erase(vInputs.begin() + iToReplace); + + nCollectedRet -= oldTxout.GetValue(); + txSize -= oldTxout.GetMarginalSpendSize(vInputs); + + AbstractTxout newTxout = vAvailable.at(iFront++); + txSize += newTxout.GetMarginalSpendSize(vInputs); + + vInputs.insert(vInputs.begin() + iToReplace, newTxout); + hasReplacedTxout = true; + txout = vInputs.at(iToReplace); + } else if (iFront + iBack == vAvailable.size()) { + // We've selected all possible inputs and still can't come up with the required amount. Fail. + + break; + } else if (fTakeFromFront) { + txout = vAvailable.at(iFront++); + fTakeFromFront = false; + } else { + txout = vAvailable.at(vAvailable.size() - ++iBack); + fTakeFromFront = true; + } + + size_t inputSize = 0; + try { + inputSize = txout.GetMarginalSpendSize(vInputs); + } catch (std::runtime_error& e) { + LogPrintf("%s(): Unexpectedly failed to determine spend size for %s-%d\n", __func__, + txout.GetOutpoint().hash.GetHex(), txout.GetOutpoint().n); + + if (coinControl && coinControl->IsSelected(txout.GetOutpoint())) + throw std::runtime_error("The spend size of a coin control input could not be determined."); + + // If we push this to the back of vSmallInputs, it will be replaced in the next iteration as the + // condition of the first if statement at the beginning of this loop will be fulfilled. nCollectedRet + // has been mutated above so that the undo in the first if block will be valid, and txSize will not be + // modified in the event we can't get the input size for this txo. + if (hasReplacedTxout) vSmallInputs.emplace_back(iToReplace); + + continue; + } + + // If this transaction will bring us over nMaxSize, don't add it and start transaction replacement logic + // above. It is technically possible for our transaction finding logic to fail to find a possible solution + // in the event that a lower value transaction has a smaller spend script, and the difference in fees + // between the two exceeds the difference in value, and this is true for every transaction larger than the + // transaction with the smaller spend script, and the difference is over what we need to get to nRequired, + // but this case should be extremely unlikely. + if (txSize + inputSize > nMaxSize) { + // If we start replacement logic, we want to keep this transaction available as an option. + if (!fReplace) + // fTakeFromFront is true if we last took from the BACK of vAvailable. + fTakeFromFront ? iBack-- : iFront--; + + fReplace = true; + continue; + } + + if (!hasReplacedTxout) { + // fTakeFromFront is true if we last took from the BACK of vAvailable. + if (fTakeFromFront) + vSmallInputs.emplace_back(vInputs.size()); + + + txSize += inputSize; + vInputs.emplace_back(txout); + } + + nCollectedRet += txout.GetValue(); + + nFeeRet = GetFee(coinControl, txSize); + + // If coin control is enabled, we want to select all inputs, so we will only check whether we have enough + // after exhausting vAvailable. + if (coinControl && coinControl->fRequireAllInputs && iCoinControl != vCoinControlInputs.size()) continue; + + // This is here so we will not return in the event that we want to subtract the fee from the amount and the + // amount collected would be sufficient to cover nRequired, but not sufficient to cover the fee. Note that + // this will allow outputs (or even entire transactions) with 0 output value. + if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; + + CAmount extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; + // In this case, we don't need to make a change output, so we don't have to add the cost of a change output. + if (nCollectedRet == nRequired + extraFee) return true; + if ( + nCollectedRet >= nRequired + extraFee && + nCollectedRet <= nRequired + GetFee(coinControl, txSize + nChangeSize) + ) { + nFeeRet = nCollectedRet - nRequired; + return true; + } + + // In this case, we have a change output too. + + if (txSize + nChangeSize > nMaxSize) { + // fTakeFromFront is true if we last took from the BACK of vAvailable. + if (!fReplace) { + if (fTakeFromFront) { + vSmallInputs.pop_back(); + iBack--; + } else { + iFront--; + } + + vInputs.pop_back(); + + nCollectedRet -= txout.GetValue(); + txSize -= inputSize; + } + + fReplace = true; + continue; + } + + nFeeRet = GetFee(coinControl, txSize + nChangeSize); + extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; + if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; + if (nCollectedRet >= extraFee + nRequired) return true; + } + + if (fAllowPartial) { + if (nFeeRet > nCollectedRet) { + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + } else if (nCollectedRet + (fSubtractFeeFromAmount ? 0 : nFeeRet) >= nRequired) { + nFeeRet = GetFee(coinControl, txSize); + } + + return false; + } + + // If we've gotten this far, we don't have the funds to make the transaction. + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + throw std::runtime_error("Insufficient funds"); + } + CWalletDB *pwalletdbEncryption; //! the current wallet version: clients below this version are not able to load the wallet @@ -682,16 +1062,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface mutable bool fAnonymizableTallyCachedNonDenom; mutable std::vector vecAnonymizableTallyCachedNonDenom; - /** - * Used to keep track of spent outpoints, and - * detect and report conflicts (double-spends or - * mutated transactions where the mutant gets mined). - */ - typedef std::multimap TxSpends; - TxSpends mapTxSpends; - void AddToSpends(const COutPoint& outpoint, const uint256& wtxid); - void AddToSpends(const uint256& wtxid); - std::set setWalletUTXO; /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ @@ -802,6 +1172,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bip47wallet.reset(); } + TxSpends mapTxSpends; std::map mapWallet; std::list laccentries; bool EraseFromWallet(uint256 hash); @@ -1011,13 +1382,19 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool overrideEstimatedFeeRate, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const std::set& setSubtractFeeFromOutputs, bool keepReserveKey = true, const CTxDestination& destChange = CNoDestination()); - /** - * Create a new transaction paying the recipients with a set of coins - * selected by SelectCoins(); Also create the change output, when needed - * @note passing nChangePosInOut as -1 will result in setting a random position - */ - bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, - std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true, int nExtraPayloadSize = 0, bool fUseInstantSend=false); + void SignTransparentInputs(CMutableTransaction& tx, const std::vector& vInputTxs, bool fSign); + void CheckTransparentTransactionSanity(CMutableTransaction& tx, const std::vector& vInputTxs, + const CCoinControl* coinControl, CAmount nFee, bool fSign); + + bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, + const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, + bool fUseInstantSend, const std::vector& vTransparentTxouts); + + bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, + const CCoinControl* coinControl = NULL, bool sign = true, int nExtraPayloadSize = 0, + bool fUseInstantSend = false); /** * Add Mint and Spend functions @@ -1273,7 +1650,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface &address, const std::string &label, bool isMine, const std::string &purpose, ChangeType status)> NotifyRAPAddressBookChanged; - + /** * Wallet transaction added, removed or updated. * @note called with lock cs_wallet held. From d8e6161cd9c09eea2cef8c49fd45f07d30c42e65 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Sat, 18 Nov 2023 17:46:14 +0700 Subject: [PATCH 02/24] Allow spending unconfirmed funds from ourself again. --- src/wallet/coincontrol.h | 2 ++ src/wallet/wallet.cpp | 19 +++++++++++++++++++ src/wallet/wallet.h | 9 +++++++-- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 218d90776d..45ba11fbae 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -39,6 +39,8 @@ class CCoinControl CAmount nMinimumTotalFee; //! Override estimated feerate bool fOverrideFeeRate; + // Allow inputs from ourself that haven't been confirmed yet. + bool fAllowUnconfirmed = true; //! Feerate to use if overrideFeeRate is true CFeeRate nFeeRate; //! Override the default confirmation target, 0 = use default diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 50036cd3d9..e0760ded00 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -250,6 +250,25 @@ bool CTransparentTxout::IsCoinBase() const { return wallet->mapWallet.at(GetHash()).IsCoinBase(); } +bool CTransparentTxout::IsFromMe() const { + if (_isMockup) + return true; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + + for (const CTxIn& txin: wallet->mapWallet.at(GetHash()).tx->vin) { + std::map::const_iterator mi = wallet->mapWallet.find(txin.prevout.hash); + if (mi == wallet->mapWallet.end()) + return false; + + if (!(::IsMine(*wallet, mi->second.tx->vout.at(txin.prevout.n).scriptPubKey, SIGVERSION_BASE) & ISMINE_ALL)) + return false; + } + + return true; +} + unsigned int CTransparentTxout::GetDepthInMainChain() const { if (_isMockup) return _mockupDepthInMainChain; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bc0ee3a349..274cfea3e7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -672,6 +672,7 @@ class CTransparentTxout { bool IsCoinTypeCompatible(const CCoinControl* coinControl) const; bool IsLLMQInstantSendLocked() const; bool IsCoinBase() const; + bool IsFromMe() const; unsigned int GetDepthInMainChain() const; private: @@ -746,16 +747,20 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface for (const AbstractTxout& tx: vRelevantTransactions) { bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); + if (!tx.IsMine(coinControl)) continue; if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs && !isSelected) continue; if (!tx.IsSpendable()) continue; if (!tx.IsCoinTypeCompatible(coinControl)) continue; if (tx.IsAbandoned()) continue; - if (!tx.GetDepthInMainChain() && (!fUseInstantSend || !tx.IsLLMQInstantSendLocked())) continue; if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) continue; if (tx.IsCoinBase() && tx.GetDepthInMainChain() < COINBASE_MATURITY) continue; - if (!tx.IsMine(coinControl)) continue; + if ( + !tx.GetDepthInMainChain() && + (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && + ((coinControl && !coinControl->fAllowUnconfirmed) || !tx.IsFromMe()) + ) continue; if (isSelected) vCoinControlInputs.push_back(tx); else vAvailableInputs.push_back(tx); From 855a7cabda55a8504b383fa7f74915be73630838 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Sat, 18 Nov 2023 17:54:34 +0700 Subject: [PATCH 03/24] Check that locked coins are not spent. --- src/wallet/wallet.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 274cfea3e7..90c340d40a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -753,6 +753,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface if (!tx.IsSpendable()) continue; if (!tx.IsCoinTypeCompatible(coinControl)) continue; if (tx.IsAbandoned()) continue; + if (tx.IsLocked()) continue; if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) continue; if (tx.IsCoinBase() && tx.GetDepthInMainChain() < COINBASE_MATURITY) continue; From 3dc40034e2592b28cb2219f38ab7f2578da1870c Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Sat, 18 Nov 2023 18:04:10 +0700 Subject: [PATCH 04/24] Use configured tx fee. --- src/wallet/wallet.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e0760ded00..3e3dbb41ed 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4520,10 +4520,15 @@ CAmount CWallet::GetFee(const CCoinControl* coinControl, size_t txSize) { AssertLockHeld(cs_main); CAmount fee = GetRequiredFee(txSize); + if (coinControl && coinControl->fOverrideFeeRate) { CAmount override = coinControl->nFeeRate.GetFee(txSize); if (override < fee) throw std::runtime_error("nFeeRate is set too low; it will lead to creation of an unrelayable tx"); + } else { + CAmount fee_ = payTxFee.GetFee(txSize); + if (fee_ > fee) + fee = fee_; } if (coinControl && coinControl->nMinimumTotalFee > fee) From 0190c0d7525b63a9661b9d69cb9ca5868776c966 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:05:47 +0700 Subject: [PATCH 05/24] Add logic to reject long chains to CreateTransaction. --- src/wallet/wallet.cpp | 31 ++++++++++++++++++++++++++++++- src/wallet/wallet.h | 7 +++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3e3dbb41ed..e727838772 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -279,6 +279,22 @@ unsigned int CTransparentTxout::GetDepthInMainChain() const { return wallet->mapWallet.at(GetHash()).GetDepthInMainChain(); } +unsigned int CTransparentTxout::GetDepthInMempool() const { + if (_isMockup) + return GetDepthInMainChain() ? 0 : 1; + + assert(wallet); + AssertLockHeld(wallet->cs_wallet); + AssertLockHeld(mempool.cs); + + auto entry = mempool.mapTx.find(GetHash()); + if (entry == mempool.mapTx.end()) + return 0; + + uint64_t nAncestors = entry->GetCountWithAncestors(); + return nAncestors; +} + const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet @@ -4627,7 +4643,7 @@ void CWallet::CheckTransparentTransactionSanity(CMutableTransaction& tx, throw std::runtime_error("txFee > maxTxFee. This is probably a bug."); bool fHasDataOut = false; - for (CTxOut& txout: tx.vout) { + for (const CTxOut& txout: tx.vout) { txnouttype whichType; if (!::IsStandard(txout.scriptPubKey, whichType, false)) @@ -4663,6 +4679,18 @@ void CWallet::CheckTransparentTransactionSanity(CMutableTransaction& tx, for (CTxOut& txout: tx.vout) totalOutputValue += txout.nValue; assert(totalInputValue == totalOutputValue + nFee); assert(totalInputValue > 0); + + if (GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { + // We should probably not make these transactions in the first place, but throwing this exception instead is the + // backwards compatible behaviour. + size_t nTotalMempoolAncestors = 0; + + for (const CTransparentTxout& txin: vInputTxs) + nTotalMempoolAncestors += txin.GetDepthInMempool(); + + if (nTotalMempoolAncestors >= GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)) + throw std::runtime_error("mempool chain"); + } } // Create a transaction to vecSend, placing it in wtxNew. reservekey is the keypool. nFeeRet will be populated with the @@ -4692,6 +4720,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, bool fUseInstantSend, const std::vector& vTransparentTxouts) { + LOCK(mempool.cs); AssertLockHeld(cs_main); if (!coinControl || coinControl->destChange.which() == 0) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 90c340d40a..bc8ea30144 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -14,6 +14,7 @@ #include "tinyformat.h" #include "ui_interface.h" #include "utilstrencodings.h" +#include "validation.h" #include "validationinterface.h" #include "script/ismine.h" #include "script/sign.h" @@ -674,6 +675,7 @@ class CTransparentTxout { bool IsCoinBase() const; bool IsFromMe() const; unsigned int GetDepthInMainChain() const; + unsigned int GetDepthInMempool() const; private: const CWallet* wallet = nullptr; @@ -744,6 +746,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface if (coinControl && coinControl->nCoinType == CoinType::WITH_MINTS) throw std::runtime_error("CoinType::WITH_MINTS is not supported for any transactions."); + unsigned int nMaxMempoolDepth = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); + bool fRejectLongChains = GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); + for (const AbstractTxout& tx: vRelevantTransactions) { bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); @@ -762,6 +767,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && ((coinControl && !coinControl->fAllowUnconfirmed) || !tx.IsFromMe()) ) continue; + if (fRejectLongChains && tx.GetDepthInMempool() + 1 >= nMaxMempoolDepth) continue; if (isSelected) vCoinControlInputs.push_back(tx); else vAvailableInputs.push_back(tx); @@ -809,6 +815,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface nCollectedRet = 0; size_t nMaxSize = coinControl && coinControl->nMaxSize ? coinControl->nMaxSize : (MAX_STANDARD_TX_WEIGHT / 4); + size_t nMaxAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); if (nRequired < 0) throw std::runtime_error("Transaction amounts must be positive"); From 56cbcba53bf0f143af2d2c4deec20bc2770ddbd9 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:20:32 +0700 Subject: [PATCH 06/24] Respect strFromAccount in CreateTransaction. --- src/wallet/wallet.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e727838772..d86303b162 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4728,7 +4728,10 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT nFeeRet = -1; strFailReason = ""; + + std::string strFromAccount = wtxNew.strFromAccount; wtxNew = CWalletTx(); + wtxNew.strFromAccount = strFromAccount; CAmount nRequired = 0; size_t nConstantSize = 4 + // version From 4dd7e2c1e51e26cf77602c7be08259c7f508b06b Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Sat, 25 Nov 2023 19:31:19 +0700 Subject: [PATCH 07/24] Fix using coinbase inputs with exactly COINBASE_MATURITY confirmations. --- src/wallet/wallet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bc8ea30144..fafe09fe49 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -761,7 +761,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface if (tx.IsLocked()) continue; if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) continue; - if (tx.IsCoinBase() && tx.GetDepthInMainChain() < COINBASE_MATURITY) continue; + if (tx.IsCoinBase() && tx.GetDepthInMainChain() <= COINBASE_MATURITY) continue; if ( !tx.GetDepthInMainChain() && (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && From 47e3638e3af4fa34ad15efcf934738b4177d931d Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Mon, 27 Nov 2023 13:30:39 +0700 Subject: [PATCH 08/24] Respect -spendzeroconfchange in CreateTransaction. --- src/wallet/coincontrol.h | 2 +- src/wallet/wallet.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 45ba11fbae..83841bd6be 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -40,7 +40,7 @@ class CCoinControl //! Override estimated feerate bool fOverrideFeeRate; // Allow inputs from ourself that haven't been confirmed yet. - bool fAllowUnconfirmed = true; + std::optional fAllowUnconfirmed; //! Feerate to use if overrideFeeRate is true CFeeRate nFeeRate; //! Override the default confirmation target, 0 = use default diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fafe09fe49..840dc26f1b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -748,6 +748,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface unsigned int nMaxMempoolDepth = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); bool fRejectLongChains = GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); + bool fAllowUnconfirmed = (coinControl && coinControl->fAllowUnconfirmed.has_value()) ? + *coinControl->fAllowUnconfirmed : bSpendZeroConfChange; for (const AbstractTxout& tx: vRelevantTransactions) { bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); @@ -765,7 +767,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface if ( !tx.GetDepthInMainChain() && (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && - ((coinControl && !coinControl->fAllowUnconfirmed) || !tx.IsFromMe()) + (!fAllowUnconfirmed || !tx.IsFromMe()) ) continue; if (fRejectLongChains && tx.GetDepthInMempool() + 1 >= nMaxMempoolDepth) continue; From 957a021d4a4df93b0ac13461c95ece510eff0b76 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Mon, 27 Nov 2023 15:25:09 +0700 Subject: [PATCH 09/24] Make txn_clone.py test work with CreateTransaction. --- qa/rpc-tests/txn_clone.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/qa/rpc-tests/txn_clone.py b/qa/rpc-tests/txn_clone.py index e3fd3a0a5f..c6927471ff 100755 --- a/qa/rpc-tests/txn_clone.py +++ b/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() - From e9ca35195e82001390b9d78801063e9ebcc347f8 Mon Sep 17 00:00:00 2001 From: sproxet <17163658+sproxet@users.noreply.github.com> Date: Mon, 4 Dec 2023 14:33:23 +0700 Subject: [PATCH 10/24] Fix createtransaction_tests. --- src/wallet/test/createtransaction_tests.cpp | 22 ++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp index 2a9034778c..7922eac2d8 100644 --- a/src/wallet/test/createtransaction_tests.cpp +++ b/src/wallet/test/createtransaction_tests.cpp @@ -319,6 +319,10 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) vTxouts.at(0)._mockupDepthInMainChain = 0; + CCoinControl coinControl; + coinControl.fAllowUnconfirmedIsSet = true; + coinControl.fAllowUnconfirmed = false; + { CWalletTx wtx; CAmount nFeeRet = 0; @@ -327,7 +331,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) CReserveKey reservekey(pwalletMain); pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, true, vTxouts); + &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); } @@ -342,7 +346,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) CReserveKey reservekey(pwalletMain); pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, true, vTxouts); + &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); ASSERT_VIN_SIZE(2); @@ -423,8 +427,11 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) vTxouts.at(0)._mockupDepthInMainChain = 0; + CCoinControl coinControl; + coinControl.fAllowUnconfirmedIsSet = true; + coinControl.fAllowUnconfirmed = false; + { - CCoinControl coinControl; CWalletTx wtx; CAmount nFeeRet = 0; int nChangePosInOut = -1; @@ -432,7 +439,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) CReserveKey reservekey(pwalletMain); pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, true, vTxouts); + &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); } @@ -447,7 +454,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) CReserveKey reservekey(pwalletMain); pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, true, vTxouts); + &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -468,7 +475,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) CReserveKey reservekey(pwalletMain); pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, false, vTxouts); + &coinControl, true, 0, false, vTxouts); ASSERT_FAILURE("Insufficient funds"); } @@ -477,7 +484,8 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(change_position) { ACQUIRE_LOCKS(); - std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, 1 << 13}); + std::vector vTxouts = GetFakeTransparentTxouts({1 << 17, 1 << 16, 1 << 15, 1 << 14, + 1 << 13}); std::vector vRecipients = GetFakeRecipients({1 << 17, 1 << 13, 1 << 16}); { From d5e10cdb9f14f98f2ae9255fa816e4fd8495f096 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Mon, 29 Jan 2024 08:48:13 +0100 Subject: [PATCH 11/24] Fix patrial rebase --- src/wallet/coincontrol.h | 2 ++ src/wallet/test/createtransaction_tests.cpp | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 83841bd6be..cc9155b776 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -6,6 +6,8 @@ #define BITCOIN_WALLET_COINCONTROL_H #include "primitives/transaction.h" +#include "script/standard.h" +#include enum class CoinType { diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp index 7922eac2d8..b527362ff3 100644 --- a/src/wallet/test/createtransaction_tests.cpp +++ b/src/wallet/test/createtransaction_tests.cpp @@ -320,7 +320,6 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) vTxouts.at(0)._mockupDepthInMainChain = 0; CCoinControl coinControl; - coinControl.fAllowUnconfirmedIsSet = true; coinControl.fAllowUnconfirmed = false; { @@ -428,7 +427,6 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) vTxouts.at(0)._mockupDepthInMainChain = 0; CCoinControl coinControl; - coinControl.fAllowUnconfirmedIsSet = true; coinControl.fAllowUnconfirmed = false; { From ce3bf31ddf843dbe116d8f7d2daeb091fbef74b2 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Thu, 1 Feb 2024 08:51:09 +0100 Subject: [PATCH 12/24] Exchange address support in CTransparentTxout --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d86303b162..cb627f6d87 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -127,6 +127,7 @@ size_t CTransparentTxout::GetMarginalSpendSize(std::vector& p case TX_SCRIPTHASH: case TX_PUBKEYHASH: + case TX_EXCHANGEADDRESS: sigDataSize = 107; break; From 08f2642ab6d087a8c3f8de5ac571bdd48fa14e83 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Tue, 27 Feb 2024 22:30:56 +0100 Subject: [PATCH 13/24] Moved large template function to .cpp file --- src/wallet/wallet.cpp | 336 ++++++++++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 311 +------------------------------------- 2 files changed, 338 insertions(+), 309 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cb627f6d87..96e94b2fdf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4869,6 +4869,342 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT return true; } +template +void CWallet::GetAvailableInputs(const std::vector& vRelevantTransactions, + std::vector& vAvailableInputs, + std::vector& vCoinControlInputs, + const CCoinControl* coinControl, + bool fUseInstantSend) const { + AssertLockHeld(cs_wallet); + vAvailableInputs.clear(); + vCoinControlInputs.clear(); + + if (coinControl && coinControl->nCoinType == CoinType::ONLY_NONDENOMINATED_NOT1000IFMN && !fMasternodeMode) + throw std::runtime_error("fMasternode must be enabled to use CoinType::ONLY_NONDENOMINATED_NOT1000IFMN"); + + if (coinControl && coinControl->nCoinType == CoinType::WITH_MINTS) + throw std::runtime_error("CoinType::WITH_MINTS is not supported for any transactions."); + + unsigned int nMaxMempoolDepth = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); + bool fRejectLongChains = GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); + bool fAllowUnconfirmed = (coinControl && coinControl->fAllowUnconfirmed.has_value()) ? + *coinControl->fAllowUnconfirmed : bSpendZeroConfChange; + + for (const AbstractTxout& tx: vRelevantTransactions) { + bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); + + if (!tx.IsMine(coinControl)) continue; + if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs && !isSelected) + continue; + if (!tx.IsSpendable()) continue; + if (!tx.IsCoinTypeCompatible(coinControl)) continue; + if (tx.IsAbandoned()) continue; + if (tx.IsLocked()) continue; + if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) + continue; + if (tx.IsCoinBase() && tx.GetDepthInMainChain() <= COINBASE_MATURITY) continue; + if ( + !tx.GetDepthInMainChain() && + (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && + (!fAllowUnconfirmed || !tx.IsFromMe()) + ) continue; + if (fRejectLongChains && tx.GetDepthInMempool() + 1 >= nMaxMempoolDepth) continue; + + if (isSelected) vCoinControlInputs.push_back(tx); + else vAvailableInputs.push_back(tx); + } + + if (coinControl) { + if (vCoinControlInputs.size() != coinControl->setSelected.size()) + throw std::runtime_error("Some coin control inputs could not be selected."); + if (coinControl->fRequireAllInputs && coinControl->nMaxInputs && + vCoinControlInputs.size() > coinControl->nMaxInputs) + throw std::runtime_error("The number of selected inputs exceeds the maximum number of inputs."); + } + + // Sort vAvailable and vCoinControlInputs by largest first. Additionally, order it so that transaction selection + // will be deterministic; this property is not otherwise required. + for (std::vector* v: {&vAvailableInputs, &vCoinControlInputs}) { + std::sort(v->begin(), v->end(), [](const AbstractTxout& a, const AbstractTxout& b) { + if (a.GetValue() != b.GetValue()) + return a.GetValue() > b.GetValue(); + if (a.GetHash() != b.GetHash()) + return a.GetHash().Compare(b.GetHash()) == -1; + return a.GetOutpoint().n < b.GetOutpoint().n; + }); + } +} + +// explicit instantiation +template +void CWallet::GetAvailableInputs(const std::vector& vRelevantTransactions, + std::vector& vAvailableInputs, + std::vector& vCoinControlInputs, + const CCoinControl* coinControl, + bool fUseInstantSend) const; + +template +bool CWallet::GetInputsForTx(const std::vector& vRelevantTransactions, std::vector& vInputs, + CAmount& nFeeRet, CAmount& nCollectedRet, CAmount nRequired, size_t nConstantSize, + const CCoinControl* coinControl, bool fUseInstantSend, bool fSubtractFeeFromAmount, + bool fAllowPartial /*=false*/, size_t nChangeSize /*=34*/) { + AssertLockHeld(cs_wallet); + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + + size_t nMaxSize = coinControl && coinControl->nMaxSize ? coinControl->nMaxSize : (MAX_STANDARD_TX_WEIGHT / 4); + size_t nMaxAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); + + if (nRequired < 0) + throw std::runtime_error("Transaction amounts must be positive"); + + if (coinControl && coinControl->nMinimumTotalFee < 0) + throw std::runtime_error("Minimum total fee must be positive"); + + if (coinControl && coinControl->nConfirmTarget && coinControl->nConfirmTarget < 0) + throw std::runtime_error("nConfirmTarget must be positive if set."); + + std::vector vCoinControlInputs; + // vAvailable contains the available inputs that are not in vCoinControlInputs. + std::vector vAvailable; + GetAvailableInputs(vRelevantTransactions, vAvailable, vCoinControlInputs, coinControl, fUseInstantSend); + + // This algorithm will first pick all the transactions selected in coinControl. If + // coinControl->fRequireAllInputs is not set, it will stop when it has enough to provide for our outputs + // otherwise it will consume all the inputs. After that, it will select the smallest UTXO in our wallet, and end + // if enough value is found. Then, if there is a UTXO which combined with the smallest that can provide for our + // entire output value, that UTXO. If there is not, it will then pick the largest UTXO, and then the next + // smallest, and so on and so forth until we can fulfill our output requirements, or until we reach nMaxSize or + // coinControl->nMaxInputs limits. Once we reach those, we will start replacing the largest small inputs we have + // chosen with the largest inputs we haven't selected yet. If that fails, it is the case that the transactions + // in our wallet are not sufficient to provide for the outputs requested within the coinControl and nMaxSize + // constraints, so we will therefore fail. + size_t txSize = nConstantSize + 1; + size_t iFront = 0; + size_t iBack = 0; + size_t iCoinControl = 0; + bool fTakeFromFront = false; + bool fTrySkipFront = true; + bool fReplace = false; + std::vector vSmallInputs; + while (true) { + // The idea here is to front-side inputs which are larger than the smallest front-side input that can + // fulfill our output value requirements. + if (fTrySkipFront && iCoinControl == vCoinControlInputs.size()) { + if (iFront) { + // If iFront is already set and we're here at another iteration of this loop, the input we last + // identified was too small to fulfill our requirements. We're therefore going to undo adding it and + // try again with the next larger input. + + assert(!vInputs.empty()); + AbstractTxout oldTxout = vInputs.back(); + vInputs.pop_back(); + + size_t inputSize = oldTxout.GetMarginalSpendSize(vInputs); + // If inputSize here is 0, nCollectedRet and txSize will have never been mutated. + if (inputSize) { + nCollectedRet -= oldTxout.GetValue(); + txSize -= inputSize; + } + + iFront -= 1; + } else { + // If we're at the first iteration, we'll identify the smallest input that's larger than nRequired + // and start looking for inputs from that. + for (const AbstractTxout& txo: vAvailable) { + if (txo.GetValue() > nRequired) iFront++; + else break; + } + if (iFront) iFront--; + + // If we got to the last element of vAvailable, which we have already added as an input element + // previously, we don't have any UTXOs which will fulfill our requirements. Therefore, we'll stop + // using the fTrySkipFront logic and pick the largest UTXOs we have. + if (iFront + 1 == vAvailable.size()) iFront = 0; + } + + // If iFront here is at 0, we've reached the end and want to try our normal logic. + if (!iFront) fTrySkipFront = false; + + // If we've done an iteration of fTrySkipFront logic before, fTakeFromFront will be false, but we need + // it to be true as this is a do-over. + fTakeFromFront = true; + } + + if (coinControl && coinControl->nMaxInputs && coinControl->nMaxInputs == vInputs.size()) + fReplace = true; + + AbstractTxout txout; + bool hasReplacedTxout = false; + // If hasReplacedTxout is false, the value of iToReplace is meaningless. + size_t iToReplace = 0; + if (iCoinControl != vCoinControlInputs.size()) { + // Select coin control inputs before dealing with other inputs. + + txout = vCoinControlInputs.at(iCoinControl++); + } else if (fReplace) { + // If this code is reached, the value of iBack and fTakeFromFront no longer carries any significance. + iBack = SIZE_MAX; + fTakeFromFront = false; + + // If we have reached the nMaxInputs or nMaxSize limit, we will replace the largest input that we've + // selected from the back of vAvailable with an input from the front of vAvailable. + + if (vSmallInputs.empty()) break; + if (iFront == vAvailable.size()) break; + + iToReplace = vSmallInputs.back(); + vSmallInputs.pop_back(); + + AbstractTxout oldTxout = vInputs.at(iToReplace); + vInputs.erase(vInputs.begin() + iToReplace); + + nCollectedRet -= oldTxout.GetValue(); + txSize -= oldTxout.GetMarginalSpendSize(vInputs); + + AbstractTxout newTxout = vAvailable.at(iFront++); + txSize += newTxout.GetMarginalSpendSize(vInputs); + + vInputs.insert(vInputs.begin() + iToReplace, newTxout); + hasReplacedTxout = true; + txout = vInputs.at(iToReplace); + } else if (iFront + iBack == vAvailable.size()) { + // We've selected all possible inputs and still can't come up with the required amount. Fail. + + break; + } else if (fTakeFromFront) { + txout = vAvailable.at(iFront++); + fTakeFromFront = false; + } else { + txout = vAvailable.at(vAvailable.size() - ++iBack); + fTakeFromFront = true; + } + + size_t inputSize = 0; + try { + inputSize = txout.GetMarginalSpendSize(vInputs); + } catch (std::runtime_error& e) { + LogPrintf("%s(): Unexpectedly failed to determine spend size for %s-%d\n", __func__, + txout.GetOutpoint().hash.GetHex(), txout.GetOutpoint().n); + + if (coinControl && coinControl->IsSelected(txout.GetOutpoint())) + throw std::runtime_error("The spend size of a coin control input could not be determined."); + + // If we push this to the back of vSmallInputs, it will be replaced in the next iteration as the + // condition of the first if statement at the beginning of this loop will be fulfilled. nCollectedRet + // has been mutated above so that the undo in the first if block will be valid, and txSize will not be + // modified in the event we can't get the input size for this txo. + if (hasReplacedTxout) vSmallInputs.emplace_back(iToReplace); + + continue; + } + + // If this transaction will bring us over nMaxSize, don't add it and start transaction replacement logic + // above. It is technically possible for our transaction finding logic to fail to find a possible solution + // in the event that a lower value transaction has a smaller spend script, and the difference in fees + // between the two exceeds the difference in value, and this is true for every transaction larger than the + // transaction with the smaller spend script, and the difference is over what we need to get to nRequired, + // but this case should be extremely unlikely. + if (txSize + inputSize > nMaxSize) { + // If we start replacement logic, we want to keep this transaction available as an option. + if (!fReplace) + // fTakeFromFront is true if we last took from the BACK of vAvailable. + fTakeFromFront ? iBack-- : iFront--; + + fReplace = true; + continue; + } + + if (!hasReplacedTxout) { + // fTakeFromFront is true if we last took from the BACK of vAvailable. + if (fTakeFromFront) + vSmallInputs.emplace_back(vInputs.size()); + + + txSize += inputSize; + vInputs.emplace_back(txout); + } + + nCollectedRet += txout.GetValue(); + + nFeeRet = GetFee(coinControl, txSize); + + // If coin control is enabled, we want to select all inputs, so we will only check whether we have enough + // after exhausting vAvailable. + if (coinControl && coinControl->fRequireAllInputs && iCoinControl != vCoinControlInputs.size()) continue; + + // This is here so we will not return in the event that we want to subtract the fee from the amount and the + // amount collected would be sufficient to cover nRequired, but not sufficient to cover the fee. Note that + // this will allow outputs (or even entire transactions) with 0 output value. + if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; + + CAmount extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; + // In this case, we don't need to make a change output, so we don't have to add the cost of a change output. + if (nCollectedRet == nRequired + extraFee) return true; + if ( + nCollectedRet >= nRequired + extraFee && + nCollectedRet <= nRequired + GetFee(coinControl, txSize + nChangeSize) + ) { + nFeeRet = nCollectedRet - nRequired; + return true; + } + + // In this case, we have a change output too. + + if (txSize + nChangeSize > nMaxSize) { + // fTakeFromFront is true if we last took from the BACK of vAvailable. + if (!fReplace) { + if (fTakeFromFront) { + vSmallInputs.pop_back(); + iBack--; + } else { + iFront--; + } + + vInputs.pop_back(); + + nCollectedRet -= txout.GetValue(); + txSize -= inputSize; + } + + fReplace = true; + continue; + } + + nFeeRet = GetFee(coinControl, txSize + nChangeSize); + extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; + if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; + if (nCollectedRet >= extraFee + nRequired) return true; + } + + if (fAllowPartial) { + if (nFeeRet > nCollectedRet) { + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + } else if (nCollectedRet + (fSubtractFeeFromAmount ? 0 : nFeeRet) >= nRequired) { + nFeeRet = GetFee(coinControl, txSize); + } + + return false; + } + + // If we've gotten this far, we don't have the funds to make the transaction. + vInputs.clear(); + nFeeRet = 0; + nCollectedRet = 0; + throw std::runtime_error("Insufficient funds"); +} + +// explicit instantiation +template +bool CWallet::GetInputsForTx(const std::vector& vRelevantTransactions, std::vector& vInputs, + CAmount& nFeeRet, CAmount& nCollectedRet, CAmount nRequired, size_t nConstantSize, + const CCoinControl* coinControl, bool fUseInstantSend, bool fSubtractFeeFromAmount, + bool fAllowPartial, size_t nChangeSize); + + /** * Call after CreateTransaction unless you want to abort */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 840dc26f1b..7a9bcc9526 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -735,66 +735,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface std::vector& vAvailableInputs, std::vector& vCoinControlInputs, const CCoinControl* coinControl, - bool fUseInstantSend) const { - AssertLockHeld(cs_wallet); - vAvailableInputs.clear(); - vCoinControlInputs.clear(); - - if (coinControl && coinControl->nCoinType == CoinType::ONLY_NONDENOMINATED_NOT1000IFMN && !fMasternodeMode) - throw std::runtime_error("fMasternode must be enabled to use CoinType::ONLY_NONDENOMINATED_NOT1000IFMN"); - - if (coinControl && coinControl->nCoinType == CoinType::WITH_MINTS) - throw std::runtime_error("CoinType::WITH_MINTS is not supported for any transactions."); - - unsigned int nMaxMempoolDepth = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - bool fRejectLongChains = GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); - bool fAllowUnconfirmed = (coinControl && coinControl->fAllowUnconfirmed.has_value()) ? - *coinControl->fAllowUnconfirmed : bSpendZeroConfChange; - - for (const AbstractTxout& tx: vRelevantTransactions) { - bool isSelected = coinControl && coinControl->HasSelected() && coinControl->IsSelected(tx.GetOutpoint()); - - if (!tx.IsMine(coinControl)) continue; - if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs && !isSelected) - continue; - if (!tx.IsSpendable()) continue; - if (!tx.IsCoinTypeCompatible(coinControl)) continue; - if (tx.IsAbandoned()) continue; - if (tx.IsLocked()) continue; - if (coinControl && coinControl->nConfirmTarget && tx.GetDepthInMainChain() < coinControl->nConfirmTarget) - continue; - if (tx.IsCoinBase() && tx.GetDepthInMainChain() <= COINBASE_MATURITY) continue; - if ( - !tx.GetDepthInMainChain() && - (!fUseInstantSend || !tx.IsLLMQInstantSendLocked()) && - (!fAllowUnconfirmed || !tx.IsFromMe()) - ) continue; - if (fRejectLongChains && tx.GetDepthInMempool() + 1 >= nMaxMempoolDepth) continue; - - if (isSelected) vCoinControlInputs.push_back(tx); - else vAvailableInputs.push_back(tx); - } - - if (coinControl) { - if (vCoinControlInputs.size() != coinControl->setSelected.size()) - throw std::runtime_error("Some coin control inputs could not be selected."); - if (coinControl->fRequireAllInputs && coinControl->nMaxInputs && - vCoinControlInputs.size() > coinControl->nMaxInputs) - throw std::runtime_error("The number of selected inputs exceeds the maximum number of inputs."); - } - - // Sort vAvailable and vCoinControlInputs by largest first. Additionally, order it so that transaction selection - // will be deterministic; this property is not otherwise required. - for (std::vector* v: {&vAvailableInputs, &vCoinControlInputs}) { - std::sort(v->begin(), v->end(), [](const AbstractTxout& a, const AbstractTxout& b) { - if (a.GetValue() != b.GetValue()) - return a.GetValue() > b.GetValue(); - if (a.GetHash() != b.GetHash()) - return a.GetHash().Compare(b.GetHash()) == -1; - return a.GetOutpoint().n < b.GetOutpoint().n; - }); - } - } + bool fUseInstantSend) const; // This populates vInputs with AbstractTxouts from vRelevantTransactions required to produce a transaction to the // given specification. nRequired is the total amount of value we need to produce (either including or excluding fee @@ -810,255 +751,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool GetInputsForTx(const std::vector& vRelevantTransactions, std::vector& vInputs, CAmount& nFeeRet, CAmount& nCollectedRet, CAmount nRequired, size_t nConstantSize, const CCoinControl* coinControl, bool fUseInstantSend, bool fSubtractFeeFromAmount, - bool fAllowPartial=false, size_t nChangeSize=34) { - AssertLockHeld(cs_wallet); - vInputs.clear(); - nFeeRet = 0; - nCollectedRet = 0; - - size_t nMaxSize = coinControl && coinControl->nMaxSize ? coinControl->nMaxSize : (MAX_STANDARD_TX_WEIGHT / 4); - size_t nMaxAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - - if (nRequired < 0) - throw std::runtime_error("Transaction amounts must be positive"); - - if (coinControl && coinControl->nMinimumTotalFee < 0) - throw std::runtime_error("Minimum total fee must be positive"); - - if (coinControl && coinControl->nConfirmTarget && coinControl->nConfirmTarget < 0) - throw std::runtime_error("nConfirmTarget must be positive if set."); - - std::vector vCoinControlInputs; - // vAvailable contains the available inputs that are not in vCoinControlInputs. - std::vector vAvailable; - GetAvailableInputs(vRelevantTransactions, vAvailable, vCoinControlInputs, coinControl, fUseInstantSend); - - // This algorithm will first pick all the transactions selected in coinControl. If - // coinControl->fRequireAllInputs is not set, it will stop when it has enough to provide for our outputs - // otherwise it will consume all the inputs. After that, it will select the smallest UTXO in our wallet, and end - // if enough value is found. Then, if there is a UTXO which combined with the smallest that can provide for our - // entire output value, that UTXO. If there is not, it will then pick the largest UTXO, and then the next - // smallest, and so on and so forth until we can fulfill our output requirements, or until we reach nMaxSize or - // coinControl->nMaxInputs limits. Once we reach those, we will start replacing the largest small inputs we have - // chosen with the largest inputs we haven't selected yet. If that fails, it is the case that the transactions - // in our wallet are not sufficient to provide for the outputs requested within the coinControl and nMaxSize - // constraints, so we will therefore fail. - size_t txSize = nConstantSize + 1; - size_t iFront = 0; - size_t iBack = 0; - size_t iCoinControl = 0; - bool fTakeFromFront = false; - bool fTrySkipFront = true; - bool fReplace = false; - std::vector vSmallInputs; - while (true) { - // The idea here is to front-side inputs which are larger than the smallest front-side input that can - // fulfill our output value requirements. - if (fTrySkipFront && iCoinControl == vCoinControlInputs.size()) { - if (iFront) { - // If iFront is already set and we're here at another iteration of this loop, the input we last - // identified was too small to fulfill our requirements. We're therefore going to undo adding it and - // try again with the next larger input. - - assert(!vInputs.empty()); - AbstractTxout oldTxout = vInputs.back(); - vInputs.pop_back(); - - size_t inputSize = oldTxout.GetMarginalSpendSize(vInputs); - // If inputSize here is 0, nCollectedRet and txSize will have never been mutated. - if (inputSize) { - nCollectedRet -= oldTxout.GetValue(); - txSize -= inputSize; - } - - iFront -= 1; - } else { - // If we're at the first iteration, we'll identify the smallest input that's larger than nRequired - // and start looking for inputs from that. - for (const AbstractTxout& txo: vAvailable) { - if (txo.GetValue() > nRequired) iFront++; - else break; - } - if (iFront) iFront--; - - // If we got to the last element of vAvailable, which we have already added as an input element - // previously, we don't have any UTXOs which will fulfill our requirements. Therefore, we'll stop - // using the fTrySkipFront logic and pick the largest UTXOs we have. - if (iFront + 1 == vAvailable.size()) iFront = 0; - } - - // If iFront here is at 0, we've reached the end and want to try our normal logic. - if (!iFront) fTrySkipFront = false; - - // If we've done an iteration of fTrySkipFront logic before, fTakeFromFront will be false, but we need - // it to be true as this is a do-over. - fTakeFromFront = true; - } - - if (coinControl && coinControl->nMaxInputs && coinControl->nMaxInputs == vInputs.size()) - fReplace = true; - - AbstractTxout txout; - bool hasReplacedTxout = false; - // If hasReplacedTxout is false, the value of iToReplace is meaningless. - size_t iToReplace = 0; - if (iCoinControl != vCoinControlInputs.size()) { - // Select coin control inputs before dealing with other inputs. - - txout = vCoinControlInputs.at(iCoinControl++); - } else if (fReplace) { - // If this code is reached, the value of iBack and fTakeFromFront no longer carries any significance. - iBack = SIZE_MAX; - fTakeFromFront = false; - - // If we have reached the nMaxInputs or nMaxSize limit, we will replace the largest input that we've - // selected from the back of vAvailable with an input from the front of vAvailable. - - if (vSmallInputs.empty()) break; - if (iFront == vAvailable.size()) break; - - iToReplace = vSmallInputs.back(); - vSmallInputs.pop_back(); - - AbstractTxout oldTxout = vInputs.at(iToReplace); - vInputs.erase(vInputs.begin() + iToReplace); - - nCollectedRet -= oldTxout.GetValue(); - txSize -= oldTxout.GetMarginalSpendSize(vInputs); - - AbstractTxout newTxout = vAvailable.at(iFront++); - txSize += newTxout.GetMarginalSpendSize(vInputs); - - vInputs.insert(vInputs.begin() + iToReplace, newTxout); - hasReplacedTxout = true; - txout = vInputs.at(iToReplace); - } else if (iFront + iBack == vAvailable.size()) { - // We've selected all possible inputs and still can't come up with the required amount. Fail. - - break; - } else if (fTakeFromFront) { - txout = vAvailable.at(iFront++); - fTakeFromFront = false; - } else { - txout = vAvailable.at(vAvailable.size() - ++iBack); - fTakeFromFront = true; - } - - size_t inputSize = 0; - try { - inputSize = txout.GetMarginalSpendSize(vInputs); - } catch (std::runtime_error& e) { - LogPrintf("%s(): Unexpectedly failed to determine spend size for %s-%d\n", __func__, - txout.GetOutpoint().hash.GetHex(), txout.GetOutpoint().n); - - if (coinControl && coinControl->IsSelected(txout.GetOutpoint())) - throw std::runtime_error("The spend size of a coin control input could not be determined."); - - // If we push this to the back of vSmallInputs, it will be replaced in the next iteration as the - // condition of the first if statement at the beginning of this loop will be fulfilled. nCollectedRet - // has been mutated above so that the undo in the first if block will be valid, and txSize will not be - // modified in the event we can't get the input size for this txo. - if (hasReplacedTxout) vSmallInputs.emplace_back(iToReplace); - - continue; - } - - // If this transaction will bring us over nMaxSize, don't add it and start transaction replacement logic - // above. It is technically possible for our transaction finding logic to fail to find a possible solution - // in the event that a lower value transaction has a smaller spend script, and the difference in fees - // between the two exceeds the difference in value, and this is true for every transaction larger than the - // transaction with the smaller spend script, and the difference is over what we need to get to nRequired, - // but this case should be extremely unlikely. - if (txSize + inputSize > nMaxSize) { - // If we start replacement logic, we want to keep this transaction available as an option. - if (!fReplace) - // fTakeFromFront is true if we last took from the BACK of vAvailable. - fTakeFromFront ? iBack-- : iFront--; - - fReplace = true; - continue; - } - - if (!hasReplacedTxout) { - // fTakeFromFront is true if we last took from the BACK of vAvailable. - if (fTakeFromFront) - vSmallInputs.emplace_back(vInputs.size()); - - - txSize += inputSize; - vInputs.emplace_back(txout); - } - - nCollectedRet += txout.GetValue(); - - nFeeRet = GetFee(coinControl, txSize); - - // If coin control is enabled, we want to select all inputs, so we will only check whether we have enough - // after exhausting vAvailable. - if (coinControl && coinControl->fRequireAllInputs && iCoinControl != vCoinControlInputs.size()) continue; - - // This is here so we will not return in the event that we want to subtract the fee from the amount and the - // amount collected would be sufficient to cover nRequired, but not sufficient to cover the fee. Note that - // this will allow outputs (or even entire transactions) with 0 output value. - if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; - - CAmount extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; - // In this case, we don't need to make a change output, so we don't have to add the cost of a change output. - if (nCollectedRet == nRequired + extraFee) return true; - if ( - nCollectedRet >= nRequired + extraFee && - nCollectedRet <= nRequired + GetFee(coinControl, txSize + nChangeSize) - ) { - nFeeRet = nCollectedRet - nRequired; - return true; - } - - // In this case, we have a change output too. - - if (txSize + nChangeSize > nMaxSize) { - // fTakeFromFront is true if we last took from the BACK of vAvailable. - if (!fReplace) { - if (fTakeFromFront) { - vSmallInputs.pop_back(); - iBack--; - } else { - iFront--; - } - - vInputs.pop_back(); - - nCollectedRet -= txout.GetValue(); - txSize -= inputSize; - } - - fReplace = true; - continue; - } - - nFeeRet = GetFee(coinControl, txSize + nChangeSize); - extraFee = fSubtractFeeFromAmount ? 0 : nFeeRet; - if (fSubtractFeeFromAmount && nFeeRet > nCollectedRet) continue; - if (nCollectedRet >= extraFee + nRequired) return true; - } - - if (fAllowPartial) { - if (nFeeRet > nCollectedRet) { - vInputs.clear(); - nFeeRet = 0; - nCollectedRet = 0; - } else if (nCollectedRet + (fSubtractFeeFromAmount ? 0 : nFeeRet) >= nRequired) { - nFeeRet = GetFee(coinControl, txSize); - } - - return false; - } - - // If we've gotten this far, we don't have the funds to make the transaction. - vInputs.clear(); - nFeeRet = 0; - nCollectedRet = 0; - throw std::runtime_error("Insufficient funds"); - } + bool fAllowPartial=false, size_t nChangeSize=34); CWalletDB *pwalletdbEncryption; From ea6326d0d7eb0e60e28fa01bb18dc9c607d686d9 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Fri, 1 Mar 2024 12:27:13 +0100 Subject: [PATCH 14/24] Changed visibility of CCoinControl members --- src/wallet/coincontrol.h | 10 ++++++++-- src/wallet/wallet.cpp | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index cc9155b776..4f47dce5b5 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -26,8 +26,6 @@ enum class CoinType class CCoinControl { public: - std::set setSelected; - CTxDestination destChange; //! If true, don't use any change bool fNoChange; @@ -105,6 +103,14 @@ class CCoinControl { vOutpoints.assign(setSelected.begin(), setSelected.end()); } + + size_t GetSelectedSize() const + { + return setSelected.size(); + } + +private: + std::set setSelected; }; #endif // BITCOIN_WALLET_COINCONTROL_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 96e94b2fdf..dba60bcb9b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -405,7 +405,7 @@ CPubKey CWallet::GetKeyFromKeypath(uint32_t nChange, uint32_t nChild, CKey& secr MnemonicContainer mContainer = mnemonicContainer; DecryptMnemonicContainer(mContainer); SecureVector seed = mContainer.GetSeed(); - masterKey.SetMaster(&seed.at(0), seed.size()); + masterKey.SetMaster(seed.data(), seed.size()); } else { // try to get the master key if (!GetKey(hdChain.masterKeyID, key)) @@ -4915,7 +4915,7 @@ void CWallet::GetAvailableInputs(const std::vector& vRelevantTran } if (coinControl) { - if (vCoinControlInputs.size() != coinControl->setSelected.size()) + if (vCoinControlInputs.size() != coinControl->GetSelectedSize()) throw std::runtime_error("Some coin control inputs could not be selected."); if (coinControl->fRequireAllInputs && coinControl->nMaxInputs && vCoinControlInputs.size() > coinControl->nMaxInputs) From beb159761d6bcf785326ce1f1feb812e21e78c16 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Fri, 1 Mar 2024 12:56:55 +0100 Subject: [PATCH 15/24] Changed signature of CreateTransaction() function --- src/qt/walletmodel.cpp | 2 +- src/rpc/rpcevo.cpp | 2 +- src/wallet/rpcwallet.cpp | 4 +- src/wallet/test/createtransaction_tests.cpp | 89 +++++++++++---------- src/wallet/wallet.cpp | 11 +-- src/wallet/wallet.h | 4 +- 6 files changed, 55 insertions(+), 57 deletions(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 898bc3e2af..66a8538941 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -341,7 +341,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact CWalletTx *newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); - bool fCreated = wallet->CreateTransaction(vecSend, *newTx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); + bool fCreated = wallet->CreateTransaction(vecSend, *newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && fCreated) transaction.reassignAmounts(nChangePosRet); diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index 716aaf163f..6037b4835f 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -213,7 +213,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci int nChangePos = -1; std::string strFailReason; - if (!pwallet->CreateTransaction(vecSend, wtx, &reservekey, nFee, nChangePos, strFailReason, &coinControl, false, tx.vExtraPayload.size())) { + if (!pwallet->CreateTransaction(vecSend, wtx, reservekey, nFee, nChangePos, strFailReason, &coinControl, false, tx.vExtraPayload.size())) { throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8548c2bca1..a5ade6b12f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -509,7 +509,7 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA int nChangePosRet = -1; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); - if (!pwallet->CreateTransaction(vecSend, wtxNew, &reservekey, nFeeRequired, nChangePosRet, strError)) { + if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, nChangePosRet, strError)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); throw JSONRPCError(RPC_WALLET_ERROR, strError); @@ -1266,7 +1266,7 @@ UniValue sendmany(const JSONRPCRequest& request) CAmount nFeeRequired = 0; int nChangePosRet = -1; std::string strFailReason; - bool fCreated = pwallet->CreateTransaction(vecSend, wtx, &keyChange, nFeeRequired, nChangePosRet, strFailReason); + bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp index b527362ff3..30bdf35ced 100644 --- a/src/wallet/test/createtransaction_tests.cpp +++ b/src/wallet/test/createtransaction_tests.cpp @@ -151,7 +151,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -177,7 +177,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -206,7 +206,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -229,7 +229,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); BOOST_ASSERT(nChangePosInOut == -1); @@ -257,7 +257,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -283,7 +283,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -298,7 +298,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -329,7 +329,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -344,7 +344,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -371,7 +371,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); @@ -403,7 +403,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); @@ -436,7 +436,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -451,7 +451,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -472,7 +472,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, false, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -494,7 +494,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -520,7 +520,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -546,7 +546,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -572,7 +572,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); BOOST_ASSERT(nChangePosInOut == -1); @@ -587,7 +587,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); BOOST_ASSERT(nChangePosInOut == -1); @@ -612,7 +612,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); BOOST_ASSERT(strFailReason == "Insufficient funds"); @@ -628,7 +628,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.fAllowWatchOnly = true; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -650,7 +650,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.fAllowWatchOnly = true; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Signing transaction failed"); @@ -671,7 +671,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, nullptr, true, 0, true, vTxouts); @@ -688,7 +688,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nCoinType = CoinType::ONLY_1000; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -714,7 +714,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) fMasternodeMode = false; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("fMasternode must be enabled to use CoinType::ONLY_NONDENOMINATED_NOT1000IFMN"); @@ -735,7 +735,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) fMasternodeMode = true; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -759,7 +759,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) fMasternodeMode = true; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -786,7 +786,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nCoinType = CoinType::WITH_1000; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -811,7 +811,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nCoinType = CoinType::WITH_1000; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -828,7 +828,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.fAllowOtherInputs = true; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Some coin control inputs could not be selected."); @@ -852,7 +852,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMaxInputs = 2; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -869,7 +869,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMaxInputs = 3; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -903,7 +903,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMaxInputs = 3; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -936,7 +936,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMaxSize = n3TxSize - 1; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); @@ -964,7 +964,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMaxSize = n2TxSize - 1; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -986,7 +986,8 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.destChange = GetRandomDest(); - pwalletMain->CreateTransaction(vRecipients, wtx, nullptr, nFeeRet, nChangePosInOut, strFailReason, + CReserveKey reservekey(nullptr); + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1015,7 +1016,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMinimumTotalFee = 1 << 13; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1037,7 +1038,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMinimumTotalFee = 1 << 20; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -1053,7 +1054,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nMinimumTotalFee = 100; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1084,7 +1085,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nConfirmTarget = 5; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_FAILURE("Insufficient funds"); @@ -1102,7 +1103,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.nConfirmTarget = 5; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1133,7 +1134,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.Select(vTxouts.at(1).GetOutpoint()); CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1158,7 +1159,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) coinControl.Select(vTxouts.at(1).GetOutpoint()); CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); ASSERT_SUCCESS(); @@ -1189,7 +1190,7 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::string strFailReason; CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, &reservekey, nFeeRet, nChangePosInOut, + pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, true, 0, true, vTxouts); if (nOurs >= n) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dba60bcb9b..274b8d55a3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4487,7 +4487,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov CReserveKey reservekey(this); CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, &reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, nExtraPayloadSize)) + if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, nExtraPayloadSize)) return false; if (nChangePosInOut != -1) @@ -4706,7 +4706,7 @@ void CWallet::CheckTransparentTransactionSanity(CMutableTransaction& tx, // true, the wallet must be unlocked. nExtraPayloadSize should be set to the number of extra bytes in the transaction // outside inputs/outputs (ie. LLMQ-related things). If fUseInstantSend is true, we will consider both locked and // confirmed UTXOs to be eligible for input; if it is not, only confirmed UTXOs will be used as inputs. -bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, +bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, bool fUseInstantSend) { @@ -4717,16 +4717,13 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT nExtraPayloadSize, fUseInstantSend, vTransparentTxouts); } -bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, +bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, bool fUseInstantSend, const std::vector& vTransparentTxouts) { LOCK(mempool.cs); AssertLockHeld(cs_main); - if (!coinControl || coinControl->destChange.which() == 0) - assert(reservekey); - nFeeRet = -1; strFailReason = ""; @@ -4810,7 +4807,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT scriptChange = GetScriptForDestination(coinControl->destChange); } else { CPubKey changeKey; - if (!reservekey->GetReservedKey(changeKey)) { + if (!reservekey.GetReservedKey(changeKey)) { strFailReason = _("Keypool ran out, please call keypoolrefill first"); nChangePosInOut = -1; nFeeRet = -1; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7a9bcc9526..3b63048262 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1094,12 +1094,12 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void CheckTransparentTransactionSanity(CMutableTransaction& tx, const std::vector& vInputTxs, const CCoinControl* coinControl, CAmount nFee, bool fSign); - bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, int nExtraPayloadSize, bool fUseInstantSend, const std::vector& vTransparentTxouts); - bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey* reservekey, + bool CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl = NULL, bool sign = true, int nExtraPayloadSize = 0, bool fUseInstantSend = false); From bfda395fd03fd89257f9df584b94856474b319bf Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Sat, 20 Jul 2024 16:42:31 +0200 Subject: [PATCH 16/24] Removed CTxOut::nRounds field --- src/primitives/transaction.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 5f8a4062c4..58478d33da 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -171,7 +171,6 @@ class CTxOut public: CAmount nValue; CScript scriptPubKey; - int nRounds = -10; 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 From 6904736123528d35a094c06e95a0556d91cd6116 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Sat, 20 Jul 2024 16:51:01 +0200 Subject: [PATCH 17/24] Changed visibility of critical section back to private --- src/llmq/quorums_instantsend.h | 2 +- src/wallet/test/createtransaction_tests.cpp | 3 +-- src/wallet/wallet.cpp | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 398c99511f..c784d12763 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -109,10 +109,10 @@ class CInstantSendManager : public CRecoveredSigsListener std::unordered_set pendingRetryTxs; std::atomic_bool isNewInstantSendEnabled{false}; -public: CCriticalSection cs; CInstantSendDb db; +public: CInstantSendManager(CDBWrapper& _llmqDb); ~CInstantSendManager(); diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp index 30bdf35ced..8017385a73 100644 --- a/src/wallet/test/createtransaction_tests.cpp +++ b/src/wallet/test/createtransaction_tests.cpp @@ -134,8 +134,7 @@ void AssertHasKey(CReserveKey& reservekey, const CWalletTx& wtx, uint32_t voutN) #define ASSERT_HAS_KEY(voutN) AssertHasKey(reservekey, wtx, voutN) #define ASSERT_SUCCESS() if (!strFailReason.empty()) BOOST_FAIL(strFailReason) #define ASSERT_FAILURE(reason) BOOST_ASSERT(strFailReason == reason) -#define ACQUIRE_LOCKS() LOCK2(cs_main, pwalletMain->cs_wallet);\ - LOCK(llmq::quorumInstantSendManager->cs); +#define ACQUIRE_LOCKS() LOCK2(cs_main, pwalletMain->cs_wallet) BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(sends_money) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 274b8d55a3..a457eaba8b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -236,9 +236,8 @@ bool CTransparentTxout::IsLLMQInstantSendLocked() const { assert(wallet); AssertLockHeld(wallet->cs_wallet); - AssertLockHeld(llmq::quorumInstantSendManager->cs); - return llmq::quorumInstantSendManager->db.GetInstantSendLockByTxid(GetHash()) != nullptr; + return llmq::quorumInstantSendManager->IsLocked(GetHash()); } bool CTransparentTxout::IsCoinBase() const { From fbea637455750fb809e92bbd98a85edd380d54d5 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Sat, 20 Jul 2024 17:36:23 +0200 Subject: [PATCH 18/24] Apply new transaction limits --- src/wallet/wallet.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a457eaba8b..a8d68d39fd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4620,8 +4620,8 @@ void CWallet::CheckTransparentTransactionSanity(CMutableTransaction& tx, const CCoinControl* coinControl, CAmount nFee, bool fSign) { size_t txSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); - if (txSize * WITNESS_SCALE_FACTOR > MAX_STANDARD_TX_WEIGHT) - throw std::runtime_error("Transaction too large"); + if (txSize * WITNESS_SCALE_FACTOR > MAX_NEW_TX_WEIGHT) + throw std::runtime_error("Transaction is too large (size limit: 250Kb). Select less inputs or consolidate your UTXOs"); if (coinControl && coinControl->nMaxSize && txSize > coinControl->nMaxSize) throw std::runtime_error("We made a transaction exceeding coinControl->nMaxSize. This is a bug."); @@ -4800,7 +4800,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CAmount nChangeAmount = nCollected - nRequired - (nRecipientsToSplitFee ? 0 : nFeeRet); // If the collected amount is exactly what is required, we don't need to make a change output. - if (nChangeAmount || (coinControl && coinControl->fNoChange)) { + if (nChangeAmount && !(coinControl && coinControl->fNoChange)) { CScript scriptChange; if (coinControl && coinControl->destChange.which() != 0) { scriptChange = GetScriptForDestination(coinControl->destChange); From f603de62703b4a0af3ef963eb291e0b61d00c579 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Sun, 18 Aug 2024 18:21:51 +0200 Subject: [PATCH 19/24] Fixed coincontrol logic (thus fixing many tests behaviour) --- src/wallet/wallet.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a8d68d39fd..d47132a062 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -215,7 +215,7 @@ bool CTransparentTxout::IsCoinTypeCompatible(const CCoinControl* coinControl) co assert(!txout.IsNull()); if (!coinControl) - return GetValue() != 1000 * COIN; + return true; else if (coinControl->nCoinType == CoinType::ONLY_MINTS) return false; else if (coinControl->nCoinType == CoinType::ONLY_NONDENOMINATED_NOT1000IFMN) @@ -224,10 +224,8 @@ bool CTransparentTxout::IsCoinTypeCompatible(const CCoinControl* coinControl) co return !fMasternodeMode || GetValue() != 1000 * COIN; else if (coinControl->nCoinType == CoinType::ONLY_1000) return GetValue() == 1000 * COIN; - else if (coinControl->nCoinType == CoinType::WITH_1000) - return true; else - return GetValue() != 1000 * COIN; + return true; } bool CTransparentTxout::IsLLMQInstantSendLocked() const { From 3423301032756559db1d24fa734b62457656cf2d Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Sun, 18 Aug 2024 21:24:45 +0200 Subject: [PATCH 20/24] Changed TransparentTxout::IsFromMe() implementation to fix bugs in tests --- src/wallet/wallet.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d47132a062..bb4986305e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -255,16 +255,7 @@ bool CTransparentTxout::IsFromMe() const { assert(wallet); AssertLockHeld(wallet->cs_wallet); - for (const CTxIn& txin: wallet->mapWallet.at(GetHash()).tx->vin) { - std::map::const_iterator mi = wallet->mapWallet.find(txin.prevout.hash); - if (mi == wallet->mapWallet.end()) - return false; - - if (!(::IsMine(*wallet, mi->second.tx->vout.at(txin.prevout.n).scriptPubKey, SIGVERSION_BASE) & ISMINE_ALL)) - return false; - } - - return true; + return wallet->GetDebit(*wallet->mapWallet.at(GetHash()).tx, ISMINE_ALL) > 0; } unsigned int CTransparentTxout::GetDepthInMainChain() const { From 11e8a404d34a5f849002752695b8bcc1f290b5bf Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Mon, 19 Aug 2024 10:36:24 +0200 Subject: [PATCH 21/24] Fixed crash/undefined behaviour due to mssing lock --- src/wallet/rpcwallet.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a5ade6b12f..d6f9e97115 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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; From 988f1c613411eb8d41409ab6433a065294da6b38 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Tue, 20 Aug 2024 14:11:17 +0200 Subject: [PATCH 22/24] Fixed undefined behaviour in dip3-deterministicmn.py test --- qa/rpc-tests/dip3-deterministicmns.py | 3 +++ qa/rpc-tests/test_framework/mn_utils.py | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/qa/rpc-tests/dip3-deterministicmns.py b/qa/rpc-tests/dip3-deterministicmns.py index f92946fe37..f97d9eca4c 100755 --- a/qa/rpc-tests/dip3-deterministicmns.py +++ b/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): diff --git a/qa/rpc-tests/test_framework/mn_utils.py b/qa/rpc-tests/test_framework/mn_utils.py index 046b2faec8..bb136bb138 100644 --- a/qa/rpc-tests/test_framework/mn_utils.py +++ b/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) From ae24627eca9c59077db172cfdb9521c98e3e89a5 Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Tue, 20 Aug 2024 16:26:35 +0200 Subject: [PATCH 23/24] Fix undefined behaviour in txn_doublespend.py test --- qa/rpc-tests/txn_doublespend.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qa/rpc-tests/txn_doublespend.py b/qa/rpc-tests/txn_doublespend.py index cb94c95813..a9f7236bb6 100755 --- a/qa/rpc-tests/txn_doublespend.py +++ b/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"]) From f65c3e856cd2c0e26c9dbe04b8fb42a1fa13d4dc Mon Sep 17 00:00:00 2001 From: Peter Shugalev Date: Wed, 21 Aug 2024 09:19:24 +0200 Subject: [PATCH 24/24] Get rid of tests for removed behaviour --- src/wallet/test/createtransaction_tests.cpp | 30 --------------------- 1 file changed, 30 deletions(-) diff --git a/src/wallet/test/createtransaction_tests.cpp b/src/wallet/test/createtransaction_tests.cpp index 8017385a73..75537db9f2 100644 --- a/src/wallet/test/createtransaction_tests.cpp +++ b/src/wallet/test/createtransaction_tests.cpp @@ -663,20 +663,6 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) std::vector vRecipients = GetFakeRecipients({999 * COIN}); - { - CWalletTx wtx; - CAmount nFeeRet = 0; - int nChangePosInOut = -1; - std::string strFailReason; - - CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, - nullptr, true, 0, true, vTxouts); - - - ASSERT_FAILURE("Insufficient funds"); - } - { CCoinControl coinControl; CWalletTx wtx; @@ -816,22 +802,6 @@ BOOST_FIXTURE_TEST_SUITE(createtransaction_tests, WalletTestingSetup) ASSERT_FAILURE("Insufficient funds"); } - { - CCoinControl coinControl; - CWalletTx wtx; - CAmount nFeeRet = 0; - int nChangePosInOut = -1; - std::string strFailReason; - - coinControl.Select(vTxouts.at(1).GetOutpoint()); - coinControl.fAllowOtherInputs = true; - - CReserveKey reservekey(pwalletMain); - pwalletMain->CreateTransaction(vRecipients, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, - &coinControl, true, 0, true, vTxouts); - - ASSERT_FAILURE("Some coin control inputs could not be selected."); - } } BOOST_AUTO_TEST_CASE(coincontrol_input_count_limit) {