💬 aureleoules commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1908490513)
@fanquake thanks for reaching out, I fixed the issue.

💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1908499136)
@aureleoules Thanks! These numbers really don't make much sense to me and don't match what I've seen locally, but I'll work on trying to reproduce.

💬 ryanofsky commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1908513362)
> So during the period where we receive blocks for the background chain in some, possibly random, order, there will be a mix of blocks with updated `nTx` and fake `nTx` ones, so that the condition could fail everywhere in the range of assumed-valid blocks.

Thanks for figuring this out and trying this. Trying to put this all together it seems like these are the cases:

1. Case where IsValid() is true. Then nChainTx must be prev nChainTx + nTx.
2. Case where nTx is 0. Then nChainTx must be 0
...

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1465282827)
The goal is not to support every possible script. Rather, we want to limit it to a sane subset to avoid technical debt and keep the implementation simple. In fact, the only reason for including `P2PKH` at all is due to the fact it is still widely used and makes up a significant percentage of the UTXO set, otherwise we would have only allowed Segwit script templates as this gives us non-malleability guarantees.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1465295573)
This assert actually failed in CI (wow!):

https://cirrus-ci.com/task/5586893237125120

```
Run autofile with args ['/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/autofile')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3765617163
INFO: Loaded 1 modules (547059 inline 8-bit counters): 547059 [0x55cd54dd29b8, 0x55cd54e582ab),
INFO: Loaded 1 PC tabl
...

👍 ryanofsky approved a pull request: "wallet: clarify replaced_by_txid and replaces_txid in help output"
(https://github.com/bitcoin/bitcoin/pull/29302#pullrequestreview-1841980191)
Code review ACK ff54314d4abed3bf9a78daf785a01c63af15c69d. Seems like a helpful clarification

💬 helpau commented on issue "Huge disk fragmentation":
(https://github.com/bitcoin/bitcoin/issues/29296#issuecomment-1908614398)
In my test environment fragmentation is only caused by Bitcoin Core, and affects Bitcoin Core. I think to eliminate fragmentation, all you need to do is increase buffers(e.g. to 2MB for .ldb files), limit simultaneous writes for different files, or preallocate disc space. I don't see your answer as a solution, as it boils down to:
1)buying an SSD(which imposes limitations, and doesn't completely solve the problem)
2)using defragmentation(requires time, user involvement and restarting Bitcoin
...

💬 achow101 commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#issuecomment-1908640373)
ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c

💬 achow101 commented on pull request "wallet: clarify replaced_by_txid and replaces_txid in help output":
(https://github.com/bitcoin/bitcoin/pull/29302#issuecomment-1908641427)
ACK ff54314d4abed3bf9a78daf785a01c63af15c69d

🚀 achow101 merged a pull request: "wallet: clarify replaced_by_txid and replaces_txid in help output"
(https://github.com/bitcoin/bitcoin/pull/29302)

👍 ryanofsky approved a pull request: "init: settings, do not load auto-generated warning msg"
(https://github.com/bitcoin/bitcoin/pull/29301#pullrequestreview-1842105713)
Code review ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c. Seems like a clean, simple fix

💬 ryanofsky commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#discussion_r1465379877)
In commit "init: settings, do not load auto-generated warning msg" (987a1b51eeb72c7fcb085470817a4fe85fcc3c7c)

Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

👍 BrandonOdiwuor approved a pull request: "doc: clarify `BroadcastTransaction` comment"
(https://github.com/bitcoin/bitcoin/pull/29308#pullrequestreview-1842137635)
lgtm ACK cca20d94b787d0881a61509445f4827f913e051d

👍 kristapsk approved a pull request: "doc: clarify `BroadcastTransaction` comment"
(https://github.com/bitcoin/bitcoin/pull/29308#pullrequestreview-1842149187)
ACK cca20d94b787d0881a61509445f4827f913e051d

💬 mzumsande commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465420112)
The python code only supports the "reconnection" use case in one direction: TestNode connects to the P2PConnection with v2, and reconnects with v1 without anything breaking (e.g. timeouts) on the python side. The reverse direction (P2PConnection actively does reconnections itself for outgoing connections) is not supported and I don't think it needs to be. So P2PConnection is receiving an inbound connection, and here it must wait to send `on_connection_send_msg` with sending until the version is
...

💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1908758657)
From some more local tests, it looks like it's the clz changes slowing things down for whatever reason. Still investigating.

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908758998)
tACK 8672721deb06e66854a085c9994e99c99160b8a1

Steps to test:

1) Build and install master, sync to tip (for this test I used 207220ce8b767d8efdb5abf042ecf23d846ded73)
2) Go to mempool.space, click on the block to be confirmed next (get the block before that if you want a bit more time for the testing before it gets confirmed); hover over the block animation and click the goggles icon in the top left of the animation.
3) Select "Bare multisig" and then click any of the highlighted transact
...

💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1465280323)
In commit "[refactor] Make MainSignals RAII styled" (dddaa962cfbeb96a8148535e74e1b53c9c717376)

Maybe unintended diff moving this line

👍 ryanofsky approved a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1841942054)
Code review ACK bdd985af62ff9e94490ecc701d6063eaab172add. Letting signals be optional and moving more initialization to AppInitMain seem like nice changes among other cleanups.

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908780342)
> > Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
>
> what do we mean by ravaging?

@moonsettler low-resource devices tend to struggle with IBD once the utxoset no longer fits in memory. My own experience with this comes from helping users sync Raspberry Pi 4 devices for a couple of years while working at Start9. (The Raspberry Pi is especially bad because the bridge connecting external USB disks to the CPU doesn't work very well.) Once you
...