Auto merge of #2342 - bitcartel:1081__mempoolpatch, r=str4d
Add ability for node to reject tx from mempool by number of tx inputs Implement short-term solution described in #2343 so that users can respond promptly to critical short-term problems caused by quadratic validation scaling, such as the getblocktemplate latency, block propagation latency, and mempool size inflation issues described in #2333.
This commit is contained in:
commit
59de56eeca
|
@ -26,6 +26,7 @@ testScripts=(
|
|||
'rest.py'
|
||||
'mempool_spendcoinbase.py'
|
||||
'mempool_coinbase_spends.py'
|
||||
'mempool_tx_input_limit.py'
|
||||
'httpbasics.py'
|
||||
'zapwallettxes.py'
|
||||
'proxy_test.py'
|
||||
|
|
|
@ -0,0 +1,152 @@
|
|||
#!/usr/bin/env python2
|
||||
# Copyright (c) 2017 The Zcash developers
|
||||
# Distributed under the MIT software license, see the accompanying
|
||||
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
|
||||
from test_framework.test_framework import BitcoinTestFramework
|
||||
from test_framework.util import *
|
||||
import os
|
||||
import shutil
|
||||
from time import sleep
|
||||
|
||||
# Test -mempooltxinputlimit
|
||||
class MempoolTxInputLimitTest(BitcoinTestFramework):
|
||||
|
||||
alert_filename = None # Set by setup_network
|
||||
|
||||
def setup_network(self):
|
||||
args = ["-checkmempool", "-debug=mempool", "-mempooltxinputlimit=2"]
|
||||
self.nodes = []
|
||||
self.nodes.append(start_node(0, self.options.tmpdir, args))
|
||||
self.nodes.append(start_node(1, self.options.tmpdir, args))
|
||||
connect_nodes(self.nodes[1], 0)
|
||||
self.is_network_split = False
|
||||
self.sync_all
|
||||
|
||||
def setup_chain(self):
|
||||
print "Initializing test directory "+self.options.tmpdir
|
||||
initialize_chain_clean(self.options.tmpdir, 2)
|
||||
|
||||
def call_z_sendmany(self, from_addr, to_addr, amount):
|
||||
recipients = []
|
||||
recipients.append({"address": to_addr, "amount": amount})
|
||||
myopid = self.nodes[0].z_sendmany(from_addr, recipients)
|
||||
return self.wait_and_assert_operationid_status(myopid)
|
||||
|
||||
def wait_and_assert_operationid_status(self, myopid, in_status='success', in_errormsg=None):
|
||||
print('waiting for async operation {}'.format(myopid))
|
||||
opids = []
|
||||
opids.append(myopid)
|
||||
timeout = 300
|
||||
status = None
|
||||
errormsg = None
|
||||
txid = None
|
||||
for x in xrange(1, timeout):
|
||||
results = self.nodes[0].z_getoperationresult(opids)
|
||||
if len(results)==0:
|
||||
sleep(1)
|
||||
else:
|
||||
status = results[0]["status"]
|
||||
if status == "failed":
|
||||
errormsg = results[0]['error']['message']
|
||||
elif status == "success":
|
||||
txid = results[0]['result']['txid']
|
||||
break
|
||||
print('...returned status: {}'.format(status))
|
||||
assert_equal(in_status, status)
|
||||
if errormsg is not None:
|
||||
assert(in_errormsg is not None)
|
||||
assert_equal(in_errormsg in errormsg, True)
|
||||
print('...returned error: {}'.format(errormsg))
|
||||
return txid
|
||||
|
||||
def run_test(self):
|
||||
start_count = self.nodes[0].getblockcount()
|
||||
|
||||
self.nodes[0].generate(100)
|
||||
self.sync_all()
|
||||
# Mine three blocks. After this, nodes[0] blocks
|
||||
# 1, 2, and 3 are spend-able.
|
||||
self.nodes[1].generate(3)
|
||||
self.sync_all()
|
||||
|
||||
# Check 1: z_sendmany is limited by -mempooltxinputlimit
|
||||
|
||||
# Add zaddr to node 0
|
||||
node0_zaddr = self.nodes[0].z_getnewaddress()
|
||||
|
||||
# Send three inputs from node 0 taddr to zaddr to get out of coinbase
|
||||
node0_taddr = self.nodes[0].getnewaddress();
|
||||
recipients = []
|
||||
recipients.append({"address":node0_zaddr, "amount":Decimal('30.0')-Decimal('0.0001')}) # utxo amount less fee
|
||||
myopid = self.nodes[0].z_sendmany(node0_taddr, recipients)
|
||||
|
||||
opids = []
|
||||
opids.append(myopid)
|
||||
|
||||
# Spend should fail due to -mempooltxinputlimit
|
||||
timeout = 120
|
||||
status = None
|
||||
for x in xrange(1, timeout):
|
||||
results = self.nodes[0].z_getoperationresult(opids)
|
||||
if len(results)==0:
|
||||
sleep(1)
|
||||
else:
|
||||
status = results[0]["status"]
|
||||
msg = results[0]["error"]["message"]
|
||||
assert_equal("failed", status)
|
||||
assert_equal("Too many transparent inputs 3 > limit 2", msg)
|
||||
break
|
||||
|
||||
# Mempool should be empty.
|
||||
assert_equal(set(self.nodes[0].getrawmempool()), set())
|
||||
|
||||
# Reduce amount to only use two inputs
|
||||
spend_zaddr_amount = Decimal('20.0') - Decimal('0.0001')
|
||||
spend_zaddr_id = self.call_z_sendmany(node0_taddr, node0_zaddr, spend_zaddr_amount) # utxo amount less fee
|
||||
self.sync_all()
|
||||
|
||||
# Spend should be in the mempool
|
||||
assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_zaddr_id ]))
|
||||
|
||||
self.nodes[0].generate(1)
|
||||
self.sync_all()
|
||||
|
||||
# mempool should be empty.
|
||||
assert_equal(set(self.nodes[0].getrawmempool()), set())
|
||||
|
||||
# Check 2: sendfrom is limited by -mempooltxinputlimit
|
||||
node1_taddr = self.nodes[1].getnewaddress();
|
||||
recipients = []
|
||||
spend_taddr_amount = spend_zaddr_amount - Decimal('0.0001')
|
||||
spend_taddr_output = Decimal('8')
|
||||
|
||||
# Create three outputs
|
||||
recipients.append({"address":self.nodes[1].getnewaddress(), "amount": spend_taddr_output})
|
||||
recipients.append({"address":self.nodes[1].getnewaddress(), "amount": spend_taddr_output})
|
||||
recipients.append({"address":self.nodes[1].getnewaddress(), "amount": spend_taddr_amount - spend_taddr_output - spend_taddr_output})
|
||||
|
||||
myopid = self.nodes[0].z_sendmany(node0_zaddr, recipients)
|
||||
self.wait_and_assert_operationid_status(myopid)
|
||||
self.nodes[1].generate(1)
|
||||
self.sync_all()
|
||||
|
||||
# Should use three UTXOs and fail
|
||||
try:
|
||||
self.nodes[1].sendfrom("", node0_taddr, spend_taddr_amount - Decimal('1'))
|
||||
assert(False)
|
||||
except JSONRPCException,e:
|
||||
msg = e.error['message']
|
||||
assert_equal("Too many transparent inputs 3 > limit 2", msg)
|
||||
|
||||
# mempool should be empty.
|
||||
assert_equal(set(self.nodes[1].getrawmempool()), set())
|
||||
|
||||
# Should use two UTXOs and succeed
|
||||
spend_taddr_id2 = self.nodes[1].sendfrom("", node0_taddr, spend_taddr_output + spend_taddr_output - Decimal('1'))
|
||||
|
||||
# Spend should be in the mempool
|
||||
assert_equal(set(self.nodes[1].getrawmempool()), set([ spend_taddr_id2 ]))
|
||||
|
||||
if __name__ == '__main__':
|
||||
MempoolTxInputLimitTest().main()
|
|
@ -1,10 +1,13 @@
|
|||
#include <gtest/gtest.h>
|
||||
#include <gtest/gtest-spi.h>
|
||||
|
||||
#include "consensus/validation.h"
|
||||
#include "core_io.h"
|
||||
#include "main.h"
|
||||
#include "primitives/transaction.h"
|
||||
#include "txmempool.h"
|
||||
#include "policy/fees.h"
|
||||
#include "util.h"
|
||||
|
||||
// Fake the input of transaction 5295156213414ed77f6e538e7e8ebe14492156906b9fe995b242477818789364
|
||||
// - 532639cc6bebed47c1c69ae36dd498c68a012e74ad12729adbd3dbb56f8f3f4a, 0
|
||||
|
@ -87,3 +90,49 @@ TEST(Mempool, PriorityStatsDoNotCrash) {
|
|||
|
||||
EXPECT_EQ(dPriority, MAX_PRIORITY);
|
||||
}
|
||||
|
||||
TEST(Mempool, TxInputLimit) {
|
||||
CTxMemPool pool(::minRelayTxFee);
|
||||
bool missingInputs;
|
||||
|
||||
// Create an obviously-invalid transaction
|
||||
// We intentionally set tx.nVersion = 0 to reliably trigger an error, as
|
||||
// it's the first check that occurs after the -mempooltxinputlimit check,
|
||||
// and it means that we don't have to mock out a lot of global state.
|
||||
CMutableTransaction mtx;
|
||||
mtx.nVersion = 0;
|
||||
mtx.vin.resize(10);
|
||||
|
||||
// Check it fails as expected
|
||||
CValidationState state1;
|
||||
CTransaction tx1(mtx);
|
||||
EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
|
||||
EXPECT_EQ(state1.GetRejectReason(), "bad-txns-version-too-low");
|
||||
|
||||
// Set a limit
|
||||
mapArgs["-mempooltxinputlimit"] = "10";
|
||||
|
||||
// Check it stil fails as expected
|
||||
CValidationState state2;
|
||||
EXPECT_FALSE(AcceptToMemoryPool(pool, state2, tx1, false, &missingInputs));
|
||||
EXPECT_EQ(state2.GetRejectReason(), "bad-txns-version-too-low");
|
||||
|
||||
// Resize the transaction
|
||||
mtx.vin.resize(11);
|
||||
|
||||
// Check it now fails due to exceeding the limit
|
||||
CValidationState state3;
|
||||
CTransaction tx3(mtx);
|
||||
EXPECT_FALSE(AcceptToMemoryPool(pool, state3, tx3, false, &missingInputs));
|
||||
// The -mempooltxinputlimit check doesn't set a reason
|
||||
EXPECT_EQ(state3.GetRejectReason(), "");
|
||||
|
||||
// Clear the limit
|
||||
mapArgs.erase("-mempooltxinputlimit");
|
||||
|
||||
// Check it no longer fails due to exceeding the limit
|
||||
CValidationState state4;
|
||||
EXPECT_FALSE(AcceptToMemoryPool(pool, state4, tx3, false, &missingInputs));
|
||||
EXPECT_EQ(state4.GetRejectReason(), "bad-txns-version-too-low");
|
||||
}
|
||||
|
||||
|
|
12
src/init.cpp
12
src/init.cpp
|
@ -352,6 +352,7 @@ std::string HelpMessage(HelpMessageMode mode)
|
|||
strUsage += HelpMessageOpt("-dbcache=<n>", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache));
|
||||
strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file") + " " + _("on startup"));
|
||||
strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
|
||||
strUsage += HelpMessageOpt("-mempooltxinputlimit=<n>", _("Set the maximum number of transparent inputs in a transaction that the mempool will accept (default: 0 = no limit applied)"));
|
||||
strUsage += HelpMessageOpt("-par=<n>", strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"),
|
||||
-GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS));
|
||||
#ifndef WIN32
|
||||
|
@ -1036,6 +1037,17 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
|
|||
}
|
||||
#endif
|
||||
|
||||
// Default value of 0 for mempooltxinputlimit means no limit is applied
|
||||
if (mapArgs.count("-mempooltxinputlimit")) {
|
||||
int64_t limit = GetArg("-mempooltxinputlimit", 0);
|
||||
if (limit < 0) {
|
||||
return InitError(_("Mempool limit on transparent inputs to a transaction cannot be negative"));
|
||||
} else if (limit > 0) {
|
||||
LogPrintf("Mempool configured to reject transactions with greater than %lld transparent inputs\n", limit);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// ********************************************************* Step 4: application initialization: dir lock, daemonize, pidfile, debug log
|
||||
|
||||
// Initialize libsodium
|
||||
|
|
11
src/main.cpp
11
src/main.cpp
|
@ -1058,6 +1058,17 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
|
|||
if (pfMissingInputs)
|
||||
*pfMissingInputs = false;
|
||||
|
||||
// Node operator can choose to reject tx by number of transparent inputs
|
||||
static_assert(std::numeric_limits<size_t>::max() >= std::numeric_limits<int64_t>::max(), "size_t too small");
|
||||
size_t limit = (size_t) GetArg("-mempooltxinputlimit", 0);
|
||||
if (limit > 0) {
|
||||
size_t n = tx.vin.size();
|
||||
if (n > limit) {
|
||||
LogPrint("mempool", "Dropping txid %s : too many transparent inputs %zu > limit %zu\n", tx.GetHash().ToString(), n, limit );
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
auto verifier = libzcash::ProofVerifier::Strict();
|
||||
if (!CheckTransaction(tx, state, verifier))
|
||||
return error("AcceptToMemoryPool: CheckTransaction failed");
|
||||
|
|
|
@ -281,6 +281,15 @@ bool AsyncRPCOperation_sendmany::main_impl() {
|
|||
t_inputs_ = selectedTInputs;
|
||||
t_inputs_total = selectedUTXOAmount;
|
||||
|
||||
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
|
||||
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
|
||||
if (limit > 0) {
|
||||
size_t n = t_inputs_.size();
|
||||
if (n > limit) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Too many transparent inputs %zu > limit %zu", n, limit));
|
||||
}
|
||||
}
|
||||
|
||||
// update the transaction with these inputs
|
||||
CMutableTransaction rawTx(tx_);
|
||||
for (SendManyInputUTXO & t : t_inputs_) {
|
||||
|
|
|
@ -2717,6 +2717,16 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
|
|||
txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(),
|
||||
std::numeric_limits<unsigned int>::max()-1));
|
||||
|
||||
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
|
||||
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
|
||||
if (limit > 0) {
|
||||
size_t n = txNew.vin.size();
|
||||
if (n > limit) {
|
||||
strFailReason = _(strprintf("Too many transparent inputs %zu > limit %zu", n, limit).c_str());
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Sign
|
||||
int nIn = 0;
|
||||
CTransaction txNewConst(txNew);
|
||||
|
|
Loading…
Reference in New Issue