Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 l0rinc commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846350891)
Is script verification completely CPU bound? If so, this limit may still be reasonable for a few more years - but unlike in 2013, it's not an unreasonably big value.
💬 TheCharlatan commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#issuecomment-2482649498)
Updated 09c9ba846ec44ba903de2b32a1cb0f49c7f93cb0 -> 8f85d36d68ab33ba237407a2ed16667eb149d61f ([chainstatemanagerArgs_0](https://github.com/TheCharlatan/bitcoin/tree/chainstatemanagerArgs_0) -> [chainstatemanagerArgs_1](https://github.com/TheCharlatan/bitcoin/tree/chainstatemanagerArgs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstatemanagerArgs_0..chainstatemanagerArgs_1))

* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846233
...
💬 l0rinc commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846353272)
Do we expect [every call-site](https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846266844) to do the clamping themselves?
BrandonOdiwuor closed a pull request: "test: run functional tests from ctest"
(https://github.com/bitcoin/bitcoin/pull/31312)
💬 TheCharlatan commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846366581)
I don't think this really matters, but seems consistent to control the arguments in the same place, just like with the batch size.
🤔 BrandonOdiwuor reviewed a pull request: "Bugfix: Correct first-run free space checks"
(https://github.com/bitcoin/bitcoin/pull/29678#pullrequestreview-2442281522)
Concept ACK using `GB` to represent 10<sup>3</sup> bytes for consistency with GUI
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1846404405)
435934fa020cec584b49c6151aa6b0d858054b37 oops, that should have been `&&`
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482738819)
Pushed a bug fix, but still need to address the main feedback.
💬 maflcko commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482760476)
Also, it would be good to add test coverage?
💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1846425672)
re https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1834551086

Are you still working on this?
maflcko closed a pull request: "nomerge: test malicious bidi char"
(https://github.com/bitcoin/bitcoin/pull/31314)
💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2482785475)
I disabled ARM and macOS-cross as well, added a link to their failures to PR description TODO.

I think the `p2p_i2p_ports.py` failure in the previous releases job is spurious, so leaving that job in.
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482789520)
@maflcko it's implicitly tested by the shutdown test, which is how I found this bug. Not sure how to explicitly test an early return?
💬 maflcko commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482799390)
Are you sure it is covered, because the code is reported as uncovered in https://corecheck.dev/bitcoin/bitcoin/pulls/31297?
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482804679)
@maflcko `feature_shutdown.py` only check that the early return is _not_ used. It makes sense to test the scenario where it is used too.
💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2482805416)
https://cirrus-ci.com/task/5063913392308224?logs=ci#L6167
💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2482830018)
For reference, the fuzz issue can be hit more frequent on machines that do not have nanosecond precision. GCE may not have it, according to the oss-fuzz crashes of the form:

```
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in remove_all: No such file or directory ["/tmp/test_common bitcoin/1731742732339168000/regtest/fuzzed_i2p_private_key"]
```

Ref: https://oss-fuzz.com/testcase-detail/5425475725361152

https:
...
💬 willcl-ark commented on pull request "Add `contrib/justfile` containing useful development workflow commands.":
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2482850113)
I really enjoy making justfiles for projects I work on.

And, whilst I would love to see some kind of community justfile added to this project, I can appreciate the hesitation to do so per the rationale given by @achow101 above.

I've therefore taken a different approach myself, hosting a justfile at [https://github.com/bitcoin-dev-tools/dotfiles](https://github.com/bitcoin-dev-tools/dotfiles/blob/master/justfile) which is designed to sit one directory higher than this repository's code, spe
...
🤔 rkrux reviewed a pull request: "wallet: Remove IsMine from migration code"
(https://github.com/bitcoin/bitcoin/pull/30328#pullrequestreview-2442077210)
Preliminary review fa155991ff30bc001229e1d71bf44e8782f02d17

I feel e6342ffdf0030f6350a07f325ce603fddfff1bd8 commit could be a separate PR, these functional tests are not dependent on the changes in the next two commits and don't need to be tied to these changes.
💬 rkrux commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1846226163)
`Lambda`