💬 fanquake commented on issue "Apple Clang 14.0 lacks support for `std::is_eq`":
(https://github.com/bitcoin/bitcoin/issues/29918#issuecomment-2068007481)
Yea. I think this can be closed with "use a compiler that better supports C++20". We still support the mentioned systems, and as far as I'm aware, installing and using a newer compiler would also work fine.

📝 paplorinc converted_to_draft a pull request: "refactor: Reduce memory copying operations in bech32 encoding/decoding"
(https://github.com/bitcoin/bitcoin/pull/29607)
Started optimizing the base conversions in [TryParseHex](https://github.com/bitcoin/bitcoin/pull/29458), [Base58](https://github.com/bitcoin/bitcoin/pull/29473) and [IsSpace](https://github.com/bitcoin/bitcoin/pull/29602) - this is the next step.

Here I've reduced the memory reallocations and copying operations in bech32 encoding/decoding, resulting in decode being `~22%` faster, encode `~32%` faster.

Before:
```
| ns/byte | byte/s | err% | total | benchma
...

💬 paplorinc commented on pull request "refactor: Reduce memory copying operations in bech32 encoding/decoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2068016788)
There seems to be a failure in https://github.com/bitcoin/bitcoin/pull/29607/checks?check_run_id=24070603728 that I can't reproduce locally, converting back to draft - how do I check at least what the failure message is?

💬 pinheadmz commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387)
After catching up on the discussion in #29903 my opinion now is that we close this issue and the pull request. The bitcoin.conf file is self-documented and the datadir location where it belongs is also well documented. If users are opening that file to change settings but ignoring the first line in that file that tells them how to use it, I'm not sure there's anything else the developers can do.

I'd recommend adding a README file in the tar but that probably won't prevent this user behavior e
...

💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2068043063)
The recent push uses approach from the https://github.com/hebasto/bitcoin/pull/157.

📝 hebasto opened a pull request: "ci: Drop no longer needed `-I` flag in "tidy" task"
(https://github.com/bitcoin/bitcoin/pull/29929)
As title says.

💬 maflcko commented on pull request "ci: Drop no longer needed `-I` flag in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2068055365)
lgtm ACK 6f5954acac2ced22dae7088e2b679bf663507d4c

👍 cbergqvist approved a pull request: "test: Extends wait_for_getheaders so a specific block hash can be checked"
(https://github.com/bitcoin/bitcoin/pull/29736#pullrequestreview-2013448704)
ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a

Nice to see the TODO in `p2p.py` resolved!

Searched through source for "getheaders" to make sure the function had been applied when suitable.

Reasoned through the modified test functions.

Modifications look good taking into account the new behavior of `wait_for_getheaders()` popping off the message and requiring the block hash to match the top one if provided.

Built and ran functional tests with `--extended --exclude=feature_dbcrash` wi
...

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068074835)
> Embedding copies of shared code just makes things more fragile, not safer. It's far more likely there's a bugfix that we miss picking up on, than that there's malicious code inserted later.

In addition to what @laanwj said, it's also a much simpler re-implementation. The libevent function provided further options that we didn't care about and hardcoded, that's how we could simplify. And as was discussed in the last IRC meeting, there is also interest in removing libevent in general, at whi
...

💬 laanwj commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068154367)
> So, to be clear, your opinion is for this part I should https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573347809 and introduce a change of behavior, right?

Yes. There's no use in emulating broken C string behavior.

⚠️ asctime opened an issue: "Crash on fail ~CCheckQueue() destructor?"
(https://github.com/bitcoin/bitcoin/issues/29930)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

This will crash the application when false:

```
~CCheckQueue()
{
assert(m_worker_threads.empty());
}
```

### Expected behaviour

Possibly do something more like this?
```

~CCheckQueue() {
try {
if (!m_worker_threads.empty()) {
// Attempt to stop worker threads if they haven't been stopped yet.
S
...

💬 luke-jr commented on pull request "ThreadSanitizer: Fix #29767":
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068164464)
>It seems like the race condition between m_synced = true and committing has been around since 4368384 from #14121, and the race condition accessing the m_next_filter_pos variable existed then too.

Until #28955, `cs_main` was held across all this to prevent a race.

Not going to look at it right now, but I suspect this fix just creates a different race instead.

⚠️ asctime opened an issue: "ReadAnchor throws exception on second run"
(https://github.com/bitcoin/bitcoin/issues/29931)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
{
std::vector<CAddress> anchors;
try {
DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
} catch (const std::exception&) {
anchors.clear();

...

💬 1440000bytes commented on issue "Crash on fail ~CCheckQueue() destructor?":
(https://github.com/bitcoin/bitcoin/issues/29930#issuecomment-2068178196)
> Open settings in bitcoin-qt, adjust and save.

I am not able to reproduce this. What exactly did you adjust in settings before clicking on ok?

💬 luke-jr commented on pull request "ThreadSanitizer: Fix #29767":
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2068179144)
Looks like furszy already got this in #29867

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899433)
Thanks for adding me as a co-author, if you force push again, please change the email to `Lőrinc <pap.lorinc@gmail.com>`

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2068180640)
I have added the behavior change in a separate commit.

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573899901)
The behavior change is now re-introduced but explicitly documented as such in a separate commit.

💬 paplorinc commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573900516)
would it make sense to mention the `&` as well?

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1573902309)
Are these commonly decoded as a space as well? I couldn't find evidence for this but I didn't spend much time on it.