💬 cedwies commented on pull request "Avoid file overwriting in fallback `AllocateFileRange` implementation":
(https://github.com/bitcoin/bitcoin/pull/33164#issuecomment-3201401890)
Clarification: The doc for `AllocateFileRange` says: “the range specified … will never contain live data.” But in reality some call paths (during reindex on a few OSes) did pass ranges that overlapped live bytes? The old fallback then zero‑wrote inside the file and corrupted data. This PR makes the fallback defensive by appending after EOF only, so even if a caller violates the promise, we still don’t overwrite (?).

But what about the guarantee from the doc of `AllocateFileRange`? Should we d
...

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201416196)
> I think this is an undesirable situation - shipping an entire new replacement set of binaries that only get eyes/practice from a tiny subset of developers and users.

Thanks for being specific about the negative outcome you see here.

I think I might not understand the significance of the situation you're describing though. The only difference between the binary that supports the `-ipcbind` option and the binary that does not support it is that one binary is linked against [`init/bitcoind.
...

💬 Crypt-iQ commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3201508089)
Latest push ab5c3be -> 5dda364 adds the diff referenced in [this comment](https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3131772733) so that test failures are more informative. No other changes.

💬 ryanofsky commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3201512895)
Note: achow101 commented against enabling ENABLE_IPC by default in https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3129705803 and sipa commented in favor in https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201020041, so more discussion about this is happening in the other PR.

I think the difference comes down to short vs. long term perspective. In the short term it doesn't make a huge amount of sense to turn this one feature on by default while comparable features a
...

💬 zaidmstrr commented on pull request "test: p2p block malleability":
(https://github.com/bitcoin/bitcoin/pull/33172#discussion_r2285844408)
nit: you can change this comment to `Confirm the block is still relayable by weight`, As this is not checking whether a block is relayable its just checking weight.

👍 Sjors approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3133140212)
ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f (after base PR lands)

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285845245)
088dc2c486af0ad6803d919d8114ab29d3cd652f: nit if you have to retouch: these comments are useful as `self.log.debug()`

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285836684)
088dc2c486af0ad6803d919d8114ab29d3cd652f: eventually I'd like to move this and `load_capnp_modules` into the test framework so that it's easier to expand e.g. `mining_template_verification.py` to call both `getblocktemplate proposal` and `checkBlock()`.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3201535092)
Addressed a number of comments, and rebased on top of #31802.

I will however not have (much) time to push this further forward until the end of this month. @Sjors or @ryanofsky, or someone else, feel free to take this over and/or integrate into other PRs.

📝 ismaelsadeeq opened a pull request: "mini miner: enable `Linearize` return package feerates"
(https://github.com/bitcoin/bitcoin/pull/33216)
This PR adds supports for mini miner to return the package fee rates of the linearized block templates #30079

Motivated by https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3191510709

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853200)
Changed to print to stdout, seems to work.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853964)
Fixed, see elsewhere.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285854554)
Added the ignore annotation.

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201559444)
> Feel free to ignore me if this PR is close to merge, but it does feel like there are two PRs here. The first 3 commits seem more about updates to miniminer, and then the 4th commit does what the title of this PR indicates.
>
> It may help to attract more review by breaking out the first 3 commits into their own PR, using this PR as a motivating use case. Either way, I plan to dig in more next week and review, but wanted to mention this.

Not close to merge I think, so far this PR received
...

📝 ismaelsadeeq converted_to_draft a pull request: "Fee Estimation: Ignore all transactions that are CPFP'd"
(https://github.com/bitcoin/bitcoin/pull/30079)
Another attempt at #25380 with an alternate approach

This PR updates `CBlockPolicyEstimator` to ignore all transactions that are CPFP'd by some child when a new block is received.
This fixes the assumption that all transactions are confirmed solely because of their fee rate.
As some transactions are confirmed due to a CPFP by some child.



This approach linearize all transactions removed from the mempool because of the new block, and only ignore transactions whose mining score is not
...

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201582620)
Putting in draft because of a dependent PR.

📝 fanquake opened a pull request: "depends: remove xinerama extension from libxcb"
(https://github.com/bitcoin/bitcoin/pull/33217)
This is listed on https://doc.qt.io/qt-5.15/linux-requirements.html as "recommended", and doesn't seem to be needed (only used for windowing over multiple screens support?) , and the fact that it's no-longer installed by default on modern linux distros (i.e Ubuntu), is annoying/confusing for users. See:

https://github.com/bitcoin/bitcoin/issues/30061
https://github.com/bitcoin/bitcoin/issues/32097
https://github.com/bitcoin/bitcoin/pull/33197
https://bitcoin.stackexchange.com/questions/122
...

💬 stickies-v commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3201612121)
re-ACK 5dda364c4b1965da586db7b81de8be90b6919414 for more helpful failure logging, no other changes

🤔 polespinasa reviewed a pull request: "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates"
(https://github.com/bitcoin/bitcoin/pull/33199#pullrequestreview-3133246727)
cACK 413c81b134a16403cc0f64e21c3c0967c211b318

This change makes much sense to me, if the min relay fee is lowered the feerate tracking should also take that into account.

💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285915354)
I was wondering if the new test could be avoided by defining `low_feerate` as MINIMUM_RELAY_FEE_RATE in the previous test `test_estimation_modes`.