💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1285714365)
nit: No longer needed, I think? (in 9a63566be5445e7026517f38f10ed760a90dbe2f)

📝 MarcoFalke opened a pull request: "rpc: Add Param() default helper"
(https://github.com/bitcoin/bitcoin/pull/28230)
Currently the RPC method implementations have many issues:

* Default RPC argument values (and their optionality state) are duplicated in the documentation and the C++ code, with no checks to prevent them from going out of sync.
* Getting an optional RPC argument is verbose, using a ternary operator, or worse, a multi-line `if`.

Fix all issues by adding default helper that can be called via `self.Param<int>(0)`. The helper needs three (3) lines of code in the `src/rpc/util.h` header file.
...

💬 MarcoFalke commented on pull request "rpc: Add RPCContext":
(https://github.com/bitcoin/bitcoin/pull/20017#issuecomment-1667667630)
I took the concept from here (Thanks!), but implemented it differently (in just three lines of code in the header file). See https://github.com/bitcoin/bitcoin/pull/28230

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285729865)
Done in https://github.com/bitcoin/bitcoin/pull/28230

💬 alxppv commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667683634)
> Only Visual Studio 2022 is [supported](https://github.com/bitcoin/bitcoin/blob/master/build_msvc/README.md).

It's probably all about habit. I don't see any problems. VS2019 supports ISO C++20 Standard. Except for the described error, everything compiles. All tests pass correctly.

💬 MarcoFalke commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667687809)
In any case this seems to be an upstream bug with your msvc not implementing the C++ standard correctly. You can try upgrading to Visual Studio 2022, but I am not sure what we are supposed to do here?

💬 ajtowns commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1667716263)
utACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a

💬 alxppv commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667717263)
> upgrading to Visual Studio 2022, but I am not sure what we are supposed to do here?

In general, this issue is informative.
Maybe someone tried, got an error and did not understand further.
But the reproducibility of this error on VS2019 is interesting.

💬 glozow commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#issuecomment-1667718598)
Rebased

💬 hebasto commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667719375)
> It's probably all about habit. I don't see any problems. VS2019 supports ISO C++20 Standard.

More details are available here: https://github.com/bitcoin/bitcoin/pull/24531.

🤔 dergoegge reviewed a pull request: "Use shared_ptr for CNode inside CConnman"
(https://github.com/bitcoin/bitcoin/pull/28222#pullrequestreview-1565029167)
Concept ACK on getting rid of our custom ref counting.

I actually implemented something similar a while ago (https://github.com/dergoegge/bitcoin/commit/b3913b4c052b37790b3737e5ddb9c2d029502477) but didn't pursue because it doesn't retain the behavior we currently have w.r.t. which thread actually deletes the `CNode`. That is also the reason why https://github.com/bitcoin/bitcoin/pull/10738 is a bit more complex and introduces new pointer types.

> which thread actually deletes the CNode.

...

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1285681548)
Is there a reason why we need to use the shared_ptr type in net processing?

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1285689978)
In a way this is worse than the custom ref counting because it is harder to assert that we actually don't take new references after this check.

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1285682361)
(I know this will get push back but)

nit: Could we call this `ConnectionRef`? After all the connection manager is managing... connections.

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#discussion_r1285683763)
It would also seem more modern to use `using` here

💬 dergoegge commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-1667726251)
cc @theuni (just in case you are interested in chiming in)

💬 stickies-v commented on pull request "rpc: Add Param() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#issuecomment-1667726750)
Concept ACK

💬 fanquake commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667732205)
> but I am not sure what we are supposed to do here?

Yea, nothing we can do about broken software that we explicitly don't support. Closing for now. Discussion can continue if needed.

✅ fanquake closed an issue: "error C3203: 'UniqueLock'"
(https://github.com/bitcoin/bitcoin/issues/28229)

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1285784839)
> Also I think it's a bit unclear where the "additional flags" go, are they part of [permissions@] ?

Yes, see the docs of `-whitelist`:
```cpp
argsman.AddArg("-whitelist=<[permissions@]IP address or network>", strprintf("Add permission flags to the peers using the given IP address (e.g. 1.2.3.4) or "
"CIDR-notated network (e.g. 1.2.3.0/24). Uses the same permissions as "
"-whitebind. "
"Additional flags \"in\" and \"out\" control whether permissions apply to i
...