Bitcoin Core Github
42 subscribers
124K links
Download Telegram
πŸ’¬ ismaelsadeeq commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1859204841)
In 2349ac7d6071746a80223358bce0d5e556b277d7 "bench: add inputfetcher bench"
nit: will be nice if we have `block413567` input's data that we can read so that we dont have to simulate this.
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859217858)
I think this is slightly harder to follow, but I like that the collections are only sorted up to some point, so I'll take it. Instead of using the `std::min`, I've added two `Assume` when defining `{outbounds, inbounds}_target_size` to ensure that the targets are never bigger than the sets the pull from, since this should never be the case
πŸ’¬ evgeniytar commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2501957591)
It looks like `Discover()` skips loopback interfaces intentionally, it's done by this line of code

https://github.com/bitcoin/bitcoin/blob/master/src/common/netif.cpp#L290

Once it commented the test passes. Maybe this test needs another routable address setup...
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859251110)
I don't think it makes a huge difference, but I'll take it
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859251614)
Covered in [4b4d99f](https://github.com/bitcoin/bitcoin/pull/30116/commits/4b4d99fb2df2c60a2214487cec627bc560f50f53)
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859252466)
Is this still an issue?
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859253278)
Covered in the last push
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859253662)
Covered in the last push
πŸ’¬ sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1859254738)
This is just temporary. It should not be merged like this. I'll leave it open once we decide on a clear way to trim
πŸš€ achow101 merged a pull request: "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors"
(https://github.com/bitcoin/bitcoin/pull/31364)
πŸ’¬ achow101 commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2502053548)
ACK ee1128ead846698db5e5633f193883837f2fbc64
πŸš€ achow101 merged a pull request: "build: increase minimum supported Windows to 10.0"
(https://github.com/bitcoin/bitcoin/pull/31172)
πŸ“ hebasto reopened a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/31306)
This PR switches to the latest [IWYU 0.23](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.23), which is compatible with Clang 19.

New "bugprone-use-after-move" and "modernize-use-starts-ends-with" warnings that emerged have been addressed.
πŸ’¬ hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#issuecomment-2502140063)
> No need to close this if we can change the approach.

Rebased on top of the merged #31305 and #31364.

> I think we can agree on an approach where we (in this order):
>
> * Fix the actual bugs
>
> * Notate the false-positives
>
> * Enable the checks.

Done.

> It sounds like it also is worth adding a line in `developer-notes.md` that describes when a `NOLINT` is/is not appropriate.

Left this change for a follow-up.
πŸ’¬ laanwj commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2502145119)
> Just wanted to clarify that I'd love to see this get in sooner rather than later, as it can provide a valuable speedup and I'd be ready to ACK with a reduced (default) size selected, for the reasons I outlined above.

Yes, i think there's been enough analysis here to confirm that changing the default to 16MB or 32MB would give almost all of the gain, with the least risk, let's go for that.
πŸ“ furszy opened a pull request: "wallet: fix crash during migration due to invalid multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31378)
Ensure legacy wallet migration skips non-standard or consensus-invalid multisig
scripts. Treating them as valid causes migration to crash because we are enforcing
this rules within the descriptors parsing logic.

Testing Notes:
This can be verified by cherry-picking and running the test commit on master.
It will crash there but pass on this branch.
πŸ€” darosior reviewed a pull request: "wallet: fix crash during migration due to invalid multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31378#pullrequestreview-2462972517)
Ruling out consensus-invalid scripts makes sense, but discarding non-standard scripts is a much bigger leap. Also it would be a wack-a-mole: does the migration crash on any non-standard script? How much of that do you want to cover? Is it not possible to make the migration not crash on valid but not standard scripts instead of changing `IsMine` for those?
πŸ’¬ andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1859491491)
Hmm, okay. I think it's better to remove it from this PR and maybe address the subject in another issue so it doesn’t interfere with the other commits.

So, the PR will include only:
- The --skip-missing-binaries option
- The error prompt to inform the user if no binaries are found in the build directory path.
πŸ’¬ jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2502387159)
Pushed a [small fix](https://github.com/bitcoin/bitcoin/compare/1ca5f7b998c14c44b6afd5e362fc4c007d8eebd3..0667ee2e5bbaba1d2965ab95644bdb27f881ffa1) addressing `blockhash` nullability.
πŸ’¬ jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1859531317)
Fixed.