From a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 30 Sep 2019 16:25:04 -0400 Subject: [validation] Fix peer punishment for bad blocks Because the call to MaybePunishNode() in PeerLogicValidation::BlockChecked() only previously happened if the REJECT code was > 0 and < REJECT_INTERNAL, then there are cases were MaybePunishNode() can get called where it wasn't previously: - when AcceptBlockHeader() fails with CACHED_INVALID. - when AcceptBlockHeader() fails with BLOCK_MISSING_PREV. Note that BlockChecked() cannot fail with an 'internal' reject code. The only internal reject code was REJECT_HIGHFEE, which was only set in ATMP. This change restores the behaviour pre-commit 5d08c9c579ba8cc7b684105c6a08263992b08d52 which did punish nodes that sent us CACHED_INVALID and BLOCK_MISSING_PREV blocks. --- src/net_processing.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b6839dcf21..8d5cccc7ff 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1234,11 +1234,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta const uint256 hash(block.GetHash()); std::map>::iterator it = mapBlockSource.find(hash); - if (state.IsInvalid()) { - // Don't send reject message with code 0 or an internal reject code. - if (it != mapBlockSource.end() && State(it->second.first) && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { + // If the block failed validation, we know where it came from and we're still connected + // to that peer, maybe punish. + if (state.IsInvalid() && + it != mapBlockSource.end() && + State(it->second.first)) { MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); - } } // Check that: // 1. The block is valid -- cgit v1.2.3 From 0053e16714323c1694c834fdca74f064a1a33529 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 10 Oct 2019 11:19:42 -0400 Subject: [logging] Don't log REJECT code when transaction is rejected Remove the BIP61 REJECT code from error messages and logs when a transaction is rejected. BIP61 support was removed from Bitcoin Core in fa25f43ac5692082dba3f90456c501eb08f1b75c. The REJECT codes will be removed from the codebase entirely in the following commit. --- src/rpc/rawtransaction.cpp | 2 +- src/util/validation.cpp | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index f548d356cf..13ff2a455f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -906,7 +906,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) result_0.pushKV("allowed", test_accept_res); if (!test_accept_res) { if (state.IsInvalid()) { - result_0.pushKV("reject-reason", strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); + result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason())); } else if (missing_inputs) { result_0.pushKV("reject-reason", "missing-inputs"); } else { diff --git a/src/util/validation.cpp b/src/util/validation.cpp index fe1f5a277e..9a0d889447 100644 --- a/src/util/validation.cpp +++ b/src/util/validation.cpp @@ -11,10 +11,9 @@ /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state) { - return strprintf("%s%s (code %i)", + return strprintf("%s%s", state.GetRejectReason(), - state.GetDebugMessage().empty() ? "" : ", "+state.GetDebugMessage(), - state.GetRejectCode()); + state.GetDebugMessage().empty() ? "" : ", "+state.GetDebugMessage()); } const std::string strMessageMagic = "Bitcoin Signed Message:\n"; -- cgit v1.2.3 From e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 30 Sep 2019 16:25:04 -0400 Subject: [validation] Remove REJECT code from CValidationState We no longer send BIP 61 REJECT messages, so there's no need to set a REJECT code in the CValidationState object. --- src/consensus/tx_check.cpp | 18 +++---- src/consensus/tx_verify.cpp | 10 ++-- src/consensus/validation.h | 19 +------- src/validation.cpp | 113 ++++++++++++++++++++++---------------------- src/validation.h | 8 ---- 5 files changed, 72 insertions(+), 96 deletions(-) (limited to 'src') diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 00ebbbd1ab..1206035839 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -11,24 +11,24 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe { // Basic checks that don't depend on any context if (tx.vin.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vin-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vin-empty"); if (tx.vout.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-empty"); // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability) if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-oversize"); // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { if (txout.nValue < 0) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-negative"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-negative"); if (txout.nValue > MAX_MONEY) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-toolarge"); nValueOut += txout.nValue; if (!MoneyRange(nValueOut)) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge"); } // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock @@ -37,20 +37,20 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate"); } } if (tx.IsCoinBase()) { if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-length"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-length"); } else { for (const auto& txin : tx.vin) if (txin.prevout.IsNull()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-prevout-null"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-prevout-null"); } return true; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 4b93cae848..ceeddc3f6d 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -160,7 +160,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c { // are the actual inputs available? if (!inputs.HaveInputs(tx)) { - return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", + return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } @@ -172,27 +172,27 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // If prev is coinbase, check that it's matured if (coin.IsCoinBase() && nSpendHeight - coin.nHeight < COINBASE_MATURITY) { - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "bad-txns-premature-spend-of-coinbase", strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } // Check for negative or overflow input values nValueIn += coin.out.nValue; if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputvalues-outofrange"); } } const CAmount value_out = tx.GetValueOut(); if (nValueIn < value_out) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-in-belowout", + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-in-belowout", strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(value_out))); } // Tally transaction fees const CAmount txfee_aux = nValueIn - value_out; if (!MoneyRange(txfee_aux)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-fee-outofrange"); } txfee = txfee_aux; diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 2e23f4b3a4..4920cdf881 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -12,20 +12,8 @@ #include #include -/** "reject" message codes */ -static const unsigned char REJECT_MALFORMED = 0x01; -static const unsigned char REJECT_INVALID = 0x10; -static const unsigned char REJECT_OBSOLETE = 0x11; -static const unsigned char REJECT_DUPLICATE = 0x12; -static const unsigned char REJECT_NONSTANDARD = 0x40; -// static const unsigned char REJECT_DUST = 0x41; // part of BIP 61 -static const unsigned char REJECT_INSUFFICIENTFEE = 0x42; -static const unsigned char REJECT_CHECKPOINT = 0x43; - /** A "reason" why something was invalid, suitable for determining whether the * provider of the object should be banned/ignored/disconnected/etc. - * These are much more granular than the rejection codes, which may be more - * useful for some other use-cases. */ enum class ValidationInvalidReason { // txn and blocks: @@ -104,15 +92,13 @@ private: } mode; ValidationInvalidReason m_reason; std::string strRejectReason; - unsigned int chRejectCode; std::string strDebugMessage; public: - CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE), chRejectCode(0) {} + CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE) {} bool Invalid(ValidationInvalidReason reasonIn, bool ret = false, - unsigned int chRejectCodeIn=0, const std::string &strRejectReasonIn="", + const std::string &strRejectReasonIn="", const std::string &strDebugMessageIn="") { m_reason = reasonIn; - chRejectCode = chRejectCodeIn; strRejectReason = strRejectReasonIn; strDebugMessage = strDebugMessageIn; if (mode == MODE_ERROR) @@ -136,7 +122,6 @@ public: return mode == MODE_ERROR; } ValidationInvalidReason GetReason() const { return m_reason; } - unsigned int GetRejectCode() const { return chRejectCode; } std::string GetRejectReason() const { return strRejectReason; } std::string GetDebugMessage() const { return strDebugMessage; } }; diff --git a/src/validation.cpp b/src/validation.cpp index 726f251c5a..3d9bf55090 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -508,11 +508,11 @@ private: { CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size); if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } if (package_fee < ::minRelayTxFee.GetFee(package_size)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); } return true; } @@ -565,29 +565,29 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Coinbase is only valid in a block, not as a loose transaction if (tx.IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "coinbase"); // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; if (fRequireStandard && !IsStandardTx(tx, reason)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, reason); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, reason); // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "tx-size-small"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "tx-size-small"); // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-final"); // is it already in the memory pool? if (m_pool.exists(hash)) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions @@ -619,7 +619,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } if (fReplacementOptOut) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_DUPLICATE, "txn-mempool-conflict"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "txn-mempool-conflict"); } setConflicts.insert(ptxConflicting->GetHash()); @@ -645,7 +645,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-known"); } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet @@ -670,7 +670,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-BIP68-final"); CAmount nFees = 0; if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { @@ -679,11 +679,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, m_view)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "bad-txns-nonstandard-inputs"); // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) - return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, "bad-witness-nonstandard"); int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -707,7 +707,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) unsigned int nSize = entry->GetTxSize(); if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); // No transactions are allowed below minRelayTxFee except from disconnected @@ -716,8 +716,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (nAbsurdFee && nFees > nAbsurdFee) return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, - REJECT_HIGHFEE, "absurdly-high-fee", - strprintf("%d > %d", nFees, nAbsurdFee)); + "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); // Calculate in-mempool ancestors, up to a limit. @@ -774,7 +773,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too-long-mempool-chain", errString); } } @@ -787,7 +786,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); if (setConflicts.count(hashAncestor)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-spends-conflicting-tx", strprintf("%s spends conflicting transaction %s", hash.ToString(), hashAncestor.ToString())); @@ -827,7 +826,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); if (newFeeRate <= oldFeeRate) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), newFeeRate.ToString(), @@ -855,7 +854,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) nConflictingSize += it->GetTxSize(); } } else { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too many potential replacements", strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", hash.ToString(), nConflictingCount, @@ -879,7 +878,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // it's cheaper to just check if the new input refers to a // tx that's in the mempool. if (m_pool.exists(tx.vin[j].prevout.hash)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "replacement-adds-unconfirmed", strprintf("replacement %s adds unconfirmed input, idx %d", hash.ToString(), j)); } @@ -891,7 +890,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // transactions would not be paid for. if (nModifiedFees < nConflictingFees) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } @@ -901,7 +900,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CAmount nDeltaFees = nModifiedFees - nConflictingFees; if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), @@ -930,7 +929,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute !CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { // Only the witness is missing, so the transaction itself may be fine. state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + state.GetRejectReason(), state.GetDebugMessage()); } assert(IsTransactionReason(state.GetReason())); return false; // state filled in by CheckInputs @@ -1013,7 +1012,7 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) if (!bypass_limits) { LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); if (!m_pool.exists(hash)) - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "mempool full"); } return true; } @@ -1548,7 +1547,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi CScriptCheck check2(coin.out, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata); if (check2()) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } // MANDATORY flag failures correspond to // ValidationInvalidReason::CONSENSUS. Because CONSENSUS @@ -1559,7 +1558,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // support, to avoid splitting the network (but this // depends on the details of how net_processing handles // such errors). - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); } } @@ -2062,7 +2061,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), - REJECT_INVALID, "bad-txns-BIP30"); + "bad-txns-BIP30"); } } } @@ -2107,14 +2106,14 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // defined for a block, so we reset the reason flag to // CONSENSUS here. state.Invalid(ValidationInvalidReason::CONSENSUS, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + state.GetRejectReason(), state.GetDebugMessage()); } return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } nFees += txfee; if (!MoneyRange(nFees)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), - REJECT_INVALID, "bad-txns-accumulated-fee-outofrange"); + "bad-txns-accumulated-fee-outofrange"); } // Check that transaction is BIP68 final @@ -2127,7 +2126,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), - REJECT_INVALID, "bad-txns-nonfinal"); + "bad-txns-nonfinal"); } } @@ -2138,7 +2137,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl nSigOpsCost += GetTransactionSigOpCost(tx, view, flags); if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): too many sigops"), - REJECT_INVALID, "bad-blk-sigops"); + "bad-blk-sigops"); txdata.emplace_back(tx); if (!tx.IsCoinBase()) @@ -2154,7 +2153,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // consider whether rewriting to CONSENSUS or // RECENT_CONSENSUS_CHANGE would be more appropriate. state.Invalid(ValidationInvalidReason::CONSENSUS, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + state.GetRejectReason(), state.GetDebugMessage()); } return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); @@ -2176,10 +2175,10 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)", block.vtx[0]->GetValueOut(), blockReward), - REJECT_INVALID, "bad-cb-amount"); + "bad-cb-amount"); if (!control.Wait()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: CheckQueue failed", __func__), REJECT_INVALID, "block-validation-failed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: CheckQueue failed", __func__), "block-validation-failed"); int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal); @@ -3256,7 +3255,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, { // Check proof of work matches claimed amount if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "high-hash", "proof of work failed"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "high-hash", "proof of work failed"); return true; } @@ -3278,13 +3277,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-txnmrklroot", "hashMerkleRoot mismatch"); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txnmrklroot", "hashMerkleRoot mismatch"); // Check for merkle tree malleability (CVE-2012-2459): repeating sequences // of transactions in a block without affecting the merkle root of a block, // while still invalidating it. if (mutated) - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-txns-duplicate", "duplicate transaction"); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); } // All potential-corruption validation must be done before we do any @@ -3295,20 +3294,20 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P // Size limits if (block.vtx.empty() || block.vtx.size() * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT || ::GetSerializeSize(block, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-length", "size limits failed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-length", "size limits failed"); // First transaction must be coinbase, the rest must not be if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-missing", "first tx is not coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-missing", "first tx is not coinbase"); for (unsigned int i = 1; i < block.vtx.size(); i++) if (block.vtx[i]->IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-multiple", "more than one coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); // Check transactions // Must check for duplicate inputs (see CVE-2018-17144) for (const auto& tx : block.vtx) if (!CheckTransaction(*tx, state, true)) - return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), + return state.Invalid(state.GetReason(), false, state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); unsigned int nSigOps = 0; @@ -3317,7 +3316,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-sigops", "out-of-bounds SigOpCount"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-sigops", "out-of-bounds SigOpCount"); if (fCheckPOW && fCheckMerkleRoot) block.fChecked = true; @@ -3418,7 +3417,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // Check proof of work const Consensus::Params& consensusParams = params.GetConsensus(); if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "bad-diffbits", "incorrect proof of work"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "bad-diffbits", "incorrect proof of work"); // Check against checkpoints if (fCheckpointsEnabled) { @@ -3427,23 +3426,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // g_blockman.m_block_index. CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), "bad-fork-prior-to-checkpoint"); } // Check timestamp against prev if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "time-too-old", "block's timestamp is too early"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "time-too-old", "block's timestamp is too early"); // Check timestamp if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future"); + return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, "time-too-new", "block timestamp too far in the future"); // Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded: // check for version 2, 3 and 4 upgrades if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) || (block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) || (block.nVersion < 4 && nHeight >= consensusParams.BIP65Height)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion), + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); return true; @@ -3473,7 +3472,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // Check that all transactions are finalized for (const auto& tx : block.vtx) { if (!IsFinalTx(*tx, nHeight, nLockTimeCutoff)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-nonfinal", "non-final transaction"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-nonfinal", "non-final transaction"); } } @@ -3483,7 +3482,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c CScript expect = CScript() << nHeight; if (block.vtx[0]->vin[0].scriptSig.size() < expect.size() || !std::equal(expect.begin(), expect.end(), block.vtx[0]->vin[0].scriptSig.begin())) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-height", "block height mismatch in coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-height", "block height mismatch in coinbase"); } } @@ -3505,11 +3504,11 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // already does not permit it, it is impossible to trigger in the // witness tree. if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } fHaveWitness = true; } @@ -3519,7 +3518,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c if (!fHaveWitness) { for (const auto& tx : block.vtx) { if (tx->HasWitness()) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); } } } @@ -3531,7 +3530,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // the block hash, so we couldn't mark the block as permanently // failed). if (GetBlockWeight(block) > MAX_BLOCK_WEIGHT) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); } return true; @@ -3551,7 +3550,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(ValidationInvalidReason::CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), 0, "duplicate"); + return state.Invalid(ValidationInvalidReason::CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), "duplicate"); return true; } @@ -3562,10 +3561,10 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState CBlockIndex* pindexPrev = nullptr; BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); if (mi == m_block_index.end()) - return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), 0, "prev-blk-not-found"); + return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), "prev-blk-not-found"); pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3602,7 +3601,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState setDirtyBlockIndex.insert(invalid_walk); invalid_walk = invalid_walk->pprev; } - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); } } } diff --git a/src/validation.h b/src/validation.h index fbbe3757e0..fe81ef83e2 100644 --- a/src/validation.h +++ b/src/validation.h @@ -779,14 +779,6 @@ extern VersionBitsCache versionbitscache; */ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params); -/** Reject codes greater or equal to this can be returned by AcceptToMemPool - * for transactions, to signal internal conditions. They cannot and should not - * be sent over the P2P network. - */ -static const unsigned int REJECT_INTERNAL = 0x100; -/** Too high fee. Can not be triggered by P2P transactions */ -static const unsigned int REJECT_HIGHFEE = 0x100; - /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); -- cgit v1.2.3 From 04a2f326ec0f06fb4fce1c4f93500752f05dede8 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 10 Oct 2019 11:28:22 -0400 Subject: [validation] Fix REJECT message comments --- src/net_processing.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8d5cccc7ff..1425f091c5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -117,8 +117,8 @@ namespace { int nSyncStarted GUARDED_BY(cs_main) = 0; /** - * Sources of received blocks, saved to be able to send them reject - * messages or ban them when processing happens afterwards. + * Sources of received blocks, saved to be able punish them when processing + * happens afterwards. * Set mapBlockSource[hash].second to false if the node should not be * punished if the block is invalid. */ @@ -2861,11 +2861,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // been run). This is handled below, so just treat this as // though the block was successfully read, and rely on the // handling in ProcessNewBlock to ensure the block index is - // updated, reject messages go out, etc. + // updated, etc. MarkBlockAsReceived(resp.blockhash); // it is now an empty pointer fBlockRead = true; - // mapBlockSource is only used for sending reject messages and DoS scores, - // so the race between here and cs_main in ProcessNewBlock is fine. + // mapBlockSource is used for potentially punishing peers and + // updating which peers send us compact blocks, so the race + // between here and cs_main in ProcessNewBlock is fine. // BIP 152 permits peers to relay compact blocks after validating // the header only; we should not punish peers if the block turns // out to be invalid. @@ -2937,8 +2938,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Also always process if we requested the block explicitly, as we may // need it even though it is not a candidate for a new best tip. forceProcessing |= MarkBlockAsReceived(hash); - // mapBlockSource is only used for sending reject messages and DoS scores, - // so the race between here and cs_main in ProcessNewBlock is fine. + // mapBlockSource is only used for punishing peers and setting + // which peers send us compact blocks, so the race between here and + // cs_main in ProcessNewBlock is fine. mapBlockSource.emplace(hash, std::make_pair(pfrom->GetId(), true)); } bool fNewBlock = false; -- cgit v1.2.3