💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051097217)
Tried it out, but then one runs into the issue that it looks like `subprocess.TimeoutExpired` could be raised anywhere in that whole function, which I prefer avoiding.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051118701)
Taken!

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051108994)
Improved comment:
"Only proceed if num_nodes > 0 (child test processes), as 0 (parent) is not supported by BitcoinTestFramework.setup_network()."

Could explore making `BitcoinTestFramework` more permissive in this aspect but would rather defer that.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051121901)
I think it leaves flexibility for adding more tests which use a subset of them.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051126771)
It is rare for functional tests to only test one thing. But I agree "various" is a bit too loose, changed to "framework".

🤔 hodlinator reviewed a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2779505028)
Hi folks!

Asked @l0rinc what it would take for him to give this more than a lightweight A-C-K, and we went through the commits together, especially feature_framework_startup_failures.py. It made me agree some minor improvements are justified in order to make the code more readable & correct.

#### Added final commit - drop or squash?

We also agreed that it would be worth experimenting with breaking up the one test class into multiple to clarify things further. Let me know what you think
...

💬 achow101 commented on pull request "torcontrol: Fix addrOnion outdated comment":
(https://github.com/bitcoin/bitcoin/pull/32282#issuecomment-2816265097)
ACK bcaa23a2b7090c355f7b048ad8cae719c23dea43

🚀 achow101 merged a pull request: "torcontrol: Fix addrOnion outdated comment"
(https://github.com/bitcoin/bitcoin/pull/32282)

💬 achow101 commented on pull request "feefrac: avoid integer overflow in temporary":
(https://github.com/bitcoin/bitcoin/pull/32300#issuecomment-2816284547)
ACK 5cb1241814b9c541c5e5e8daadbbea595f2960ae

✅ achow101 closed an issue: "feefrac_mul_div: Integer-overflow in FeeFrac::Div"
(https://github.com/bitcoin/bitcoin/issues/32294)

🚀 achow101 merged a pull request: "feefrac: avoid integer overflow in temporary"
(https://github.com/bitcoin/bitcoin/pull/32300)

💬 jonatack commented on pull request "[IBD] flush UTXOs in bigger batches based on dbcache size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2051237644)
LGTM, suggestion s/are/is/ and drop one of the two "provided"s

"(default: calculated from the `-dbcache` value, or 16777216 if none is provided)"

💬 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:

...