Auto merge of #3098 - str4d:2343-overwinter-disable-mempooltxinputlimit, r=str4d

Ignore -mempooltxinputlimit once Overwinter activates

Overwinter changes the SignatureHash function to solve the quadratic hashing problem, so this option will no longer be needed.
This commit is contained in:
Homu
2018-03-30 08:33:58 -07:00
11 changed files with 112 additions and 21 deletions

View File

@@ -96,7 +96,7 @@ TEST(Mempool, PriorityStatsDoNotCrash) {
}
TEST(Mempool, TxInputLimit) {
SelectParams(CBaseChainParams::TESTNET);
SelectParams(CBaseChainParams::REGTEST);
CTxMemPool pool(::minRelayTxFee);
bool missingInputs;
@@ -133,13 +133,30 @@ TEST(Mempool, TxInputLimit) {
// The -mempooltxinputlimit check doesn't set a reason
EXPECT_EQ(state3.GetRejectReason(), "");
// Clear the limit
mapArgs.erase("-mempooltxinputlimit");
// Activate Overwinter
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
// 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");
// Deactivate Overwinter
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
// Check it now fails due to exceeding the limit
CValidationState state5;
EXPECT_FALSE(AcceptToMemoryPool(pool, state5, tx3, false, &missingInputs));
// The -mempooltxinputlimit check doesn't set a reason
EXPECT_EQ(state5.GetRejectReason(), "");
// Clear the limit
mapArgs.erase("-mempooltxinputlimit");
// Check it no longer fails due to exceeding the limit
CValidationState state6;
EXPECT_FALSE(AcceptToMemoryPool(pool, state6, tx3, false, &missingInputs));
EXPECT_EQ(state6.GetRejectReason(), "bad-txns-version-too-low");
}
// Valid overwinter v3 format tx gets rejected because overwinter hasn't activated yet.

View File

@@ -355,7 +355,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("-mempooltxinputlimit=<n>", _("[DEPRECATED FROM OVERWINTER] 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

View File

@@ -1169,6 +1169,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
// 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 (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
limit = 0;
}
if (limit > 0) {
size_t n = tx.vin.size();
if (n > limit) {

View File

@@ -204,6 +204,12 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
{
LOCK(cs_main);
if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
limit = 0;
}
}
if (limit > 0 && numInputs > limit) {
throw JSONRPCError(RPC_WALLET_ERROR,
strprintf("Number of transparent inputs %d is greater than mempooltxinputlimit of %d",

View File

@@ -310,6 +310,12 @@ bool AsyncRPCOperation_sendmany::main_impl() {
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
{
LOCK(cs_main);
if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
limit = 0;
}
}
if (limit > 0) {
size_t n = t_inputs_.size();
if (n > limit) {

View File

@@ -186,6 +186,12 @@ bool AsyncRPCOperation_shieldcoinbase::main_impl() {
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
{
LOCK(cs_main);
if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
limit = 0;
}
}
if (limit>0 && numInputs > limit) {
throw JSONRPCError(RPC_WALLET_ERROR,
strprintf("Number of inputs %d is greater than mempooltxinputlimit of %d",

View File

@@ -3712,8 +3712,9 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
"\nShield transparent coinbase funds by sending to a shielded zaddr. This is an asynchronous operation and utxos"
"\nselected for shielding will be locked. If there is an error, they are unlocked. The RPC call `listlockunspent`"
"\ncan be used to return a list of locked utxos. The number of coinbase utxos selected for shielding can be limited"
"\nby the caller. If the limit parameter is set to zero, the -mempooltxinputlimit option will determine the number"
"\nof uxtos. Any limit is constrained by the consensus rule defining a maximum transaction size of "
"\nby the caller. If the limit parameter is set to zero, and Overwinter is not yet active, the -mempooltxinputlimit"
"\noption will determine the number of uxtos. Any limit is constrained by the consensus rule defining a maximum"
"\ntransaction size of "
+ strprintf("%d bytes.", MAX_TX_SIZE)
+ HelpRequiringPassphrase() + "\n"
"\nArguments:\n"
@@ -3722,7 +3723,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
"3. fee (numeric, optional, default="
+ strprintf("%s", FormatMoney(SHIELD_COINBASE_DEFAULT_MINERS_FEE)) + ") The fee amount to attach to this transaction.\n"
"4. limit (numeric, optional, default="
+ strprintf("%d", SHIELD_COINBASE_DEFAULT_LIMIT) + ") Limit on the maximum number of utxos to shield. Set to 0 to use node option -mempooltxinputlimit.\n"
+ strprintf("%d", SHIELD_COINBASE_DEFAULT_LIMIT) + ") Limit on the maximum number of utxos to shield. Set to 0 to use node option -mempooltxinputlimit (before Overwinter), or as many as will fit in the transaction (after Overwinter).\n"
"\nResult:\n"
"{\n"
" \"remainingUTXOs\": xxx (numeric) Number of coinbase utxos still available for shielding.\n"
@@ -3776,6 +3777,9 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
}
}
int nextBlockHeight = chainActive.Height() + 1;
bool overwinterActive = NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER);
// Prepare to get coinbase utxos
std::vector<ShieldCoinbaseUTXO> inputs;
CAmount shieldedValue = 0;
@@ -3783,7 +3787,7 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
size_t estimatedTxSize = 2000; // 1802 joinsplit description + tx overhead + wiggle room
size_t utxoCounter = 0;
bool maxedOutFlag = false;
size_t mempoolLimit = (nLimit != 0) ? nLimit : (size_t)GetArg("-mempooltxinputlimit", 0);
size_t mempoolLimit = (nLimit != 0) ? nLimit : (overwinterActive ? 0 : (size_t)GetArg("-mempooltxinputlimit", 0));
// Set of addresses to filter utxos by
set<CBitcoinAddress> setAddress = {};
@@ -3862,13 +3866,12 @@ 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(), nextBlockHeight);
if (contextualTx.nVersion == 1) {
contextualTx.nVersion = 2; // Tx format should support vjoinsplits
}
if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
if (overwinterActive) {
contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta;
}
@@ -3914,8 +3917,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
"\n\nThis is an asynchronous operation, and UTXOs selected for merging will be locked. If there is an error, they"
"\nare unlocked. The RPC call `listlockunspent` can be used to return a list of locked UTXOs."
"\n\nThe number of UTXOs and notes selected for merging can be limited by the caller. If the transparent limit"
"\nparameter is set to zero, the -mempooltxinputlimit option will determine the number of UTXOs. Any limit is"
"\nconstrained by the consensus rule defining a maximum transaction size of "
"\nparameter is set to zero, and Overwinter is not yet active, the -mempooltxinputlimit option will determine the"
"\nnumber of UTXOs. Any limit is constrained by the consensus rule defining a maximum transaction size of "
+ strprintf("%d bytes.", MAX_TX_SIZE)
+ HelpRequiringPassphrase() + "\n"
"\nArguments:\n"
@@ -3933,7 +3936,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
"3. fee (numeric, optional, default="
+ strprintf("%s", FormatMoney(MERGE_TO_ADDRESS_OPERATION_DEFAULT_MINERS_FEE)) + ") The fee amount to attach to this transaction.\n"
"4. transparent_limit (numeric, optional, default="
+ strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT) + ") Limit on the maximum number of UTXOs to merge. Set to 0 to use node option -mempooltxinputlimit.\n"
+ strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_TRANSPARENT_LIMIT) + ") Limit on the maximum number of UTXOs to merge. Set to 0 to use node option -mempooltxinputlimit (before Overwinter), or as many as will fit in the transaction (after Overwinter).\n"
"4. shielded_limit (numeric, optional, default="
+ strprintf("%d", MERGE_TO_ADDRESS_DEFAULT_SHIELDED_LIMIT) + ") Limit on the maximum number of notes to merge. Set to 0 to merge as many as will fit in the transaction.\n"
"5. \"memo\" (string, optional) Encoded as hex. When toaddress is a z-addr, this will be stored in the memo field of the new note.\n"
@@ -4067,6 +4070,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
MergeToAddressRecipient recipient(destaddress, memo);
int nextBlockHeight = chainActive.Height() + 1;
bool overwinterActive = NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER);
// Prepare to get UTXOs and notes
std::vector<MergeToAddressInputUTXO> utxoInputs;
std::vector<MergeToAddressInputNote> noteInputs;
@@ -4078,7 +4084,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
size_t noteCounter = 0;
bool maxedOutUTXOsFlag = false;
bool maxedOutNotesFlag = false;
size_t mempoolLimit = (nUTXOLimit != 0) ? nUTXOLimit : (size_t)GetArg("-mempooltxinputlimit", 0);
size_t mempoolLimit = (nUTXOLimit != 0) ? nUTXOLimit : (overwinterActive ? 0 : (size_t)GetArg("-mempooltxinputlimit", 0));
size_t estimatedTxSize = 200; // tx overhead + wiggle room
if (isToZaddr) {
@@ -4197,7 +4203,6 @@ 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(),
nextBlockHeight);
@@ -4205,7 +4210,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
if (contextualTx.nVersion == 1 && isShielded) {
contextualTx.nVersion = 2; // Tx format should support vjoinsplit
}
if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
if (overwinterActive) {
contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta;
}

View File

@@ -2775,6 +2775,12 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
// Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
{
LOCK(cs_main);
if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
limit = 0;
}
}
if (limit > 0) {
size_t n = txNew.vin.size();
if (n > limit) {