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 |
tdb3
left a comment
tdb3
left a comment
There was a problem hiding this comment.
Approach ACK
The commit message of f2f3d802c9dbc096808dca8924501ca90081d813 can be updated to remove "and mention v30 target"
fjahr
force-pushed
the
2024-08-t4-follow-up
branch
from
f7e2ed3 to
35ad61a
Compare
Fixed |
tdb3
left a comment
tdb3
left a comment
There was a problem hiding this comment.
ACK 35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46
src/validation.cpp
Outdated
There was a problem hiding this comment.
35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46: 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.
Sjors
left a comment
Sjors
left a comment
There was a problem hiding this comment.
ACK 35ad61a6bde14a74cc07dc8ecf3bb1cd49590e46 modulo the above comment
| 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
5 participants
This completes follow-ups left open in #29775.
MAX_TIMEWARPconstant as the timewarp defense delta, equal toMAX_FUTURE_BLOCK_TIME.