💬 Retropex commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1669520329)
Bitcoin: A Peer-to-Peer Electronic ~~Database~~ Cash System

📝 MarcoFalke opened a pull request: "refactor: Remove unused boost signals2 from torcontrol "
(https://github.com/bitcoin/bitcoin/pull/28240)
Remove unused boost, and other includes, and other legacy functions from torcontrol.

💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669563048)
Removing boost seems unrelated and can be done in a follow-up. You can help by reviewing the first step: https://github.com/bitcoin/bitcoin/pull/28240

💬 iBeizsley commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1669583985)
> Bitcoin is primarily for data storage

No. This is an absurd take.

Bitcoin is an absolutely horrible solution for storing arbitrary data in a permissionless, uncensorable manner.

If you want to store data as such, use something like a DHT torrent. It's a better solution, and doesn't force others to unwillingly store your arbitrary data.

Bitcoin is a monetary network first, but it does also happen to be the best decentralised option we have for timestamping too. But to make use of that secon
...

📝 fanquake locked a pull request: "Remove arbitrary restrictions on OP_RETURN by default"
(https://github.com/bitcoin/bitcoin/pull/28130)
Any number or size of data-carrying OP_RETURN outputs are allowed, and the `-datacarrier` option is removed. For those who want to limit data carrying outputs, `-datacarriersize` is still supported, and has the functionality of applying the specified data carrier limit as well as limiting the number of data carrying OP_RETURN outputs to one. If `-datacarriersize=0` is set, no data carrying output is allowed.

Rational: there's lots of ways for people to publish data in the Bitcoin chain, a
...

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287123567)
Yea that works, applied in the latest push

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287123814)
removed

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287124882)
I kinda like making this explicit but the assignment operators are not necessary so i removed them

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287125133)
Done

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287125465)
Done

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287125880)
Done

💬 instagibbs commented on pull request "p2p: ensure mapBlockSource is removed from in ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1669693642)
Since I think this behavior is fairly long-running, I think I might try to recap the current logic:

Every time a compact block is reconstructed, or full block directly given, we put an entry in a map for one of two reasons:
1) reward if they're a good high-bandwidth peer candidates because they gave us a new most-PoW block
2) punish if they gave us a full block and it has any fundamental issue, or if they handed us a compact block they should have known was faulty(e.g., invalid header)

I
...

💬 MarcoFalke commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#discussion_r1287209576)
Apart from compare, assignment isn't type safe either. For example:

```cpp
uint256 b = Txid{};
Wtxid a {b};
```

compiles.

So my preference would be to add a comment that this is allowed, and remove this code that tries to imply the opposite and attempts to trick reviewers.

Alternatively it could be disallowed properly.

👍 TheCharlatan approved a pull request: "refactor: Remove unused includes from wallet.cpp"
(https://github.com/bitcoin/bitcoin/pull/28200#pullrequestreview-1567463775)
ACK fac3e2fa6adf9cb034ea5ee28c572b25cc7e3c2d

Re https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669483448

> No. The boost include is still there (via src/wallet/scriptpubkeyman.h:#include <boost/signals2/signal.hpp>

Yeah, totally. Should have checked this before trusting the iwyu suggestion.

💬 Crypt-iQ commented on pull request "p2p: ensure mapBlockSource is removed from in ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1669750122)
Yup that makes sense

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1287224406)
Could you say that in the test then? This really throws me off as a reader!

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1669753053)
ACK https://github.com/bitcoin/bitcoin/pull/28199/commits/abe8536192c9f2cd6ba9d0e083f23dec4d20841f

https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1283271742 would be nice to be slightly cleaned up for future readers

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1669788993)
thanks for the concept acks @dergoegge @jamesob @brunoerg @Empact @jonatack, would appreciate a review of test too :pray:

💬 fjahr commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1669793604)
> There hasn't been much activity lately. What is the status here?
>
> [Finding reviewers](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#finding-reviewers) may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Still relevant... How good is your ASM, @DrahtBot ? 😁

💬 fanquake commented on pull request "guix: use GCC 12.3.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1669797798)
Upstreamed a patch to (re-)add `powerpc64-linux` as a platform definition in Guix: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65151. Using a self-compiled Guix with this patch, fixes building our cross-compilation toolchain for `powerpc64-linux-gnu`, and means we'd just drop 26ee1fc5998cce6808f022fd1a4e1764115c2d57 from this PR.

Also pushed a patch to update mingw-w64 to `11.0.1`: https://lists.gnu.org/archive/html/guix-patches/2023-08/msg00299.html.