💬 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.

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1553741863)
It took several days to track down, but... I believe the issue with clang >=11 comes from: https://github.com/llvm/llvm-project/commit/6fa3894c4e771c773712b1ae777f78c1c922a908 . From my local tests I'm able to revert that and get back to expected results.

So I guess we'll need to patch it out of guix.

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198379250)
Still a nub on cpp,
`constexpr auto FEE_FLUSH_INTERVAL{1h};` does not compile
I added instead `static constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1};`

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198379499)
Thanks fixed

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1198388360)
>Mine as well use this constant here for the filename?

:thinking: it is an anon namespace, not defined in policy/fees_args.h,
I think it is okay as is, or else

policy/fees_args.h
```
const char* GetFeeEstimatesFilename();
```
policy/fees.args.cpp
```
const char* GetFeeEstimatesFilename() {
return FEE_ESTIMATES_FILENAME;
}
```
policy/fees.cpp
```
LogPrintf("Flushed fee estimates to %s.\n", GetFeeEstimatesFilename());
```
but if it's possible another way I will be glad
...

💬 sdaftuar commented on pull request "validation: have LoadBlockIndex account for snapshot use":
(https://github.com/bitcoin/bitcoin/pull/23174#discussion_r1198450499)
Sorry for chiming in on what is now a very old PR, but I believe this block of code is incorrect. If we have a snapshot in place (and so some block index is marked IsAssumedValid), but we have downloaded blocks that haven't yet been activated (eg on the background-validation chainstate), this logic prevents us from adding those downloaded-but-not-yet verified blocks to `setBlockIndexCandidate` for the background chainstate.

I ran into this while working on a download implementation for assum
...

💬 ajtowns commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1553973702)
Maybe we can move the boost option into a new `--enable-slow-debug` option, and just enable that for fuzzing and perhaps one of the CI tasks?

```diff
diff --git a/configure.ac b/configure.ac
index cbbf8d6172..fbf7a72e7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,6 +306,13 @@ AC_ARG_ENABLE([debug],
[enable_debug=$enableval],
[enable_debug=no])

+dnl Enable slow debug
+AC_ARG_ENABLE([slow-debug],
+ [AS_HELP_STRING([--enable-slow-debug],
+ [
...

💬 MarcoFalke commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1198582434)
This function is now unused?

💬 MarcoFalke commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1198583011)
```suggestion
}
// Silence a compiler warning about unused function.
(void)GetDevURandom;
```