💬 rkrux commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3032050281)
I want to mention about how long is the deprecation/grace period generally but I don't know right now. Suggestions welcome.

📝 theStack opened a pull request: "test: refactor: overhaul block hash determination for `CBlock{,Header}` objects"
(https://github.com/bitcoin/bitcoin/pull/32868)
Similar to what #32421 did for `CTransaction` instances, this PR aims to improve the block hash determination of `CBlockHeader`/`CBlock` (the latter is a subclass of the former) instances by removing the block header caching mechanism and introducing consistent naming. Without the statefulness, sneaky testing bugs like #32742 and #32823 are less likely to happen in the future. Note that performance is even less of an issue here compared to `CTransaction`, as we only need to hash 80 bytes, which
...

🤔 hebasto reviewed a pull request: "ci: update pwsh to use custom shell that fails-fast"
(https://github.com/bitcoin/bitcoin/pull/32672#pullrequestreview-2983019444)
Left a couple of notes regarding the current implementation:

1. The output in the "Get tool information" step has changed:
```diff
--- old 2025-07-03 13:37:57.062194420 +0100
+++ new 2025-07-03 13:35:57.800242845 +0100
@@ -1,3 +1,15 @@
+
+Name Value
+---- -----
+PSVersion 7.4.10
+PSEdition Core
+GitCommitId 7.4.10
+OS Microsoft Windows 10.0.20
...

💬 fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2182696708)
I've added `.github/ci-test-each-commit-exec.py` & `.github/workflows/ci.yml`. I'll add a commit for Guix once the build passes.

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3032145576)
The `walletprocesspsbt` RPC now also has this option. Added test coverage by expanding `wallet_taproot.py`.

💬 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#discussion_r2182726209)
Unfortunate that it can only be resolved by marking the function as `NOLINT`. Addressed.

💬 maflcko commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2182756040)
Just for additional reference, the check is already sometimes failing on current master: https://github.com/bitcoin/bitcoin/issues/30797#issue-2503043392

💬 hebasto commented on pull request "cmake: Create subdirectories in build tree in advance":
(https://github.com/bitcoin/bitcoin/pull/32773#issuecomment-3032234676)
Rebased.

@janb84
> Any hints how to verify / test this ?
>
> Have tried to see any difference between this PR and master but it's not clear to me what difference I should see. When used on #32697 there is a difference that without this PR `build/test/functional/data/util/*` is populated with this PR the util directory is not being created (looking at the source this is logical but unsure if there needs to be things done after 32697 is merged or not having the directory is intentional )

...

💬 pinheadmz commented on issue "intermittent issue in wallet_keypool.py: assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0) AssertionError: not(1725366101 == 0)":
(https://github.com/bitcoin/bitcoin/issues/30797#issuecomment-3032235787)
Oh ha! I should've checked closed issues before opening #32793. I didn't see any other recent failures outside my own PR so I suspected it was just me.

💬 fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2182775275)
Added a commit for Guix as well.

👋 fanquake's pull request is ready for review: "cmake: Use `AUTHOR_WARNING` for warnings"
(https://github.com/bitcoin/bitcoin/pull/32865)

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2182562681)
Nit: this seems a bit repetitive now that the above lines were edited - if we keep it, consider unifying how we write `scriptPubKey`:
```suggestion
// This means sigops in the spent scriptPubKey count toward the limit.
```

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2182560754)
```suggestion
// `fAccurate` means correctly accounting sigops for CHECKMULTISIG(VERIFY)s with 16 pubkeys or fewer. This
```

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2182812849)
Wanted to understand the orders of magnitude we're talking about - especially for low-end raspberry pi 4 devices -, ran a `-reindex-chainstate` with `-assumevalid=0` until 400k blocks to see the correlation between script size and validation time - and the order of magnitude of validating tens of thousands of scripts in a single tx (since these are normally skipped during my IBD benchmarks at these historic heights).

I simply subtracted the subsequent times to get the real time (including bl
...

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2182817167)

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2182555489)
Can we move this closer to its usage - to obviate that the first check doesn't need this yet.

📝 instagibbs opened a pull request: "WIP p2p: Relax BlockRequestAllowed to respond to advertised blocks"
(https://github.com/bitcoin/bitcoin/pull/32869)
First commit adds coverage to a similar case to #13370 . h/t @dergoegge for detecting this issue independently. Documentation of this behavior with a test is then followed by a proposed change where anytime we considered something `VALID_TRANSACTIONS` and potentially advertised via compact blocks, we should respond to requests for the `getdata` if the peer fell back for whatever reason.

If the second commit is deemed controversial it can be shelved and current behavior documented alone.

💬 maflcko commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2182835137)
Also, it doesn't seem to be working right now anyway, see the CI failure (needs rebase)

💬 dergoegge commented on pull request "WIP p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3032377986)
Concept ACK

> h/t dergoegge for detecting this issue independently.

For context, I found this case (as demonstrated by the first commit in the PR) by adding a netsplit oracle (see https://gist.github.com/dergoegge/3036796551095a9ce7535fb5d74e6656#detecting-network-split-bugs) to [fuzzamoto](https://github.com/dergoegge/fuzzamoto). While extremely unrealistic/unlikely to trigger in the wild, I'd still consider this timeout case a bug, especially because it could cause disconnects across th
...

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3032396743)
I split off a non-behavior-changing commit that adds `avoid_script_path` to `FillPSBT`. I think it makes sense to pass this option here, although in general we may want to refactor `FillPSBT` to an options struct instead of this ever expanding argument list. That could be an independent or prerequisite PR.

I then did the same for `SignPSBTInput`, where I also think struct would be better.

Rinse and repeat for `::SignTransaction`.

Will continue later to further split 09334ae0bd7939ed7476
...