📝 maflcko opened a pull request: " ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG "
(https://github.com/bitcoin/bitcoin/pull/33888)
The sanity check to check the last few merge commit signatures on the main branch was accidentally and silently disabled while moving from cirrus-ci to GHA.

So fix that by re-enabling it.

Also, contains a few other lint cleanup commits.

💬 hebasto commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3542936019)
> Yeah, if it is pulled in through LIEF, it could make sense to strip the unmaintained dependency from LIEF upstream.

FWIW:
```
# guix graph --path python-lief python-py-cpuinfo
python-lief@0.17.1
python-pydantic@2.10.4
python-pytest-benchmark@5.1.0
python-py-cpuinfo@9.0.0
```

(guix checked out at a different commit though)

🤔 l0rinc requested changes to a pull request: "http: replace WorkQueue and single threads handling for ThreadPool"
(https://github.com/bitcoin/bitcoin/pull/33689#pullrequestreview-3473454632)
Left a few more comments to help with moving this forward.
I like the new structure more and after the tests and the restructuring are finished, I would like to spend some more time reviewing the `ThreadPool` as well - but I want to make sure we have a solid foundation first.

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534665307)
Can you please apply this to the other cases as well?

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534663536)
Can you please apply this to the other cases as well?

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534655978)
Continuing the thread from https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477534828:

```patch
#include <common/system.h>
+#include <test/util/setup_common.h>
#include <util/string.h>
```

```suggestion
BOOST_CHECK_EXCEPTION(threadPool.Submit([]{ return false; }), std::runtime_error, HasReason{"No active workers; cannot accept new tasks"});
```

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534675487)
Continuing the discussion in https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477876362, could we randomize this on the call-site, so that we can exercise cases when we have other than 3 workers, i.e. in the test it would be something like:
```C++
const auto num_tasks{1 + m_rng.randrange<size_t>(20)};
```

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534738205)
The original failure looks like this:
> unknown location:0: fatal error: in "threadpool_tests/submit_tasks_complete_successfully": std::runtime_error: Timeout waiting for: test1 task, task index 0
>
>test/threadpool_tests.cpp:77: last checkpoint: "submit_tasks_complete_successfully" test entry

If you want the extra method to not mask the failure stack, we can make it a macro, like we did with e.g. https://github.com/bitcoin/bitcoin/blob/cc5dda1de333cf7aa10e2237ee2c9221f705dbd9/src/test/headers
...

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534746780)
I still see init + reserve + loop + emplace in most tests, could we apply to the rest as well?

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534813432)
> we want to exercise some concurrency too

You're right, that's better indeed.
My suggestion updated to keep the vector

```C++
// Test 5, throw exceptions and catch it on the consumer side
BOOST_AUTO_TEST_CASE(task_exception_propagates_to_future)
{
ThreadPool threadPool("exception_test");
threadPool.Start(NUM_WORKERS_DEFAULT);

const auto make_err{[&](size_t n) { return strprintf("error on thread #%s", n); }};

const auto num_tasks{1 + m_rng.randrange<size_t>(20)}
...

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2534845237)
Not exactly sure what you mean by that - `Note 2` does seem like a good idea to me and it's probably similar to what I meant. It would help the review process to gain more confidence in the implementation if we migrated away in smaller steps, documenting how the tests we add for the old implementation also pass when we switch over to the new implementation.

💬 waketraindev commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2534893018)
```suggestion
const uint8_t value = std::to_integer<uint8_t>(byte);
for (int bit = 0; bit < 8; ++bit) {
asmap_bits.push_back((value >> bit) & 1);
}
```

Extract `value` so the `std::to_integer` cast is only called once per byte instead of once per bit.

💬 plebhash commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3543028239)
sorry, I should retract the message above

I just realized there is a `getCoinbaseMerklePath`!

so indeed, there's no need to do a `getBlock` every time!

📝 djkazic opened a pull request: "guix: update per-host disk-space estimates in build gate"
(https://github.com/bitcoin/bitcoin/pull/33889)
Today, the build gates are obsolete due to a change made in the build process (`-static-libgcc`). This PR aims to readjust the build gate disk space per-host estimates based on some testing I did locally in my build VM.

This would prevent any surprises at build-time for guix for there not being enough free disk space.

(Copied from https://github.com/kevkevinpal/bitcoin/issues/200):

I ran `HOSTS="x86_64-w64-mingw32" ./contrib/guix/guix-build`

After (+10 GiB):
```
Filesystem 1G-b
...

💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534670188)
typo nit
```suggestion
* @brief Processes and validates the provided block header.
```

💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534865290)
This should be `LogError`

💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534945865)
I'm not sure why this function is included in this PR, it doesn't seem to be related to any of the block header functionality? Would make more sense to leave it out?

💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534716218)
nit: inconsistent capitalization
```suggestion
* @return Block header, or null on error.
```

💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534948549)
These docstrings don't add any value imo.

💬 fanquake commented on pull request "guix: update per-host disk-space estimates in build gate":
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2534956518)
> due to a change made in the build process (-static-libgcc).

Can you explain a bit more how this is related to `-static-libgcc`, given you haven't changed the allowance for any Linux builds, and macos/mingw don't use `-static-libgcc`?