💬 ajtowns commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1427447113)
Reworded. Note there's lots of places in that file that talk about "bitcoind"...

💬 ajtowns commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1427448503)
Added a test; tagged as refactor for whatever that's worth

💬 ariard commented on pull request "Covenant tools softfork":
(https://github.com/bitcoin/bitcoin/pull/28550#issuecomment-1857107415)
To the best of my comprehension, most of the use-cases brought as a justification for the consensus changes introduced in this PR are relying on UTXO-sharing / time-locks as part of their design. I think most of them are affected by the following security & risks issues:
- [thundering herds](https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-September/004095.html)
- replacement cycling and pinning attacks
- [timewarp-based miners harvesting risks](https://delvingbitcoin.org/t/time
...

💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1427479087)
> And, again, the user can set up a symlink from /tmp/test_common_Bitcoin\ Core/ to anywhere, if there's a desire not to use /tmp for some reason (but that should be rare).

Have you seen https://github.com/bitcoin/bitcoin/pull/26684#pullrequestreview-1566578747 and https://github.com/bitcoin/bitcoin/pull/28955#issuecomment-1829948567?
It seems that we need it.

> Well, if we want the datadir pathnames to be static (which is what I'm trying to accomplish with this PR), then we must delete t
...

📝 luke-jr opened a pull request: "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition"
(https://github.com/bitcoin/bitcoin/pull/29086)
Instead of duplicating mempool options two places, just include the Options struct directly on the CTxMemPool

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525133)
good find! i've added a comment.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525183)
definitely useful to have. done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525201)
changed it to send 0-9 number of decoy packets randomly.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525236)
good catch! changed it to `None`.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525256)
right! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525305)
oh makes sense to `s/ellswift_and_garbage_data/send_handshake_bytes` in `p2p.py`. didn't change the name in a test where we [send ellswift in parts](https://github.com/bitcoin/bitcoin/blob/ad0ae3d2128d04ff4f62a4bf612286d153dc314b/test/functional/p2p_v2_earlykeyresponse.py#L78).

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525326)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525347)
nice, i've kept it outside the loop since we don't need it anywhere else for now.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525364)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1427525400)
true, replaced it with a check using `v2_receive_packet` return values which are anyways computed.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1857250302)
thanks for the reviews @mzumsande, @sipa, @theStack! i've updated the PR to address your comments. sorry for the delay since i was away.

🤔 ajtowns reviewed a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1783013964)
utACK fa45567e030272d34845e6b563a6b626b9eda739

💬 ajtowns commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1427470728)
Perhaps describe it as "Helper concept for basic byte types" ? "BasicByte" is a concept that applies to types, not a type itself? Could name the concept as `BasicByteType` since it applies to types?

Consider adding .clang-format fields:

```
BreakBeforeConceptDeclarations: Always
RequiresExpressionIndentation: OuterScope
```

💬 ajtowns commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1427481753)
Could consider:

```c++
template <class Stream>
concept WritableStream = requires(Stream& s, const Span<std::byte> src) { s.write(src); };

template <class T, class Stream>
concept Serializable = WritableStream<Stream> && requires(T a, Stream s) { a.Serialize(s); };

template <class Stream>
concept ReadableStream = requires(Stream& s, Span<std::byte> dst) { s.read(dst); };

template <class T, class Stream>
concept Unserializable = ReadableStream<Stream> && requires(T a, Stream s) {
...

💬 DOD1000 commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427576230)
Thank you for your interest, my dear friend. Can you explain further? Thank you