💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2513989045)
do we still need `#include <pow.h>` here after the move?

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514018993)
8515594b3b716d05a89b9fef36dfe7e8032d19c7

Do we still need this after https://github.com/bitcoin/bitcoin/pull/33550?

> This is currently only used in fuzzing.

Are we adding dead code in this commit? It's hard to judge if a solution is accurate if we see it before the problem is presented - can we make the question obvious before we provide the answer?

And more concretely: my IDE is confused about the usage of this even at the tip - would it be possible to invalidate the conversions yo
...

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514673448)
What was your reason for instantiating new objects just for comparison, is it just to make sure you're sorting backwards?

```suggestion
return pa->GetBlockHash().Compare(pb->GetBlockHash());
```
or
```suggestion
return pa->GetBlockHash() < pb->GetBlockHash();
```

I don't think that's necessary, the above should likely be safe as well.

But since it's a lot faster to compare pointers for equality (which seems like an important feature, it's why most of these are a set in the first place
...

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514157586)
5a0f0c3fac47053f7d0245ac0b005d4b26cacd48 nit:
```patch
- Since the caller is BasicTestingSetup() is not aware of the random path element
+ Since the caller of BasicTestingSetup() is not aware of the random path element

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514085576)
nit: seems excessive to add a helper for a single usage here:
```suggestion
const CAmount amount_in = Assert(amount_view.GetCoin(outpoint))->out.nValue;
```

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514213312)
5a0f0c3fac47053f7d0245ac0b005d4b26cacd48 commit is also quite big, can we split by functionality instead? I would prefer the commits telling a story, to each make be self-contained as much as possible, to make sense on their own as far as it's reasonable. Can we split this one by features, e.g. adding `rand_path ` to be able to specify strong random, maybe `fuzzcopydatadir` param and usage next, `EnableFuzzDeterminism` last - or something similar

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514076456)

```suggestion
// Expected for truncated/invalid fuzzed messages; swallow to continue exercising code paths.
// TODO validate that the error something we do actually expect here, otherwise anything can happen here
}
```

💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3517820699)
Odd, the invalid-version error is happening at the bottom of this block:

```
self.log.debug("Submit a block with a bad version")
block.nVersion = 0
block.solve()
res = await mining.result.checkBlock(block.serialize(), check_opts)
assert_equal(res.result, False)
assert_equal(res.reason, "bad-version(0x00000000)")
res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())
assert_equal(res.result, False)
self.log.debug("Submit a vali
...

💬 fanquake commented on pull request "depends: drop qtbase_avoid_native_float16 qt patch":
(https://github.com/bitcoin/bitcoin/pull/33850#issuecomment-3517860961)
Guix Build (x86_64):
```bash
945f7146ca27fd741e0512d214171cecb61f9c4d1a07a56821b0c5765aa7db13 guix-build-169f93d2ac88/output/aarch64-linux-gnu/SHA256SUMS.part
ff1debd447ad07d09c29f37ce1da4723aef28c40f56e8ca64c1c83ee80a744a4 guix-build-169f93d2ac88/output/aarch64-linux-gnu/bitcoin-169f93d2ac88-aarch64-linux-gnu-debug.tar.gz
49ebd79348ffad69d3448252896900a3c42c6ac4b0abfc99503887cbff59f190 guix-build-169f93d2ac88/output/aarch64-linux-gnu/bitcoin-169f93d2ac88-aarch64-linux-gnu.tar.gz
5419239
...

🤔 hebasto reviewed a pull request: "depends: drop qtbase_avoid_native_float16 qt patch"
(https://github.com/bitcoin/bitcoin/pull/33850#pullrequestreview-3449037755)
My Guix build:
```
aarch64
945f7146ca27fd741e0512d214171cecb61f9c4d1a07a56821b0c5765aa7db13 guix-build-169f93d2ac88/output/aarch64-linux-gnu/SHA256SUMS.part
ff1debd447ad07d09c29f37ce1da4723aef28c40f56e8ca64c1c83ee80a744a4 guix-build-169f93d2ac88/output/aarch64-linux-gnu/bitcoin-169f93d2ac88-aarch64-linux-gnu-debug.tar.gz
49ebd79348ffad69d3448252896900a3c42c6ac4b0abfc99503887cbff59f190 guix-build-169f93d2ac88/output/aarch64-linux-gnu/bitcoin-169f93d2ac88-aarch64-linux-gnu.tar.gz
5419239208ebce
...

👍 hebasto approved a pull request: "depends: drop qtbase_avoid_native_float16 qt patch"
(https://github.com/bitcoin/bitcoin/pull/33850#pullrequestreview-3449040676)
ACK 169f93d2ac887a7295e87f0a69c2407cdd0a936e.

💬 fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3517903783)
> build libxcb_util_image or skip building the tests.

@tobtoht Thanks. I've pushed a change into the `libxcb_util_image` commit to skip the tests and drop the patch I was applying (not well tested yet).

🚀 hebasto merged a pull request: "depends: drop qtbase_avoid_native_float16 qt patch"
(https://github.com/bitcoin/bitcoin/pull/33850)

💬 stickies-v commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#issuecomment-3517916262)
Force-pushed to add clang-tidy argument names, addressing [feedback](https://github.com/bitcoin/bitcoin/pull/33855#discussion_r2514804401).

🤔 hebasto reviewed a pull request: "kernel: Allow null arguments for serialized data"
(https://github.com/bitcoin/bitcoin/pull/33853#pullrequestreview-3449073010)
Concept ACK.

💬 willcl-ark commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3517958972)
We opened this issue to give Luke a chance to respond to the concerns in the OP. He has replied but IMO has not addressed the concerns around version filtering.

As documented in this thread, `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` is currently not returning any v29 or v30 nodes, which violates point 1 of our DNS seed policy: "The DNS seed results must consist exclusively of fairly selected and functioning Bitcoin nodes from the public network." Multiple independent tests have confirmed th
...

📝 yuvicc opened a pull request: "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState"
(https://github.com/bitcoin/bitcoin/pull/33856)
## Motivation

This PR refactors `ProcessNewBlock()` and `ProcessNewBlockHeaders()` to return `BlockValidationState` by value instead of using out-parameters or boolean returns. This follows the pattern established in #31981 (commit 74690f4) which refactored `TestBlockValidity()` in a similar manner.

### Current Issues
1. **ProcessNewBlockHeaders()**: Uses an out-parameter `BlockValidationState& state`, making the API less intuitive.
2. **ProcessNewBlock()**: Returns a simple boolean wi
...

💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2515053631)
Opened #33856 to address this.

💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3517995745)
From the [issue](https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2388394165) that prompted this:
> I asked for this because of a friend in a country where they are having issues, very real issues and they did not want to doxx themselves

I still think that this is misguided and can get people into "very real issues" if such an option gives the impression that it:

1. Hides the fact that you run a bitcoin node. It does not at all because you will still connect to others at port
...

💬 fanquake commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3518009942)
Guix Build (aarch64):
```bash
451630ecff800ab320a9d5ad8062758df9d700310690110e764c916ae4c8121e guix-build-527acc5ee497/output/dist-archive/bitcoin-527acc5ee497.tar.gz
bf19a3b8e9e9cf609102c38cd6c00dca4d2645f66ae3af591f29fdeceef7b6cf guix-build-527acc5ee497/output/x86_64-w64-mingw32/SHA256SUMS.part
b69dc956f6fd6bb9b4e20e5f4d29eb4ef74e450d250346bf0450cc051b5cfa95 guix-build-527acc5ee497/output/x86_64-w64-mingw32/bitcoin-527acc5ee497-win64-codesigning.tar.gz
9e44c7635597430ee32fb26dff8fcd326
...