💬 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
...
💬 davidgumberg commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3045807408)
> @davidgumberg might be talking past each other, but what's happening here is that locally as soon as the block is reconstructed (including merkle checks), prior to script checks, the block is advertised forward via compact blocks to its peers. This is inline with the protocol description, and helps speed up block propagation.
I'm not suggesting any changes to the pre-validation relay described in BIP-152. I'm asking about propagating blocks post-validation that a node knows to be invalid, s
...
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3045807408)
> @davidgumberg might be talking past each other, but what's happening here is that locally as soon as the block is reconstructed (including merkle checks), prior to script checks, the block is advertised forward via compact blocks to its peers. This is inline with the protocol description, and helps speed up block propagation.
I'm not suggesting any changes to the pre-validation relay described in BIP-152. I'm asking about propagating blocks post-validation that a node knows to be invalid, s
...
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2190599831)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2190599831)
Fixed.
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3045908320)
Last push:
- Reformat to address https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2185221868
- Rebase on master
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3045908320)
Last push:
- Reformat to address https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2185221868
- Rebase on master
💬 ismaelsadeeq commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2190631992)
Yeah, the call should be outside the if statement because the fee estimator decays previously tracked data points after block connection in order to make stale data points less relevant and recent data points more relevant over time.
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2190631992)
Yeah, the call should be outside the if statement because the fee estimator decays previously tracked data points after block connection in order to make stale data points less relevant and recent data points more relevant over time.
💬 ismaelsadeeq commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2190634605)
On what I discussed with @furszy I posted comment here https://github.com/bitcoin/bitcoin/pull/32748#discussion_r2187355851
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2190634605)
On what I discussed with @furszy I posted comment here https://github.com/bitcoin/bitcoin/pull/32748#discussion_r2187355851
💬 Sjors commented on pull request "rpc: use anti-fee-sniping by default":
(https://github.com/bitcoin/bitcoin/pull/32892#issuecomment-3045926086)
Expanded the existing `test_anti_fee_sniping` case in `wallet_create_tx.py` to cover `send` `sendall`. Making `sendall` actually work required an additional refactor.
(https://github.com/bitcoin/bitcoin/pull/32892#issuecomment-3045926086)
Expanded the existing `test_anti_fee_sniping` case in `wallet_create_tx.py` to cover `send` `sendall`. Making `sendall` actually work required an additional refactor.
💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3045929125)
Rebased and addressed comment from @DrahtBot see [diff](https://github.com/bitcoin/bitcoin/compare/b487edcffab9ace19fd927998d82e945bd39b0a4..3cde7f185255b1bc07a90e95708665e0b6d5a46d)
(https://github.com/bitcoin/bitcoin/pull/29060#issuecomment-3045929125)
Rebased and addressed comment from @DrahtBot see [diff](https://github.com/bitcoin/bitcoin/compare/b487edcffab9ace19fd927998d82e945bd39b0a4..3cde7f185255b1bc07a90e95708665e0b6d5a46d)
💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3045932178)
Rebased to address conflict [diff](https://github.com/bitcoin/bitcoin/compare/bad72f1a24585768f4bac937a17597879063400e..f9b811cb041c14d514f3e41bc8c944b83b37e066)
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3045932178)
Rebased to address conflict [diff](https://github.com/bitcoin/bitcoin/compare/bad72f1a24585768f4bac937a17597879063400e..f9b811cb041c14d514f3e41bc8c944b83b37e066)
💬 mzumsande commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3045975801)
> disconnect peers after the time-out period if the BlockRequested is part of the active chain
Not sure I understand this - why would the block be part of the active chain by that time? We started a request in the first place because we don't know the full block yet, and if that request has timed out, chances are we still don't have the block, in which case it won't be part of our active chain. Or do you count on a parallel compact-block download being successful in the meantime?
> But, ma
...
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3045975801)
> disconnect peers after the time-out period if the BlockRequested is part of the active chain
Not sure I understand this - why would the block be part of the active chain by that time? We started a request in the first place because we don't know the full block yet, and if that request has timed out, chances are we still don't have the block, in which case it won't be part of our active chain. Or do you count on a parallel compact-block download being successful in the meantime?
> But, ma
...
🤔 stickies-v reviewed a pull request: "Cache m_cached_finished_ibd where SetTip is called."
(https://github.com/bitcoin/bitcoin/pull/32885#pullrequestreview-2994752288)
Conceptually not a bad idea to cache and lock less, but imo this makes the code more brittle (and harder to understand), e.g. if any tip updates happen without the cache being updated separately.
Do you have any data as to the actual performance improvements from this PR?
(https://github.com/bitcoin/bitcoin/pull/32885#pullrequestreview-2994752288)
Conceptually not a bad idea to cache and lock less, but imo this makes the code more brittle (and harder to understand), e.g. if any tip updates happen without the cache being updated separately.
Do you have any data as to the actual performance improvements from this PR?
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3045985477)
The latest push f47e2ea9137c3a832e07d6dd845c55d35d533fa9 has a few (minor) changes:
- removing the comment in `logging_filesize_rate_limit`
- documents restart behavior in the commit message of 6e47dad25c5a167cd171e5c11d51a7bee7c8d3c6
- fixes the typo of `-logsourcelocations` in the release notes
- makes the release notes more clear wrt. `LogPrintLevel`
> We're getting closer, but the PR is not ready yet. I went over the code multiple times, left nits and comments and questions all over,
...
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3045985477)
The latest push f47e2ea9137c3a832e07d6dd845c55d35d533fa9 has a few (minor) changes:
- removing the comment in `logging_filesize_rate_limit`
- documents restart behavior in the commit message of 6e47dad25c5a167cd171e5c11d51a7bee7c8d3c6
- fixes the typo of `-logsourcelocations` in the release notes
- makes the release notes more clear wrt. `LogPrintLevel`
> We're getting closer, but the PR is not ready yet. I went over the code multiple times, left nits and comments and questions all over,
...