Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ furszy commented on pull request "test: descriptor: fix test for `MaxSatisfactionWeight`":
(https://github.com/bitcoin/bitcoin/pull/31570#discussion_r1898539802)
Haven't verified all the scenarios but maybe you could verify that `max_sat_nonmaxsig < max_sat_maxsig` (or <= if that causes any complication).
πŸ’¬ Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#discussion_r1898543849)
Just that I didn't want to touch `CheckProofOfWorkImpl`.
πŸ’¬ Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2563756657)
> The newly introduced `CheckNewBlock()` looks very similar to the existing `TestBlockValidity()`

It looks like I ended up with a similar design after a long detour. I'll look into combining these.

> Instead of a multiplier I could also allow a custom target

@instagibbs is there any reason you went for a multiplier?
πŸš€ ryanofsky merged a pull request: "coins: warn on shutdown for big UTXO set flushes"
(https://github.com/bitcoin/bitcoin/pull/31534)
πŸ’¬ alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2563810059)
> @alfonsoromanz While #30909 is stalling a bit it seems like it is the way we'll move forward when there is some more review since there was no push back on concept/approach. So I would suggest you proceed here assuming that #30909 is acceptable. Ideally you would change this PR to have the second test standalone, if that's not possible you can base it on #30909.

Thanks for the feedback @fjahr! I will be working on this
⚠️ otech47 opened an issue: "Running out of memory on a 2GB box - Initializing chainstate Chainstate [ibd] @ height -1 (null)"
(https://github.com/bitcoin/bitcoin/issues/31573)
I have a server running a pruned bitcoind node as a Docker container with 2G RAM. There are some other services using maybe 500MB of the RAM but I got into a state where bitcoind tries to initialize the chainstate but then starts sucking up all the available RAM so it gets killed with OOM error. Docker then restarts it and it loops like this forever

I tried to configure bitcoind to be constrained to low memory but it doesn't seem to have any effect

```
[main]
printtoconsole=1
rpcallowip
...
πŸ€” furszy reviewed a pull request: "wallet: refactor: various master key encryption cleanups"
(https://github.com/bitcoin/bitcoin/pull/31398#pullrequestreview-2524287853)
Concept ACK, nice code dedup.
πŸ’¬ danielabrozzoni commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r1898656359)
I don't think it's needed to clear the script witness, as it's not taken into account when calculating the txid
πŸ“ SecondPort opened a pull request: "Fix memory issue during chainstate initialization on 2GB box"
(https://github.com/bitcoin/bitcoin/pull/31574)
Fixes #31573

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/bitcoin/bitcoin/pull/31574?shareId=adfd9f84-4320-4707-98a2-35d355535cbf).
βœ… 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.