💬 achow101 commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2501899041)
ACK 3305972f7bfd78181566e4297891c2dd7cae0f2b
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2501899041)
ACK 3305972f7bfd78181566e4297891c2dd7cae0f2b
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1859205128)
Done.
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1859205128)
Done.
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1859205308)
Done.
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1859205308)
Done.
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2501906569)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828422802
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2501906569)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1828422802
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2501907189)
> Would be good to ensure that https://github.com/bitcoin/bitcoin/pull/31374 can be replicated here.
Since this target is only for spkm migration, I don't think it can be replicated here.
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2501907189)
> Would be good to ensure that https://github.com/bitcoin/bitcoin/pull/31374 can be replicated here.
Since this target is only for spkm migration, I don't think it can be replicated here.
🤔 ismaelsadeeq reviewed a pull request: "validation: fetch block inputs on parallel threads ~17% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-2462618817)
Concept ACK
This is nice. Although I have not yet benchmarked this branch, I also like @furszy's idea of having a general-purpose thread pool.
I just have one test improvement comment, question and a nit after first pass of the PR
(https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-2462618817)
Concept ACK
This is nice. Although I have not yet benchmarked this branch, I also like @furszy's idea of having a general-purpose thread pool.
I just have one test improvement comment, question and a nit after first pass of the PR
💬 ismaelsadeeq commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1859189838)
In 3c201bcffc1d7e382e8afa9a88750a4c261c1cf8 "tests: add inputfetcher tests"
You can set this up in `InputFetcherTest` so that you don't have to repeat it in the rest of the tests.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1859189838)
In 3c201bcffc1d7e382e8afa9a88750a4c261c1cf8 "tests: add inputfetcher tests"
You can set this up in `InputFetcherTest` so that you don't have to repeat it in the rest of the tests.
💬 ismaelsadeeq commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1859192217)
In 2349ac7d6071746a80223358bce0d5e556b277d7 "bench: add inputfetcher bench"
How did you select this batch size?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1859192217)
In 2349ac7d6071746a80223358bce0d5e556b277d7 "bench: add inputfetcher bench"
How did you select this batch size?
💬 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.
(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
(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...
(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
(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)
(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?
(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
(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
(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
(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)
(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
(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)
(https://github.com/bitcoin/bitcoin/pull/31172)