🚀 achow101 merged a pull request: "multiprocess: Add basic type conversion hooks"
(https://github.com/bitcoin/bitcoin/pull/28921)

📝 theuni opened a pull request: "depends: patch libool out of libnatpmp/miniupnpc"
(https://github.com/bitcoin/bitcoin/pull/29298)
An alternative to https://github.com/bitcoin/bitcoin/pull/29232

Rather than switching to the CMake builds which [proved problematic](https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1898513919), do the quick and dirty thing of just patching out libtool. Doesn't seem to introduce any new issues.

This should buy us time to upstream the necessary CMake fixes.

💬 theuni commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1906942254)
Closing in favor of #29298.

💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1906951455)
Added a quick note about `std::byteswap` and c++23.

🤔 achow101 reviewed a pull request: "wallet, rpc: `FundTransaction` refactor"
(https://github.com/bitcoin/bitcoin/pull/28560#pullrequestreview-1839971435)
ACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d

💬 achow101 commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1464018197)
In f7384b921c3460c7a3cc7827a68b2c613bd98f8e "refactor: move parsing to new function"

nit: Making temp variables is not necessary.

✅ theuni closed a pull request: "depends: remove dependency on Darwin libtool"
(https://github.com/bitcoin/bitcoin/pull/29232)

🚀 achow101 merged a pull request: "wallet, rpc: `FundTransaction` refactor"
(https://github.com/bitcoin/bitcoin/pull/28560)

💬 furszy commented on issue "Prune Node Rescan Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/29183#issuecomment-1906996240)
> Does `scanblocks` work on pruned nodes yet? If not, does this project aim to add that capability?

`scanblocks` always worked on pruned nodes. Just need to enable `-blockfilterindex` since the node's inception.

💬 mzumsande commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1907059722)
> * The final pindex->IsAssumedValid() check added in [#28791](https://github.com/bitcoin/bitcoin/pull/28791) seems a little overbroad. I think the condition `(pindex->nChainTx == pindex->nTx + prev_chain_tx)` should actually be true for all assumed-valid blocks except the first one and the last one, so that check could be tightened up

After trying this out, I think that this is not the case. The `BLOCK_ASSUMED_VALID` status is only removed when the block is connected to the chain and raised
...

💬 fanquake commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1907071391)
So now we will (always?) log this on startup:
```bash
2024-01-23T22:48:12.205684Z [init] Setting file arg: _warning_ = "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."
```

Which seems a bit pointless/noisy, and could maybe even be confusing to users?

📝 mzumsande opened a pull request: "validation: improve checkblockindex comments"
(https://github.com/bitcoin/bitcoin/pull/29299)
The two assumptions there were described as test-only, which has led to confusion whether they should exist.
However, they are necessary in general, as the changed comment explains - without them, the check would fail everywhere where it is enabled.
The second commit moves this assert down to the other checks.

Closes #29261

💬 mzumsande commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1907088890)
See #29299 for a fix (only changing the doc and moving the check into a proper section).

📝 furszy opened a pull request: "init: settings, do not load auto-generated warning msg"
(https://github.com/bitcoin/bitcoin/pull/29301)
Fixes https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1907071391.

The settings warning message is meant to be used only to discourage users from
modifying the file manually. Therefore, there is no need to keep it in memory.

💬 furszy commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1907124245)
> So now we will (always?) log this on startup:
>
> ```shell
> 2024-01-23T22:48:12.205684Z [init] Setting file arg: _warning_ = "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."
> ```
>
> Which seems a bit pointless/noisy, and could maybe even be confusing to users?

Agree. Good catch. Fixed in #29301.

📝 marcofleon opened a pull request: "wallet: clarify replaced_by_txid and replaces_txid in help output"
(https://github.com/bitcoin/bitcoin/pull/29302)
Resolves issue #27781

💬 Eunovo commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1464304643)
@josibake p2sh-p2pkh or p2sh-p2pk are not addressed in the BIP. While I understand they are not commonly used scripts but `Solver(redeem_script, solutions)` can still detect p2pkh or p2pk in the redeem_script. Is there anything against adding support for these scripts? assuming that we ignore non-standard or malleated p2pkh in the redeem script

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464318846)
(originally because `on_connection_send_msg` would be end up being unused in v2 connection anyways and wanted to keep the `on_connection_send_msg` send during the v1 reconnection phase.)

interesting question! you're talking about a less common scenario where initiator node is preparing for v2 connection thinking responder node is v2 but v1 version message from responder node gets sent before v2 ellswift bytes from initiator node. if this happens the initiator node wouldn't attempt a reconnect
...

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464356180)
hmm other than the ellswift bytes, `v2_handshake()` also needs to read garbage bytes (could be received as separate chunks with size < 16 bytes) and other things like garbage terminator, decoy messages and version packet. i'm leaning towards keeping the interface generic and processing as we receive bytes.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464363550)
oh that way! the last test was for v1 behaviour and peer7 needed a v1 `TestNode` while all the others needed a v2 `TestNode` which is why it was kept separately in the end.