👍 john-moffett approved a pull request: "refactor: remove windows-only compat.h usage in random"
(https://github.com/bitcoin/bitcoin/pull/26814)
ACK 621cfb77227b5a240d66547947f73130f0c51f44

💬 jonatack commented on pull request "contrib: remove install_db4.sh":
(https://github.com/bitcoin/bitcoin/pull/26834#issuecomment-1435058468)
Made a number of updates to https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests that include this change.

⚠️ GaloisField2718 opened an issue: "error: timeout on transient error: Could not connect to the server 127.0.0.1:8333 (error code 1 - "EOF reached")"
(https://github.com/bitcoin/bitcoin/issues/27121)
<!-- This issue tracker is only for technical issues related to Bitcoin Core.

General bitcoin questions and/or support requests are best directed to the Bitcoin StackExchange at https://bitcoin.stackexchange.com.

For reporting security issues, please read instructions at https://bitcoincore.org/en/contact/.

If the node is "stuck" during sync or giving "block checksum mismatch" errors, please ensure your hardware is stable by running memtest and observe CPU temperature with a load-test t
...

💬 achow101 commented on pull request "net: avoid overriding non-virtual ToString() in CService and use better naming":
(https://github.com/bitcoin/bitcoin/pull/25619#issuecomment-1435065439)
ACK c9d548c91fb12fba516dee896f1f97692cfa2104

💬 brunoerg commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1110179294)
Yes, better to call `LookupSubNet()`.

💬 brunoerg commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1110180280)
Done

💬 brunoerg commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1110180577)
Make sense, done!

💬 brunoerg commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1110180782)
Done!

💬 brunoerg commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#issuecomment-1435068141)
Force-pushed addressing @vasild's review.

💬 brunoerg commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1435079192)
> Subnetting does not make sense for tor/i2p/cjdns
> It follows that banman is doing something that does not make sense (e.g. subnetting tor). This is the root of the problem. I think if that is eradicated, then the rest will untangle by itself.

`CConnman::DisconnectNode` also handles addresses as subnets. Is this a problem?

🚀 achow101 merged a pull request: "net: avoid overriding non-virtual ToString() in CService and use better naming"
(https://github.com/bitcoin/bitcoin/pull/25619)

💬 yancyribbens commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1435080650)
@jonatack thanks for the response. I agree with you that feedback is not just a technical process but also a social one. You're links also help as well.

@achow101 @MarcoFalke thanks for taking the time to review.

💬 yancyribbens commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1435081051)
s/you're/your

✅ pinheadmz closed an issue: "on OSX, bitcoind chooses different data directory than Bitcoin-Qt"
(https://github.com/bitcoin/bitcoin/issues/27119)

💬 pinheadmz commented on issue "on OSX, bitcoind chooses different data directory than Bitcoin-Qt":
(https://github.com/bitcoin/bitcoin/issues/27119#issuecomment-1435081900)
This is my fault, there were old Qt preferences saved and the custom directory was recalled from there

💬 yancyribbens commented on pull request "wallet: Guard against undefined behaviour":
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1435094706)
@MarcoFalke it looks like the following test_case [here](https://github.com/bitcoin/bitcoin/blob/35fbc972082eca0fc848fba77360ff35f1ba69e1/src/wallet/test/coinselector_tests.cpp#L949) was added after I made the PR (that test doesn't exist in my PR branch).

The `cost_of_change` should always be greater than zero so something is wrong with the test case. However, there doesn't seem to be much enthusiasm for this PR so its probably not worth it for me to spend time fixing it unless I get feed
...

💬 achow101 commented on pull request "p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified":
(https://github.com/bitcoin/bitcoin/pull/20018#issuecomment-1435121819)
ACK 2555a3950f0304b7af7609c1e6c696993c50ac72

🚀 achow101 merged a pull request: "p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified"
(https://github.com/bitcoin/bitcoin/pull/20018)

💬 ryanofsky commented on pull request "refactor, kernel: Remove gArgs accesses from dbwrapper and txdb":
(https://github.com/bitcoin/bitcoin/pull/25862#issuecomment-1435133240)
This has 2 code reviews. Not sure if it might be ready for merge, or if it needs another reviewer.

💬 Xekyo commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1435138448)
Concept ACK

Did some light review, looks reasonable