💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583375522)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583375709)
done

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583375852)
added

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583376000)
taken

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583376085)
I prefer to keep this one as is because I find it more intuitive.

💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#issuecomment-2083168939)
Adressed feedback from @stickies-v , thanks again!

> nit: I think the last 3 commits should be squashed

The doc change is only tangentially related so I will keep it separate. But I squashed the 2nd and 3rd.

💬 glozow commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#issuecomment-2083176753)
ACK f8a141c2dae2471a7ce7248e28a0bbeb8a291acd

💬 glozow commented on pull request "fuzz: don't allow adding duplicate transactions to the mempool":
(https://github.com/bitcoin/bitcoin/pull/29990#issuecomment-2083179739)
utACK cc15c5bfd1eb3903de246c124702a7c66c687008 makes sense to me

💬 kevkevinpal commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#discussion_r1583385359)
fixed in [95897ff](https://github.com/bitcoin/bitcoin/pull/29994/commits/95897ff181c0757e445f0e066a2a590a0a0120d2)

💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2083190086)
@1440000bytes,
Thank you so much for testing the PR and for the feedback. That's very helpful.

(1) Yes, that's good idea, I'll rename it.
(2) and (5) are related. The selectable frequencies are actually also randomized, within the selected frequency range - for example hourly is random from 1 to 2 hours, daily is random from 1 to 2 days etc (see line 934 in deniabilitydialog.cpp). Perhaps we need to rename the frequency UI selection to something that better describes this?
(3) Great idea
...

💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2083201174)
I've tested 5eeafeb302dcf56dd5c16c1e4785a41a132344c7 on Ubuntu 23.10.

While all changes look correct and reasonable to me, I was expecting that this PR removes some of dependencies mentioned in https://github.com/bitcoin/bitcoin/issues/28548.

But that is not the case. The `libbitcoin_util` still depends on `libbitcoin_crypto`, `libbitcoin_consensus` and `libbitcoin_common` libraries.

If such an outcome is expected, maybe update the `doc/design/libraries.md` accordingly?

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1583414798)
Right, *even if* the test would manage to hit the case where it would have considered a package rbf(this case won't), it should be caught in both PreChecks and package mempool checks separately.

👍 stickies-v approved a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2029199846)
ACK 95897ff181c0757e445f0e066a2a590a0a0120d2

🤔 ryanofsky reviewed a pull request: "prune, rpc: Check undo data when finding pruneheight"
(https://github.com/bitcoin/bitcoin/pull/29668#pullrequestreview-2028926937)
Code review 697d0a23f5d0a75829ffe1ac874eaf15fff3bfbe. Approach seems good and first commit seems ok in its current form but I think version of GetPruneHeight in the second commit is not handling the genesis block correctly.

💬 ryanofsky commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583454227)
In commit "rpc: Make pruneheight also reflect undo data presence" (61f352d6bcf485862815fdd68064cc07ea8ab6ce)

This doesn't seem right. If chain consists of just genesis block, and genesis block doesn't have undo data this will return height 0 as if genesis block were pruned.

💬 ryanofsky commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583398465)
In commit "refactor, blockstorage: Generalize GetFirstStoredBlock" (4e62129d8aed839c83fa4a595aab06ed5ca7eeb6)

IMO, this logic is a little confusing because it using both `nHeight` and `pprev` instead of just using `pprev` consistently. Also I think the cases here could have more explanation.

```c++
// Get first block with data, after the last block without data.
// This is the start of the unpruned range of blocks.
const auto first_unpruned{active_chainstate.m_blockman.GetFirstBlock(cha
...

💬 ryanofsky commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583472496)
In commit "refactor, blockstorage: Generalize GetFirstStoredBlock" (4e62129d8aed839c83fa4a595aab06ed5ca7eeb6)

I don't think it makes sense to call this parameter `active_chainstate`. It is true the active chainstate is always passed, but this function should care whether the chainstate is active or not. Probably also it would be better for this function not to access the Chainstate class at all and instead be passed separate BlockMan and CChain parameters.

💬 ryanofsky commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583515582)
In commit "rpc: Make pruneheight also reflect undo data presence" (61f352d6bcf485862815fdd68064cc07ea8ab6ce)

This comment "Result can never be 0 because genesis block is never pruned" is misleading and basically describes the opposite of what is happening. If the genesis block was not pruned you would expect GetFirstBlock to return the genesis block, because it has data, and for the result to still be 0 here like it was in the previous commit. The reason `GetFirstBlock` is no longer returning
...

💬 ryanofsky commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1583335298)
In commit "refactor, blockstorage: Generalize GetFirstStoredBlock" (dd7696b97f20d756df1afa523a8f9e7bf3924c7d)

I think there are a few issues with this comment:

- The @<!-- -->brief and @<!-- -->return sections make it sound like it is returning the earliest block matching the flags, when actually it is returning the earliest block matching the flags after the latest block _not_ matching them. This is different because it means not only does the returned block have the flags, but all blocks
...

📝 Sjors opened a pull request: "guix: fix suggested fake date for openssl-1.1.1l"
(https://github.com/bitcoin/bitcoin/pull/29999)
Using `2020-10-01` as the fake timestamp will cause many test failures with `/gnu/store/bfirgq65ndhf63nn4q6vlkbha9zd931q-openssl-1.1.1l.drv`. I didn't investigate why, but I guess because it's _before_ the test certificates were created. They expired in June 2022. I tried a month before that, which worked.

Also fixes layout of instructions.

🤔 MarnixCroes reviewed a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2029148876)
cack