💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136182559)
I think I need to touch five or six different places for this. Is that worth it?

💬 mzumsande commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1136210879)
This is just being moved here and I don't think this can overflow in practice, but the init value should be `int64_t{0}`, so maybe it would make sense to fix this somewhere within this PR. (searched the rest of the codebase after https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1136136446).

💬 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.

💬 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

💬 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.

💬 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.

💬 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.

💬 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.

💬 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.

💬 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.

💬 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!

💬 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.

💬 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.

👍 ajtowns approved a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(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`
...

💬 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.

🚀 fanquake merged a pull request: "guix: pass `--enable-initfini-array` to release GCC"
(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.

💬 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`?

💬 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
...