💬 fanquake commented on pull request "p2p: diversity network outbounds follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28189#issuecomment-1667517304)
@amitiuttarwar want to undraft this now?
cc @mzumsande @willcl-ark @vasild @naumenkogs

💬 fanquake commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#issuecomment-1667535093)
Concept ACK

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285643464)
thx, done

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285644159)
Maybe submit a pull with your suggestion applied to another RPC? Once and if it is merged, I could rebase this one.

💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1667552061)
@real-or-random

> But I don't understand the numbers.

There was a [bug](https://github.com/cirruslabs/cirrus-ci-web/pull/572#issuecomment-1667038748), which has been fixed now.

💬 dergoegge commented on pull request "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1667572204)
Concept ACK

💬 theStack commented on pull request "test: check for specific bip157 disconnect reasons, add test coverage":
(https://github.com/bitcoin/bitcoin/pull/28227#discussion_r1285678494)
Thanks, fixed.

💬 Retropex commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#discussion_r1285679380)
> You'll need to squash the updates required for the tests to pass into the same commit as the code change.

Done.

💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1667596577)
> (for me, `sizeof(CTxMemPoolEntry)` even grows from 264 to 280 bytes - packing effects?)

Added a commit to fix this.

> My understanding is that there is a bit of a trade-off with respect to memory (correct me if the byte counting is off!): While it saves 33kB per peer (so ~4MB with 125 peers; ~250kB for non-listening nodes with 8 peers), it also adds at least 8 bytes per mempool entry for `uint64_t entry_sequence`. [..] This seems a bit unsatisfactory because the `entry_sequence` data won
...

⚠️ alxppv opened an issue: "error C3203: 'UniqueLock'"
(https://github.com/bitcoin/bitcoin/issues/28229)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When compiling the libbitcoin_node library in Microsoft Visual Studio Community 2019, Version 16.11.28
got error C3203: 'UniqueLock': unspecialized class template can't be used as a template argument for template parameter '_Ty', expected a real type
in the validationinterface.cpp file,
code section:

template<typename F> void Iterate(F&& f) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)

...

💬 hebasto commented on issue "error C3203: 'UniqueLock'":
(https://github.com/bitcoin/bitcoin/issues/28229#issuecomment-1667650013)
> When compiling the libbitcoin_node library in Microsoft Visual Studio Community 2019,

Only Visual Studio 2022 is [supported](https://github.com/bitcoin/bitcoin/blob/master/build_msvc/README.md).

👍 stickies-v approved a pull request: "kernel: Prune leveldb headers"
(https://github.com/bitcoin/bitcoin/pull/28186#pullrequestreview-1565082131)
re-ACK 1be04724a3ac061859118c09b90e2e15ea8d04b0

nit: these don't seem to actually have been addressed
> Addressed @stickies-v's https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283098892 and https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283280722, added some line breaks in constructor lists.

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