💬 cedwies commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3206161232)
ACK 2885bd0

The PR adjusts the -datacarrier/-datacarriersize deprecation warning to be less absolute and better match the release notes.
I think the new wording still communicates deprecation, but without overstating certainty about removal.
Code change is minimal and the functional test was updated accordingly.

💬 fanquake commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#issuecomment-3206177685)
Guix Build (aarch64):
```bash
ccc8209475974ddef8da13c2b4d09c3798ed26f1f8ca64bfad5cbaca858af1c0 guix-build-0022e25333a8/output/aarch64-linux-gnu/SHA256SUMS.part
4f1cc05dcf87206cd2e6dd601098c0df5518bbc347b9b42a6dbadbd45a24dbde guix-build-0022e25333a8/output/aarch64-linux-gnu/bitcoin-0022e25333a8-aarch64-linux-gnu-debug.tar.gz
053fba6c3981b92e9c7af91149d49cae6a1f10d1e8c431e90cdde47eb1d0bab8 guix-build-0022e25333a8/output/aarch64-linux-gnu/bitcoin-0022e25333a8-aarch64-linux-gnu.tar.gz
8dfde4
...

💬 josibake commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3206209759)
Thanks for taking the suggestion of splitting this up! I think it would be nice to add a blurb to the description that this PR depends on https://github.com/bitcoin/bitcoin/pull/33216 and is in draft pending review on the parent PR. You mention this in the comments, but having it in the description ensures reviewers dont miss it / get confused on the status of the PR.

👍 instagibbs approved a pull request: "rpc: followups for 33106"
(https://github.com/bitcoin/bitcoin/pull/33189#pullrequestreview-3136380951)
ACK daa40a3ff97346face9dcc64564010a66c91ccb2

💬 instagibbs commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2288015854)
personal opinion only but printing 8 places always to easily eyeball the rate is better, so -0 on this suggestion

💬 instagibbs commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2288034462)
small note with explanation on how miners can use it would be nice since some miners are opting out of mining such low feerates for now

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206253497)
> My concern is not the fact that there are added binaries, but that we force people to use a different one (bitcoin-node, even if it's not visible) if they want the mining IPC but don't care about multiprocess - resulting in them using an approach that isn't tested/looked at/used by anyone else.

To be clear, this does not sound like a concern currently, but would be a concern if future PR's do something we **both** agree would be bad: automatically opt users into the process separation featu
...

💬 instagibbs commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#issuecomment-3206261504)
utACK 0022e25333a8eabf79c0341f94cf06db36e32f4f

re-applied all commits and only difference is release notes change location

💬 stickies-v commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2288145855)
+1, small change that might turn out helpful during debugging

👍 stickies-v approved a pull request: "[29.x] Backport logging ratelimiting"
(https://github.com/bitcoin/bitcoin/pull/33225#pullrequestreview-3136573025)
ACK 0022e25333a8eabf79c0341f94cf06db36e32f4f - all backports clean except the release notes one, as indicated.

Can't see any weird interactions with the 29.x branch.

📝 glozow opened a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226)
Backports #33106 and includes final changes for 29.1rc2. Built on top of #33225. I'll rebase shortly after that's in.

I did not include #32750 because it causes #33177 and I don't foresee any problems; it was just a nice to have.
For reviewers: the backport is unclean but fairly straightforward. I just had to adapt a test that is no longer in master (#32973) and include `-datacarriersize` in order to pad transaction size (#32406).

🚀 glozow merged a pull request: "[29.x] Backport logging ratelimiting"
(https://github.com/bitcoin/bitcoin/pull/33225)

🤔 enirox001 reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3136742267)
re-ACK be776a1

💬 luke-jr commented on pull request "depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33217#issuecomment-3206473104)
> (only used for windowing over multiple screens support?)

Deprecated by and incompatible with XRandR which was introduced in ~2001.

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2288193651)
In commit "ci: add functional test for IPC interface" (088dc2c486af0ad6803d919d8114ab29d3cd652f)

As mentioned https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2283314830, would be curious to know if force-system-libcapnp=True works for people, since it seems wasteful to download and build a dependency that we already know must be installed (or the test would be skipped).

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2288166616)
In commit "test: add is_ipc_enabled() and skip_if_no_ipc() functions" (eae32d1a9b5c62707de6afd9832d40de859dd42d)

Not important, but might be better to rename enabled to compiled here to be more similar to surrounding functions which check ENABLE_XXX but are called is_xxx_compiled. (I think your PR actually did this originally but I didn't notice the pattern it was following)

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2288233217)
In commit "ci: add functional test for IPC interface" (088dc2c486af0ad6803d919d8114ab29d3cd652f)

I might describe this as # Create a remote thread on the server for the IPC calls to be executed in

👍 ryanofsky approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3136603264)
Code review ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f. Looks great! Left some comments that you should feel free to ignore. I am shocked and amazed about how much more friendly the python async API is compared to the c++ API, but I guess coroutines can bring similar benefits to c++. It is also nice how this tests covers different scenarios with waitNext()

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2288203120)
In commit "ci: add functional test for IPC interface" (088dc2c486af0ad6803d919d8114ab29d3cd652f)

Not important, but for easier review and to avoid conflicts in the CI files, it may make sense to split this commit into two commits: one which adds the interface_ipc.py test and allows running it on systems with pycapnp installed, and another commit before or after that installs pycapnp in CI.

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2288227488)
In commit "ci: add functional test for IPC interface" (088dc2c486af0ad6803d919d8114ab29d3cd652f)

This is probably fine in practice, but it does seem possible for this to fail if the `capnp` binary isn't on the path because `shutil.which` will return None. It might be better to write this as:

````python
if capnp_bin := shutil.which("capnp"):
# Add the system cap'nproto path so include/capnp/c++.capnp can be found.
capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
...