💬 sdaftuar commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690139736)
Maybe a different variable here would be better, since `i` is already used in the outer loop?

💬 sdaftuar commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1688552973)
We could consider checking that the entries in `linearization` are all in the expected range [0, depGraph.TxCount()), to avoid a crash here if a garbage linearization were passed in?

👍 hodlinator approved a pull request: "rest: Reject truncated hex txid early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30482#pullrequestreview-2196468742)
ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a

`make check` passed and `test_runner.py` passed.

Happy you went with `FromHex()`!

💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689620422)
Man, I was worried for a moment that this modification of switching to the `uint8_t` was setting the other end of the `base_blob` array, but it's the same. Will be glad to see the fragile function go. :+1:

💬 sdaftuar commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2248546746)
Benchmark results on my ryzen 7995wx:

| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1,474.01 | 678,422.17 | 0.3% | 0.01 | `LinearizeNoIters16TxWorstCaseAnc`
| 1,473.48 | 678,667.50 | 0.2% | 0.01 | `LinearizeNoIters16TxWorstCaseLIMO`
| 4,763.02 | 209,951.04 | 0.1% | 0.01 | `LinearizeNoIters32TxWo
...

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2248589227)
re: https://github.com/bitcoin/bitcoin/pull/30509#issue-2425377062

> The default socket path is currently `<datadir>/sockets/bitcoin-node.sock`, but this can be customized.

I changed the default socket path to `<datadir>/node.sock` after running into some "exceeded maximum socket path length" errors. The maximum unix socket path length is only 107 characters, so I thought a shorter suffix might avoid problems on systems with long data directory or home directory paths.

Another possible
...

👍 willcl-ark approved a pull request: "Fee Estimation: change `estimatesmartfee` default mode to `economical`"
(https://github.com/bitcoin/bitcoin/pull/30275#pullrequestreview-2197582258)
ACK 25bf86a225b0df3f48ade1016b47f5ee1636b988

Looks good to me with those nits fixed 👍🏼

💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690313716)
Attempted cleaning that up, but didn't uncover anything useful to the end users, unlike #30444, so probably not worth following up on:

```diff
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index 89c03c1647..dab8b81128 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -274,8 +274,8 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu

// extract and validate vout
const std::string& strVout = vStrInputParts[1];
- int64_t vou
...

💬 mzumsande commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690338607)
nit: After this, the log
`Committed X changed transaction outputs (out of Y) to coin database...`
has become less interesting because we only iterate over dirty coins, so `X=Y` always.
I might be more helpful to report the initial size of `m_map` of the cursor (before entries are deleted from it) as the total.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690361434)
There are other places that could be cleaned up, such as obviated dirty checks like [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-cafbe1353eff6084b73fd3b6c3dee603e0827348fdd2fe12dfad1e01003a84edR118) and [here](https://github.com/bitcoin/bitcoin/pull/28280/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77cR195), but this should probably done after a https://github.com/bitcoin/bitcoin/pull/18746 revival so there's no confusion.

💬 mzumsande commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690365906)
yes, no need to do that here. I just mentioned the logging here because I was interested in the ratios to assess the efficiency of this PR, and had to write my own log.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1690367507)
Also, that line predates #17487, back when all entries were dirty anyways.

👍 TheCharlatan approved a pull request: "guix: GCC 12 consolidation"
(https://github.com/bitcoin/bitcoin/pull/30511#pullrequestreview-2197680394)
ACK d1592d2eee1913e734a4f92907e796eb3350c64a

Guix build (aarch64):
```
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d
...

💬 TheCharlatan commented on pull request "guix: GCC 12 consolidation":
(https://github.com/bitcoin/bitcoin/pull/30511#discussion_r1690370855)
Hope this gets merged soon so we can drop this again.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385057)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385537)
Added the mermaid code as comments to the benchmark.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385632)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690385744)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386555)
I've added comments explaining this, and also moved the "test correspondence between Cluster and DepGraph" to a new `VerifyDepGraphFromCluster` function that's invoked in both the unit and fuzz tests.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1690386667)
Done.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2248844497)
added fuzz and unit tests which cover more than the functional tests can alone