💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831459829)
I guess this could use a bug label too if someone with permission can get around to that :)

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2060900943)
(Done in latest push from ~8 hours ago).

📝 kevkevinpal opened a pull request: "tests: Added progress tracker when running fuzz test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/32354)
Right now if you run `test_runner.py` on all the targets it is hard to tell how far you are in the progress even with `--loglevel=DEBUG` on. As it just spits out the target that was completed an no info on how many targets are left.

This change gives you a percentage in 5% increments on how far along you are finishing the targets you've selected

This also applies when you use `-g/--generate` and `--m_dir`

I can append this change to either change the percent increments or move it to th
...

💬 jonatack commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#issuecomment-2831574347)
ACK 4b6171982a20d736b2c627ab9b7ea788b06af457

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061100509)
In commit "refactor: Operate on bytes instead of bits in Asmap code"

This is not correct. The code (and comment) above indicate that `buffer` is being interpreted using 1 bit of asmap code per byte. The line here is interpreting it as 1 byte per byte.

You need some conversion function, I think:

```c++
namespace {

std::vector<std::byte> BitsToBytes(std::span<const uint8_t> input) noexcept
{
std::vector<std::byte> ret;
uint8_t next_byte{0};
int next_byte_bits{0};

...

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061112524)
In commit "refactor: Use span instead of vector for data in util/asmap"

No need for the explicit conversion:
```diff
-if (SanityCheckASMap(std::span<const std::byte>(asmap), buffer.size() - 1 - sep_pos)) {
+if (SanityCheckASMap(asmap, buffer.size() - 1 - sep_pos)) {
```

Also twice the same further in this function.

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096624)
In commit "refactor: Operate on bytes instead of bits in Asmap code"

I don't think this is necessary? The code above could operate on `asmap` instead of `asmap_vec` directly?

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061096369)
In commit "refactor: Operate on bytes instead of bits in Asmap code"

Simpler:

```c++
asmap_vec.push_back(std::byte(buffer[1 + i]));
```

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061101276)
In commit "refactor: Operate on bytes instead of bits in Asmap code"

This doesn't make sense anymore. You can't slice off individual bits from an `std::byte`.

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061109899)
"In commit "refactor: Operate on bytes instead of bits in Asmap code":

I think it would be helpful to have a bit getter helper function:

```c++
inline bool GetBit(std::span<const std::byte> bytes, uint32_t bitpos) noexcept
{
return (std::to_integer<uint8_t>(bytes[bitpos / 8]) >> (bitpos % 8)) & 1;
}
```

And then for example:
```c++
...
bit = GetBit(data, bitpos++);
...
```

and

```c++
...
if (GetBit(ip, ip.size() * 8 - bits)) {
--bits;

...

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2061094811)
In commit "refactor: Operate on bytes instead of bits in Asmap code"

Simpler:
```diff
--- a/src/netgroup.cpp
+++ b/src/netgroup.cpp
@@ -90,18 +90,18 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
if (address.HasLinkedIPv4()) {
// For lookup, treat as if it was just an IPv4 address (IPV4_IN_IPV6_PREFIX + IPv4 bits)
for (int8_t byte_i = 0; byte_i < 12; ++byte_i) {
- ip_bits[byte_i] = static_cast<std::byte>(IPV4_IN_IPV6_PREFIX[by
...

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114715)
I don't think this is needed. `TxGraph` doesn't promise a specific ordering of equal-feerate diagram segments. The implementation is allowed to be more specific than the interface promises.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061114932)
No, because it's computing the diagram for the linearization `vec1`, which is constructed using the real graph's transaction comparison function.

It's just using `sims[0].graph` to pull the feerate information from.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2061115163)
Sure, the caller is responsible for not violating the contract of the interface.

🚀 hebasto merged a pull request: "Replace stray tfm::format to cerr with qWarning"
(https://github.com/bitcoin-core/gui/pull/868)

💬 hebasto commented on pull request "build: Fix `macdeployqtplus` after switching to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2061202483)
Thanks! Adjusted as well.

📝 luke-jr opened a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355)
PR #30356 incorrectly changed a constant of `4000` to `m_options.coinbase_max_additional_weight` in the check for when to give up finding another transaction to fill the block:

```diff
if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight >
- m_options.nBlockMaxWeight - 4000) {
+ m_options.nBlockMaxWeight - m_options.block_reserved_weight) {
// Give up if we're close to full and haven't succeeded in a while

...

💬 shahsb commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#discussion_r2061229822)
I do not see the call to the function `DescriptorScriptPubKeyMan::CanUpdateToWalletDescriptor()` being made in this test case?

💬 shahsb commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#issuecomment-2831976141)
Concept ACK

💬 hebasto commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2831979533)
> [@hebasto](https://github.com/hebasto) can you followup with all the questions asked in this thread:
>
> [#31491 (comment)](https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2573886240)
>
> It'd be good to get some clarification on which behaviour is meant to work.

@theuni

> [@hebasto](https://github.com/hebasto) Have you experimented with using regular cache variables as opposed to the _INIT ones?

Cache variables are designed to be set by the user. With the suggested approach
...