👍 vasild approved a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1729314172)
ACK a0687ffd568b0984a8b1e51fd0061f87f25e95d7

Thanks!

💬 bufo24 commented on pull request "log: torcontrol opt checks":
(https://github.com/bitcoin/bitcoin/pull/28780#issuecomment-1809867084)
> Are you still working on this?

no unfortunately not

✅ fanquake closed a pull request: "log: torcontrol opt checks"
(https://github.com/bitcoin/bitcoin/pull/28780)

🚀 fanquake merged a pull request: "build: remove `-bind_at_load` usage"
(https://github.com/bitcoin/bitcoin/pull/28783)

👍 vasild approved a pull request: "net: support unix domain sockets for -proxy and -onion"
(https://github.com/bitcoin/bitcoin/pull/27375#pullrequestreview-1729334104)
ACK 6dddc1c2eda514d35219cce03c229bd6f822be55

💬 dergoegge commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1809928608)
Should we do the same for `make cov_fuzz`? i.e. separate running the fuzz test runner from generating the coverage report.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1809939629)
It would be useful to add a `mempool_backwards_compatibility.py` test to illustrate how the new rules interact with older nodes. It could have two modern nodes and one v25 (or v26) node. Some of the tests you deleted in this branch could be moved there. E.g. the test could demonstrate how RBF rule 2 is not enforced when relaying to the new node, but it is when relaying to the v25 node.

Benchmarks on a 2019 MacBook Pro (2,3 GHz 8-Core Intel Core i9), plugged in:

```
% src/bench/bench_bitco
...

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392325037)
dd6684ab665bc8ae76b76fdd2e578fc77d562a52: While you're touching this, can you rename `MempoolCheck` to `MemPoolCheck`, `MempoolEviction` to `MemPoolEviction` and `ComplexMemPool` to `MempoolComplex`? That makes `-filter=MemPool.*` work

As a workaround, `-filter=.*Mem.*` does work.

💬 maflcko commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1809957950)
> I don't think so? (besides renaming, if we do that). The way I read the docs was that `make check` had to be run first, and separately. I hadn't expected `make cov` to run them for me.

The docs don't mention `make check` and the snippet similar to the docs may also used by external scripts. For example, https://github.com/maflcko/b-c-cov/blob/83bc0912e2a33cfac2195645ab3a12d092fa7ba3/.cirrus.yml#L46-L47

💬 maflcko commented on pull request "tests: Fix LCOV_OPTS to be in the correct position":
(https://github.com/bitcoin/bitcoin/pull/28771#issuecomment-1809977209)
I agree that it is wrong. I mostly wonder why it was working fine previously, when it shouldn't.

💬 fjahr commented on pull request "test: add stress test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1809988538)
I would like it more if you keep the `asmap.py` in seeds because ultimately I want to move it from there to `contrib/asmap` in #28793 and I think the primary function of that code is as a tool, not as a test lib.

And if this is not being run as part of the functional test suite (which I only understood just now), then I am not sure it belongs there. Maybe there is precedent for something similar but to me it feels more like this is something comparable to `test_utxo_snapshots.sh`? I think the
...

💬 maflcko commented on pull request "[WIP] test: migrate to some per-symbol ubsan suppressions":
(https://github.com/bitcoin/bitcoin/pull/28865#issuecomment-1809989839)
Should be fine, if CI passes. Can easily be reverted, if it turns out failing locally.

💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1809997651)
> It's pretty misleading though, since `CScriptNum` is little-endian: if you see `0x0100` you would expect that to be 256, but for us it would actually be non-minimally-encoded 1; For us, `0x0201` would actually be less than `0x0102`, etc.

Those smaller numbers will be shown in decimal still. For larger pushdata, hex is already shown in the little-endian encoded byte order. If `0x` is too suggestive of a big-endian hex number (a fair point), then maybe `bytes(123456789abcdef)` is clearer and
...

📝 BrandonOdiwuor opened a pull request: "GUI getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777)
Fixes https://github.com/bitcoin-core/gui/issues/645.
Add `getrawtransaction` RPC to GUI on `Help -> Verify external txid.` tab


verbose off: `getrawtransaction txid false blockhash`

verbose on: `getrawtransaction txid true blockha
...

📝 hebasto opened a pull request: "depends: Include `config.guess` and `config.sub` into `meta_depends`"
(https://github.com/bitcoin/bitcoin/pull/28870)

💬 willcl-ark commented on pull request "GUI getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1810068057)
Cool, that's pretty nice!

Just thinking out loud, but it does make me wonder if perhaps a new `Verify` or `Utils` button, in line with "Send | Recieve | Transactions" could be even nicer? If we added it there, we might consider adding a few more utils to the GUI like "Decode raw transaction", "Decode Script", "Get Descriptor Info", "Verify Message" etc. which folks who use the gui generally use other software for today...

💬 brunoerg commented on pull request "test: add stress test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1810074189)
> And if this is not being run as part of the functional test suite (which I only understood just now), then I am not sure it belongs there. Maybe there is precedent for something similar but to me it feels more like this is something comparable to test_utxo_snapshots.sh?

I agree. I'm inclined to move it to `contrib` instead of being a functional test. This way we could be more free to test stuff without having to be caution with CI.

💬 fanquake commented on pull request "test: add stress test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1810083470)
> The idea of this test is "stressing" the addrman using asmap.

Can you clarify what this meant to do? What is it stressing, where do the magic numbers "2000, 1/3, 2/3" come from, what is expected to fail under this "stress"?

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1392492963)
The `CreateTransaction()` and `FundTransaction()` want the `CWallet` parameter to be non-const. I'm struggling to find a work around but I'm trying.

Just wanted to clear this, I think the `CWallet` instance inside these functions doesn't seem to be directly mutated or changed anytime. So, do we really want to make it to const. Am I understanding this correct?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392364899)
I made an attempt at dropping `-limitdescendantsize` and friends: https://github.com/Sjors/bitcoin/tree/2022/11/cluster-mempool

I (naively) replaced ancestor and descendent limits in coin selection with the new cluster limit. At least the tests pass.

When we drop these settings anyone who uses them will get an error when starting the node. That's probably a good thing, since they should read about this change.