Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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
🤔 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.
💬 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(...))`.
💬 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 :)
🤔 l0rinc reviewed a pull request: "Release: 30.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/33275#pullrequestreview-3177636174)
Left some notes for the Hungarian translations, they're mostly ok (though they all sound very mechanical), there's one definite mistake and a few ones where the previous one was better
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2316822016)
the previous one was correct:
```suggestion
<translation type="unfinished">Tárca megnyitása</translation>
```
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2316825223)
previous one was better
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2316828200)
this is incorrect, `cím` is `address`:
```suggestion
<translation type="unfinished">A jelenleg kiválasztott aláírás másolása a vágólapra</translation>
```
⚠️ instagibbs opened an issue: "GUI (?): Copying output from console causes large mem usage/OOM"
(https://github.com/bitcoin/bitcoin/issues/33285)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

Running the GUI on mainnet, I open the console, run:

`getblocktemplate '{"rules": ["segwit"]}'`

everything acts normally until then. If I then try to copy the test, memory usage blows out:

on release build, jumps from <2GB to >7GB memory

on debug build, it jumps to dozens of GB, often causing OOM and kills the process.



### Expected behaviour

I expect the memory usage to not blow up
...
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2316855727)
I believe the ancestor and descendant count limits are still used by the wallet when using `-walletrejectlongchains`, which makes sense for at least a couple of releases after cluster mempool is released while there may still be a lot of nodes running with ancestor/descendant limits in place. So I think these variables should stay for now, perhaps I can add a comment mentioning all this so someone remembers to get rid of these later.
💬 w0xlt commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2316892720)
nit: perhaps it makes review easier.

```suggestion
fromme_wallet = self.nodes[0].get_wallet_rpc("fromme")
```
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246455241)
> Left some notes for the Hungarian translations, they're mostly ok (though they all sound very mechanical), there's one definite mistake and a few ones where the previous one was better

Mind adjusting the mentioned translations on Transifex directly?
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#discussion_r2316999726)
Fixed in Transifex, please close