From 93f6514d8672f2eec78532c3b54bb3f40122586b Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 26 Sep 2024 11:13:07 -0400 Subject: [PATCH 1/2] Disable absurd fee checks when adding to the mempool To protect users who are not using opreturn we prevent any use of z_sendmany with absurd fees if opreturn is not being used, so this change only affects users who are adding opreturn data. Since there is no way to currently send opreturn data via a GUI this still protects all GUI users from absurd fees while allowing CLI users to decide to use higher fees. --- src/main.cpp | 14 ++++++++++++-- src/wallet/rpcwallet.cpp | 17 ++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 3ddf5bb54..f2f3bf831 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1960,7 +1960,17 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa dFreeCount += nSize; } - if (!tx.IsCoinImport() && fRejectAbsurdFee && nFees > ::minRelayTxFee.GetFee(nSize) * 10000 && nFees > nValueOut/19) + // Disable checks for absurd fees when adding to the mempool. Instead, this check is done + // when a user attempts to make a transaction with an absurd fee and only rejects absurd + // fees when OP_RETURN data is NOT being used. This means users making normal financial + // transactions (z2z) are protected from absurd fees, it is only users who are storing + // arbitrary data via a z2t transaction are allowed to (or potentially required) to pay high fees + // It would be nice to detect the use of OP_RETURN right here but it seems to only be known + // inside of IsStandard() inside of IsStandardTx() and we want to avoid doing expensive checks + // multiple times. + fRejectAbsurdFee = false; + + if (fRejectAbsurdFee && !tx.IsCoinImport() && nFees > ::minRelayTxFee.GetFee(nSize) * 10000 && nFees > nValueOut/19) { string errmsg = strprintf("absurdly high fees %s, %d > %d", hash.ToString(), @@ -1968,7 +1978,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa LogPrint("mempool", errmsg.c_str()); return state.Error("AcceptToMemoryPool: " + errmsg); } -//fprintf(stderr,"addmempool 6\n"); + //fprintf(stderr,"addmempool 6\n"); // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 46a059892..1b6110ec5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5155,6 +5155,8 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk) CAmount nTotalOut = 0; // Optional OP_RETURN data CScript opret; + // TODO: enforce that only a single opreturn exists + UniValue opretValue; bool containsSaplingOutput = false; @@ -5189,7 +5191,10 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk) // throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated address: ")+address); setAddress.insert(address); - UniValue opretValue = find_value(o, "opreturn"); + UniValue this_opret = find_value(o, "opreturn"); + if (!this_opret.isNull()) { + opretValue = this_opret; + } // Create the CScript representation of the OP_RETURN if (!opretValue.isNull()) { @@ -5342,12 +5347,18 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk) // or anything less than nDefaultFee instead of being forced to use a custom fee and leak metadata if (nTotalOut < nDefaultFee) { if (nFee > nDefaultFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Small transaction amount %s has fee %s that is greater than the default fee %s", FormatMoney(nTotalOut), FormatMoney(nFee), FormatMoney(nDefaultFee))); + // Allow large fees if OP_RETURN is being used + if( opretValue.isNull() ) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Small transaction amount %s has fee %s that is greater than the default fee %s", FormatMoney(nTotalOut), FormatMoney(nFee), FormatMoney(nDefaultFee))); + } } } else { // Check that the user specified fee is not absurd. if (nFee > nTotalOut) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee %s is greater than the sum of outputs %s and also greater than the default fee", FormatMoney(nFee), FormatMoney(nTotalOut))); + // Allow large fees if OP_RETURN is being used + if( opretValue.isNull() ) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee %s is greater than the sum of outputs %s and also greater than the default fee", FormatMoney(nFee), FormatMoney(nTotalOut))); + } } } } From 47cc49bcc580dfe1b772f42046e0aad19b976477 Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 27 Sep 2024 08:44:14 -0400 Subject: [PATCH 2/2] Change OP_RETURN fee requirements This commit removes the CLI option -opretmintxfee, makes it always enabled, changes the required fee rate and changes which OP_RETURNS get "amnesty" and do not need to pay this fee rate. --- src/init.cpp | 1 - src/miner.cpp | 20 ++++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 7458d4847..c034826d6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -479,7 +479,6 @@ std::string HelpMessage(HelpMessageMode mode) if (showDebug) strUsage += HelpMessageOpt("-mintxfee=", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(CWallet::minTxFee.GetFeePerK()))); - strUsage += HelpMessageOpt("-opretmintxfee=", strprintf(_("Minimum fee (in %s/kB) to allow for OP_RETURN transactions (default: %s)"), CURRENCY_UNIT, 400000 )); strUsage += HelpMessageOpt("-paytxfee=", strprintf(_("Fee (in %s/kB) to add to transactions you send (default: %s)"), CURRENCY_UNIT, FormatMoney(payTxFee.GetFeePerK()))); // If this is used incorrectly (-rescanheight too large), then the local wallet may attempt to spend funds which it does not have witness data about // which will cause a "missing inputs" error when added to the mempool. Rescanning from correct height will fix this. diff --git a/src/miner.cpp b/src/miner.cpp index 2b5c7c033..bb7582d7b 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -475,14 +475,10 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 // fprintf(stderr,"%s: nTxSize = %u\n", __func__, nTxSize); // Opret spam limits - if (mapArgs.count("-opretmintxfee")) + const bool opretminfee = true; + if (opretminfee) { - CAmount n = 0; - CFeeRate opretMinFeeRate; - if (ParseMoney(mapArgs["-opretmintxfee"], n) && n > 0) - opretMinFeeRate = CFeeRate(n); - else - opretMinFeeRate = CFeeRate(400000); // default opretMinFeeRate (1 HUSH per 250 Kb = 0.004 per 1 Kb = 400000 puposhis per 1 Kb) + CFeeRate opretMinFeeRate = CFeeRate(10000000); // default opretMinFeeRate 0.1 HUSH bool fSpamTx = false; unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); @@ -501,9 +497,13 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 } } - if ((nTxOpretSize > 256) && (feeRate < opretMinFeeRate)) fSpamTx = true; - // std::cerr << tx.GetHash().ToString() << " nTxSize." << nTxSize << " nTxOpretSize." << nTxOpretSize << " feeRate." << feeRate.ToString() << " opretMinFeeRate." << opretMinFeeRate.ToString() << " fSpamTx." << fSpamTx << std::endl; - if (fSpamTx) continue; + // opreturns of this size or smaller get amnesty and do not have to pay increased fees + int amnestySize = 128; + if ((nTxOpretSize > amnestySize) && (feeRate < opretMinFeeRate)) { + fSpamTx = true; + std::cerr << __func__ << ": " << tx.GetHash().ToString() << " nTxSize=" << nTxSize << " nTxOpretSize=" << nTxOpretSize << " feeRate=" << feeRate.ToString() << " opretMinFeeRate=" << opretMinFeeRate.ToString() << " fSpamTx=" << fSpamTx << std::endl; + continue; + } // std::cerr << tx.GetHash().ToString() << " vecPriority.size() = " << vecPriority.size() << std::endl; }