💬 maflcko commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246003431)
> > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > "Cannot add WalletDescriptor to a non-descriptor wallet"
>
> An internal name.

There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?

💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246010706)
> > > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > > "Cannot add WalletDescriptor to a non-descriptor wallet"
> >
> >
> > An internal name.
>
> There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?

Yes, the origi
...

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316615719)
In e043a279da6ea901b5f6d6690f96ff0d419faf13 _ci: enable IPC tests in CI_: `pip3 install` did the trick for me, see https://github.com/Sjors/bitcoin/actions/runs/17409223017/job/49421870136?pr=103#step:8:3514

🤔 w0xlt reviewed a pull request: "kernel: chainparams & headersync updates for 30.0"
(https://github.com/bitcoin/bitcoin/pull/33274#pullrequestreview-3177418705)
ACK https://github.com/bitcoin/bitcoin/pull/33274/commits/755152ac819a23acf2f9e70316134d74a04d589b

💬 l0rinc commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3246155677)
My intention wasn't to hurt, thanks for clarifying the title.

💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246189935)
> Is there some additional context that translators have?

We could probably get more accurate translations by creating a bot that goes through each translation (<2000 values I think, we could batch them by files) fetches the source code context for the given translations and gives 3-4 examples for translations that we think are close to how we want it to translate and ask for the missing languages from the AI (based on the English + source code usage + 3-4 other correct translations)

⚠️ fanquake opened an issue: "build: secp256k1 warnings not turned into errors in MSAN job"
(https://github.com/bitcoin/bitcoin/issues/33284)
master (7e58c94112d061af38648808aae87c369098f44c) https://cirrus-ci.com/task/6098524451897344:
```bash
[06:05:23.776] Treat compiler warnings as errors ..... ON
<snip>
[06:05:24.677] [ 10%] Building CXX object src/crypto/CMakeFiles/bitcoin_crypto.dir/sha256_avx2.cpp.o
[06:05:24.678] /ci_container_base/src/secp256k1/src/tests.c:6049:34: warning: variable 'pubkey' is uninitialized when passed as a const pointer argument here [-Wuninitialized-const-pointer]
[06:05:24.678] 6049 | SECP256K1_CHEC
...

💬 achow101 commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3246220959)
ACK 755152ac819a23acf2f9e70316134d74a04d589b

💬 achow101 commented on pull request "contrib: update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/33283#issuecomment-3246233173)
Huh, seems to have lost a bunch of i2p and cjdns nodes. My crawler might be having some connection issues.

💬 l0rinc commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3246245404)
It wouldn't be difficult to make this into a manual script which generates an id mapping, if we're worried about python, something like:
```
sed -E -i \
-e 's|id="_msg1160"|id="_msg1239"|g' \
-e 's|id="_msg1159"|id="_msg1160"|g' \
-e 's|id="_msg120\[0\]"|id="_msg1240\[0\]"|g' \
-e 's|id="_msg120\[1\]"|id="_msg1240\[1\]"|g' \
'src/qt/locale/bitcoin_en.xlf'
```

Would that be more useful in your opinion? What are the worries exactly, can you please point me to the discussion th
...

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316777908)
7eb71775dce6f3f2fd0e4b62f439aab86d1b65a0: tidy doesn't run functional tests currently, because I guess it doesn't harm either.

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3246278325)
ACK 235016f5b78ba9f472b56df0825690307fffc7e6

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316788847)
Done.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316789881)
Fixed.

🤔 glozow reviewed a pull request: "Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3177119367)
Would it be a good idea to split this PR into a main PR with Cluster Implementation + Cleanups + Docs and Tests, then a followups PR with Optimizations and more cleanups? Goal is to get everything in v31, but this might help with reducing the size of the main PR and make it easier to punt cleanup-related comments to later.

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2316480489)
nit: the comment above the `class CTxMemPool` definition now needs an update. It still mentions the index by descendant feerate, the `m_parents` field in `CTxMemPoolEntry`, etc.

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2316505098)
I think we can do something similar to #32941 and just emit a warning (not error) for the first release, then remove it afterwards. We can't deprecate / continue supporting the options, but this is more user-friendly than dropping immediately.

```
InitWarning(_("Option '-limitdescendantcount' is set but no longer has any effect (see release notes about cluster limits). Please remove it from your configuration."));
```

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2316466962)
I think `{ancestor,descendant}_{count,size_vbytes}` can be cleaned up after 74f51262688e1f6a4af1664c13f6c895b1499a39 as well? Perhaps `Flatten()` in txmempool.cpp can do the sanity check against default cluster size instead. This can be done in a followup.

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2316792057)
Is this todo already covered by `test_too_many_replacements()`?

💬 sr-gi commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246305675)
I've reviewed the Catalan version (and updatred) the Catalan version issues, plus a few more pending strings