💬 stickies-v commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#discussion_r1143747292)
nit
```suggestion
// The modulus operation is much more expensive than byte
// comparison and addition, so we keep it out of the loop to
// improve performance (see #19690 for discussion).
```

💬 hebasto commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1479583846)
Friendly ping @glozow :)

💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1144830646)
@MarcoFalke so it seems that it is not possible to move this particular entry (nor the bottom two) to the top... Apparently we can re-order our custom templates by changing the filenames to `01_xxx`, `02_xxx` etc., but the two bottom entries, and the security report are just part of the GitHub UI :(

There is a GitHub beta setting to enable the `Report a Vulnerability` big green button for _all_ users (changing it from a duplicate `View Policy` big green button), as I've done now for [my fork]
...

📝 willcl-ark opened a pull request: "build: debug enable addrman consistency checks"
(https://github.com/bitcoin/bitcoin/pull/27300)
The `--enable-debug` flag should default to enabling debug checks, of which
`addrman` consitency checking is one. This PR enables that behaviour.

To opt out of `addrman` consistency checks when running a debug build use the
`-checkaddrman=0` runtime option for `bitcoind` as described in `developer-notes.md`.

fixes #24709

💬 dergoegge commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1479606895)
Concept ACK

📝 fanquake opened a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)
We need to bump this as the current version doesn't compile under `clang-16`, which is blocking upgrading sanitizer/fuzzing infrastructure (see #27298).

Untested. Need to double-check the gperf/patch dropping.

💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1479618243)
Concept ACK.

💬 fanquake commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1479622034)
Looks like we can't drop the patch, as we'd still end up needing gperf, however just adapting our current patch to the new code also doens't build, so we'll need to do more there. In any case, even if fontconfig builds, Qt doesn't work (out of the box) with clang-16, so this seems like a waste of time.

💬 MarcoFalke commented on pull request "build: debug enable addrman consistency checks":
(https://github.com/bitcoin/bitcoin/pull/27300#discussion_r1144866456)
```suggestion
static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{
#ifdef DEBUG_ADDRMAN
1
#else
0
#endif
};
```

style nit (feel free to ignore, if you don't like it)

💬 dergoegge commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1479628226)
Could we drop the GUI from the TSan task?

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1144872917)
Hmmm, traced out a package size of 4, looks like I'm referencing non-existent outputs, not re-using them(maybe both :) ).

Clearly this is confusing and broken either way.

To take a step back, after doing this I realized https://github.com/bitcoin/bitcoin/blob/master/src/bench/mempool_stress.cpp exists. I'm fine if this doesn't get merged if it doesn't add anything new? Helped me learn about the benchmarks if nothing else. Thoughts?

💬 glozow commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1144880345)
Ah true, I suppose `ComplexMemPool` is already benching eviction of packages. Fine with closing. The effort is appreciated :pray:

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1144884349)
I could change the PR to remove this silly bench.. :P

✅ instagibbs closed a pull request: "bench: Expand mempool eviction benchmarking"
(https://github.com/bitcoin/bitcoin/pull/27019)

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1144885003)
Missing `#include <string>` and `#include <string_view>`

👍 stickies-v approved a pull request: "httpserver, rest: fix segmentation fault on evhttp_uri_get_query"
(https://github.com/bitcoin/bitcoin/pull/27253)
ACK 729287530e458febf32e01e542a36e7be4a74fbf

Also checked that the new functional test fails without the patch and succeeds with the patch.

but I think the commit message [should be updated](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1348929370) to better describe the bug, the fix, and the behaviour change introduced

💬 instagibbs commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1479651136)
@furszy is that an ACK? :)

💬 hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1479655825)
Updated 5a7932f395c675fad332cbcd0498bb9fefcb33e0 -> f1370b2c1586f7fe487d9f17ee53bcd9b87a9f23 ([pr26762.04](https://github.com/hebasto/bitcoin/commits/pr26762.04) -> [pr26762.05](https://github.com/hebasto/bitcoin/commits/pr26762.05)):

- rebased
- addressed some of @TheCharlatan's comments

💬 pinheadmz commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1479656461)
> @willcl-ark That still won't work when the pruning point is later than the wallet birth date.

What if it is combined with `getblockfrompeer` logic?

I think we could close this issue in favor of https://github.com/bitcoin/bitcoin/issues/21267 and with the goal of essentially adding neutrino functionality to a pruning node for the sake of wallet rescans and imports ?

💬 hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1144896704)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1479655825).