💬 laanwj commented on issue "Use of deprecated CryptAcquireContext in random.cpp":
(https://github.com/bitcoin/bitcoin/issues/32391#issuecomment-2844260438)
OH this is a duplicate of #14089, which was closed at some point. Not sure whether to leave this open.

🤔 shahsb reviewed a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2809300600)
Concept ACK

💬 shahsb commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#issuecomment-2844282604)
ACK https://github.com/bitcoin/bitcoin/pull/32364/commits/657e58bcd739b52e7b9c9349cd14f05b6b79297c

💬 laanwj commented on pull request "test: add test for malleated transaction with valid witness":
(https://github.com/bitcoin/bitcoin/pull/32385#discussion_r2069950447)
Could use `elif`, but as the effect is the same, better to roll this into one `if`, i think:
```python
if (low_s and s > secp256k1.GE.ORDER_HALF) or (not low_s and s < secp256k1.GE.ORDER_HALF):
s = ORDER - s
```

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844353635)
i'm going to approach this differently, turning this PR draft for now.

Going to remove https://github.com/bitcoin/bitcoin/pull/32343/commits/269c2a3dc5c3c85ce73f66c354895f768576ce51 here and try to upstream similar functionality optimizing `close_fds` (written more generally, not using bitcoin-core code) to cpp-subprocess. We can then use that code here.

📝 laanwj converted_to_draft a pull request: "common: Close non-std fds before exec in RunCommandJSON"
(https://github.com/bitcoin/bitcoin/pull/32343)
Picks up stale #30756, while addressing my fallback comment (https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2030844440).

> Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don't think there's any practical scenario where this is a problem right now, but there's a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it's hanging - the listening port(s) won't get released and star
...

📝 laanwj converted_to_draft a pull request: "common: Close non-std fds before exec in RunCommandJSON"
(https://github.com/bitcoin/bitcoin/pull/32343)
Picks up stale #30756, while addressing my fallback comment (https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2030844440).

> Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don't think there's any practical scenario where this is a problem right now, but there's a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it's hanging - the listening port(s) won't get released and star
...

💬 hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844357436)
FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844368951)
> FWIW, I'm running the current PR branch on some other OSes: https://github.com/hebasto/bitcoin-core-nightly/pull/51.

Testing is very welcome, the overall approach is going to stay the same.

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844382194)
> (written more generally, not using bitcoin-core code)

Okay, after a bit of consideration, maybe this was a bad idea: the directory iteration and string-parsing is going to be horrible.

cpp-subprocess is C++11 only, while `std::filesystem` and `std::from_chars` are C++17. i really don't want to go back to using locale-dependent number parsing functions.

📝 AndreaLanfranchi opened a pull request: "Update .gitignore"
(https://github.com/bitcoin/bitcoin/pull/32392)
Build process on WIndows creates the "out" folder by default which is not excluded from changes tracked by git.
Also additional directories `.vs` and `.vscode` are created with files exclusively relevant to the local development environment: commits should not be dirtied by those.

This PR
* Excludes dirs used by VS family.
* Also exclude "out" build directory (default on WIndows)

💬 hebasto commented on pull request "Update .gitignore":
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844423790)
We generally do not accept IDE-specific entries in the project’s `.gitignore`. Please adjust your build environment by other means.

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844423967)
`cc8f239663...8ef769cc2d`: rebase and use the naming:

```cpp
bool IsConnectedToAddrPort(const std::string& addr_port);
bool IsConnectedToAddrPort(const CService& addr_port);
bool IsConnectedToAddr(const CNetAddr& addr);
```

✅ fanquake closed a pull request: "Update .gitignore"
(https://github.com/bitcoin/bitcoin/pull/32392)

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070021226)
Done.

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070024083)
#32338 was merged and these variables are not necessary now.

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844434880)
`8ef769cc2d...4f635d100d`: make the methods `const`

👋 vasild's pull request is ready for review: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326)

💬 hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844437349)
@laanwj

Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.

💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844447695)
> Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.

Sure. Rebased to master.