👍 dergoegge approved a pull request: "ci: run native fuzz with MSAN job"
(https://github.com/bitcoin/bitcoin/pull/33626#pullrequestreview-3416571878)
utACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
(https://github.com/bitcoin/bitcoin/pull/33626#pullrequestreview-3416571878)
utACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490601023)
I'd rather we use the version of the compiler with more bug fixes, and less known bugs.
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490601023)
I'd rather we use the version of the compiler with more bug fixes, and less known bugs.
💬 luke-jr commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486116400)
Don't twist my quote out of context. Removing my DNS seed would be no harm to me, but it still shouldn't be done and would reflect poorly on Bitcoin Core if you do it anyway.
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486116400)
Don't twist my quote out of context. Removing my DNS seed would be no harm to me, but it still shouldn't be done and would reflect poorly on Bitcoin Core if you do it anyway.
💬 maflcko commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3486123197)
lgtm ACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3486123197)
lgtm ACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3486133274)
I added a commit with further improvements.
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3486133274)
I added a commit with further improvements.
💬 fanquake commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3486136927)
cc @willcl-ark @m3dwards
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3486136927)
cc @willcl-ark @m3dwards
🚀 fanquake merged a pull request: "ci: run native fuzz with MSAN job"
(https://github.com/bitcoin/bitcoin/pull/33626)
(https://github.com/bitcoin/bitcoin/pull/33626)
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3486197237)
`69c015b258...6091e94750`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3486197237)
`69c015b258...6091e94750`: rebase due to conflicts
💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490689248)
Can we do it for the shorter ones as well? it's easier to compare them and more consistent with the rest
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490689248)
Can we do it for the shorter ones as well? it's easier to compare them and more consistent with the rest
💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3486217821)
> A year later, adoption is now almost at ~70% according to https://21.ninja/reachable-nodes/node-service-share/ - could update the OP with that info.
Concept ACK. i was critical of this in the past, but i'm more convinced that it makes some sense now, and believe it outweighs the implementation/maintenance cost of such a feature.
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3486217821)
> A year later, adoption is now almost at ~70% according to https://21.ninja/reachable-nodes/node-service-share/ - could update the OP with that info.
Concept ACK. i was critical of this in the past, but i'm more convinced that it makes some sense now, and believe it outweighs the implementation/maintenance cost of such a feature.
💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490696786)
The test became more complicated than it was before (which was my main critique of it, seems arbitrary).
Could we simplify it? I think we're testing too many things, a single median check should likely suffice in my opinion.
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490696786)
The test became more complicated than it was before (which was my main critique of it, seems arbitrary).
Could we simplify it? I think we're testing too many things, a single median check should likely suffice in my opinion.
💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490699922)
what does `--` mean here?
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490699922)
what does `--` mean here?
💬 frankomosh commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490694848)
Only 1 coin is created (~1 BTC), but the target is 8 BTC. So my superficial thought is that `result_b` might never reach the target, therefore, the resulting failure would not be because of `TOTAL_TRIES` being hit, but simply insufficient funds. Or might I be missing some context here?
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490694848)
Only 1 coin is created (~1 BTC), but the target is 8 BTC. So my superficial thought is that `result_b` might never reach the target, therefore, the resulting failure would not be because of `TOTAL_TRIES` being hit, but simply insufficient funds. Or might I be missing some context here?
💬 frankomosh commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490695744)
nit: I think there's a double space after `=`. (maybe should be single spaced for consistency?)
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490695744)
nit: I think there's a double space after `=`. (maybe should be single spaced for consistency?)
💬 Sjors commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486236896)
Note that there's other seeds run by people who are not active contributors. That shouldn't be a reason to remove a seed, unless we're consistent about that.
Active hostility is a different matter. I don't read that in "no longer support the Bitcoin Core project", I do see it in social media. The question is whether that's sufficient grounds for removing a seed, or whether something additional should happen.
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486236896)
Note that there's other seeds run by people who are not active contributors. That shouldn't be a reason to remove a seed, unless we're consistent about that.
Active hostility is a different matter. I don't read that in "no longer support the Bitcoin Core project", I do see it in social media. The question is whether that's sufficient grounds for removing a seed, or whether something additional should happen.
💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490707151)
I don't understand where `16` and the `67%` come from.
Can we just keep the very last check of the median instead?
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490707151)
I don't understand where `16` and the `67%` come from.
Can we just keep the very last check of the median instead?
💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490708490)
k, please resolve the comment
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490708490)
k, please resolve the comment
💬 brunoerg commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2490759547)
cfeb160baec1369452c42d05c51f2a6af76ed077: nit: Perhaps we could remove this `(default: none)` to maintain consistency with other `=<file>` options?
(https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2490759547)
cfeb160baec1369452c42d05c51f2a6af76ed077: nit: Perhaps we could remove this `(default: none)` to maintain consistency with other `=<file>` options?
👍 brunoerg approved a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416811104)
code review ACK cfeb160baec1369452c42d05c51f2a6af76ed077
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416811104)
code review ACK cfeb160baec1369452c42d05c51f2a6af76ed077
👍 TheCharlatan approved a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416860885)
ACK cfeb160baec1369452c42d05c51f2a6af76ed077
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416860885)
ACK cfeb160baec1369452c42d05c51f2a6af76ed077