👍 brunoerg approved a pull request: "test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py"
(https://github.com/bitcoin/bitcoin/pull/33782#pullrequestreview-3422561866)
code review ACK ec8516ceb7568d7b09836b830023978bd37f8462
(https://github.com/bitcoin/bitcoin/pull/33782#pullrequestreview-3422561866)
code review ACK ec8516ceb7568d7b09836b830023978bd37f8462
💬 alexanderwiederin commented on pull request "kernel: Use enumeration type for flags argument":
(https://github.com/bitcoin/bitcoin/pull/33791#issuecomment-3491703847)
ACK https://github.com/bitcoin/bitcoin/pull/33791/commits/ed5720509f03ddd7f9158b9adf0d8fd7f56c8578
(https://github.com/bitcoin/bitcoin/pull/33791#issuecomment-3491703847)
ACK https://github.com/bitcoin/bitcoin/pull/33791/commits/ed5720509f03ddd7f9158b9adf0d8fd7f56c8578
👍 stickies-v approved a pull request: "kernel: Use enumeration type for flags argument"
(https://github.com/bitcoin/bitcoin/pull/33791#pullrequestreview-3422655653)
ACK ed5720509f03ddd7f9158b9adf0d8fd7f56c8578
(https://github.com/bitcoin/bitcoin/pull/33791#pullrequestreview-3422655653)
ACK ed5720509f03ddd7f9158b9adf0d8fd7f56c8578
🤔 pablomartin4btc reviewed a pull request: "doc: add cmake help option in Windows build docs"
(https://github.com/bitcoin/bitcoin/pull/33789#pullrequestreview-3422616007)
ACK 9577daa3b88a8fbe8c96d2848adcc88c55c55a29
_(please try to avoid mentions/ @-prefixed github names in PR descriptions as it can cause potential issues, primarily related to notification spam)_
(https://github.com/bitcoin/bitcoin/pull/33789#pullrequestreview-3422616007)
ACK 9577daa3b88a8fbe8c96d2848adcc88c55c55a29
_(please try to avoid mentions/ @-prefixed github names in PR descriptions as it can cause potential issues, primarily related to notification spam)_
💬 pablomartin4btc commented on pull request "doc: add cmake help option in Windows build docs":
(https://github.com/bitcoin/bitcoin/pull/33789#discussion_r2494905335)
nit:
```suggestion
cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
(run `cmake -B build -LH` to see the full list of available options)
```
(https://github.com/bitcoin/bitcoin/pull/33789#discussion_r2494905335)
nit:
```suggestion
cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
(run `cmake -B build -LH` to see the full list of available options)
```
📝 vasild opened a pull request: "net_processing: reorder the code that handles the VERSION message"
(https://github.com/bitcoin/bitcoin/pull/33792)
Change the order in which code snippets are executed as a result of receiving the `VERSION` message. Move the snippets that do `MakeAndPushMessage()` near the end. This makes it easier to interrupt the execution when no messages should be sent as a response to the `VERSION` messages, in private broadcast connections.
This is a non-functional change.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull
...
(https://github.com/bitcoin/bitcoin/pull/33792)
Change the order in which code snippets are executed as a result of receiving the `VERSION` message. Move the snippets that do `MakeAndPushMessage()` near the end. This makes it easier to interrupt the execution when no messages should be sent as a response to the `VERSION` messages, in private broadcast connections.
This is a non-functional change.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull
...
💬 vasild commented on pull request "net_processing: reorder the code that handles the VERSION message":
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3491723472)
This is just moving code around. Easier reviewed with `~/.gitconfig` like:
```gitconfig
[diff]
colorMoved = dimmed-zebra
colorMovedWS = allow-indentation-change
```
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3491723472)
This is just moving code around. Easier reviewed with `~/.gitconfig` like:
```gitconfig
[diff]
colorMoved = dimmed-zebra
colorMovedWS = allow-indentation-change
```
🚀 fanquake merged a pull request: "kernel: Use enumeration type for flags argument"
(https://github.com/bitcoin/bitcoin/pull/33791)
(https://github.com/bitcoin/bitcoin/pull/33791)
💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3491797678)
Windows is non-deterministic across x86_64 && aarch64.
Guix build (aarch64):
```bash
6c31a0f19daec94c3d629e50d6bc2461ceb6e0fbb0823cb10a40bc735e37f9b5 guix-build-25ede968bada/output/aarch64-linux-gnu/SHA256SUMS.part
f3ba2f0c5f1d99d377ea11ea2b9983187192d733c7db444060d112d1be48ecd2 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ede968bada-aarch64-linux-gnu-debug.tar.gz
321db43f3a4327287813274b7f9874fb76799643fdc9cbbc9ee13bce10b6e004 guix-build-25ede968bada/output/aarch64-linux-
...
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3491797678)
Windows is non-deterministic across x86_64 && aarch64.
Guix build (aarch64):
```bash
6c31a0f19daec94c3d629e50d6bc2461ceb6e0fbb0823cb10a40bc735e37f9b5 guix-build-25ede968bada/output/aarch64-linux-gnu/SHA256SUMS.part
f3ba2f0c5f1d99d377ea11ea2b9983187192d733c7db444060d112d1be48ecd2 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ede968bada-aarch64-linux-gnu-debug.tar.gz
321db43f3a4327287813274b7f9874fb76799643fdc9cbbc9ee13bce10b6e004 guix-build-25ede968bada/output/aarch64-linux-
...
📝 vasild opened a pull request: "test: move create_malleated_version() to messages.py for reuse"
(https://github.com/bitcoin/bitcoin/pull/33793)
Move `create_malleated_version()` from `p2p_orphan_handling.py` to `test_framework/messages.py` so that it can be reused by other tests.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.
(https://github.com/bitcoin/bitcoin/pull/33793)
Move `create_malleated_version()` from `p2p_orphan_handling.py` to `test_framework/messages.py` so that it can be reused by other tests.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3491839226)
Extracted two more commits into separate smaller PRs for easier review:
https://github.com/bitcoin/bitcoin/pull/33792
https://github.com/bitcoin/bitcoin/pull/33793
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3491839226)
Extracted two more commits into separate smaller PRs for easier review:
https://github.com/bitcoin/bitcoin/pull/33792
https://github.com/bitcoin/bitcoin/pull/33793
👍 theStack approved a pull request: "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found"
(https://github.com/bitcoin/bitcoin/pull/33433#pullrequestreview-3422901949)
Tested ACK 79b4c276e7b9b526fa8f563b1e09b2b970baece6
Verified also in a local branch rebased on master, given that the merge-base is extremely old (>7 years).
(https://github.com/bitcoin/bitcoin/pull/33433#pullrequestreview-3422901949)
Tested ACK 79b4c276e7b9b526fa8f563b1e09b2b970baece6
Verified also in a local branch rebased on master, given that the merge-base is extremely old (>7 years).
💬 laisial commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3492055411)
> DNS seeds still pose **a small amount of risk** for the network. As such, DNS seeds must be **run by entities which have some minimum level of trust within the Bitcoin community**. (DNS policy)
this discussion is inherently personal with the question being how much 'some minimum level of trust' is.
> Luke: as for trust within the Bitcoin community, generally it seems **I am much more trusted than some of the other DNS seed operators**. Indeed, it probably makes sense to intentionally continu
...
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3492055411)
> DNS seeds still pose **a small amount of risk** for the network. As such, DNS seeds must be **run by entities which have some minimum level of trust within the Bitcoin community**. (DNS policy)
this discussion is inherently personal with the question being how much 'some minimum level of trust' is.
> Luke: as for trust within the Bitcoin community, generally it seems **I am much more trusted than some of the other DNS seed operators**. Indeed, it probably makes sense to intentionally continu
...
👍 stickies-v approved a pull request: "util: Allow Assert() in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3423080904)
ACK fae1d99651e29341e486a10e6340335c71a2144e
Note that this also introduces behaviour change by including the entire function signature instead of just the name. Might be worth mentioning in OP.
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3423080904)
ACK fae1d99651e29341e486a10e6340335c71a2144e
Note that this also introduces behaviour change by including the entire function signature instead of just the name. Might be worth mentioning in OP.
💬 maflcko commented on pull request "test: move create_malleated_version() to messages.py for reuse":
(https://github.com/bitcoin/bitcoin/pull/33793#issuecomment-3492252019)
review ACK 2bd155e6ee7e3cabd76083ac921b34bb45d98769 🍨
<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: review ACK 2bd155e6ee7e
...
(https://github.com/bitcoin/bitcoin/pull/33793#issuecomment-3492252019)
review ACK 2bd155e6ee7e3cabd76083ac921b34bb45d98769 🍨
<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: review ACK 2bd155e6ee7e
...
👍 brunoerg approved a pull request: "test: move create_malleated_version() to messages.py for reuse"
(https://github.com/bitcoin/bitcoin/pull/33793#pullrequestreview-3423238890)
ACK 2bd155e6ee7e3cabd76083ac921b34bb45d98769
(https://github.com/bitcoin/bitcoin/pull/33793#pullrequestreview-3423238890)
ACK 2bd155e6ee7e3cabd76083ac921b34bb45d98769
💬 stringintech commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2495351885)
Related to your discussion:
The cleanup check on line 466 assumes invalid chains on disk always start with `BLOCK_FAILED_VALID`, followed by descendants with `BLOCK_FAILED_CHILD`:
```cpp
if (!(pindex->nStatus & BLOCK_FAILED_VALID) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_VALID))
```
However, if we load a chain where the first invalid block is `BLOCK_FAILED_CHILD` (with a valid parent), none of the blocks in that chain would be cleaned up.
I'm not sure if this scen
...
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2495351885)
Related to your discussion:
The cleanup check on line 466 assumes invalid chains on disk always start with `BLOCK_FAILED_VALID`, followed by descendants with `BLOCK_FAILED_CHILD`:
```cpp
if (!(pindex->nStatus & BLOCK_FAILED_VALID) && pindex->pprev && (pindex->pprev->nStatus & BLOCK_FAILED_VALID))
```
However, if we load a chain where the first invalid block is `BLOCK_FAILED_CHILD` (with a valid parent), none of the blocks in that chain would be cleaned up.
I'm not sure if this scen
...
💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2495460002)
This is fixed now
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2495460002)
This is fixed now
💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2495465245)
This is not possible I think because we need to pass a non const block further down.
I instead avoid the second copy by moving.
Let me know what you think
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2495465245)
This is not possible I think because we need to pass a non const block further down.
I instead avoid the second copy by moving.
Let me know what you think
💬 hodlinator commented on pull request "refactor: avoid double hashing in `SourceLocationHasher`":
(https://github.com/bitcoin/bitcoin/pull/32939#issuecomment-3492515824)
> Since there are quite a few follow-ups in #32604, it might make sense to do them all in one (or at least larger, related PRs) instead of carving it out into dozens of small ones?
Just noting that this was done in #33011, in case anyone else also wonders.
(https://github.com/bitcoin/bitcoin/pull/32939#issuecomment-3492515824)
> Since there are quite a few follow-ups in #32604, it might make sense to do them all in one (or at least larger, related PRs) instead of carving it out into dozens of small ones?
Just noting that this was done in #33011, in case anyone else also wonders.