doc, chainparams: 29775 release notes and follow-ups #30604
Conversation
This chainwork was observed at height 38487.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
fjahr
changed the title
fjahr
force-pushed
the
2024-08-t4-follow-up
branch
from
d9e486a to
f7e2ed3
Compare
|
Removed reference to a specific version in the deprecation warning and addressed @Sjors comment by using the |
fjahr
force-pushed
the
2024-08-t4-follow-up
branch
from
f7e2ed3 to
35ad61a
Compare
Fixed |
src/validation.cpp
Outdated
| @@ -4188,7 +4188,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio | |||
| // Check timestamp for the first block of each difficulty adjustment | |||
| // interval, except the genesis block. | |||
| if (nHeight % consensusParams.DifficultyAdjustmentInterval() == 0) { | |||
| if (block.GetBlockTime() < pindexPrev->GetBlockTime() - 60 * 60 * 2) { | |||
| if (block.GetBlockTime() < pindexPrev->GetBlockTime() - MAX_FUTURE_BLOCK_TIME) { | |||
There was a problem hiding this comment.
35ad61a: let's define a separate constant and make it clear these are different but related:
/**
* Maximum number of seconds that the timestamp of the first
* block of a difficulty adjustment period is allowed to
* be earlier than the last block of the previous period (BIP94).
*/
static constexpr int64_t MAX_TIMEWARP{MAX_FUTURE_BLOCK_TIME};
/**
* If the timestamp of the last block in a difficulty period is set
* MAX_FUTURE_BLOCK_TIME seconds in the future, an honest miner should
* be able to mine the first block using the current time. This is not
* a consensus rule, but changing MAX_TIMEWARP should be done with caution.
*/
static_assert(MAX_FUTURE_BLOCK_TIME <= MAX_TIMEWARP);(I'm less sure about having the static_assert and its documentation in our codebase)
See ongoing discussion in bitcoin/bips#1658
There was a problem hiding this comment.
And see #30614, but that should probably be its own followup.
There was a problem hiding this comment.
let's define a separate constant
done
There was a problem hiding this comment.
And see #30614, but that should probably be its own followup.
Agree, it seems that we currently have the safer option given we don't really know what other mining software is doing (at least that's my feeling). If we do still get consensus on tightening the limit we can do that in another follow-up in the next two weeks but it shouldn't hold up this PR here.
| @@ -334,7 +334,7 @@ class CTestNet4Params : public CChainParams { | |||
| consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; | |||
| consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay | |||
|
|
|||
| consensus.nMinimumChainWork = uint256{}; | |||
| consensus.nMinimumChainWork = uint256{"000000000000000000000000000000000000000000000056faca98a0cd9bdf5f"}; | |||
The value is equal to MAX_FUTURE_BLOCK_TIME.
fjahr
force-pushed
the
2024-08-t4-follow-up
branch
from
35ad61a to
92c1d7d
Compare
|
Addressed @Sjors feedback and introduced a separate constant |
|
ACK 92c1d7d |
|
ACK 92c1d7d |
99eeb51 [doc] mention bip94 support (glozow) Pull request description: Followup to #29775, noticed while looking at #30604 and #30647. See [release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-every-major-and-minor-release). ACKs for top commit: maflcko: ACK 99eeb51 fjahr: ACK 99eeb51 tdb3: ACK 99eeb51 Tree-SHA512: 95838d3ace7e5d7b1a2481f2d7bd82902081713e6e89dbf21e0dad16d1cf5295e0c1cfda1f03af72304a5844743d24769f5fa04d4dc9f02f36462ef0ae82a552
This completes follow-ups left open in #29775.
MAX_TIMEWARPconstant as the timewarp defense delta, equal toMAX_FUTURE_BLOCK_TIME.