💬 Sjors commented on issue "intermittent issue in p2p_orphan_handling.py":
(https://github.com/bitcoin/bitcoin/issues/31700#issuecomment-2720869273)
cc orhpanage fans @glozow, @sipa, @mzumsande

📝 janb84 opened a pull request: "contrib: Fix deterministic-unittest-coverage tool path"
(https://github.com/bitcoin/bitcoin/pull/32055)
Fix for the tooling introduced/modified in #31901 but the tool path is broken due to silent merge conflict introduced by #31161.

The `deterministic-unittest-coverage` and `deterministic-fuzz-coverage` tools uses the `fuzz` and `test_bitcoind` binaries, for which the location was modified in #31161. This patch updates the location to align with that change.

💬 janb84 commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#issuecomment-2720894341)
Fix created in https://github.com/bitcoin/bitcoin/pull/32055

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2720894982)
Rebased after #31283. There was no conflict, but the PR description makes reference to `waitNext()` so this should make it easier to (re-)review with that context in mind.

💬 TheCharlatan commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#issuecomment-2720898462)
> Maybe what you are are suggesting could be an improvement, through, and it's been a while since I thought about this and reasons I listed above are speculative. Could try implementing the suggestion and see if it simplifies the code.

This is what I had in mind: https://github.com/TheCharlatan/bitcoin/commit/9c89548c670e7b807a969e13408ff03df15143a2 (also rebased this branch to get cmake) . It is very broken in its current state though. There are a few problems with it, but I guess the fundam
...

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1993309811)
I don't understand this. Don't the docs claim that `set -e` is set for bash/sh?

And if `&& '` is needed here, wouldn't it be required in all other `run`s as well that span over more than one line?

💬 maflcko commented on pull request "contrib: Fix deterministic-unittest-coverage tool path":
(https://github.com/bitcoin/bitcoin/pull/32055#issuecomment-2720906097)
lgtm ACK 893ca5458503b432b055f0cf8a9a5aa2cfe7f6e4

Haven't tested this, but it looks plausible.

💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2720917354)
The recent [feedback](https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1988501276) regarding verbosity has been addressed.

Rebased on top of bitcoin/bitcoin#31161.

💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1993318514)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2720917354).

💬 l0rinc commented on pull request "[IBD] specialize CheckBlock's input & coinbase checks":
(https://github.com/bitcoin/bitcoin/pull/31682#discussion_r1993324156)
By the way, the distributions of inputs lengths in `block413567` used in `CheckBlockBench ` are:

size | count
|---:|-----:|
| 1 | 1052
| 2 | 255
| 3 | 113
| 4 | 26
| 5 | 41
| 6 | 22
| 7 | 14
| >=8 | 33

<details>
<summary>>=8</summary>

```bash
8
8
8
8
8
9
9
10
10
12
12
12
14
17
17
21
24
25
26
27
31
32
35
45
56
80
90
120
200
200
200
274
357
442
```

</details>

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2720929755)
Occasional rebase.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1993326662)
> I don't understand this. Don't the docs claim that `set -e` is set for bash/sh?

But it is not `bash` or `sh` in the Windows environment.

👍 hebasto approved a pull request: "contrib: Fix deterministic-unittest-coverage tool path"
(https://github.com/bitcoin/bitcoin/pull/32055#pullrequestreview-2681554633)
ACK 893ca5458503b432b055f0cf8a9a5aa2cfe7f6e4, I have reviewed the code and it looks OK.

🚀 hebasto merged a pull request: "contrib: Fix deterministic-unittest-coverage tool path"
(https://github.com/bitcoin/bitcoin/pull/32055)

📝 hebasto opened a pull request: "doc: Adjust path in comment"
(https://github.com/bitcoin/bitcoin/pull/32056)
It was overlooked in bitcoin/bitcoin#31161.

💬 hebasto commented on pull request "doc: Adjust path in comment":
(https://github.com/bitcoin/bitcoin/pull/32056#issuecomment-2720986013)
cc @dergoegge

👍 dergoegge approved a pull request: "doc: Adjust path in comment"
(https://github.com/bitcoin/bitcoin/pull/32056#pullrequestreview-2681604534)
ACK de1ada079bf52b9d751bbeb1e7bb3011beb62429

💬 yancyribbens commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2721005512)
I believe it's possible for effective_value to _not_ be equal between two successive UTXO combinations while two fees _are_ equal. This would have the consequence of _not_ applying the optimization when in principle the optimization _should_ be applied. In fact, I think it's very likely for two fees to be equal since it's likely the fee_rates are equal and the weight is equal of two UTXOs. Therefore, leaving this condition in has the disadvantage of nullifying the effect of the UTXO optimizat
...

💬 moonsettler commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#discussion_r1993374840)
Computation required by CTV ideally would not be carried out when CTV could not have been active.
In this context the `tx.version` field is available. If CTV required version 3 or higher (not sure this is a good idea or not?), then it would be trivial to augment the old logic that was more conservative with resource use. (again not sure how much that matters in practice?)

💬 Nouridin commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2721031585)
Your reasoning is sound. If you consider that many UTXOs will have equal fees—because they share the same fee rate and weight—then requiring both the effective value and the fee to match before applying the shortcut can indeed be overly strict. In scenarios where the fees are identical but the effective values differ slightly (perhaps due to minor rounding or variation in the UTXO values), the condition
`utxo.fee != utxo_pool.at(utxo_pool_index - 1).fee`

will cause the optimization to be bypass
...