doc: Improve versionbits.h documentation#16587
Conversation
|
I like it but I NACK about the change in the header file. |
|
@emilengler "Note that the functions below take a pindexPrev as input: they compute information for block B based on its parent." you mean this one ? I've tried to incorporate it in both |
There was a problem hiding this comment.
Should be "in-progress" :-)
|
ACK, thanks for adding documentation
The comment is moved from the cpp to the header file, where it should be. Public functions should be documented at declaration and not at the implementation as this keeps the API documentation better together. |
jnewbery
left a comment
jnewbery
left a comment
There was a problem hiding this comment.
Concept ACK. Thanks for adding documentation.
A couple of nits inline.
There was a problem hiding this comment.
nit: drop the part about 'a monotonic clock', since it's confusing. The chain is not monotonic (timestamps of blocks may precede their ancestors' timestamps). Besides, BIP 9 doesn't have any dependence on clock time.
There was a problem hiding this comment.
Sorry was thinking in MTP. "The expression GetMedianTimePast(block.parent) is referred to as MTP in the diagram above, and is treated as a monotonic clock defined by the chain." In the context of Median-Time-Past, I think BIP 9 envision the chain as a monotonic counter. Further, softfork timeout doesn't rely on it ?
(dropped it anyway, maybe too much)
There was a problem hiding this comment.
s/Caches state for/Caches state from/
ariard
force-pushed
the
2019-08-docs-versionbits
branch
from
bcc2a0f to
001b58b
Compare
|
Updated at 001b58b with typos and corrections, thanks all for reviews |
ajtowns
left a comment
ajtowns
left a comment
There was a problem hiding this comment.
This seems mostly fine to me
There was a problem hiding this comment.
The FAILED description there is verbatim from BIP-9 but it's nevertheless not accurate -- it'll be all blocks once the first retarget period after the timeout time is reached, if LOCKED_IN wasn't already reached.
|
ACK 001b58b20413f692166aca19c8f21cab64b2a09a after @ajtowns's nit is addressed. |
ariard
force-pushed
the
2019-08-docs-versionbits
branch
from
001b58b to
6576a87
Compare
|
ACK 6576a87 |
|
ACK 6576a87 |
6576a87 doc: Improve versionbits.h documentation (Antoine Riard) Pull request description: While reviewing burying of BIP 9 deployments, seen that versionbits.h wasn't that much documented. This is an attempt to improve it. It can be useful, given after burying this code isn't going to be used anymore and isn't straightforward at first sight. ACKs for top commit: jnewbery: ACK 6576a87 ajtowns: ACK 6576a87 fanquake: ACK 6576a87 Tree-SHA512: 906463e0b22b988f89d77f798bf94d294f70467d29975088b87384764fb5d0dd1350be67562cc264656f61f1eada2cba20f99c0d797d1d7f90203c269e34c714
6576a87 doc: Improve versionbits.h documentation (Antoine Riard) Pull request description: While reviewing burying of BIP 9 deployments, seen that versionbits.h wasn't that much documented. This is an attempt to improve it. It can be useful, given after burying this code isn't going to be used anymore and isn't straightforward at first sight. ACKs for top commit: jnewbery: ACK 6576a87 ajtowns: ACK 6576a87 fanquake: ACK 6576a87 Tree-SHA512: 906463e0b22b988f89d77f798bf94d294f70467d29975088b87384764fb5d0dd1350be67562cc264656f61f1eada2cba20f99c0d797d1d7f90203c269e34c714
7 participants
While reviewing burying of BIP 9 deployments, seen that versionbits.h wasn't that much documented. This is an attempt to improve it. It can be useful, given after burying this code isn't going to be used anymore and isn't straightforward at first sight.