💬 ryanofsky commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372684495)
In commit "ci: Update Clang in "tidy" job" (1bb0f7f8b6d927046fefa2eb29d0132713415cca)

I'm confused about why these are necessary. What makes these two members different from the `fAnyoneCanPay` member and causes clang-tidy to warn about them but not `fAnyoneCanPay`? All three members seem to be set in the same way by the same `nHashTypeIn` constructor argument.

This seems more like a bug that should be reported upstream than a change that should be made to our code. And if the change is ne
...

💬 fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2372729201)
> CRCCheck force

Where are options like this being passed now?

💬 maflcko commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#discussion_r2372747277)
hardening was added in xcode 16: https://developer.apple.com/documentation/xcode-release-notes/xcode-16-release-notes#C++-Standard-Library

```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 52d21ef3ab..a0902e2700 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -145,8 +145,8 @@ jobs:
# Use the earliest Xcode supported by the version of macOS denoted in
# doc/release-notes-empty-template.md and providing at least t
...

✅ maflcko closed an issue: "ci: GHA fallback centos task runs out of space"
(https://github.com/bitcoin/bitcoin/issues/33293)

💬 RandyMcMillan commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3324678979)
When displayed in the gui - a consistent finger print makes more sense to the user.


💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3324707093)
> Yes, I've seen GHA caching work, will try and find logs.

For reference, this still fails for me with `#4 ERROR: failed to parse error response 400: <h2>Our services aren't available right now</h2><p>We're working to restore all services as soon as possible. Please check back soon.</p>0eZvSaAAAAAClNmDP6CfYS7fcNJbDhpBqRE0yRURHRTA1MTYARWRnZQ==: invalid character '<' looking for beginning of value`

💬 Sjors commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2372889066)
Do I understand correctly that this also (positively) impacts the `ASan/TSan, depends` jobs?

💬 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?