💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1225488646)
@vasild
>This PR currently expands the semantic of the `noban` permission

Could you please elaborate on this point? I thought that with this PR, `noban` nodes are still [completely protected](https://github.com/bitcoin/bitcoin/pull/27600/files#diff-2404112ebf57bee5f9a16f1a6e1ecfc27a981d37a1c0ff202b4cd9bdfa3e48ccR183) (they are removed from the eviction candidates list before the random node is chosen). But it's certain that I'm missing something. Thanks.

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1225502218)
I'm not a fan of default arguments; consider making this not have a default. It makes sense if there are many existing calls to a function and you don't want to touch them all, but there's only one call to this function in production code. You would have to touch about 6 call sites in test code, but that's not too bad. The reason I'm not a fan of default arguments is that they hide something that may be helpful to see when just looking at the call site. (You might think, "Oh, it's possible that
...

💬 pinheadmz commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#issuecomment-1585761906)
concept ACK

I was looking for this writing https://github.com/bitcoin/bitcoin/pull/27850 was surprised that a fatal internal error didn't satisfy the python test for assert init error on startup

💬 dimitaracev commented on pull request "ci: Use podman stop over podman kill":
(https://github.com/bitcoin/bitcoin/pull/27844#issuecomment-1585766862)
ACK [faaa627](https://github.com/bitcoin/bitcoin/pull/27844/commits/faaa62754e84417baa917f20db379db78146687d)

💬 kallerosenbaum commented on issue "Can't start bitcoin-qt by double-click on Debian 11":
(https://github.com/bitcoin-core/gui/issues/730#issuecomment-1585774030)
Thanks @hebasto

Since Ubuntu is based on "Debian Testing", and the current ubuntu LTS works, I'm guessing (I haven't tested this issue on Debian Testing so I don't really know this) this issue will be resolved in the next major version of debian.

💬 dimitaracev commented on pull request "Supporting parameter "h" and "?" in -netinfo.":
(https://github.com/bitcoin/bitcoin/pull/27830#issuecomment-1585774322)
Honestly I would just create a function that does the check, in case we use `-h` and `-?` in other arguments. A few changes in the `bitcoin-cli.cpp` are required as well since only `-help` is mentioned e.g on line 90 and 636.

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1585793347)
Even though not needed for this PR, you may want to consider adding a commit to remove the unnecessary templating. It's perfectly reasonable not to make this change in this PR, but I thought I'd document it just in case; I do think it makes the code easier to understand.
```
--- a/src/node/eviction.cpp
+++ b/src/node/eviction.cpp
@@ -74,9 +74,10 @@ struct CompareNodeNetworkTime {
};

//! Sort an array by the specified comparator, then erase the last K elements where predicate is true.
...

💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1585814637)
@LarryRuane thanks for reviewing. I actually had removed the templating in the [very first version](https://github.com/bitcoin/bitcoin/compare/e71d495ffbda3bc072bbaecd7580809d5087f9e6..c826187b5070ce89edcde0536183714a1c2e3207#diff-2404112ebf57bee5f9a16f1a6e1ecfc27a981d37a1c0ff202b4cd9bdfa3e48ccL77) of this branch but after changing the approach I just left it alone. The function is written like a generic utility function, even though we don't currently use it for any other vectors. So I think th
...

📝 kevkevinpal opened a pull request: "test: added coverage to rpc_blockchain.py"
(https://github.com/bitcoin/bitcoin/pull/27852)
Included a test that checks the functionality of setting -1 for the first param of getnetworkhashps which uses the number of blocks since last difficulty adjustment.

checks that the hashes per second * 300 -1 is less than 0.006

Adding coverage for this line of code
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/mining.cpp#L67

not totally sure if it is best practice to have that 0.006 value hard set I can change it to something else if wanted, I was getting `hashes_per_second_si
...

🤔 w0xlt reviewed a pull request: "[Silent Payments]: Base functionality"
(https://github.com/bitcoin/bitcoin/pull/27827#pullrequestreview-1473530000)
Approach ACK naturally,
Will review in detail soon.

Thanks for moving this project forward and splitting the draft PR in smaller ones.

💬 w0xlt commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#discussion_r1225654137)
These comments can be removed.
```suggestion
```

💬 w0xlt commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#discussion_r1225654181)
These comments can be removed.

```suggestion
```

📝 brunoerg opened a pull request: "rest: bugfix, fix crash error when calling `/deploymentinfo`"
(https://github.com/bitcoin/bitcoin/pull/27853)
Calling `/deploymentinfo` passing a valid blockhash makes bitcoind to crash. It happens because we're pushing a JSON value of type array when it expects type object. See:
```cpp
jsonRequest.params = UniValue(UniValue::VARR);
```
```cpp
jsonRequest.params.pushKV("blockhash", hash_str);
```

This PR fixes it by changing `VARR` to `VOBJ` for `UniValue` and adds more test coverage.

💬 brunoerg commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1585979754)
Credits to @andrewtoth for noticing it!

💬 Miguelmavs commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#discussion_r1225696041)
test/functional/interface_rest.py

💬 Miguelmavs commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1586000002)
test/functional/interface_rest.py

💬 Miguelmavs commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1586003305)
jsonRequest.params.pushKV("blockhash", hash_str);

💬 Miguelmavs commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#issuecomment-1586003399)
jsonRequest.params.pushKV("blockhash", hash_str);

💬 MarcoFalke commented on issue "Can't start bitcoin-qt by double-click on Debian 11":
(https://github.com/bitcoin-core/gui/issues/730#issuecomment-1586047488)
Yeah, I was about to ask if this is an issue with Debian 12

💬 Miguelmavs commented on pull request "rest: bugfix, fix crash error when calling `/deploymentinfo`":
(https://github.com/bitcoin/bitcoin/pull/27853#discussion_r1225728703)
jsonRequest.params.pushKV("blockhash", hash_str);