💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1624133280)
I could only find the following in the docs and based the changes here on that.

```
This flag causes the receive operation to return data from
the beginning of the receive queue without removing that
data from the queue. Thus, a subsequent receive call will
return the same data.
```

We could make it so that a peek read only returns one byte and a subsequent normal read return N bytes where the first byte corresponds to the peek byte. But I'm not sure if that would change much for our
...

💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624155073)
Why do this `Mul`'s return type and `MulFallback`'s one differ?

💬 maflcko commented on pull request "build: Fix building `fuzz` binary on on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/30216#issuecomment-2144798700)
ACK 3299abce948f205bb1354993614b669189f9b89f

💬 maflcko commented on pull request "depends: Update Boost download link":
(https://github.com/bitcoin/bitcoin/pull/30217#issuecomment-2144806703)
ACK ffbc173ca1ed6b93de8bf3f88b8aed0743f4916c

💬 hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2144810134)
> Is this still an issue given recent CMake changes?

Yes. Tested with the master branch @ 80bdd4b6beb878c95478b5623c9f9ff0b948ad57.

And this PR still fixes it.

💬 maflcko commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2144818621)
Any thoughts on splitting the first commit out into a separate pull? Seems to be a requested feature by three people so far, but I am not sure about bundling it for review with the other changes here.

💬 hebasto commented on pull request "guix: show `*_FLAGS` variables in pre-build output":
(https://github.com/bitcoin/bitcoin/pull/30185#discussion_r1624187207)
Why `ADDITIONAL_GUIX_TIMEMACHINE_FLAGS` skipped? Despite that it was used before this point, it can be useful for users and consistency.

💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624220296)
That's the point; they represent the 96-bit integer product in a different way.

💬 fanquake commented on pull request "guix: show `*_FLAGS` variables in pre-build output":
(https://github.com/bitcoin/bitcoin/pull/30185#discussion_r1624227288)
Sure. Added that as well

💬 laanwj commented on pull request "doc: JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/30215#issuecomment-2144912650)
ACK 3c08e11c3ea4499e8d20609e2417cac859b3e98e
This is an obvious documentation improvement.

> nit: Technically, I think the trailing semicolon application/json;

Good catch. Content type doesn't need to be suffixed with `;`. It's pretty uncommon, even. According to the `curl` doc the semicolon is a special syntax for providing an empty header, but not required otherwise, eg the example they give is:
```bash
curl -H "X-First-Name: Joe" http://example.com/
```

💬 fanquake commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2144925933)
@mzumsande I'm guessing you switched to using a newer Clang here? Note that our minimum required Clang is also now 15, which would "solve" the original issue, however not the `RequiresExpressionIndentation` issue, but my suggestion would be to use clang-16+ if possible.

👍 hebasto approved a pull request: "guix: show `*_FLAGS` variables in pre-build output"
(https://github.com/bitcoin/bitcoin/pull/30185#pullrequestreview-2093531787)
ACK 5f2c1d84e37697f4f8a20e3c12f37bba71b3c2a6.

💬 fanquake commented on issue "Erlay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/28646#issuecomment-2144932529)
I've updated the PR to review, at the top of the issue here. @sr-gi are you going to open your own tracking issue going forward? That'll likely be easier to keep up to date.

💬 sipa commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1624277101)
It would be possible to use `std::pair<int32_t, uint64_t>` here too (as in, that type would be big enough to store the result), but `_mul128` returns 64-bit results, which match the register size of x86_64, so no conversion is needed.

🚀 fanquake merged a pull request: "build: Fix building `fuzz` binary on on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/pull/30216)

💬 laanwj commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1624295221)
Returning `sa_family_t` is slightly more self-documenting, imo, would prefer to keep it as-is.
But yes it's simply a type-def'd integer type not a real enum, i suppose because it's "open", as in, new OS headers could potentially define their own address family without having to update a central list.

💬 chrisguida commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1624301456)
Yep, that's great!!

💬 m3dwards commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2145020810)
@maflcko @hebasto @fanquake do we want to just extract the USDT part of the job into Github Actions?

💬 maflcko commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2145039565)
Not sure. It seems more work and overhead to maintain two tasks, instead of one, no?

💬 maflcko commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2145060525)
Well, if you really wanted to do it, you could move it into the "test each commit" task, and make that one run every time, to keep the number of tasks the same.