💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1282968177)
See my previous comment: https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1663626986

💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1282970013)
Obviously, no existing software that can only read version `1`, can thus not read version `2`.

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282831868)
removed comment block

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282832651)
Done

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282920253)
Sure, changed.

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282889026)
Added

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282853373)
added

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282834565)
Done, and renamed to `assert_no_immediate_response`

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282919246)
changed so we grab the utxos before making any transactions, so they all have to be confirmed and not related

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282837698)
added

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282850596)
Edited the comments.

Without delays and filtering right before sending, I figured you could query whether `tx_real` has arrived in the node's mempool yet by sending a fake orphan that spends from it (if the node requests `tx_real` they don't have it yet, if they don't request it then it's already in mempool/seen). Hence "reveal." But you can only know whether `tx_real` arrived in the last ~2-4 seconds.

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282862310)
True this could be combined. The main idea for this test is that this transaction's failure propagates all the way to the grandchild.

💬 fanquake commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1663715708)
> Created in the context of https://github.com/bitcoin/bitcoin/issues/28109, but I don't think it is enough to actually fix this problem.

It's a step in the right direction, we're now failing on `45934` sections, compared to `46006` in #28109. However we're still a long way north of the limit, which as I understand it, is `32768`.

👍 stickies-v approved a pull request: "net, refactor: remove unneeded exports, use helpers over low-level code, use optional"
(https://github.com/bitcoin/bitcoin/pull/28078#pullrequestreview-1560767388)
ACK 4ecfd3eaf434d868455466e001adae4b68515ab8

That said:
1. I was ~0 on the `[[nodiscard]]` changes before, but am now in favour of removing all the ones in this PR. The only place where it made sense, imo, was in 5ba73cd0ee1e661ec4d041ac8ae7a60cfd31f0c0 for `[[nodiscard]] static bool GetLocal(CService& addr, const CNode& peer)` where it's dangerous to use the out-parameter without checking the return value. Now, in all cases, the functions are entirely self explanatory and I don't really see
...

💬 feed3r commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663735680)
Thank you all guys, yes this is not a bug and I was only looking for any common experience.
I had a solution anyway and will go with a release checking the compatibility in the release notes.
Thank you all again!

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1283012394)
Note to self, this should be 1023

🤔 glozow reviewed a pull request: "refactor: Make more transaction size variables signed"
(https://github.com/bitcoin/bitcoin/pull/28059#pullrequestreview-1560810508)
ACK 92de74ef181b42d774bc6b12329bc0c27caf0081

🚀 glozow merged a pull request: "refactor: Make more transaction size variables signed"
(https://github.com/bitcoin/bitcoin/pull/28059)

💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283023840)
Yeah, unless any objections this would have my preference, less side effects and easier to understand imo:

<details>
<summary>git diff on 26fe348e6e05d9b50614fdcba9cfb7a711ae86dc</summary>

```diff
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 54c4685c48..48a6c9af00 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -92,9 +92,6 @@ private:
const CDBWrapper &parent;
leveldb::WriteBatch batch;

- DataStream ssKey{};
- CDataStream ssValue;
-
size_t
...

📝 darosior opened a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209)
This introduces a small fuzz target for `CBlockTreeDB` which asserts a few invariants by using an in-memory LevelDb.