💬 nervana21 commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2245589620)
Small nit. This comment could be updated now that we're checking for both `nonstandard` and `stripped` witnesses

```suggestion
// Check for witness-related policy violations.
```

💬 fanquake commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3140270122)
TSAN failure (https://github.com/bitcoin/bitcoin/actions/runs/16651908244/job/47126650397?pr=32989#step:8:6056):
```bash
2025-07-31T14:39:40.396402Z [test] [net.cpp:2419] [void CConnman::SetTryNewOutboundPeer(bool)] [net] setting try another outbound peer=false
2025-07-31T14:39:40.396438Z [test] [net.cpp:3192] [void==================
WARNING: ThreadSanitizer: data race (pid=11584)
Write of size 8 at 0x7234000016f0 by main thread:
#0 blockmanager_tests::blockmanager_readblock_hash_mi
...

💬 1440000bytes commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3140353595)
Actually minrelaytxfee should be 10 sat/vbyte

👍 pablomartin4btc approved a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3075915901)
ACK 95c11728f423e1c655439f9248a314f083ef68ef

Since now `migrate_and_get_rpc` allows to the `mocked_time` to be passed, as it was already validating the timestamp in the backup filename within the function, wouldn't make sense anymore to do the validation [again](https://github.com/bitcoin/bitcoin/pull/33104/files#r2244750342) outside the function.

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2245743993)
Fixed in latest push. Only modification to https://github.com/stickies-v/bitcoin/commit/3d96ff75f161419654b14a7e9fd884e52aec26c4 was that I removed the `LogInstance().EnableCategory(BCLog::LogFlags::NONE);` line.

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3140442210)
Latest push d434155db5f9c34b8ab3e19038d824d161b34195 -> f57496e:
- ensures the category mask does not leak across tests
- modifies `logging_filesize_rate_limit` to disable `BCLog::LogFlags::LOCK` if `DEBUG_LOCKCONTENTION` is defined

This gets rid of the test flake without hacking around `ReadDebugLogLines`.

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3140480780)
Though, it fails on GHA: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/16649916226/job/47119624805#step:8:30398

Without a way to reproduce interactively, it makes it hard to debug :(

💬 fanquake commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3140526097)
> TSAN failure

Didn't fail with a re-run: https://github.com/bitcoin/bitcoin/actions/runs/16651908244/job/47133023619.

💬 stickies-v commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2245830190)
Yeah sorry that should've been `LogInstance().DisableCategory(BCLog::LogFlags::ALL);`. I still think that's worth keeping, so we can always assume `LogTest` starts without any categories (as is default)?

💬 TheCharlatan commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3140574746)
Concept ACK

Implemented the same thing some time ago https://github.com/TheCharlatan/bitcoin/commit/de7e3a973ef5773714cd15ead499fbf734547cf1#diff-ac5fbeb5de6f28370bc348a579fc465fe7f7b91df0e0483c6edbf10273278c0d.

💬 BitcoinMechanic commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3140588217)
>It's irrelevant whether or not anyone considers any transaction "spam", all that matters is if miners are mining them, and they are.

Of course it's relevant if we're discussing what we relay and what we don't.

>All you accomplish by rejecting transactions that are getting mined is slowing down block propagation, and potentially wasting CPU cycles and bandwidth by downloading and processing the transaction twice.

Completely false. It's sad to see such obvious dishonesty in here.

🤔 enirox001 reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3076132085)
ACK d801926

Confirmed this fixes the timing race condition. The test was failing due to different timestamps being captured in the test vs inside `migrate_and_get_rpc()`, causing backup filename mismatches in slow environments.

Tested locally - reproduces the failure without the fix and passes with it. Good solution that maintains backward compatibility while centralizing mocktime management.

🤔 enirox001 reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3076154925)
ACK

💬 l0rinc commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3140630235)
The [IRC meeting](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-07-31#1140025) contained feedback on the overall direction:
> \<achow101> #topic dbwrapper read/write asymmetry (l0rinc)
> \<l0rinc> dbwrapper writes currently pretend to return bool but always return true, while real errors surface via exceptions – #33042 makes that explicit, but we need to decide the canonical error path
> \<l0rinc> `read` catches the exception and returns, but `write` always returned `true`, while th
...

👍 l0rinc approved a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430#pullrequestreview-3076264451)
code review fa80e750e702e2f0429fc6b83e9af2b8ed9b7b3c

🤔 fjahr reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3075780570)
tACK 95c11728f423e1c655439f9248a314f083ef68ef

💬 fjahr commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2245633370)
Could also move this one into `migrate_and_get_rpc` and replace the one there since this one is a bit more detailed.

💬 achow101 commented on pull request "wallet: Set descriptor cache upgraded flag for migrated wallets":
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2245990018)
oops fixed

💬 l0rinc commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3140801943)
> I ran two identically compiled nodes on two identical VMs on the same host

I did something similar, measuring two reindex runs until 900k blocks for master vs current PR vs enabling all `use_sighash_cache` values.

<details>
<summary>patch of last commit</summary>

```patch
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 7167d13af3..6d57c80de3 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -321,7 +321,7 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsi
...

💬 achow101 commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246020199)
Moved to `migrate_and_get_rpc`.

💬 achow101 commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246021885)
That's a good point. Moving the `listwalletdir` check into `migrate_and_get_rpc` also lets us drop the part where this test needs to pass in the mocktime too.