💬 fanquake commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3517687922)
https://github.com/bitcoin/bitcoin/actions/runs/19269384275/job/55093373232?pr=33819#step:7:3549:
```bash
Error: node0 2025-11-11T15:38:48.659039Z [unknown] [validation.cpp:4535] [bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock> &, bool, bool, bool *)] [error] ProcessNewBlock: AcceptBlock FAILED (bad-version(0x00000000), rejected nVersion=0x00000000 block)
```

💬 tobtoht commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3517715698)
>Still some issues making libcxb fully static.

Just sharing this here in case it is useful: https://github.com/tobtoht/bitcoin/commit/24cfce443aedb2c7cb7ac3266029ad01af2ec47c

There probably is a better way to build `libxcb_util_image` or skip building the tests. Rather than editing these autogenerated scripts (and without reintroducing autotools).


```
$ lddtree bitcoin-qt
bitcoin-qt (interpreter => /lib64/ld-linux-x86-64.so.2)
libfontconfig.so.1 => /usr/lib/libfontconfig.so.1
...

🤔 l0rinc requested changes to a pull request: "fuzz: compact block harness"
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3447750850)
Concept and approach ACK, thanks for tackling this.

I like how you've extracted the first few commits that I could quickly get out of the way to continue to the more difficult ones - which are a bit too difficult though, it would help me personally if we could split it into smaller functional chunks, so that the reviewers are guided through the change. The commit messages could give extra context where needed. I don't think we should split by functions, but rather functionalities so that each c
...

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514060602)
I have the same problem in `71719d172e3a435dd983293fc9da6a08974f8b01` - we're introducing dead code, I don't have a way to judge if this is indeed the correct solution, since up to this point there wasn't any pain that this alleviates.
Looking at the commit I see that `setup_validation_interface_no_scheduler` is always false therefore we can delete the code - and I have to check the next commits and come back here to reevaluate that.
Can we add a simpler fuzz test which needs this in the same
...

💬 l0rinc commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2514170270)
This commit is huge, it's hard to review it meaningfully. Can we separate some of these cases to independent commits that can provide more context?

💬 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.