π¬ TheCharlatan commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554084325)
We should include post-condition checks here. For the `TestBlockValidity` refactor, we added
```
if (state.IsValid()) NONFATAL_UNREACHABLE();
```
I think such a check should be added wherever a boolean was returned previously.
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554084325)
We should include post-condition checks here. For the `TestBlockValidity` refactor, we added
```
if (state.IsValid()) NONFATAL_UNREACHABLE();
```
I think such a check should be added wherever a boolean was returned previously.
π¬ TheCharlatan commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554092568)
I don't think this change is correct. Previously false was returned when the block failed to be accepted or on a fatal error condition, not when it failed to validate. If you look into `ActivateBestChainStep`, you'll see that we break on a validation failure, reset the state again and don't return false. Can you drop this change again?
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554092568)
I don't think this change is correct. Previously false was returned when the block failed to be accepted or on a fatal error condition, not when it failed to validate. If you look into `ActivateBestChainStep`, you'll see that we break on a validation failure, reset the state again and don't return false. Can you drop this change again?
π¬ hebasto commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3568067474)
> This introduces two behaviour changes:
>
> 1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF` will now be respected by our build system, instead of silently overriding it like before
Concept ACK on that.
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3568067474)
> This introduces two behaviour changes:
>
> 1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF` will now be respected by our build system, instead of silently overriding it like before
Concept ACK on that.
π¬ sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3568091456)
Rebased on top of #33629.
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3568091456)
Rebased on top of #33629.
π andrewtoth's pull request is ready for review: "validation: fetch block inputs on parallel threads >20% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132)
(https://github.com/bitcoin/bitcoin/pull/31132)
π¬ andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3568164009)
I've updated the PR to make the `InputFetcher` a subclass of `CCoinsViewCache`. Instead of waiting on the MPSC queue to be finished before connecting the block, the queue can be processed during `CCoinsViewCache::FetchCoin` during `ConnectBlock`. This makes the fetching completely non-blocking, which is a significant performance improvement. It's been renamed to `CoinsViewCacheAsync`.
@l0rinc thank you for your very thorough benchmarks. I've updated this to use 4 worker threads, which yields
...
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3568164009)
I've updated the PR to make the `InputFetcher` a subclass of `CCoinsViewCache`. Instead of waiting on the MPSC queue to be finished before connecting the block, the queue can be processed during `CCoinsViewCache::FetchCoin` during `ConnectBlock`. This makes the fetching completely non-blocking, which is a significant performance improvement. It's been renamed to `CoinsViewCacheAsync`.
@l0rinc thank you for your very thorough benchmarks. I've updated this to use 4 worker threads, which yields
...
π¬ 151henry151 commented on pull request "Defer transaction signing until user clicks Send":
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3568201828)
It looks like the one failing check is a CI issue with disk space too low. I'm not sure how to address that. I think the code of the PR is correct.
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3568201828)
It looks like the one failing check is a CI issue with disk space too low. I'm not sure how to address that. I think the code of the PR is correct.
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568224698)
I updated the tests to match the new policy. The NONSTANDARD script in p2p_segwit.py now passes AreInputsStandard since it has zero sigops, so I kept it as-is. For test_segwit_versions, I switched the input from the NONSTANDARD UTXO to the standard UTXOs created earlier, so the transaction now passes all checks and gets accepted. For feature_taproot.py, I adjusted the is_standard flag for the compat/nocsa spender because bare NONSTANDARD scripts with low sigop counts now pass policy checks.
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568224698)
I updated the tests to match the new policy. The NONSTANDARD script in p2p_segwit.py now passes AreInputsStandard since it has zero sigops, so I kept it as-is. For test_segwit_versions, I switched the input from the NONSTANDARD UTXO to the standard UTXOs created earlier, so the transaction now passes all checks and gets accepted. For feature_taproot.py, I adjusted the is_standard flag for the compat/nocsa spender because bare NONSTANDARD scripts with low sigop counts now pass policy checks.
...
π¬ instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3568292879)
reACK 17cf9ff7efdbab07644fc2f9017fcac1b0757c38
Just the `POST_CHANGE_WORK` change. I ran with the changes manually a bit and logged when things were non-optimal as a sanity check (it never happened). As a follow-up, may be nice to have non-optimality logged in some sensible way for diagnostics.
`git range-diff master 3fb3c230c93e39706b813f67006ae86c9e34e803 17cf9ff7efdbab07644fc2f9017fcac1b0757c38`
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3568292879)
reACK 17cf9ff7efdbab07644fc2f9017fcac1b0757c38
Just the `POST_CHANGE_WORK` change. I ran with the changes manually a bit and logged when things were non-optimal as a sanity check (it never happened). As a follow-up, may be nice to have non-optimality logged in some sensible way for diagnostics.
`git range-diff master 3fb3c230c93e39706b813f67006ae86c9e34e803 17cf9ff7efdbab07644fc2f9017fcac1b0757c38`
π€ furszy reviewed a pull request: "index: Deduplicate HashKey / HeightKey handling"
(https://github.com/bitcoin/bitcoin/pull/32997#pullrequestreview-3498048964)
what do you think about wrapping all the code in `db_key.h` under a namespace like "util", "common", "index::util", or "util::index" (just threw some names), so it's clear when reading the code that these functions are defined elsewhere and shared, without needing to search around for the definition.
(https://github.com/bitcoin/bitcoin/pull/32997#pullrequestreview-3498048964)
what do you think about wrapping all the code in `db_key.h` under a namespace like "util", "common", "index::util", or "util::index" (just threw some names), so it's clear when reading the code that these functions are defined elsewhere and shared, without needing to search around for the definition.
π hebasto opened a pull request: "test: Remove `system_tests/run_command` runtime dependencies"
(https://github.com/bitcoin/bitcoin/pull/33929)
`system_tests` currently rely on the presence of `cat`, `echo`, `false` and `sh` in `PATH` at runtime.
This PR:
1. Removes these dependencies.
2. Eliminates all but one platform-specific code paths.
(https://github.com/bitcoin/bitcoin/pull/33929)
`system_tests` currently rely on the presence of `cat`, `echo`, `false` and `sh` in `PATH` at runtime.
This PR:
1. Removes these dependencies.
2. Eliminates all but one platform-specific code paths.
β οΈ taoteh1221 opened an issue: "Documentation on 'services' parsing in RPC response for getnodeaddresses"
(https://github.com/bitcoin/bitcoin/issues/33930)
### Please describe the feature you'd like to see added.
I can't find any docs on how you parse `services` results for `getnodeaddresses` RPC data requests:
https://bitcoincore.org/en/doc/30.0.0/rpc/network/getnodeaddresses/
I'm hoping to have a more user friendly interface, other than the raw services number for a geolocation search filter:
<img width="1836" height="1206" alt="Image" src="https://github.com/user-attachments/assets/4b845c48-10b4-4098-86c2-fcf22ce49cff" />
### Is your featu
...
(https://github.com/bitcoin/bitcoin/issues/33930)
### Please describe the feature you'd like to see added.
I can't find any docs on how you parse `services` results for `getnodeaddresses` RPC data requests:
https://bitcoincore.org/en/doc/30.0.0/rpc/network/getnodeaddresses/
I'm hoping to have a more user friendly interface, other than the raw services number for a geolocation search filter:
<img width="1836" height="1206" alt="Image" src="https://github.com/user-attachments/assets/4b845c48-10b4-4098-86c2-fcf22ce49cff" />
### Is your featu
...
π¬ pinheadmz commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568571196)
It's a bitfield, defined here:
https://github.com/bitcoin/bitcoin/blob/0690514d4f72aac251ee0b876cded9187d42c63e/src/protocol.h#L308-L339
You're right that it's not super well documented but you can find some information:
https://en.bitcoin.it/wiki/Protocol_documentation#version
https://bitcoin.stackexchange.com/search?q=service+bits
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568571196)
It's a bitfield, defined here:
https://github.com/bitcoin/bitcoin/blob/0690514d4f72aac251ee0b876cded9187d42c63e/src/protocol.h#L308-L339
You're right that it's not super well documented but you can find some information:
https://en.bitcoin.it/wiki/Protocol_documentation#version
https://bitcoin.stackexchange.com/search?q=service+bits
π¬ taoteh1221 commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568602551)
Thanks for pointing me in the right direction @pinheadmz. π Googling didn't get me too far.
I'm looking to filter mining, ln, etc (if possible), in the geolocation GUI results filters. Hopefully this is possible. Will deep dive your source file and 3rd party links this week, thanks very much.
I can close this issue, unless you all think it's feasible enough to keep open? I'm sure their are a ton of higher priorities, I just came up nearly blank on google, so figured I'd ping it here. Thanks
...
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568602551)
Thanks for pointing me in the right direction @pinheadmz. π Googling didn't get me too far.
I'm looking to filter mining, ln, etc (if possible), in the geolocation GUI results filters. Hopefully this is possible. Will deep dive your source file and 3rd party links this week, thanks very much.
I can close this issue, unless you all think it's feasible enough to keep open? I'm sure their are a ton of higher priorities, I just came up nearly blank on google, so figured I'd ping it here. Thanks
...
π¬ taoteh1221 commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568609429)
I see the source seems to suggest only P2P-related stuff is flagged into the `services` value, if I'm reading it right.
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568609429)
I see the source seems to suggest only P2P-related stuff is flagged into the `services` value, if I'm reading it right.
π¬ Sakzz1994 commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#issuecomment-3568621143)
π
(https://github.com/bitcoin/bitcoin/pull/32543#issuecomment-3568621143)
π
π¬ taoteh1221 commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568621814)
Seems I may need to look at other endpoints for the data I seek. Was hoping to keep overhead low with a single call to `getnodeaddresses`, as this will be run on low power devices on home / internal networks often...but doesn't look like I can do that.
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568621814)
Seems I may need to look at other endpoints for the data I seek. Was hoping to keep overhead low with a single call to `getnodeaddresses`, as this will be run on low power devices on home / internal networks often...but doesn't look like I can do that.
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568629509)
The fuzz tests were failing because we called `GetSigOpCount(true)` on potentially malformed NONSTANDARD scripts. When fuzzing feeds invalid opcodes, `DecodeOP_N` can hit an assert. I added a `HasValidOps()` check before calling `GetSigOpCount` so we only process well-formed scripts, which fixes the fuzz failures.
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568629509)
The fuzz tests were failing because we called `GetSigOpCount(true)` on potentially malformed NONSTANDARD scripts. When fuzzing feeds invalid opcodes, `DecodeOP_N` can hit an assert. I added a `HasValidOps()` check before calling `GetSigOpCount` so we only process well-formed scripts, which fixes the fuzz failures.
β οΈ dachengcheng2022 opened an issue: "how to syn testnet4 data"
(https://github.com/bitcoin/bitcoin/issues/33931)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Bitcoin Core daemon version v30.0.0
config is
`# Listen for RPC connections on this TCP port:
testnet=1
server=1
txindex=1
[test] # β εΏ ι‘»εθΏδΈͺ section
wallet=testwallet
# Allow internal networks to communicate to the RPC endpoint
rpcbind=0.0.0.0
rpcallowip=0.0.0.0/0
rpcport=18332
rpcuser=test
rpcpassword=test
fallbackfee=0.0001`
syn is testnet3 data now
### Expected behavio
...
(https://github.com/bitcoin/bitcoin/issues/33931)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
Bitcoin Core daemon version v30.0.0
config is
`# Listen for RPC connections on this TCP port:
testnet=1
server=1
txindex=1
[test] # β εΏ ι‘»εθΏδΈͺ section
wallet=testwallet
# Allow internal networks to communicate to the RPC endpoint
rpcbind=0.0.0.0
rpcallowip=0.0.0.0/0
rpcport=18332
rpcuser=test
rpcpassword=test
fallbackfee=0.0001`
syn is testnet3 data now
### Expected behavio
...
π¬ sipa commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568645276)
Bitcoin nodes have no knowledge about what other functionality may run on top, like LN or mining. The "services" the P2P node network knows about only relate to functionality offered over that P2P network.
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568645276)
Bitcoin nodes have no knowledge about what other functionality may run on top, like LN or mining. The "services" the P2P node network knows about only relate to functionality offered over that P2P network.