test: Add feature_taproot.py --previous_release

[maflcko]

Disabling the new consensus code at runtime is fine, but potentially fragile and incomplete. Fix that by giving the option to run with a version that has been compiled without any taproot code.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• test: add v0.20.1, v0.21.0 and v22.0 to backwards compatibility test  #19013 by Sjors
• Multiprocess bitcoin #10102 by ryanofsky

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[luke-jr]

Concept ACK

[theStack]

Concept ACK

[michaelfolkson]

The motivation for this is that someone might run the functional tests of a post Taproot release with the code of a pre Taproot release (without any Taproot code)? I'm not entirely sure on the motivation.

Also I'd like to review and test this but unsure how the feature_taproot.py --previous_release test should get triggered. Obviously I can run this test individually but I don't think there is much point to that.

I asked on IRC and @jonatack pointed me to -vbparams config option which requires a start and end time "for specified version bits deployment" Let me know if this is the best way to test this.

[maflcko]

Taproot is a softfork that comes with some consensus and policy (transaction relay) changes. While there is a way to "remove" the taproot code at runtime with the vbparams option, there is currently no formal proof that none of the taproot code is executed at runtime. It was unknown if the new taproot code influences the test result of this test in any way.

The subtest checks that taproot is a softfork, i.e. nodes without the taproot code can still sync with the network at a small cost of missing some consensus checks. As said, vbparams can be used to disable the taproot code. A cleaner approach is to run the test against a previously released version of a full node that isn't aware of taproot.

You can run the test manually with the --previous_release or via the test_runner's built-in list.

If you want to test -vbparams outside of what the test does, you can pass it as an option to Bitcoin Core.

[maflcko]

You'll also have to download the previous release first. See https://github.com/bitcoin/bitcoin/pull/19013/files#diff-5de36acd90308dc62abf7855a686ee7052ffb6e762c756fd735fb0c9fbd9595d for instructions.

[michaelfolkson]

ACK fa3c615

Ran ./test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2 though only needed one previous release. Didn't include v0.20.1 as that isn't added yet. (Added in #19013)

Then feature_taproot.py --previous_release test passes. Reviewed code as well. Test successfully skips when you haven't downloaded a previous release.

[instagibbs]

If I wanted to intentionally break this, how do I mess with "older releases"? There documentation for this testing feature?

[maflcko]

See https://github.com/bitcoin/bitcoin/pull/19013/files#diff-5de36acd90308dc62abf7855a686ee7052ffb6e762c756fd735fb0c9fbd9595d for instructions.

[maflcko]

Rebased

[Sjors]

Concept ACK

[maflcko]

(force pushed to tidy up the first commit)

[Sjors]

I was a bit worried that this PR would break #19013, but the subset of changes you're using is simple enough to rebase on (this PR just downloads the binary and doesn't touch the other backwards compatibility tests).

tACK fa80e10

@instagibbs replace releases/v0.20.1/bin/bitcoind with ransomware :-)

@MarcoFalke your excellent explanation would make a nice comment in the test.

[maflcko]

Will do in a follow-up or if I have to force push.

[NelsonGaldeman]

I ran it and succesfully skipped the test when previous releases are missing. After downloading previous versions it ran and tests pass! Code looks fine.

tACK fa80e10

Am I right saying it will only be ran by the test runner for people who already had older versions compiled or purposely downloaded binaries?

Unrelated to the PR itself but my first try to download previous versions ended up in a Checksum did not match on the 0.21.0. Tried again and worked fine. Checked both files size showed on the console and they were slightly different.
Update on above: #22442

[maflcko]

Am I right saying it will only be ran by the test runner for people who already had older versions compiled or purposely downloaded binaries?

Yes. This is the case for the ci config (one task is using downloaded binaries) and locally if you have compiled/downloaded the binaries.

Unrelated to the PR itself but my first try to download previous versions ended up in a Checksum did not match on the 0.21.0. Tried again and worked fine. Checked both files size showed on the console and they were slightly different.

This is a "known" issue. See bitcoin-core/bitcoincore.org#753

[TheBlueMatt]

This is a "known" issue. See bitcoin-core/bitcoincore.org#753

This is absolutely not a known issue. Not at all. Not even remotely.

[DrahtBot]

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

[fanquake]

Looks like this has been merged?

[maflcko]

Jup, it's the usual brokenness of GitHub.

[Sjors]

Ah, this took a few commits from #19013. I'm rebasing that now...