💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1562538942)
> I'm going to see if we can maybe port this over to our depends

I am not sure if compiling a compiler for a CI task is a worthwhile thing to maintain in this repo? Ideally we can use one from the Debian/Ubuntu distros, as they are used the most, so that it will be easy to bootstrap the test config outside the CI env. If the Debian/Ubuntu one isn't working, we could try into using a different distro temporarily? But imo compiling it from scratch should be for the last fallback and not a long
...

💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1205214490)
Looks like memory usage isn't reported in https://cirrus-ci.com/task/4543816401682432 ?

💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1562543301)
> I am not sure if compiling a compiler for a CI task is a worthwhile thing to maintain in this repo?

This point is in regards to the libc++ flag usage. Not compiling a compiler for anything else.

💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1205220173)
> Looks like memory usage isn't reported in https://cirrus-ci.com/task/4543816401682432 ?

Is that a CIrrus bug? You should be able to see an example memory usage here: https://cirrus-ci.com/task/4632561431871488.

🚀 fanquake merged a pull request: "p2p: Unconditionally return when compact block status == READ_STATUS_FAILED"
(https://github.com/bitcoin/bitcoin/pull/27743)

💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1205226114)
ah ok, so it is using 12 GB?

Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds?

💬 fanquake commented on pull request "test: Disable legacy wallet for mempool_packages.py":
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1562569950)
cc @glozow

💬 fanquake commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1562574435)
@Sjors

💬 fanquake commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1562580491)
https://cirrus-ci.com/task/4751246913961984?logs=ci#L2724:
```bash
test/denialofservice_tests.cpp(134): Entering test case "stale_tip_peer_management"
test/denialofservice_tests.cpp(212): error: in "denialofservice_tests/stale_tip_peer_management": check vNodes[i]->fDisconnect == false has failed
test/denialofservice_tests.cpp(214): error: in "denialofservice_tests/stale_tip_peer_management": check vNodes[max_outbound_full_relay - 2]->fDisconnect == true has failed
test/denialofservice_test
...

💬 glozow commented on pull request "test: Disable legacy wallet for mempool_packages.py":
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1562597550)
> Yeah, the test doesn't need the wallet, but it is providing mempool-package coverage for the listunspent wallet RPC. So maybe an alternative would be to move it out into a new test, idk?

Would be in favor of moving the listunspent coverage to a new/existing `wallet_` functional test instead.

💬 dergoegge commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1562602731)
ACK 6ee3881551f2cd411c4e4d8b0ccedf0f0416d8c2

💬 MarcoFalke commented on pull request "test: Disable legacy wallet for mempool_packages.py":
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1562610976)
Ok, I'll leave this bugfix open for review for now. I can close once and if there is another pull that moves the check somewhere else.

👍 willcl-ark approved a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1443525129)
re-ACK eefe56967b

💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1205282280)
Yeah you're right, I was leaving comments on a per-commit review basis, but it's removed later on in 7d3b35004b039f2bd606bb46a540de7babdbc41e . Can (should) be removed in this commit already, though. But no biggie either way.

💬 MarcoFalke commented on pull request "rpc: Use 'byte'/'bytes' for bech32(m) validation error message":
(https://github.com/bitcoin/bitcoin/pull/27747#issuecomment-1562633587)
lgtm ACK 3d0a5c37e9ccedfa4ecfaa48eeeca1ada5b4eec1

🚀 fanquake merged a pull request: "25.0 Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27686)

💬 ChrisCho-H commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1562645400)
rebase done!

💬 fanquake commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1562651354)
`25.0` has been tagged.

✅ fanquake closed an issue: "v25.0 testing"
(https://github.com/bitcoin/bitcoin/issues/27621)

📝 fanquake opened a pull request: "[25.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27750)
Backports https://github.com/bitcoin/bitcoin/pull/27727 to 25.x.