💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087167)
Done.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087167)
Done.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087197)
Done.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087197)
Done.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087251)
Removed it.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087251)
Removed it.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087341)
Done.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087341)
Done.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087547)
Done.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087547)
Done.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087694)
Fixed. But, it wouldn't affect the correctness of the test.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087694)
Fixed. But, it wouldn't affect the correctness of the test.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087978)
Rewrote this part to make it more clear. I was trying to be clever by avoiding some extra checks, but they are probably meaningless and clarity is better here.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485087978)
Rewrote this part to make it more clear. I was trying to be clever by avoiding some extra checks, but they are probably meaningless and clarity is better here.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485157101)
I wrote benchmarks to check how fast it was to construct, since this is the work that is done not in parallel:
```C++
static void SortedVectorBenchmark(benchmark::Bench& bench)
{
CBlock block;
DataStream{benchmark::data::block413567} >> TX_WITH_WITNESS(block);
std::vector<Txid> v{};
v.reserve(block.vtx.size());
bench.run([&] {
for (const auto& tx : block.vtx) {
v.emplace_back(tx->GetHash());
}
std::sort(v.begin(), v.end());
...
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485157101)
I wrote benchmarks to check how fast it was to construct, since this is the work that is done not in parallel:
```C++
static void SortedVectorBenchmark(benchmark::Bench& bench)
{
CBlock block;
DataStream{benchmark::data::block413567} >> TX_WITH_WITNESS(block);
std::vector<Txid> v{};
v.reserve(block.vtx.size());
bench.run([&] {
for (const auto& tx : block.vtx) {
v.emplace_back(tx->GetHash());
}
std::sort(v.begin(), v.end());
...
🤔 danielabrozzoni reviewed a pull request: "doc: document fingerprinting risk when operating node on multiple networks"
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3409743891)
ACK e346ecae830e10310979e5f64de63e043a383ff1
I really like how it's written, it's really fair and balanced, without sounding scary. Thanks for working on this! :)
Just a small nit, you included the suggestions from https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2481496048 and https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482405261 in the second commit (e346ecae830e10310979e5f64de63e043a383ff1). This makes the first commit modify only tor.md, while the second one cha
...
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3409743891)
ACK e346ecae830e10310979e5f64de63e043a383ff1
I really like how it's written, it's really fair and balanced, without sounding scary. Thanks for working on this! :)
Just a small nit, you included the suggestions from https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2481496048 and https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482405261 in the second commit (e346ecae830e10310979e5f64de63e043a383ff1). This makes the first commit modify only tor.md, while the second one cha
...
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485569647)
I don't think we should abort, it's not the fetcher's job to validate. If we didn't, we could even use short ids for the intra-block spends (64 bit likely won't even result in a single duplicate, and when they do xollide we would just do a db check)
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485569647)
I don't think we should abort, it's not the fetcher's job to validate. If we didn't, we could even use short ids for the intra-block spends (64 bit likely won't even result in a single duplicate, and when they do xollide we would just do a db check)
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485572792)
atomics work via CAS, they need to retry in case of high contention. the previous solution didn't have contention, the threads all knew beforehand what to work on.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485572792)
atomics work via CAS, they need to retry in case of high contention. the previous solution didn't have contention, the threads all knew beforehand what to work on.
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485575807)
that's my point, I don't think we should validate at all, it would simplify the code if we didn't
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485575807)
that's my point, I don't think we should validate at all, it would simplify the code if we didn't
✅ maflcko closed an issue: "Decouple datacarrier size and count limits (Draft PR)"
(https://github.com/bitcoin/bitcoin/issues/33595)
(https://github.com/bitcoin/bitcoin/issues/33595)
💬 maflcko commented on issue "Decouple datacarrier size and count limits (Draft PR)":
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3479289768)
The feature request didn't seem to attract much positive attention in the past. Thus, closing due to lack of interest, progress and direction. It is possible to re-open this issue or create a new issue referencing it, if there is fresh interest.
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3479289768)
The feature request didn't seem to attract much positive attention in the past. Thus, closing due to lack of interest, progress and direction. It is possible to re-open this issue or create a new issue referencing it, if there is fresh interest.
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485579371)
how so? Can we assert the behavior of the main cache as well so that the previous version doesn't pass?
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485579371)
how so? Can we assert the behavior of the main cache as well so that the previous version doesn't pass?
💬 alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3479398323)
re-ACK https://github.com/bitcoin/bitcoin/commit/e95efc00842d5d0df96ee9294cdf818741be539e
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3479398323)
re-ACK https://github.com/bitcoin/bitcoin/commit/e95efc00842d5d0df96ee9294cdf818741be539e
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485697378)
There are other ones that I didn't mention, reformatted the change, please take the ones that make sense:
<details>
<summary>Details</summary>
```patch
diff --git a/src/coins.cpp b/src/coins.cpp
index 2ef2e36ccc..baac1a32b5 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -173,7 +173,8 @@ bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const {
return (it != cacheCoins.end() && !it->second.coin.IsSpent());
}
-std::optional<Coin> CCoinsViewCache::GetPossib
...
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2485697378)
There are other ones that I didn't mention, reformatted the change, please take the ones that make sense:
<details>
<summary>Details</summary>
```patch
diff --git a/src/coins.cpp b/src/coins.cpp
index 2ef2e36ccc..baac1a32b5 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -173,7 +173,8 @@ bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const {
return (it != cacheCoins.end() && !it->second.coin.IsSpent());
}
-std::optional<Coin> CCoinsViewCache::GetPossib
...
👍 hodlinator approved a pull request: "refactor: Header sync optimisations & simplifications"
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3409949311)
re-ACK 8a4ab45c13dd7ca4474bc4621110da82e0a634f8
Changes since previous review (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3128785887):
* More helpful commit messages.
* Rebased, especially to handle #32579.
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3409949311)
re-ACK 8a4ab45c13dd7ca4474bc4621110da82e0a634f8
Changes since previous review (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3128785887):
* More helpful commit messages.
* Rebased, especially to handle #32579.
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3479517366)
@hodlinator thanks! Was about to comment with the update haha :)
In my last push:
- Rebase after #32579
- Address https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289739420, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289729638, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289725133
- Specify in 8a4ab45c13dd7ca4474bc4621110da82e0a634f8 commit message that I'm also slightly reformatting the code
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3479517366)
@hodlinator thanks! Was about to comment with the update haha :)
In my last push:
- Rebase after #32579
- Address https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289739420, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289729638, https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289725133
- Specify in 8a4ab45c13dd7ca4474bc4621110da82e0a634f8 commit message that I'm also slightly reformatting the code
💬 Sjors commented on pull request "miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC":
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3479528530)
I switched to the approach of adding an `include_dummy_extranonce`, having the tests set it to `true` while IPC calls use the default `false`. Expanded `test/functional/interface_ipc.py` to demonstrate it.
(https://github.com/bitcoin/bitcoin/pull/32420#issuecomment-3479528530)
I switched to the approach of adding an `include_dummy_extranonce`, having the tests set it to `true` while IPC calls use the default `false`. Expanded `test/functional/interface_ipc.py` to demonstrate it.