💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1669412860)
> Not sure how all of you can be so comfortable with this kind of distortion of status quo, particularly when it serves no one but Peter Todd, some misguided wannabe Core Wizards, and whichever miners Peter can persuade to run code they dont really care about anyway.
FYI excluding Luxor, I haven't had any significant communications with any mining pools. I of course attempted to contact every mining pool I could find contact details for. But other than two I didn't get any responses back, and
...
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1669412860)
> Not sure how all of you can be so comfortable with this kind of distortion of status quo, particularly when it serves no one but Peter Todd, some misguided wannabe Core Wizards, and whichever miners Peter can persuade to run code they dont really care about anyway.
FYI excluding Luxor, I haven't had any significant communications with any mining pools. I of course attempted to contact every mining pool I could find contact details for. But other than two I didn't get any responses back, and
...
👍 MarcoFalke approved a pull request: "crypto: more `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1535733421)
nice ACK 153a6745fa523a3fd0ccce1304ceb75d4511df20 🔖
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK 153a6745fa523a3f
...
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1535733421)
nice ACK 153a6745fa523a3fd0ccce1304ceb75d4511df20 🔖
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK 153a6745fa523a3f
...
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267199359)
nit in 9e1199c7ce5d9a29c6ddf3dc28d8814bb1190a64: Can replace `std::byte{}` with `{}`, if it compiles, same below?
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267199359)
nit in 9e1199c7ce5d9a29c6ddf3dc28d8814bb1190a64: Can replace `std::byte{}` with `{}`, if it compiles, same below?
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267206399)
9e1199c7ce5d9a29c6ddf3dc28d8814bb1190a64: Could replace this whole function with:
```cpp
fillrand(MakeWritableByteSpan(ret));
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267206399)
9e1199c7ce5d9a29c6ddf3dc28d8814bb1190a64: Could replace this whole function with:
```cpp
fillrand(MakeWritableByteSpan(ret));
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267215003)
same
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267215003)
same
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267214464)
Any reason to do this? Fuzz inputs may already invalidate for any code change and maintaining compat for some code changes seems inconsistent, will bloat the fuzz code, and cause maintenance overhead in the future.
Would be good to either remove this, or if you *actually* want to maintain compat use a ternary operator:
```cpp
const auto key{fdp.ConsumeBool()?ConsumeFixedLen(...):std::vec<std::byte>(32)};
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1267214464)
Any reason to do this? Fuzz inputs may already invalidate for any code change and maintaining compat for some code changes seems inconsistent, will bloat the fuzz code, and cause maintenance overhead in the future.
Would be good to either remove this, or if you *actually* want to maintain compat use a ternary operator:
```cpp
const auto key{fdp.ConsumeBool()?ConsumeFixedLen(...):std::vec<std::byte>(32)};
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286960165)
2ff930de1c7f4683d0a7a72970683e21bf20ba73: SetKey. (no 32)
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286960165)
2ff930de1c7f4683d0a7a72970683e21bf20ba73: SetKey. (no 32)
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286968172)
nit in 2ff930de1c7f4683d0a7a72970683e21bf20ba73: Could use `{}` over `std::byte{}`, if it compiles?
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286968172)
nit in 2ff930de1c7f4683d0a7a72970683e21bf20ba73: Could use `{}` over `std::byte{}`, if it compiles?
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286961781)
2ff930de1c7f4683d0a7a72970683e21bf20ba73: My preference would be to instead base this on a new commit that just does a `input` -> `m_input` ("scripted-diff") rename
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286961781)
2ff930de1c7f4683d0a7a72970683e21bf20ba73: My preference would be to instead base this on a new commit that just does a `input` -> `m_input` ("scripted-diff") rename
💬 MarcoFalke commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286971030)
unrelated: In C++20, we'll probably have to provide overloads for `operator==(std::span, std::span)`, because equality checks are only defined in the std lib for containers that store memory, it seems.
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1286971030)
unrelated: In C++20, we'll probably have to provide overloads for `operator==(std::span, std::span)`, because equality checks are only defined in the std lib for containers that store memory, it seems.
✅ hebasto closed a pull request: "Avoid lock order inversion in `Chainstate::ConnectTip` function"
(https://github.com/bitcoin/bitcoin/pull/27684)
(https://github.com/bitcoin/bitcoin/pull/27684)
💬 TheCharlatan commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669476588)
Re https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1668087768
> Can you post a patch that compiles?
Seems like an easy win, no?
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7c74b6b9ff..fe4c3d0f67 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -79,6 +79,8 @@
#include <tuple>
#include <variant>
+#include <boost/signals2/signal.hpp>
+
struct KeyOriginInfo;
using interfaces::FoundBlock;
diff --git a/src/wallet/w
...
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669476588)
Re https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1668087768
> Can you post a patch that compiles?
Seems like an easy win, no?
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7c74b6b9ff..fe4c3d0f67 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -79,6 +79,8 @@
#include <tuple>
#include <variant>
+#include <boost/signals2/signal.hpp>
+
struct KeyOriginInfo;
using interfaces::FoundBlock;
diff --git a/src/wallet/w
...
💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669483448)
> Seems like an easy win, no?
No. The boost include is still there (via `src/wallet/scriptpubkeyman.h:#include <boost/signals2/signal.hpp>`
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1669483448)
> Seems like an easy win, no?
No. The boost include is still there (via `src/wallet/scriptpubkeyman.h:#include <boost/signals2/signal.hpp>`
💬 ChrisCho-H commented on pull request "fix: bad opcode err msg includes reserved opcode":
(https://github.com/bitcoin/bitcoin/pull/28234#issuecomment-1669509083)
Thx, I updated in same commit.
(https://github.com/bitcoin/bitcoin/pull/28234#issuecomment-1669509083)
Thx, I updated in same commit.
💬 Daniel600 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1669514712)
For accuracy purposes GAP600 supports a range of clients who use 0-conf for their end users in a range of applications - including gaming/gambling, instant crypto swops, and Forex. All industries which take their privacy very seriously. GAP600 has provided email quotes and root addresses (here and in the mailing list) in order to validate this claim for this signifiant market.
You have accused GAP600 and myself of outright lying, based on your perceived understanding of how GAP600 should run
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1669514712)
For accuracy purposes GAP600 supports a range of clients who use 0-conf for their end users in a range of applications - including gaming/gambling, instant crypto swops, and Forex. All industries which take their privacy very seriously. GAP600 has provided email quotes and root addresses (here and in the mailing list) in order to validate this claim for this signifiant market.
You have accused GAP600 and myself of outright lying, based on your perceived understanding of how GAP600 should run
...
💬 Crypt-iQ commented on pull request "p2p: ensure mapBlockSource is removed from in ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1669520073)
I guess I forgot to run the functional tests - the issue is that this patch removes from `mapBlockSource` in `ProcessBlock` if `new_block` is set. I assumed that this meant it was a valid block, but that's not always the case. With this patch, if we receive more blocks on top of this invalid block that give us more work than the tip, the `BlockChecked` callback will run and won't find the peer to punish. I can't think of a scenario where you're sure that you can remove from `mapBlockSource` unle
...
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1669520073)
I guess I forgot to run the functional tests - the issue is that this patch removes from `mapBlockSource` in `ProcessBlock` if `new_block` is set. I assumed that this meant it was a valid block, but that's not always the case. With this patch, if we receive more blocks on top of this invalid block that give us more work than the tip, the `BlockChecked` callback will run and won't find the peer to punish. I can't think of a scenario where you're sure that you can remove from `mapBlockSource` unle
...
💬 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
(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.
(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
(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
...
(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
...