Bitcoin Core Github
44 subscribers
120K links
Download Telegram
βœ… SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
πŸ“ SecondPort reopened a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
Fixes #31573
βœ… SecondPort closed a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
πŸ“ SecondPort opened a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31575)
Fixes #31573

Reduce memory usage during chainstate initialization on low-memory systems.

* **src/txdb.h**
- Reduce the default `dbcache` value to 100 MiB.
* **src/validation.h**
- Add a function to adjust `dbcache` to 100 MiB if the system has less than 4GB of RAM.
* **doc/reduce-memory.md**
- Update the documentation to reflect the new default `dbcache` value.
- Add information about the memory check for systems with less than 4GB of RAM.
πŸ’¬ l0rinc commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31574#discussion_r1898673122)
NACK
πŸ’¬ l0rinc commented on pull request "Fix memory issue during chainstate initialization on 2GB box":
(https://github.com/bitcoin/bitcoin/pull/31575#issuecomment-2563957123)
> GetTotalSystemMemory’ was not declared in this scope

Please compile it locally before pushing.
πŸ“ hebasto opened a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:

- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
πŸ“ hebasto converted_to_draft a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
This PR ensures that the `script_assets_tests` test case is explicitly reported as "Skipped" when it is not run, making it clearer when running the test suite with `ctest`:

- on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:
```
$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
Test project /home/hebasto/git/bitcoin/build
Start 87: script_tests
Start 83: script_p2sh_tests
...
πŸ’¬ mzumsande commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1898689788)
This comment could be adjusted - `fPeriodicWrite` is no longer just about the block index.
πŸ‘‹ hebasto's pull request is ready for review: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576)
πŸ’¬ deepgrewal commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2563996883)
I do not agree with this change and it should not be merged.
πŸ’¬ davidgumberg commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2564008796)
> The node in question finished sync after I ran it inside heaptrack and is humming along just fine now. I printed out the flamegraph although I have trouble interpreting it:
>
> [ flamegraph.svg]

At first glance it seems pretty unusual that so much time (~40% of samples) is spent in the call stack above `CScriptCheck::operator()` seemingly doing allocations.
πŸ’¬ mzumsande commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2564016601)
> I do not agree with this change and it should not be merged.

"[A NACK needs to include a rationale why the change is not worthwhile. NACKs without accompanying reasoning may be disregarded.](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md)"
πŸ’¬ l0rinc commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2564016916)
@ismaelsadeeq, I ran your benchmarks from https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2446067820, but I only see a 7% speedup in the `TxVerbosity::SHOW_TXID` bench - while the rest were exactly the same as before.
But if it's *that* important here, we could easily get a 30% speedup as well there by deduplicating the `GetSerializeSize` calls:
```C++
static int64_t GetBlockWeight(const size_t size_no_witness, const size_t size_with_witness)
{
return size_no_witness * (WITNE
...
πŸ€” furszy reviewed a pull request: "validation: Send correct notification during snapshot completion"
(https://github.com/bitcoin/bitcoin/pull/31556#pullrequestreview-2524378694)
Code review ACK bc43ecaf6dc
πŸ’¬ brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898710992)
Done.
πŸ’¬ brunoerg commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#issuecomment-2564026547)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898539802
πŸ“ brunoerg opened a pull request: "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`"
(https://github.com/bitcoin/bitcoin/pull/31577)
`DecodeBase58` and `DecodeBase58Check` are fuzzed with a fixed value for `max_ret_len` (100). However, different values are used for this parameter depending on the usage (see key_io and unit tests).
πŸ’¬ l0rinc commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564034989)
Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746/files instead?
πŸ’¬ brunoerg commented on pull request "fuzz: fuzz `max_ret_len` for `DecodeBase58`/`DecodeBase58Check`":
(https://github.com/bitcoin/bitcoin/pull/31577#issuecomment-2564037774)
> Would it make sense to extend https://github.com/bitcoin/bitcoin/pull/30746 instead?

Feel free to cherry-pick this change there, then I close this one.