👍 stickies-v approved a pull request: "test: wallet rescan with reorged parent + IsFromMe child in mempool"
(https://github.com/bitcoin/bitcoin/pull/29179#pullrequestreview-1818404985)
re-ACK 9db0d42509e0e223b45238fd8087fc98ef32c091

💬 sipa commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450536603)
Done. I needed to re-touch anyway because apparently `nool` is not a C++ type.

👍 ismaelsadeeq approved a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1818400509)
ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c

It will be nice to also log how many transactions were dumped not just unbroadcast set
q
```diff
diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp
index cae4b04bf65..404f23b21a9 100644
--- a/src/kernel/mempool_persist.cpp
+++ b/src/kernel/mempool_persist.cpp
@@ -194,7 +194,6 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock

file << mapDeltas;

- LogPrintf("Wr
...

💬 ismaelsadeeq commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#discussion_r1450533993)
nit
```suggestion
LogInfo("Progress loading mempool transactions from disk: %d%% completed (tried %u txs, %u txs remaining)\n",
```

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450547499)
Deleted this line altogether, there's no reason to generate 1 either.

💬 sipa commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1889415495)
It appears to me that whitelisting of outbound connections really only applies to manual ones: if you have reason to specifically trust another peer or otherwise give them elevated privileges, you probably want to also try actually connecting to them.

If so, it seems more natural to me to specify outbound net permissions wherever that manual connection is instructed (so inside `-addnode`, -connect=`, or as argument to the `addnode` RPC?

👍 dergoegge approved a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1818441827)
Code review ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1889422328)
> See commit https://github.com/bitcoin/bitcoin/commit/f8273512a50d9ed03bc48803634fb8780b9023c7 (feel free to pull it).

Sorry I hadn't seen this. Thanks, taken with a couple of edits.

📝 fjahr opened a pull request: "doc: Add missing backtick in developer notes logging section"
(https://github.com/bitcoin/bitcoin/pull/29241)
Newly added logging section is missing a single backtick.

💬 maflcko commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889432589)
```
test/functional/wallet_assumeutxo.py:14:1: F401 'platform' imported but unused

👍 theStack approved a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1818452953)
re-ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c

💬 ismaelsadeeq commented on pull request "doc: Add missing backtick in developer notes logging section":
(https://github.com/bitcoin/bitcoin/pull/29241#issuecomment-1889437511)
utACK a29d5c9f5100a5e15b243960ebc505305a70901e 👍🏾

Thanks was just reading this

🤔 jonatack reviewed a pull request: "doc: Add missing backtick in developer notes logging section"
(https://github.com/bitcoin/bitcoin/pull/29241#pullrequestreview-1818473528)
ACK, one suggestion

💬 jonatack commented on pull request "doc: Add missing backtick in developer notes logging section":
(https://github.com/bitcoin/bitcoin/pull/29241#discussion_r1450579579)
While touching these, suggest also this

```diff
@@ -742,7 +742,7 @@ logging messages. They should be used as follows:
- `LogInfo(fmt, params...)` should only be used rarely, eg for startup
messages or for infrequent and important events such as a new block tip
being found or a new outbound connection being made. These log messages
- are unconditional so care must be taken that they can't be used by an
+ are unconditional, so care must be taken that they can't be used by an

...

📝 TheCharlatan converted_to_draft a pull request: "build: Remove HAVE_CONSENSUS_LIB"
(https://github.com/bitcoin/bitcoin/pull/29123)
The `script/bitcoinconsensus` module defines the public interface for the `bitcoinconsensus` library. Even though this module is only required by the tests and the `bitcoinconsensus` library, it is currently compiled into the static internal `libbitcoin_consensus` library, and therefore used by a bunch of build targets that do not require it.

Since it is always part of the internal library, the `HAVE_CONSENSUS_LIB` define used by the tests is essentially
meaningless, since the module is alwa
...

💬 fjahr commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1889477510)
> > `* =` in the current implementation this involves giving someone a malicious binary or getting them to recompile
>
> Not sure if this pull request is needed then.
>
> If someone is running a malicious binary or malicious code, the malicious actor will have this code removed from the binary or code.

I have been thinking about this concept for a while now and I tend to agree that this is not necessary for similar reasons as mentioned above but I am also not against this being added. H
...

💬 tiero commented on pull request "Implement 64 bit arithmetic op codes in the Script interpreter":
(https://github.com/bitcoin/bitcoin/pull/29221#issuecomment-1889485166)
Concept ACK

💬 maflcko commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-1889501254)
Maybe mark as draft while this is waiting on #29165? (29165 won't happen unless someone bumps google's oss-fuzz's clang)

💬 fjahr commented on pull request "doc: Add missing backtick in developer notes logging section":
(https://github.com/bitcoin/bitcoin/pull/29241#discussion_r1450602869)
Done, thanks!

📝 sipa converted_to_draft a pull request: "miniscript: make operator""_mst consteval"
(https://github.com/bitcoin/bitcoin/pull/28657)
It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the `constexpr` keyword.

Since C++20, we can force them to evaluate at compile time using `consteval`, turning all the miniscript type constants into actual compile-time constants.

This should give a nice but not very important speedup for miniscript logic, but it's also a way to start testing C++20 features.