💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481344639)
> Looks like we can't drop the patch, as we'd still end up needing gperf

Maybe include `gperf` in our build tools (like `bison`)?

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

Fixed in #27314.

💬 beirut-boop commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481344902)
@furszy I understand, concur, and have removed the link from my comment.

> Time is a scarce resource for everyone.

I agree. That's why I thought a quick assessment on the observed issue with the coin control classes before spending time setting up a Bitcoin testnet wallet with the intention of taking up more of your time to look into it was a good approach. The project is different but the coin control classes are identical, so I assumed I was helping everyone.

Apologies. I will not cro
...

💬 fanquake commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481351232)
> Maybe include gperf in our build tools (like bison)?

I'd rather not add even more gui-only requirements, and dependencies to depends or our release env.

Going to close this, as I don't think I'm going to follow up.

✅ fanquake closed a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)

💬 fanquake commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1481355808)
> I feel like I'm missing some context here. Why move these out of compat?

My plan was to split compat up further, so we didn't have the singular compat.h, with everything thrown in.

🚀 fanquake merged a pull request: "test: Replace threading with concurrent.futures"
(https://github.com/bitcoin/bitcoin/pull/27287)

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1481372074)
CI's hate this one weird PR.

Updated c661be4e7fd9abe5bbec0a9b123dc66d04ffc1bf -> 780c696fc310a6df8464b40983b23f8b0a3074f0 ([`pr/ignoredconf.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.3) -> [`pr/ignoredconf.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.3..pr/ignoredconf.4)) to fix CI errors

---

```
tuple[] TypeError: 'type' object is not subscriptable`
```
https://cirrus-ci
...

👋 hebasto's pull request is ready for review: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481382733)
Rebased on top of the merged #27311.

All @martinus's comments have been addressed.

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1146360114)
[Updated](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481382733).

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481390885)
> > It would be better to have two separate commits - one with the non-functional optimization and one with the fix.

> Agreed, that could probably be improved.

I can do this.


> > Sure, lets first see what happens with this PR. Maybe @pablomartin4btc would decide to check the return value of evhttp_request_get_uri() in this PR.

It was under my radar and I thought to do it in a follow up, but I could do it here if it's more practical (need to update title and description of this PR)
...

📝 hebasto converted_to_draft a pull request: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)
null

💬 fanquake commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481400657)
Seeing lots of https://cirrus-ci.com/task/6739422046060544?logs=ci#L339:
```bash
/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/server.cpp:88:9: error: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front()));
^
/tmp/cirrus-ci-build/ci/scratch/bui
...

💬 fanquake commented on pull request "p2p: remove adjusted time":
(https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1481401163)
Rebased on master. @ajtowns putting aside the current comments, any thoughts on including changes like this in bitcoin-inquisition?

💬 TheCharlatan commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1481403430)
Light code review ACK cf0d86ed15041822e2a0a55ddf231af7809a9e66

👋 hebasto's pull request is ready for review: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)

💬 achow101 commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1146387794)
Yes, `FundTransaction` currently throws away `nLockTime` thereby disabling anti-fee-sniping, and this preserves that behavior.

💬 hebasto commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1481425866)
> Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored.

Concept ACK on that.

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481428896)
> Seeing lots of https://cirrus-ci.com/task/6739422046060544?logs=ci#L339:

Sorry. Pushed a wrong branch.

Should be OK now.

✅ glozow closed an issue: "Plumb "too-long-mempool-chain" to RPC error for send/sendtoaddress"
(https://github.com/bitcoin/bitcoin/issues/23144)