💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324812123)
Forced pushed from dca8200c71bb568ffe4821c68209e07a305db80d to 16d55585aa2d26e9177734927b8446faa72570e7 [5db80d..16d5558](https://github.com/bitcoin/bitcoin/compare/dca8200c71bb568ffe4821c68209e07a305db80d..16d55585aa2d26e9177734927b8446faa72570e7)

Main Changes are
1. Moved the `BlockTemplateCache` to `src/node/miner.cpp` to fix the circular dependency issue
2. The response now return the block template creation time
3. We search for match from right to left to return the fresh hit first

...

💬 l0rinc commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2372927921)
I don't understand how this related to the PR. Is this really needed here or can it be split out to a different PR?

💬 benthecarman commented on pull request "Fix dark mode detection on Linux":
(https://github.com/bitcoin-core/gui/pull/895#issuecomment-3324841313)
> Could you please clarify which build this change is intended to affect, statically linked or shared?
>
> If the latter, which system was this tested on (OS, desktop environment, Qt version)?

this sadly only effects dynamically linked builds.

I have tested on ubuntu 24.04 with Qt 6.4.2

⚠️ plebhash opened an issue: "`bitcoin-node` is unkillable after mining IPC connection is established"
(https://github.com/bitcoin/bitcoin/issues/33463)
I'm using `bitcoin-node` from [`v30.0rc1`](https://github.com/bitcoin/bitcoin/tree/v30.0rc1) to drive my development towards Sv2 integration over the new mining IPC interface https://github.com/stratum-mining/stratum/issues/1885

I noticed that after the first IPC connection is established, I'm unable to kill `bitcoin-node` via Ctrl+C.

<img width="1906" height="847" alt="Image" src="https://github.com/user-attachments/assets/97857485-2a8a-4f03-b388-73ed27c6d1d7" />

💬 Sjors commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3324853126)
cc @ryanofsky

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324859282)
It seems the new fuzz test in [16d5558](16d55585aa2d26e9177734927b8446faa72570e7)
caught a minor UndefinedBehaviorSanitizer: unsigned-integer-overflow issue on master.


```
/home/admin/actions-runner/_work/_temp/src/node/miner.cpp:414:47: runtime error: unsigned integer overflow: 2000 - 4000 cannot be represented in type 'size_t' (aka 'unsigned long')
#0 0x5b2b2753aee0 in node::BlockAssembler::addPackageTxs(int&, int&) /home/admin/actions-runner/_work/_temp/build/src/./node/miner.cpp
...

💬 l0rinc commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2372950950)
You mean to only log enabled/disabled changes, but not enabled-after-assumevalid/enabled-no-burried changes? This was also deliberate to show the exact reason so that users understand the exact reason instead of just whether it's enabled or not.

💬 l0rinc commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2372950999)
The original message contained these indentations - expect in a very few cases. It seemed simpler to me to unify the text that way - otherwise I would have rewritten it completely since I don't particularly like the way it describes the situation.

💬 l0rinc commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2372951037)
I thought of this also, we can even add this error message as default and overwrite in every other case. If other reviewers also thing this would be simpler, I can do it in the next push, if needed.

💬 l0rinc commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2372951112)
that was the original suggestion as well, see https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351556972
I decided against it since I don't want to introduce an extra explicit assumevalid branch, it's still the same case (not part of av chain), just with better message - seems conceptually it's easier to grasp if we have fewer overall categories.

💬 l0rinc commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2372951175)
I don't mind doing this in a separate PR, but here I have decided against it since it's not strictly related to the change, see: https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2347916838

💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3324880657)
the Rust code is still WIP, but FWIW, up until the ctrl+c, the interactions with `bitcoin-node` were:
- one call to `createNewBlock`
- one call to `waitTipChanged` (that hasn't been answered yet)
- all the bootstrapping to make the above possible (bootstrap RPC system, thread client, mining client)

💬 PiRK commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2372969095)
Not sure why this commit was pulled into my PR the last time i rebased on master. This was the top commit from master that had just been merged (#33364) at the time

Some kind of github glitch?

💬 l0rinc commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971637)
Agree, pushed a better solution to rate limit existing message + announcement of work ahead (as suggested by @ajtowns).
Rate limiting changes can be done in a separate PR now if needed.

💬 l0rinc commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372971804)
> Should be used with extreme care

Given that we didn't even have any rate limiting so far and as far as I can tell it wasn't weaponized so far I'm not sure this is an "extreme" danger.
But I don't mind removing the rate-limit related commit suggested by @ajtowns, since it seems to be more contentious than just reducing the rolling forward messages.
And I have extended it for the rolling forward messages as well and limit the log info to every 10k blocks to be less spammy.

💬 Sjors commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3324900365)
I'm not sure if `waitTipChanged` can handle being interrupted before its timeout yet, from either side. How does ctrl + c happens if you don't call `submitBlock`?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3324906640)
Did the same benchmark with 5000 dbcache and there is a 6% speedup :rocket: - 4:27 vs 4:44. Even with far less cache misses this change still a benefit, and will continue to improve block connection speed as the blockchain and utxo set gets bigger.

| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `echo 688c03597afb0b76077f1ffc4608eef19481056e && /usr/bin/time ./build/bin/bitcoind -printtoconsole=0 -connect=192.168.2.171 -stopatheight=912683 -dbcache=5000` |
...

📝 mzumsande opened a pull request: "p2p: Use network-dependent timers for inbound inv scheduling"
(https://github.com/bitcoin/bitcoin/pull/33464)
Currently, `NextInvToInbounds` schedules each round of `inv` at the same time for all inbound peers. It's being done this way because with a separate timer per peer (like it's done for outbounds), an attacker could do multiple connections to learn about the time a transaction arrived. (#13298).

However, having a single timer for inbounds of all networks is also an obvious fingerprinting vector: Connecting to a suspected pair of privacy-netowrk and clearnet addresses and observing the `inv` p
...

💬 l0rinc commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3324911767)
> The issues it finds seems to happen less than once per year

For the record a big-endian CI machine would have helped with https://github.com/bitcoin/bitcoin/pull/31144

💬 ismaelsadeeq commented on issue "RFC: Bitcoin Core Node `BlockTemplateManager`":
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3324920650)
I opened #33421 last week and made an update to it today, I think it is ready for for review now.

The fuzz test added in the PR also caught an issue on master that should be backported to v30 see https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324859282