From 9bb37bf0d5630b41af25fc6f4a45eb895bb9be16 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Wed, 21 Feb 2018 20:21:06 -0800 Subject: [PATCH 1/5] Implement transaction expiry for Overwinter --- src/bitcoin-tx.cpp | 11 +++++++++++ src/consensus/consensus.h | 2 ++ src/main.cpp | 19 +++++++++++++++++++ src/main.h | 9 +++++++++ src/miner.cpp | 4 +++- src/primitives/transaction.h | 3 --- src/rpcrawtransaction.cpp | 13 +++++++++++-- src/txmempool.cpp | 18 ++++++++++++++++++ src/txmempool.h | 1 + src/wallet/rpcwallet.cpp | 14 +++++++++++--- src/wallet/wallet.cpp | 15 ++++++++++++++- 11 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 710d0082f..4fcc13e41 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -164,6 +164,15 @@ static void MutateTxVersion(CMutableTransaction& tx, const string& cmdVal) tx.nVersion = (int) newVersion; } +static void MutateTxExpiry(CMutableTransaction& tx, const string& cmdVal) +{ + int64_t newExpiry = atoi64(cmdVal); + if (newExpiry >= TX_EXPIRY_HEIGHT_THRESHOLD) { + throw runtime_error("Invalid TX expiry requested"); + } + tx.nExpiryHeight = (int) newExpiry; +} + static void MutateTxLocktime(CMutableTransaction& tx, const string& cmdVal) { int64_t newLocktime = atoi64(cmdVal); @@ -503,6 +512,8 @@ static void MutateTx(CMutableTransaction& tx, const string& command, MutateTxVersion(tx, commandVal); else if (command == "locktime") MutateTxLocktime(tx, commandVal); + else if (command == "expiry") + MutateTxExpiry(tx, commandVal); else if (command == "delin") MutateTxDelInput(tx, commandVal); diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 43fd1e828..8650c453a 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -22,6 +22,8 @@ static const unsigned int MAX_BLOCK_SIGOPS = 20000; static const unsigned int MAX_TX_SIZE = 100000; /** Coinbase transaction outputs can only be spent after this number of new blocks (network rule) */ static const int COINBASE_MATURITY = 100; +/** The minimum value which is invalid for expiry height, used by CTransaction and CMutableTransaction */ +static constexpr uint32_t TX_EXPIRY_HEIGHT_THRESHOLD = 500000000; /** Flags for LockTime() */ enum { diff --git a/src/main.cpp b/src/main.cpp index 9e41d9c4e..161c3933c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -73,6 +73,8 @@ size_t nCoinCacheUsage = 5000 * 300; uint64_t nPruneTarget = 0; bool fAlerts = DEFAULT_ALERTS; +unsigned int expiryDelta = DEFAULT_TX_EXPIRY_DELTA; + /** Fees smaller than this (in satoshi) are considered zero fee (for relaying and mining) */ CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); @@ -718,6 +720,14 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime) return true; } +bool IsExpiredTx(const CTransaction &tx, int nBlockHeight) +{ + if (tx.nExpiryHeight == 0 || tx.IsCoinBase()) { + return false; + } + return static_cast(nBlockHeight) > tx.nExpiryHeight; +} + bool CheckFinalTx(const CTransaction &tx, int flags) { AssertLockHeld(cs_main); @@ -884,6 +894,11 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, return state.DoS(dosLevel, error("ContextualCheckTransaction: overwinter is active"), REJECT_INVALID, "tx-overwinter-active"); } + + // Check that all transactions are unexpired + if (IsExpiredTx(tx, nHeight)) { + return state.DoS(dosLevel, error("ContextualCheckTransaction(): transaction is expired"), REJECT_INVALID, "tx-overwinter-expired"); + } } if (!(tx.IsCoinBase() || tx.vjoinsplit.empty())) { @@ -2659,6 +2674,10 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * // Remove conflicting transactions from the mempool. list txConflicted; mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload()); + + // Remove transactions that expire at new block height from mempool + mempool.removeExpired(pindexNew->nHeight); + // Update chainActive & related variables. UpdateTip(pindexNew); // Tell wallet about transactions that went from mempool diff --git a/src/main.h b/src/main.h index 2cae24b22..2a3b4175a 100644 --- a/src/main.h +++ b/src/main.h @@ -68,6 +68,8 @@ static const unsigned int MAX_STANDARD_TX_SIGOPS = MAX_BLOCK_SIGOPS/5; static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 100; /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; +/** Default for -txexpirydelta, in number of blocks */ +static const unsigned int DEFAULT_TX_EXPIRY_DELTA = 20; /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** The pre-allocation chunk size for blk?????.dat files (since 0.8) */ @@ -110,6 +112,7 @@ struct BlockHasher size_t operator()(const uint256& hash) const { return hash.GetCheapHash(); } }; +extern unsigned int expiryDelta; extern CScript COINBASE_FLAGS; extern CCriticalSection cs_main; extern CTxMemPool mempool; @@ -371,6 +374,12 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins */ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); +/** + * Check if transaction is expired and can be included in a block with the + * specified height. Consensus critical. + */ +bool IsExpiredTx(const CTransaction &tx, int nBlockHeight); + /** * Check if transaction will be final in the next block to be created. * diff --git a/src/miner.cpp b/src/miner.cpp index d87205c64..ccbee75ab 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -166,7 +166,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) ? nMedianTimePast : pblock->GetBlockTime(); - if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, nLockTimeCutoff)) + if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, nLockTimeCutoff) || IsExpiredTx(tx, nHeight)) continue; COrphan* porphan = NULL; @@ -345,6 +345,8 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) txNew.vout.resize(1); txNew.vout[0].scriptPubKey = scriptPubKeyIn; txNew.vout[0].nValue = GetBlockSubsidy(nHeight, chainparams.GetConsensus()); + // Set to 0 so expiry height does not apply to coinbase txs + txNew.nExpiryHeight = 0; if ((nHeight > 0) && (nHeight <= chainparams.GetConsensus().GetLastFoundersRewardBlockHeight())) { // Founders reward is 20% of the block subsidy diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 654a68b8a..bcd7eaa8a 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -304,9 +304,6 @@ public: std::string ToString() const; }; -// The maximum value which is valid for expiry height, used by CTransaction and CMutableTransaction -static constexpr uint32_t TX_EXPIRY_HEIGHT_THRESHOLD = 500000000; - // Overwinter version group id static constexpr uint32_t OVERWINTER_VERSION_GROUP_ID = 0x03C48270; static_assert(OVERWINTER_VERSION_GROUP_ID != 0, "version group id must be non-zero as specified in ZIP 202"); diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index 2f89529de..f28066300 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -198,6 +198,7 @@ UniValue getrawtransaction(const UniValue& params, bool fHelp) " \"txid\" : \"id\", (string) The transaction id (same as provided)\n" " \"version\" : n, (numeric) The version\n" " \"locktime\" : ttt, (numeric) The lock time\n" + " \"expiryheight\" : ttt, (numeric, optional) The block height after which the transaction expires\n" " \"vin\" : [ (array of json objects)\n" " {\n" " \"txid\": \"id\", (string) The transaction id\n" @@ -443,8 +444,16 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) UniValue inputs = params[0].get_array(); UniValue sendTo = params[1].get_obj(); + int nextBlockHeight = chainActive.Height() + 1; CMutableTransaction rawTx = CreateNewContextualCMutableTransaction( - Params().GetConsensus(), chainActive.Height() + 1); + Params().GetConsensus(), nextBlockHeight); + + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + rawTx.nExpiryHeight = nextBlockHeight + expiryDelta; + if (rawTx.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD){ + throw JSONRPCError(RPC_INVALID_PARAMETER, "nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); + } + } for (size_t idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; @@ -497,7 +506,7 @@ UniValue decoderawtransaction(const UniValue& params, bool fHelp) "\nResult:\n" "{\n" " \"txid\" : \"id\", (string) The transaction id\n" - " \"overwintered\" : bool (boolean) The Overwintered flag\n" + " \"overwintered\" : bool (boolean) The Overwintered flag\n" " \"version\" : n, (numeric) The version\n" " \"versiongroupid\": \"hex\" (string, optional) The version group id (Overwintered txs)\n" " \"locktime\" : ttt, (numeric) The lock time\n" diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 6b3c16f2a..acb671ec2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -256,6 +256,24 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list } } +void CTxMemPool::removeExpired(unsigned int nBlockHeight) +{ + // Remove expired txs from the mempool + LOCK(cs); + list transactionsToRemove; + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) + { + const CTransaction& tx = it->GetTx(); + if (IsExpiredTx(tx, nBlockHeight)) { + transactionsToRemove.push_back(tx); + } + } + for (const CTransaction& tx : transactionsToRemove) { + list removed; + remove(tx, removed, true); + } +} + /** * Called when a block is connected. Removes from mempool and updates the miner fee estimator. */ diff --git a/src/txmempool.h b/src/txmempool.h index 2cb2c8f05..6b6434f1e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -168,6 +168,7 @@ public: void removeWithAnchor(const uint256 &invalidRoot); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::list& removed); + void removeExpired(unsigned int nBlockHeight); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate = true); void removeWithoutBranchId(uint32_t nMemPoolBranchId); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d3da306ed..9050c4b1b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -86,6 +86,7 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) entry.push_back(Pair("blockhash", wtx.hashBlock.GetHex())); entry.push_back(Pair("blockindex", wtx.nIndex)); entry.push_back(Pair("blocktime", mapBlockIndex[wtx.hashBlock]->GetBlockTime())); + entry.push_back(Pair("expiryheight", (int64_t)wtx.nExpiryHeight)); } uint256 hash = wtx.GetHash(); entry.push_back(Pair("txid", hash.GetHex())); @@ -3534,11 +3535,15 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) UniValue contextInfo = o; // Contextual transaction we will build on - CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), chainActive.Height() + 1); + int nextBlockHeight = chainActive.Height() + 1; + CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nextBlockHeight); bool isShielded = !fromTaddr || zaddrRecipients.size() > 0; if (contextualTx.nVersion == 1 && isShielded) { contextualTx.nVersion = 2; // Tx format should support vjoinsplits } + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta; + } // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); @@ -3725,12 +3730,15 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) contextInfo.push_back(Pair("fee", ValueFromAmount(nFee))); // Contextual transaction we will build on + int nextBlockHeight = chainActive.Height() + 1; CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction( - Params().GetConsensus(), - chainActive.Height() + 1); + Params().GetConsensus(), nextBlockHeight); if (contextualTx.nVersion == 1) { contextualTx.nVersion = 2; // Tx format should support vjoinsplits } + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta; + } // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ce86cbad0..253eb6169 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -10,6 +10,7 @@ #include "coincontrol.h" #include "consensus/upgrades.h" #include "consensus/validation.h" +#include "consensus/consensus.h" #include "init.h" #include "main.h" #include "net.h" @@ -2523,6 +2524,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount &nFeeRet, int& nC CReserveKey reservekey(this); CWalletTx wtx; + if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosRet, strFailReason, &coinControl, false)) return false; @@ -2573,9 +2575,20 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt wtxNew.fTimeReceivedIsTxTime = true; wtxNew.BindWallet(this); + int nextBlockHeight = chainActive.Height() + 1; CMutableTransaction txNew = CreateNewContextualCMutableTransaction( - Params().GetConsensus(), chainActive.Height() + 1); + Params().GetConsensus(), nextBlockHeight); + // Activates after Overwinter network upgrade + // Set nExpiryHeight to expiryDelta (default 20) blocks past current block height + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + if (nextBlockHeight + expiryDelta >= TX_EXPIRY_HEIGHT_THRESHOLD){ + strFailReason = _("nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); + } else { + txNew.nExpiryHeight = nextBlockHeight + expiryDelta; + } + } + // Discourage fee sniping. // // However because of a off-by-one-error in previous versions we need to From 9c12b8e90360f393d79f06cc5f80dc87240387b4 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Wed, 21 Feb 2018 20:21:24 -0800 Subject: [PATCH 2/5] Add -txexpirydelta cli option --- src/init.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/init.cpp b/src/init.cpp index dd10025dc..85507f9a3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -414,6 +414,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-sendfreetransactions", strprintf(_("Send transactions as zero-fee transactions if possible (default: %u)"), 0)); strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), 1)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); + strUsage += HelpMessageOpt("-txexpirydelta", strprintf(_("Set the number of blocks after which a transaction that has not been mined will become invalid (default: %u)"), DEFAULT_TX_EXPIRY_DELTA)); strUsage += HelpMessageOpt("-maxtxfee=", strprintf(_("Maximum total fees (in %s) to use in a single wallet transaction; setting this too low may abort large transactions (default: %s)"), CURRENCY_UNIT, FormatMoney(maxTxFee))); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format") + " " + _("on startup")); @@ -1009,6 +1010,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) } } nTxConfirmTarget = GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); + expiryDelta = GetArg("-txexpirydelta", DEFAULT_TX_EXPIRY_DELTA); bSpendZeroConfChange = GetBoolArg("-spendzeroconfchange", true); fSendFreeTransactions = GetBoolArg("-sendfreetransactions", false); From 5943f227dafaece87bf2a2febd3d604ca5de18cc Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Thu, 22 Feb 2018 23:27:38 -0800 Subject: [PATCH 3/5] Add mempool_tx_expiry.py test --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/mempool_tx_expiry.py | 187 ++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100755 qa/rpc-tests/mempool_tx_expiry.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index e2272ad43..6b0125aa8 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -32,6 +32,7 @@ testScripts=( 'mempool_reorg.py' 'mempool_tx_input_limit.py' 'mempool_nu_activation.py' + 'mempool_tx_expiry.py' 'httpbasics.py' 'zapwallettxes.py' 'proxy_test.py' diff --git a/qa/rpc-tests/mempool_tx_expiry.py b/qa/rpc-tests/mempool_tx_expiry.py new file mode 100755 index 000000000..b78af986c --- /dev/null +++ b/qa/rpc-tests/mempool_tx_expiry.py @@ -0,0 +1,187 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# +# Test proper expiry for transactions >= version 3 +# + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.authproxy import JSONRPCException +from test_framework.util import assert_equal, connect_nodes, \ + connect_nodes_bi, sync_blocks, start_nodes, sync_blocks, sync_mempools, \ + wait_and_assert_operationid_status + +from decimal import Decimal +import time + +class MempoolTxExpiryTest(BitcoinTestFramework): + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [["-nuparams=5ba81b19:205", "-txexpirydelta=4"]] * 4) + + # Test before, at, and after expiry block + # TODO: Test case of dependent txs in reorgs + # chain is at block height 199 when run_test executes + def run_test(self): + alice = self.nodes[0].getnewaddress() + z_alice = self.nodes[0].z_getnewaddress() + bob = self.nodes[2].getnewaddress() + z_bob = self.nodes[2].z_getnewaddress() + + # When Overwinter not yet activated, no expiryheight in tx + sapling_tx = self.nodes[0].sendtoaddress(bob, 0.01) + rawtx = self.nodes[0].getrawtransaction(sapling_tx, 1) + assert_equal(rawtx["overwintered"], False) + assert("expiryheight" not in rawtx) + + self.nodes[0].generate(6) + self.sync_all() + + ## Shield one of Alice's coinbase funds to her zaddr + res = self.nodes[0].z_shieldcoinbase("*", z_alice, 0.0001, 1) + wait_and_assert_operationid_status(self.nodes[0], res['opid']) + self.nodes[0].generate(1) + self.sync_all() + + # Get balance on node 0 + bal = self.nodes[0].z_gettotalbalance() + print "Balance before zsend, after shielding 10: ", bal + assert_equal(Decimal(bal["private"]), Decimal("9.9999")) + + print "Splitting network..." + self.split_network() + + # Create transactions + zsendamount = Decimal('1.0') - Decimal('0.0001') + recipients = [] + recipients.append({"address": z_bob, "amount": zsendamount}) + myopid = self.nodes[0].z_sendmany(z_alice, recipients) + persist_shielded = wait_and_assert_operationid_status(self.nodes[0], myopid) + persist_transparent = self.nodes[0].sendtoaddress(bob, 0.01) + # Verify transparent transaction is version 3 intended for Overwinter branch + rawtx = self.nodes[0].getrawtransaction(persist_transparent, 1) + assert_equal(rawtx["version"], 3) + assert_equal(rawtx["overwintered"], True) + assert_equal(rawtx["expiryheight"], 212) + print "Blockheight at persist_transparent & persist_shielded creation:", self.nodes[0].getblockchaininfo()['blocks'] + print "Expiryheight of persist_transparent:", rawtx['expiryheight'] + # Verify shielded transaction is version 3 intended for Overwinter branch + rawtx = self.nodes[0].getrawtransaction(persist_shielded, 1) + print "Expiryheight of persist_shielded", rawtx['expiryheight'] + assert_equal(rawtx["version"], 3) + assert_equal(rawtx["overwintered"], True) + assert_equal(rawtx["expiryheight"], 212) + + print "\n Blockheight advances to less than expiry block height. After reorg, txs should persist in mempool" + assert(persist_transparent in self.nodes[0].getrawmempool()) + assert(persist_shielded in self.nodes[0].getrawmempool()) + assert_equal(set(self.nodes[2].getrawmempool()), set()) + print "mempool node 0:", self.nodes[0].getrawmempool() + print "mempool node 2:", self.nodes[2].getrawmempool() + bal = self.nodes[0].z_gettotalbalance() + print "Printing balance before persist_shielded & persist_transparent are initially mined from mempool", bal + # Txs are mined on node 0; will later be rolled back + self.nodes[0].generate(1) + print "Node 0 generated 1 block" + print "Node 0 height:", self.nodes[0].getblockchaininfo()['blocks'] + print "Node 2 height:", self.nodes[2].getblockchaininfo()['blocks'] + bal = self.nodes[0].z_gettotalbalance() + print "Printing balance after persist_shielded & persist_transparent are mined:", bal + assert_equal(set(self.nodes[0].getrawmempool()), set()) + + print "Mine 2 competing blocks on Node 2..." + blocks = self.nodes[2].generate(2) + for block in blocks: + blk = self.nodes[2].getblock(block) + print "Height: {0}, Mined block txs: {1}".format(blk["height"], blk["tx"]) + print "Connect nodes to force a reorg" + connect_nodes_bi(self.nodes,0,2) + self.is_network_split = False + + print "Syncing blocks" + sync_blocks(self.nodes) + + print "Ensure that txs are back in mempool of node 0" + print "Blockheight node 0:", self.nodes[0].getblockchaininfo()['blocks'] + print "Blockheight node 2:", self.nodes[2].getblockchaininfo()['blocks'] + print "mempool node 0: ", self.nodes[0].getrawmempool() + print "mempool node 2: ", self.nodes[2].getrawmempool() + assert(persist_transparent in self.nodes[0].getrawmempool()) + assert(persist_shielded in self.nodes[0].getrawmempool()) + bal = self.nodes[0].z_gettotalbalance() + # Mine txs to get them out of the way of mempool sync in split_network() + print "Generating another block on node 0 to clear txs from mempool" + self.nodes[0].generate(1) + assert_equal(set(self.nodes[0].getrawmempool()), set()) + sync_blocks(self.nodes) + + print "Splitting network..." + self.split_network() + + print "\n Blockheight advances to equal expiry block height. After reorg, txs should persist in mempool" + myopid = self.nodes[0].z_sendmany(z_alice, recipients) + persist_shielded_2 = wait_and_assert_operationid_status(self.nodes[0], myopid) + persist_transparent_2 = self.nodes[0].sendtoaddress(bob, 0.01) + rawtx_trans = self.nodes[0].getrawtransaction(persist_transparent_2, 1) + rawtx_shield = self.nodes[0].getrawtransaction(persist_shielded_2, 1) + print "Blockheight node 0 at persist_transparent_2 creation:", self.nodes[0].getblockchaininfo()['blocks'] + print "Blockheight node 2 at persist_transparent_2 creation:", self.nodes[2].getblockchaininfo()['blocks'] + print "Expiryheight of persist_transparent_2:", rawtx_trans['expiryheight'] + print "Expiryheight of persist_shielded_2:", rawtx_shield['expiryheight'] + blocks = self.nodes[2].generate(4) + for block in blocks: + blk = self.nodes[2].getblock(block) + print "Height: {0}, Mined block txs: {1}".format(blk["height"], blk["tx"]) + print "Connect nodes to force a reorg" + connect_nodes_bi(self.nodes, 0, 2) + self.is_network_split = False + sync_blocks(self.nodes) + print "Ensure that persist_transparent_2 & persist_shielded_2 are in mempool at expiry block height" + print "Blockheight node 0:", self.nodes[0].getblockchaininfo()['blocks'] + print "Blockheight node 2:", self.nodes[2].getblockchaininfo()['blocks'] + print "mempool node 0: ", self.nodes[0].getrawmempool() + print "mempool node 2: ", self.nodes[2].getrawmempool() + assert(persist_transparent_2 in self.nodes[0].getrawmempool()) + assert(persist_shielded_2 in self.nodes[0].getrawmempool()) + # Mine persist txs to get them out of the way of mempool sync in split_network() + self.nodes[0].generate(1) + assert_equal(set(self.nodes[0].getrawmempool()), set()) + sync_blocks(self.nodes) + print "Balance after persist_shielded_2 is mined to remove from mempool: ", self.nodes[0].z_gettotalbalance() + + print "Splitting network..." + self.split_network() + + print "\n Blockheight advances to greater than expiry block height. After reorg, txs should expire from mempool" + print "Balance before expire_shielded is sent: ", self.nodes[0].z_gettotalbalance() + myopid = self.nodes[0].z_sendmany(z_alice, recipients) + expire_shielded = wait_and_assert_operationid_status(self.nodes[0], myopid) + expire_transparent = self.nodes[0].sendtoaddress(bob, 0.01) + print "Blockheight node 0 at expire_transparent creation:", self.nodes[0].getblockchaininfo()['blocks'] + print "Blockheight node 2 at expire_shielded creation:", self.nodes[2].getblockchaininfo()['blocks'] + print "Expiryheight of expire_transparent:", self.nodes[0].getrawtransaction(expire_transparent, 1)['expiryheight'] + print "Expiryheight of expire_shielded:", self.nodes[0].getrawtransaction(expire_shielded, 1)['expiryheight'] + assert(expire_transparent in self.nodes[0].getrawmempool()) + assert(expire_shielded in self.nodes[0].getrawmempool()) + blocks = self.nodes[2].generate(6) + for block in blocks: + blk = self.nodes[2].getblock(block) + print "Height: {0}, Mined block txs: {1}".format(blk["height"], blk["tx"]) + print "Connect nodes to force a reorg" + connect_nodes_bi(self.nodes, 0, 2) + self.is_network_split = False + sync_blocks(self.nodes) + print "Ensure that expire_transparent & expire_shielded are in mempool at expiry block height" + print "mempool node 0: ", self.nodes[0].getrawmempool() + print "mempool node 2: ", self.nodes[2].getrawmempool() + assert_equal(set(self.nodes[0].getrawmempool()), set()) + print "Ensure balance of node 0 is correct" + bal = self.nodes[0].z_gettotalbalance() + print "Balance after expire_shielded has expired: ", bal + assert_equal(Decimal(bal["private"]), Decimal("7.9999")) + + +if __name__ == '__main__': + MempoolTxExpiryTest().main() From 7b92f27e18056132a5073e65b70e0aeaa1ebbb0b Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Fri, 23 Feb 2018 13:24:53 -0800 Subject: [PATCH 4/5] Add expiry to z_mergetoaddress --- src/wallet/rpcwallet.cpp | 6 +++++- src/wallet/wallet.cpp | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9050c4b1b..af9064d44 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4065,13 +4065,17 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) contextInfo.push_back(Pair("fee", ValueFromAmount(nFee))); // Contextual transaction we will build on + int nextBlockHeight = chainActive.Height() + 1; CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction( Params().GetConsensus(), - chainActive.Height() + 1); + nextBlockHeight); bool isShielded = numNotes > 0 || isToZaddr; if (contextualTx.nVersion == 1 && isShielded) { contextualTx.nVersion = 2; // Tx format should support vjoinsplit } + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta; + } // Create operation and add to global queue std::shared_ptr q = getAsyncRPCQueue(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 253eb6169..e2cd01637 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2584,6 +2584,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { if (nextBlockHeight + expiryDelta >= TX_EXPIRY_HEIGHT_THRESHOLD){ strFailReason = _("nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); + return false; } else { txNew.nExpiryHeight = nextBlockHeight + expiryDelta; } From 59da58cdb09371fee6cfb845aaa7533ee6b53405 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Fri, 23 Feb 2018 18:20:30 -0800 Subject: [PATCH 5/5] Change rpc_tests to 21 --- src/test/rpc_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 184b9d54e..b7223657c 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -316,7 +316,7 @@ BOOST_AUTO_TEST_CASE(rpc_raw_create_overwinter_v3) BOOST_CHECK_NO_THROW(r = CallRPC(string("decoderawtransaction ") + rawhex)); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "overwintered").get_bool(), true); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "version").get_int(), 3); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "expiryheight").get_int(), 0); + BOOST_CHECK_EQUAL(find_value(r.get_obj(), "expiryheight").get_int(), 21); BOOST_CHECK_EQUAL( ParseHexToUInt32(find_value(r.get_obj(), "versiongroupid").get_str()), OVERWINTER_VERSION_GROUP_ID);