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.

Sign up for GitHub

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

Jump to bottom

backport: trivial 2024 10 23 pr2 #6346

Merged
merged 15 commits into from
Oct 24, 2024
Merged

backport: trivial 2024 10 23 pr2 #6346

merged 15 commits into from
Oct 24, 2024

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-pr2 branch 2 times, most recently from c35e47e to 3eba0e6 Compare October 24, 2024 01:04
fanquake and others added 15 commits October 23, 2024 22:19
@fanquake @PastaPastaPasta
Merge bitcoin#28902: doc: Simplify guix install doc, after 1.4 release
dfd53da 
fa552e8 doc: Simplify guix install doc, after 1.4 release (MarcoFalke)

Pull request description:

  Now that 1.4 is out (for a while), remove the recommendation to build a random commit.

ACKs for top commit:
  fanquake:
    ACK fa552e8
  hebasto:
    ACK fa552e8.

Tree-SHA512: f5642df201ff0e2af8a7ae9660a66920ddbb5f522b3e921f6f4aa7c411ced23afa91bdfe43b943ac012228eebbaad3396df505d00aa8f721a4358f03fda9d8e3
@fanquake @PastaPastaPasta
Merge bitcoin#28913: coins: make sure PoolAllocator uses the correct …
02741a7 
…alignment

d5b4c0b pool: change memusage_test to use int64_t, add allocation check (Martin Leitner-Ankerl)
ce881bf pool: make sure PoolAllocator uses the correct alignment (Martin Leitner-Ankerl)

Pull request description:

  The class `CTxOut` has a member `CAmount` which is an int64_t, and on ARM 32bit int64_t are 8 byte aligned, which is larger than the pointer alignment of 4 bytes.

  So for `CCoinsMap` to be able to use the pool, we need to use the alignment of the member instead of just `alignof(void*)`.

  This fixes bitcoin#28906 (first noted in bitcoin#28718 (comment)) and bitcoin#28440.

ACKs for top commit:
  pinheadmz:
    ACK d5b4c0b
  hebasto:
    re-ACK d5b4c0b, the only change since my recent [review](bitcoin#28913 (review)) is an updated test.
  theStack:
    Tested ACK d5b4c0b

Tree-SHA512: 4446793fad6d56f0fe22e09ac9ade051e86de11ac039cd61c0f6b7f79874242878a6a46a2c76ac3b8f1d53464872620d39139f54b1471daccad26d6bb1ae8ca1
@fanquake @PastaPastaPasta
Merge bitcoin#26077: guix: switch from guix environment to `guix sh…
fd2e88d 
…ell`

66c4b58 guix: switch from guix environment to guix shell (fanquake)

Pull request description:

  See https://guix.gnu.org/manual/devel/en/html_node/Invoking-guix-environment.html.

  > Deprecation warning: The guix environment command is deprecated
  in favor of guix shell, which performs similar functions but is more convenient to use. See Invoking guix shell.

  > Being deprecated, guix environment is slated for eventual removal,
  but the Guix project is committed to keeping it until May 1st, 2023. Please get in touch with us at [email protected] if you would like to discuss it.

  See also https://guix.gnu.org/blog/2021/from-guix-environment-to-guix-shell/ for a blog post and additional details.

  Guix `shell` was added to Guix ~1 year ago, in this commit, https://git.savannah.gnu.org/cgit/guix.git/commit/?id=80edb7df6586464aa40e84e103f0045452de95db, which isn't part of the 1.3.0 release binaries out of the box, but invoking a `guix pull`, and updating will make it available. i.e:
  ```bash
  bash-5.1# guix --version
  guix (GNU Guix) 1.3.0
  Copyright (C) 2021 the Guix authors
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.

  bash-5.1# guix shell
  guix: shell: command not found
  Try 'guix --help' for more information.

  bash-5.1# guix pull
  Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
  Authenticating channel 'guix', commits 9edb3f6 to 7a980bb (6,278 new commits)...
  Building from this channel:
    guix      https://git.savannah.gnu.org/git/guix.git7a980bb
  < snip >
  building /gnu/store/2wwwsczxcw61m05p4mv0kf0advx4fqsb-inferior-script.scm.drv...
  building package cache...
  building profile with 1 package...
  New in this revision:
    6,866 new packages: a2jmidid, abjad,

  bash-5.1# guix help shell
  Usage: guix shell [OPTION] PACKAGES... [-- COMMAND...]
  Build an environment that includes PACKAGES and execute COMMAND or an
  interactive shell in that environment.
  ```

ACKs for top commit:
  TheCharlatan:
    ACK 66c4b58

Tree-SHA512: caa3fd2ca8d0f261c50ecdda3728a75389d24d89b51293dedc704ee77ab1342b2bb08ca8c871dcb4646229f056ec86cb15500934ded1b0c501a3ffc25aaa8ae6
@ryanofsky @PastaPastaPasta
Merge bitcoin#28852: script, assumeutxo: Enhance validations in utxo_…
8cd85d3 
…snapshot.sh

11b7269 script: Enhance validations in utxo_snapshot.sh (pablomartin4btc)

Pull request description:

  This PR resolves bitcoin#27841 and some more:

  - Ensure that the snapshot height is higher than the pruned block height when the node is pruned (Suggested by @Sjors [here](bitcoin#28553 (comment))).

  - Validate the correctness of the file path and check if the file already exists (@hazeycode's [bitcoin#27845](bitcoin#27845)).

  - Make network activity disablement optional for the user (Suggested by @Sjors [here](bitcoin#16899 (comment)) and [here](bitcoin#16899 (comment))).

  - Ensure the `reconsiderblock` command is triggered on exit (@hazeycode's same PR as above), even in the case of user interruption (Ctrl-C).

  In order to perform some testing please follow the instructions in the description of previous @hazeycode's PR bitcoin#27845.

ACKs for top commit:
  Sjors:
    tACK 11b7269
  ryanofsky:
    Code review ACK 11b7269

Tree-SHA512: 2b699894c6f732ad5104f5a2bcf5dc86ed31edcc9d664690cab55b94a8ab00e2ca5bde901ee1d63acddca7ea80ad1734d8cfe78f9c02f8470f264fe93a2af759
@achow101 @PastaPastaPasta
Merge bitcoin#28989: test: Fix test by checking the actual exception …
75e0334 
…instance

55e3dc3 test: Fix test by checking the actual exception instance (Hennadii Stepanov)

Pull request description:

  The `system_tests/run_command` test is broken because it passes even with the diff as follows:
  ```diff
  --- a/src/test/system_tests.cpp
  +++ b/src/test/system_tests.cpp
  @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(run_command)
           });
       }
       {
  -        BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON
  +        BOOST_REQUIRE_THROW(RunCommandParseJSON("invalid_command \"{\""), std::runtime_error); // Unable to parse JSON
       }
       // Test std::in, except for Windows
   #ifndef WIN32
  ```

  The reason of such fragility is that the [`BOOST_REQUIRE_THROW`](https://www.boost.org/doc/libs/1_83_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level_throw.html) macro passes even if the command raises an exception in the underlying subprocess implementation, which might have a type derived from `std::runtime_error`.

ACKs for top commit:
  maflcko:
    lgtm ACK 55e3dc3
  achow101:
    ACK 55e3dc3
  furszy:
    Non-Windows code ACK 55e3dc3
  pablomartin4btc:
    ACK 55e3dc3

Tree-SHA512: 32f49421bdcc94744c81e82dc10cfa02e3f8ed111974edf1c2a47bdaeb56d7baec1bede67301cc89464fba613029ecb131dedc6bc5948777ab52f0f12df8bfe9
@fanquake @PastaPastaPasta
Merge bitcoin#29068: test: Actually fail when a python unit test fails
e6f19e7 
fa0534d test: Actually fail when a python unit test fails (MarcoFalke)

Pull request description:

  Currently python unit test failures are ignored.

  Fix this.

ACKs for top commit:
  theStack:
    ACK fa0534d
  BrandonOdiwuor:
    ACK fa0534d

Tree-SHA512: c136be4c8d861d966f380e04d5d14b711b90c4011101302dae1332496e493207c5c673927586ed35b02b61a0b050bf45053a31e6ff766ec52f1d054caf0985e2
@fanquake @PastaPastaPasta
Merge bitcoin#28844: contrib: drop GCC MAX_VERSION to 4.3.0 in symbol…
c70ff5d 
…-check

ff896d2 contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check (fanquake)

Pull request description:

  Reflect the actual symbols used, i.e:

  ```bash
  bitcoind: symbol __bswapsi2 from unsupported version GCC_4.3.0(7)
  ```

ACKs for top commit:
  TheCharlatan:
    ACK ff896d2

Tree-SHA512: b38ff8f4dd78d2d1c9063c53544dc4f240c3043f142e1581f7ba42f088a509293f6f17cc402c60ac82bff3b36668866b87e0e9e4d10d929484bb4c7a3e654f25
@fanquake @PastaPastaPasta
Merge bitcoin#28962: doc: Rework guix docs after 1.4 release
97012ea 
fad444f doc: Rework guix docs after 1.4 release (MarcoFalke)

Pull request description:

  Follow-up to bitcoin#28902

  Fixes bitcoin#28957

ACKs for top commit:
  TheCharlatan:
    ACK fad444f
  fanquake:
    ACK fad444f

Tree-SHA512: 23f270b438ede4e3173da68e63c1d022e2ef23bfd83f0ec038ec63a62348038722278385c5dac63ac29a460b4b61f23d8c9939667e00a1a3571b041d3eecb4cb
@fanquake @PastaPastaPasta
Merge bitcoin#29172: fuzz: set nMaxOutboundLimit in connman target
4cdd1a8 
e5b9ee0 fuzz: set `nMaxOutboundLimit` in connman target (brunoerg)

Pull request description:

  Setting `nMaxOutboundLimit` (`-maxuploadtarget`) will make fuzz to reach more coverage in connman target. This value is used in `GetMaxOutboundTimeLeftInCycle`, `OutboundTargetReached` and `GetOutboundTargetBytesLeft`.

ACKs for top commit:
  dergoegge:
    utACK e5b9ee0
  jonatack:
    ACK e5b9ee0

Tree-SHA512: d19c83602b0a487e6da0e3be539aa2abc95b8bbf36cf9a3e391a4af53b959f68ca38548a96d27d56742e3b772f648da04e2bf8973dfc0ab1cdabf4f2e8d44de6
@fanquake @PastaPastaPasta
Merge bitcoin#29200: net: create I2P sessions using both ECIES-X25519…
df42d41 
… and ElGamal encryption

9d72891 net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack)

Pull request description:

  A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type.  Encryption type is a property of the session, not the destination.  Sessions may support multiple encryption types.

  As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0).

  This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred.

  See also:

  - discussion around qbittorrent/qBittorrent#19625 (comment)
  - recently updated "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3

  Thank you and credit to zzzi2p for reporting and to vort for the patch.

  Closes bitcoin#29197.

ACKs for top commit:
  zzzi2p:
    ACK 9d72891
  recursive-rat4:
    ACK 9d72891
  kristapsk:
    cr utACK 9d72891
  brunoerg:
    crACK 9d72891
  shaavan:
    crACK 9d72891

Tree-SHA512: 0912fc01af9706914a7854f7479b9d82fc86c9530466cad8674e30f7eb4894d90d514efbc1aee8b7ea690faa6ff4a23b62cf5de8737cffdbc463300082c9b917
@achow101 @PastaPastaPasta
Merge bitcoin#29211: fuzz: fix connman initialization
b091329 
e84dc36 fuzz: fix `connman` initialization (brunoerg)

Pull request description:

  Fixes bitcoin#29172 (comment)

ACKs for top commit:
  achow101:
    ACK e84dc36

Tree-SHA512: e5f3c378cfe367cc4c387fa1b13663a74d8b667a5d130d62919e21455861cfb9383b63ef4ebe56daab7b2c09e3b5031acc463065455f71607c5fb9e3c370d3ad
@fanquake @PastaPastaPasta
Merge bitcoin#29219: fuzz: Improve fuzzing stability for ellswift_rou…
2e41562 
…ndtrip harness

154fcce [fuzz] Improve fuzzing stability for ellswift_roundtrip harness (dergoegge)

Pull request description:

  See bitcoin#29018

ACKs for top commit:
  sipa:
    utACK 154fcce
  brunoerg:
    crACK 154fcce

Tree-SHA512: 1e1ee47467a4a0d3a4e79f672018b440d8b3ccafba7428d37b9d0b8d3afd07e3f64f53ee668ed8a6a9ad1919422b5970814eaf857890acae7546951d8cb141d6
@fanquake @PastaPastaPasta
Merge bitcoin#28870: depends: Include config.guess and config.sub…
da371b8 
… into `meta_depends`

ff3f51b depends: Include `config.guess` and `config.sub` into `meta_depends` (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  theuni:
    ACK ff3f51b.

Tree-SHA512: e8575473d3fca2293181131c76bd6d43017fe753d2e670c53227a646b64b069dc542a0fc50a77b43e74bc6a0c0159ffa2fb1c3ff3aef9625684e0f78c16ad960
@hebasto @PastaPastaPasta
Merge bitcoin-core/gui#789: Avoid non-self-contained Windows header
2a77808 
8023640 qt: Avoid non-self-contained Windows header (Hennadii Stepanov)

Pull request description:

  Using the `windows.h` header guarantees correctness regardless of the content of other headers.

  For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio

  Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager.

  Related to hebasto#77.

ACKs for top commit:
  theuni:
    ACK 8023640. It's not completely clear to me why this currently works, but I don't think it's worth wasting more time on. `windows.h` seems more correct regardless.

Tree-SHA512: 1c03f909943111fb2663f86d33ec9a947bc5903819e5bd94f436f6b0782d9f5c5d80d9cd3490674ecd8921b2981c509e97e41580bccc436f8b5c7db84b4e493c
@glozow @PastaPastaPasta
Merge bitcoin#29308: doc: update BroadcastTransaction comment
8bf1d06 
31cce4a doc: update `BroadcastTransaction` comment (ismaelsadeeq)

Pull request description:

  `BroadcastTransaction` is also called by `submitpackage` RPC.

  All transactions that are accepted into the mempool post package processing are broadcasted to peers individually here
  https://github.com/bitcoin/bitcoin/blob/ea4ddd8652d9dd1e7698e2a6f84c606cf24a2e3e/src/rpc/mempool.cpp#L926

  It's not maintainable to list all the callers of a function.

ACKs for top commit:
  stickies-v:
    ACK 31cce4a
  kristapsk:
    ACK 31cce4a
  naumenkogs:
    ACK 31cce4a

Tree-SHA512: 8aea92c53c1911a0ac36fe9e3a24d37d83e7d9b40a16f0832bfa7a719328697621e3f94a5dc80d1840e7ae705e0c3aab7a3df7064986e1e53a4a4114adf078a8
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please rebase.

UdjinM6
UdjinM6 approved these changes Oct 24, 2024
View reviewed changes
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 8bf1d06

knst
knst approved these changes Oct 24, 2024
View reviewed changes
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.

utACK 8bf1d06

contrib/guix/INSTALL.md
when you're reading this is still 1.3.0 then you may want to use 998eda30 instead
to avoid the issues described in [#25099](
https://github.com/bitcoin/bitcoin/pull/25099).
You will likely want to build the latest release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

28902: I just noticed that we didn't dashify tags names here, let's add fix for it?

see 2 lines below:

-git branch -a -l 'origin/version-*' # check for the latest release
+git branch -a -l 'origin/v*' # check for the latest release

@PastaPastaPasta PastaPastaPasta merged commit a673193 into dashpay:develop Oct 24, 2024
11 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knst knst knst approved these changes

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@kwvg kwvg Awaiting requested review from kwvg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
22
Development

Successfully merging this pull request may close these issues.
8 participants
@PastaPastaPasta @knst @UdjinM6 @fanquake @ryanofsky @achow101 @hebasto @glozow