💬 hebasto commented on pull request "[29.x] depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3218125089)
> Guess we can't backport this unless we also backport the removal in Qt. Will take a look.

Indeed. And there's no `configure` option to disable the XCB Xinerama extension.

I assume that backporting https://github.com/qt/qtbase/commit/c91d1fdc100cda88b94217153def52ab7fe63d21 might be non-trivial, and it would definitely require extensive testing across a range of platforms.

While the dependency on this extension ["is annoying/confusing for users"](https://github.com/bitcoin/bitcoin/pull
...

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218145102)
> > This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!
>
> Thank you for your interest in contributing to this project!
>
> The changes you are referring to as completed, i.e. "reordering Guix script commands to perform binary checks after the installation step", have not actually been done yet. See [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh), from line
...

🤔 hebasto reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3149363243)
In a02723427f5f0b142a6ef97b92df093e00ad6004, all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.

💬 hebasto commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218149480)
> ... I tested it locally and it seemed to build successfully...

When a developer tests changes in the Guix scripts locally, they usually posts the resulting binary hashes. For example, as in https://github.com/bitcoin/bitcoin/pull/33217#pullrequestreview-3142128074.

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218151904)
> ### Conflicts
>
> Reviewers, this pull request conflicts with the following ones:
>
> * [#32380](https://github.com/bitcoin/bitcoin/pull/32380) (Modernize use of UTF-8 in Windows code by hebasto)
>
The conflict resolves easily by keeping both changes.

💬 l0rinc commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3218160937)
@hebasto, how can we help with finding a solution?
I personally would be okay with only fixing the English version, if updating the rest is indeed an unsolvable problem.

💬 janb84 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218182511)
> This is my first attempt at contributing to bitcoin core so please, provide me any feedback that you might have. Thanks!

Welcome, please also change this message for a description of the changes made in this PR. Helpful tips are in [creating-the-pull-request](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-the-pull-request)

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218186464)
> In [a027234](https://github.com/bitcoin/bitcoin/commit/a02723427f5f0b142a6ef97b92df093e00ad6004), all checks are still performed on the binaries in the build tree, whereas they are supposed to be performed on the binaries in the installation location.

Thanks for the feedback.

The previous change moved checks after installation but was still using cmake --build targets which check build tree binaries. I've now fixed this to directly call the security and symbol check scripts on the instal
...

💬 surajoibirahim143-ops commented on something "":
(https://github.com/bitcoin/bitcoin/commit/9703b7e6d563ea58f83a6eca819562365404f4ab#commitcomment-164519142)
10000

📝 fjahr opened a pull request: "[29.x] backport #33212"
(https://github.com/bitcoin/bitcoin/pull/33251)
Backports #33212 to 29.x

💬 fanquake commented on pull request "[29.x] backport #33212":
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3218395662)
Can you add the GitHub-Pull / Rebased-From metadata to each commit.

💬 fanquake commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#issuecomment-3218397979)
Backported to 29.x in 33251.

💬 fjahr commented on pull request "[29.x] backport #33212":
(https://github.com/bitcoin/bitcoin/pull/33251#issuecomment-3218400051)
> Can you add the GitHub-Pull / Rebased-From metadata to each commit.

done

💬 ariard commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-3218427453)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Concept ACK.

I'm not going to enter into the polemic about what should be or not the broader consensus process.
During the year of 2022 and 2023, in reaction to the failed CTV activation attempt, I did launch
and maintain for roughly a year the bitcoin contracting WG on IRC open to everyone, as a transparent
forum to discuss consensus changes, in the goal to make real and step-by-step progress among all the
domain experts.

While, I d
...

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3218539977)
My Guix build resulting binary hashes:

`75d4eb10b95a64c595ee1ddf31d3c1e0086958a1ae3d64f9e2bbc4930cd88366 dist-archive/bitcoin-e922e27a2ecd.tar.gz
f7d2d6cd820561229ab5fa476c06484ced81a1fa8b90d2fbbd7a82fa915aa956 x86_64-linux-gnu/bitcoin-e922e27a2ecd-x86_64-linux-gnu-debug.tar.gz
df24c2269cf96dbf011692fbc0b21dab7786abd43dbb82e4351f0117e95169e5 x86_64-linux-gnu/bitcoin-e922e27a2ecd-x86_64-linux-gnu.tar.gz`

📝 frankomosh opened a pull request: "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`"
(https://github.com/bitcoin/bitcoin/pull/33252)
Adds a fuzz test for the [`DifferenceFormatter`](https://github.com/bitcoin/bitcoin/blob/e3f416dbf7633b2fb19c933e5508bd231cc7e9cf/src/blockencodings.h#L22-L42) (used in [`BlockTransactionsRequest`](https://github.com/bitcoin/bitcoin/blob/master/src/blockencodings.h#L44-L54), [BIP 152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki)). The DifferenceFormatter class implements differential encoding for compact block transactions (BIP 152). This PR ensures that its strictly-monotonic
...

📝 frankomosh converted_to_draft a pull request: "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`"
(https://github.com/bitcoin/bitcoin/pull/33252)
Adds a fuzz test for the [`DifferenceFormatter`](https://github.com/bitcoin/bitcoin/blob/e3f416dbf7633b2fb19c933e5508bd231cc7e9cf/src/blockencodings.h#L22-L42) (used in [`BlockTransactionsRequest`](https://github.com/bitcoin/bitcoin/blob/master/src/blockencodings.h#L44-L54), [BIP 152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki)). The DifferenceFormatter class implements differential encoding for compact block transactions (BIP 152). This PR ensures that its strictly-monotonic
...

🤔 l0rinc requested changes to a pull request: "net: Prevent node from binding to the same `CService`"
(https://github.com/bitcoin/bitcoin/pull/33231#pullrequestreview-3149500267)
Concept ACK

I personally would not try to correct the behavior (especially silently), given that it's rare, I would simply give the user a warning or error and let them fix the arguments.

I also think the tests are a bit complicated and more stateful now, I have added a few suggestions, maybe we can simplify based on them.

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296803517)
output parameters are quite hard to reason about - given that the call-site assigns `connOptions` directly, can we either split this into 2-3 helper methods that return the new values (the vectors are tiny, they're likely as fast as passing it around), or if you insist on this big one, can we pass a single `CConnman::Options& connOptions` instead?
Also, some branches return, others don't erase, others replace one of the inputs - seems like has more than a single, easily defined responsibility (
...

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296801852)
nit:
```suggestion
if (whitebind_services.contains(bind)) {
```