💬 l0rinc commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3058951465)
code review ACK f43571010e3853e83a21aa4774b1c8da47b5d961
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3058951465)
code review ACK f43571010e3853e83a21aa4774b1c8da47b5d961
🤔 instagibbs reviewed a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3007132369)
review through 18e40b6cd5a11e23b7a0654bdd12f08e610ce980
no big concerns on first pass
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3007132369)
review through 18e40b6cd5a11e23b7a0654bdd12f08e610ce980
no big concerns on first pass
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198611036)
```Suggestion
/** The number of linearization improvements steps needed per cluster to be considered acceptable. */
```
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198611036)
```Suggestion
/** The number of linearization improvements steps needed per cluster to be considered acceptable. */
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198624434)
```Suggestion
Assume(iters >= iters_done);
auto [cost, optimal] = queue[pos].get()->Relinearize(*this, iters - iters_done);
```
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198624434)
```Suggestion
Assume(iters >= iters_done);
auto [cost, optimal] = queue[pos].get()->Relinearize(*this, iters - iters_done);
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198587961)
would this also be trivially true for `<=2`?
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198587961)
would this also be trivially true for `<=2`?
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198654706)
would it be possible to assert that this must return true if the value is high enough since we have a limited txgraph size?
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198654706)
would it be possible to assert that this must return true if the value is high enough since we have a limited txgraph size?
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198606359)
comment below says
"// Make the graph linearize all clusters acceptably."
I'm guessing these chains are trivial to linearize, so this was likely already achieved with 10k iterations?
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198606359)
comment below says
"// Make the graph linearize all clusters acceptably."
I'm guessing these chains are trivial to linearize, so this was likely already achieved with 10k iterations?
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198615257)
```Suggestion
* steps will be performed per cluster before it is considered to be of acceptable quality. */
```
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198615257)
```Suggestion
* steps will be performed per cluster before it is considered to be of acceptable quality. */
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198669205)
double-checking: NEEDS_RELINEARIZE, ACCEPTABLE and OPTIMAL are the only possible cases here, right?
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2198669205)
double-checking: NEEDS_RELINEARIZE, ACCEPTABLE and OPTIMAL are the only possible cases here, right?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198682795)
in case this seems more straight forward to understand
```suggestion
// are wtxid which were already reconsiderable for some peer due to a previous AddChildrenToWorkSet().
```
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198682795)
in case this seems more straight forward to understand
```suggestion
// are wtxid which were already reconsiderable for some peer due to a previous AddChildrenToWorkSet().
```
🤔 w0xlt reviewed a pull request: "rest/rpc: use more util::Join"
(https://github.com/bitcoin/bitcoin/pull/32942#pullrequestreview-3007283767)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/32942/commits/e68a4ba976af59dfd15e3aa432e1ce80e7d21083
(https://github.com/bitcoin/bitcoin/pull/32942#pullrequestreview-3007283767)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/32942/commits/e68a4ba976af59dfd15e3aa432e1ce80e7d21083
🤔 w0xlt reviewed a pull request: "fuzz: Add missing calls to `SetMockTime` for determinism"
(https://github.com/bitcoin/bitcoin/pull/32927#pullrequestreview-3007292861)
ACK https://github.com/bitcoin/bitcoin/pull/32927/commits/fa8862723c14cdd471bbffc7a4897ece2e6d8a7f
(https://github.com/bitcoin/bitcoin/pull/32927#pullrequestreview-3007292861)
ACK https://github.com/bitcoin/bitcoin/pull/32927/commits/fa8862723c14cdd471bbffc7a4897ece2e6d8a7f
💬 achow101 commented on issue "guix: Zip file non-determinism when building in WSL":
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3059133385)
Switching to a non-ntfs drive works I guess. But it would be preferable if the build was filesystem agnostic.
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3059133385)
Switching to a non-ntfs drive works I guess. But it would be preferable if the build was filesystem agnostic.
💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3059151670)
Follow-up from a while back: let's just explicitly say which layer of txgraph we're querying: https://github.com/instagibbs/bitcoin/commit/bb48f6c4736c227bcee6c4dda8e95b0b0287cfef
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3059151670)
Follow-up from a while back: let's just explicitly say which layer of txgraph we're querying: https://github.com/instagibbs/bitcoin/commit/bb48f6c4736c227bcee6c4dda8e95b0b0287cfef
💬 achow101 commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#issuecomment-3059157165)
ACK 150b5c99ca11885ef15d04139d919d734e2c211a
(https://github.com/bitcoin/bitcoin/pull/32758#issuecomment-3059157165)
ACK 150b5c99ca11885ef15d04139d919d734e2c211a
🚀 achow101 merged a pull request: "wallet: remove dead code in legacy wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32758)
(https://github.com/bitcoin/bitcoin/pull/32758)
💬 achow101 commented on pull request "test: less ambiguous error if bitcoind is missing":
(https://github.com/bitcoin/bitcoin/pull/32921#issuecomment-3059204041)
ACK 83bb41455715a9e05320ba791987204031626c10
(https://github.com/bitcoin/bitcoin/pull/32921#issuecomment-3059204041)
ACK 83bb41455715a9e05320ba791987204031626c10
🚀 achow101 merged a pull request: "test: less ambiguous error if bitcoind is missing"
(https://github.com/bitcoin/bitcoin/pull/32921)
(https://github.com/bitcoin/bitcoin/pull/32921)
💬 theuni commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3059223967)
Yikes. Concept ACK.
> - The [export(PACKAGE)](https://cmake.org/cmake/help/latest/command/export.html#package) command does not populate the user package registry when [CMP0090](https://cmake.org/cmake/help/latest/policy/CMP0090.html#policy:CMP0090) is set to NEW unless the [CMAKE_EXPORT_PACKAGE_REGISTRY](https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_PACKAGE_REGISTRY.html#variable:CMAKE_EXPORT_PACKAGE_REGISTRY) variable explicitly enables it. When [CMP0090](https://cmake.org/cmak
...
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3059223967)
Yikes. Concept ACK.
> - The [export(PACKAGE)](https://cmake.org/cmake/help/latest/command/export.html#package) command does not populate the user package registry when [CMP0090](https://cmake.org/cmake/help/latest/policy/CMP0090.html#policy:CMP0090) is set to NEW unless the [CMAKE_EXPORT_PACKAGE_REGISTRY](https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_PACKAGE_REGISTRY.html#variable:CMAKE_EXPORT_PACKAGE_REGISTRY) variable explicitly enables it. When [CMP0090](https://cmake.org/cmak
...
📝 w0xlt opened a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944)
Based on discussions in https://github.com/bitcoin/bitcoin/pull/32803, this PR proposes removing the ` upgradewallet` RPC.
(https://github.com/bitcoin/bitcoin/pull/32944)
Based on discussions in https://github.com/bitcoin/bitcoin/pull/32803, this PR proposes removing the ` upgradewallet` RPC.
🤔 w0xlt reviewed a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-3007546189)
ACK https://github.com/bitcoin/bitcoin/pull/32631/commits/a60f863d3e276534444571282f432b913d3967db
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-3007546189)
ACK https://github.com/bitcoin/bitcoin/pull/32631/commits/a60f863d3e276534444571282f432b913d3967db