💬 darosior commented on pull request "net, validation: don't punish peers for consensus-invalid txs":
(https://github.com/bitcoin/bitcoin/pull/33050#discussion_r2231118764)
If only the non-standard error message is renamed, and "block-script-verify-flag-failed" is left as a followup, this should not meaningfully affect the size of the diff at all. Here is a diff which does this with your "mempool-script-verify-flag-failed" suggestion:
<details>
<summary>Expand patch</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 3acdc6f63a1..a73ebc56eeb 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2218,7 +2218,7 @@ bool CheckI
...

📝 maflcko opened a pull request: "util: Revert "common: Close non-std fds before exec in RunCommandJSON""
(https://github.com/bitcoin/bitcoin/pull/33063)
After a fork() in a multithreaded program, the child can safely
call only async-signal-safe functions (see [signal-safety(7)](https://www.man7.org/linux/man-pages/man7/signal-safety.7.html))
until such time as it calls execv.

The standard library (`std` namespace) is not async-signal-safe. Also, `throw`, isn't.

There was an alternative implementation using `readdir` (https://github.com/bitcoin/bitcoin/pull/32529), but that isn't async-signal-safe either, and that implementation was still
...

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3117880896)
I did some more testing with 587c63e06955ce5c5f2d64f1c1700ca8628572cf, noticed improvements:
- rescan uses the slow version (when `-blockfilterindex` is enabled), as expected
- if I add a label to a transaction, it stores it

New bug:
- when I send with change, using my wallet that only has an sp descriptor, the change output is dropped and added to fees

Some remaining issues:
- add silent payments to "output types" in the receive screen (see patch below)

Some style suggestions:
- I
...

💬 fanquake commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3117894742)
ACK - to stop the flow of CI failures.

cc @hebasto @laanwj.

💬 fanquake commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-3117894878)
Note that this is being reverted in #33063.

📝 fanquake opened a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/33064)
This is #27593 cleaned up / rebased, now that the legacy wallet has been dropped.

Currently a draft given:
```bash
ALL | ✓ Passed | 2752 s (accumulated)
Runtime: 194 s

Uncovered RPC commands:
- upgradewallet

Cleaning up coverage data
```

`upgradewallet` is likely to be removed in #32944.

Closes #27593.

💬 fanquake commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-3117940249)
Picked up in #33064.

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2231169665)
5be304dfa50bcfd3b0a92eff52bac3d8023ce523 nit: no need to set `next_index` during import

💬 Sjors commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#discussion_r2231248253)
Maybe only change this and don't undo the already upstreamed change (yet)?

💬 Sjors commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118093293)
> call only async-signal-safe functions

Is this related? https://github.com/arun11299/cpp-subprocess/issues/53

💬 Sjors commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#discussion_r2231269913)
Oh never mind, we didn't upstream this ourselves, it was already there and we copied it here.

🚀 fanquake merged a pull request: "ci: Enable more shellcheck"
(https://github.com/bitcoin/bitcoin/pull/32970)

💬 maflcko commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#discussion_r2231277189)
> Maybe only change this and don't undo the already upstreamed change (yet)?

I don't understand what you are referring to. Can you provide a link?

💬 maflcko commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118175723)
> Is this related? [arun11299/cpp-subprocess#53](https://github.com/arun11299/cpp-subprocess/issues/53)

Yes. My recommendation would be to fix this upstream. I currently don't plan to do it, so anyone else is free to pick it up.

💬 Sjors commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118320711)
Concept ACK

I agree it makes sense to fix upstream first.

How do you verify a reverted merge commit? I guess this worked:

```sh
maflcko/2507-tempoary-revert
git cherry-pick 4f5e04da135080291853f71e6f81dd0302224c3a^..a0eed55398f882d9390e50582b10272d18f2b836
git diff maflcko/2507-tempoary-revert~1
```

So do I understand correctly from the man page you linked to that `close()` and `getpid` and are already signal safe, and it's just `throw` and `sysconf` that are the problem?

💬 fanquake commented on issue "ci: Lower and unify default stack size":
(https://github.com/bitcoin/bitcoin/issues/29840#issuecomment-3118322437)
Any opinions on doing this via link flags? We can instruct the linker to set stack size limits on the executable during linking. One advantage might be that we can set varying limits, per build, but that might not be useful.

If not, I'll pickup the ulimit approach.

👍 darosior approved a pull request: "[29.x] final changes for v29.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33056#pullrequestreview-3055779227)
ACK 4c2d285b706058ece7092d03f0e5ad1112c2097f

✅ Sjors closed a pull request: "subprocess: always check result of close()"
(https://github.com/bitcoin/bitcoin/pull/33061)

💬 Sjors commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#issuecomment-3118343686)
It appears that underlying issue is now understood to be signal safety, see #33063. So improving `close` error reporting and handling isn't a priority.

💬 maflcko commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118364788)
> it's just `throw` and `sysconf` that are the problem?

No, all of the C++ standard libary is problematic as well. (See the pull description and the bt)



> How do you verify a reverted merge commit? I guess this worked:

I typed `git revert -m1 31d3eebfb92ae0521e18225d69be95e78fb02672`, but `git` is very flexible and you can type many different things to arrive at the same result.