The RandomOrphan function and the function ecdsa_signature_parse_der_lax
in pubkey.cpp were causing non-deterministic test coverage.
Force seed in the beginning of the test to make it deterministic.
The seed is selected carefully so that all branches of the function
ecdsa_signature_parse_der_lax are executed. Prior to this fix, the test
was exhibiting non-deterministic coverage since none of the ECDSA
signatures that were generated during the test had leading zeroes in
either R, S, or both, resulting in some branches of said function not
being executed. The seed ensures that both conditions are hit.
Removed denialofservice_tests test entry from the list of non-deterministic
tests in the coverage script.
e12e970df6fcae08ff8008812cdeef600d6b2db8 docs: match usage text to script and location (Peter Bushnell)
Pull request description:
Update the usage text in the README to match the usage text in the Python script.
02b26ba1c1/contrib/testgen/gen_key_io_test_vectors.py (L9)02b26ba1c1/contrib/testgen/gen_key_io_test_vectors.py (L10)
Also to match the file names in the actual destination.
02b26ba1c1/src/test/data/key_io_valid.json02b26ba1c1/src/test/data/key_io_invalid.json
Following the README usage text generates new files when the user is likely to have wanted to update the existing files.
ACKs for top commit:
fanquake:
ACK e12e970df6fcae08ff8008812cdeef600d6b2db8 - this looks correct.
Tree-SHA512: b7ab61e19a54597a8fbd1844b9cfaef78879e53b882eefe4e0140fa115674df7f061e468835186963b89c963244a17d922f2ad0829b10f62b84f02019ee33edb
Previously, we did not include the macOS SDK libc++ headers in our SDK
creation process and instead used whichever libc++ headers shipped with
the clang package we downloaded in depends.
This change adds a script (which works on both GNU/Linux and macOS) to
correctly generate the macOS SDK including the libc++ headers. This can
be thought of as a simplified rewrite of tpoechtrager's script:
d3392f4eae/tools/gen_sdk_package.sh
The location within the SDK where we place the libc++ headers is chosen
such that clang's search path detection logic for sysroots would pick up
the headers properly.
We also document this change.
47b49a05eafddcaef373f70436d794e9f9f7495c contrib: Fix SyntaxWarning in Python base58 implementation (Alex Willmer)
Pull request description:
In Python integers should be compared for equality (`i == j`), not identity (`i is j`). Recent versions of CPython 3.x emit a SyntaxWarning when they encounter this incorrect usage, e.g.
```
$ python3 base58.py
base58.py:110: SyntaxWarning: "is" with a literal. Did you mean "=="?
assert get_bcaddress_version('15VjRaDX9zpbA8LVnbrCAFzrVzN7ixHNsC') is 0
Tests passed
```
ACKs for top commit:
MarcoFalke:
ACK 47b49a05eafddcaef373f70436d794e9f9f7495c
Tree-SHA512: 9f8962025dcdfa062c0515c68a1864f5bbeb86bd0510c0ec0e413a5edb6afbfd5f41b4c0255784e53db8eaf39c68b7cfa7cc8a33a2e5214aae463fda374f8719
In Python integers should be compared for equality (`i == j`), not identity (`i is j`). Recent versions of CPython 3.x emit a SyntaxWarning when they encounter this incorrect usage, e.g.
```
$ python3 base58.py
base58.py:110: SyntaxWarning: "is" with a literal. Did you mean "=="?
assert get_bcaddress_version('15VjRaDX9zpbA8LVnbrCAFzrVzN7ixHNsC') is 0
Tests passed
```
f852761aec81ed23c7b9e4546c08d1ef303f2507 guix: Add clarifying documentation for V env var (Carl Dong)
85f4a4b0822e3aa10310c4623eff719f301e9263 guix: Make V=1 more powerful for debugging (Carl Dong)
Pull request description:
```
- Print commands in both unexpanded and expanded forms
- Set VERBOSE=1 for CMake
```
Ping MarcoFalke hopefully you use `V=1` already for the Guix builds on DrahtBot?
ACKs for top commit:
fanquake:
ACK f852761aec81ed23c7b9e4546c08d1ef303f2507. Ran a Windows Guix build and compared the output from master and this PR when using `V=1`. i.e `HOSTS=x86_64-w64-mingw32 PATH="/root/.config/guix/current/bin${PATH:+:}$PATH" V=1 ./contrib/guix/guix-build.sh`.
Tree-SHA512: 8bc466fa7b869618bbd5a0a91c6b23d4785009289f8dfb93b0349317463a9ab9ece128c72436e02a0819722a63e703100aed15807867a716fda891292fcb9d9d
bfe1ba2f5b36056e0c41edf8206b93d3d83098df rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong)
27e63e01cce368d67092de8f0c736927d6f6aa69 build: Accomodate makensis v2.x (Carl Dong)
1f2c39a30e0f82046c7aecddfda3eb99cb536816 guix: Remove logical cores requirement (Carl Dong)
a4f6ffa71e335d4b2a6bf525b7f416968f9cd9f7 lint: Also enable source statements for non-gitian (Carl Dong)
d256f91cb1b0d6ff5170106b99b0266cbe51f5a2 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong)
fa791da02f9684e3fd554b687fb692ae6a23d65a nsis: Specify OutFile path only once (Carl Dong)
14701604d0904bc5bbf1c67de08f8ee6d3215523 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong)
f5a6ac4f48b18f93050d77bcb23f9cf45ec34647 guix: Make source tarball using git-archive (Carl Dong)
395c1137f630dc495ffb2752a23bc1dfd470ee53 gitian: Limit sourced script to just assignments (Carl Dong)
Pull request description:
Based on: #18556
Related: https://github.com/bitcoin/bitcoin/pull/17595#discussion_r399728721
ACKs for top commit:
fanquake:
ACK bfe1ba2f5b36056e0c41edf8206b93d3d83098df - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase #18818.
Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
fa13090d2048c9a1e475eb25de756763f8adddb8 contrib: Remove optimize-pngs.py script, which lives in the maintainer repo (MarcoFalke)
Pull request description:
Moved to https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/optimize-pngs.py
Bitcoin Core should focus on the full node implementation, not on scripts to compress png images.
This script is only used when new PNG files are added to the repo. This happens about once every two years. So fetching the script from the other repo should not be a burden, but removing it from this repo is going to cut down on the meta files we need to maintain in the main repo.
ACKs for top commit:
practicalswift:
ACK fa13090d2048c9a1e475eb25de756763f8adddb8 -- `+0 lines, -82 lines` :)
promag:
ACK fa13090d2048c9a1e475eb25de756763f8adddb8.
hebasto:
ACK fa13090d2048c9a1e475eb25de756763f8adddb8, verified that script is already [moved](https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/56).
Tree-SHA512: 37d111adae769bcddc6ae88041032d5a2b8b228fec67f555c8333c38de3992f5138b30bea868d7d6d6b7f966a47133e5853134373b149ab23cba3b8b560ecb31
When using worktrees or submodules, you'll see a `.git' plain text file
at the root of your working tree instead of the usual `.git' directory.
This plain text file will point to the real GIT_DIR, under the
GIT_COMMON_DIR. From experimentation, the full GIT_COMMON_DIR is
required to exist for operations such as git-archive(1), so we expose it
as readonly inside the container.
Previously, the sourced script would create the source tarball. Now, it
only assigns variables and the source-ing script has more flexibility in
determining what to do with these variables.
See later commit showing how this flexibility is useful in our Guix
builds.
2aa48edec0101f8a77a2189244fc62722ff7a123 refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov)
1362be044724bb49d785ca2e296a3b43343c1690 build: Drop make dist in gitian builds (Hennadii Stepanov)
Pull request description:
After the merge of #18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`.
With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users.
Close#16588.
Close#18547.
As a good side-effect, #18349 becomes redundant.
**Change in behavior**
The following variables 1b151e3ffc/configure.ac (L2-L6)
are no longer used for naming of directories and tarballs.
Instead of them the gitian descriptors use a git tag (if available) or a commit hash.
---
Also a small refactor commit picked from #18404.
ACKs for top commit:
dongcarl:
ACK 2aa48edec0101f8a77a2189244fc62722ff7a123
MarcoFalke:
ACK 2aa48edec0101f8a77a2189244fc62722ff7a123
fanquake:
ACK 2aa48edec0101f8a77a2189244fc62722ff7a123 - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got #18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific).
Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
3e38023af724a76972d39cbccfb0bba4c54a0323 scripts: add PE .reloc section check to security-check.py (fanquake)
Pull request description:
The `ld` in binutils has historically had a few issues with PE binaries, there's a good summary in this [thread](https://sourceware.org/bugzilla/show_bug.cgi?id=19011).
One issue in particular was `ld` stripping the `.reloc` section out of PE binaries, even though it's required for functioning ASLR. This was [reported by a Tor developer in 2014](https://sourceware.org/bugzilla/show_bug.cgi?id=17321) and they have been patching their [own binutils](https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/binutils) ever since. However their patch only made it into binutils at the [start of this year](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0). It adds an `--enable-reloc-section` flag, which is turned on by default if you are using `--dynamic-base`. In the mean time this issue has also been worked around by other projects, such as FFmpeg, see [this commit](91b668acd6).
I have checked our recent supported Windows release binaries, and they do contain a `.reloc` section. From what I understand, we are using all the right compile/linker flags, including `-pie` & `-fPIE`, and have never run into the crashing/entrypoint issues that other projects might have seen.
One other thing worth noting here, it how Debian/Ubuntu patch the binutils that they distribute, because that's what we end up using in our gitian builds.
In the binutils-mingw-w64 in Bionic (18.04), which we currently use in gitian, PE hardening options/security flags are enabled by default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8ubuntu1/changelog) and the [relevant commit](452b3013b8).
However in Focal (20.04), this has now been reversed. PE hardening options are no-longer the default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8.8/changelog) and [relevant commit](7bd8b2fbc2), which cites same .reloc issue mentioned here.
Given that we explicitly specify/opt-in to everything that we want to use, the defaults aren't necessarily an issue for us. However I think it highlights the importance of continuing to be explicit about what we want, and not falling-back or relying on upstream.
This was also prompted by the possibility of us doing link time garbage collection, see #18579 & #18605. It seemed some sanity checks would be worthwhile in-case the linker goes haywire while garbage collecting.
I think Guix is going to bring great benefits when dealing with these kinds of issues. Carl you might have something to say in that regard.
ACKs for top commit:
dongcarl:
ACK 3e38023af724a76972d39cbccfb0bba4c54a0323
Tree-SHA512: af14d63bdb334bde548dd7de3e0946556b7e2598d817b56eb4e75b3f56c705c26aa85dd9783134c4b6a7aeb7cb4de567eed996e94d533d31511f57ed332287da
eb37275a6f972c81caef010b4ee9c5dc88edc759 Fix naming of macOS SDK and clarify version (Andrew Chow)
Pull request description:
Fixes the `MacOSX10.14.sdk.tar.gz` creation command to have `MacOSX.sdk` be correctly named as `MacOSX10.14.sdk` and for the resulting file to be placed in the current directory. Gitian requires that `tar.gz` contains a folder named `MacOSX10.14.sdk` and the command did not do this originally. Having the file be placed in the current directory is a convenience so builders don't have to go find it.
Also clarifies which version of Xcode to download and where it can be downloaded.
ACKs for top commit:
fanquake:
ACK eb37275a6f972c81caef010b4ee9c5dc88edc759 - tested the macOS and Linux SDK extraction. Also noticed something seemingly broken with Apple `tar`, but will open an issue to follow up.
Sjors:
ACK eb37275 for the macOS instruction
Tree-SHA512: d691e14711cf195999291dd6fb7ffe552c86f8b30d2b1a77e88b4db6050dd817ba128b047cf36d29b0bb0d4183e709b7c03aa27f31b64e562ea8cd948434ca55
fa4632c41714dfaa699bacc6a947d72668a4deef test: Move boost/stdlib includes last (MarcoFalke)
fa488f131fd4f5bab0d01376c5a5013306f1abcd scripted-diff: Bump copyright headers (MarcoFalke)
fac5c373006a9e4bcbb56843bb85f1aca4d87599 scripted-diff: Sort test includes (MarcoFalke)
Pull request description:
When writing tests, often includes need to be added or removed. Currently the list of includes is not sorted, so developers that write tests and have `clang-format` installed will either have an unrelated change (sorting) included in their commit or they will have to manually undo the sort.
This pull preempts both issues by just sorting all includes in one commit.
Please be aware that this is **NOT** a change to policy to enforce clang-format or any other developer guideline or process. Developers are free to use whatever tool they want, see also #18651.
Edit: Also includes a commit to bump the copyright headers, so that the touched files don't need to be touched again for that.
ACKs for top commit:
practicalswift:
ACK fa4632c41714dfaa699bacc6a947d72668a4deef
jonatack:
ACK fa4632c41714dfaa, light review and sanity checks with gcc build and clang fuzz build
Tree-SHA512: 130a8d073a379ba556b1e64104d37c46b671425c0aef0ed725fd60156a95e8dc83fb6f0b5330b2f8152cf5daaf3983b4aca5e75812598f2626c39fd12b88b180
e44aeefaaed8d698d1b9004b66f85384397b1a75 gitian: Add missing automake package to gitian-win-signer.yml (Andrew Chow)
Pull request description:
automake is needed to build osslsigncode otherwise autogen.sh fails with the docker virtualization method.
ACKs for top commit:
hebasto:
ACK e44aeefaaed8d698d1b9004b66f85384397b1a75, for `osslsigncode-1.7.1` we did not run `autogen.sh` in the past.
fanquake:
ACK e44aeefaaed8d698d1b9004b66f85384397b1a75
jonatack:
ACK e44aeef
Tree-SHA512: a0e615c1b099ee1c469ce41f886f2ece6746234a5a800743a4e8be671e4114fd30e1c35bc0ddcb75778409564129d0fde7ac4e3d70b0f7691f97f729f34c8e0c
c47adf8df435831c26ca25813fb7272176bd4eb7 Added my fingerprint Stephan Oeste (Emzy) (Stephan Oeste)
Pull request description:
By request from laanwj added my PGP fingerprint.
See: https://github.com/bitcoin-core/gitian.sigs/pull/1220#issuecomment-612778442
ACKs for top commit:
Sjors:
ACK c47adf8. Fingerprint matches Twitter profile: https://twitter.com/emzy (haven't verified it in any other way)
fanquake:
ACK c47adf8df435831c26ca25813fb7272176bd4eb7
Tree-SHA512: 3e39ae88f507a12f11fb2d5c779eba79ee2daeddecd0dc3f1fddfa29ce963d0e9af3fa5a10357157812597c10205a6beae31cc70af9471a782da23d8753b7cbd
