✅ hebasto closed a pull request: "Make network graph slider easier to use"
(https://github.com/bitcoin-core/gui/pull/483)

🤔 ryanofsky reviewed a pull request: "index: improve initialization and pruning violation check"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1433254608)
Reviewed dd9bc393ebd4ba915ca94991390b6e98d29dcfaf, which looks good overall, but I had questions about few things

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198165818)
In commit "init: start indexes after the loadblk thread finishes" (ca3041984cf3665e27b6783c923ab1c32682500a)

Could we change "indexers" to "indexes" in all new code? I don't think distinguishing between an "index" and "indexer" really clarifies anything, and the word "indexers" seems a little odd and unexpected.

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198184049)
In commit "init: start indexes after the loadblk thread finishes" (ca3041984cf3665e27b6783c923ab1c32682500a)

One side-effect of this change is that now if there is a "best block of the index goes beyond pruned data" error, bitcoind successfully starts and returns 0 then exits, where previously it could fail early and actually return an error code.

That aspect of the previous behavior seems better to me, though I'm not sure what the tradeoffs are.

Do you think the the main benefit of th
...

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198191133)
In commit "index: simplify pruning violation check" (704c9e4cf67468708db655226a78489b92ef0523)

Would be good to make this declaration use the same argument name as the definition which is `BlockManager::GetFirstStoredBlock(const CBlockIndex& start_block)`

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198195900)
In commit "index: prevent race by calling 'CustomInit' prior setting 'm_synced' flag" (21812970cc56191e18f96692e47f00ed881bd596)

I think it would be nice to split this commit off into a separate PR is possible, so we have a minimal bugfix that could backported (and also be easier to understand)

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198193156)
In commit "index: simplify pruning violation check" (704c9e4cf67468708db655226a78489b92ef0523)

Could the commit message clarify if any behavior is changing here? Would be good to label it a refactoring and say it does not change behavior if nothing is changing, and say what is changing otherwise.

Also, it seems like new code that is added here just gets moved / deleted later in the PR in the last commit "index: verify blocks data existence only once" (dd9bc393ebd4ba915ca94991390b6e98d29dcf
...

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1198198410)
In commit "refactor: index, decouple 'Init' from 'Start'" (26bd60eafc42484d80d5dfef0fa0abb77e7817ce)

Init return value is ignored. I think this needs to return false if it fails. Would also be good to mark Init declaration with `[[nodiscard]]`

💬 dooglus commented on issue "v25.0rc2 unusably slow at times":
(https://github.com/bitcoin/bitcoin/issues/27700#issuecomment-1553533679)
@MarcoFalke Thanks. I use this:

`./configure CXXFLAGS=-fno-omit-frame-pointer --with-incompatible-bdb --with-miniupnpc=no --enable-debug`

Closing the issue. And sorry for the dup.

✅ dooglus closed an issue: "v25.0rc2 unusably slow at times"
(https://github.com/bitcoin/bitcoin/issues/27700)

💬 pinheadmz commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1553563822)
Question about the guide @ismaelsadeeq

https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Candidate-Testing-Guide#test-finalizing-a-psbt-with-inputs-spending-miniscript-compatible-p2wsh-scripts-and-test-spending-the-coin

Is the explanation of the descriptor correct?

it looks to me like simplified its `or(tprv8Zgx, and(tprv8Zgx, and(tprv8iF7, older(10))))` which IIUC means "either the first key OR all three of the remaining conditions (the two other keys and the timelock)
...

💬 instagibbs commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1553565821)
Having the number of allowed evictions be a function of the descendant limit makes replacements easier to reason about from a pinning perspective. e.g., limit of 25/100 means that you can batch CPFP 4 transactions safely, ignoring other pinning vectors(we'll get around to those!). Another smaller point is currently people opting into larger descendant limits for whatever reason can cause this to fall to possibly 0. i.e. a chain of 101 txns that can't have eldest tx evicted.

💬 brunoerg commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1198247881)
Suggestion to simplify it, in case you touch it again:
```diff
index 50cce9318..cd9268f6b 100755
--- a/test/functional/p2p_compactblocks.py
+++ b/test/functional/p2p_compactblocks.py
@@ -847,29 +847,14 @@ class CompactBlocksTest(BitcoinTestFramework):
return block, cmpct_block

# First, make delivery_peer, inbound_peer, and outbound_peer high
bandwidth
- block, cmpct_block = announce_cmpct_block(node, delivery_peer, 1)
- msg = msg_blocktxn()
-
...

💬 pinheadmz commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1553584049)
Other feedback about the guide:

- You can add `-named` and `-minconf=100` to `listunspent` to find the mature coinbase coins more easily

- If testers download a binary, it may be GUI only (e.g. macOS) which is fine but may require a few extra params for example I ran this:
` .../Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -server -regtest -printtoconsole=1`

🤔 theuni reviewed a pull request: "test: Explicitly specify directory where to search tests for"
(https://github.com/bitcoin/bitcoin/pull/27561#pullrequestreview-1433378196)
Concept ACK for CMake.

The `path.join` change is easy, but I'll defer to someone more familiar with the test framework on the path change.

:lock: fanquake locked an issue: "DSE"
(https://github.com/bitcoin/bitcoin/issues/27701)

💬 sdaftuar commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1553666857)
> Having the number of allowed evictions be a function of the descendant limit makes replacements easier to reason about from a pinning perspective. e.g., limit of 25/100 means that you can batch CPFP 4 transactions safely, ignoring other pinning vectors(we'll get around to those!). Another smaller point is currently people opting into larger descendant limits for whatever reason can cause this to fall to possibly 0. i.e. a chain of 101 txns that can't have eldest tx evicted.

Thanks for menti
...

💬 ismaelsadeeq commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1553669447)
> https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Candidate-Testing-Guide#test-finalizing-a-psbt-with-inputs-spending-miniscript-compatible-p2wsh-scripts-and-test-spending-the-coin
>
> Is the explanation of the descriptor correct?

Sure that is the correct explanation, I got the same output from https://bitcoin.sipa.be/miniscript/
Miniscript input `or_d(pk(A),and_v(v:pk(B),and_v(v:pk(C),older(10))))`
Script output
```
<A> OP_CHECKSIG OP_IFDUP OP_NOTIF
<B> OP_CHECKSI
...

👍 pablomartin4btc approved a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1433505009)
Liked the refactoring and the 2 commits split as @hebasto requested.
Tested ACK both commits separately, also checked also other RPC commands, it works as expected.

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198355535)
Thanks for your review, I have fixed it.