👍 theStack approved a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2409334427)
ACK ad5c012157c5f261503022cfa22d7124bfda5765

> On average, this counts about ~100 extra bytes per packet on x86_64:
>
> ```
> 2024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
> 2024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes
> 2024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes
> 2024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes
> ```

Seeing very similar numbers here w.r.t. to the extra byte count (tested on OpenBSD 7.6, x86_64):
```
2024-11-01T01:25:18Z msg type: inv, raw s
...

💬 github12101 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2451241236)
> maybe ECC ram would be smarter too... could just be getting hit by cosmic rays or something

Cosmic rays are quite unlikely. But simple error because of failing RAM stick, very likely. You may not even seen this in day to day work. But because bitcoind is writing a lot of data (and everything what it writes go through RAM first), then it may be helping you to expose otherwise unknown hardware problem.

💬 maflcko commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2451410917)
> I don't have a view on this, but just want to document that this PR does not address the described use case of building for fuzzing without building the fuzz binary ([#31057 (comment)](https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2400458885)) which is what motivated making `g_fuzzing` a runtime check.
>
> > ```
> > BUILD_FOR_FUZZING:BOOL=OFF
> > BUILD_FUZZ_BINARY:BOOL=ON
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
...

💬 maflcko commented on issue "CI: Make failure message easier to spot":
(https://github.com/bitcoin/bitcoin/issues/31200#issuecomment-2451427806)
I don't think there is an easy solution to always show the error reason at the tail of the CI log. To expand for each task:

* The lint tests are designed to run all (even if a failure occurs in the first one), so that all failures are collected. However, some of the lint tests print other output, so the failures may not be at the very tail. I've added the `⚠️` emoji to error messages to make them easier to spot, but maybe there are other ways to improve. Example: https://github.com/bitcoin/bi
...

💬 maflcko commented on issue "CI: Improve documentation around replicating CI locally":
(https://github.com/bitcoin/bitcoin/issues/31199#issuecomment-2451435052)
Yeah, this keeps coming up (ex: https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2450578651), but I don't know how to improve the docs.

Another related idea was to move the CC/CXX/SANITZER/... build options into a cmake preset, which could make it easier to reproduce some CI failures. Ref: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-09-05#1050658 and some more discussion in https://github.com/bitcoin/bitcoin/pull/30871#issuecomment-2344047118

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825645357)
fd7c35a22669d440afe182a153bed28fd326b51f

Often there will be no parents (a parent is in the mempool, but been flooded/reconciled already). It would be nice to give a random order in that case.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825649181)
This function could easily return two numbers: how many of these peers are inbound and outbound. And then just pass this pair along to `ShouldFanoutTo`. Should be not that complex code. A cheap cost to preserve the efficiency model.

I might try if you don't get to it earlier.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825659145)
Why having a collision means we should fanout the children? They may get reconciled in their own time.

💬 hebasto commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1825683419)
I've double-checked prerequisites required for building depends on macOS, and there are no additional requirements specific to depends beyond those listed in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#3-install-required-dependencies.

💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825711845)
The thread is not supposed to exit if `ProcessPCP()` fails. It should be more persistent: it should just wait a while and try again, maybe for the user to join a different network, swap their router, upgrade firmware, etc.

💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825712264)
Nice! Good catch.

💬 kevkevinpal commented on pull request "ci: Do not error on unused-member-function in test each commit":
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2451759791)
lgtm ACK [54d07dd](https://github.com/bitcoin/bitcoin/pull/31187/commits/54d07dd37d5919c4e3bc535ae498d623282741d1)

👍 rkrux approved a pull request: "scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}"
(https://github.com/bitcoin/bitcoin/pull/31163#pullrequestreview-2409965556)
tACK 4120c7543ee32efed7396d7153411ecbbd588ad3

Successful make and functional tests. I tested the script using the script checker script without any issues; ended up installing gnu sed and gnu grep on my MAC.

```
➜ bitcoin git:(202410-remaining_command_terminology) ✗ test/lint/commit-script-check.sh 2a52718d734cf65aaeeb18f627547e5bca3483f4..4120c7543ee32efed7396d7153411ecbbd588ad3
Running script for: 4120c7543ee32efed7396d7153411ecbbd588ad3
sed -i s/COMMAND_SIZE/MESSAGE_TYPE_SIZE/g $(gi
...

💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#discussion_r1825758256)
The branch and the PR description have been updated.

💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451786070)
> > --- Checking for module 'libqrencode'
> > --- Found libqrencode, version 4.1.1
> > +-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so
>
> This output seems less useful, given it no-longer has the version.

Updated.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825760358)
Found an answer! ([here](https://gist.github.com/naumenkogs/514ff3901960161a7b7ee08449840768))

```
09:42:53<@gmaxwell> Existing relay logic checks that transactions are still in the mempool before relaying them. I think the issue there would go away if instead of keeping around some erlay datastructure you just keep growing the queue of the peers transactions to relay until its time to reconcile, enh? then the existing logic to check if things are still in the mempool is sufficient.
```

💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2451788766)
@davidgumberg @hodlinator

Want to test this PR on Windows?

💬 hebasto commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451791489)
@maflcko

I'll address your feedback once [new actions](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2444060142) are enabled.

💬 maflcko commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2451797575)
Well, if it is made a nightly task in another repo, it won't need new actions enabled.

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825808253)
Yes, this will not exit if `ProcessPCP` fails. (Unless it's interrupted of course.)