💬 l0rinc commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2490998686)
Having no fully end-to-end tests sounds a bit dangerous, since it's part of the fundamental behavior of Bitcoin, not sure we should completely mock that away (i.e. certain bugs could creep in that work locally only - no idea how that would be possible, but that's exactly the point).

💬 fanquake commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491001441)
Which tests are you seeing this from? Neither the unit nor functional tests should be making outside connections. This has certainly been discussed/avoided previously so may be a recent regression.

💬 vasild commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491003878)
I have the idea to log non local traffic with `iptables` and fail the CI. But I do not understand why nothing is logged - I added some quick and dirty commands to `ci/test/03_test_script.sh`:

```sh
iptables -A OUTPUT -m addrtype \! --dst-type LOCAL -j LOG --log-prefix "eeeee1" --log-level emerg
iptables -A OUTPUT -j LOG --log-prefix "eeeee2" --log-level emerg
... generate some traffic with telnet and ping ...
# confirm some packets matched:
iptables -v -x -n -L
...
2024-11-21T11:38:4
...

💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#discussion_r1851965340)
Good point. Just tested that that's indeed the case. I'll mention that in the commit message.

cc @hebasto

🤔 hebasto reviewed a pull request: "build: Enable -Wbidi-chars=any"
(https://github.com/bitcoin/bitcoin/pull/31315#pullrequestreview-2451227488)
Post-merge ACK fa7857ccda5d82739bba16eeb7244433978d28f3. This compiler option is recommended in the [Compiler Options Hardening Guide for C and C++](https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html).

💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491006682)
309bd56d97f87f973f45897fc00b1bd2fc5cff1a -> 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5 ([submitblock_prechecks_2](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_2) -> [submitblock_prechecks_3](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/submitblock_prechecks_2..submitblock_prechecks_3))


* Added release notes documenting the submitblock change w.rt. duplicate block handling and pruning.

💬 vasild commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491017207)
@l0rinc the current end to end tests do communicate locally via the `lo` interface.

@fanquake Honestly, I forgot which test was that. During latest coredev I investigated that briefly with @maflcko or @0xB10C (?) and we observed the non local traffic with `tcpdump(1)`. Now, my point is to enforce this in the CI, even if such test does not exist currently - has been fixed since coredev or my investigation back then was wrong.

💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1851973626)
I decided to add a release note for now. I think the changes in this PR are small enough that we can hash the changes out wholesale and it is also not a particularly pressing change. Still not wholly convinced, but thinking about it some more I don't think somebody was relying on this behaviour beforehand either.

💬 maflcko commented on pull request "refactor: Prepare compile-time check of bilingual format strings":
(https://github.com/bitcoin/bitcoin/pull/31295#discussion_r1851976250)
It shouldn't reproduce. This commit is just taken from the other pulls, so that clang-tidy is happy with them.

💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#discussion_r1851979781)
Added

💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2491024996)
Rebased, added comment and expanded commit message to note the GUI improvement.

💬 maflcko commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491034618)
I think it makes sense to enforce this in CI (and have a tracking issue or pull for that). Otherwise, it is easy to regress, like in https://github.com/bitcoin/bitcoin/pull/30529#issuecomment-2252854223

However, if there is a violation of this, or a bug, it would be good to add exact steps to reproduce, or an explanation. Claiming that this is broken on "Ubuntu 28.04 LTS" doesn't seem helpful to uncover the bug, if there is one.

Related: https://github.com/bitcoin/bitcoin/issues/30030

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1851990186)
Actually, fixed. You can drop this now.

👍 theStack approved a pull request: "test: Deduplicate assert_mempool_contents()"
(https://github.com/bitcoin/bitcoin/pull/31338#pullrequestreview-2451281918)
ACK a0eafc10f94362408f54195ffd5a9237dc1ef638

Thanks for following up!

💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2491063362)
I added a commit to make `waitfornewblock` visible in `help`. It was introduced in 2016 by #8680 as a quick fix to deal with CI failures.

At this point is seem mature enough, no longer has weird quirks like requiring GUI users to enable the RPC server, it's probably actually useful and at least [one library uses it anyway](https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/06a6c156b4d469c60c1abe8afdc002e7e11e5d3b/client/src/client.rs#L1100). So it seems fine to list it.

💬 m3dwards commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2491116799)
> Made some changes to this part of `TestNode.stop_node()` to handle an already dead process

Looks cleaner.

ACK ba28593147ccc880c3a4d40db0d4ef57f4766254

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2491178957)
Rebased after #31288 and #31122.

This PR is now based on #31325 since that has enough support. Keeping this draft until that's merged (or dropped).

The test changes will probably cause a merge conflict with #31318, but it should be trivial.

I dropped the early return in `waitNext()` because `wait_until` should already take care of that. I also made the while loop use `<=` instead of `<` so this actually works: https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1843589729 Added a
...

✅ fanquake closed an issue: "ci: brew link workaround step no-longer working"
(https://github.com/bitcoin/bitcoin/issues/31334)

🚀 fanquake merged a pull request: "macOS: swap docs & CI from pkg-config to pkgconf"
(https://github.com/bitcoin/bitcoin/pull/31335)

👍 brunoerg approved a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2451460077)
code review ACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49