diff options
author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-07-22 19:28:09 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-07-22 19:48:55 +0200 |
commit | 1397afc5ec032366c0edadc46b8d55af3e18d095 (patch) | |
tree | c4a7416cae5c3a33c6e3e0dbe93401b0409609ee /src | |
parent | 93decbc7a486939aeed6205fd45fffddd8a62e58 (diff) | |
parent | fa56eda58e5ec2f2345bbe14c798e83f2abb4728 (diff) | |
download | bitcoin-1397afc5ec032366c0edadc46b8d55af3e18d095.tar.xz |
Merge #19526: log: Avoid treating remote misbehvior as local system error
fa56eda58e5ec2f2345bbe14c798e83f2abb4728 log: Avoid treating remote misbehvior as local system error (MarcoFalke)
fa492895b572a1196ca8652006b6fc2fa1d16951 refactor: Switch ValidationState mode to C++11 enum class (MarcoFalke)
Pull request description:
When logging failures of `CheckBlockHeader` (high-hash), they are always logged as system error. This is problematic for several reasons:
* Submitting a blockheader that fails `CheckBlockHeader` over RPC will result in a debug log line that starts with `ERROR`. Proper behaviour should be to log not anything and instead only return the failure reason to the RPC user. This pull does not fix this issue entirely, but is a good first step in the right direction.
* A misbehaving peer that sends us an invalid block header that fails `CheckBlockHeader` will result in a debug log line that starts with `ERROR`. Proper behavior should be to log the remote peer misbehavior if logging for that category was enabled. This pull fixes this issue for `CheckBlockHeader` and other functions can be adjusted as well if needed in follow-ups. This should be a good first step in the right direction.
ACKs for top commit:
practicalswift:
re-ACK fa56eda58e5ec2f2345bbe14c798e83f2abb4728
Tree-SHA512: 9793191f5cb57bdff7c93926e94877e8ca2ef89dcebcf9eb155899c733961839ec7c3f9b9f001dc082ada4234fe6e75f6df431301678d6822325840771166d77
Diffstat (limited to 'src')
-rw-r--r-- | src/consensus/validation.h | 30 | ||||
-rw-r--r-- | src/validation.cpp | 6 |
2 files changed, 20 insertions, 16 deletions
diff --git a/src/consensus/validation.h b/src/consensus/validation.h index a79e7b9d12..74d6138041 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -75,37 +75,39 @@ enum class BlockValidationResult { * by TxValidationState and BlockValidationState for validation information on transactions * and blocks respectively. */ template <typename Result> -class ValidationState { +class ValidationState +{ private: - enum mode_state { - MODE_VALID, //!< everything ok - MODE_INVALID, //!< network rule violation (DoS value may be set) - MODE_ERROR, //!< run-time error - } m_mode{MODE_VALID}; + enum class ModeState { + M_VALID, //!< everything ok + M_INVALID, //!< network rule violation (DoS value may be set) + M_ERROR, //!< run-time error + } m_mode{ModeState::M_VALID}; Result m_result{}; std::string m_reject_reason; std::string m_debug_message; + public: bool Invalid(Result result, - const std::string &reject_reason="", - const std::string &debug_message="") + const std::string& reject_reason = "", + const std::string& debug_message = "") { m_result = result; m_reject_reason = reject_reason; m_debug_message = debug_message; - if (m_mode != MODE_ERROR) m_mode = MODE_INVALID; + if (m_mode != ModeState::M_ERROR) m_mode = ModeState::M_INVALID; return false; } bool Error(const std::string& reject_reason) { - if (m_mode == MODE_VALID) + if (m_mode == ModeState::M_VALID) m_reject_reason = reject_reason; - m_mode = MODE_ERROR; + m_mode = ModeState::M_ERROR; return false; } - bool IsValid() const { return m_mode == MODE_VALID; } - bool IsInvalid() const { return m_mode == MODE_INVALID; } - bool IsError() const { return m_mode == MODE_ERROR; } + bool IsValid() const { return m_mode == ModeState::M_VALID; } + bool IsInvalid() const { return m_mode == ModeState::M_INVALID; } + bool IsError() const { return m_mode == ModeState::M_ERROR; } Result GetResult() const { return m_result; } std::string GetRejectReason() const { return m_reject_reason; } std::string GetDebugMessage() const { return m_debug_message; } diff --git a/src/validation.cpp b/src/validation.cpp index 4fe02ed244..a9cb99aafb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3641,8 +3641,10 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS return true; } - if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) - return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__, hash.ToString(), state.ToString()); + if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) { + LogPrint(BCLog::VALIDATION, "%s: Consensus::CheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); + return false; + } // Get prev block index CBlockIndex* pindexPrev = nullptr; |