Blockchain: improve check_block_timestamp() overloads
What the developer wrote
Authored by jeffro256
Blockchain: improve check_block_timestamp() overloads
What changed, and why it matters
This commit refactors how Monero checks whether a new block's timestamp is valid. It splits one timestamp-check function into two: a general helper and one specifically for the main chain. The change also moves the 'future time' check (rejecting blocks stamped more than 2 hours ahead of local time) and the 'not enough blocks' check into the general helper, and it makes the median timestamp output optional. The commit appears to be a code-quality and consistency improvement rather than a clear-cut security fix, but it does tighten behavior when there are fewer than 60 prior blocks and removes a const qualifier from one function. Without a vendor statement, we cannot say it fixes a known vulnerability.
Treat this as a routine refactor of consensus code. Review the new helper to confirm the future-time and short-chain checks behave identically for all callers, and verify that removing `const` does not introduce thread-safety or state issues. No emergency action is indicated without additional vendor or researcher disclosure.
Security signals we found
Refactoring of consensus-critical timestamp validation code
Future-time limit check relocated into shared helper
Short-chain timestamp bypass relocated into shared helper
Removal of `const` from static helper may indicate internal state or static data usage
Optional median output changes caller contract
No explicit security or bug-fix language in commit message
Evidence from the diff
The patch renames check_block_timestamp(const block&, uint64_t&) to check_block_timestamp_main_chain and makes it delegate to a new static check_block_timestamp(std::vector<uint64_t>&, const block&, uint64_t*). The future-time check (b.timestamp > time(NULL) + CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT) and the short-chain bypass (timestamps.size() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) are moved from the main-chain overload into the static helper. The median timestamp output becomes an optional pointer, callers are updated, and the static helper is no longer const. The main-chain overload now builds a timestamp window even when the chain height is below BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW (using offset 0), leaving the bypass to the helper. The header comments are updated accordingly.
Changed components
src/cryptonote_core/blockchain.cppsrc/cryptonote_core/blockchain.hBlockchain::check_block_timestamp_main_chainBlockchain::check_block_timestamp (static helper)Blockchain::create_block_templateBlockchain::handle_block_to_main_chainInspect captured patch +37 / −28
diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index c2a119b..65347f6 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -1650,8 +1650,8 @@ bool Blockchain::create_block_template(block& b, const crypto::hash *from_block,
}
b.timestamp = time(NULL);
- uint64_t median_ts;
- if (!check_block_timestamp(b, median_ts))
+ uint64_t median_ts{};
+ if (!check_block_timestamp_main_chain(b, &median_ts))
{
b.timestamp = median_ts;
}
@@ -3791,11 +3791,29 @@ uint64_t Blockchain::get_adjusted_time(uint64_t height) const
return (adjusted_current_block_ts < median_ts ? adjusted_current_block_ts : median_ts);
}
//------------------------------------------------------------------
-//TODO: revisit, has changed a bit on upstream
-bool Blockchain::check_block_timestamp(std::vector<uint64_t>& timestamps, const block& b, uint64_t& median_ts) const
+bool Blockchain::check_block_timestamp(std::vector<uint64_t>& timestamps, const block& b, uint64_t* median_ts_out)
{
+ if (median_ts_out)
+ *median_ts_out = 0;
+
LOG_PRINT_L3("Blockchain::" << __func__);
- median_ts = epee::misc_utils::median(timestamps);
+
+ if(b.timestamp > (uint64_t)time(NULL) + CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT)
+ {
+ MERROR_VER("Timestamp of block with id: " << get_block_hash(b) << ", "
+ << b.timestamp << ", bigger than local time + 2 hours");
+ return false;
+ }
+
+ // if not enough blocks, no proper median yet, return true
+ if(timestamps.size() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW)
+ {
+ return true;
+ }
+
+ const uint64_t median_ts = epee::misc_utils::median(timestamps);
+ if (median_ts_out)
+ *median_ts_out = median_ts;
if(b.timestamp < median_ts)
{
@@ -3813,34 +3831,24 @@ bool Blockchain::check_block_timestamp(std::vector<uint64_t>& timestamps, const
// true if the block's timestamp is not less than the timestamp of the
// median of the selected blocks
// false otherwise
-bool Blockchain::check_block_timestamp(const block& b, uint64_t& median_ts) const
+bool Blockchain::check_block_timestamp_main_chain(const block& b, uint64_t* median_ts_out) const
{
LOG_PRINT_L3("Blockchain::" << __func__);
- if(b.timestamp > (uint64_t)time(NULL) + CRYPTONOTE_BLOCK_FUTURE_TIME_LIMIT)
- {
- MERROR_VER("Timestamp of block with id: " << get_block_hash(b) << ", " << b.timestamp << ", bigger than local time + 2 hours");
- return false;
- }
const auto h = m_db->height();
- // if not enough blocks, no proper median yet, return true
- if(h < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW)
- {
- return true;
- }
-
std::vector<uint64_t> timestamps;
// need most recent 60 blocks, get index of first of those
- size_t offset = h - BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW;
+ size_t offset = (h >= BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) ? (h - BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) : 0;
+ assert(offset <= h);
timestamps.reserve(h - offset);
for(;offset < h; ++offset)
{
timestamps.push_back(m_db->get_block_timestamp(offset));
}
- return check_block_timestamp(timestamps, b, median_ts);
+ return check_block_timestamp(timestamps, b, median_ts_out);
}
//------------------------------------------------------------------
bool Blockchain::flush_txes_from_pool(const std::vector<crypto::hash> &txids)
@@ -3919,7 +3927,7 @@ leave:
// make sure block timestamp is not less than the median timestamp
// of a set number of the most recent blocks.
- if(!check_block_timestamp(bl))
+ if(!check_block_timestamp_main_chain(bl))
{
MERROR_VER("Block with id: " << id << std::endl << "has invalid timestamp: " << bl.timestamp);
bvc.m_verifivation_failed = true;
diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h
index 79c1c26..53d3098 100644
--- a/src/cryptonote_core/blockchain.h
+++ b/src/cryptonote_core/blockchain.h
@@ -1552,7 +1552,7 @@ namespace cryptonote
bool add_block_as_invalid(const block_extended_info& bei, const crypto::hash& h);
/**
- * @brief checks a block's timestamp
+ * @brief checks a block's timestamp on top of the main chain
*
* This function grabs the timestamps from the most recent <n> blocks,
* where n = BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW. If there are not those many
@@ -1563,26 +1563,27 @@ namespace cryptonote
* false otherwise
*
* @param b the block to be checked
- * @param median_ts return-by-reference the median of timestamps
+ * @param[out] median_ts_out the median of timestamps (optional)
*
* @return true if the block's timestamp is valid, otherwise false
*/
- bool check_block_timestamp(const block& b, uint64_t& median_ts) const;
- bool check_block_timestamp(const block& b) const { uint64_t median_ts; return check_block_timestamp(b, median_ts); }
+ bool check_block_timestamp_main_chain(const block& b, uint64_t* median_ts_out = nullptr) const;
/**
* @brief checks a block's timestamp
*
* If the block is not more recent than the median of the recent
- * timestamps passed here, it is considered invalid.
+ * timestamps passed here, it is considered invalid. If the block is too
+ * recent, according to the local system clock, it is considered invalid.
*
- * @param timestamps a list of the most recent timestamps to check against
+ * @param[inout] timestamps a list of the most recent timestamps to check against
* @param b the block to be checked
+ * @param[out] median_ts_out the median of `timestamps` (optional)
*
* @return true if the block's timestamp is valid, otherwise false
*/
- bool check_block_timestamp(std::vector<uint64_t>& timestamps, const block& b, uint64_t& median_ts) const;
- bool check_block_timestamp(std::vector<uint64_t>& timestamps, const block& b) const { uint64_t median_ts; return check_block_timestamp(timestamps, b, median_ts); }
+ static bool check_block_timestamp(std::vector<uint64_t>& timestamps, const block& b,
+ uint64_t* median_ts_out = nullptr);
/**
* @brief finish an alternate chain's timestamp window from the main chain
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.