Update verify.py to support Bitcoin Knots. #181
Conversation
|
README can also be updated in this pull request: https://github.com/bitcoinknots/bitcoin/blob/72b0de1a76e30bacb4cfde0c1489d72e1754b36e/contrib/verify-binaries/README.md |
|
@luke-jr do we have another place where we upload bitcoin releases? Seems like a good way to reduce risk for client downloadss.. |
contrib/verify-binaries/verify.py
Outdated
| HOST2 = "https://bitcoin.org" | ||
| VERSIONPREFIX = "bitcoin-core-" | ||
| HOST1 = "https://bitcoinknots.org" | ||
| HOST2 = "https://bitcoinknots.org" |
There was a problem hiding this comment.
hmm, this is bad.
We should have an alternative url to upload releases, however im not aware of one now either..
IMO it doesnt make sense to block the verification script because of this
Currently the script requires atleast 2 hosts to function.
Removing this requirement entirely seems like a potential downgrade in script quality to me, in terms of security requirements.
I think the best path over here would be to leave the requirement in for now, where you need 2 hosts, leave a note describing the odd code above HOST2, and also open an issue for creating an alternative release client and tagging it here on the PR
| log.error(f"No files matched the platform specified. Did you mean: {closest_match}") | ||
| return ReturnCode.NO_BINARIES_MATCH | ||
|
|
||
| # remove binaries that are known not to be hosted by bitcoincore.org |
There was a problem hiding this comment.
Why are you ripping out this functionality? just creates more unnecessary constraints on both the hosts,
we should leave the option to avoid these binaries fi we can since they arent the ones used by users..
There was a problem hiding this comment.
The block was removed simply because it was ad-hoc code to handle bitcoincore.org. However it is a good feature, and this functionality could be re-introduced as a cli option, like so:
./verify.py --ignore "-unsigned","-debug",".zip" . . .
Which would let the user filter out files he doesn't need. This would only apply if you're running the script with just the version, for example, and all matching files get verified and downloaded. Files matching the ignored keywords would be dropped.
There was a problem hiding this comment.
yes, this is better, or for now a simpler one that says -ignore-unused or something of that sort. I dont think ripping this functionality is good, it would add more constraints on hosts and make rebases harder. We should make the script more flexible rather than less.
|
GitHub? |
Added github as HOST2. Updated file download logic to support different routes for each host. Made test case require all hosts to provide files to validate both hosts.
|
I've updated the README as well as added github as the second host. I've also updated the test script to pass '--require-all-hosts' to verify both hosts work properly. |
This PR updates
verify.pyand its corresponding test file to support Bitcoin Knots releases. The script handles Knots' version format and downloads files from https://bitcoinknots.org.I am not aware of a Bitcoin Knots release mirror, so both
HOST1andHOST2have been set to bitcoinknots.org. If in the future a mirror is set up, then this script may be updated to provide proper redundancy.Changes:
XX.X.knotsYYYYMMDD)/files/XX.x/XX.X.knotsYYYYMMDD/)VERSION_FORMATandVERSION_EXAMPLE)Testing:
test.pypass successfully*Closes #180
*I am inexperienced with
gpgand don't know how to trust signatures. The tests passed when I checked for "good_untrusted_sigs" because none of my imported signatures are trusted. On a system where the signatures are trusted, the tests should pass without that modification.