💬 151henry151 commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3503646384)
> Sorry, but one more rebase is needed.

I think I've rebased correctly here -- I removed an additional `SKIP_BUILD_RPATH OFF` override that seemed like it would no longer be needed, https://github.com/bitcoin/bitcoin/commit/a02282d20c87471a3399e4061c7edad7ecdb391f#diff-2a07660db8ebdbbf967cf42f6b4da3ffc8558a322f45a3a08ce292c9e91767b8L13

Guix build for a02282d20c87:
a3647a46e00124b207263b695d9598cf9fe6c845ea9072c933462dd0e31a0ed9 guix-build-a02282d20c87/output/aarch64-linux-gnu/SHA256SUMS
...

🤔 hebasto reviewed a pull request: "refactor: Add missing include in bitcoinkernel_wrapper.h"
(https://github.com/bitcoin/bitcoin/pull/33825#pullrequestreview-3438418410)
ACK fa1e8d8bad92f5fba2b086d78581df4c8123b098.

[Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on https://github.com/bitcoin/bitcoin/pull/33810:
```diff
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -5,12 +5,19 @@
#ifndef BITCOIN_KERNEL_BITCOINKERNEL_WRAPPER_H
#define BITCOIN_KERNEL_BITCOINKERNEL_WRAPPER_H

-#include <kern
...

💬 l0rinc commented on pull request "scripted-diff: Remove obsolete comment":
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3506770544)
code review ACK 36724205fc1f226d7b5493ed0212c336e7f2ae84

Looks like this is a leftover from [before the cmake migration](https://github.com/bitcoin/bitcoin/commit/6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c#diff-7533d479b9e3ad56dcfbf6daaa21317373e8233da5ea13714554115c66057aa2L22-L132).
Checked locally, other Python 3 references seem unrelated.

💬 maflcko commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511768897)
> [Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on #33810:

I can't find that in the CI output. I don't think iwyu runs on stand-alone headers without a cpp file?

💬 hebasto commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511843037)
> > [Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on #33810:
>
> I can't find that in the CI output. I don't think iwyu runs on stand-alone headers without a cpp file?

It's here: https://github.com/hebasto/bitcoin/actions/runs/19194191546/job/54872770781:
```
<snip>
2025-11-08T14:21:35.1873049Z (/home/runner/work/_temp/src/kernel/bitcoinkernel_wrapper.h
...

💬 l0rinc commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3521439654)
> Since when is this "unused"?

Did some digging, https://github.com/bitcoin/bitcoin/commit/1f0e7ca09c9d7c5787c218156fa5096a1bdf2ea8#diff-706988c23877f8a557484053887f932b2cafb3b5998b50497ce7ff8118ac85a3L135-L165 inlined `ComputeMerkleRoot` in one case, leaving the `proot` and `pmutated` values unused in `MerkleComputation` (cc: @sipa, @Sjors).
And `BlockWitnessMerkleRoot` was introduced in https://github.com/bitcoin/bitcoin/commit/8b49040854be2e26b66366aeae1cba4716f93d93 where it was already
...

💬 rkrux commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2527400928)
In d1a2e278572f70d0d68a21312b2cc7de27f326df "test: replace class ValidWitnessMalleatedTx with function"

Unlike the `malleate_tx_to_invalid_witness` function that is quite generic (https://github.com/bitcoin/bitcoin/pull/33793/files#diff-1698fabd4ddd298eb28e234d97f34e744f568852b77361384c8edb38819de5e3R259), this function at the moment doesn't seem generic enough to be present in an util file. Few points regarding the function I noticed:

1. The name doesn't reflect the implementation, the fu
...

💬 l0rinc commented on pull request "refactor: remove incorrect lifetimebounds":
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2534940354)
Good find, this parameter was added in https://github.com/bitcoin/bitcoin/pull/27607/files#diff-ed3f90693a242b38b9719af171de8f55183576957676dfa358945bea22276bd5R232 where [`lower_block`](https://github.com/bitcoin/bitcoin/pull/27607/files#diff-114c2880ec1ff2c5293ac65ceda0637bf92c05745b74b58410585a549464a33fR413) was already a possible return value (cc: @furszy, @ryanofsky, @TheCharlatan, @mzumsande)

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2542843754)
I don't have Windows, it's why I asked if instead of adding a new method we can just delete the conversions, somewhat similarly to the mentioned https://github.com/bitcoin/bitcoin/pull/33550/files#diff-69423eb01bf14b3bd0d930c0b3e1fd6f4f061ffefacab579053eaa734fc22f38R65 (since the error seemed superficially similar to me).

👍 l0rinc approved a pull request: "qa: Account for unset errno in ConnectionResetError"
(https://github.com/bitcoin/bitcoin/pull/33875#pullrequestreview-3484132142)
untested code review ACK 76e0e6087d0310ec31f43d751de60adf0c0a2faa

This is a continuation of https://github.com/bitcoin/bitcoin/pull/30660/files#diff-7f22a082e3a80ca2f212e36193f87dd80237035afedb7f15b116ef7fa265f3eeR326, separating the existing `TimeoutError` to add extra details for `ConnectionResetError` as well.

💬 fanquake commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3557317662)
> This [branch](https://github.com/hebasto/bitcoin/tree/251119-qt6.9) fails to cross-compile on Debian Bookworm with GCC 12.2.0:

That's the same issue we work around in CMake, which is fixed by just turning off stack-clash-protection: https://github.com/fanquake/bitcoin/tree/qt_6_9_no_stack_clash_win. However it looks like there are other issues, possibly due to https://github.com/qt/qtbase/commit/d25d9f2c2673bc287590d9a83bd7ef1357d7021a#diff-4ae1a340d7e79a6402009cb635708b4950b1633f4bc850aba4
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2549698974)
I also found it confusing at first, but it's a branchless structure that we're using in a [few](https://github.com/bitcoin/bitcoin/blob/1fe851a4781a6af6264f1ba9d93c3281b59a6cac/src/net_processing.cpp#L1632) [other](https://github.com/bitcoin/bitcoin/blob/fa9ca13f35be0a023aeed78775ad66f95717b28b/src/util/feefrac.h#L75) [places](https://github.com/bitcoin/bitcoin/pull/33512/files#diff-3d0856e8b7f136c588b229e0cbd3b2e2c309cd097ade0879521daba4e1bb2a33R684) [already](https://github.com/bitcoin/bitcoin
...

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2549969970)
is this removal related to the mac cleanup?
it seems this was before the Mac section originally https://github.com/bitcoin/bitcoin/commit/33dd764984def9371f324d3add19ee894a0260bf#diff-7ff2e1d1ed59fa96766a663236d71bfdd00b4af3321fde39f7dbf022bd968a0aL87-L88

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2551029734)
nit: With the latest push (https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c#diff-d6d633592a40f5f3d8b03863e41547de8751b874c1d20f129a616b9dd719b999), we are testing the same thing on two lines.

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3576053570)
Rebased after https://github.com/bitcoin/bitcoin/commit/4f65a1c5db84426b9d814a18f63ac46632691ab2#diff-f14e9423bd43609969886b2e42751c55606344e0b71780564cdb23515802cec0R12, ready for review again.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3581723693)
`ade7fcb6a0...12dfb1cb7c`: use `std::ranges::max_element()` instead of manual find, inspired by https://github.com/l0rinc/bitcoin/commit/d42c922e9256625b166ab388ff234ba9bc272233#diff-ad1d5d83183a5facaf06e2b44c42a3e942a261b7506576720588bf6b158e4c3dR37

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2568598322)
We’re [moving towards Flush not returning a value](https://github.com/bitcoin/bitcoin/pull/33866/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR257) – can we avoid adding new code that relies on its return? Not sure it’s possible before that other PR lands, but worth keeping in mind.

💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2603384853)
Updated the last commit: [`1b810390fe51b79c7063966c9722c01cb8a4f7bf..6eb5ba569141b722a5e27ef36e04994886769c44`#diff-70a1abe85d](https://github.com/bitcoin/bitcoin/compare/1b810390fe51b79c7063966c9722c01cb8a4f7bf..6eb5ba569141b722a5e27ef36e04994886769c44#diff-70a1abe85d9d9fa8729884b20556bcf4f8ce94f50a051df85cccd6b9ca873078)

💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2603387114)
Done throughout the class, thanks: [`1b810390fe51b79c7063966c9722c01cb8a4f7bf..6eb5ba569141b722a5e27ef36e04994886769c44`#diff-66e9563fc5](https://github.com/bitcoin/bitcoin/compare/1b810390fe51b79c7063966c9722c01cb8a4f7bf..6eb5ba569141b722a5e27ef36e04994886769c44#diff-66e9563fc5032b8ac0ab910034faadf65cbbb282bee6c46d5c92134cb86c66fd)

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2607004069)
8c990cc2f5a7e93a5debb7eaf20e7f2c887df70d

to make sure I'm understanding, the two chunks being chosen to be merged are described here, correct? https://github.com/bitcoin/bitcoin/pull/32545/commits/8c990cc2f5a7e93a5debb7eaf20e7f2c887df70d#diff-1433c1fc4926a466291656ba67cf6b029523e4bd5da177ade812f25edf07343cR543

top or bottom chunk along with highest/lowest feerate difference which is maximally different

in effect the new info here is just "uniformly random"