💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442764742)
```suggestion
const uint64_t max_outbound_limit{fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
```

this will result in 0 some of the time.

💬 TheCharlatan commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878520710)
Re comment https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1804912723
> Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?

That code seems to be unused currently if `BUILD_BITCOIN_INTERNAL` is defined, so this seems like an improvement to me.

💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878521297)
> compiles regardless of the BUILD_BITCOIN_INTERNAL macro. Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?

Thanks for pointing this out. Yes it was intentional as I believe this is currently wonky in master.

Note that libbitcoinconsensus is the only user of `LIBBITCOIN_CRYPTO_BASE`. And because it never calls `SHA256AutoDetect` it will never actually use `sha256_sse4::Transform`. It's therefore useless to include sse4 in `LIBBITCOIN_CRYPTO_BASE`.

I'll push anot
...

💬 theuni commented on pull request "rpc: Remove deprecated -rpcserialversion":
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1878533182)
Post-merge utACK fa46cc22bc696e6845915ae91d6b68e36bf4c242. Nice cleanup :)

👍 TheCharlatan approved a pull request: "build: remove systemtap variadic patch"
(https://github.com/bitcoin/bitcoin/pull/29181#pullrequestreview-1805812259)
ACK 6047e25035b7dc07f25e8a756930d5cf3d92cd9f

🤔 hebasto reviewed a pull request: "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256"
(https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1805820587)
Approach ACK 9235801256a63a9ca4f8c95a005b5151afa9a5d4.

🤔 maflcko reviewed a pull request: "test: randomize perturbed files excluding ldb"
(https://github.com/bitcoin/bitcoin/pull/29182#pullrequestreview-1805830780)
Please keep your fixup commits squashed according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 maflcko commented on pull request "test: randomize perturbed files excluding ldb":
(https://github.com/bitcoin/bitcoin/pull/29182#discussion_r1442799108)
are block files guaranteed to contain this many bytes of blocks, always?

💬 glozow commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878572381)
This is a summary of the arguments on the PRs, mailing list posts, and some tweets.

### ACKs by reason

If you feel I have misrepresented something you said, feel free to let me know.

#### "Stop inscriptions, which are spam"

These types of transactions are used for ordinals, NFTs, data-carrying, or some use case that is not
financial transactions. This is "spam" and undermines the utility of Bitcoin for
payments due to high transaction traffic and fees.
- "these spam transactions m
...

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1878580405)
[clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt](https://github.com/bitcoin/bitcoin/files/13841869/clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt)


✅ TheCharlatan closed a pull request: "mempool: Don't sort in entryAll"
(https://github.com/bitcoin/bitcoin/pull/29019)

💬 TheCharlatan commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1878590817)
Seems like the new behavior incidentally fixed a bug, so closing this again in favor of #29179. Could do a follow-up eventually to make the `entryAll` call when processing the `getrawmempool` RPC call more efficient by skipping the sorting.

📝 glozow unlocked a pull request: "doc: revert clarify -datacarriersize"
(https://github.com/bitcoin/bitcoin/pull/29173)
The latest update of the help text of `-datacarriersize` is incorrect.

The purpose of this standardization rule is not to target only the data contained in the raw scriptPubKey, but all forms of arbitrary data.

It is incorrect to change the description of this option if an attempt to update it was made without being merged.

Context:

The [first inscription](https://mempool.space/tx/6fb976ab49dcec017f1e201e84395983204ae1a7c2abf7ced0a85d692e442799) appeared December 14, 2022 at the heig
...

💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878604328)
Added a brief description of crypto_base.

💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442825960)
The `ConsumeBool` is to ensure that the default value will be used in most cases.

👍 hebasto approved a pull request: "doc: Clarify C++20 comments"
(https://github.com/bitcoin/bitcoin/pull/29042#pullrequestreview-1805901540)
ACK fa87f8feb76da42eeb5c4d32ee7be070b2bd559f, I verified the code with clang-{16,17}.

💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442832240)
No objections on calling `CConnman::Init` instead. With `SetMaxOutboundLimit` we ensure we're only messing with `nMaxOutboundLimit` and perhaps we avoid other locks (?).

💬 jangorecki commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878619928)
You missed the most important, censorship

💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442916555)
`Init` is called in the connman constructor: https://github.com/bitcoin/bitcoin/blob/c80f57ba575af96890f185765a53a62ef58ef2c8/src/net.cpp#L3117-L3118

Calling it again and only changing `nMaxOutboundLimit` achieves the same as you have here with less new code.

💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442914249)
> The ConsumeBool is to ensure that the default value will be used in most cases.

How does it ensure that and why is that useful?