💬 gzuuus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1667509988)
Concept NACK. It is easy to see that bitcoin is neither an efficient nor convenient way to store data in the "cloud", so why support the use of a feature that does not benefit and goes against of bitcoin's value transaction and is inconvenient for node operators and users in general? Strongly agree with the above answers NACK

🤔 TheCharlatan reviewed a pull request: "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block"
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1564904754)
ACK 2ccbef905fb6faa2480b49103680529dc7cfc482

💬 TheCharlatan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1285602074)
In commit 6229158f77a7b1803f2767a089dd94dedf62b533
Nit: You could just forward-declare `struct FlatFilePos` without having to include flatfile (same for `zmqpublishnotifier.h`).

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