Skip to content

Commit d4ed81c

Browse files
random-zebrarandom-zebra
committed
Remove most logging from transaction validation
- backports bitcoin/bitcoin@6cab808 Remove unnecessary direct logging in CheckTransaction, AcceptToMemoryPool, CheckTxInputs, CScriptCheck::operator() All status information should be returned in the CValidationState. Relevant debug information is also added to the CValidationState using the recently introduced debug message. Do keep the "BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags" error as it is meant to appear as bug in the log.
1 parent 94b2577 commit d4ed81c

File tree

2 files changed

+49
-79
lines changed

2 files changed

+49
-79
lines changed

‎src/consensus/tx_verify.cpp‎

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,42 +58,35 @@ bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, bool fReject
5858
{
5959
// Basic checks that don't depend on any context
6060
if (tx.vin.empty())
61-
return state.DoS(10, error("CheckTransaction() : vin empty"),
62-
REJECT_INVALID, "bad-txns-vin-empty");
61+
return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty");
6362
if (tx.vout.empty())
64-
return state.DoS(10, error("CheckTransaction() : vout empty"),
65-
REJECT_INVALID, "bad-txns-vout-empty");
63+
return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty");
6664

6765
// Size limits
6866
unsigned int nMaxSize = MAX_ZEROCOIN_TX_SIZE;
6967

7068
if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > nMaxSize)
71-
return state.DoS(100, error("CheckTransaction() : size limits failed"),
72-
REJECT_INVALID, "bad-txns-oversize");
69+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize");
7370

7471
// Check for negative or overflow output values
7572
const Consensus::Params& consensus = Params().GetConsensus();
7673
CAmount nValueOut = 0;
7774
for (const CTxOut& txout : tx.vout) {
7875
if (txout.IsEmpty() && !tx.IsCoinBase() && !tx.IsCoinStake())
79-
return state.DoS(100, error("CheckTransaction(): txout empty for user transaction"));
76+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-empty");
8077
if (txout.nValue < 0)
81-
return state.DoS(100, error("CheckTransaction() : txout.nValue negative"),
82-
REJECT_INVALID, "bad-txns-vout-negative");
78+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative");
8379
if (txout.nValue > consensus.nMaxMoneyOut)
84-
return state.DoS(100, error("CheckTransaction() : txout.nValue too high"),
85-
REJECT_INVALID, "bad-txns-vout-toolarge");
80+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge");
8681
nValueOut += txout.nValue;
8782
if (!consensus.MoneyRange(nValueOut))
88-
return state.DoS(100, error("CheckTransaction() : txout total out of range"),
89-
REJECT_INVALID, "bad-txns-txouttotal-toolarge");
83+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge");
9084
// check cold staking enforcement (for delegations) and value out
9185
if (txout.scriptPubKey.IsPayToColdStaking()) {
9286
if (!fColdStakingActive)
93-
return state.DoS(10, error("%s: cold staking not active", __func__), REJECT_INVALID, "bad-txns-cold-stake");
87+
return state.DoS(10, false, REJECT_INVALID, "cold-stake-inactive");
9488
if (txout.nValue < MIN_COLDSTAKING_AMOUNT)
95-
return state.DoS(100, error("%s: dust amount (%d) not allowed for cold staking. Min amount: %d",
96-
__func__, txout.nValue, MIN_COLDSTAKING_AMOUNT), REJECT_INVALID, "bad-txns-cold-stake");
89+
return state.DoS(100, false, REJECT_INVALID, "cold-stake-vout-toosmall");
9790
}
9891
}
9992

@@ -104,7 +97,7 @@ bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, bool fReject
10497
for (const CTxIn& txin : tx.vin) {
10598
// Check for duplicate inputs
10699
if (vInOutPoints.count(txin.prevout))
107-
return state.DoS(100, error("CheckTransaction() : duplicate inputs"), REJECT_INVALID, "bad-txns-inputs-duplicate");
100+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
108101

109102
//duplicate zcspend serials are checked in CheckZerocoinSpend()
110103
if (!txin.IsZerocoinSpend()) {
@@ -135,26 +128,24 @@ bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, bool fReject
135128

136129
if (tx.IsCoinBase()) {
137130
if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 150)
138-
return state.DoS(100, error("CheckTransaction() : coinbase script size=%d", tx.vin[0].scriptSig.size()),
139-
REJECT_INVALID, "bad-cb-length");
131+
return state.DoS(100, false, REJECT_INVALID, "bad-cb-length");
140132
} else if (fZerocoinActive && tx.HasZerocoinSpendInputs()) {
141133
if (tx.vin.size() < 1)
142-
return state.DoS(10, error("CheckTransaction() : Zerocoin Spend has less than allowed txin's"), REJECT_INVALID, "bad-zerocoinspend");
134+
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-min-inputs");
143135
if (tx.HasZerocoinPublicSpendInputs()) {
144136
// tx has public zerocoin spend inputs
145137
if(static_cast<int>(tx.vin.size()) > consensus.ZC_MaxPublicSpendsPerTx)
146-
return state.DoS(10, error("CheckTransaction() : Zerocoin Spend has more than allowed txin's"), REJECT_INVALID, "bad-zerocoinspend");
138+
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-max-inputs");
147139
} else {
148140
// tx has regular zerocoin spend inputs
149141
if(static_cast<int>(tx.vin.size()) > consensus.ZC_MaxSpendsPerTx)
150-
return state.DoS(10, error("CheckTransaction() : Zerocoin Spend has more than allowed txin's"), REJECT_INVALID, "bad-zerocoinspend");
142+
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-max-inputs");
151143
}
152144

153145
} else {
154146
for (const CTxIn& txin : tx.vin)
155147
if (txin.prevout.IsNull() && (fZerocoinActive && !txin.IsZerocoinSpend()))
156-
return state.DoS(10, error("CheckTransaction() : prevout is null"),
157-
REJECT_INVALID, "bad-txns-prevout-null");
148+
return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null");
158149
}
159150

160151
return true;

‎src/main.cpp‎

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -976,17 +976,15 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
976976
int chainHeight = chainActive.Height();
977977
bool fColdStakingActive = sporkManager.IsSporkActive(SPORK_17_COLDSTAKING_ENFORCEMENT);
978978
if (!CheckTransaction(tx, chainHeight >= consensus.height_start_ZC, true, state, isBlockBetweenFakeSerialAttackRange(chainHeight), fColdStakingActive))
979-
return state.DoS(100, error("%s : CheckTransaction failed", __func__), REJECT_INVALID, "bad-tx");
979+
return false;
980980

981981
// Coinbase is only valid in a block, not as a loose transaction
982982
if (tx.IsCoinBase())
983-
return state.DoS(100, error("%s : coinbase as individual tx",
984-
__func__), REJECT_INVALID, "coinbase");
983+
return state.DoS(100, false, REJECT_INVALID, "coinbase");
985984

986985
//Coinstake is also only valid in a block, not as a loose transaction
987986
if (tx.IsCoinStake())
988-
return state.DoS(100, error("%s : coinstake as individual tx (id=%s): %s",
989-
__func__, tx.GetHash().GetHex(), tx.ToString()), REJECT_INVALID, "coinstake");
987+
return state.DoS(100, false, REJECT_INVALID, "coinstake");
990988

991989
// Only accept nLockTime-using transactions that can be mined in the next
992990
// block; we don't want our mempool filled up with transactions that can't
@@ -997,8 +995,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
997995
// Rather not work on nonstandard transactions (unless regtest)
998996
std::string reason;
999997
if (!Params().IsRegTestNet() && !IsStandardTx(tx, reason))
1000-
return state.DoS(0, error("%s : nonstandard transaction: %s",
1001-
__func__, reason), REJECT_NONSTANDARD, reason);
998+
return state.DoS(0, false, REJECT_NONSTANDARD, reason);
1002999
// is it already in the memory pool?
10031000
uint256 hash = tx.GetHash();
10041001
if (pool.exists(hash)) {
@@ -1010,9 +1007,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
10101007
for (const CTxIn& in : tx.vin) {
10111008
if (mapLockedInputs.count(in.prevout)) {
10121009
if (mapLockedInputs[in.prevout] != tx.GetHash()) {
1013-
return state.DoS(0,
1014-
error("%s : conflicts with existing transaction lock: %s",
1015-
__func__, reason), REJECT_INVALID, "tx-lock-conflict");
1010+
return state.DoS(0, false, REJECT_INVALID, "tx-lock-conflict");
10161011
}
10171012
}
10181013
}
@@ -1043,29 +1038,26 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
10431038
//Check that txid is not already in the chain
10441039
int nHeightTx = 0;
10451040
if (IsTransactionInChain(tx.GetHash(), nHeightTx))
1046-
return state.Invalid(error("%s : zPIV spend tx %s already in block %d",
1047-
__func__, tx.GetHash().GetHex(), nHeightTx), REJECT_DUPLICATE, "bad-txns-inputs-spent");
1041+
return state.Invalid(error("%s : zPIV spend tx %s already in block %d", __func__, tx.GetHash().GetHex(), nHeightTx),
1042+
REJECT_DUPLICATE, "bad-txns-inputs-spent");
10481043

10491044
//Check for double spending of serial #'s
10501045
for (const CTxIn& txIn : tx.vin) {
10511046
// Only allow for public zc spends inputs
10521047
if (!txIn.IsZerocoinPublicSpend())
1053-
return state.Invalid(error("%s: failed for tx %s, every input must be a zcpublicspend",
1054-
__func__, tx.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-zpiv");
1048+
return state.Invalid(false, REJECT_INVALID, "bad-zc-spend-notpublic");
10551049

10561050
libzerocoin::ZerocoinParams* params = consensus.Zerocoin_Params(false);
10571051
PublicCoinSpend publicSpend(params);
10581052
if (!ZPIVModule::ParseZerocoinPublicSpend(txIn, tx, state, publicSpend)){
10591053
return false;
10601054
}
10611055
if (!ContextualCheckZerocoinSpend(tx, &publicSpend, chainHeight, UINT256_ZERO))
1062-
return state.Invalid(error("%s: ContextualCheckZerocoinSpend failed for tx %s",
1063-
__func__, tx.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-zpiv");
1056+
return state.Invalid(false, REJECT_INVALID, "bad-zc-spend-contextcheck");
10641057

10651058
// Check that the version matches the one enforced with SPORK_18
10661059
if (!CheckPublicCoinSpendVersion(publicSpend.getVersion())) {
1067-
return state.Invalid(error("%s : Public Zerocoin spend version %d not accepted. must be version %d.",
1068-
__func__, publicSpend.getVersion(), CurrentPublicCoinSpendVersion()), REJECT_INVALID, "bad-txns-invalid-zpiv");
1060+
return state.Invalid(false, REJECT_INVALID, "bad-zc-spend-version");
10691061
}
10701062

10711063
}
@@ -1089,21 +1081,18 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
10891081
}
10901082

10911083
//Check for invalid/fraudulent inputs
1092-
if (!ValidOutPoint(txin.prevout, chainHeight)) {
1093-
return state.Invalid(error("%s : tried to spend invalid input %s in tx %s",
1094-
__func__, txin.prevout.ToString(), tx.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-inputs");
1095-
}
1084+
if (!ValidOutPoint(txin.prevout, chainHeight))
1085+
return state.Invalid(false, REJECT_INVALID, "bad-txns-invalid-inputs");
10961086
}
10971087

10981088
// Reject legacy zPIV mints
10991089
if (!Params().IsRegTestNet() && tx.HasZerocoinMintOutputs())
11001090
return state.Invalid(error("%s : tried to include zPIV mint output in tx %s",
1101-
__func__, tx.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-outputs");
1091+
__func__, tx.GetHash().GetHex()), REJECT_INVALID, "bad-zc-spend-mint");
11021092

11031093
// are the actual inputs available?
11041094
if (!view.HaveInputs(tx))
1105-
return state.Invalid(error("%s : inputs already spent",
1106-
__func__), REJECT_DUPLICATE, "bad-txns-inputs-spent");
1095+
return state.Invalid(false, REJECT_DUPLICATE, "bad-txns-inputs-spent");
11071096

11081097
// Bring the best block into scope
11091098
view.GetBestBlock();
@@ -1128,8 +1117,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
11281117
unsigned int nMaxSigOps = MAX_TX_SIGOPS_CURRENT;
11291118
nSigOps += GetP2SHSigOpCount(tx, view);
11301119
if(nSigOps > nMaxSigOps)
1131-
return state.DoS(0, error("%s : too many sigops %s, %d > %d",
1132-
__func__, hash.ToString(), nSigOps, nMaxSigOps), REJECT_NONSTANDARD, "bad-txns-too-many-sigops");
1120+
return state.DoS(0, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", false,
1121+
strprintf("%d > %d", nSigOps, nMaxSigOps));
11331122
}
11341123

11351124
CAmount nValueOut = tx.GetValueOut();
@@ -1145,8 +1134,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
11451134
if (!ignoreFees) {
11461135
CAmount txMinFee = GetMinRelayFee(tx, pool, nSize, true);
11471136
if (fLimitFree && nFees < txMinFee && !hasZcSpendInputs)
1148-
return state.DoS(0, error("%s : not enough fees %s, %d < %d",
1149-
__func__, hash.ToString(), nFees, txMinFee), REJECT_INSUFFICIENTFEE, "insufficient fee");
1137+
return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient fee", false,
1138+
strprintf("%d < %d", nFees, txMinFee));
11501139

11511140
// Require that free transactions have sufficient priority to be mined in the next block.
11521141
if (!hasZcSpendInputs && GetBoolArg("-relaypriority", true) && nFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(view.GetPriority(tx, chainHeight + 1))) {
@@ -1170,16 +1159,16 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
11701159
// -limitfreerelay unit is thousand-bytes-per-minute
11711160
// At default rate it would take over a month to fill 1GB
11721161
if (dFreeCount >= GetArg("-limitfreerelay", 30) * 10 * 1000)
1173-
return state.DoS(0, error("%s : free transaction rejected by rate limiter",
1174-
__func__), REJECT_INSUFFICIENTFEE, "rate limited free transaction");
1162+
return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "rate limited free transaction");
11751163
LogPrint(BCLog::MEMPOOL, "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount + nSize);
11761164
dFreeCount += nSize;
11771165
}
11781166
}
11791167

11801168
if (fRejectInsaneFee && nFees > ::minRelayTxFee.GetFee(nSize) * 10000)
1181-
return state.Invalid(error("%s : absurdly high fees %s, %d > %d", __func__, hash.ToString(),
1182-
nFees, ::minRelayTxFee.GetFee(nSize) * 10000), REJECT_HIGHFEE, "absurdly-high-fee");
1169+
return state.Invalid(false,
1170+
REJECT_HIGHFEE, "absurdly-high-fee",
1171+
strprintf("%d > %d", nFees, ::minRelayTxFee.GetFee(nSize) * 10000));
11831172

11841173
// As zero fee transactions are not going to be accepted in the near future (4.0) and the code will be fully refactored soon.
11851174
// This is just a quick inline towards that goal, the mempool by default will not accept them. Blocking
@@ -1208,7 +1197,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
12081197
if (fCLTVIsActivated)
12091198
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
12101199
if (!CheckInputs(tx, state, view, true, flags, true)) {
1211-
return error("%s : ConnectInputs failed %s", __func__, hash.ToString());
1200+
return false;
12121201
}
12131202

12141203
// Check again against just the consensus-critical mandatory script
@@ -1224,8 +1213,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
12241213
if (fCLTVIsActivated)
12251214
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
12261215
if (!CheckInputs(tx, state, view, true, flags, true)) {
1227-
return error("%s : BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s",
1228-
__func__, hash.ToString());
1216+
return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s",
1217+
__func__, hash.ToString(), FormatStateMessage(state));
12291218
}
12301219

12311220
// Store transaction in memory
@@ -1849,10 +1838,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txund
18491838
bool CScriptCheck::operator()()
18501839
{
18511840
const CScript& scriptSig = ptxTo->vin[nIn].scriptSig;
1852-
if (!VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, cacheStore), &error)) {
1853-
return ::error("CScriptCheck(): %s:%d VerifySignature failed: %s", ptxTo->GetHash().ToString(), nIn, ScriptErrorString(error));
1854-
}
1855-
return true;
1841+
return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, cacheStore), &error);
18561842
}
18571843

18581844
std::map<COutPoint, COutPoint> mapInvalidOutPoints;
@@ -1935,7 +1921,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
19351921
// This doesn't trigger the DoS code on purpose; if it did, it would make it easier
19361922
// for an attacker to attempt to split the network.
19371923
if (!inputs.HaveInputs(tx))
1938-
return state.Invalid(error("CheckInputs() : %s inputs unavailable", tx.GetHash().ToString()));
1924+
return state.Invalid(false, 0, "", "Inputs unavailable");
19391925

19401926
const Consensus::Params& consensus = ::Params().GetConsensus();
19411927
CAmount nValueIn = 0;
@@ -1948,35 +1934,28 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
19481934
// If prev is coinbase, check that it's matured
19491935
if (coins->IsCoinBase() || coins->IsCoinStake()) {
19501936
if (nSpendHeight - coins->nHeight < consensus.nCoinbaseMaturity)
1951-
return state.Invalid(
1952-
error("CheckInputs() : tried to spend coinbase at depth %d, coinstake=%d",
1953-
nSpendHeight - coins->nHeight, coins->IsCoinStake()),
1954-
REJECT_INVALID, "bad-txns-premature-spend-of-coinbase");
1937+
return state.Invalid(false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase-coinstake",
1938+
strprintf("tried to spend coinbase/coinstake at depth %d", nSpendHeight - coins->nHeight));
19551939
}
19561940

19571941
// Check for negative or overflow input values
19581942
nValueIn += coins->vout[prevout.n].nValue;
19591943
if (!consensus.MoneyRange(coins->vout[prevout.n].nValue) || !consensus.MoneyRange(nValueIn))
1960-
return state.DoS(100, error("CheckInputs() : txin values out of range"),
1961-
REJECT_INVALID, "bad-txns-inputvalues-outofrange");
1944+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange");
19621945
}
19631946

19641947
if (!tx.IsCoinStake()) {
19651948
if (nValueIn < tx.GetValueOut())
1966-
return state.DoS(100, error("CheckInputs() : %s value in (%s) < value out (%s)",
1967-
tx.GetHash().ToString(), FormatMoney(nValueIn),
1968-
FormatMoney(tx.GetValueOut())),
1969-
REJECT_INVALID, "bad-txns-in-belowout");
1949+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false,
1950+
strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(tx.GetValueOut())));
19701951

19711952
// Tally transaction fees
19721953
CAmount nTxFee = nValueIn - tx.GetValueOut();
19731954
if (nTxFee < 0)
1974-
return state.DoS(100, error("CheckInputs() : %s nTxFee < 0", tx.GetHash().ToString()),
1975-
REJECT_INVALID, "bad-txns-fee-negative");
1955+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-negative");
19761956
nFees += nTxFee;
19771957
if (!consensus.MoneyRange(nFees))
1978-
return state.DoS(100, error("CheckInputs() : nFees out of range"),
1979-
REJECT_INVALID, "bad-txns-fee-outofrange");
1958+
return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange");
19801959
}
19811960
return true;
19821961
}

0 commit comments

Comments
 (0)