backport: bitcoin#12134, #18426, #18534, #18828, #18864, #19153, #19201, #19205, #19560, #19813, #19859, partial #20354, #20955, #22442, #22790 #5537
Conversation
This file contains 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
knst
changed the title
knst
force-pushed
the
bc-bp-v20-missing-8v2
branch
3 times, most recently
from
113aedb
to
005737a
Compare
samettek
approved these changes
Aug 15, 2023
knst
force-pushed
the
bc-bp-v20-missing-8v2
branch
2 times, most recently
from
7a2a31a
to
87d59ad
Compare
little too early, it was not ready for review yet |
knst
changed the title
knst
force-pushed
the
bc-bp-v20-missing-8v2
branch
from
87d59ad
to
ce8c478
Compare
|
LGTM, will utACK after #5490 merge |
|
This pull request has conflicts, please rebase. |
knst
force-pushed
the
bc-bp-v20-missing-8v2
branch
from
ce8c478
to
03ed63d
Compare
rebased to top of |
| @@ -31,7 +31,7 @@ services: | |||
| cache: | |||
| ccache: true | |||
| directories: | |||
| - $BASE_BUILD_DIR/ci/scratch/.ccache | |||
There was a problem hiding this comment.
is this dropping cache?
There was a problem hiding this comment.
I guess that is true :D
There was a problem hiding this comment.
We don't use Travis, I'm just keeping this changes for backport compatibility. It doesn't affect any caches bcs no travis at all. But if there's any good idea, we can adapt it for our CI.
There was a problem hiding this comment.
imo it's fine to just backport changes for Travis as is and implement similar dashified ones for Gitlab
what is missing in our backport is release cache they added when they changed this line, we could add one too - pls see (and test) 7fe8c49
this PR:
first run https://gitlab.com/dashpay/dash/-/jobs/4911762972#L106 (downloading)
second run https://gitlab.com/dashpay/dash/-/jobs/4934280594#L106 (downloading)
7fe8c49:
first run https://gitlab.com/UdjinM6/dash/-/jobs/4935068647#L59 (downloading)
another branch with a dummy commit on top https://gitlab.com/UdjinM6/dash/-/jobs/4935596048#L62 (using cache)
PastaPastaPasta
previously approved these changes
Aug 23, 2023
UdjinM6
requested changes
Aug 23, 2023
|
maybe also ad2ab00 cause why having unused files/hashes listed there? (I know they have it in bitcoin but it makes no sense to me and we are dashifying the list anyway) |
knst
force-pushed
the
bc-bp-v20-missing-8v2
branch
from
03ed63d
to
f78bde4
Compare
applied suggestions and removed one commits that should not belong to this PR 6201997
I made this list not based on list of files, that bitcoin used, but based on list of hashes that we provide with each release, for example for version 19.3.0 I used this list of hashes: https://github.com/dashpay/dash/releases/download/v19.3.0/SHA256SUMS.asc Instead choosing which exactly files is used for which version I just included all of them for any possible further changes and whatever new file would be required by further changes in related scripts. So, I don't feel like removing some hashes from list is "dashyfication", it's more for me as an extra step of clean-up used data. |
PastaPastaPasta
approved these changes
Sep 13, 2023
PastaPastaPasta
force-pushed
the
bc-bp-v20-missing-8v2
branch
from
582f1d4
to
0f8a6cf
Compare
c456145 [test] add 0.19 backwards compatibility tests (Sjors Provoost) b769cd1 [test] add v0.17.1 wallet upgrade test (Sjors Provoost) 9d9390d [tests] add wallet backwards compatility tests (Sjors Provoost) c7ca630 [scripts] support release candidates of earlier releases (Sjors Provoost) 8b1460d [tests] check v0.17.1 and v0.18.1 backwards compatibility (Sjors Provoost) ae379cf [scripts] build earlier releases (Sjors Provoost) Pull request description: This PR adds binaries for 0.17, 0.18 and 0.19 to Travis and runs a basic block propagation test. Includes test for upgrading v0.17.1 wallets and opening master wallets with older versions. Usage: ```sh contrib/devtools/previous_release.sh -f -b v0.19.0.1 v0.18.1 v0.17.1 test/functional/backwards_compatibility.py ``` Travis caches these earlier releases, so it should be able to run these tests with little performance impact. Additional scenarios where it might be useful to run tests against earlier releases: * creating a wallet with bitcoin#11403's segwit implementation, copying it to an older node and making sure the user didn't lose any funds (although this PR doesn't support `v0.15.1`) * future consensus changes * P2P changes (e.g. to make sure we don't accidentally ban old nodes) ACKs for top commit: MarcoFalke: ACK c456145 🔨 Tree-SHA512: 360bd870603f95b14dc0cd629532cc147344f632b808617c18e1b585dfb1f082b401e5d493a48196b719e0aeaee533ae0a773dfc9f217f704aae898576c19232
…failed download 332f373 [scripts] previous_release: improve failed download error message (Sebastian Falbesoner) Pull request description: Currently, if the earlier release build/fetch script `previous_release.sh` is invoked with the option `-b` (intending to fetch a binary package from `https://bitcoin.org`) and the download fails, the user sees the following confusing output: ``` $ contrib/devtools/previous_release.sh -r -b v0.9.5 [...] gzip: stdin: not in gzip format tar: Child returned status 1 tar: Error is not recoverable: exiting now ``` This implies that the download worked, but the archive is corrupted, when in reality the HTML document containing the delivery fail reason (most likely 404 Not Found) is saved and tried to get unpacked. In contrast to wget, curl is a bit stubborn and needs explicit instructions to react to server errors via the flag `-f` (outputs error message and returns error code, ideal for scripts): https://curl.haxx.se/docs/manpage.html#-f On the PR branch, the output on failed download looks now the following: ``` $ contrib/devtools/previous_release.sh -r -b v0.9.5 [...] curl: (22) The requested URL returned error: 404 Not Found Download failed. ``` ACKs for top commit: fanquake: ACK 332f373 Tree-SHA512: 046c931ad9e78aeb2d13faa4866d46122ed325aa142483547c2b04032d03223ed2411783b00106fcab0cd91b2f78691531ac526ed7bb3ed7547b6e2adbfb2e93
…d with wallet c0c43ae test: skip backwards compat tests if not compiled with wallet (fanquake) Pull request description: Top commit has no ACKs. Tree-SHA512: d9975a1490e69134408b6b724cea26a6c1397d43f59850283b9e338ae38e00fefbcd868fb141e0a4bb55f02076690a99331f29cfa2d0fa66c165032b24a94081
fa359d1 test: Strip down previous releases boilerplate (MarcoFalke) Pull request description: Reduces code bloat and mental load to write compatibility tests ACKs for top commit: Sjors: tACK fa359d1 on macOS Tree-SHA512: dc66286b24b2f137e5bca99412850ec7eee8cc61cf9cdc7ab532d529220808189baea8d1b077f8b7f40d3e8881d981e1ffc5a877adb394816f1225b1186253e4
fa4cd1f ci: Switch to bitcoincore.org download (MarcoFalke) Pull request description: bitcoin.org is down and not in our control, so it seems odd to rely on it for our ci infrastructure ACKs for top commit: troygiorshev: ACK fa4cd1f Tree-SHA512: f9f0e9c69a52b8b1906ceae195e8bcc189799fb39be921b26e3a37d1f8f3999831f86c96c3546848c0d01429c36cfb2d7c5f314655ac5282d3e8e4cdd838960e
16d4b3f test: mempool.dat compatibility between versions (Ivan Metlushko) Pull request description: Rationale: Verify mempool.dat compatibility between versions The format of mempool.dat has been changed in bitcoin#18038 The tests verifies the fix made in bitcoin#18807 and ensures that the file format is compatible between current version and v0.19.1 The test verifies both backward and forward compatibility. This PR also adds a log when we fail to add a tx loaded from mempool.dat. It was useful when debugging this test and could be potentially useful to debug other scenarios as well. Closes bitcoin#19037 ACKs for top commit: Sjors: tACK 16d4b3f Tree-SHA512: 00a38bf528c6478cb0da467af216488f83c1e3ca4d9166c109202ea8284023e99d87a3d6e252c4d88d08d9b5ed1a730b3e1970d6e5c0aef526fa7ced40de7490
…0.19.0.1 to v0.19.1 d135c29 [ci] make list of previous releases to download a setting (Sjors Provoost) 9c246b8 [test] backwards compatibility: bump v0.19.0.1 to v0.19.1 (Sjors Provoost) 89a28e0 [test] add v0.16.3 backwards compatibility test (Sjors Provoost) Pull request description: Thanks to bitcoin#18774's `adjust_bitcoin_conf_for_pre_17` we can now test backwards compatibility for v0.16.3, both for sync and loading a recent wallet. This PR bumps v0.19.0.1 to v0.19.1. I also made the version list consistent for the `contrib/devtools/previous_release.sh` instruction, between both tests. ACKs for top commit: MarcoFalke: ACK d135c29 Tree-SHA512: 5ff137a7a934237fa220f1c2807ce9abeeb75929266558bf3e4045bec7dfcd0a8747fa74d700065c568330b18badf58c60c308eb13d1eed444d4bbfe6decc48b
9c34aff Remove previous_release.sh (Brian Liotti) e1e5960 script: Add previous_release.py (Brian Liotti) Pull request description: Closes bitcoin#18132 Added functionality: 1) checks file hash before untarring when using the binary download option ACKs for top commit: fjahr: re-ACK 9c34aff Sjors: tACK 9c34aff Tree-SHA512: 323f11828736a372a47f048592de8b027ddcd75b38f312dfc73f7b495d1e078bfeb384d9cdf434b3e70f2c6c0ce2da2df48e9a6460ac0e1967c6829a411c52d5
facdf53 contrib: Clean up previous_releases.py (MarcoFalke) Pull request description: ACKs for top commit: fjahr: tACK facdf53 Sjors: tACK facdf53 hebasto: ACK facdf53, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: c3543320572267035aa342dd170128bbdeb83ca4e2e36a8e46596dd76c8ff1b26ed6759a8073884228b133c45b06ec48889cd0ec83a13bef276b48073d8248e4
…cksums 0374e82 util: Hard code previous release tarball checksums (Hennadii Stepanov) bd897ce scripted-diff: Move previous_release.py to test/get_previous_releases.py (Hennadii Stepanov) Pull request description: bitcoin#19205 introduced signature verifying for the downloaded `SHA256SUMS.asc`. This approach is brittle and does not work in CI environment for many reasons: - bitcoin#19812 (comment) - bitcoin#19013 (comment) This PR: - implements **Sjors**' [idea](bitcoin#19205 (review)): > Alternatively we might as well hard code the checksum for each `tar.gz` release in the source code, here. - is an alternative to 5a2c31e (bitcoin#19013) - fixes bitcoin#19812 - updates v0.17.1 to v0.17.2 ACKs for top commit: MarcoFalke: cr ACK 0374e82 Sjors: tACK 0374e82 Tree-SHA512: cacdcf9f5209eae7da357abb3445585ad2f980920fd5bf75527ce89974d3f531a4cf8b5b35edfc116b23bfdfb45c0437cb14cbc416d76ed2dc5b9e6d33cdad71
…rsion 6de9429 qa: Changes v0.17.1 to v0.17.2 (nthumann) Pull request description: As of bitcoin@0374e82 v0.17.2 is downloaded instead of v0.17.1 for functional testing. This causes `test/functional/feature_backwards_compatibility.py` to fail, because it [requires](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_backwards_compatibility.py#L57) v0.17.1. Steps to reproduce: Run `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2`. It cannot be downloaded at all because the sha256sum is missing [here](https://github.com/bitcoin/bitcoin/blob/c1e0c2ad3b6cd9e7ef55287fb572cfcf10a0e660/test/get_previous_releases.py#L23). Or adjust the command and run `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2`, then run `test/functional/test_runner.py feature_backwards_compatibility`. It´ll fail because the test is missing v0.17.1. This PR changes v0.17.1 to v0.17.2 in this test and in a few comments. ACKs for top commit: laanwj: ACK 6de9429 fanquake: ACK 6de9429 - looks correct. Surprised this wasn't caught/part of bitcoin#19813. In future you could add any explanations & extra info as part of your commit message as well (even though PR descriptions are included as part of the merge). Tree-SHA512: bbe50c4fd5c1aedd6dc1cdc3d93ef9005db1c67adca3f263b6b0d869c40b495a3221e706c9389fedea4748e31911dbd591062f60ce9836e58099fbdd9515b4d9
fa1d5e5 test: Fix get_previous_releases.py for aarch64 (MarcoFalke) Pull request description: Otherwise it will fail with "Not sure which binary to download..." ACKs for top commit: laanwj: Code review ACK fa1d5e5 Tree-SHA512: 0db71e898a431665757ce835016a4e05c629a95abc4a2951eac9bd9b5876ec3dc3d6f156d58565e2bcdf918cde4f2649183d4a58038ac13c705a7e914c0094d1
…leases script 179a051 util: improves error messages on get_previous_releases script (Nelson Galdeman) Pull request description: When previous releases are fetched and the specified version wasn't added to the checksum list we used to get a "Checksum did not match" which isn't true (bitcoin-core/bitcoincore.org#753 (comment)). If the specified version number is not on the list, it now logs cannot do the comparison instead. ACKs for top commit: practicalswift: cr ACK 179a051 theStack: tACK 179a051, tested on Debian bullseye/sid Tree-SHA512: 2a07ce75232f853fd311c43581f8faf12d423668946ae6ad784feece5b4d0edd57fc018ba1f0c5a73bfaccb326e0df9a643580d16bf427c1ec3ff34a9cdbc80c
…release fa80e10 test: Add feature_taproot.py --previous_release (MarcoFalke) 85ccffa test: move releases download incantation to README (Sjors Provoost) 29d6b1d test: previous releases: add v0.20.1 (Sjors Provoost) Pull request description: 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. ACKs for top commit: Sjors: tACK fa80e10 NelsonGaldeman: tACK fa80e10 Tree-SHA512: 1a1feef823f08c05268759645a8974e1b2d39a024258f5e6acecbe25097aae3fa9302c27262978b40f1aa8e7b525b60c0047199010f2a5d6017dd6434b4066f0
…ScriptPubKeyMan::CanProvide script recognition [test] check for addmultisigaddress regression
…blocks failures
…errors in comments doc: Correct spelling errors in comments And ci script output. Identified via test/lint/lint-spelling
…t RPC - partial dashification - disabling this test so far as it does not work anyway
… get_previous_releases f6e4db2 test: add aarch64-apple-darwin platform entry to get_previous_releases (Zero-1729) Pull request description: Over the course of reviewing a PR, I had to edit `test/get_previous_releases.py` (after I ran `git clean -xdff`) to run the backwards compatibility tests (e.g. `wallet_upgradewallet`, `feature_backwards_compatibility`, etc.), as currently on master, running the script as indicated in [`test/README.md`](https://github.com/bitcoin/bitcoin/blob/master/test/README.md), for example, on an M1 machine results in the following error, as the `aarch64-apple-darwin*` platform entry is presently not recognised: > Output from an M1 machine running macOS v11.5.2 ```sh $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2 Releases directory: releases Not sure which binary to download for aarch64-apple-darwin20.6.0 ``` As a quick fix, this PR adds the missing `aarch64-apple-darwin*` platform entry. Running the script now results in fetching the old binaries, as expected: ```sh $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2 Releases directory: releases Fetching: https://bitcoincore.org/bin/bitcoin-core-0.20.1/bitcoin-0.20.1-osx64.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 20.9M 0 0 0 0 0 0 --:--:-- 0:00:02 --:--:-- 0 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 20.9M 100 20.9M 0 0 136k 0 0:02:37 0:02:37 --:--:-- 95607 Checksum matched … Checksum matched ``` After this patch, the backwards compatibility tests also run successfully, as expected. **Note**: I am open to other possible solutions. --- Steps to reproduce: > Ensure you take out the binaries in `releases` if they already exist. Try running `test/get_previous_releases.py -b v0.15.2` or similar to fetch the old release binaries. Top commit has no ACKs. Tree-SHA512: a238d909b70a61be622234bc49b05d2e91a8acfc5ea348d29f2c8a927fb793cb97365e558571e3f46d6a5650c4f3c6e28fa126c6e56b38e1eb98f7c3e3594d0f
PastaPastaPasta
force-pushed
the
bc-bp-v20-missing-8v2
branch
from
0f8a6cf
to
affcaca
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue being fixed or feature implemented
That's the last batch of backports from bitcoin v20. Beside that there's some related backports from other versions and related fixes.
Note for reviewer: please, notice, that some particular commits are not dashified because later on theirs code is changed or removed in next backports of this PR. But final version of all changes is dashified (except wallet_upgradewallet.py that was backported some time ago in past and it still doesn't work).
What was done?
Backports:
Missing changes from other backports that were done before:
All these backports are bringing next changes:
mempool_compatibility.pyfeature_backwards_compatibility.pywallet_upgradewallet.pytest/get_previous_releases.pyto get binaries with previous versionAlthough,
wallet_upgradewallet.pyis disabled intentionally - it's not adopted for dash specific so far as HD wallets are not enabled by default atm.How Has This Been Tested?
Run unit/functional tests.
Beside that tested new functional script. Steps to do it:
Breaking Changes
N/A
Checklist: