Skip to content

Commit 54a7ef6

Browse files
author
MarcoFalke
committed
Merge #17399: validation: Templatize ValidationState instead of subclassing
10efc04 Templatize ValidationState instead of subclassing (Jeffrey Czyz) 10e85d4 Remove ValidationState's constructor (Jeffrey Czyz) 0aed17e Refactor FormatStateMessage into ValidationState (Jeffrey Czyz) Pull request description: This removes boilerplate code in the subclasses which otherwise only differ by the result type. The subclassing was introduced in a27a295. ACKs for top commit: MarcoFalke: ACK 10efc04 🐱 ajtowns: ACK 10efc04 -- looks good to me jonatack: ACK 10efc04 code review, build/tests green, nice cleanup Tree-SHA512: 765dd52dde7d49b9a5c6d99d97c96f4492673e2aed0b0604faa88db0308fa4500a26bf755cca0b896be283874096c215932e1110a2d01dc012cd36a5fce58a42
2 parents 715dbbe + 10efc04 commit 54a7ef6

15 files changed

+58
-125
lines changed

‎src/Makefile.am

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,6 @@ BITCOIN_CORE_H = \
229229
util/time.h \
230230
util/translation.h \
231231
util/url.h \
232-
util/validation.h \
233232
util/vector.h \
234233
validation.h \
235234
validationinterface.h \
@@ -528,7 +527,6 @@ libbitcoin_util_a_SOURCES = \
528527
util/string.cpp \
529528
util/time.cpp \
530529
util/url.cpp \
531-
util/validation.cpp \
532530
$(BITCOIN_CORE_H)
533531

534532
if GLIBC_BACK_COMPAT

‎src/consensus/validation.h

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* provider of the transaction should be banned/ignored/disconnected/etc.
1717
*/
1818
enum class TxValidationResult {
19-
TX_RESULT_UNSET, //!< initial value. Tx has not yet been rejected
19+
TX_RESULT_UNSET = 0, //!< initial value. Tx has not yet been rejected
2020
TX_CONSENSUS, //!< invalid by consensus rules
2121
/**
2222
* Invalid by a change to consensus rules more recent than SegWit.
@@ -50,7 +50,7 @@ enum class TxValidationResult {
5050
* useful for some other use-cases.
5151
*/
5252
enum class BlockValidationResult {
53-
BLOCK_RESULT_UNSET, //!< initial value. Block has not yet been rejected
53+
BLOCK_RESULT_UNSET = 0, //!< initial value. Block has not yet been rejected
5454
BLOCK_CONSENSUS, //!< invalid by consensus rules (excluding any below reasons)
5555
/**
5656
* Invalid by a change to consensus rules more recent than SegWit.
@@ -71,31 +71,31 @@ enum class BlockValidationResult {
7171

7272

7373

74-
/** Base class for capturing information about block/transaction validation. This is subclassed
74+
/** Template for capturing information about block/transaction validation. This is instantiated
7575
* by TxValidationState and BlockValidationState for validation information on transactions
7676
* and blocks respectively. */
77+
template <typename Result>
7778
class ValidationState {
7879
private:
7980
enum mode_state {
8081
MODE_VALID, //!< everything ok
8182
MODE_INVALID, //!< network rule violation (DoS value may be set)
8283
MODE_ERROR, //!< run-time error
83-
} m_mode;
84+
} m_mode{MODE_VALID};
85+
Result m_result{};
8486
std::string m_reject_reason;
8587
std::string m_debug_message;
86-
protected:
87-
void Invalid(const std::string &reject_reason="",
88+
public:
89+
bool Invalid(Result result,
90+
const std::string &reject_reason="",
8891
const std::string &debug_message="")
8992
{
93+
m_result = result;
9094
m_reject_reason = reject_reason;
9195
m_debug_message = debug_message;
9296
if (m_mode != MODE_ERROR) m_mode = MODE_INVALID;
97+
return false;
9398
}
94-
public:
95-
// ValidationState is abstract. Have a pure virtual destructor.
96-
virtual ~ValidationState() = 0;
97-
98-
ValidationState() : m_mode(MODE_VALID) {}
9999
bool Error(const std::string& reject_reason)
100100
{
101101
if (m_mode == MODE_VALID)
@@ -106,40 +106,25 @@ class ValidationState {
106106
bool IsValid() const { return m_mode == MODE_VALID; }
107107
bool IsInvalid() const { return m_mode == MODE_INVALID; }
108108
bool IsError() const { return m_mode == MODE_ERROR; }
109+
Result GetResult() const { return m_result; }
109110
std::string GetRejectReason() const { return m_reject_reason; }
110111
std::string GetDebugMessage() const { return m_debug_message; }
111-
};
112+
std::string ToString() const
113+
{
114+
if (IsValid()) {
115+
return "Valid";
116+
}
112117

113-
inline ValidationState::~ValidationState() {};
118+
if (!m_debug_message.empty()) {
119+
return m_reject_reason + ", " + m_debug_message;
120+
}
114121

115-
class TxValidationState : public ValidationState {
116-
private:
117-
TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
118-
public:
119-
bool Invalid(TxValidationResult result,
120-
const std::string &reject_reason="",
121-
const std::string &debug_message="")
122-
{
123-
m_result = result;
124-
ValidationState::Invalid(reject_reason, debug_message);
125-
return false;
122+
return m_reject_reason;
126123
}
127-
TxValidationResult GetResult() const { return m_result; }
128124
};
129125

130-
class BlockValidationState : public ValidationState {
131-
private:
132-
BlockValidationResult m_result = BlockValidationResult::BLOCK_RESULT_UNSET;
133-
public:
134-
bool Invalid(BlockValidationResult result,
135-
const std::string &reject_reason="",
136-
const std::string &debug_message="") {
137-
m_result = result;
138-
ValidationState::Invalid(reject_reason, debug_message);
139-
return false;
140-
}
141-
BlockValidationResult GetResult() const { return m_result; }
142-
};
126+
class TxValidationState : public ValidationState<TxValidationResult> {};
127+
class BlockValidationState : public ValidationState<BlockValidationResult> {};
143128

144129
// These implement the weight = (stripped_size * 4) + witness_size formula,
145130
// using only serialization with and without witness data. As witness_size

‎src/init.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
#include <util/system.h>
5252
#include <util/threadnames.h>
5353
#include <util/translation.h>
54-
#include <util/validation.h>
5554
#include <util/asmap.h>
5655
#include <validation.h>
5756
#include <hash.h>
@@ -710,7 +709,7 @@ static void ThreadImport(std::vector<fs::path> vImportFiles)
710709
// scan for better chains in the block chain database, that are not yet connected in the active best chain
711710
BlockValidationState state;
712711
if (!ActivateBestChain(state, chainparams)) {
713-
LogPrintf("Failed to connect best block (%s)\n", FormatStateMessage(state));
712+
LogPrintf("Failed to connect best block (%s)\n", state.ToString());
714713
StartShutdown();
715714
return;
716715
}

‎src/miner.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include <timedata.h>
2121
#include <util/moneystr.h>
2222
#include <util/system.h>
23-
#include <util/validation.h>
2423

2524
#include <algorithm>
2625
#include <utility>
@@ -167,7 +166,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
167166

168167
BlockValidationState state;
169168
if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) {
170-
throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state)));
169+
throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString()));
171170
}
172171
int64_t nTime2 = GetTimeMicros();
173172

‎src/net_processing.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include <txmempool.h>
2727
#include <util/system.h>
2828
#include <util/strencodings.h>
29-
#include <util/validation.h>
3029

3130
#include <memory>
3231
#include <typeinfo>
@@ -1432,7 +1431,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
14321431
if (need_activate_chain) {
14331432
BlockValidationState state;
14341433
if (!ActivateBestChain(state, Params(), a_recent_block)) {
1435-
LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state));
1434+
LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString());
14361435
}
14371436
}
14381437

@@ -2342,7 +2341,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
23422341
}
23432342
BlockValidationState state;
23442343
if (!ActivateBestChain(state, Params(), a_recent_block)) {
2345-
LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state));
2344+
LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString());
23462345
}
23472346
}
23482347

@@ -2636,7 +2635,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
26362635
{
26372636
LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(),
26382637
pfrom->GetId(),
2639-
FormatStateMessage(state));
2638+
state.ToString());
26402639
MaybePunishNodeForTx(pfrom->GetId(), state);
26412640
}
26422641
return true;

‎src/node/transaction.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
#include <net.h>
88
#include <net_processing.h>
99
#include <node/context.h>
10-
#include <util/validation.h>
1110
#include <validation.h>
1211
#include <validationinterface.h>
1312
#include <node/transaction.h>
@@ -41,7 +40,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
4140
TxValidationState state;
4241
if (!AcceptToMemoryPool(*node.mempool, state, std::move(tx),
4342
nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) {
44-
err_string = FormatStateMessage(state);
43+
err_string = state.ToString();
4544
if (state.IsInvalid()) {
4645
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
4746
return TransactionError::MISSING_INPUTS;

‎src/rpc/blockchain.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#include <undo.h>
3232
#include <util/strencodings.h>
3333
#include <util/system.h>
34-
#include <util/validation.h>
3534
#include <validation.h>
3635
#include <validationinterface.h>
3736
#include <warnings.h>
@@ -1486,7 +1485,7 @@ static UniValue preciousblock(const JSONRPCRequest& request)
14861485
PreciousBlock(state, Params(), pblockindex);
14871486

14881487
if (!state.IsValid()) {
1489-
throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state));
1488+
throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
14901489
}
14911490

14921491
return NullUniValue;
@@ -1524,7 +1523,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request)
15241523
}
15251524

15261525
if (!state.IsValid()) {
1527-
throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state));
1526+
throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
15281527
}
15291528

15301529
return NullUniValue;
@@ -1561,7 +1560,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request)
15611560
ActivateBestChain(state, Params());
15621561

15631562
if (!state.IsValid()) {
1564-
throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state));
1563+
throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString());
15651564
}
15661565

15671566
return NullUniValue;

‎src/rpc/mining.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include <util/fees.h>
2929
#include <util/strencodings.h>
3030
#include <util/system.h>
31-
#include <util/validation.h>
3231
#include <validation.h>
3332
#include <validationinterface.h>
3433
#include <versionbitsinfo.h>
@@ -307,7 +306,7 @@ static UniValue BIP22ValidationResult(const BlockValidationState& state)
307306
return NullUniValue;
308307

309308
if (state.IsError())
310-
throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state));
309+
throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString());
311310
if (state.IsInvalid())
312311
{
313312
std::string strRejectReason = state.GetRejectReason();
@@ -823,7 +822,7 @@ static UniValue submitheader(const JSONRPCRequest& request)
823822
ProcessNewBlockHeaders({h}, state, Params());
824823
if (state.IsValid()) return NullUniValue;
825824
if (state.IsError()) {
826-
throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state));
825+
throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString());
827826
}
828827
throw JSONRPCError(RPC_VERIFY_ERROR, state.GetRejectReason());
829828
}

‎src/test/util/setup_common.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
#include <util/strencodings.h>
2626
#include <util/time.h>
2727
#include <util/translation.h>
28-
#include <util/validation.h>
2928
#include <validation.h>
3029
#include <validationinterface.h>
3130

@@ -123,7 +122,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
123122

124123
BlockValidationState state;
125124
if (!ActivateBestChain(state, chainparams)) {
126-
throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", FormatStateMessage(state)));
125+
throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString()));
127126
}
128127

129128
// Start script-checking threads. Set g_parallel_script_checks to true so they are used.

‎src/util/validation.cpp

Lines changed: 0 additions & 23 deletions
This file was deleted.

0 commit comments

Comments
 (0)