From 71cc1b58a1abab12dae1966d27b04fcb78fcd412 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Tue, 18 Oct 2016 18:10:42 +0100 Subject: [PATCH 1/2] Add tests for IsStandardTx applied to v2 transactions. Signed-off-by: Daira Hopwood --- src/test/transaction_tests.cpp | 51 ++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c2b0a7017..438e33178 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -592,4 +592,55 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK(!IsStandardTx(t, reason)); } +BOOST_AUTO_TEST_CASE(test_IsStandardV2) +{ + LOCK(cs_main); + CBasicKeyStore keystore; + CCoinsView coinsDummy; + CCoinsViewCache coins(&coinsDummy); + std::vector dummyTransactions = SetupDummyInputs(keystore, coins); + + CMutableTransaction t; + t.vin.resize(1); + t.vin[0].prevout.hash = dummyTransactions[0].GetHash(); + t.vin[0].prevout.n = 1; + t.vin[0].scriptSig << std::vector(65, 0); + t.vout.resize(1); + t.vout[0].nValue = 90*CENT; + CKey key; + key.MakeNewKey(true); + t.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); + + string reason; + // A v2 transaction with no JoinSplits is still standard. + t.nVersion = 2; + BOOST_CHECK(IsStandardTx(t, reason)); + + // ... and with one JoinSplit. + t.vjoinsplit.push_back(JSDescription()); + BOOST_CHECK(IsStandardTx(t, reason)); + + // ... and when that JoinSplit takes from a transparent input. + JSDescription *jsdesc = &t.vjoinsplit[0]; + jsdesc->vpub_old = 10*CENT; + t.vout[0].nValue -= 10*CENT; + BOOST_CHECK(IsStandardTx(t, reason)); + + // A v2 transaction with JoinSplits but no transparent inputs is standard. + jsdesc->vpub_old = 0; + jsdesc->vpub_new = 100*CENT; + t.vout[0].nValue = 90*CENT; + t.vin.resize(0); + BOOST_CHECK(IsStandardTx(t, reason)); + + // v2 transactions can still be non-standard for the same reasons as v1. + t.vout[0].nValue = 501; // dust + BOOST_CHECK(!IsStandardTx(t, reason)); + + // v3 is not standard. + t.nVersion = 3; + t.vout[0].nValue = 90*CENT; + BOOST_CHECK(!IsStandardTx(t, reason)); +} + BOOST_AUTO_TEST_SUITE_END() From e923e3ae0f6e1591d28a991e51c2b1ecae07a215 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Tue, 18 Oct 2016 18:14:18 +0100 Subject: [PATCH 2/2] Make v2 transactions standard. This also corrects a rule about admitting large orphan transactions into the mempool, to account for v2-specific fields. Signed-off-by: Daira Hopwood --- src/bitcoin-tx.cpp | 2 +- src/main.cpp | 4 ++-- src/primitives/transaction.cpp | 4 ++-- src/primitives/transaction.h | 4 +++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 319be2991..29fabafd8 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -157,7 +157,7 @@ static void RegisterLoad(const string& strInput) static void MutateTxVersion(CMutableTransaction& tx, const string& cmdVal) { int64_t newVersion = atoi64(cmdVal); - if (newVersion < 1 || newVersion > CTransaction::CURRENT_VERSION) + if (newVersion < CTransaction::MIN_CURRENT_VERSION || newVersion > CTransaction::MAX_CURRENT_VERSION) throw runtime_error("Invalid TX version requested"); tx.nVersion = (int) newVersion; diff --git a/src/main.cpp b/src/main.cpp index 2b09a50d6..7a1336fb7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -564,7 +564,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) // have been mined or received. // 10,000 orphans, each of which is at most 5,000 bytes big is // at most 500 megabytes of orphans: - unsigned int sz = tx.GetSerializeSize(SER_NETWORK, CTransaction::CURRENT_VERSION); + unsigned int sz = tx.GetSerializeSize(SER_NETWORK, tx.nVersion); if (sz > 5000) { LogPrint("mempool", "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); @@ -639,7 +639,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) bool IsStandardTx(const CTransaction& tx, string& reason) { - if (tx.nVersion > CTransaction::CURRENT_VERSION || tx.nVersion < 1) { + if (tx.nVersion > CTransaction::MAX_CURRENT_VERSION || tx.nVersion < CTransaction::MIN_CURRENT_VERSION) { reason = "version"; return false; } diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 1cedd6f27..e18024327 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -113,7 +113,7 @@ std::string CTxOut::ToString() const return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, scriptPubKey.ToString().substr(0,30)); } -CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {} +CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::MIN_CURRENT_VERSION), nLockTime(0) {} CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vjoinsplit(tx.vjoinsplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig) { @@ -130,7 +130,7 @@ void CTransaction::UpdateHash() const *const_cast(&hash) = SerializeHash(*this); } -CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0), vjoinsplit(), joinSplitPubKey(), joinSplitSig() { } +CTransaction::CTransaction() : nVersion(CTransaction::MIN_CURRENT_VERSION), vin(), vout(), nLockTime(0), vjoinsplit(), joinSplitPubKey(), joinSplitSig() { } CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vjoinsplit(tx.vjoinsplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig) diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 2e83773c4..f43a75a88 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -294,7 +294,9 @@ private: public: typedef boost::array joinsplit_sig_t; - static const int32_t CURRENT_VERSION=1; + // Transactions that include a list of JoinSplits are version 2. + static const int32_t MIN_CURRENT_VERSION = 1; + static const int32_t MAX_CURRENT_VERSION = 2; // The local variables are made const to prevent unintended modification // without updating the cached hash value. However, CTransaction is not