💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2402403167)
review ping @maflcko

💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402454601)
Ah yes, I'll look into whether the lower load does anything.

Just to be complete on what happened. The IBD continued after restarting Core again, but after an eventual third shutdown and restart of Core to test what would happen, the block database was corrupted again.

```
2024-10-09T13:52:42Z UpdateTip: new best=00000000000000000878cd8de3dccda91cdcf6ef0e6e8536120c65f487dd0197 height=390296 version=0x00000004 log2_work=83.803743 tx=100238753 date='2015-12-26T14:25:06Z' progress=0.09181
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402462147)
@maflcko

How do I clean depends sources cache on the Cirrus CI for this PR?

💬 ajtowns commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#discussion_r1793618993)
Added to the inquisition PR in https://github.com/bitcoin-inquisition/bitcoin/pull/68

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402496447)
They'd have to be cleared on all machines. Let me known if you want that to happen. If you are just looking for a temporary workaround for testing, you can pick a different qt version, like `6.7.1`. Or you can temporarily use a new sources volume, similar to the diff in https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337816419.

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793642022)
> Also (not sure about this, but) it might be nice to add an explicit conversion method to Original class, so we could write something like `_("Error opening block database").str()` instead of `bilingual_str{_("Error opening block database")}`

Sure, I am happy to add `bilingual_str str()` and `operator bilingual_str()` to the `Original` type and remove the constructors from `bilingual_str`. However, this means that calling `bilingual_str{Original{}}` is now forbidden and one would have to use
...

👍 maflcko approved a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936#pullrequestreview-2357490000)
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4 🏻

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c495731a316d9c97ee05a08cf5
...

💬 maflcko commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1793727754)
nit in c495731a316d9c97ee05a08cf5087c5535f84bd4: starting with C++17, you can use `try_emplace`:


```diff
diff --git a/src/wallet/test/fuzz/spend.cpp b/src/wallet/test/fuzz/spend.cpp
index 9abd9e402a..cd092256c1 100644
--- a/src/wallet/test/fuzz/spend.cpp
+++ b/src/wallet/test/fuzz/spend.cpp
@@ -68,7 +68,7 @@ FUZZ_TARGET(wallet_create_transaction, .init = initialize_setup)
tx.vout[0].scriptPubKey = GetScriptForDestination(fuzzed_wallet.GetDestination(fuzzed_data_provider));

...

💬 mzumsande commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2402684547)
> I haven't looked closer into why these happen.

Two mechanisms have been pointed out above, both are due to timing issues in orphan resolving: https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2273685985 and https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2274310940 - I wouldn't be surprised if there were others.

Obviously resolving these are blockers for any further progress, whether with a BIP or not. If this PR was deployed as is, any two upgraded peers would discon
...

💬 hebasto commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2402733760)
I cannot neither confirm nor reproduce the issue on my MacBook Pro (2019, Intel).

The following scenarios were tested:
- Sonoma 14.6.1 + Command-Line Tools 15.0.
- Sonoma 14.7 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Xcode 16.0.
- Sequoia 15.0.1 + Xcode 16.0 installed and then deleted.

For all scenarios, here is the list of the installed Homebrew's packages:
```
% brew list
==> Formulae
cmake pkg-config

==> Casks
```

> But i
...

📝 maflcko opened a pull request: "lint: commit-script-check.sh: echo to stderr"
(https://github.com/bitcoin/bitcoin/pull/31063)
This makes it easier to redirect the produced `git diff` on failure. On success, it shouldn't hurt, because the same output is still present, just on stderr.

Can be tested by introducing a fault in any scripted diff and then calling `commit-script-check.sh HEAD~..HEAD > any_file.txt`. Previously the file contained the full output, now it contains just the diff.

💬 maflcko commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2402756550)
lgtm ACK c15ef096ce317ae911dd756593adeac9d9a0b5a1

✅ maflcko closed a pull request: "bench: Adds a benchmark for CheckInputScripts"
(https://github.com/bitcoin/bitcoin/pull/29745)

💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2402769455)
> > Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
>
> Why are ninja builds not affected?

Ninja builds are sped up significantly by pch (this PR), it's the combo with #30911 doesn't seem to have much effect.

💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793800548)
2658d2b2dfaacbfb2ddf2f1305ed8c3c9e134b71

won't this fail in `DIFFERENT_WITNESS` case?

💬 maflcko commented on pull request "bench: Adds a benchmark for CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/29745#issuecomment-2402770771)
Closing for now due to inactivity since the day it was opened. Leave a comment below if you want this re-opened, or open a new pull.

✅ maflcko closed a pull request: "validation: Use witness maleation flag for non-segwit blocks"
(https://github.com/bitcoin/bitcoin/pull/29540)

💬 maflcko commented on pull request "validation: Use witness maleation flag for non-segwit blocks":
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2402780104)
Closing for now due to inactivity (lack of reply to a question about the motivation for this change). Please leave a comment, if you want this re-opened.

🤔 stickies-v reviewed a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2357070300)
Concept ACK, and this is a smaller diff than I'd expected to implement this, nice!

I need to think about it more and work out some code, but I feel like the new interface is not as intuitive yet as it could be. Will follow up with that in next review.

💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793488009)
in aaaa4fb20156b4375d92e1eca4acc90a425a1896: seems more straightforward to just extend `ConstevalStringLiteral` instead of creating a new `util::Original`? This would 1) minimize the diff and 2) I find `ConstevalStringLiteral` a much more helpful name than `Original`.