test: set segwit height back to 0 on regtest #24527
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was changed in bitcoin#22818 from 0 to 1. Since it changes BLOCK_OPT_WIT of the genesis block, older versions of bitcoin core would not read regtest directories created with newer versions without a reindex.
Member
|
The workaround in the meantime would be to write the genesis block with a previous version (or call |
Member
|
I haven't looked, but an alternative code patch would be to not check the genesis block for the witness flag. (Though, such a patch might be harder to backport) Unrelated: It is still a problem that the genesis block is passed around too much in validation. For example it is passed to |
maflcko
approved these changes
Mar 10, 2022
theStack
approved these changes
Mar 11, 2022
Contributor
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
Concept and code-review ACK 5ce3057
In #22818 I suggested setting the segwit height to 1 only for consistency reasons (see #22818 (comment)), but given the problem you describe it seems reasonable to set it back to 0.
jonatack
pushed a commit
to jonatack/bitcoin
that referenced
this pull request
Mar 13, 2022
This was changed in bitcoin#22818 from 0 to 1. Since it changes BLOCK_OPT_WIT of the genesis block, older versions of bitcoin core would not read regtest directories created with newer versions without a reindex. Github-Pull: bitcoin#24527 Rebased-From: 5ce3057
Merged
Member
|
Included in #24512 for backport to v23. |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 13, 2022
5ce3057 test: set segwit height back to 0 on regtest (Martin Zumsande) Pull request description: The change of `consensus.SegwitHeight` from 0 to 1 for regtest in bitcoin#22818 had the effect that if I create a regtest enviroment with current master (or 23.x), and then try to load this chain with an older version (22.x), I get an InitError `Witness data for blocks after height 0 requires validation. Please restart with -reindex` and have to reindex because `BLOCK_OPT_WITNESS` is no longer set for the Genesis block and `NeedsRedownload()` in validation returns `true` with an older version. That might be a bit annoying for tests that use a shared regtest dir with different versions. If people think this is enough of an issue to be worth fixing, I think it should also make it into 23.x ACKs for top commit: theStack: Concept and code-review ACK 5ce3057 Tree-SHA512: b0e89ff7fc953bc0ae929d2da44cde7149321d987fb4763934f6c9635d00d807129a50b459cc5e69e86bb1819e4b063b969486e8016a1cb8db8f905fa315653d
fanquake
added a commit
that referenced
this pull request
Mar 31, 2022
174af33 util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) ded10fe build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) 26c2f23 build: Fix Boost.Process detection on macOS arm64 (Hennadii Stepanov) 85f85c7 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake) eaa0419 contrib: fix signet miner (sighash mismatch) (Sebastian Falbesoner) 235b042 rpc: Exclude descriptor when address is excluded (MarcoFalke) b05a59b ci: Temporarily use clang-13 to work around clang-14 TSan bug (MarcoFalke) 65b9667 doc, init: add links to doc/cjdns.md (Jon Atack) 7a553d4 doc: update i2p.md with cjdns, improve local addresses section (Jon Atack) 4148396 doc: update tor.md with cjdns and getnodeaddresses, fix tor grep, (Jon Atack) 4690e8a doc: create initial doc/cjdns.md for cjdns how-to documentation (Jon Atack) 5d24f61 Clarify in -maxtimeadjustment that only outbound peers influence time data (Jon Atack) b1646f1 test: set segwit height back to 0 on regtest (Martin Zumsande) ef6a37b rpc: rename getdeploymentinfo status-next to status_next (Jon Atack) 2a6fcf9 init, doc: improve -onlynet help and tor/i2p documentation (Jon Atack) Pull request description: Backport the following to 23.x: - #24468 - #24528 - #24527 - #24609 - #24555 - #24663 - #24572 - #24636 - #24553 - #24659 - #24521 - #24523 - #24690 - #24710 Possibly also: - #24579 - #24691 ACKs for top commit: laanwj: List-of-commits ACK 174af33, I think we should merge this and move forward with rc3.. hebasto: ACK 174af33 Tree-SHA512: 5a493e1652b780b527767d6ca9e67012abd2fa5573496e85e0d8aa4bed3eb332bfcd72610b8dfb954ff274d42450623233c96c479de2085b9c8344ba5abf1935
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change of
consensus.SegwitHeightfrom 0 to 1 for regtest in #22818 had the effect that if I create a regtest enviroment with current master (or 23.x), and then try to load this chain with an older version (22.x), I get an InitErrorWitness data for blocks after height 0 requires validation. Please restart with -reindexand have to reindex because
BLOCK_OPT_WITNESSis no longer set for the Genesis block andNeedsRedownload()in validation returnstruewith an older version.That might be a bit annoying for tests that use a shared regtest dir with different versions.
If people think this is enough of an issue to be worth fixing, I think it should also make it into 23.x