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 25 pr2 #6363

Merged

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

Trivial backports

What was done?

How Has This Been Tested?

built locally

Breaking Changes

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)

84fbf9b depends: remove -g from sqlite debug flags (fanquake)
eef51af depends: add -g to DEBUG=1 flags (fanquake)

Pull request description:

  Add `-g` to the base DEBUG=1 flags in depends.
  Avoids the need to specify it per-package.
  More alignment with `--enable-debug` behaviour in configure.

  We also want to align the optimization flags, currently -O1 vs -O0, however that can be it's own PR.

ACKs for top commit:
  theuni:
    ACK 84fbf9b

Tree-SHA512: 1ee98ba0c13e4b80bb87632658b4f53ce49c73e0e7712990c30da60deca4a349a744232f2d78f243dee9a07f5b9b70f9c2c4ae34082c34ae51b37b054fac61fd
@PastaPastaPasta PastaPastaPasta added this to the 22 milestone Oct 25, 2024
fanquake and others added 16 commits October 26, 2024 12:29
5efebc0 depends: add the new LLVM debug macro (fanquake)

Pull request description:

  `LIBCXX_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18.

  See https://libcxx.llvm.org/Hardening.html.

  Required before google/oss-fuzz#11725 will do anything (with the bump to 18.x).

  Seems reasonable to do now that almost all our test infra is using LLVM 18.

ACKs for top commit:
  theuni:
    ACK 5efebc0

Tree-SHA512: 43078eeb5940c55ef4f95c72682f8a372dcd3eb97956b3114149c16d9f59b067a999b2aab7f34ffb57eab191524514408e2bba154ff4a6ea0cd6ec4d119c5d18
…y must not exist

d4e36ae test: Update --tmpdir doc string to say directory must not exist (kevkevin)

Pull request description:

  The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist

  This change is motivated by this comment bitcoin#29335 (comment)

ACKs for top commit:
  maflcko:
    lgtm ACK d4e36ae
  davidgumberg:
    ACK bitcoin@d4e36ae

Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1
03b87a3 Drop Windows Socket dependency for `randomenv.cpp` (Hennadii Stepanov)

Pull request description:

  This change drops a dependency on the ws2_32 library for our libbitcoinkernel by switching to [`GetComputerName`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamew) function.

ACKs for top commit:
  sipsorcery:
    utACK 03b87a3.
  laanwj:
    Code review ACK 03b87a3.
  fanquake:
    ACK 03b87a3

Tree-SHA512: a4abd5499176634d5f3fbf4e794a7504c40232fb73bd7f41955fbfb2cc7c44bc7ea4518c5203836e52f552c30414c6c3e1b24f0922641dbf1c8377981c0ffaf0
…cks_hb.py

1ae5b20 test: fix intermittent failure in p2p_compactblocks_hb.py (Martin Zumsande)

Pull request description:

  Fixes bitcoin#29860

  As a result of node1 receiving a block, it sends out SENDCMPCT messages to some of its peers to update the high-bandwidth status. We need to wait until those are received and processed by the peers to avoid intermittent failures. Before, we'd only wait until all peers have synced with the new block (within `generate`) which is not sufficient.

  I could reproduce the failure by adding a `std::this_thread::sleep_for(std::chrono::milliseconds(1000));` sleep to the [net_processing code](https://github.com/bitcoin/bitcoin/blob/c7567d9223a927a88173ff04eeb4f54a5c02b43d/src/net_processing.cpp#L3763) that processes `NetMsgType::SENDCMPCT`.

ACKs for top commit:
  instagibbs:
    ACK 1ae5b20
  alfonsoromanz:
    Tested ACK 1ae5b20
  glozow:
    ACK 1ae5b20

Tree-SHA512: 47c29616e73a5e0ff966fc231e4f672c1a6892511e5c10a3905b30ad6b2a3d1267fa0a88bd8f64b523fe580199d22a43545c84e361879e5096483152065c4b9a
dd3e0fa build: Fix false positive `CHECK_ATOMIC` test for clang-15 (Hennadii Stepanov)

Pull request description:

  On the master branch @ 0de63b8, a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link:
  ```
    CXXLD    bitcoind
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile<double>::type std::__atomic_impl::load<double>(double const*, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o): in function `void std::__atomic_impl::store<double>(double*, std::remove_volatile<double>::type, std::memory_order)':
  /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store'
  /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o):/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: more undefined references to `__atomic_store' follow
  clang: error: linker command failed with exit code 1 (use -v to see invocation)
  ```

  due to false positive `CHECK_ATOMIC` test in the `configure` script.

  This PR fixes this test.

ACKs for top commit:
  maflcko:
    review ACK dd3e0fa
  fanquake:
    ACK dd3e0fa

Tree-SHA512: b60acf8d83fc84cc3280d95028395d341ed9ed2fcf38ae0a101d50aa19cc35540e9763aa36668c4dc1e1bc7e1f33dbda0e662df39c9e414a284ef91d7fc55fba
…ys in descriptors

24b67fa doc: Add example of mixing private and public keys in descriptors (Anton A)

Pull request description:

  closes: bitcoin#27414

ACKs for top commit:
  achow101:
    ACK 24b67fa
  alfonsoromanz:
    Re ACK 24b67fa

Tree-SHA512: 8c063f23199ac0ff35909f786a5b0de1b4a9b15d1e93bdcdac10cb4bd2002c12e99b6fb1c2e56d16971e7622b67d910b79088429df92c48279be2d7797049911
0244416 security: restrict abis in bitcoind.service (Charlie)

Pull request description:

  [As noted here](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#MemoryDenyWriteExecute=), it's a good idea to pair `MemoryDenyWriteExecute=true` with `SystemCallArchitectures=native` because `MemoryDenyWriteExecute` can be circumvented in some operating systems which support multiple ABIs like x86/x86-64.
  This helps restrict the possible application binary interfaces (ABIs) that can be used when running bitcoind through systemd, reducing the attack surface area.

ACKs for top commit:
  laanwj:
    ACK 0244416 . This is a sensible security feature.
  0xB10C:
    ACK 0244416

Tree-SHA512: 77a35b0674d8d67d857cd20ae1b8cd011f82d6f5ed21bc106cbe45bfa937e786ddc1bf7261e3bdb8c289df1224e91658760905d2c8f37cc4c6506ef8037ad158
f2e3662 net: Decrease nMaxIPs when learning from DNS seeds (laanwj)

Pull request description:

  Limit number of IPs learned from a single DNS seed to 32, to prevent the results from one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be returned.

  Closes bitcoin#16070.

ACKs for top commit:
  Sjors:
    utACK f2e3662
  achow101:
    ACK f2e3662
  1440000bytes:
    utACK bitcoin@f2e3662
  mzumsande:
    utACK f2e3662

Tree-SHA512: 3f108c2baba7adfedb8019daaf60aa00e628b38d3942e1319c7183a4683670be01929ced9e6372c8e983c902e8633f81fbef12d7cdcaadd7f77ed729c1019942
…ed on FreeBSD by default

9381052 doc: Bash is needed in gen_id and is not installed on FreeBSD by default (Hennadii Stepanov)

Pull request description:

  On FreeBSD 14.0, in the `depends` directory:

  - without `bash`:
  ```
  $ gmake print-bdb_build_id_long
  env: bash: No such file or directory
  env: bash: No such file or directory
  bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release
  $ gmake print-final_build_id
  env: bash: No such file or directory
  env: bash: No such file or directory
  final_build_id=722b2d3e264
  ```

  - with `bash`:
  ```
  $ gmake print-bdb_build_id_long
  bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release  1ed47cefe468014c79dedb275cf921f44ab28d91dd56bf94712409b81326d765
  $ gmake print-final_build_id
  final_build_id=7b4f9aaa683
  ```

ACKs for top commit:
  vasild:
    ACK 9381052
  kristapsk:
    ACK 9381052
  alfonsoromanz:
    ACK 9381052

Tree-SHA512: da3f3469ac416518180194f09fb054fb352a2793848fb9a7982439de08244ff6149a7f449ad21fcdf0e9bd79b6949a91751f9cc35833953d2b6a35cea5c6ae21
…NU grep

3bf4f8d lint: scripted-diff verification also requires GNU grep (Sjors Provoost)

Pull request description:

  I noticed while trying to verify all historical `scripted-diff:` commits on macOS that some scripts require GNU sed.

  For example 0d6d2b6 uses `git grep --perl-regexp`.

ACKs for top commit:
  hernanmarino:
    cr ACK 3bf4f8d
  maflcko:
    utACK 3bf4f8d
  achow101:
    ACK 3bf4f8d
  alfonsoromanz:
    Tested ACK 3bf4f8d
  kristapsk:
    cr utACK 3bf4f8d

Tree-SHA512: 09a060ab1bafad03df60d0f20c3dd1451850868dbd66ea38b18178b6230c1f06cf48622db82d9c51422d5689962ee0cd7aae0a31f84bd6d878215e6d73c1d47e
…SetMockTime

fae0db5 refactor: Use chrono type for g_mock_time (MarcoFalke)
fa382d3 test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (MarcoFalke)

Pull request description:

  Seems odd to have the assert in the *deprecated* function, but not in the other.

  Fix this by adding it to the other, and by inlining the deprecated one.

  Also, use chrono type for the global mocktime variable.

ACKs for top commit:
  davidgumberg:
    crACK bitcoin@fae0db5
  stickies-v:
    ACK fae0db5

Tree-SHA512: 630c2917422ff2a7fa307114f95f22ad3c205429ffe36e67f0b2650733e40c876289c1aecebe882a9123d3106db7606bd6eff067ed6e2ecb95765984d3fe8612
…n SunOS / illumos

976e5d8 test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (Hennadii Stepanov)

Pull request description:

  On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined.

  This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead.

  No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems.

  Fixes bitcoin#29884.

  An alternative approach is mentioned in bitcoin#29884 (comment) as well.

ACKs for top commit:
  maflcko:
    lgtm ACK 976e5d8
  theuni:
    ACK 976e5d8. Nice to have the serialization concept actually tested :)

Tree-SHA512: 1033863e584fa8e99a281b236fa01fc919f610a024bcec792116762e28c1c16ee481bd01325c3a0ca9dd9d753176aa63bd9ac7e08a9bbce772db2949d06f6e61
…efiles

7c69baf depends: pass verbose through to cmake based make (Max Edwards)

Pull request description:

  While testing bitcoin#29708 I was not able to enable verbose output to check which flags were being given to the compiler.

  With this PR, running depends with V=1 will enable verbose output from makefiles generated by cmake.

  How to test:

  ```shell
  make -C depends libnatpmp V=1
  ```

ACKs for top commit:
  hebasto:
    ACK 7c69baf. Tested using the folowing command:
  fanquake:
    ACK 7c69baf

Tree-SHA512: 81cd1326e940c5f14cbde96735fd02b03c1150881ed88d1e8dfa9385dfa12284bfa2cdfe097ce5f43a726c1718afb76ae16f71552ab68c207d74fdc1f7bb46ae
…iscover

a68fed1 net: Fix misleading comment for Discover (laanwj)
7766dd2 net: Replace ifname check with IFF_LOOPBACK in Discover (laanwj)

Pull request description:

  Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently.

  Check for the `IFF_LOOPBACK` flag to detect loopback interface instead.

  Also remove a misleading comment in Discover's doc comment.

ACKs for top commit:
  sipa:
    utACK a68fed1
  willcl-ark:
    utACK a68fed1
  theuni:
    utACK a68fed1. Satoshi-era brittleness :)

Tree-SHA512: e2d7fc541f40f6a6af08286e7bcb0873ff55debdcd8b38b03f274897b673a6fb51d84d6c7241a02a9567ddf2645f50231d91bb1f55307ba7c6e68196c29b0edf
…in MempoolPackagesTest

e912717 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi)

Pull request description:

  bitcoin#29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed.

  Add missing comparison for TODO comments in `mempool_packages.py`

  Also, notice that the ancestor size limits and descendant size limits actually implemented in bitcoin#21800   ,  so I removed the todo for those two size limits.

ACKs for top commit:
  kevkevinpal:
    ACK [e912717](bitcoin@e912717)
  achow101:
    ACK e912717
  alfonsoromanz:
    Tested ACK e912717. The code looks good to me and the test execution is successful.
  rkrux:
    tACK [e912717](bitcoin@e912717)

Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
…ng newline, so don't add an extra one

d1ed09a Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (Luke Dashjr)

Pull request description:

  Reviewing bitcoin#29585, I noticed that `bitcoin-qt` adds an extra newline for `-help` and `-version` beyond the other binaries'.

ACKs for top commit:
  hebasto:
    ACK d1ed09a, tested on Ubuntu 24.04.

Tree-SHA512: 15ee9d1060c2492bb3b04a0ac4cb53f7b959bbe32bce415793da0c221f1c963c8f2bb3996ea07d1a7c192bfc2e23be2cd7d4e5649c592eb3fc03906c2763f1aa
@PastaPastaPasta
Copy link
Member Author

dropped all three

@PastaPastaPasta PastaPastaPasta force-pushed the develop-trivial-2024-10-25-pr2 branch from 9c85378 to 700b8c5 Compare October 26, 2024 17:30
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 700b8c5 but pls confirm Guix is happy now before merging

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

utACK 700b8c5

@PastaPastaPasta PastaPastaPasta merged commit f211bb9 into dashpay:develop Oct 27, 2024
35 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.

8 participants