💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422946155)
ha, that's awesome. The queue's `std::packaged_task<void()>` change broke my mind because the task has obviously a different return value..
I see the trick now, it seems the `packaged_task<R()>` is wrapped into another `packaged_task<void()>` which forwards the call internally. That's elegant.
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422946155)
ha, that's awesome. The queue's `std::packaged_task<void()>` change broke my mind because the task has obviously a different return value..
I see the trick now, it seems the `packaged_task<R()>` is wrapped into another `packaged_task<void()>` which forwards the call internally. That's elegant.
💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3393496902)
So I've made the changes.
**Specifically:**
1. I added the .svg file, like discussed.
2. I set the coins tab to be in the same window and not open a new one.
3. I set the coins tab to be visible only if "coin control features" are enabled in the advanced options.
**The way it works is:**
When coin control features are disabled, then the "Coins" tab will be hidden, like in this picture:
<img width="600" height="400" alt="new_coins_1" src="https://github.com/user-attachments/asse
...
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3393496902)
So I've made the changes.
**Specifically:**
1. I added the .svg file, like discussed.
2. I set the coins tab to be in the same window and not open a new one.
3. I set the coins tab to be visible only if "coin control features" are enabled in the advanced options.
**The way it works is:**
When coin control features are disabled, then the "Coins" tab will be hidden, like in this picture:
<img width="600" height="400" alt="new_coins_1" src="https://github.com/user-attachments/asse
...
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3393553555)
Thank you for considering the suggestions, I find the new layout a lot easier to follow.
I have left a few more nits, but I'm fine doing them in a follow-up.
I like these memory optimizations, my only concern is that we're likely doing a lot heavier copying because of the regular compactions (especially since most move constructors seem unused based on previous fuzzing reports). As mentioned, in a follow-up we can experiment with leaving a tiny buffer.
I have also added a few redundant as
...
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3393553555)
Thank you for considering the suggestions, I find the new layout a lot easier to follow.
I have left a few more nits, but I'm fine doing them in a follow-up.
I like these memory optimizations, my only concern is that we're likely doing a lot heavier copying because of the regular compactions (especially since most move constructors seem unused based on previous fuzzing reports). As mentioned, in a follow-up we can experiment with leaving a tiny buffer.
I have also added a few redundant as
...
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422338701)
This method is called from two places, one of which is dropping the pre-cacululated cluster level that we could access through `FindClusterAndLevel` - the other call doesn't seem to, I guess that's why you decided to always recalculate.
It seems to me it wouldn't complicate much to provide it when it's available and recalculate when it's not, something like:
```patch
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
--- a/src/txgraph.cpp (revision 262762bbb6f7157ba8c54972909c9214448c304b)
+++ b/src
...
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422338701)
This method is called from two places, one of which is dropping the pre-cacululated cluster level that we could access through `FindClusterAndLevel` - the other call doesn't seem to, I guess that's why you decided to always recalculate.
It seems to me it wouldn't complicate much to provide it when it's available and recalculate when it's not, something like:
```patch
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
--- a/src/txgraph.cpp (revision 262762bbb6f7157ba8c54972909c9214448c304b)
+++ b/src
...
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371348)
nit: for consistency:
```suggestion
Assume(!GetTxCount());
```
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371348)
nit: for consistency:
```suggestion
Assume(!GetTxCount());
```
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371564)
nit: likely missed because of the extra space
```suggestion
if (!GetTxCount()) return 0;
```
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422371564)
nit: likely missed because of the extra space
```suggestion
if (!GetTxCount()) return 0;
```
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422454996)
nit:
```suggestion
void GenericClusterImpl::AddDependencies(SetType parents, DepGraphIndex child) noexcept
```
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422454996)
nit:
```suggestion
void GenericClusterImpl::AddDependencies(SetType parents, DepGraphIndex child) noexcept
```
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422374209)
nit: I'd put the assume before the first usage of the validated value
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422374209)
nit: I'd put the assume before the first usage of the validated value
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422890998)
nit: it seems to me this can now be private
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422890998)
nit: it seems to me this can now be private
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422453665)
nit: for consistency we could add the name hints here as well
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422453665)
nit: for consistency we could add the name hints here as well
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422457512)
nit: kinda' unrelated but my linter is complaining here that `Member 'm_known_end_of_cluster' is not initialized in this constructor`.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422457512)
nit: kinda' unrelated but my linter is complaining here that `Member 'm_known_end_of_cluster' is not initialized in this constructor`.
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422459512)
```suggestion
/** Like FindCluster, but also return what level the match was found in (-1 if not found). */
```
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422459512)
```suggestion
/** Like FindCluster, but also return what level the match was found in (-1 if not found). */
```
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422889784)
nit: in a follow-up we could make this: `assert(!sequences.contains(cluster.m_sequence));`
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2422889784)
nit: in a follow-up we could make this: `assert(!sequences.contains(cluster.m_sequence));`
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072675)
Added a log.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072675)
Added a log.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072759)
Changed to `FIXED_SEED`.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072759)
Changed to `FIXED_SEED`.
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072787)
Done!
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2423072787)
Done!
📝 fanquake opened a pull request: "doc: archive release notes for v30.0"
(https://github.com/bitcoin/bitcoin/pull/33601)
Archive v30.0 release notes.
(https://github.com/bitcoin/bitcoin/pull/33601)
Archive v30.0 release notes.
💬 TheCharlatan commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393623978)
> I would try this approach in a separate bindex branch, to evaluate its performance.
What came of this @romanz?
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393623978)
> I would try this approach in a separate bindex branch, to evaluate its performance.
What came of this @romanz?
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393628474)
Sorry, unfortunately I didn't have the time to implement and benchmark the new approach in the last month - but I expect it to have similar lookup performance, and since it wouldn'd require an additional Bitcoin Core index, I will prefer it over my original approach. WDYT?
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3393628474)
Sorry, unfortunately I didn't have the time to implement and benchmark the new approach in the last month - but I expect it to have similar lookup performance, and since it wouldn'd require an additional Bitcoin Core index, I will prefer it over my original approach. WDYT?
💬 epcgrs commented on pull request "contrib: Add bash completion for new bitcoin command":
(https://github.com/bitcoin/bitcoin/pull/33385#issuecomment-3393630750)
ACK
[b5d45f0](https://github.com/bitcoin/bitcoin/pull/33385/commits/b5d45f060161a19d6e9286a434713d36c97b3701)
<img width="574" height="260" alt="image" src="https://github.com/user-attachments/assets/a67e3216-babe-4282-8f36-ed3fef4197c1" />
(https://github.com/bitcoin/bitcoin/pull/33385#issuecomment-3393630750)
ACK
[b5d45f0](https://github.com/bitcoin/bitcoin/pull/33385/commits/b5d45f060161a19d6e9286a434713d36c97b3701)
<img width="574" height="260" alt="image" src="https://github.com/user-attachments/assets/a67e3216-babe-4282-8f36-ed3fef4197c1" />