💬 0xB10C commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1664441492)
Concept ACK. I opened issue https://github.com/bitcoin/bitcoin/issues/16721 a while ago but closed it as it didn't get much attention back then.

> I am not aware of anyone reading the `mempool.dat`, is there?

I've written a mempool.dat parser a few years ago for fun. However, as you said, the RPCs are powerful enough and if someone really really wants to read the file, they can implement XOR functionality. Similar to block0000.dat files, these files are not something considered an interf
...

💬 jonatack commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#discussion_r1283579011)
done

💬 jonatack commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#discussion_r1283579212)
done

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283582108)
updated the commit message and incorporated brace initialization in latest push

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283584387)
updated in latest push. no longer any references to the bundling in the commit messages

💬 jonatack commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#issuecomment-1664459802)
Updated the pull description and repushed to take @MarcoFalke's review feedback (thanks!)

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283585262)
updated in latest push. I think I caught them all

💬 instagibbs commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283587279)
`assert_raises_rpc_error` is probably what you want

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283594372)
updated to an `std:array` in the latest push

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283595087)
incorporated here in latest push

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283596113)
done in latest push

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283597304)
@paplorinc I like the second approach, it seems a bit cleaner and clearer to me. However, it doesn't seem to be used in other functional tests so maybe the first approach is better to be more consistent?

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283599071)
added the network to print statement

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283599461)
@instagibbs Thanks! I was looking at that option too, however I couldn't see a way to use it in this context without having to change a lot of the code, but maybe I'm missing something obvious? 🤔

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1283605312)
We can - In fact the handshake itself is sent this way (that's the nice part about this abstraction, the caller doesn't know or care whether bytes being sent are on behalf of a message we're trying to send or something else).

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1283606280)
Yeah, it should be 0 at this point, but that won't remain with the next commit. A temporary explanation message could be added to explain that here.

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1283610490)
It's because I don't want a recursive lock, and I'm introducing a caller of `CompleteInternal()` that already holds `m_cs_recv`.

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283615894)
good catch! totally agree with the suggestion - def agree that we shouldn't introduce more uses of recursive mutexes, esp when not necessary.

that said, I'm struggling a bit to communicate with the compiler-

when I add both the runtime `AssertLockHeld` and annotation of `EXCLUSIVE_LOCKS_REQUIRED`, I'm getting a compile time warning from `-Wthread-safety-analysis`

<details>
<summary>diff</summary>


```
diff --git a/src/net.cpp b/src/net.cpp
index a504289fa5..228d081aa6 100644
--
...

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1664500630)
thanks for the reviews & feedback @naumenkogs, @vasild, @willcl-ark & @fanquake

I've updated this PR to incorporate all the open feedback & fix up the commit messages. everything should be addressed except for these two open items:
1. question about locking improvement https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283615894
2. release note which I will do in the follow-up, to prevent possible review churn on language feedback

otherwise, should be ready for re-review. thank
...

💬 Malk8628 commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283620563)
I've been hacked