💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3045465990)
> I don't think the test is useful. The change is being tested against a `Sync()` mechanism introduced within the test itself and not the real one, which can change over time (see for example #26966 that changes it completely). It would be better, and simpler for you, to share a patch that reproduces the issue (as @mzumsande mentioned, just adding a sleep() call and triggering the reorg is enough to reproduce it. See #32835 PR description for a patch example).
>
> Other than that, concept ACK
...

💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3045471393)
> I think that this assert could be hit if a reorg was happening while `Sync()` was going on:
>
> 1. `Sync()` syncs some blocks but isn't finished (doesn't update m_best_block)
> 2. The node undergoes a reorg, so that some of the synced blocks need to be rewinded.
> 3. `Rewind()` is called and the assert fails.
>
> I could trigger this on regtest with an added sleep in `Sync()`, freezing the sync so that I could do the reorg. I think it's going to be hard / impossible to trigger it in a
...

💬 theuni commented on pull request "ci: Use optimized Debug build type in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3045478804)
Is it necessary to use `-g` here? The output in #32829, for ex, doesn't show a backtrace. So at least in that case the debug symbols are just taking up space.

💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3045485948)
Hi all, I've written a test with tweaked Sync() to demonstrate this bug: [https://github.com/bitcoin/bitcoin/commit/e9f4ffcd7d8b7211cc049a8ab7632b19c06f7b11](url)

💬 maflcko commented on pull request "ci: Use optimized Debug build type in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3045502250)
No, `-g` isn't needed for the CI itself. It is only there for developers who happen to run this locally in a container and want to reproduce something in a debugger without having to modify the script and re-compile. Storage is cheap/free, so I thought it can't hurt to have it, but no strong opinion. Anything should be fine for the setting.

🚀 fanquake merged a pull request: "threading: use correct mutex name in reverse_lock fatal error messages"
(https://github.com/bitcoin/bitcoin/pull/32829)

📝 Sjors opened a pull request: "rpc: use anti-fee-sniping by default"
(https://github.com/bitcoin/bitcoin/pull/32892)
If the user does not specify `locktime` for `send`, `sendall` and `walletcreatefundedpsbt`, enable anti-fee-sniping so RPC behavior matches the GUI wallet.

💬 rkrux commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2190343122)
Done

👋 fanquake's pull request is ready for review: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32863)

👍 stickies-v approved a pull request: "bench: Avoid tmp files in pwd"
(https://github.com/bitcoin/bitcoin/pull/32890#pullrequestreview-2994321094)
ACK fa2fbaa4a29f80d3c7d5f0ad6b64035c3156dd12

Was able to reproduce both the pre- and post-fixed situation described, and can't find any other test instances that should be using the testing setup temp dir instead.

💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2190390786)
Yes absolutely! I think it all comes down to https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2149379657:
- This PR changes MakeProcessNextHeaders/PopHeadersReadyForAcceptance to use headers as a I/O parameter (09c7d0f703067d455aae4ee458ba7953c29d72fb)
- #32579 changes MakeProcessNextHeaders to use span instead of vector for headers (4c45b5d349c59ed6a795819cdfbaec90bc189a24)

The reasons for switching to headers as I/O parameter are explained in sipa's commit (09c7d0f703067d455aae4e
...

🤔 mzumsande reviewed a pull request: "p2p: Relax BlockRequestAllowed to respond to advertised blocks"
(https://github.com/bitcoin/bitcoin/pull/32869#pullrequestreview-2994324888)
I'll have to think a bit more about the consequences of this - but if we do this change, can we remove the `need_activate_chain` / `ActivateBestChain` logic from `ProcessGetBlockData()`? It doesn't belong in `net_processing` from an architectural POV, and now it shouldn't matter anymore for the request if the block is fully validated / invalid.

Also, just noting that as a side effect we'd now serve blockfilters for invalid / unvalidated blocks (`PrepareBlockFilterRequest`).

💬 mzumsande commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2190389460)
Should adjust doc of `BlockRequestAllowed` above ("we fully-validated them at some point")

🤔 ryanofsky reviewed a pull request: "ipc: add bitcoin-mine test program"
(https://github.com/bitcoin/bitcoin/pull/30437#pullrequestreview-2994128311)
Rebased 278101f6ee723e6401ddb3e46f2fce3b60cc42ca -> 1a87ed000e09b10c173ba87d4bf2f79bf63abfa5 ([`pr/mine.25`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.25) -> [`pr/mine.26`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.26), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.25-rebase..pr/mine.26)) to resolve conflicts. Also split commits to separate code shared with #32297 and applied suggestions on test environment variables and extending the mining example to gene
...

💬 ryanofsky commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2190310723)
re: https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2139659083

> needs rebase after #32719?

Thanks! Updated

💬 ryanofsky commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2190262118)
re: https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2133866597

Thanks! Renamed variables to be consistent and set them in the GHA config.

💬 ryanofsky commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2190195146)
re: https://github.com/bitcoin/bitcoin/pull/32297#discussion_r2168951779

> NIT can be made private ?

Sure, made private

🤔 ryanofsky reviewed a pull request: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2994016409)
Rebased 9ffe57f81b2a6abd161ae010f07916b05e57191d -> 37cd2c076434e7acbdbb20996cf87afb2cb5bc84 ([`pr/ipc-cli.8`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.8) -> [`pr/ipc-cli.9`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-cli.9), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-cli.8-rebase..pr/ipc-cli.9)) to resolve conflicts and also updated with suggestions

💬 Prabhat1308 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3045660856)
Trying to debug the issue I see some irregularities in the how the test behaves in `rpc` mode and `cli` mode.

<details>
<summary>diff</summary>

```diff

diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index a241978b5f..f92e8d6e88 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -135,8 +135,64 @@ class PSBTTest(BitcoinTestFramework):
assert "non_witness_utxo" in mining_node.decodepsbt(psbt_new.to_base64())["inputs"][0]


...

👍 stickies-v approved a pull request: "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError"
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2994424840)
re-ACK f47e2ea9137c3a832e07d6dd845c55d35d533fa9

No changes since 6a7147358c9d6e3883dcdbbee9fb2c1cb4baf5ff except commit message and release notes improvements, and removing a commented-out test line.