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

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2145109407)
> `BatchWrite` already iterates the entire cache anyways

@achow101 I think this is what you might not be clear about. `BatchWrite` iterates the entire cache, but this patch does not. After this patch `BatchWrite` only iterates flagged entries, which is a much smaller subset of the cache.

I tried moving everything in the loop in `Sync` into `BatchWrite`, along with `cachedCoinsUsage` so it can be decremented as we delete spent coins from the child cache, and it looks like it will take many
...

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1624421369)
Added the comment, thanks.

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1624421583)
Fixed

🚀 fanquake merged a pull request: "fuzz: add more coverage for `ScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/30134)

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2145153918)
> > > Do we have charts anywhere tracking % of transactions that are in a cluster size of 1?
> >
> >
> > I will analyze the percentage of cluster size 1 transactions mined in previous blocks.
>
> I tracked recent 1000 blocks from block `842457` to `843457`
>
> ~61% of transactions in the last 1000 blocks were confirmed in a cluster size > 1. ~38% of transactions in the last 1000 blocks were confirmed in a cluster size 1
>
> Transactions: 3974143 Cluster size 1 transactions: 1516505
...

💬 hebasto commented on issue "Guix builds are affected by external environment":
(https://github.com/bitcoin/bitcoin/issues/29754#issuecomment-2145155832)
> Can you confirm this is still an issue / provide a new reproducer / confirm if it's only happening on Ubuntu 23.10.

I can confirm the issue on Ubuntu 23.10, but not [able](https://bugs.launchpad.net/apparmor/+bug/2046844) to test on Ubuntu 24.04 using the system's `guix` package.

💬 TheCharlatan commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-2145163638)
Might be good to note that since #29251 it is also possible to manually specify a clang format binary with the `-binary` option. It should be trivial to install a newer clang format on ubuntu, so I think this can be closed?