From a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Mon, 30 Sep 2019 16:25:04 -0400
Subject: [PATCH 1/5] [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(-)
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index b6839dcf215..8d5cccc7ff1 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<uint256, std::pair<NodeId, bool>>::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
From 0053e16714323c1694c834fdca74f064a1a33529 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Thu, 10 Oct 2019 11:19:42 -0400
Subject: [PATCH 2/5] [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 ++--
test/functional/feature_bip68_sequence.py | 2 +-
test/functional/feature_cltv.py | 2 +-
test/functional/feature_dersig.py | 2 +-
test/functional/feature_nulldummy.py | 2 +-
test/functional/feature_segwit.py | 8 ++---
test/functional/mempool_accept.py | 36 +++++++++++------------
test/functional/p2p_dos_header_tree.py | 2 +-
test/functional/rpc_rawtransaction.py | 4 +--
10 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index f548d356cf9..13ff2a455f5 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 fe1f5a277e8..9a0d889447f 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";
diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py
index 677362756c9..dd495ba41e8 100755
--- a/test/functional/feature_bip68_sequence.py
+++ b/test/functional/feature_bip68_sequence.py
@@ -24,7 +24,7 @@ SEQUENCE_LOCKTIME_GRANULARITY = 9 # this is a bit-shift
SEQUENCE_LOCKTIME_MASK = 0x0000ffff
# RPC error for non-BIP68 final transactions
-NOT_FINAL_ERROR = "non-BIP68-final (code 64)"
+NOT_FINAL_ERROR = "non-BIP68-final"
class BIP68Test(BitcoinTestFramework):
def set_test_params(self):
diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py
index 13f1c9216a5..7d131e60453 100755
--- a/test/functional/feature_cltv.py
+++ b/test/functional/feature_cltv.py
@@ -126,7 +126,7 @@ class BIP65Test(BitcoinTestFramework):
# First we show that this tx is valid except for CLTV by getting it
# rejected from the mempool for exactly that reason.
assert_equal(
- [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Negative locktime)'}],
+ [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': 'non-mandatory-script-verify-flag (Negative locktime)'}],
self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0)
)
diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py
index 92f6eea5b20..2ace96fef40 100755
--- a/test/functional/feature_dersig.py
+++ b/test/functional/feature_dersig.py
@@ -110,7 +110,7 @@ class BIP66Test(BitcoinTestFramework):
# First we show that this tx is valid except for DERSIG by getting it
# rejected from the mempool for exactly that reason.
assert_equal(
- [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': '64: non-mandatory-script-verify-flag (Non-canonical DER signature)'}],
+ [{'txid': spendtx.hash, 'allowed': False, 'reject-reason': 'non-mandatory-script-verify-flag (Non-canonical DER signature)'}],
self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0)
)
diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py
index 250dee1528e..aaf56a42d00 100755
--- a/test/functional/feature_nulldummy.py
+++ b/test/functional/feature_nulldummy.py
@@ -20,7 +20,7 @@ from test_framework.script import CScript
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error
-NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero) (code 64)"
+NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)"
def trueDummy(tx):
scriptSig = CScript(tx.vin[0].scriptSig)
diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py
index c69c7f90e83..82c7e55245c 100755
--- a/test/functional/feature_segwit.py
+++ b/test/functional/feature_segwit.py
@@ -193,10 +193,10 @@ class SegWitTest(BitcoinTestFramework):
assert self.nodes[0].getrawtransaction(tx_id, False, blockhash) == tx.serialize_without_witness().hex()
self.log.info("Verify witness txs without witness data are invalid after the fork")
- self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)', wit_ids[NODE_2][WIT_V0][2], sign=False)
- self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)', wit_ids[NODE_2][WIT_V1][2], sign=False)
- self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)', p2sh_ids[NODE_2][WIT_V0][2], sign=False, redeem_script=witness_script(False, self.pubkey[2]))
- self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)', p2sh_ids[NODE_2][WIT_V1][2], sign=False, redeem_script=witness_script(True, self.pubkey[2]))
+ self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch)', wit_ids[NODE_2][WIT_V0][2], sign=False)
+ self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness)', wit_ids[NODE_2][WIT_V1][2], sign=False)
+ self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch)', p2sh_ids[NODE_2][WIT_V0][2], sign=False, redeem_script=witness_script(False, self.pubkey[2]))
+ self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness)', p2sh_ids[NODE_2][WIT_V1][2], sign=False, redeem_script=witness_script(True, self.pubkey[2]))
self.log.info("Verify default node can now use witness txs")
self.success_mine(self.nodes[0], wit_ids[NODE_0][WIT_V0][0], True) # block 432
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index dee7a04516a..c466b120f28 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -71,7 +71,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
node.generate(1)
self.mempool_size = 0
self.check_mempool_result(
- result_expected=[{'txid': txid_in_block, 'allowed': False, 'reject-reason': '18: txn-already-known'}],
+ result_expected=[{'txid': txid_in_block, 'allowed': False, 'reject-reason': 'txn-already-known'}],
rawtxs=[raw_tx_in_block],
)
@@ -109,7 +109,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
node.sendrawtransaction(hexstring=raw_tx_0)
self.mempool_size += 1
self.check_mempool_result(
- result_expected=[{'txid': txid_0, 'allowed': False, 'reject-reason': '18: txn-already-in-mempool'}],
+ result_expected=[{'txid': txid_0, 'allowed': False, 'reject-reason': 'txn-already-in-mempool'}],
rawtxs=[raw_tx_0],
)
@@ -133,7 +133,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.vout[0].nValue -= int(4 * fee * COIN) # Set more fee
# skip re-signing the tx
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '18: txn-mempool-conflict'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'txn-mempool-conflict'}],
rawtxs=[tx.serialize().hex()],
maxfeerate=0,
)
@@ -192,7 +192,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
# Skip re-signing the transaction for context independent checks from now on
# tx.deserialize(BytesIO(hex_str_to_bytes(node.signrawtransactionwithwallet(tx.serialize().hex())['hex'])))
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-vout-empty'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-vout-empty'}],
rawtxs=[tx.serialize().hex()],
)
@@ -200,7 +200,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vin = [tx.vin[0]] * math.ceil(MAX_BLOCK_BASE_SIZE / len(tx.vin[0].serialize()))
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-oversize'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-oversize'}],
rawtxs=[tx.serialize().hex()],
)
@@ -208,7 +208,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vout[0].nValue *= -1
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-vout-negative'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-vout-negative'}],
rawtxs=[tx.serialize().hex()],
)
@@ -217,7 +217,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vout[0].nValue = 21000000 * COIN + 1
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-vout-toolarge'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-vout-toolarge'}],
rawtxs=[tx.serialize().hex()],
)
@@ -226,7 +226,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.vout = [tx.vout[0]] * 2
tx.vout[0].nValue = 21000000 * COIN
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-txouttotal-toolarge'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-txouttotal-toolarge'}],
rawtxs=[tx.serialize().hex()],
)
@@ -234,7 +234,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vin = [tx.vin[0]] * 2
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: bad-txns-inputs-duplicate'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-inputs-duplicate'}],
rawtxs=[tx.serialize().hex()],
)
@@ -243,7 +243,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
raw_tx_coinbase_spent = node.getrawtransaction(txid=node.decoderawtransaction(hexstring=raw_tx_in_block)['vin'][0]['txid'])
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_coinbase_spent)))
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '16: coinbase'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'coinbase'}],
rawtxs=[tx.serialize().hex()],
)
@@ -251,19 +251,19 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.nVersion = 3 # A version currently non-standard
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: version'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'version'}],
rawtxs=[tx.serialize().hex()],
)
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vout[0].scriptPubKey = CScript([OP_0]) # Some non-standard script
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: scriptpubkey'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'scriptpubkey'}],
rawtxs=[tx.serialize().hex()],
)
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vin[0].scriptSig = CScript([OP_HASH160]) # Some not-pushonly scriptSig
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: scriptsig-not-pushonly'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'scriptsig-not-pushonly'}],
rawtxs=[tx.serialize().hex()],
)
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
@@ -271,21 +271,21 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
num_scripts = 100000 // len(output_p2sh_burn.serialize()) # Use enough outputs to make the tx too large for our policy
tx.vout = [output_p2sh_burn] * num_scripts
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: tx-size'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-size'}],
rawtxs=[tx.serialize().hex()],
)
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vout[0] = output_p2sh_burn
tx.vout[0].nValue -= 1 # Make output smaller, such that it is dust for our policy
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: dust'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'dust'}],
rawtxs=[tx.serialize().hex()],
)
tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference)))
tx.vout[0].scriptPubKey = CScript([OP_RETURN, b'\xff'])
tx.vout = [tx.vout[0]] * 2
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: multi-op-return'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'multi-op-return'}],
rawtxs=[tx.serialize().hex()],
)
@@ -294,7 +294,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.vin[0].nSequence -= 1 # Should be non-max, so locktime is not ignored
tx.nLockTime = node.getblockcount() + 1
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: non-final'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'non-final'}],
rawtxs=[tx.serialize().hex()],
)
@@ -303,7 +303,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
tx.vin[0].nSequence = 2 # We could include it in the second block mined from now, but not the very next one
# Can skip re-signing the tx because of early rejection
self.check_mempool_result(
- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': '64: non-BIP68-final'}],
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'non-BIP68-final'}],
rawtxs=[tx.serialize().hex()],
maxfeerate=0,
)
diff --git a/test/functional/p2p_dos_header_tree.py b/test/functional/p2p_dos_header_tree.py
index 7d386c47f6e..6676b84e542 100755
--- a/test/functional/p2p_dos_header_tree.py
+++ b/test/functional/p2p_dos_header_tree.py
@@ -57,7 +57,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
} in self.nodes[0].getchaintips()
self.log.info("Feed all fork headers (fails due to checkpoint)")
- with self.nodes[0].assert_debug_log(['bad-fork-prior-to-checkpoint (code 67)']):
+ with self.nodes[0].assert_debug_log(['bad-fork-prior-to-checkpoint']):
self.nodes[0].p2p.send_message(msg_headers(self.headers_fork))
self.nodes[0].p2p.wait_for_disconnect()
diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index ca0d47a5f89..74fea07350e 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -454,7 +454,7 @@ class RawTransactionsTest(BitcoinTestFramework):
# Thus, testmempoolaccept should reject
testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0]
assert_equal(testres['allowed'], False)
- assert_equal(testres['reject-reason'], '256: absurdly-high-fee')
+ assert_equal(testres['reject-reason'], 'absurdly-high-fee')
# and sendrawtransaction should throw
assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
# and the following calls should both succeed
@@ -478,7 +478,7 @@ class RawTransactionsTest(BitcoinTestFramework):
# Thus, testmempoolaccept should reject
testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0]
assert_equal(testres['allowed'], False)
- assert_equal(testres['reject-reason'], '256: absurdly-high-fee')
+ assert_equal(testres['reject-reason'], 'absurdly-high-fee')
# and sendrawtransaction should throw
assert_raises_rpc_error(-26, "absurdly-high-fee", self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
# and the following calls should both succeed
From e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Mon, 30 Sep 2019 16:25:04 -0400
Subject: [PATCH 3/5] [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(-)
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index 00ebbbd1ab7..12060358397 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 4b93cae8484..ceeddc3f6d0 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 2e23f4b3a4f..4920cdf8811 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -12,20 +12,8 @@
#include <primitives/transaction.h>
#include <primitives/block.h>
-/** "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 726f251c5aa..3d9bf550900 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 fbbe3757e0d..fe81ef83e2c 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);
From 04a2f326ec0f06fb4fce1c4f93500752f05dede8 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Thu, 10 Oct 2019 11:28:22 -0400
Subject: [PATCH 4/5] [validation] Fix REJECT message comments
---
src/net_processing.cpp | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 8d5cccc7ff1..1425f091c59 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;
From 9075d13153ce06cd59a45644831ecc43126e1e82 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Tue, 8 Oct 2019 16:24:30 -0400
Subject: [PATCH 5/5] [docs] Add release notes for removal of REJECT reasons
---
doc/release-notes-15437.md | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/doc/release-notes-15437.md b/doc/release-notes-15437.md
index 031e90ccd24..66142077575 100644
--- a/doc/release-notes-15437.md
+++ b/doc/release-notes-15437.md
@@ -32,3 +32,22 @@ Please use the recommended alternatives if you rely on this deprecated feature:
could wait until the transaction has confirmed (taking into account the fee
target they set (compare the RPC `estimatesmartfee`)) or listen for the
transaction announcement by other network peers to check for propagation.
+
+The removal of BIP61 REJECT message support also has the following minor RPC
+and logging implications:
+
+* `testmempoolaccept` and `sendrawtransaction` no longer return the P2P REJECT
+ code when a transaction is not accepted to the mempool. They still return the
+ verbal reject reason.
+
+* Log messages that previously reported the REJECT code when a transaction was
+ not accepted to the mempool now no longer report the REJECT code. The reason
+ for rejection is still reported.
+
+Updated RPCs
+------------
+
+- `testmempoolaccept` and `sendrawtransaction` no longer return the P2P REJECT
+ code when a transaction is not accepted to the mempool. See the Section
+ _Removal of reject network messages from Bitcoin Core (BIP61)_ for details on
+ the removal of BIP61 REJECT message support.