💬 laanwj commented on issue "build: `guix` package was removed from debian":
(https://github.com/bitcoin/bitcoin/issues/33982#issuecomment-3596225176)
> A 1.5.0 release is planned for sometime in January 2026: https://codeberg.org/guix/release-planning/wiki/release-1.5.0-project/. So hopefully the situation is going to improve in future.

"Wait for 1.5.0" SGTM, too.

If it isn't re-added i think it makes sense to remove the mention of the package from the docs completely, as most people are bound to run the newest debian release. The wording of "various releases" is a bit vague, more people are likely going to run into this.

So let's keep thi
...

🤔 rkrux reviewed a pull request: "validation: Improve warnings in case of chain corruption"
(https://github.com/bitcoin/bitcoin/pull/33553#pullrequestreview-3524622915)
Concept ACK 53ef86f0ecd4ca2b66c4e3dce591ac4718e38f0e

Adding the warning in such a case should help the user in diagnosing the issue; I have limited exposure to this area, hence refraining from giving a full a-c-k.

💬 rkrux commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2576795045)
Looking forward to this functional test!

👍 rkrux approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3524889277)
lgtm ACK 189437a3dedefce3945ab2a8c05785eb283c2c6f

Shared a non-blocking nit.

💬 rkrux commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2576993777)
Nit: some consistency in the warning format would be easier on the eyes.

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 8289120164..132d695a1a 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1557,11 +1557,11 @@ public:
const uint32_t raw = node.k;
const uint32_t value_part = raw & ~CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
if (value_part > CTxIn::SEQUENCE_LOCKTIME_MASK) {
-
...

👍 l0rinc approved a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3524866538)
The latest version splits out a few trivial moves with explanations to unburden heavier commits. I particularly like how `PrivateBroadcast` methods are added commit by commit when they're actually needed.
The signal-to-noise ratio is also improved significantly now - irrelevant debug logs aren't dominating anymore.
We also finally found a way to write the priority comparator in an intuitive way that the CI gods didn't reject!
`GetHash()` calls for hash and equality have been migrated to witne
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2577109437)
to get 100% coverage of `PrivateBroadcast`, we could add:
```C++
BOOST_CHECK(!pb.DidNodeConfirmReception(nonexistent_recipient));
```
to exercise:
https://github.com/bitcoin/bitcoin/blob/2d398050ee31db05e9222149b5e60572ac31d803/src/private_broadcast.cpp#L82

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2576975959)
nit:
```suggestion
* Avoid calling this for other peers since it will degrade privacy.
```

💬 maflcko commented on pull request "refactor: replace manual promise with SyncWithValidationInterfaceQueue":
(https://github.com/bitcoin/bitcoin/pull/33962#discussion_r2577151438)
> Nit: The `callback_set` variable seems unnecessary now.

Why would it be unnecessary now? Sure, it is harmless to call `SyncWithValidationInterfaceQueue` when the tx is in the mempool, but this will need to be mentioned

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2577234887)
doc: Add design notes for cluster mempool and explain new mempool limits

Grammar: "we can can compare" -> "we can compare"

(thanks DrahtBot LLM)

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2577225993)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"

With these options made debug-only, maybe it's better to list the defaults?

> Transactions submitted to the mempool must not result in clusters that would
exceed the cluster limits (64 transactions and 101 kvB total per cluster).

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2577238632)
In commit "doc: add release notes snippet for cluster mempool"

"overriden" -> "overridden"

(thanks to DrahtBot LLM)

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2577251357)
In commit "rpc: improve getmempoolcluster output"

`all_chunks.push_back(std::move(chunk))` will avoid a copy

💬 billymcbip commented on pull request "script: Remove dead code from OP_CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33977#issuecomment-3596782767)
I think it wouldn't hurt to have an `assert(!stack.empty());` instead, just in case the code above is ever refactored.

🤔 sipa reviewed a pull request: "Cluster mempool followups"
(https://github.com/bitcoin/bitcoin/pull/33591#pullrequestreview-3525225192)
ACK 48c29fbe0974664e28564af07fbfd753cf125c36

See nits above.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577293052)
That would be racy. We could have the worker thread count be a variable instead of hard coded, then for a test we could make it zero.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577297663)
Oh, we override so we make sure all threads are stopped before we do the batch write. This is in a comment two lines below.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577299367)
Looks like this needs more work, it's not as easy as I though - we could use an `std::deque` instead of a `vector` here:
```patch
Subject: [PATCH] std::deque<InputToFetch> m_inputs
---
diff --git a/src/coinsviewcacheasync.h b/src/coinsviewcacheasync.h
--- a/src/coinsviewcacheasync.h (revision a3f56354d6e3f64eaca84a16e4951e6073090f60)
+++ b/src/coinsviewcacheasync.h (revision 462408b897197de3a7067dcbdee318ad9dc1e546)
@@ -17,6 +17,7 @@
#include <atomic>
#include <barrier>
#include <cs
...

💬 vostrnad commented on pull request "script: Remove dead code from OP_CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33977#issuecomment-3596822343)
The script interpreter code is so thoroughly tested, so infrequently refactored, and any change to it reviewed with such extreme caution that I don't think we need to bother with an assert, but I can add it if others think it's a good idea.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577321551)
you sure it would be racy?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2577345226)
> I disagree, I think the current move construction is incorrect and if I understand it correctly, we should reserve instead and delete (or ignore) the other constructors.

I think you are disagreeing with my statement:

> since it only happens before we start doing work, might as well make it simple and not bother moving the other fields

because the other things I said were facts. Do you think we should also construct new `atomic_flag`s when doing the move construction? As I mentioned I
...