💬 Prabhat1308 commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1634736227)
What is the intention behind moving salt calculation out of PreRegisterPeer ? Now if PreRegisterPeer is not executed due to lock being held we are doing meaningless computation .

👍 willcl-ark approved a pull request: "[27.1] Finalize"
(https://github.com/bitcoin/bitcoin/pull/30222#pullrequestreview-2110287163)
ACK d756a384d2bebe85f2ce0d192e4d31bbbbe750a1

Version updates and manpages LGTM.

Also ran a scripted backport check to verify no unwarranted changes:

<details>
<summary>output</summary>

```diff
₿ bitcoin-diff-backports --base-branch="upstream/27.x"
1c1
< commit ffbc173ca1ed6b93de8bf3f88b8aed0743f4916c
---
> commit f2e05cd2a9bdc42fa45d284a5162f1f670d07ab6
7a8,10
>
> Github-Pull: #30217
> Rebased-From: ffbc173ca1ed6b93de8bf3f88b8aed0743f4916c
10c13
< index 7f0389b30d
...

💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#issuecomment-2160706291)
Guix Build (aarch64):
```bash
f8487a885a04a4b3c273b2d1ba3ef5e100026f16b03c08e866dbf4cd468d0802 guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/SHA256SUMS.part
b3da7604bc7302213d2864bddbccc54ede6374711377b89b2d17a989d2e9e64d guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/bitcoin-7cbfd7a7ce45-aarch64-linux-gnu-debug.tar.gz
d2340b2c084faf01a0db8ea0c077ba9f8c51ef23b680396d88001b33d126788b guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/bitcoin-7cbfd7a7ce45-aarch64-linux-gnu.tar.gz
a79f46
...

🚀 fanquake merged a pull request: "test: Remove redundant verack check"
(https://github.com/bitcoin/bitcoin/pull/30252)

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634857819)
In d3d653dce34ad1f3235869b8a54f9572e89e5b68 [policy] package rbf

Update places where it is claimed that package RBF is disabled?


```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 8372fcadca1..d18fc1c0f2c 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -602,8 +602,8 @@ public:
/**
* Submission of a subpackage.
* If subpackage size == 1, calls AcceptSingleTransaction() with adjusted ATMPArgs to avoid
- * package policy restrictio
...

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634859550)
> It hits in the fuzzing package_eval fuzzing target.

I was able to reproduce this locally.

🚀 fanquake merged a pull request: "[27.1] Finalize"
(https://github.com/bitcoin/bitcoin/pull/30222)

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634885258)
nit:
```suggestion
// 1 parent paying 199sat, 1 child paying 1300sat.
```

💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160764070)
> > Additionally the template provider needs to wait, to see if there's a new tip, with g_best_block_cv.wait_until. How would I go about moving that into this interface?

Maybe the simplest way to do it would be to have a waitTipChanged() method that just blocks waiting for a new tip, and would be interrupted when the node shuts down or the mining interface is destroyed. If needed, the waiting interface could have more features and accept a timeout or return an object with wait() and cancel()
...

👍 fanquake approved a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260#pullrequestreview-2110469350)
ACK 7ca424f8e651707effe1380aaf72d9ad0e97aa18

👍 theuni approved a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201#pullrequestreview-2110480884)
Tested on Ubuntu 22.04 with downloaded llvm 18 binaries in PATH.

ACK 7cbfd7a7ce45ac68d6041f42f468862f5c193d8c

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923804)
done, and found one more place with out of date comment

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923975)
done

💬 ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2160819048)
> This PR adds tests to cover two scenarios of loading a snapshot when the current chain tip is:
>
> * Not an ancestor of the snapshot block but has less work
>
> * Not an ancestor or a descendant of the snapshot block and has more work
>
> In the second scenario, the snapshot block does not belong to the most-work chain anymore so I believe it covers this scenario too: `TODO: Valid snapshot file and snapshot block, but the block is not on the most-work chain`. Therefore I delet
...

🤔 theuni reviewed a pull request: "build: Bump clang minimum supported version to 16"
(https://github.com/bitcoin/bitcoin/pull/30263#pullrequestreview-2110536012)
Concept ACK.

FWIW on Ubuntu 20.04 I'm using pre-compiled binaries from https://github.com/llvm/llvm-project/releases/tag/llvmorg-18.1.4 with no issues.

👍 tdb3 approved a pull request: "test: add coverage for errors for `combinerawtransaction`"
(https://github.com/bitcoin/bitcoin/pull/30264#pullrequestreview-2110589147)
ACK ab98e6fd03970d6b5a593674c84e762a47b90ea6
Thanks for adding coverage. The new asserts look like they fit in nicely with the existing test code. Ran `rpc_transaction` locally (passed).

💬 theuni commented on pull request "doc: add release note for 29091 and 29165":
(https://github.com/bitcoin/bitcoin/pull/30261#issuecomment-2160864707)
Wait for https://github.com/bitcoin/bitcoin/pull/30263 ?

🚀 fanquake merged a pull request: "test: add coverage for errors for `combinerawtransaction`"
(https://github.com/bitcoin/bitcoin/pull/30264)

💬 brunoerg commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635006887)
> Where is GetTime() called in i2p?

It's used in `Sock` (e.g. `RecvUntilTerminator`).

💬 theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160956259)
Changes look good. The bench is not really useful though, because it's testing things that aren't in our code.

I believe @josibake was asking for a bench that demonstrates a before/after of `CreateTransactionInternal`. I'm guessing that's not really feasible though, so I think it's enough to use your bench numbers without actually committing it.