💬 ajtowns commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1397632706)
@theuni I think my idea there was that `template<typename T...> PushMessage(CNode& node, T... stuff)` that accepts `CBlock` and so on would be in net_processing.cpp, and that would call `CNode::PushMessage(msg)` after serializing? That would fit with https://github.com/ajtowns/bitcoin/commit/6be38ef045cd1b974ae13d705260e83b769a48b5 at least. I'm just making this up as I go along though :man_shrugging:

💬 maflcko commented on pull request "Drop CAutoFile":
(https://github.com/bitcoin/bitcoin/pull/28904#discussion_r1397653703)
> The tidy job says:

For reference, on master it doesn't say that. See for example https://cirrus-ci.com/task/5754375344226304 (https://api.cirrus-ci.com/v1/task/5754375344226304/logs/ci.log):

```
The full include-list for node/blockstorage.h:
#include <attributes.h> // for LIFETIMEBOUND
#include <chain.h> // for CBlockFileInfo, CBlockIndex
#include <dbwrapper.h> // for CDBWrapper
#include <kernel/blockmanager_opts.h> // for BlockMan
...

💬 maflcko commented on pull request "Drop CAutoFile":
(https://github.com/bitcoin/bitcoin/pull/28904#issuecomment-1816819522)
re-ACK 4eb2a9ea4b6262bec0bc7c20cb3e684ea75caf42 🖼

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 4eb2a9ea4b6262bec0bc
...

💬 ajtowns commented on pull request "refactor: Make CTxMemPoolEntry non-copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#discussion_r1397661685)
Replacing a simple copy with listing all the details of the object seems cumbersome. If we want to just prevent accidental copies, how about doing something like this instead:

```c++
class CTxMemPoolEntry {
private:
CTxMemPoolEntry(const CTxMemPoolEntry&) = default;
struct ExplicitCopyTag { explicit ExplicitCopyTag() = default; };
public:
static constexpr ExplicitCopyTag ExplicitCopy;
CTxMemPoolEntry(ExplicitCopyTag /*unused*/, const CTxMemPoolEntry& entry) : CTxMemPool
...

💬 hebasto commented on pull request "ci: Avoid toolset ambiguity that MSVC can't handle":
(https://github.com/bitcoin/bitcoin/pull/28905#issuecomment-1816831601)
This PR CI runs:
- Version: 20231029.1.0: https://github.com/bitcoin/bitcoin/actions/runs/6906384370/job/18791294660
- Version: 20231115.2.0: https://github.com/bitcoin/bitcoin/actions/runs/6906384370/job/18792841149

💬 hebasto commented on issue "ci: failure in Windows MSVC build":
(https://github.com/bitcoin/bitcoin/issues/28901#issuecomment-1816833050)
Fixed in https://github.com/bitcoin/bitcoin/pull/28905.

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1397673476)
Oh, I see you'd add the function as a method to `PeerManagerImpl`. That should be simple enough to push here as well. I'll try to do that next week.

💬 Sjors commented on pull request "wallet: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1816841223)
Note that with cluster mempool #28676 the error would change. Which is another reason why I think it's useful to have tests cases for all four scenario's. But it can wait for another PR, and perhaps @sdaftuar could write it :-)

💬 ismaelsadeeq commented on pull request "wallet: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1816860057)

Thanks @Sjors
I Would also like to work on it whenever that happens.

💬 brunoerg commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1816917874)
Force-pushed changed the approach:

- Moved it to `contrib`
- Now it's possible to specify the asmap file and the number of unique ASNs to be used in the test
- When adding an address into new table, check the log: `LogPrint(BCLog::ADDRMAN, "Added %s%s to new[%i][%i]\n"`. It ensures that Core is mapping the addresses correctly according to the ASN.

💬 petertodd commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1816943019)


On November 17, 2023 4:52:53 AM EST, Markus ***@***.***> wrote:
>> The tweet you link shows how pointless this pull-req is.
>
>I want to be in charge of my mempool policy and I want to decide what is spam and what is not, in a straightforward and easy way. This is a matter of principle and principles are not pointless. I don't get why you would want to withhold that from node runners, especially since it is completely opt-in and only people who feel strongly about this will choose to do so.

T
...

💬 TheCharlatan commented on pull request "refactor: Make CTxMemPoolEntry non-copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#discussion_r1397768562)
This looks nice, I'll try to apply it.

💬 1440000bytes commented on issue "new RPC: sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/issues/28636#issuecomment-1816957857)
> In the meantime, you could presumably do bitcoin-cli sendmsgtopeer 0 "tx" "$txhex"

Tried on regtest. Did not work because I could not see the transaction in peer 0's mempool.

⚠️ hebasto opened an issue: "RAM usage regression in 26.x and master on ARM 32-bit"
(https://github.com/bitcoin/bitcoin/issues/28906)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The `bitcoind -dbcache=100 -par=4 -disablewallet -reindex-chainstate` command fails with OOM.

### Expected behaviour

RAM usage does not exceed 0.9 GB.

### Steps to reproduce

100% reproducibility with Guix compiled binaries.

### Relevant log output

```
2023-11-17T18:43:31Z [loadblk] UpdateTip: new best=000000000000000016d35e6ca77dea6f4e2caa387f245a712b1df5bbab3c559c height=314541 ver
...

💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1816973103)
A regression caused by https://github.com/bitcoin/bitcoin/pull/25325.

cc @martinus @sipa @achow101

💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1816979340)
> On November 17, 2023 4:52:53 AM EST, Markus ***@***.***> wrote: > The tweet you link shows how pointless this pull-req is. I want to be in charge of my mempool policy and I want to decide what is spam and what is not, in a straightforward and easy way. This is a matter of principle and principles are not pointless. I don't get why you would want to withhold that from node runners, especially since it is completely opt-in and only people who feel strongly about this will choose to do so.
> Thi
...

💬 hebasto commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1816985984)
> Therefore, I'm in the middle of bisecting using Guix builds.

See: https://github.com/bitcoin/bitcoin/issues/28906.

💬 pinheadmz commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1816995291)
Do we have a 32-bit CI task already?

💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1816997141)
> Do we have a 32-bit CI task already?

x86, not ARM.

💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817042102)
This is a shot in the dark, but could this be due to memory fragmentation? The `PoolResource` allocates blocks of 262144 bytes, which seems quite large for such a system. Could you try this patch, or is there a way for me to try this? It reduces the allocated block size to 16K (and fixes a few tests that rely on the hardcoded size).

```patch
diff --git a/src/support/allocators/pool.h b/src/support/allocators/pool.h
index c8e70ebacff..9f93511995e 100644
--- a/src/support/allocators/pool.h

...