💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1667401501)
Thanks, I've split each commit to be iwyu targeted to one header/module. The overall force-push diff is zero.

🤔 mzumsande reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1564808157)
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`.
(for me, `sizeof(CTxMemPoolEntry)` even grows from 264 to 280 bytes - packing effects?).
This seems a bit unsatisfactory because the `entry_sequence` data won't be needed after a few seconds (i.e.
...

💬 viresinnumeris-1 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667417085)
Concept ACK

Bare multisig has barely any non-spam use cases left.

💬 russeree commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667418511)
Idea, what about reducing the standardness to only accept compressed format public keys this reducing the capacity to spam?

Second I just scraped my UTXO set using https://github.com/in3rsha/bitcoin-utxo-dump and filtered my set for only p2ms keeping all row data and the total size is 278MB which is incredibly small even compared to P2PKH which sits at 7.9GB

🚀 fanquake merged a pull request: "refactor: Remove unused MessageStartChars parameters from BlockManager methods"
(https://github.com/bitcoin/bitcoin/pull/28191)

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