💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484331218)
Is dropping the locktime argument here intentional? (I guess all the explicit arguments are caught in kwargs?)

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483776227)
I can't figure out what this comment means? Maybe a holdover from when we were trimming 0-fee stuff?

💬 epiccurious commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1484386762)
very minor nit - To make the log more readable, can you please add a comma after the 'file'?

So it displays as:
```
Writing 2529 mempool transactions to file, xx MB...
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484387932)
I still would like a test for this, maybe as a follow-up

💬 epiccurious commented on pull request "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`":
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1936030123)
utACK 864e2e9097de8f1fda63137f803687dd5cc96c03.

💬 epiccurious commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1936033190)
Concept ACK.

💬 epiccurious commented on pull request "test: Remove struct.pack from almost all places":
(https://github.com/bitcoin/bitcoin/pull/29401#issuecomment-1936039085)
Concept ACK. Is this ready for testing?

✅ jamesob closed a pull request: "Covenant tools softfork"
(https://github.com/bitcoin/bitcoin/pull/28550)

💬 maflcko commented on pull request "test: Remove struct.pack from almost all places":
(https://github.com/bitcoin/bitcoin/pull/29401#issuecomment-1936044118)
My preference would be to get the pull requests mentioned in the description merged first

💬 maflcko commented on pull request "test: Fix utxo set hash serialisation signedness":
(https://github.com/bitcoin/bitcoin/pull/29399#discussion_r1484406087)
Did that in https://github.com/bitcoin/bitcoin/pull/29401

💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1936047945)
concept ACK

might be good to recap why it was only added in BLOCK processing but not other `ProcessBlocks`: In every other case we already don't punish the sender of compact blocks for failing these higher level checks, while full blocks allow for punishment.

💬 ryanofsky commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484425597)
re: https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484322490

> In any case `m_substream{substream}` still creates a copy when it shouldn't, due to a missing `std::forward`, no?

Oh that's interesting. I wouldn't think to use `std::forward` because normally it's just used for && rvalue reference function template parameters, not class template parameters, and doesn't do anything useful if && is not used and special template deduction rules for it are not applied.

We are actual
...

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484427635)
nit: The reference to "in-package" doesn't fit here.

💬 epiccurious commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1936072870)
Concept ACK 7dc0442ff5013827c051c8ff50c8fd8aa9440d7c.

💬 epiccurious commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#issuecomment-1936081626)
re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796.

💬 maflcko commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1484436443)
> I could imagine this being useful if you wanted to write something like `ParamsStream mystream{FileStream{"file.bin"}, param1, param2}` and have the file automatically closed when the stream was destroyed.

Yes, an example would be in net.cpp, but feel free to ignore, if it is too much hassle.

```cpp
src/net.cpp- DataStream underlying_stream{vSeedsIn};
src/net.cpp: ParamsStream s{underlying_stream, CAddress::V2_NETWORK};

💬 epiccurious commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1936096250)
Concept ACK 0eec878e514c23d33d2cd81c44566ea601eb263f.

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484467280)
I think if I comment out these lines of code, no unit tests or functional tests fail -- is it possible this isn't covered anywhere? (I didn't run the fuzzer with this change)

💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1936174128)
ACK 29029df5c700e6940c712028303761d91ae15847

In addition to my code review and running a slightly earlier version of this branch on a year's worth of historical data, I also did a bunch of mutation testing to confirm code coverage for every component of the new logic (found one block that I think is unreachable, which I commented on above, but doesn't seem like an issue either way).

⚠️ derekm opened an issue: "Enable sanctions enforcement in Spam Filter Code"
(https://github.com/bitcoin/bitcoin/issues/29416)
### Please describe the feature you'd like to see added.

As a miner operating within a sanctions regime,
I need to be able to reject transactions and maybe even blocks that contain sanctioned hashes.

Powerful sanctions regimes provide lists of sanctioned hashes for all major blockchains, including Bitcoin. These hashes end up being incorporated into output script templates that reside on the Bitcoin blockchain, and when these hashes are included, transactions need to be rejected.

### Is yo
...