From 4949004d68dc08382df2c34ae519c1b1cfd60f1a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 7 Jun 2014 13:53:27 +0200 Subject: [PATCH] Add CMutableTransaction and make CTransaction immutable. In addition, introduce a cached hash inside CTransaction, to prevent recalculating it over and over again. --- src/chainparams.cpp | 2 +- src/core.cpp | 25 ++++++++- src/core.h | 93 ++++++++++++++++++++++------------ src/miner.cpp | 18 ++++--- src/qt/coincontroldialog.cpp | 2 +- src/rpcrawtransaction.cpp | 10 ++-- src/script.cpp | 6 +-- src/script.h | 5 +- src/test/DoS_tests.cpp | 12 ++--- src/test/accounting_tests.cpp | 12 ++++- src/test/miner_tests.cpp | 12 +++-- src/test/multisig_tests.cpp | 8 +-- src/test/pmt_tests.cpp | 4 +- src/test/script_P2SH_tests.cpp | 18 +++---- src/test/script_tests.cpp | 12 ++--- src/test/sighash_tests.cpp | 6 +-- src/test/transaction_tests.cpp | 14 ++--- src/test/wallet_tests.cpp | 10 ++-- src/wallet.cpp | 18 ++++--- 19 files changed, 181 insertions(+), 106 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 31eac62d4..afbae6fc5 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -127,7 +127,7 @@ public: // CTxOut(nValue=50.00000000, scriptPubKey=0x5F1DF16B2B704C8A578D0B) // vMerkleTree: 4a5e1e const char* pszTimestamp = "The Times 03/Jan/2009 Chancellor on brink of second bailout for banks"; - CTransaction txNew; + CMutableTransaction txNew; txNew.vin.resize(1); txNew.vout.resize(1); txNew.vin[0].scriptSig = CScript() << 486604799 << CScriptNum(4) << vector((const unsigned char*)pszTimestamp, (const unsigned char*)pszTimestamp + strlen(pszTimestamp)); diff --git a/src/core.cpp b/src/core.cpp index 6039986e6..6c5ee1c0f 100644 --- a/src/core.cpp +++ b/src/core.cpp @@ -91,11 +91,34 @@ std::string CFeeRate::ToString() const return result; } -uint256 CTransaction::GetHash() const +CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {} +CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime) {} + +uint256 CMutableTransaction::GetHash() const { return SerializeHash(*this); } +void CTransaction::UpdateHash() const +{ + *const_cast(&hash) = SerializeHash(*this); +} + +CTransaction::CTransaction() : hash(0), nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0) { } + +CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime) { + UpdateHash(); +} + +CTransaction& CTransaction::operator=(const CTransaction &tx) { + *const_cast(&nVersion) = tx.nVersion; + *const_cast*>(&vin) = tx.vin; + *const_cast*>(&vout) = tx.vout; + *const_cast(&nLockTime) = tx.nLockTime; + *const_cast(&hash) = tx.hash; + return *this; +} + int64_t CTransaction::GetValueOut() const { int64_t nValueOut = 0; diff --git a/src/core.h b/src/core.h index 0e5912934..1a20145cc 100644 --- a/src/core.h +++ b/src/core.h @@ -203,49 +203,59 @@ public: }; +struct CMutableTransaction; + /** The basic transaction that is broadcasted on the network and contained in * blocks. A transaction can contain multiple inputs and outputs. */ class CTransaction { +private: + /** Memory only. */ + const uint256 hash; + void UpdateHash() const; + public: static CFeeRate minTxFee; static CFeeRate minRelayTxFee; static const int CURRENT_VERSION=1; - int nVersion; - std::vector vin; - std::vector vout; - unsigned int nLockTime; - CTransaction() - { - SetNull(); - } + // The local variables are made const to prevent unintended modification + // without updating the cached hash value. However, CTransaction is not + // actually immutable; deserialization and assignment are implemented, + // and bypass the constness. This is safe, as they update the entire + // structure, including the hash. + const int nVersion; + const std::vector vin; + const std::vector vout; + const unsigned int nLockTime; - IMPLEMENT_SERIALIZE - ( - READWRITE(this->nVersion); + /** Construct a CTransaction that qualifies as IsNull() */ + CTransaction(); + + /** Convert a CMutableTransaction into a CTransaction. */ + CTransaction(const CMutableTransaction &tx); + + CTransaction& operator=(const CTransaction& tx); + + IMPLEMENT_SERIALIZE( + READWRITE(*const_cast(&this->nVersion)); nVersion = this->nVersion; - READWRITE(vin); - READWRITE(vout); - READWRITE(nLockTime); + READWRITE(*const_cast*>(&vin)); + READWRITE(*const_cast*>(&vout)); + READWRITE(*const_cast(&nLockTime)); + if (fRead) + UpdateHash(); ) - void SetNull() - { - nVersion = CTransaction::CURRENT_VERSION; - vin.clear(); - vout.clear(); - nLockTime = 0; + bool IsNull() const { + return vin.empty() && vout.empty(); } - bool IsNull() const - { - return (vin.empty() && vout.empty()); + const uint256& GetHash() const { + return hash; } - uint256 GetHash() const; - // Return sum of txouts. int64_t GetValueOut() const; // GetValueIn() is a method on CCoinsViewCache, because @@ -261,22 +271,43 @@ public: friend bool operator==(const CTransaction& a, const CTransaction& b) { - return (a.nVersion == b.nVersion && - a.vin == b.vin && - a.vout == b.vout && - a.nLockTime == b.nLockTime); + return a.hash == b.hash; } friend bool operator!=(const CTransaction& a, const CTransaction& b) { - return !(a == b); + return a.hash != b.hash; } - std::string ToString() const; void print() const; }; +/** A mutable version of CTransaction. */ +struct CMutableTransaction +{ + int nVersion; + std::vector vin; + std::vector vout; + unsigned int nLockTime; + + CMutableTransaction(); + CMutableTransaction(const CTransaction& tx); + + IMPLEMENT_SERIALIZE( + READWRITE(this->nVersion); + nVersion = this->nVersion; + READWRITE(vin); + READWRITE(vout); + READWRITE(nLockTime); + ) + + /** Compute the hash of this CMutableTransaction. This is computed on the + * fly, as opposed to GetHash() in CTransaction, which uses a cached result. + */ + uint256 GetHash() const; +}; + /** wrapper for CTxOut that provides a more compact serialization */ class CTxOutCompressor { diff --git a/src/miner.cpp b/src/miner.cpp index 7efca7cff..37cdc7d84 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -86,14 +86,14 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CBlock *pblock = &pblocktemplate->block; // pointer for convenience // Create coinbase tx - CTransaction txNew; + CMutableTransaction txNew; txNew.vin.resize(1); txNew.vin[0].prevout.SetNull(); txNew.vout.resize(1); txNew.vout[0].scriptPubKey = scriptPubKeyIn; - // Add our coinbase tx as first transaction - pblock->vtx.push_back(txNew); + // Add dummy coinbase tx as first transaction + pblock->vtx.push_back(CTransaction()); pblocktemplate->vTxFees.push_back(-1); // updated at end pblocktemplate->vTxSigOps.push_back(-1); // updated at end @@ -294,7 +294,10 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) nLastBlockSize = nBlockSize; LogPrintf("CreateNewBlock(): total size %u\n", nBlockSize); - pblock->vtx[0].vout[0].nValue = GetBlockValue(pindexPrev->nHeight+1, nFees); + // Compute final coinbase transaction. + txNew.vout[0].nValue = GetBlockValue(pindexPrev->nHeight+1, nFees); + txNew.vin[0].scriptSig = CScript() << OP_0 << OP_0; + pblock->vtx[0] = txNew; pblocktemplate->vTxFees[0] = -nFees; // Fill in header @@ -302,7 +305,6 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) UpdateTime(*pblock, pindexPrev); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock); pblock->nNonce = 0; - pblock->vtx[0].vin[0].scriptSig = CScript() << OP_0 << OP_0; pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(pblock->vtx[0]); CBlockIndex indexDummy(*pblock); @@ -328,9 +330,11 @@ void IncrementExtraNonce(CBlock* pblock, CBlockIndex* pindexPrev, unsigned int& } ++nExtraNonce; unsigned int nHeight = pindexPrev->nHeight+1; // Height first in coinbase required for block.version=2 - pblock->vtx[0].vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS; - assert(pblock->vtx[0].vin[0].scriptSig.size() <= 100); + CMutableTransaction txCoinbase(pblock->vtx[0]); + txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS; + assert(txCoinbase.vin[0].scriptSig.size() <= 100); + pblock->vtx[0] = txCoinbase; pblock->hashMerkleRoot = pblock->BuildMerkleTree(); } diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 42d6da7d3..52bdf9673 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -440,7 +440,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) // nPayAmount qint64 nPayAmount = 0; bool fDust = false; - CTransaction txDummy; + CMutableTransaction txDummy; foreach(const qint64 &amount, CoinControlDialog::payAmounts) { nPayAmount += amount; diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index dee7daeb2..1b5d494be 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -349,7 +349,7 @@ Value createrawtransaction(const Array& params, bool fHelp) Array inputs = params[0].get_array(); Object sendTo = params[1].get_obj(); - CTransaction rawTx; + CMutableTransaction rawTx; BOOST_FOREACH(const Value& input, inputs) { @@ -554,11 +554,11 @@ Value signrawtransaction(const Array& params, bool fHelp) vector txData(ParseHexV(params[0], "argument 1")); CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); - vector txVariants; + vector txVariants; while (!ssData.empty()) { try { - CTransaction tx; + CMutableTransaction tx; ssData >> tx; txVariants.push_back(tx); } @@ -572,7 +572,7 @@ Value signrawtransaction(const Array& params, bool fHelp) // mergedTx will end up with all the signatures; it // starts as a clone of the rawtx: - CTransaction mergedTx(txVariants[0]); + CMutableTransaction mergedTx(txVariants[0]); bool fComplete = true; // Fetch previous transactions (inputs): @@ -713,7 +713,7 @@ Value signrawtransaction(const Array& params, bool fHelp) SignSignature(keystore, prevPubKey, mergedTx, i, nHashType); // ... and merge in other signatures: - BOOST_FOREACH(const CTransaction& txv, txVariants) + BOOST_FOREACH(const CMutableTransaction& txv, txVariants) { txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, txin.scriptSig, txv.vin[i].scriptSig); } diff --git a/src/script.cpp b/src/script.cpp index b383a00a6..c83d26885 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -1636,7 +1636,7 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C } -bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CTransaction& txTo, unsigned int nIn, int nHashType) +bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, int nHashType) { assert(nIn < txTo.vin.size()); CTxIn& txin = txTo.vin[nIn]; @@ -1671,7 +1671,7 @@ bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CTransa return VerifyScript(txin.scriptSig, fromPubKey, txTo, nIn, STANDARD_SCRIPT_VERIFY_FLAGS, 0); } -bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CTransaction& txTo, unsigned int nIn, int nHashType) +bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType) { assert(nIn < txTo.vin.size()); CTxIn& txin = txTo.vin[nIn]; @@ -1689,7 +1689,7 @@ static CScript PushAll(const vector& values) return result; } -static CScript CombineMultisig(CScript scriptPubKey, const CTransaction& txTo, unsigned int nIn, +static CScript CombineMultisig(CScript scriptPubKey, const CMutableTransaction& txTo, unsigned int nIn, const vector& vSolutions, vector& sigs1, vector& sigs2) { diff --git a/src/script.h b/src/script.h index a282e7dc0..bd6574627 100644 --- a/src/script.h +++ b/src/script.h @@ -20,6 +20,7 @@ class CCoins; class CKeyStore; class CTransaction; +class CMutableTransaction; static const unsigned int MAX_SCRIPT_ELEMENT_SIZE = 520; // bytes static const unsigned int MAX_OP_RETURN_RELAY = 40; // bytes @@ -805,8 +806,8 @@ bool IsMine(const CKeyStore& keystore, const CTxDestination &dest); void ExtractAffectedKeys(const CKeyStore &keystore, const CScript& scriptPubKey, std::vector &vKeys); bool ExtractDestination(const CScript& scriptPubKey, CTxDestination& addressRet); bool ExtractDestinations(const CScript& scriptPubKey, txnouttype& typeRet, std::vector& addressRet, int& nRequiredRet); -bool SignSignature(const CKeyStore& keystore, const CScript& fromPubKey, CTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); -bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); +bool SignSignature(const CKeyStore& keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); +bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL); bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, unsigned int flags, int nHashType); // Given two sets of signatures for scriptPubKey, possibly with OP_0 placeholders, diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index fb06fb343..3a4584441 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -167,7 +167,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) // 50 orphan transactions: for (int i = 0; i < 50; i++) { - CTransaction tx; + CMutableTransaction tx; tx.vin.resize(1); tx.vin[0].prevout.n = 0; tx.vin[0].prevout.hash = GetRandHash(); @@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) { CTransaction txPrev = RandomOrphan(); - CTransaction tx; + CMutableTransaction tx; tx.vin.resize(1); tx.vin[0].prevout.n = 0; tx.vin[0].prevout.hash = txPrev.GetHash(); @@ -201,7 +201,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) { CTransaction txPrev = RandomOrphan(); - CTransaction tx; + CMutableTransaction tx; tx.vout.resize(1); tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); @@ -242,10 +242,10 @@ BOOST_AUTO_TEST_CASE(DoS_checkSig) // 100 orphan transactions: static const int NPREV=100; - CTransaction orphans[NPREV]; + CMutableTransaction orphans[NPREV]; for (int i = 0; i < NPREV; i++) { - CTransaction& tx = orphans[i]; + CMutableTransaction& tx = orphans[i]; tx.vin.resize(1); tx.vin[0].prevout.n = 0; tx.vin[0].prevout.hash = GetRandHash(); @@ -258,7 +258,7 @@ BOOST_AUTO_TEST_CASE(DoS_checkSig) } // Create a transaction that depends on orphans: - CTransaction tx; + CMutableTransaction tx; tx.vout.resize(1); tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); diff --git a/src/test/accounting_tests.cpp b/src/test/accounting_tests.cpp index e2a75da34..4bee0f6b6 100644 --- a/src/test/accounting_tests.cpp +++ b/src/test/accounting_tests.cpp @@ -83,13 +83,21 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "y"; - --wtx.nLockTime; // Just to change the hash :) + { + CMutableTransaction tx(wtx); + --tx.nLockTime; // Just to change the hash :) + *static_cast(&wtx) = CTransaction(tx); + } pwalletMain->AddToWallet(wtx); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; - --wtx.nLockTime; // Just to change the hash :) + { + CMutableTransaction tx(wtx); + --tx.nLockTime; // Just to change the hash :) + *static_cast(&wtx) = CTransaction(tx); + } pwalletMain->AddToWallet(wtx); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index bff6de41a..47977cf29 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; CBlockTemplate *pblocktemplate; - CTransaction tx,tx2; + CMutableTransaction tx,tx2; CScript script; uint256 hash; @@ -68,10 +68,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CBlock *pblock = &pblocktemplate->block; // pointer for convenience pblock->nVersion = 1; pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; - pblock->vtx[0].vin[0].scriptSig = CScript(); - pblock->vtx[0].vin[0].scriptSig.push_back(blockinfo[i].extranonce); - pblock->vtx[0].vin[0].scriptSig.push_back(chainActive.Height()); - pblock->vtx[0].vout[0].scriptPubKey = CScript(); + CMutableTransaction txCoinbase(pblock->vtx[0]); + txCoinbase.vin[0].scriptSig = CScript(); + txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); + txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); + txCoinbase.vout[0].scriptPubKey = CScript(); + pblock->vtx[0] = CTransaction(txCoinbase); if (txFirst.size() < 2) txFirst.push_back(new CTransaction(pblock->vtx[0])); pblock->hashMerkleRoot = pblock->BuildMerkleTree(); diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 3775abd63..452cf084a 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -55,13 +55,13 @@ BOOST_AUTO_TEST_CASE(multisig_verify) CScript escrow; escrow << OP_2 << key[0].GetPubKey() << key[1].GetPubKey() << key[2].GetPubKey() << OP_3 << OP_CHECKMULTISIG; - CTransaction txFrom; // Funding transaction + CMutableTransaction txFrom; // Funding transaction txFrom.vout.resize(3); txFrom.vout[0].scriptPubKey = a_and_b; txFrom.vout[1].scriptPubKey = a_or_b; txFrom.vout[2].scriptPubKey = escrow; - CTransaction txTo[3]; // Spending transaction + CMutableTransaction txTo[3]; // Spending transaction for (int i = 0; i < 3; i++) { txTo[i].vin.resize(1); @@ -270,13 +270,13 @@ BOOST_AUTO_TEST_CASE(multisig_Sign) CScript escrow; escrow << OP_2 << key[0].GetPubKey() << key[1].GetPubKey() << key[2].GetPubKey() << OP_3 << OP_CHECKMULTISIG; - CTransaction txFrom; // Funding transaction + CMutableTransaction txFrom; // Funding transaction txFrom.vout.resize(3); txFrom.vout[0].scriptPubKey = a_and_b; txFrom.vout[1].scriptPubKey = a_or_b; txFrom.vout[2].scriptPubKey = escrow; - CTransaction txTo[3]; // Spending transaction + CMutableTransaction txTo[3]; // Spending transaction for (int i = 0; i < 3; i++) { txTo[i].vin.resize(1); diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp index 7d7e6681d..9dce4daac 100644 --- a/src/test/pmt_tests.cpp +++ b/src/test/pmt_tests.cpp @@ -36,9 +36,9 @@ BOOST_AUTO_TEST_CASE(pmt_test1) // build a block with some dummy transactions CBlock block; for (unsigned int j=0; j vch(ch, ch + sizeof(ch) -1); CDataStream stream(vch, SER_DISK, CLIENT_VERSION); - CTransaction tx; + CMutableTransaction tx; stream >> tx; CValidationState state; BOOST_CHECK_MESSAGE(CheckTransaction(tx, state) && state.IsValid(), "Simple deserialized transaction should be valid."); @@ -224,10 +224,10 @@ BOOST_AUTO_TEST_CASE(basic_transaction_tests) // paid to a TX_PUBKEY, the second 21 and 22 CENT outputs // paid to a TX_PUBKEYHASH. // -static std::vector +static std::vector SetupDummyInputs(CBasicKeyStore& keystoreRet, CCoinsView & coinsRet) { - std::vector dummyTransactions; + std::vector dummyTransactions; dummyTransactions.resize(2); // Add some keys to the keystore: @@ -261,9 +261,9 @@ BOOST_AUTO_TEST_CASE(test_Get) CBasicKeyStore keystore; CCoinsView coinsDummy; CCoinsViewCache coins(coinsDummy); - std::vector dummyTransactions = SetupDummyInputs(keystore, coins); + std::vector dummyTransactions = SetupDummyInputs(keystore, coins); - CTransaction t1; + CMutableTransaction t1; t1.vin.resize(3); t1.vin[0].prevout.hash = dummyTransactions[0].GetHash(); t1.vin[0].prevout.n = 1; @@ -296,9 +296,9 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) CBasicKeyStore keystore; CCoinsView coinsDummy; CCoinsViewCache coins(coinsDummy); - std::vector dummyTransactions = SetupDummyInputs(keystore, coins); + std::vector dummyTransactions = SetupDummyInputs(keystore, coins); - CTransaction t; + CMutableTransaction t; t.vin.resize(1); t.vin[0].prevout.hash = dummyTransactions[0].GetHash(); t.vin[0].prevout.n = 1; diff --git a/src/test/wallet_tests.cpp b/src/test/wallet_tests.cpp index 199335882..86a83f516 100644 --- a/src/test/wallet_tests.cpp +++ b/src/test/wallet_tests.cpp @@ -31,16 +31,18 @@ static vector vCoins; static void add_coin(int64_t nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { static int nextLockTime = 0; - CTransaction tx; + CMutableTransaction tx; tx.nLockTime = nextLockTime++; // so all transactions get different hashes tx.vout.resize(nInput+1); tx.vout[nInput].nValue = nValue; + if (fIsFromMe) { + // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), + // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() + tx.vin.resize(1); + } CWalletTx* wtx = new CWalletTx(&wallet, tx); if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - wtx->vin.resize(1); wtx->fDebitCached = true; wtx->nDebitCached = 1; } diff --git a/src/wallet.cpp b/src/wallet.cpp index 3176cf893..0e0a0e87d 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -1245,6 +1245,7 @@ bool CWallet::CreateTransaction(const vector >& vecSend, } wtxNew.BindWallet(this); + CMutableTransaction txNew; { LOCK2(cs_main, cs_wallet); @@ -1252,8 +1253,8 @@ bool CWallet::CreateTransaction(const vector >& vecSend, nFeeRet = payTxFee.GetFeePerK(); while (true) { - wtxNew.vin.clear(); - wtxNew.vout.clear(); + txNew.vin.clear(); + txNew.vout.clear(); wtxNew.fFromMe = true; int64_t nTotalValue = nValue + nFeeRet; @@ -1267,7 +1268,7 @@ bool CWallet::CreateTransaction(const vector >& vecSend, strFailReason = _("Transaction amount too small"); return false; } - wtxNew.vout.push_back(txout); + txNew.vout.push_back(txout); } // Choose coins to use @@ -1331,8 +1332,8 @@ bool CWallet::CreateTransaction(const vector >& vecSend, else { // Insert change txn at random position: - vector::iterator position = wtxNew.vout.begin()+GetRandInt(wtxNew.vout.size()+1); - wtxNew.vout.insert(position, newTxOut); + vector::iterator position = txNew.vout.begin()+GetRandInt(txNew.vout.size()+1); + txNew.vout.insert(position, newTxOut); } } else @@ -1340,17 +1341,20 @@ bool CWallet::CreateTransaction(const vector >& vecSend, // Fill vin BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins) - wtxNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second)); + txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second)); // Sign int nIn = 0; BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins) - if (!SignSignature(*this, *coin.first, wtxNew, nIn++)) + if (!SignSignature(*this, *coin.first, txNew, nIn++)) { strFailReason = _("Signing transaction failed"); return false; } + // Embed the constructed transaction data in wtxNew. + *static_cast(&wtxNew) = CTransaction(txNew); + // Limit size unsigned int nBytes = ::GetSerializeSize(*(CTransaction*)&wtxNew, SER_NETWORK, PROTOCOL_VERSION); if (nBytes >= MAX_STANDARD_TX_SIZE)