💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1909471752)
Done.

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2581313846)
@ryanofsky

> Code review ACK [06ff942](https://github.com/bitcoin/bitcoin/commit/06ff9429d32943aa4debec38ea105c8af4283aec). It seems like a improvement to me overall if binaries in the build directory have the same layout as binaries in releases. I'm not sure current implementation of this is ideal but it seems like a a step in the right direction.
>
> Having binaries built in consistent locations that match install locations should make it is easier to find them, make scripts and wrapper
...

💬 theuni commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909477090)
> > Why is this doing an effective swap?
>
> I think this is quite common, that move-construction is effectively performing a swap.
>
> > I would expect this to call `UnlinkRef` on the moved-from value and reset its `m_graph` and `m_index`
>
> That's possible too, and slightly more efficient I guess.
>
> > Otherwise it wouldn't be unlinked until the moved-from variable goes out of scope, no?
>
> Indeed. I don't think that's a problem.

Afaik the move/swap idiom is only safe if t
...

💬 theuni commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1909485967)
Yes, it's from `coreutils`. But shasum exists out of the box.

💬 theuni commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r1909492867)
Ah, right, I missed that the move ctor would handle the update. Thanks for explaining.

💬 psgreco commented on issue "Potential crash (assert) rescanning wallet":
(https://github.com/bitcoin/bitcoin/issues/31474#issuecomment-2581367570)
Looks good, thank you!
Do you want me to ack or do anything in https://github.com/bitcoin/bitcoin/pull/31629 ?

💬 achow101 commented on pull request "test: raise explicit error if any of the needed release binaries is missing":
(https://github.com/bitcoin/bitcoin/pull/31462#issuecomment-2581434327)
ACK 1ea7e45a1f445d32a2b690d52befb2e63418653b

🚀 achow101 merged a pull request: "test: raise explicit error if any of the needed release binaries is missing"
(https://github.com/bitcoin/bitcoin/pull/31462)

✅ achow101 closed an issue: "wallet: wallet_migration.py fails on sqlite-only build"
(https://github.com/bitcoin/bitcoin/issues/31447)

🚀 achow101 merged a pull request: "wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled"
(https://github.com/bitcoin/bitcoin/pull/31451)

🚀 achow101 merged a pull request: "build, test: Build `db_tests.cpp` regardless of `USE_BDB`"
(https://github.com/bitcoin/bitcoin/pull/31617)

💬 achow101 commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2581498389)
ACK a96b84cb1b76e65a639e62f0224f534f89858c18

🚀 achow101 merged a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549)

💬 pinheadmz commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1909648045)
I noticed this while reviewing https://github.com/bitcoin/bitcoin/pull/27468 (comments locked on that PR) for my own work on `HTTPRequest` -- this test vector doesn't have a `?` so even if the URI didn't contain invalid characters it would still return null for the query parameter request. It doesn't really matter for the invalid-URI-throws-error test but it should matter for this follow up, part of which I'm trying to integrate.

So, my question is - should the test vector always have been:

...

💬 davidgumberg commented on pull request "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor":
(https://github.com/bitcoin/bitcoin/pull/31590#discussion_r1909748960)
nit: The equality operator could be explicitly defaulted for `SigningProvider`:

```diff
diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h
index 5b1da681f8..8262db0b52 100644
--- a/src/script/signingprovider.h
+++ b/src/script/signingprovider.h
@@ -153,6 +153,7 @@ class SigningProvider
{
public:
virtual ~SigningProvider() = default;
+ virtual bool operator==(const SigningProvider&) const = default;
```

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1909955145)
@hodlinator since you initially proposed the current solution, do you have any fresh opinions here?

📝 barajeel opened a pull request: "fix: Optimized Performance Update bloom.cpp"
(https://github.com/bitcoin/bitcoin/pull/31632)
I made several optimizations to the Bloom filter code to enhance its performance and readability:

- Removed redundant calculations by precomputing frequently used values.
- Added early exits in loops to avoid unnecessary iterations.
- Retained efficient bitwise operations for setting and checking bits.
- Improved memory resizing for the `CRollingBloomFilter` by using bit shifts.
- Simplified conditionals in the `IsRelevantAndUpdate` method for better clarity.
- Improved comments to bette
...

✅ maflcko closed a pull request: "fix: Optimized Performance Update bloom.cpp"
(https://github.com/bitcoin/bitcoin/pull/31632)

💬 maflcko commented on pull request "depends: Use base system's `sha256sum` utility":
(https://github.com/bitcoin/bitcoin/pull/31626#issuecomment-2582053263)
Could add freebsd to the title now?

💬 maflcko commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2582087785)
post-merge reACK a96b84cb1b76e65a639e62f0224f534f89858c18 (only change is adding missing mocktime in init)