Skip to content

Commit

Permalink
validation: Remove RECENT_CONSENSUS_CHANGE validation result
Browse files Browse the repository at this point in the history
The *_RECENT_CONSENSUS_CHANGE variants in the validation result
enumerations were always unused. They seem to have been kept around
speculatively for a soft fork after segwit, however they were never used
for taproot either. This points at them not having a clear purpose.
Based on the original pull requests' comments their usage was never
entirely clear:
bitcoin#11639 (comment)
bitcoin#15141 (comment)

Since they are part of the validation interface and need to exposed by
the kernel library keeping them around may also be confusing to future
users of the library.
  • Loading branch information
TheCharlatan committed Nov 11, 2024
1 parent 018e5fc commit e80e4c6
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 32 deletions.
3 changes: 0 additions & 3 deletions src/bitcoin-chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,6 @@ int main(int argc, char* argv[])
case BlockValidationResult::BLOCK_CONSENSUS:
std::cerr << "invalid by consensus rules (excluding any below reasons)" << std::endl;
break;
case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE:
std::cerr << "Invalid by a change to consensus rules more recent than SegWit." << std::endl;
break;
case BlockValidationResult::BLOCK_CACHED_INVALID:
std::cerr << "this block was cached as being invalid and we didn't store the reason why" << std::endl;
break;
Expand Down
16 changes: 0 additions & 16 deletions src/consensus/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@ static constexpr size_t MINIMUM_WITNESS_COMMITMENT{38};
enum class TxValidationResult {
TX_RESULT_UNSET = 0, //!< initial value. Tx has not yet been rejected
TX_CONSENSUS, //!< invalid by consensus rules
/**
* Invalid by a change to consensus rules more recent than SegWit.
* Currently unused as there are no such consensus rule changes, and any download
* sources realistically need to support SegWit in order to provide useful data,
* so differentiating between always-invalid and invalid-by-pre-SegWit-soft-fork
* is uninteresting.
*/
TX_RECENT_CONSENSUS_CHANGE,
TX_INPUTS_NOT_STANDARD, //!< inputs (covered by txid) failed policy rules
TX_NOT_STANDARD, //!< otherwise didn't meet our local policy rules
TX_MISSING_INPUTS, //!< transaction was missing some of its inputs
Expand Down Expand Up @@ -65,14 +57,6 @@ enum class TxValidationResult {
enum class BlockValidationResult {
BLOCK_RESULT_UNSET = 0, //!< initial value. Block has not yet been rejected
BLOCK_CONSENSUS, //!< invalid by consensus rules (excluding any below reasons)
/**
* Invalid by a change to consensus rules more recent than SegWit.
* Currently unused as there are no such consensus rule changes, and any download
* sources realistically need to support SegWit in order to provide useful data,
* so differentiating between always-invalid and invalid-by-pre-SegWit-soft-fork
* is uninteresting.
*/
BLOCK_RECENT_CONSENSUS_CHANGE,
BLOCK_CACHED_INVALID, //!< this block was cached as being invalid and we didn't store the reason why
BLOCK_INVALID_HEADER, //!< invalid proof of work or time too old
BLOCK_MUTATED, //!< the block's data didn't match the data committed to by the PoW
Expand Down
2 changes: 0 additions & 2 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,6 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati
case BlockValidationResult::BLOCK_MISSING_PREV:
if (peer) Misbehaving(*peer, message);
return;
case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE:
case BlockValidationResult::BLOCK_TIME_FUTURE:
break;
}
Expand All @@ -1810,7 +1809,6 @@ void PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat
if (peer) Misbehaving(*peer, "");
return;
// Conflicting (but not necessarily invalid) data or different policy:
case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE:
case TxValidationResult::TX_INPUTS_NOT_STANDARD:
case TxValidationResult::TX_NOT_STANDARD:
case TxValidationResult::TX_MISSING_INPUTS:
Expand Down
1 change: 0 additions & 1 deletion src/test/fuzz/partially_downloaded_block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb)
fuzzed_data_provider.PickValueInArray(
{BlockValidationResult::BLOCK_RESULT_UNSET,
BlockValidationResult::BLOCK_CONSENSUS,
BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE,
BlockValidationResult::BLOCK_CACHED_INVALID,
BlockValidationResult::BLOCK_INVALID_HEADER,
BlockValidationResult::BLOCK_MUTATED,
Expand Down
1 change: 0 additions & 1 deletion src/test/fuzz/txdownloadman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ COutPoint COINS[NUM_COINS];
static TxValidationResult TESTED_TX_RESULTS[] = {
// Skip TX_RESULT_UNSET
TxValidationResult::TX_CONSENSUS,
TxValidationResult::TX_RECENT_CONSENSUS_CHANGE,
TxValidationResult::TX_INPUTS_NOT_STANDARD,
TxValidationResult::TX_NOT_STANDARD,
TxValidationResult::TX_MISSING_INPUTS,
Expand Down
1 change: 0 additions & 1 deletion src/test/txdownload_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ struct Behaviors {
// Txid and Wtxid are assumed to be different here. For a nonsegwit transaction, use the wtxid results.
static std::map<TxValidationResult, Behaviors> expected_behaviors{
{TxValidationResult::TX_CONSENSUS, {/*txid_rejects*/0,/*wtxid_rejects*/1,/*txid_recon*/0,/*wtxid_recon*/0,/*keep*/1,/*txid_inv*/0,/*wtxid_inv*/1}},
{TxValidationResult::TX_RECENT_CONSENSUS_CHANGE, { 0, 1, 0, 0, 1, 0, 1}},
{TxValidationResult::TX_INPUTS_NOT_STANDARD, { 1, 1, 0, 0, 1, 1, 1}},
{TxValidationResult::TX_NOT_STANDARD, { 0, 1, 0, 0, 1, 0, 1}},
{TxValidationResult::TX_MISSING_INPUTS, { 0, 0, 0, 0, 1, 0, 1}},
Expand Down
10 changes: 2 additions & 8 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2201,15 +2201,9 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
// string by reporting the error from the second check.
error = check2.GetScriptError();
}

// MANDATORY flag failures correspond to
// TxValidationResult::TX_CONSENSUS. Because CONSENSUS
// failures are the most serious case of validation
// failures, we may need to consider using
// RECENT_CONSENSUS_CHANGE for any script failure that
// could be due to non-upgraded nodes which we may want to
// support, to avoid splitting the network (but this
// depends on the details of how net_processing handles
// such errors).
// TxValidationResult::TX_CONSENSUS.
return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(error)));
}
}
Expand Down

0 comments on commit e80e4c6

Please sign in to comment.