🤔 mzumsande reviewed a pull request: "fuzz: Test headers pre-sync through p2p interface"
(https://github.com/bitcoin/bitcoin/pull/28043#pullrequestreview-1846077000)
just fyi, I started reviewing at some point, my problem was that I don't like test-specific / mocking code in production code (especially the consensus-critical parts), so I wasn't really comfortable (concept)-acking it.
On the other hand, I didn't see a better way to do it and fuzzing is great obviously...

As a result, I was undecided and didn't write anything. Maybe it'd be worth to have a general discussion about the extent of mocking / test-specific production code we want as a project,
...

👍 dergoegge approved a pull request: "fuzz: Print coverage summary after run_once"
(https://github.com/bitcoin/bitcoin/pull/29329#pullrequestreview-1846088199)
tACK fa74d2d54f3a102221fb24790c0dfb5c60758535

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1912341078)
For your consideration, I have pushed a variant with some recommended changes here: https://github.com/luke-jr/bitcoin/commits/rpccookieperms-26%2Bknots/

Feel free to cherry-pick or squash into your PR as you deem best.

💬 luke-jr commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1912353847)
Concept NACK to restricting it to only manual connections. This will cause random breakage when automatic peering happens to connect to a manual peer first.

🚀 fanquake merged a pull request: "fuzz: also set MSAN_SYMBOLIZER_PATH"
(https://github.com/bitcoin/bitcoin/pull/29327)

💬 mzumsande commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1912368151)
> Concept NACK to restricting it to only manual connections. This will cause random breakage when automatic peering happens to connect to a manual peer first.

did you see #28895? We don't make automatic connections to manually specified peers anymore.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467890537)
I've made it "We don't check whether the sibling is to-be-replaced (done in ApplyV3Rules) because that doesn't apply in a package."

💬 glozow commented on pull request "doc: update `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1467894205)
You've deleted the comment entirely?

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467907205)
Cleaned up, thanks

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467908805)
Added "Should be called for package transactions to fail more quickly" to the doc (as in: not must call but should call).

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909437)
Or I guess we could turn it off for `AcceptMultiple`?

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909613)
Done

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909813)
Done

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467909921)
Deleted

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467910021)
Added

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467910132)
Deleted

💬 vasild commented on pull request "CKey: add Serialize and Unserialize":
(https://github.com/bitcoin/bitcoin/pull/29295#discussion_r1467912714)
Do `CKey::Check()` on `keydata[0]`?

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1467934024)
If we're focusing on small packages for now, can we just disable it, rename the function something "obviously" single txn related, and then in future updates we can reintroduce the check as an optimization if we like?

💬 josibake commented on pull request "CKey: add Serialize and Unserialize":
(https://github.com/bitcoin/bitcoin/pull/29295#issuecomment-1912459938)
> I need to read and write two private keys to/from disk

Not familiar with the stratumv2 spec, but this seems odd to me and also not a good fit for `CKey`. FWIW, there is `CPrivKey` for producing a serialized OpenSSL private key

💬 josibake commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1912466568)
Concept ACK