💬 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
...
(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.
(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)
(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.
(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)
(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.
(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
(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)
(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.
(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
...
(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`).
(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")
(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
...
(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
(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.
(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
(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
(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]
...
(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.
(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.
💬 yuvicc commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3045757878)
Concept ACK
Made a Java wrapper library for the Java folks out there!
https://github.com/yuvicc/java-bitcoinkernel
While playing with the API I also wrote a [benchmarking test](https://github.com/yuvicc/bitcoin/tree/2025-06-kernel_benchmarking) for the script validation using the API vs the internal code just to see the performane overhead:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchma
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3045757878)
Concept ACK
Made a Java wrapper library for the Java folks out there!
https://github.com/yuvicc/java-bitcoinkernel
While playing with the API I also wrote a [benchmarking test](https://github.com/yuvicc/bitcoin/tree/2025-06-kernel_benchmarking) for the script validation using the API vs the internal code just to see the performane overhead:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchma
...