📝 martinus opened a pull request: "refactor: serialization simplifications"
(https://github.com/bitcoin/bitcoin/pull/28203)
This simplifies the serialization code a bit and should also make it a bit faster.

* use fold expressions instead of recursive calls. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations.

* use `if constexpr` instead of unnecessarily creating a temporary object only to call the right overload. This is used for `std::vector` and `prevector` serialization.

🤔 instagibbs reviewed a pull request: "test: tx orphan handling"
(https://github.com/bitcoin/bitcoin/pull/28199#pullrequestreview-1559366940)
moar coverage good

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282251680)
took me a second, just assert it's not in mempool for those quickly reading

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282234657)
```suggestion
# The parent should be requested since the unstripped wtxid would differ. Delayed because it's by txid and this is not a preferred relay peer.
```

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282208201)
Stick the constant inside `cleanup` and move the comment for `cleanup`, since it explains what the whole thing is doing.

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282243358)
can't we just reconsider it again?

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282131242)
comment block seems extraneous if there's logging for each test

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282258402)
can we assert that peer_spy hasn't received the INV, just to be sure?

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282259677)
I don't get this case. Why wouldn't you request a "fake" parent if you don't know it's fake already?

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282268679)
"The node should not request a parent if it has an in-flight txrequest" ? Seems like parents are being reqiested. Maybe a typo or I can't tell what scenario it's covering.

💬 instagibbs commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1282261573)
does this wallet never use 0-conf change? would be nice to assert to make it clear the lack of utxo connection

📝 hebasto opened a pull request: "qa: Close SQLite connection properly"
(https://github.com/bitcoin/bitcoin/pull/28204)
This PR is a follow-up for https://github.com/bitcoin/bitcoin/pull/26462 that introduced a bug on Windows:
```
>test\functional\wallet_descriptor.py
...
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process:
...
```

From `sqlite3` Python module [docs](https://docs.python.org/3/library/sqlite3.html#how-to-use-the-connection-context-manager):
> `Connection` object used as context manager only commits or rollbacks transactions, so th
...

💬 hebasto commented on pull request "wallet: fix crash on loading descriptor wallet containing legacy key type entries":
(https://github.com/bitcoin/bitcoin/pull/26462#issuecomment-1662749647)
The test is broken on Windows:
```
>test\functional\wallet_descriptor.py
...
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process:
...
```

Fixed in https://github.com/bitcoin/bitcoin/pull/28204.

💬 hebasto commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#discussion_r1282298267)
```suggestion
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```

💬 ryanofsky commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1662828515)
Updated e025bb3f8d229969fdfe16e3c20191a5382c0443 -> c553d9f0b0f796c30124fddb920b5e8b5b32de11 ([`pr/indexy.41`](https://github.com/ryanofsky/bitcoin/commits/pr/indexy.41) -> [`pr/indexy.42`](https://github.com/ryanofsky/bitcoin/commits/pr/indexy.42), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/indexy.41..pr/indexy.42)) to avoid race condition pointed out by furszy in an intermediate commit: https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1282110542 which could potentially
...

👍 theStack approved a pull request: "qa: Close SQLite connection properly"
(https://github.com/bitcoin/bitcoin/pull/28204#pullrequestreview-1559672559)
utACK 703b758e187492d4752270cd9922eaf0af20e2d0

Thanks for fixing! Can't reproduce the error locally as I don't have a Windows machine available, but the change looks correct to me and is in line with the Python documentation (https://docs.python.org/3/library/sqlite3.html#sqlite3-connection-context-manager).

💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1282316651)
Alright yeah I was a bit too quick with my initial suggestion, the current implementation makes sense from a (de)alloc perspective, thanks for pointing that both.

> Let me know if you think passing them arguments here is more confusing.

I still don't really like it, but I agree that taking these 2 arguments is the most natural interface for `WriteImpl`. The only reason we have this weirdness is just because `Write` wants to do some allocation optimization. So I'm okay keeping it like this.
...

👍 theStack approved a pull request: "rpc, test: `addnode` improv + add test coverage for invalid command"
(https://github.com/bitcoin/bitcoin/pull/26366#pullrequestreview-1559679246)
re-ACK f52cb02f700b58bca921a7aa24bfeee04760262b

💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1282340187)
```
2023-08-02T19:32:23Z [sensitiverelay:debug]
Request for 5 new connections, incremented the number of connections to open from 90 to 95
```

💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1282341818)
I meant, if IPv4 and Tor are the only reachable networks, that Tor connections would *only* be made for sensitive relay connections. That's why I tried `-onlynet=ipv4 -sensitiverelayowntx=1`