💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2568135949)
> > This change is backward-compatible, as per the old rules, such a signet challenge would always evaluate to false, rendering it unused.
>
> I don't completely agree with this, since starting an old version of bitcoind with (for example) `-signetchallenge=6a4c0901ff000000000000004c0151` would create an incompatible node. It would have a different network magic bytes and I think the `OP_RETURN` challenge would cause it to reject all blocks.
The change is backward-compatible in the sense t
...
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2568135949)
> > This change is backward-compatible, as per the old rules, such a signet challenge would always evaluate to false, rendering it unused.
>
> I don't completely agree with this, since starting an old version of bitcoind with (for example) `-signetchallenge=6a4c0901ff000000000000004c0151` would create an incompatible node. It would have a different network magic bytes and I think the `OP_RETURN` challenge would cause it to reject all blocks.
The change is backward-compatible in the sense t
...
💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2568138710)
> This isn't accurate.
I meant, if you passed the extended challenge to an old version of bitcoin it would compute different magic bytes than if you passed the same challenge to an upgraded node.
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2568138710)
> This isn't accurate.
I meant, if you passed the extended challenge to an old version of bitcoin it would compute different magic bytes than if you passed the same challenge to an upgraded node.
🤔 danielabrozzoni reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2527941633)
Concept ACK - I still haven't reviewed every commit, but this seems a nice cleanup overall.
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2527941633)
Concept ACK - I still haven't reviewed every commit, but this seems a nice cleanup overall.
💬 danielabrozzoni commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1901095831)
In b8c83c3a90452a35f63daac70d7cdf3ad0f2e280:
The commit message and release notes say that there are corner cases being fixed in `-nosignetseednode` and `-nosignetchallenge`, but I can't seem to reproduce them, as the code disallows the two options:
```
❯ ./build/src/bitcoind -nosignetchallenge
Error: Error parsing command line arguments: Negating of -signetchallenge is meaningless and therefore forbidden
...
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1901095831)
In b8c83c3a90452a35f63daac70d7cdf3ad0f2e280:
The commit message and release notes say that there are corner cases being fixed in `-nosignetseednode` and `-nosignetchallenge`, but I can't seem to reproduce them, as the code disallows the two options:
```
❯ ./build/src/bitcoind -nosignetchallenge
Error: Error parsing command line arguments: Negating of -signetchallenge is meaningless and therefore forbidden
...
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1901119937)
I verify that nodes accept blocks mined using the default signet challenge. This test includes one default signet node (2) and another node (6) configured with an extended signet challenge that uses the actual script OP_TRUE (accepts all blocks).
I added a comment with this explanation to the code.
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1901119937)
I verify that nodes accept blocks mined using the default signet challenge. This test includes one default signet node (2) and another node (6) configured with an extended signet challenge that uses the actual script OP_TRUE (accepts all blocks).
I added a comment with this explanation to the code.
💬 achow101 commented on pull request "[28.x] 28.1rc3 backports":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2568212159)
Are these significant enough to warrant a rc3 or can we go straight to final?
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2568212159)
Are these significant enough to warrant a rc3 or can we go straight to final?
✅ maflcko closed an issue: "Single-Glyph Bitcoin Transaction System"
(https://github.com/bitcoin/bitcoin/issues/31568)
(https://github.com/bitcoin/bitcoin/issues/31568)
💬 maflcko commented on issue "Single-Glyph Bitcoin Transaction System":
(https://github.com/bitcoin/bitcoin/issues/31568#issuecomment-2568233702)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.c
...
(https://github.com/bitcoin/bitcoin/issues/31568#issuecomment-2568233702)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.c
...
💬 maflcko commented on issue "failure in wallet_sendall.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/31571#issuecomment-2568254664)
Did you run any other command while running the tests?
Does it reproduce?
I can't reproduce with your steps on Ubuntu 22.04: `cmake -B ./bld-cmake -DBUILD_GUI=OFF -DWITH_BDB=OFF -DBUILD_BENCH=OFF -DBUILD_FOR_FUZZING=OFF -DBUILD_KERNEL_LIB=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DBUILD_WALLET_TOOL=OFF && cmake --build ./bld-cmake --parallel $(nproc) && ./bld-cmake/test/functional/test_runner.py`
(https://github.com/bitcoin/bitcoin/issues/31571#issuecomment-2568254664)
Did you run any other command while running the tests?
Does it reproduce?
I can't reproduce with your steps on Ubuntu 22.04: `cmake -B ./bld-cmake -DBUILD_GUI=OFF -DWITH_BDB=OFF -DBUILD_BENCH=OFF -DBUILD_FOR_FUZZING=OFF -DBUILD_KERNEL_LIB=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DBUILD_WALLET_TOOL=OFF && cmake --build ./bld-cmake --parallel $(nproc) && ./bld-cmake/test/functional/test_runner.py`
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901189022)
It does not. I'll remove it
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901189022)
It does not. I'll remove it
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901192156)
I've cached `m_outbounds_count` to prevent this from happening.
This made me realize something that `fanout_targets` was incorrectly initialized, instead of reserving space for `inbounds_target_size + outbounds_target_size)`, it was being initialized with `inbounds_target_size + outbounds_target_size)` pre-existing elements initialized to 0, meaning that the returned vector contained more elements that expected
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901192156)
I've cached `m_outbounds_count` to prevent this from happening.
This made me realize something that `fanout_targets` was incorrectly initialized, instead of reserving space for `inbounds_target_size + outbounds_target_size)`, it was being initialized with `inbounds_target_size + outbounds_target_size)` pre-existing elements initialized to 0, meaning that the returned vector contained more elements that expected
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901195650)
Addressed in 804ccf030362fe5b232fe5c4213b77899a425f2b
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901195650)
Addressed in 804ccf030362fe5b232fe5c4213b77899a425f2b
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901195715)
Addressed in 804ccf030362fe5b232fe5c4213b77899a425f2b
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901195715)
Addressed in 804ccf030362fe5b232fe5c4213b77899a425f2b
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901208893)
This is already the case. The transaction that is passed to `RelayTransaction` is added to `invs_to_send` using `wtxid/txid` based on `m_wtxid_relay` (that is part of the code you quoted already). For other transactions that may be in `inv_to_send` (e.g. ancestors and/or descendants), this only apply to Erlay, and Erlay enforces `WTXID_RELAY`
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901208893)
This is already the case. The transaction that is passed to `RelayTransaction` is added to `invs_to_send` using `wtxid/txid` based on `m_wtxid_relay` (that is part of the code you quoted already). For other transactions that may be in `inv_to_send` (e.g. ancestors and/or descendants), this only apply to Erlay, and Erlay enforces `WTXID_RELAY`
💬 edilmedeiros commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2568291604)
After many hours, I could make it work.
Add this to ~/.lldbinit to make it load a local .lldbinit file: `settings set target.load-cwd-lldbinit true`. This is a convenience so that I can keep everything related to Bitcoin Core in its repo.
In the Bitcoin Core repo folder, add a .lldbinit file with (mine is located at `/Users/jose.edil/2-development/bitcoin/bitcoin-core-dev`, adjust to your path):
```
settings set target.source-map /Users/jose.edil/2-development/bitcoin/bitcoin-core-dev/bu
...
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2568291604)
After many hours, I could make it work.
Add this to ~/.lldbinit to make it load a local .lldbinit file: `settings set target.load-cwd-lldbinit true`. This is a convenience so that I can keep everything related to Bitcoin Core in its repo.
In the Bitcoin Core repo folder, add a .lldbinit file with (mine is located at `/Users/jose.edil/2-development/bitcoin/bitcoin-core-dev`, adjust to your path):
```
settings set target.source-map /Users/jose.edil/2-development/bitcoin/bitcoin-core-dev/bu
...
💬 maflcko commented on pull request "[28.x] 28.1 backports (final?)":
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2568299441)
> Are these significant enough to warrant a rc3 or can we go straight to final?
Yes, seems fine in this special case, as the depends patch shouldn't affect the release bin and the other change is test-only. I went ahead and picked your commits from https://github.com/bitcoin/bitcoin/pull/31582
(https://github.com/bitcoin/bitcoin/pull/31594#issuecomment-2568299441)
> Are these significant enough to warrant a rc3 or can we go straight to final?
Yes, seems fine in this special case, as the depends patch shouldn't affect the release bin and the other change is test-only. I went ahead and picked your commits from https://github.com/bitcoin/bitcoin/pull/31582
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901215478)
The rationale for who this method is used is provided in `RelayTransaction` when the method is used
https://github.com/bitcoin/bitcoin/blob/804ccf030362fe5b232fe5c4213b77899a425f2b/src/net_processing.cpp#L2154-L2159
I think the docs give enough rationale as to what this is intended for without being excessively narrow as to what the use cases could be.
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1901215478)
The rationale for who this method is used is provided in `RelayTransaction` when the method is used
https://github.com/bitcoin/bitcoin/blob/804ccf030362fe5b232fe5c4213b77899a425f2b/src/net_processing.cpp#L2154-L2159
I think the docs give enough rationale as to what this is intended for without being excessively narrow as to what the use cases could be.
💬 brunoerg commented on issue "Fuzz: Runtime errors when running fuzz tests on MacOs":
(https://github.com/bitcoin/bitcoin/issues/31591#issuecomment-2568305291)
I have the same issue and I think other people also have reported it before, I simply ignore it.
(https://github.com/bitcoin/bitcoin/issues/31591#issuecomment-2568305291)
I have the same issue and I think other people also have reported it before, I simply ignore it.
💬 marcofleon commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2568305724)
> I am unable to crash the fuzz test when `setmocktime()` is removed from one of the targets.
Which target was it?
It could be that running the test from scratch doesn't quickly reach code where `now()` is called. Try running the target on a corpus of inputs:
```
FUZZ=load_external_block_file ./mocktimefuzzbuild/src/test/fuzz/fuzz ../qa-assets/fuzz_corpora/load_external_block_file -runs=1
```
You can find the corpora here: https://github.com/bitcoin-core/qa-assets
I think it's also
...
(https://github.com/bitcoin/bitcoin/pull/31549#issuecomment-2568305724)
> I am unable to crash the fuzz test when `setmocktime()` is removed from one of the targets.
Which target was it?
It could be that running the test from scratch doesn't quickly reach code where `now()` is called. Try running the target on a corpus of inputs:
```
FUZZ=load_external_block_file ./mocktimefuzzbuild/src/test/fuzz/fuzz ../qa-assets/fuzz_corpora/load_external_block_file -runs=1
```
You can find the corpora here: https://github.com/bitcoin-core/qa-assets
I think it's also
...
💬 marcofleon commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#discussion_r1901223667)
Thanks, yeah that makes more sense. This actually caught system time usage in `p2p_headers_presync` that wasn't crashing before. Because the `mocktime.count()` check is now in the function itself rather than at the end of the fuzz iteration.
Using that syntax didn't compile for me, but I believe what I just pushed should work.
(https://github.com/bitcoin/bitcoin/pull/31549#discussion_r1901223667)
Thanks, yeah that makes more sense. This actually caught system time usage in `p2p_headers_presync` that wasn't crashing before. Because the `mocktime.count()` check is now in the function itself rather than at the end of the fuzz iteration.
Using that syntax didn't compile for me, but I believe what I just pushed should work.