💬 hebasto commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253787672)
I've tested this in my personal repo and can confirm the re-run job [behaviour](https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3185092065) described by @janb84, which is also documented [here](https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs).

In my opinion, the current behavior is useful for reproducing issues in the CI.

However, before merging a PR, it's much easier to re-run the CI to check for merge conflicts than to rebase the
...

💬 ryanofsky commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2322207556)
In commit "ci: detect outbound internet traffic generated while running tests" (f400e0bb82920adf50fec1b9d701e8e85e62030a)

Thanks for the fix! I think instead of manipulating CreateSock for this specific test it would be probably better to add `gArgs.ForceSetArg("-natpmp", "0");` to `BasicTestingSetup::BasicTestingSetup`. Reasons:

- It would work generally for all tests, not just this one test
- This would make unit tests more consistent with [functional tests](https://github.com/bitcoin/b
...

👍 ryanofsky approved a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-3185369594)
Code review ACK a9ac49c8accffeffaae72f54021fd939c951844c

Thanks for the fix! Note that this fix may (I'm not sure) mask an integer overflow issue https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294091366 that seems like it is a real bug.

The new tcpdump mechanism to detect unexpected traffic is pretty simple and seems very nice. Another idea could be to have unit test setup use `CreateSock` to throw errors if code attempts a remote connection to make errors a little easier to ca
...

📝 fjahr opened a pull request: "index: Force database compaction in coinstatsindex"
(https://github.com/bitcoin/bitcoin/pull/33306)
This PR forces full database compaction on coinstatsindex every 10,000 blocks. This does not seem to come with any considerable performance impact, though I did not test on many different systems.

Motivation: Currently, coinstatsindex ends up using a lot of 55kb files if this is not the case. This is likely related to very fast writes occurring during sync but so far I could not pin down where exactly the difference in behavior is coming from. It seems to be a LevelDB internal thing though, s
...

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3253868966)
> Is my understanding correct that AFL will run init (create the static dir), then fork into several fuzz processes, which run for N iterations and then shut down and delete the static dir? (On the next re-fork, it will fail to find the static dir)?

My description was a bit vague. Since the harness uses `-testdatadir` in init to create the static datadir, it does not get deleted in the TestingSetup destructor (by design of `-testdatadir`). Any time init is called, it will wipe the static data
...

💬 ryanofsky commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3253880125)
Do we know if this change might fix the integer overflow error reported https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294091366?

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2322297692)
re: https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2294123857

> But maybe there is a real bug on this line if it is subtracting `20 - 28`:

This might be fixed by #32159

✅ fanquake closed an issue: "ci: single Windows cache takes up > 25% of total cache space"
(https://github.com/bitcoin/bitcoin/issues/33305)

💬 fanquake commented on issue "ci: single Windows cache takes up > 25% of total cache space":
(https://github.com/bitcoin/bitcoin/issues/33305#issuecomment-3253902831)
This was modifed, but only for master (#32182), so the cache entries are still generated from any release branch, and bloat the global cache. Apparently this may be non-trivial to backport, so can just be left as-is, for now.

💬 purpleKarrot commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#issuecomment-3253905515)
> Here is some historical context.

Don't take it personal. You did a great job migrating from autotools to CMake. I have zero doubts about that.

💬 wrapperband commented on issue "Please restrict Data Carrier/OP Return to < 80 bytes please before releasing 3":
(https://github.com/bitcoin/bitcoin/issues/33298#issuecomment-3253925320)
Please do not excessively increase op_return size. The coder will be as responsible for miss use as tornado cash developer - and they ended up in prison.

💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253938081)
> documented

thx, added to description

> In my opinion, the current behavior is useful for reproducing issues in the CI.

If there is a hypothetical issue to reproduce by re-running, which is no longer present with current master, it seems best to reproduce with an exact commit outside this repo. (Also, I can't really remember a single instance where this was ever needed)

🤔 rkrux reviewed a pull request: "doc: update multisig tutorial to use multipath descriptors"
(https://github.com/bitcoin/bitcoin/pull/33286#pullrequestreview-3185548980)
Concept ACK 226836e70501abf9ac2434900e74eec31edd716e

As mentioned in this comment https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3248584420 as well, I think all these commits can be squashed into one.

💬 rkrux commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2322332266)
In cf2986684ba0db4e0f7b32deb65d7767f80e9c01 "Update multisig-tutorial.md to use multipath descriptors"

Can consider moving this note in the previous section where the multipath descriptor is actually sed-ed.
```diff
diff --git a/doc/multisig-tutorial.md b/doc/multisig-tutorial.md
index fa6366df91..c59359de0a 100644
--- a/doc/multisig-tutorial.md
+++ b/doc/multisig-tutorial.md
@@ -63,6 +63,8 @@ for x in "${!xpubs[@]}"; do printf "[%s]=%s\n" "$x" "${xpubs[$x]}" ; done

As previously m
...

⚠️ wrapperband opened an issue: "Developers will end up in prison. do not excessively increase op_return size"
(https://github.com/bitcoin/bitcoin/issues/33307)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [x] I still think this issue should be opened here

### Report

Please do not excessively increase op_return size. The coder will be as responsible for miss use as shown with tornado cash developer - and they ended up in prison.

💬 hebasto commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253949621)
While testing this PR, I noticed that GitHub’s behaviour seems unreliable.

In my personal repo, I opened a PR, then pushed a commit to the default branch. After that, I checked it out with:
```
git fetch origin pull/4/merge
git rev-parse FETCH_HEAD
```

The output still refers to the branch based on the previous state of the default branch.

It took GitHub a few minutes to update the references.

✅ pinheadmz closed an issue: "Developers will end up in prison. do not excessively increase op_return size"
(https://github.com/bitcoin/bitcoin/issues/33307)

💬 pinheadmz commented on issue "Developers will end up in prison. do not excessively increase op_return size":
(https://github.com/bitcoin/bitcoin/issues/33307#issuecomment-3253951961)
This is a duplicate issue.

💬 wrapperband commented on issue "Developers will end up in prison. do not excessively increase op_return size":
(https://github.com/bitcoin/bitcoin/issues/33307#issuecomment-3253956541)
which issue is it a duplicate of pinheadmz ?

💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3253973715)
> It took GitHub a few minutes to update the references.

Correct. See https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database?apiVersion=2022-11-28#checking-mergeability-of-pull-requests for the docs