💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1491028554)
I think this is fine here. We use lower case names for similar `insert` and `size` methods that mimic the standard library in our codebase already, and we always have this guideline in the [dev-notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#internal-interface-naming-style), that indicates that these kind of interface classes could be lower case.

💬 vasild commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1491029750)
Why this sort here? Anybody remembers? It is only used like:

```cpp
g_all_messages[v % g_all_messages.size()]
```

where `v` is a random number. The sort seems unnecessary?

The question arised in: https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1490653235

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-1946127103)
Force-pushed addressing:
https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490381331
https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490757436
https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490780583
https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490807334
https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1490770478

Thanks @vasild and @amitiuttarwar for reviewing!

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1491039911)
Yes but in reverse I think.
Suppose to be the other way around no?

A sorted `FeeFracs` should have undefined chunks first, the highest fee rate chunk second continuously...
If their is a tie, the chunk with lower size comes first.
Hence the are sorted in increasing fee rates, and then by decreasing size.

I've added test to verify this here https://github.com/ismaelsadeeq/bitcoin/commit/8ce89b132f4304a7feda067a88fd0a72330044a6, this test passed on this branch.

⚠️ achow101 opened an issue: "Gathering Priorities of 28.0"
(https://github.com/bitcoin/bitcoin/issues/29439)
Please nominate projects that you are the champion of that could become priorities for the next ~6 months (until the 28.0 release). We (frequent contributors) will vote on which projects will be priorities next week in a separate issue.

There were concerns in the voting of previous priority projects that some projects nominated did not have champions who were willing to have their project be a priority project. To ensure that all nominated projects have champions who want their project to be
...

💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1491046223)
@ismaelsadeeq The comparator you provide to `std::sort` is supposed to return whether the first argument should sort *before* the second one (if no comparator is provided, `operator<` is used). With that, you should see lowest feerate first, highest feerate last, and the undefined feefrac at the very end. Within equal-feerate groups, larger size comes first.

💬 achow101 commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1946144606)
Legacy wallet removal #20160

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1491051732)
Well it's not random, it's a fuzzer input. They're random in a way, but definitely not uniformly random.

I think the sorting is there to make sure that just reorderings in the list don't affect fuzzer behavior.

💬 TheCharlatan commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1946153756)
libbitcoinkernel #27587

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1491054596)
Yes thanks for clarification 👍🏾

💬 josibake commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1946158788)
Silent payments (BIP352) #28536

💬 Sjors commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#issuecomment-1946159253)
Light utACK 0a3b5739ce8b9a2d219bcf3208069436c66fd5bb

💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1491058952)
sorry I got confused by the comment and thought you meant that I changed the log line to include `disk` first and then in the second commit changed it to `file`

I updated `LogPrintf` -> `LogInfo` in [27251e8](https://github.com/bitcoin/bitcoin/pull/29402/commits/27251e89b67f83a40cfd70eeded4120a28f82cf9)

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1491060699)
I looked at it the other way around, using the result from `BuildDiagramFromUnsortedChunks` sort which custom > operator @instagibbs.

💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1491062613)
instead, I opted not to cast the value and instead print out the number of bytes, let me know if MB is strongly preferred and I can find a solution.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-1946168171)
Added `m_tp_mutex` to `Sv2TemplateProvider`.

💬 fjahr commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1946170816)
Default ASMap #28794

There isn't so much left to do in terms of code/review within the repo but it needs some more focus from reviewers to finish it up. Until recently I considered file creation creation and tooling outside of Bitcoin Core a blocker, which I why I didn't propose it for v27. Now I think we got that out of the way with a simple process that has been demonstrated to work with many contributors involved: https://delvingbitcoin.org/t/asmap-creation-process/548

💬 achow101 commented on issue "wallet: pre-HD HDD migratewallet ":
(https://github.com/bitcoin/bitcoin/issues/29438#issuecomment-1946184974)
It seems like most of the time is being spent in reading the wallet rather than writing it?

Can you provide the raw perf data?

💬 furszy commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1946189748)
Prune node rescan #29183.

Mid-size functionality. Mentioning in response to requests from users and contributors.

💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1491090655)
@sdaftuar Indeed. It wouldn't require anything more than dropping those `Assume()` calls, but right now FeeFrac objects represent (the aggregate fee and size of) sets of transactions, rather differences between such aggregates (as would be needed for this use case).