💬 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
...
📝 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
...
(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
(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
(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
(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