💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598170246)
Sorry to nit my own previous suggestion, but I think "parent paying for child replacement" might still be too specific, since it could be that the child has nothing to replace but is lower feerate (like `package_txns6` in the functional test).

Maybe
```suggestion
"package RBF failed: package feerate is less than parent feerate",
strprintf("package feerate %s <= parent feerate %s", package_feerate.ToString(), parent_f
...

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2107220524)
ACK d196442c015c4cbf490751a650ecb3aa29321442

✅ maflcko closed an issue: "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb"
(https://github.com/bitcoin/bitcoin/issues/29909)

💬 maflcko commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2107249656)
Closing for now, due to inactivity, but the discussion can be continued regardless.

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