💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1468854086)
> What do you think?
As @stickies-v explained to me outside this PR: "... by moving the uri parsing to http_request_cb we're failing super early, and even when the query parameter would never be called by the endpoint... ", this makes more sense.
I'm working on this approach which I think it's better as well.
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1468854086)
> What do you think?
As @stickies-v explained to me outside this PR: "... by moving the uri parsing to http_request_cb we're failing super early, and even when the query parameter would never be called by the endpoint... ", this makes more sense.
I'm working on this approach which I think it's better as well.
💬 desirepl commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1468925984)
@pinheadmz bitcoin-cli also ignores .conf's datadir, so not only GUI
maybe bitcoind too, didn't checked it, because spent too much time for -qt to understand why it's not working as must be
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1468925984)
@pinheadmz bitcoin-cli also ignores .conf's datadir, so not only GUI
maybe bitcoind too, didn't checked it, because spent too much time for -qt to understand why it's not working as must be
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136214058)
I’ve updated the comment to mention the 500 entry limit and the resulting empty vector.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136214058)
I’ve updated the comment to mention the 500 entry limit and the resulting empty vector.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136311603)
Thanks, I adopted your suggestion.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136311603)
Thanks, I adopted your suggestion.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136215648)
Good catch. I’ve removed the line and amended the comment to clarify that the transaction is part of the descendants.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136215648)
Good catch. I’ve removed the line and amended the comment to clarify that the transaction is part of the descendants.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136294354)
Thanks I took your suggestion with slightly more elaborate variable names.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136294354)
Thanks I took your suggestion with slightly more elaborate variable names.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136217858)
Thanks, I’ve adopted your suggestion and moved `!mempool.exists` first.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136217858)
Thanks, I’ve adopted your suggestion and moved `!mempool.exists` first.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136319884)
Thanks, this improves the fuzzer. I’ve adopted your suggested change.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136319884)
Thanks, this improves the fuzzer. I’ve adopted your suggested change.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136299879)
Uff, great catch, thanks!
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136299879)
Uff, great catch, thanks!
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136330617)
I see how this might be a performance improvement, but I don’t think this is going to be heavy enough to warrant this level of rewrite at this stage in the PR. Perhaps this could be done in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136330617)
I see how this might be a performance improvement, but I don’t think this is going to be heavy enough to warrant this level of rewrite at this stage in the PR. Perhaps this could be done in a follow-up.
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1469045147)
force-pushed: Added `self.whitelist_peers` in `test_framework` to avoid having to add `-whitelist` flag everywhere.
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1469045147)
force-pushed: Added `self.whitelist_peers` in `test_framework` to avoid having to add `-whitelist` flag everywhere.
👍 ajtowns approved a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(https://github.com/bitcoin/bitcoin/pull/26177)
ACK 600ab02bf58e073a93936438a7e884b3a7110f1c
(https://github.com/bitcoin/bitcoin/pull/26177)
ACK 600ab02bf58e073a93936438a7e884b3a7110f1c
💬 ishaanam commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1136502150)
Yes, I have implemented this both for `walletlock` and `walletpassphrasechange` to ensure that this error is raised in both. However, it is important to note that this test now tests something slightly different than what it did previously. Previously this checked that the wallet was still able to complete a full rescan even if an attempt was made to relock the wallet (because of the timeout in `walletpassphrase`). This new test does not test this because other than using the `walletpassphrase`
...
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1136502150)
Yes, I have implemented this both for `walletlock` and `walletpassphrasechange` to ensure that this error is raised in both. However, it is important to note that this test now tests something slightly different than what it did previously. Previously this checked that the wallet was still able to complete a full rescan even if an attempt was made to relock the wallet (because of the timeout in `walletpassphrase`). This new test does not test this because other than using the `walletpassphrase`
...
💬 ajtowns commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1469360153)
I'm not sure it has any influence on this pr, but did you consider doing something similar for block-relay-only peers?
Something like: connect to one generic block-relay-only peer, plus an additional block-relay-only peer for every network we support? I think that should only improve things, including during IBD. It wouldn't do anything to improve tx propagation of course, so wouldn't make this PR redundant.
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1469360153)
I'm not sure it has any influence on this pr, but did you consider doing something similar for block-relay-only peers?
Something like: connect to one generic block-relay-only peer, plus an additional block-relay-only peer for every network we support? I think that should only improve things, including during IBD. It wouldn't do anything to improve tx propagation of course, so wouldn't make this PR redundant.
🚀 fanquake merged a pull request: "guix: pass `--enable-initfini-array` to release GCC"
(https://github.com/bitcoin/bitcoin/pull/27153)
(https://github.com/bitcoin/bitcoin/pull/27153)
💬 bigspider commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1136724397)
Perhaps `multi_a` for consistency, since `multi` is a valid fragment elsewhere?
Or `Tapscript multisig` if you want to explicitly _not_ refer to the fragment.
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1136724397)
Perhaps `multi_a` for consistency, since `multi` is a valid fragment elsewhere?
Or `Tapscript multisig` if you want to explicitly _not_ refer to the fragment.
💬 MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1136746362)
> walletpassphrase
If you want to keep coverage for that, you can add it as well (before `walletlock`) with a timeout of `1`, which should override the previous timeout of `999999`?
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1136746362)
> walletpassphrase
If you want to keep coverage for that, you can add it as well (before `walletlock`) with a timeout of `1`, which should override the previous timeout of `999999`?
💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1469699591)
Updated ea278094e9937bf96157161941d38a0c08a0aa4e -> c4f2b6d619f6c0faee5aa3118ade1040105fdd8b ([splitSystemFs_1](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_1) -> [splitSystemFs_2](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_1..splitSystemFs_2)) to rename `util/fs.*` to `util/fs_helpers.*`
Added commit dac4d1dc3d75d510efe6c84c2cb6d7d7dc96e2c6 ([splitSystemFs_3](https://github.com/The
...
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1469699591)
Updated ea278094e9937bf96157161941d38a0c08a0aa4e -> c4f2b6d619f6c0faee5aa3118ade1040105fdd8b ([splitSystemFs_1](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_1) -> [splitSystemFs_2](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_1..splitSystemFs_2)) to rename `util/fs.*` to `util/fs_helpers.*`
Added commit dac4d1dc3d75d510efe6c84c2cb6d7d7dc96e2c6 ([splitSystemFs_3](https://github.com/The
...
📝 russeree opened a pull request: "rpc: enhanced error message for invalid network prefix during address parsing."
(https://github.com/bitcoin/bitcoin/pull/27260)
This commit addresses an issue in Bitcoin Core where an incorrect bech32 chain network hrp (prefix) would lead to the address being treated as entirely invalid for both base58 and bech32 decoding, without providing the user with an appropriate error message.
reference
https://github.com/bitcoin/bitcoin/issues/26290
**General Changes**
When a user inputs an incorrect address DecodeDestination now informs the user of the input and expected chain prefixes as well as the network name string.
...
(https://github.com/bitcoin/bitcoin/pull/27260)
This commit addresses an issue in Bitcoin Core where an incorrect bech32 chain network hrp (prefix) would lead to the address being treated as entirely invalid for both base58 and bech32 decoding, without providing the user with an appropriate error message.
reference
https://github.com/bitcoin/bitcoin/issues/26290
**General Changes**
When a user inputs an incorrect address DecodeDestination now informs the user of the input and expected chain prefixes as well as the network name string.
...
💬 vasild commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1136853031)
@mzumsande,
> I'd expect performance to go down a bit for the case where we have many addresses to choose from. Do you see this in your benchmark?
Yes, that slows down a bit the fast `Select()` (any network is ok). I observed that and think it is ok because it is still super fast with or without the shuffling.
> Why does this need `ALREADY_VISITED_AND_BORING`?
The `for` loop could still repeat the whole array from the start if some address was found but was skipped. `ALREADY_VISITED_
...
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1136853031)
@mzumsande,
> I'd expect performance to go down a bit for the case where we have many addresses to choose from. Do you see this in your benchmark?
Yes, that slows down a bit the fast `Select()` (any network is ok). I observed that and think it is ok because it is still super fast with or without the shuffling.
> Why does this need `ALREADY_VISITED_AND_BORING`?
The `for` loop could still repeat the whole array from the start if some address was found but was skipped. `ALREADY_VISITED_
...