💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589789136)
Marking this as resolved as work on it continues at https://github.com/bitcoin/bitcoin/pull/33954
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589789136)
Marking this as resolved as work on it continues at https://github.com/bitcoin/bitcoin/pull/33954
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589802670)
Was separated in a different commit.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589802670)
Was separated in a different commit.
👍 instagibbs approved a pull request: "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us"
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3540970909)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3540970909)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
🚀 fanquake merged a pull request: "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us"
(https://github.com/bitcoin/bitcoin/pull/33723)
(https://github.com/bitcoin/bitcoin/pull/33723)
💬 janb84 commented on pull request "depends: Propagate native C compiler to `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3613189801)
My Guix Build Output (matching)
**Host architecture:** `aarch64`
**Commit:** `710031ebef83`
```shell
5ea5588f1e2ee4e37f4b90313a8c32ec17474a39d1dff77d9d585ae9e106c761 guix-build-710031ebef83/output/aarch64-linux-gnu/SHA256SUMS.part
8d7b95ecb5950220f6d70c069d7fdf5add92f8135daee0d0acb9af753c9bab0c guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu-debug.tar.gz
dd0f06c48c57e1243437298d02c219758ecd167c88d3a59be15a9051434a99cb guix-build-710031ebef83/
...
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3613189801)
My Guix Build Output (matching)
**Host architecture:** `aarch64`
**Commit:** `710031ebef83`
```shell
5ea5588f1e2ee4e37f4b90313a8c32ec17474a39d1dff77d9d585ae9e106c761 guix-build-710031ebef83/output/aarch64-linux-gnu/SHA256SUMS.part
8d7b95ecb5950220f6d70c069d7fdf5add92f8135daee0d0acb9af753c9bab0c guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu-debug.tar.gz
dd0f06c48c57e1243437298d02c219758ecd167c88d3a59be15a9051434a99cb guix-build-710031ebef83/
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589857633)
I have not applied that patch back then. Looks like disconnecting early would unnecessary reveal the time when the originator received back the transaction from the network. So, leaving it as it as marking this as resolved. Feel free to comment if you disagree.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589857633)
I have not applied that patch back then. Looks like disconnecting early would unnecessary reveal the time when the originator received back the transaction from the network. So, leaving it as it as marking this as resolved. Feel free to comment if you disagree.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589884418)
Marking as resolved, this code does not exist anymore.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589884418)
Marking as resolved, this code does not exist anymore.
💬 wnqqnw19 commented on issue "Batch tx broadcast RPC":
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3613286511)
The current submitpackage RPC requires callers to pre-construct a valid package in strict parent→child order.
If transactions are given in the wrong order or structure, the call fails.
This is difficult for wallets, Lightning implementations, and APIs, because they need to manually detect parent/child relationships and build the exact shape Bitcoin Core expects.
It would be much more convenient if the RPC accepted an unordered batch of transactions and Bitcoin Core automatically figured out
...
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3613286511)
The current submitpackage RPC requires callers to pre-construct a valid package in strict parent→child order.
If transactions are given in the wrong order or structure, the call fails.
This is difficult for wallets, Lightning implementations, and APIs, because they need to manually detect parent/child relationships and build the exact shape Bitcoin Core expects.
It would be much more convenient if the RPC accepted an unordered batch of transactions and Bitcoin Core automatically figured out
...
💬 fanquake commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3613305317)
Backported to `30.x` in #33997.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3613305317)
Backported to `30.x` in #33997.
👋 fanquake's pull request is ready for review: "[30.x] Backports & 30.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33997)
(https://github.com/bitcoin/bitcoin/pull/33997)
👍 ryanofsky approved a pull request: "mining: fix -blockreservedweight shadows IPC option"
(https://github.com/bitcoin/bitcoin/pull/33965#pullrequestreview-3541102064)
Code review ACK 54d3054db129d4dd5aaf9faa3f6e6922159059b0. Just some more review suggestions applied since last review. This is a pretty clean fix preventing the `-blockreservedweight` configuration value from overriding the IPC requested value.
The followup #33966 will make the code merging IPC options and configuration options more clear. And a different followup could give IPC clients ability to use the `-blockreservedweight` value instead of needing to provide their own value.
(https://github.com/bitcoin/bitcoin/pull/33965#pullrequestreview-3541102064)
Code review ACK 54d3054db129d4dd5aaf9faa3f6e6922159059b0. Just some more review suggestions applied since last review. This is a pretty clean fix preventing the `-blockreservedweight` configuration value from overriding the IPC requested value.
The followup #33966 will make the code merging IPC options and configuration options more clear. And a different followup could give IPC clients ability to use the `-blockreservedweight` value instead of needing to provide their own value.
🤔 Crypt-iQ reviewed a pull request: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-3536286958)
Looks good, left some comments. There is some non-determinism I'm looking into that I didn't see before which is a little weird. It might be completely unrelated to this PR.
(https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-3536286958)
Looks good, left some comments. There is some non-determinism I'm looking into that I didn't see before which is a little weird. It might be completely unrelated to this PR.
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586114315)
nit: I think the `it->nHeight > 0` branch is unnecessary? If `nHeight` is 0 and we are here, then that means the fork point was before the genesis block which should be impossible?
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586114315)
nit: I think the `it->nHeight > 0` branch is unnecessary? If `nHeight` is 0 and we are here, then that means the fork point was before the genesis block which should be impossible?
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586270472)
Would it be a good idea to also call `InvalidChainFound(to_connect.front())` similar to [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.cpp#L3289)?
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586270472)
Would it be a good idea to also call `InvalidChainFound(to_connect.front())` similar to [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.cpp#L3289)?
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586804372)
Should this be 2 instead? See [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.h#L1058).
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586804372)
Should this be 2 instead? See [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.h#L1058).
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2589659925)
I hacked together a patch that does this, it hasn't found anything. Feel free to use if you want:
<details>
<summary> diff </summary>
```diff
diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp
index 037d168e0e..a8c0dc98cf 100644
--- a/src/test/fuzz/block_index_tree.cpp
+++ b/src/test/fuzz/block_index_tree.cpp
@@ -49,6 +49,8 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree)
blocks.push_back(genesis);
bool abort_run{fa
...
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2589659925)
I hacked together a patch that does this, it hasn't found anything. Feel free to use if you want:
<details>
<summary> diff </summary>
```diff
diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp
index 037d168e0e..a8c0dc98cf 100644
--- a/src/test/fuzz/block_index_tree.cpp
+++ b/src/test/fuzz/block_index_tree.cpp
@@ -49,6 +49,8 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree)
blocks.push_back(genesis);
bool abort_run{fa
...
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586279490)
(Just a clarifying question)
Is this true of ABC? From my reading, this is true of ABCStep but not ABC since ABC will stop (besides failure or interrupt) only when our tip is the block with most work (see: [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.cpp#L3519)).
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2586279490)
(Just a clarifying question)
Is this true of ABC? From my reading, this is true of ABCStep but not ABC since ABC will stop (besides failure or interrupt) only when our tip is the block with most work (see: [here](https://github.com/bitcoin/bitcoin/blob/9a29b2d331eed5b4cbd6922f63e397b68ff12447/src/validation.cpp#L3519)).
📝 wnqqnw19 opened a pull request: "Wnqqnw19"
(https://github.com/bitcoin/bitcoin/pull/34007)
```
## Changes Made
1. **Renamed `format` parameter to `format_version`** (line 18)
- Avoids shadowing Python's built-in `format()` function
- Improves code clarity and follows Python best practices
2. **Removed redundant local assignment of `ADDRMAN_NEW_BUCKET_COUNT`** (line 35)
- Now uses the imported constant from `test_framework.netutil` directly
- Ensures consistency with the centralized constant definition
- If the constant changes in the future, this code will a
...
(https://github.com/bitcoin/bitcoin/pull/34007)
```
## Changes Made
1. **Renamed `format` parameter to `format_version`** (line 18)
- Avoids shadowing Python's built-in `format()` function
- Improves code clarity and follows Python best practices
2. **Removed redundant local assignment of `ADDRMAN_NEW_BUCKET_COUNT`** (line 35)
- Now uses the imported constant from `test_framework.netutil` directly
- Ensures consistency with the centralized constant definition
- If the constant changes in the future, this code will a
...
✅ fanquake closed a pull request: "Wnqqnw19"
(https://github.com/bitcoin/bitcoin/pull/34007)
(https://github.com/bitcoin/bitcoin/pull/34007)
💬 purpleKarrot commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3613330088)
> Some low-level code could benefit from being able to use `std::expected` from C++23.
Can you elaborate? How would it benefit? Over what alternative?
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3613330088)
> Some low-level code could benefit from being able to use `std::expected` from C++23.
Can you elaborate? How would it benefit? Over what alternative?