test: Properly set BIP34 height in CreateNewBlock_validity unit test#22277
test: Properly set BIP34 height in CreateNewBlock_validity unit test#22277maflcko merged 1 commit intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
Code review ACK faa670d
Checked that the block's coinbase scriptSig assignment matches now the BIP34 specification, verified upated proof of work (BLOCKINFO) by running the test.
(By the way, being curious, how did you re-create it? I guess putting some grinding code directly into the loop? Could be interesting if ever other contributors want to change the test in a way that needs new PoW)
Jup, IIRC I just ground in the loop and used std::cout or something similar to create the new table. |
…k_validity unit test faa670d test: Properly set BIP34 height in CreateNewBlock_validity unit test (MarcoFalke) Pull request description: The coinbase scriptSig in this unit test has several issues: * The BIP34 height is not the "first item" as required (See https://github.com/bitcoin/bips/blob/master/bip-0034.mediawiki#specification) * It uses the wrong encoding ( See https://github.com/bitcoin/bitcoin/blob/da69d9965a112c6421fce5649b5a18beb7513526/src/validation.cpp#L3250 ) * It uses the wrong height (off by one) While BIP34 isn't currently enforced in this unit test, this should be fixed to avoid confusion and to promote self-consistency. The change obviously requires new proof of work (`BLOCKINFO`). Also change the block version from `1` to `VERSIONBITS_TOP_BITS`, because this test shouldn't care about the block version and bumping it is required for other changes. ACKs for top commit: theStack: Code review ACK faa670d Tree-SHA512: 8dbe2d5300a640f3e1817ff048906e60463aca64ba50fec8ee4f18fb1c70e511008755b0b5baba81114a1a6265fdfae9a4b7ae8acadfb2c7ad43223157a0386c
3 participants
The coinbase scriptSig in this unit test has several issues:
bitcoin/src/validation.cpp
Line 3250 in da69d99
While BIP34 isn't currently enforced in this unit test, this should be fixed to avoid confusion and to promote self-consistency.
The change obviously requires new proof of work (
BLOCKINFO).Also change the block version from
1toVERSIONBITS_TOP_BITS, because this test shouldn't care about the block version and bumping it is required for other changes.