💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075469037)
```c++
InitWarning(_("Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in future versions."));
```

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075473533)
At the end of the test add:

```python
for i in range(self.num_nodes):
self.stop_node(i, expected_stderr=self.expected_stderr[i])
```

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075481564)
Code suggested here: https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2818267234

👍 hebasto approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2818293785)
re-ACK 22cff32319de64cb98e1c89b9a7ed35611e89e27.

💬 hebasto commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2854604811)
> > Would be good to register this new type into the Qt's meta-object system. Look at the RegisterMetaTypes() function on src/qt/bitcoin.cpp.
>
> Good point. It's used in at least the `TransactionView::bumpedFee` signal, which might be broken now.
>
> It's likely that adding only `Q_DECLARE_METATYPE(Txid)` at the top is enough as this was enough for uint256.

I don't think it is required in Qt 6.

See: https://www.qt.io/blog/whats-new-in-qmetatype-qvariant

✅ maflcko closed an issue: "ARM Windows build and release"
(https://github.com/bitcoin/bitcoin/issues/31388)

💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2854625982)
Closing for now. This can be re-opened when there is a toolchain to achieve this. Even then, the toolchain would need to make it into guix and ideally other package managers as well.

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2075520941)
forgot to remove the comment?

💬 stickies-v commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#issuecomment-2854656467)
Force-pushed to remove the `GetAll() const` method (and commit), replacing its callsites with directly accessing `{m_ibd_chainstate.get(), m_snapshot_chainstate.get()}` as [suggested](https://github.com/bitcoin/bitcoin/pull/32364?notification_referrer_id=NT_kwDOBB0EGbQxNjExMTQxNTE5MDo2OTAxMDQ1Nw#pullrequestreview-2810060421) by @mzumsande.

> Given that #30214 wants to removes `GetAll()` anyway in [64fc660](https://github.com/bitcoin/bitcoin/commit/64fc6603525f515c5303f557b038753904c6541e)


...

💬 stickies-v commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#discussion_r2075528317)
Oh yes, keeping only `self` is a better approach, thanks! Marking as resolved because the commit has been [dropped](https://github.com/bitcoin/bitcoin/pull/32364?notification_referrer_id=NT_kwDOBB0EGbQxNjExMTQxNTE5MDo2OTAxMDQ1Nw#issuecomment-2854656467).

👍 willcl-ark approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2818380798)
ACK 22cff32319de64cb98e1c89b9a7ed35611e89e27

💬 andrewtoth commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2075559595)
Sorry, what is the original cache-usage-update? AFAICT this line was added in this PR, and there is no cache-usage-update here?

💬 laanwj commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075574236)
First thought this change was a bit strange, but "lose all your bitcoin" singular might make more sense even in English?

💬 McBeThC commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854739598)
Concept NACK

💬 laanwj commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075580265)
This update looks correct, but we should really avoid having format codes in the translations, `%n` substitutes combined with HTML make it doubly confusing for translators.

💬 laanwj commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075591796)
Another translation with HTML in it.

💬 hebasto commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075606081)
This comes from:https://github.com/bitcoin/bitcoin/blob/f490f5562d4b20857ef8d042c050763795fd43da/src/qt/walletcontroller.cpp#L94

📝 TheCharlatan opened a pull request: "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store"
(https://github.com/bitcoin/bitcoin/pull/32427)
Opening this PR mostly to get concept/approach feedback.

This is motivated by the kernel library, where the internal usage of leveldb is a limiting factor to its future use cases. Specifically it is not possible to share leveldb databases between two processes. A notable use-case for the kernel library is accessing and analyzing existing block data. Currently this can only be done by first shutting down the node writing this data. Moving away from leveldb opens the door towards doing this in
...

💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2075611741)
I prefer code over comments - and since the usual pattern is basically:
```C++
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
if (cursor.WillErase(*it)) {
itUs->second.coin = std::move(it->second.coin);
} else {
itUs->second.coin = it->second.coin;
}
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
```

it makes sense to explain (via code) why here we don't need the first part:
```C++
assert(entry.coin.DynamicMemoryUsage() == 0); // i.e. we didn't for
...

💬 hebasto commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075613658)
This comes from: https://github.com/bitcoin/bitcoin/blob/f490f5562d4b20857ef8d042c050763795fd43da/src/qt/walletcontroller.cpp#L251-L253