🚀 fanquake merged a pull request: "build: Bump clang minimum supported version to 16"
(https://github.com/bitcoin/bitcoin/pull/30263)
(https://github.com/bitcoin/bitcoin/pull/30263)
💬 ryanofsky commented on pull request "refactor: Make uint256S(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2214432177)
Concept ACK, and would be very nice for this to cover ParseHex. If it did, it seems like it would fix the unexpected consensus library dependency on the util library that hebasto reported in https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2209258843:
https://github.com/bitcoin/bitcoin/blob/a83f050dbe1392fc6b1b6c2a140c7346653b40d3/src/pubkey.cpp#L193
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2214432177)
Concept ACK, and would be very nice for this to cover ParseHex. If it did, it seems like it would fix the unexpected consensus library dependency on the util library that hebasto reported in https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2209258843:
https://github.com/bitcoin/bitcoin/blob/a83f050dbe1392fc6b1b6c2a140c7346653b40d3/src/pubkey.cpp#L193
👋 maflcko's pull request is ready for review: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071)
(https://github.com/bitcoin/bitcoin/pull/29071)
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668526162)
4b1e978b2bafd9da564aa52d2ce64a723cf64036 nit:
```suggestion
// Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and if
```
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668526162)
4b1e978b2bafd9da564aa52d2ce64a723cf64036 nit:
```suggestion
// Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and if
```
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1644451932)
maybe mention that caller must ensure inc includes `Ancestors(inc)`
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1644451932)
maybe mention that caller must ensure inc includes `Ancestors(inc)`
🤔 glozow reviewed a pull request: "cluster mempool: cluster linearization algorithm"
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2118340933)
Looked at the first 4 commits so far, just doing code review and trying the algos out in `MiniMiner`.
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2118340933)
Looked at the first 4 commits so far, just doing code review and trying the algos out in `MiniMiner`.
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1639799588)
In the docs, maybe instead of "best remaining ancestor set", say "highest feerate ancestor set" ?
Side note - I was thinking that this was a replica of the ancestor set algo in `BlockAssembler` but this isn't the minimum of individual and ancestor feerate.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1639799588)
In the docs, maybe instead of "best remaining ancestor set", say "highest feerate ancestor set" ?
Side note - I was thinking that this was a replica of the ancestor set algo in `BlockAssembler` but this isn't the minimum of individual and ancestor feerate.
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668545670)
304d3cb23ba9f084b98f9f29b47e3dfbb61ca334
may be helpful context
```suggestion
// Never MarkDone the same transaction more than once as this function
// blindly subtracts the transaction's feerate from m_ancestor_set_feerates
Assume(select.IsSubsetOf(m_todo));
```
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668545670)
304d3cb23ba9f084b98f9f29b47e3dfbb61ca334
may be helpful context
```suggestion
// Never MarkDone the same transaction more than once as this function
// blindly subtracts the transaction's feerate from m_ancestor_set_feerates
Assume(select.IsSubsetOf(m_todo));
```
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668665030)
For each of the `CandidateFinder`s, would it be helpful to expose a `AllDone()` function? Seems slightly inconvenient for the caller to separately keep track of `todo` for a `while(todo.Any())` loop. `FindCandidateSet()` Assumes that todo isn't empty, so we can't use that to query.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668665030)
For each of the `CandidateFinder`s, would it be helpful to expose a `AllDone()` function? Seems slightly inconvenient for the caller to separately keep track of `todo` for a `while(todo.Any())` loop. `FindCandidateSet()` Assumes that todo isn't empty, so we can't use that to query.
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668716016)
Note to reviewers: pseudocode for the function when first introduced in 5661fcdb15244bc6d602379c294276c7ec686505 can be found [here](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-21-searching-6)
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668716016)
Note to reviewers: pseudocode for the function when first introduced in 5661fcdb15244bc6d602379c294276c7ec686505 can be found [here](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-21-searching-6)
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668724062)
And I suppose it's not problematic if it is optimal but didn't claim to be so?
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668724062)
And I suppose it's not problematic if it is optimal but didn't claim to be so?
📝 theStack opened a pull request: "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results"
(https://github.com/bitcoin/bitcoin/pull/30408)
The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script".
See also the draft BIP "Terminology for Transaction Components" (https://github.
...
(https://github.com/bitcoin/bitcoin/pull/30408)
The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script".
See also the draft BIP "Terminology for Transaction Components" (https://github.
...
💬 maflcko commented on pull request "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results":
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2214477162)
ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2214477162)
ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b
🚀 ryanofsky merged a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141)
(https://github.com/bitcoin/bitcoin/pull/30141)
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1668941931)
I updated this to no longer clear flags.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1668941931)
I updated this to no longer clear flags.
👍 BrandonOdiwuor approved a pull request: "test, assumeutxo: Remove resolved todo comments and add new test"
(https://github.com/bitcoin/bitcoin/pull/30403#pullrequestreview-2163834970)
Code Review ACK 29d19c414c9e138c99f8de84e398528db92ff07e
I will close https://github.com/bitcoin/bitcoin/pull/29681in favor of this
(https://github.com/bitcoin/bitcoin/pull/30403#pullrequestreview-2163834970)
Code Review ACK 29d19c414c9e138c99f8de84e398528db92ff07e
I will close https://github.com/bitcoin/bitcoin/pull/29681in favor of this
✅ BrandonOdiwuor closed a pull request: "test: loading assumeutxo snapshot start states"
(https://github.com/bitcoin/bitcoin/pull/29681)
(https://github.com/bitcoin/bitcoin/pull/29681)
💬 BrandonOdiwuor commented on pull request "test: loading assumeutxo snapshot start states":
(https://github.com/bitcoin/bitcoin/pull/29681#issuecomment-2214630023)
Closing PR in favor of https://github.com/bitcoin/bitcoin/pull/30403
(https://github.com/bitcoin/bitcoin/pull/29681#issuecomment-2214630023)
Closing PR in favor of https://github.com/bitcoin/bitcoin/pull/30403
💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2214654114)
Oh, sorry @BrandonOdiwuor , I missed https://github.com/bitcoin/bitcoin/pull/29681 😞
(https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2214654114)
Oh, sorry @BrandonOdiwuor , I missed https://github.com/bitcoin/bitcoin/pull/29681 😞
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2214655675)
re: https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2210605752
Thanks for the questions.
> I'm not sure what a good approach for reviewing this might be. How can I validate the changes without tests? Should I be running this as part of a parent pull request?
You could test this as part of #10102, and I should probably add some unit test code to [ipc_test.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/test/ipc_test.cpp) to make sure new runtime code in [capnp/chain.cp
...
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2214655675)
re: https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2210605752
Thanks for the questions.
> I'm not sure what a good approach for reviewing this might be. How can I validate the changes without tests? Should I be running this as part of a parent pull request?
You could test this as part of #10102, and I should probably add some unit test code to [ipc_test.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/test/ipc_test.cpp) to make sure new runtime code in [capnp/chain.cp
...