💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214968788)
> With only a single link, how would you delete arbitrary map entries?
My naive thinking was that maybe we don't actually need random-element deletion. Based on how `ClearFlags` and `erase` are currently called from loops that already have the previous node, we could often call `ClearFlags(*prev)`.
The destructor is trickier. Maybe we can handle this in the `CCoinsViewCache` destructor instead, batch-destructing entries there, which would involve a single extra $\mathcal{O}(n)$ traversal.
I
...
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214968788)
> With only a single link, how would you delete arbitrary map entries?
My naive thinking was that maybe we don't actually need random-element deletion. Based on how `ClearFlags` and `erase` are currently called from loops that already have the previous node, we could often call `ClearFlags(*prev)`.
The destructor is trickier. Maybe we can handle this in the `CCoinsViewCache` destructor instead, batch-destructing entries there, which would involve a single extra $\mathcal{O}(n)$ traversal.
I
...
💬 petertodd commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2214969086)
On July 8, 2024 8:42:50 PM GMT+02:00, Luke Dashjr ***@***.***> wrote:
>>This proposal is merely a way of doing something that is already done - signed anchor outputs - in a more efficient way with less overall bytes consumed.
>
>And at a reduced cost. Unless this proposal increases vsize to make it identical?
The cost reduction is a natural one, in line with the byte reduction.
>> The creating and spending of keyless anchor outputs is completely fixed, with no ability to encode any data. It
...
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2214969086)
On July 8, 2024 8:42:50 PM GMT+02:00, Luke Dashjr ***@***.***> wrote:
>>This proposal is merely a way of doing something that is already done - signed anchor outputs - in a more efficient way with less overall bytes consumed.
>
>And at a reduced cost. Unless this proposal increases vsize to make it identical?
The cost reduction is a natural one, in line with the byte reduction.
>> The creating and spending of keyless anchor outputs is completely fixed, with no ability to encode any data. It
...
💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214976515)
@paplorinc When UTXOs are spent while FRESH, they get deleted from the cache; this is quite possibly the most significant performance optimization Bitcoin Core has during IBD, as it avoids the need for those UTXOs to ever end up on disk. It's done based on `COutPoint` lookup, not during iteration.
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214976515)
@paplorinc When UTXOs are spent while FRESH, they get deleted from the cache; this is quite possibly the most significant performance optimization Bitcoin Core has during IBD, as it avoids the need for those UTXOs to ever end up on disk. It's done based on `COutPoint` lookup, not during iteration.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214993679)
Makes sense, thanks for explaining @sipa!
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214993679)
Makes sense, thanks for explaining @sipa!
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669189198)
This code is gone.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669189198)
This code is gone.
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215089061)
> What is the 0x4e73 for?
To be segwit, it needs to be a witness version + witness program(or p2sh nested segwit script). A witness program is 2 to 40 bytes long push. For this use-case we simply use size two. The specific bytes chosen are simply "cutesy", as they cannot be 0 so something else has to be chosen and have an associated address. As far as I know this is the only proposed size 2 witness program use case so far.
> These outputs bloat the UTXO set, without sufficient value to i
...
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215089061)
> What is the 0x4e73 for?
To be segwit, it needs to be a witness version + witness program(or p2sh nested segwit script). A witness program is 2 to 40 bytes long push. For this use-case we simply use size two. The specific bytes chosen are simply "cutesy", as they cannot be 0 so something else has to be chosen and have an associated address. As far as I know this is the only proposed size 2 witness program use case so far.
> These outputs bloat the UTXO set, without sufficient value to i
...
🤔 mzumsande reviewed a pull request: "Assumeutxo: bugfix on loadtxoutset with a divergent chain + test"
(https://github.com/bitcoin/bitcoin/pull/29996#pullrequestreview-2164290570)
Code Review ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c
(https://github.com/bitcoin/bitcoin/pull/29996#pullrequestreview-2164290570)
Code Review ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c
🤔 fjahr reviewed a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2164298309)
re-ACK ab478c5fa16427b496e8a93e4780770d4c270214
Confirmed only changes since last review were the rebase and addressing comments (per `git range-diff master 29ab88dfe1c43af3620480c99cba844aa414023c ab478c5fa16427b496e8a93e4780770d4c270214`).
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2164298309)
re-ACK ab478c5fa16427b496e8a93e4780770d4c270214
Confirmed only changes since last review were the rebase and addressing comments (per `git range-diff master 29ab88dfe1c43af3620480c99cba844aa414023c ab478c5fa16427b496e8a93e4780770d4c270214`).
💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669238451)
nit: seems the "snapshot" added before is back out, still not a block for me though
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669238451)
nit: seems the "snapshot" added before is back out, still not a block for me though
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2215221618)
rebased on master, and s/anchor/dust/ everywhere
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2215221618)
rebased on master, and s/anchor/dust/ everywhere
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669272313)
ah, sorry, am on a different computer now and forgot to update the branch... Fixed now.
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669272313)
ah, sorry, am on a different computer now and forgot to update the branch... Fixed now.
💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2215232338)
re-ACK b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4
Just addressed my latest re-nit.
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2215232338)
re-ACK b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4
Just addressed my latest re-nit.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669280166)
I've changed it to "Find the best (highest-feerate, smallest among those in case of a tie) ancestor set among the remaining transactions.", because "best" is a bit more specific than highest-feerate.
Regarding the side note: that rule in `BlockAssembler` is an optimization, as it avoids looking at child transactions with higher ancestor feerate, because the ancestors will have been picked earlier anyway. The same optimization could be made in `AncestorCandidateFinder`, but because it's an $\m
...
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669280166)
I've changed it to "Find the best (highest-feerate, smallest among those in case of a tie) ancestor set among the remaining transactions.", because "best" is a bit more specific than highest-feerate.
Regarding the side note: that rule in `BlockAssembler` is an optimization, as it avoids looking at child transactions with higher ancestor feerate, because the ancestors will have been picked earlier anyway. The same optimization could be made in `AncestorCandidateFinder`, but because it's an $\m
...
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281361)
I've added a comment that inc needs to be topological (a term which is defined in the `WorkItem` definition above).
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281361)
I've added a comment that inc needs to be topological (a term which is defined in the `WorkItem` definition above).
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281468)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669281468)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669282068)
I've added a short comment to the docstring of the function instead.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669282068)
I've added a short comment to the docstring of the function instead.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669283836)
Done, good idea. Avoiding a separate copy `m_todo` in `Linearize` forced me to rewrite some of the code, which turned out to simplify the LIMO logic slightly.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669283836)
Done, good idea. Avoiding a separate copy `m_todo` in `Linearize` forced me to rewrite some of the code, which turned out to simplify the LIMO logic slightly.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669288946)
@glozow Indeed. Even if this function was changed to return an explicit `optimal`, it would remain quite possible that it returns the optimal without *knowing* it's optimal (because it's possible that the `best` passed in for example was already optimal, but it requires a ton of iterations to exhaust the search space to prove nothing better exists).
@instagibbs Well I think we may want to cache in the mempool clusters whether or not the linearization for them is known to be optimal too, so th
...
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669288946)
@glozow Indeed. Even if this function was changed to return an explicit `optimal`, it would remain quite possible that it returns the optimal without *knowing* it's optimal (because it's possible that the `best` passed in for example was already optimal, but it requires a ton of iterations to exhaust the search space to prove nothing better exists).
@instagibbs Well I think we may want to cache in the mempool clusters whether or not the linearization for them is known to be optimal too, so th
...
💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215368575)
@achow101 `wallet_backwards_compatibility.py --legacy` failure seems spurious? can you check?
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2215368575)
@achow101 `wallet_backwards_compatibility.py --legacy` failure seems spurious? can you check?
🤔 stickies-v reviewed a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2163854874)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2163854874)
Concept ACK