🤔 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
(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.
(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)
(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
...
(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
(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.
(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
...
(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.
(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
(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.
(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.
(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.
(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.
(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."));
```
(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.
(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()`?
(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
(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
🤔 janb84 reviewed a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3177614507)
re ACK 235016f5b78ba9f472b56df0825690307fffc7e6
changes since last ACK:
- Rebase
- Fixes for macOS native job (pip -> pip3)
- Added IPC interface tests to more hosts.
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3177614507)
re ACK 235016f5b78ba9f472b56df0825690307fffc7e6
changes since last ACK:
- Rebase
- Fixes for macOS native job (pip -> pip3)
- Added IPC interface tests to more hosts.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316809613)
I found it logistically simpler to give each their own, as otherwise all the test need to run within a single `asyncio.run(capnp.run(...))`.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316809613)
I found it logistically simpler to give each their own, as otherwise all the test need to run within a single `asyncio.run(capnp.run(...))`.
💬 AO-LocLab commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246344948)
The LLM check is an interesting tool that we should definitely use. However, language coordinators and translators should not just apply the LLM’s suggestions as they can be misleading. It can miss the nuances and claim the translation is wrong. It also sometimes misses the context. Like anything AI powered, a great tool to use when properly checked by a human.
I have addressed the errors that need to be in the Fr translation :)
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246344948)
The LLM check is an interesting tool that we should definitely use. However, language coordinators and translators should not just apply the LLM’s suggestions as they can be misleading. It can miss the nuances and claim the translation is wrong. It also sometimes misses the context. Like anything AI powered, a great tool to use when properly checked by a human.
I have addressed the errors that need to be in the Fr translation :)