Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: trivial 2024 10 23 pr3 #6347

Merged

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Batch of trivial backports

What was done?

See commits

How Has This Been Tested?

built locally; large combined merge passed tests locally

Breaking Changes

Should be none

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@PastaPastaPasta PastaPastaPasta added this to the 22 milestone Oct 24, 2024
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-23-pr3 branch 2 times, most recently from 2ac274a to 0ed3984 Compare October 24, 2024 01:05
Copy link

This pull request has conflicts, please rebase.

…iprocess.md

d9b172c doc: fix link to developer-notes.md file in multiprocess.md (David Álvarez Rosa)

Pull request description:

  Fix link to `developer-notes.md` file in `multiprocess.md`.

ACKs for top commit:
  fanquake:
    ACK d9b172c

Tree-SHA512: 55fffefb37c4d67acb1fa8b0660216ec1c7f2c2314d11e4d319cae40480ed59ef448909fa2ca334167c86d60d41932220dce4186e28fa300f4d03eb0b3c769d0
f0cebbd qt: enable -ltcg for windows HOST (fanquake)

Pull request description:

  Patch around multiple definition issues in Qt, and enable `-ltcg` when using `LTO=1`.

  Split from bitcoin#25391.

ACKs for top commit:
  hebasto:
    ACK f0cebbd

Tree-SHA512: 2d6e34779f360bf6dfea4f70fc9004a16e95da79716fcb3046afbf2b01317b7e16965cb51b967b7b5fb64549306c5f48cf59082884289c52016bc1e86949e062
57131bf docs: fix typo (vuittont60)

Pull request description:

ACKs for top commit:
  maflcko:
    lgtm ACK 57131bf
  hebasto:
    ACK 57131bf, the `codespell` is powerless to catch this typo.
  jarolrod:
    ACK 57131bf

Tree-SHA512: 816dfc5ff64531ea92acf35feca2286a71c75344df2524ff003a3d375e60100b8531e1678be0ed11863d03ab522d5733d8a0bf4b6f5f79c495a65246fe0b697f
…o hardening flags

61a6c3b build: add `-mbranch-protection=bti` to aarch64 hardening flags (fanquake)

Pull request description:

  This is a simpler (less hardening) version of bitcoin#24123.

  You can inspect binaries using `readelf -n`, and look for BTI in a `.note.gnu.property`. i.e
  ```bash
  readelf -n src/bitcoin-cli

  Displaying notes found in: .note.gnu.property
    Owner                Data size Description
    GNU                  0x00000010NT_GNU_PROPERTY_TYPE_0
        Properties: AArch64 feature: BTI
  ```

  Related to bitcoin#19075.

ACKs for top commit:
  TheCharlatan:
    utACK 61a6c3b

Tree-SHA512: 64504de44e91d853165daf4111dca905d8eb9ef3f4bfb0d447c677b02c9100dbd56f13e6fe6539fb06c2343a094229591ac5d1bd9e184b32b512c0ac3f9bac36
fa858d6 fuzz: Merge with -set_cover_merge=1 (MarcoFalke)

Pull request description:

  This should be less controversial than commit 151a2b1. The overall size of the qa-assets repo is reduced further from 1.9GB to 1.6GB. Also, the runtime to iterate on the resulting folder is reduced further from ~1699s to ~1149s (N=1).

ACKs for top commit:
  murchandamus:
    crACK fa858d6
  dergoegge:
    ACK fa858d6

Tree-SHA512: e23fa93bd48f01d11c551b035004c678bd6d76bc24ac7d0d0a7883060804e6711763cbd0cd0ded3aad3e4c40da764decae81c2703388cc11961def3c89a4f9ba
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-23-pr3 branch from 0ed3984 to 1613fce Compare October 24, 2024 16:18
@UdjinM6
Copy link

UdjinM6 commented Oct 24, 2024

guix fails

======================================================================
FAIL: test_MACHO (__main__.TestSymbolChecks)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/distsrc-base/distsrc-21.1.1-1147-g1613fce5d74a-arm64-apple-darwin/./contrib/devtools/test-symbol-check.py", line 99, in test_MACHO
    self.assertEqual(call_symbol_check(cc, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']),
AssertionError: Tuples differ: (1, '[33 chars]D_LIBRARIES!\ntest1: failed DYNAMIC_LIBRARIES MIN_OS SDK LD64') != (1, '[33 chars]D_LIBRARIES!\ntest1: failed DYNAMIC_LIBRARIES MIN_OS SDK')

First differing element 1:
'libe[28 chars]ED_LIBRARIES!\ntest1: failed DYNAMIC_LIBRARIES MIN_OS SDK LD64'
'libe[28 chars]ED_LIBRARIES!\ntest1: failed DYNAMIC_LIBRARIES MIN_OS SDK'

  (1,
   'libexpat.1.dylib is not in ALLOWED_LIBRARIES!\n'
-  'test1: failed DYNAMIC_LIBRARIES MIN_OS SDK LD64')
?                                             -----

+  'test1: failed DYNAMIC_LIBRARIES MIN_OS SDK')

----------------------------------------------------------------------

I guess it's 28630

fanquake and others added 11 commits October 24, 2024 13:50
fa21535 fuzz: Increase merge -rss_limit_mb (MarcoFalke)

Pull request description:

  For some reason, the limit is hit. (Presumably due to `-set_cover_merge=1` eating more memory, or by simply having more fuzz inputs).

  Fix it by increasing it for the merge operation.

ACKs for top commit:
  dergoegge:
    ACK fa21535
  hebasto:
    ACK fa21535, considering the discussion in bitcoin-core/qa-assets#155.

Tree-SHA512: 4fed0f254eccc6fe0b53656bc345ff898b13811dc39387387317d34b521ab77cee03d82b0896dd92d253b7546b6a7e4bdcd478749f47064374ab44ad759ab9ff
fac36b9 refactor: Remove CBlockFileInfo::SetNull (MarcoFalke)

Pull request description:

  Seems better to use C++11 member initializers and then let the compiler figure out how to construct objects of this class.

ACKs for top commit:
  stickies-v:
    ACK fac36b9
  pablomartin4btc:
    ACK fac36b9
  theStack:
    LGTM ACK fac36b9

Tree-SHA512: aee741c8f668f0e5b658fc83f4ebd196b43fead3dd437afdb0a2dafe092ae3d559332b3d9d61985c92e1a59982d8f24942606e6a98598c6ef7ff43697e858725
…tly in `util/trace.h`

6bdff42 build: Include `config/bitcoin-config.h` explicitly in `util/trace.h` (Hennadii Stepanov)

Pull request description:

  The `ENABLE_TRACING` macro is expected to be defined in the `config/bitcoin-config.h` header.

  Therefore, the current code is error-prone as it depends on whether the `config/bitcoin-config.h` header was included before or not.

  This bug was noticed while working on CMake [stuff](hebasto#37).

ACKs for top commit:
  fanquake:
    ACK 6bdff42

Tree-SHA512: 22c4fdeb51628814050eb99a83db4268a4f3106207eeef918a07214bbc52f2b22490f6b05fcb96216f147afa4197c51102503738131e2583e750b6d195747a49
a5e39d3 Fee estimation: extend bucket ranges consistently (Anthony Towns)

Pull request description:

  When calculating a median fee for a confirmation target at a particular threshold, we analyse buckets in ranges rather than individually in case some buckets have very little data. This patch ensures the breaks between ranges are independent of the the confirmation target.

  Fixes bitcoin#20725

ACKs for top commit:
  ismaelsadeeq:
    Code review ACK a5e39d3
  glozow:
    btw what I meant by [this](bitcoin#21161 (review)) was ACK a5e39d3
  jonatack:
    Initial ACK a5e39d3

Tree-SHA512: 0edf4e56717c4ab8d4ab0bc0f1d7ab36a13b99de12f689e55c9142c6b81691367ffd8df2e8260c5e14335310b1a51770c6c22995db31109976239befcb558ef8
… build

664c873 depends: drop -O1 workaround from arm64 apple Qt build (fanquake)

Pull request description:

  Drop the workaround of setting optimization flags to -O1 for the `arm64-apple-darwin` builds. I no-longer see reproducibility issues when building across `x86_64` and `aarch64`:
  ```bash
  real7m21.192s
  user67m41.047s
  sys5m8.596s
  fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  fedora-32gb-hel1-1 bitcoin]# uname -m
  aarch64
  ```

  ```bash
  real18m10.759s
  user108m21.656s
  sys6m12.930s
  ubuntu-32gb-hel1-1:~/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  ubuntu-32gb-hel1-1:~/bitcoin# uname -m
  x86_64
  ```

ACKs for top commit:
  hebasto:
    ACK 664c873.
  TheCharlatan:
    ACK 664c873

Tree-SHA512: 79527df4181eb0a0c42fe526581479abcdeba8fb09e1faf52265d697d39a8f3a3532ee3c573579b9af00b7330a401e4b6f1686636f9bac6bf9839be8381a2033
aee5404 Add support for RNDR/RNDRRS for aarch64 on Linux (John Moffett)

Pull request description:

  This checks whether the ARMv8.5-A optional TRNG extensions [RNDR](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDR--Random-Number) and [RNDRRS](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number) are available and, if they are, uses them for random entropy purposes.

  They are nearly functionally identical to the x86 RDRAND/RDSEED extensions and are used in a similar manner.

  Currently, there [appears to be](https://marcin.juszkiewicz.com.pl/download/tables/arm-socs.html) only one actual hardware implementation -- the Amazon Graviton 3. (See the `rnd` column in the link.) However, future hardware implementations may become available.

  It's not possible to directly query for the capability in userspace, but the Linux kernel [added support](torvalds/linux@1a50ec0) for querying the extension via `getauxval` in version 5.6 (in 2020), so this is limited to Linux-only for now.

  Reviewers may want to launch any of the `c7g` instances from AWS to test the Graviton 3 hardware. Alternatively, QEMU emulates these opcodes for `aarch64` with CPU setting `max`.

  Output from Graviton 3 hardware:

  ```
  ubuntu@ip:~/bitcoin$ src/bitcoind -regtest
  2023-01-06T20:01:48Z Bitcoin Core version v24.99.0-3670266ce89a (release build)
  2023-01-06T20:01:48Z Using the 'arm_shani(1way,2way)' SHA256 implementation
  2023-01-06T20:01:48Z Using RNDR and RNDRRS as additional entropy sources
  2023-01-06T20:01:48Z Default data directory /home/ubuntu/.bitcoin
  ```

  Graviton 2 (doesn't support extensions):

  ```
  ubuntu@ip:~/bitcoin$ src/bitcoind -regtest
  2023-01-06T20:05:04Z Bitcoin Core version v24.99.0-3670266ce89a (release build)
  2023-01-06T20:05:04Z Using the 'arm_shani(1way,2way)' SHA256 implementation
  2023-01-06T20:05:04Z Default data directory /home/ubuntu/.bitcoin
  ```

  This partially closes bitcoin#26796. As noted in that issue, OpenSSL [added support](openssl/openssl#15361) for these extensions a little over a year ago.

ACKs for top commit:
  achow101:
    ACK aee5404
  laanwj:
    Tested ACK aee5404

Tree-SHA512: 1c1eb345d6690f5307a87e9bac8f06a0d1fdc7ca35db38fa22192510a44289a03252e4677dc7cbf731a27e6e3a9a4e42b6eb4149fe063bc1c905eb2536cdb1d3
ebc7063 doc: update docs for CHECK_ATOMIC macro (fanquake)

Pull request description:

  Clarify that supported versions of GCC are not affected, and that Clang
  prior to version 15 still requires the explicit `-latomic` linking, when
  compiling for 32-bit.

ACKs for top commit:
  hebasto:
    ACK ebc7063.

Tree-SHA512: 6044dc28547431cfde7e89b663b5f9a86a4cb801212a21c3dbb18a1c41a53640480c3e4e944050dc3ec4cded9bc4c1f8eae8dbb60596289fef49bb13a8b53b76
…gure error

30bd4b1 doc: remove mention of missing bdb being a configure error (fanquake)

Pull request description:

  This is no-longer the case, unless you're passing additional flags, which is not the case in this example.

ACKs for top commit:
  maflcko:
    lgtm ACK 30bd4b1
  TheCharlatan:
    ACK 30bd4b1
  hebasto:
    ACK 30bd4b1.

Tree-SHA512: b3730546d7ff1f49854b88e710c72c4f6e4b6d238147599d4c4e4adeeb256424c2096635f6c51dcfe2e5a9c1155c1c9915fe03a09c5c38605bee2722756c8f6e
… doc

821a8a1 doc: remove x86_64 build assumption from depends doc (fanquake)

Pull request description:

  This dates from the introduction of depends, and has not been the case for some time now.

ACKs for top commit:
  maflcko:
    lgtm ACK 821a8a1
  hebasto:
    ACK 821a8a1.
  theuni:
    ACK 821a8a1

Tree-SHA512: 640967a3e6dfab495fd733d3379aa916ac7f67e89a92ef6a94c3bea0494dc7921a9d7485e1b90a1beab00548b575cdab8fb08eb9267dcc5e890cc796ae1b6875
…properly

05aca09 build: Patch Qt to handle minimum macOS version properly (Hennadii Stepanov)

Pull request description:

  This PR is:
  - required to [switch](bitcoin#28622) to macOS 14 SDK (Xcode 15).
  - an alternative to bitcoin#28732 and bitcoin#28775.

  Qt relies on the `__MAC_OS_X_VERSION_MIN_REQUIRED` macro, which is set in the `AvailabilityInternal.h` SDK header to
  the value provided by the Clang driver from the `-mmacos-version-min` / `-mmacosx-version-min` option.

  Xcode 12 SDK expects the OS-specific `__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__` macro:
  ```c++
  #ifndef __MAC_OS_X_VERSION_MIN_REQUIRED
      #ifdef __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
          /* compiler for Mac OS X sets __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ */
          #define __MAC_OS_X_VERSION_MIN_REQUIRED __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
      #endif
  #endif /* __MAC_OS_X_VERSION_MIN_REQUIRED*/
  ```

  In the other hand, Xcode 15 SDK expects a general `__ENVIRONMENT_OS_VERSION_MIN_REQUIRED__` macro:
  ```c++
  #ifndef __MAC_OS_X_VERSION_MIN_REQUIRED
      #if defined(__has_builtin) && __has_builtin(__is_target_os)
          #if __is_target_os(macos)
              #define __MAC_OS_X_VERSION_MIN_REQUIRED __ENVIRONMENT_OS_VERSION_MIN_REQUIRED__
              #define __MAC_OS_X_VERSION_MAX_ALLOWED __MAC_14_0
          #endif
      #elif  __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
          #define __MAC_OS_X_VERSION_MIN_REQUIRED __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__
          #define __MAC_OS_X_VERSION_MAX_ALLOWED __MAC_14_0
      #endif /*  __has_builtin(__is_target_os) && __is_target_os(macos) */
  #endif /* __MAC_OS_X_VERSION_MIN_REQUIRED */
  ```

  The latter macro is not provided by LLVM Clang until llvm/llvm-project@c8e2dd8, which is available in Clang 17.

  The suggested patch makes Qt "borrow" the `__MAC_OS_X_VERSION_MIN_REQUIRED` value from `MAC_OS_X_VERSION_MIN_REQUIRED`, which is set in the `AvailabilityMacros.h` SDK header.

ACKs for top commit:
  maflcko:
    lgtm ACK 05aca09

Tree-SHA512: 8891aefde4b8a48885abf0648f4ec71a22f7fcfca1e17ebb8c70ce1ef44751ea5db6b8b652de6ee8a716ca5f96f720fef01600bc23986162d0146c946e2e8743
f8ca135 build: Fix check whether `-latomic` needed (Hennadii Stepanov)

Pull request description:

  Clang >=15 still might need linking against `libatomic`.

  We use `std::atomic<std::chrono::seconds>::compare_exchange_strong` in `net_processing.cpp`.

  Addresses the bitcoin#29165 (comment).

ACKs for top commit:
  maflcko:
    lgtm ACK f8ca135
  fanquake:
    ACK f8ca135

Tree-SHA512: ba8b6a88fd3471a206d068e8a000a053c99cb46d26bd04624418ddb066b3b9664a569ec8a1569af67c96b3e27f13dccbd5e24f985290ac072b6d74c92524e35d
@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-23-pr3 branch from 1613fce to c75a0d4 Compare October 24, 2024 18:50
@PastaPastaPasta
Copy link
Member Author

Dropped

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c75a0d4

@@ -133,9 +133,6 @@ $(package)_config_opts_darwin += -no-feature-corewlan
$(package)_config_opts_darwin += -no-freetype
$(package)_config_opts_darwin += QMAKE_MACOSX_DEPLOYMENT_TARGET=$(OSX_MIN_VERSION)

# Optimizing using > -O1 causes non-determinism when building across arches.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

28778 - should we test it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should; I think let's merge it in and then we'll build guix here and on my repo, and compare. Revert if needed

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

beside not tested 28778

@PastaPastaPasta PastaPastaPasta merged commit 0587790 into dashpay:develop Oct 24, 2024
40 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants