💬 maflcko commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2107257666)
> I have installed them but turned --with-gui=no in the configure option

Do you want the gui? If not, then disabling is the right choice, but then the gui unit tests shouldn't fail, because they shouldn't exist. If they do, that is a bug.

Again, there is little that can be done here, unless you share more details on the steps to reproduce from the book. See https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2090021205

✅ maflcko closed an issue: "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs"
(https://github.com/bitcoin/bitcoin/issues/30001)

💬 maflcko commented on issue "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs":
(https://github.com/bitcoin/bitcoin/issues/30001#issuecomment-2107260441)
Closing for now due to inactivity, but please leave a comment if you have more details or information.

💬 maflcko commented on pull request "kernel: Remove batchpriority from kernel library":
(https://github.com/bitcoin/bitcoin/pull/30083#issuecomment-2107333523)
utACK 04ffe4061da2d0135f206032e167703772b5da78 🎪

<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: utACK 04ffe4061da2d0135f2060
...

💬 fanquake commented on pull request "contrib: use ENV flags in get_arch":
(https://github.com/bitcoin/bitcoin/pull/30074#issuecomment-2107335721)
Guix Build (aarch64):
```bash
fcb75f2a3b61befeabe143301e5db490d23b1ee4a5edb109f219dea575395b49 guix-build-b59a027d957a/output/aarch64-linux-gnu/SHA256SUMS.part
15310e3aff2a9ef71bad06315fcd425e56611d5bed7f6c394f3fe2248140e17b guix-build-b59a027d957a/output/aarch64-linux-gnu/bitcoin-b59a027d957a-aarch64-linux-gnu-debug.tar.gz
8e60146d39a47e9c332e3b842e239f1ca7773ca2e6788876155377f4568ab1fa guix-build-b59a027d957a/output/aarch64-linux-gnu/bitcoin-b59a027d957a-aarch64-linux-gnu.tar.gz
104322
...

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598332839)
Oops, this is a bad copypaste. I forgot to delete this commen I copied over.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1598333278)
Same - forgot to delete, sorry

💬 maflcko commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-2107357612)
> And if I understand correctly, it is then copied _again_ [here](https://github.com/bitcoin/bitcoin/blob/98dd4e712efaa2b77cb168426756879c6405c3f4/src/univalue/lib/univalue.cpp#L134) since `UniValue` doesn't have a move constructor. So improving UniValue move semantics seems to make sense here indeed.

Can you explain this? It has a move constructor, at least the following compiles for me:

```diff
diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp
index 656d2e8203.
...

💬 fanquake commented on pull request "depends: set AR & RANLIB for CMake":
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1598351939)
> What works for me is:
> Bug here: https://gitlab.kitware.com/cmake/cmake/-/issues/18087

Given the bug report is 5 years old, I can't imagine CMake is going to either "fix" or change things now. I can switch to `CMAKE_` using `which` if that is what's preferred. Will add `CMAKE_NM` even if that's missing from the CMake docs. Maybe we can upstream a fix for that as well. Whatever will move things forward.

👍 brunoerg approved a pull request: "fuzz: wallet, add target for `Crypter`"
(https://github.com/bitcoin/bitcoin/pull/28074#pullrequestreview-2052516066)
utACK d7290d662f494503f28e087dd728b492c0bb2c5f

📝 fanquake converted_to_draft a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
Based on #21778.

This reduces the size of the binary by 2-3% when building with `--enable-reduce-exports`.

> -no_exported_symbols
> Useful for main executable that don't have plugins and thus need no symbol exports.

Can be tested with `dyld_info -exports src/bitcoind`. The only exported symbol should be `__mh_execute_header`.

One question is if this should be under `--enable-reduce-exports`, or we just try and use it at all times, similar to `-dead_strip(_dylibs)`.

💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2107414649)
Rebased, but drafted while based on #21778.

💬 maflcko commented on pull request "rpc: Remove index-based Arg accessor":
(https://github.com/bitcoin/bitcoin/pull/29997#discussion_r1598381548)
Not sure what you mean. This sentence introduces the `Type`, which is referred to in the next sentence, so I don't think it can be removed.

Maybe you can share a diff?

📝 vasild converted_to_draft a pull request: "util: check for errors after close and read in AutoFile"
(https://github.com/bitcoin/bitcoin/pull/29307)
`fclose(3)` may fail to flush the previously written data to disk, thus a failing `fclose(3)` is as serious as a failing `fwrite(3)`.

Previously the code ignored `fclose(3)` failures. This PR improves the explicit callers to check whether it failed. However there is a design issue that `fclose(3)` is also called from the `AutoFile` destructor. There is no good way to signal a failure to the caller from the destructor. So, if closing from the destructor fails, then log a message (the file name
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2107449866)
I added the highest output amount for each transaction to the index, which the new `dust` argument for `getsilentpaymentblockdata` can use to filter.

The amount is right shifted by 4 and stored as a single byte. This lets us track a useful range (0 - 4080 sat) with only ~5% overhead.

💬 glozow commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1598392660)
> The downside to this patch is that the log line may now be printed more than once in the for loop.

Question: do we expect this to loop many times / under what circumstances? I tried adding logging to see what happens on my node, and I haven't seen it go more than once yet.

🤔 glozow reviewed a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2052565010)
ACK d447bdcfb0e38353940e4a7fc89d09482d8d39c3 - code review, light fuzzing

💬 martinus commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-2107457736)
>The UniValue [created inside blockToJSON](https://github.com/bitcoin/bitcoin/blob/98dd4e712efaa2b77cb168426756879c6405c3f4/src/rpc/blockchain.cpp#L170) is ~40MB worst-case, but it is then copied into a [second UniValue](https://github.com/bitcoin/bitcoin/blob/98dd4e712efaa2b77cb168426756879c6405c3f4/src/rpc/blockchain.cpp#L196), doubling the required heap size.

I had a PR somewhere where I removed the copy constructor and made it mandatory to call a `.copy()`. Then you'd get a compile error
...

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1598411774)
I think you're right. Might have added this as belt-and-suspenders, but have checked again with non-Guix builds, and I still see the correct lld used, even on systems with other llds installed & similar.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598416494)
> Filtering < 546 dust

I should add that I was using `IsDust()` with `DUST_RELAY_TX_FEE`, which takes SegWit into account to decide the exact dust limit.

> 10% of taproot UTXOs

I looked at the largest output, not all outputs.