💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2228153853)
Also added `getHashPrevBlock()`.

💬 hebasto commented on pull request "OptionsDialog: Allow Maximize of window":
(https://github.com/bitcoin-core/gui/pull/826#issuecomment-2228155212)
The PR description is empty, so I have to ask: why is this needed?

🤔 hebasto reviewed a pull request: "OptionsDialog: Prefer to stretch actual options area rather than waste space"
(https://github.com/bitcoin-core/gui/pull/827#pullrequestreview-2177355412)
Tested b71bfd9eef8b0017cef3c05361c02ddbd837e6ac on Ubuntu 24.04 + Qt 5.15.13:

- the master branch @ 6ae903e24afa95214e59a9dc87dc74db3535a5b5:

- this PR:

@GBKS What do you think?

💬 GBKS commented on pull request "OptionsDialog: Prefer to stretch actual options area rather than waste space":
(https://github.com/bitcoin-core/gui/pull/827#issuecomment-2228175727)
That is a nice improvement.

💬 1440000bytes commented on issue "scriptPubKey no address":
(https://github.com/bitcoin/bitcoin/issues/30450#issuecomment-2228177423)
https://github.com/bitcoin/bitcoin/issues/22524#issuecomment-884663685

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2228190409)
Fixed some outdated information in the remainin design doc and added that to the last commit.

✅ fanquake closed an issue: "fuzz: crypter: Abrt in __cxxabiv1::failed_throw"
(https://github.com/bitcoin/bitcoin/issues/30251)

🚀 fanquake merged a pull request: "fuzz: fix key size in `crypter`"
(https://github.com/bitcoin/bitcoin/pull/30373)

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2228203041)
Also added `getBlockHeader()` which the Template Provider needs for the `NewTemplate` message and a few other places.

📝 fanquake opened a pull request: "depends: remove Darwin ENV unsetting"
(https://github.com/bitcoin/bitcoin/pull/30451)
Now that we use the native compiler, and have fixed Qt, and these vars
are unset it Guix, we can remove the unsetting from our compiler command
here.

Couldn't manage to make a darwin-clang-cross only exclusion of `-lm` work properly,
so opted for just removing the explicit link entirely. I do not think this should have any
other unwanted side-effects.

Fixes #21552.

💬 maflcko commented on pull request "kernel: De-globalize static validation variables":
(https://github.com/bitcoin/bitcoin/pull/30425#discussion_r1677633601)
39f9b80fba85d9818222c4d76e99ea1a804f5dda: Not sure about making this public mutable. Would it not be better to make this a private field (along with making `NotifyHeaderTip` a private method)?

🤔 maflcko reviewed a pull request: "kernel: De-globalize static validation variables"
(https://github.com/bitcoin/bitcoin/pull/30425#pullrequestreview-2177390369)
ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d 🍚

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 51fa26239af9bbfd44029aaf59
...

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1677594301)
nit: I think this is obvious
```suggestion
```

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1677650738)
maybe just return early in that case and avoid the looping?

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1677652342)
Should we return early if dependency already exist ?

💬 ismaelsadeeq commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1677606491)
nit: be explicit we are adding direct parents
```suggestion
for (ClusterIndex i = 0; i < cluster.size(); ++i) {
// Fill in fee, size
entries[i].feerate = cluster[i].first;
// Fill in direct parents as ancestors
entries[i].ancestors = cluster[i].second;
// Make sure transactions are ancestors of themselves.
entries[i].ancestors.Set(i);
}
```

🤔 fjahr reviewed a pull request: "refactor: Improve assumeutxo state representation"
(https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-2177291599)
Approach ACK

I think pretty much everything here is a worth while improvement and should set up the following improvements well. I am not so deep into the current state of the kernel work though, so I can not comment on that part, for example the newly introduced typing file. I guess @TheCharlatan could give his nod to that here.

> It removes uses of the terms "active chainstate," "usable chainstate," "disabled chainstate," "ibd chainstate," and "snapshot chainstate" which are confusing fo
...

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1677640152)
Not sure why we need this when in another commit we remove `GetAll()` and instead use `m_chainstates`. Seems kind of inconsistent but maybe I am missing something.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1677599944)
Why do you add this function around here instead of using `m_chainstates` as well?

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1677573399)
nit: In other places similar checks are inlined without a helper method. That might be actually a bit more clear as well.