π¬ 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.
π¬ pinheadmz commented on issue "how to syn testnet4 data":
(https://github.com/bitcoin/bitcoin/issues/33931#issuecomment-3568647252)
use `testnet4=1`
https://github.com/bitcoin/bitcoin/blob/a14e7b9dee9145920f93eab0254ce92942bd1e5e/share/examples/bitcoin.conf#L568-L570
(https://github.com/bitcoin/bitcoin/issues/33931#issuecomment-3568647252)
use `testnet4=1`
https://github.com/bitcoin/bitcoin/blob/a14e7b9dee9145920f93eab0254ce92942bd1e5e/share/examples/bitcoin.conf#L568-L570
β
taoteh1221 closed an issue: "Documentation on 'services' parsing in RPC response for getnodeaddresses"
(https://github.com/bitcoin/bitcoin/issues/33930)
(https://github.com/bitcoin/bitcoin/issues/33930)
π¬ taoteh1221 commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568664817)
Thanks @sipa, I appreciate the quick response. π
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568664817)
Thanks @sipa, I appreciate the quick response. π
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568675720)
There was another issue in the `feature_taproot.py` test where the "compat/nocsa" test case incorrectly assumed bare NONSTANDARD scripts with `OP_CHECKSIGADD` would pass `AreInputsStandard`. Since `OP_CHECKSIGADD` (0xba) exceeds `MAX_OPCODE` (0xb9), `HasValidOps()` returns false, so `AreInputsStandard` rejects it regardless of sigop count. I restored the original standardness logic requiring wrapping (p2sh or witv0) for this case.
There's also a fuzzer failure in `process_messages` with an as
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568675720)
There was another issue in the `feature_taproot.py` test where the "compat/nocsa" test case incorrectly assumed bare NONSTANDARD scripts with `OP_CHECKSIGADD` would pass `AreInputsStandard`. Since `OP_CHECKSIGADD` (0xba) exceeds `MAX_OPCODE` (0xb9), `HasValidOps()` returns false, so `AreInputsStandard` rejects it regardless of sigop count. I restored the original standardness logic requiring wrapping (p2sh or witv0) for this case.
There's also a fuzzer failure in `process_messages` with an as
...
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568706803)
It looks like the fuzzer failure in `process_messages` came from the mempool consistency check. When checking inputs, if an input references an output from another mempool transaction, that output must be in `mempoolDuplicate` before the assertion. Transactions are sorted by ancestor count, but there can be cases where a parent's output isn't in `mempoolDuplicate` when a child is checked. The fix adds the parent transaction's output to `mempoolDuplicate` before the assertion, making the check mo
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568706803)
It looks like the fuzzer failure in `process_messages` came from the mempool consistency check. When checking inputs, if an input references an output from another mempool transaction, that output must be in `mempoolDuplicate` before the assertion. Transactions are sorted by ancestor count, but there can be cases where a parent's output isn't in `mempoolDuplicate` when a child is checked. The fix adds the parent transaction's output to `mempoolDuplicate` before the assertion, making the check mo
...
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568779013)
I think the mempool consistency check was failing because input coins weren't always available in mempoolDuplicate when the assertion checked for them. The fix in [73399e8](https://github.com/bitcoin/bitcoin/pull/33926/commits/73399e80a20c57cb6ba3a8e766be9166d840e526) ensures both mempool transaction outputs and UTXO set coins are available in mempoolDuplicate before the assertion, which should resolve the fuzzer failure.
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568779013)
I think the mempool consistency check was failing because input coins weren't always available in mempoolDuplicate when the assertion checked for them. The fix in [73399e8](https://github.com/bitcoin/bitcoin/pull/33926/commits/73399e80a20c57cb6ba3a8e766be9166d840e526) ensures both mempool transaction outputs and UTXO set coins are available in mempoolDuplicate before the assertion, which should resolve the fuzzer failure.