💬 isaackielma commented on issue "[RFC] What security expectations does/should the RPC server have from credentialed RPC clients?":
(https://github.com/bitcoin/bitcoin/issues/32274#issuecomment-2816363732)
What if docs at [doc/JSON-RPC-interface.md](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md) include an explicit warning stating that:

"RPC credentials grant full administrative control of the node and any resources the `bitcoind` process can access. Clients with valid credentials should be considered as trusted as the local user running `bitcoind`. If untrusted clients must be allowed to access RPC, strong isolation mechanisms (e.g., containers, separate machines, rest
...

📝 ta2ozg opened a pull request: "Update netaddress.cpp"
(https://github.com/bitcoin/bitcoin/pull/32307)
Sets an internal network address based on a user-defined identifier. This approach ensures deterministic internal address generation, while avoiding accidental mismatches due to case or formatting variations.

💬 shahsb commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#issuecomment-2816518575)
Concept ACK

💬 shahsb commented on pull request "wallet: make coinbase that will mature on the next block available for selection":
(https://github.com/bitcoin/bitcoin/pull/32123#discussion_r2051378269)
when are we unlocking this?

💬 l0rinc commented on pull request "[IBD] flush UTXOs in bigger batches based on dbcache size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2051409131)
I wrote "are", since we can set two values which would override the default.

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051419211)
Thanks for checking

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051419245)
The point is to narrow the scope of variables so that we can give them more focused names.
The example in the doc https://docs.python.org/3/whatsnew/3.8.html is basically this exact escenario
```python
if (n := len(a)) > 10:
print(f"List is too long ({n} elements, expected <= 10)")
```

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051419293)
> looks like subprocess.TimeoutExpired could be raised anywhere in that whole function

Isn't that the whole point of a try/catch, that the happy path isn't disrupted, that you don't *have* to know where it's coming from? Otherwise we're treating exceptions as return values.

Quoting again from the Python docs https://docs.python.org/3/tutorial/errors.html
```python
try:
f = open('myfile.txt')
s = f.readline()
i = int(s.strip())
except [...]
```
and not
```python
try:

...

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051419454)
It is, thanks

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2816587753)
You could rebase your change before the fix reverted in https://github.com/bitcoin/bitcoin/pull/32302 to make it green

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051422887)
Or we could avoid repetition as well by a more functional approach of separating data from algorithm:
```python
patterns = {
"Traceback": "tracebacks - expecting exactly one with no knock-on exceptions",
expected_exception: f"occurrences of the specific exception: {expected_exception}",
"Test failed. Test logging available at": "test failure output messages"
}
errors = [f"Found {count}/1 {message}."
for pattern, message in patterns.items()
if (count := le
...

👍 l0rinc approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2779902319)
Concept ACK, I like the new `FeatureFrameworkStartupFailures`, it's untangled a lot better now in my opinion. I assume from the last commit message that it's not final, right?

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051423400)
nit: can probably be `if arg.startswith('--internal_test='):`
or walrus to the rescue again:
```python
if arg.startswith(prefix := '--internal_test='):
class_name = arg[len(prefix):]
```

💬 l0rinc commented on pull request "ci: Temporarily disable `WalletMigration` benchmark":
(https://github.com/bitcoin/bitcoin/pull/32306#discussion_r2051424583)
Yet another alternative would be to comment out `BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW);` instead (so that when we fix it we won't need to tough ci workflows again)

💬 hebasto commented on pull request "ci: Temporarily disable `WalletMigration` benchmark":
(https://github.com/bitcoin/bitcoin/pull/32306#discussion_r2051425168)
Will it have a broader impact beyond the CI?

📝 hebasto opened a pull request: "ci: Drop no longer necessary `-Wno-error=array-bounds`"
(https://github.com/bitcoin/bitcoin/pull/32308)
The build log of the "Linux->Windows cross" CI job no longer shows any `-Warray-bounds` compiler warnings. Therefore, there's no need to suppress them with `-Wno-error=array-bounds`.

I likely overlooked this when reviewing https://github.com/bitcoin/bitcoin/pull/29881, as I can run that CI job locally without such warnings even at commit 785649f3977517a4ba45c5d2fedfbda778fb52cb.

👍 rkrux approved a pull request: "docs: remove passing CI section in guidelines for PR merging as it's common sense"
(https://github.com/bitcoin/bitcoin/pull/32006#pullrequestreview-2779915055)
ACK 17a1630f226e1c8eb13eb7c7425290e933ad9368

📝 hodlinator converted_to_draft a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660)
Improves handling of startup errors in functional tests and puts tests in place to ensure knock-on errors don't creep in.
- `wait_for_rpc_connection()` now appends specific failures leading up to the `Unable to connect to bitcoind` error to that error message:
`[node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 1, 'OSError.ECONNREFUSED': 239}, latest error: ConnectionRefusedError(111, 'Connection refused'))`
- Fixes Windows Python issue where `socket.t
...

👍 TheCharlatan approved a pull request: "ci: Temporarily disable `WalletMigration` benchmark"
(https://github.com/bitcoin/bitcoin/pull/32306#pullrequestreview-2779922342)
ACK 18a035145d6abb41e49711de60c4feabe3ba31e9

Hope this gets fixed properly soon though.

👍 TheCharlatan approved a pull request: "ci: Drop no longer necessary `-Wno-error=array-bounds`"
(https://github.com/bitcoin/bitcoin/pull/32308#pullrequestreview-2779922820)
ACK e34f12bdd417862874fe75f5c4c2126a35724de6