🤔 pablomartin4btc reviewed a pull request: "test: Fix test by checking the actual exception instance"
(https://github.com/bitcoin/bitcoin/pull/28989#pullrequestreview-1766346784)
ACK 55e3dc3e03510e97caba1547a82e3e022b0bbd42

💬 Farnoosh85 commented on issue "test: BDB wallets":
(https://github.com/bitcoin/bitcoin/issues/28606#issuecomment-1841874793)
> I wonder what the best way is to test BDB wallets, which can still be loaded.
>
>
>
> The current status seems to be that each test is run twice, once with a BDB backend, and once with an sqlite backend. However, this will be harder, once BDB creation is removed. So I wonder what should be done:
>
>
>
> 0) Remove coverage for loading and testing with a BDB backend, except for the previous-releases test
>
> 0) Create BDB wallets with a previous release, if available
>
> 0) Everything fr
...

👍 theStack approved a pull request: "rpc: fix getrawtransaction segfault"
(https://github.com/bitcoin/bitcoin/pull/29003#pullrequestreview-1766476361)
Tested ACK 9075a446461ccbc446d21af778aac50b604f39b3

Reproduced the crash on master with my pruned node manually via
```
$ ./src/bitcoin-cli getrawmempool | head -n 2
[
"sometxid...",
$ ./src/bitcoin-cli getrawtransaction sometxid... 2
error: timeout on transient error: Could not connect to the server 127.0.0.1:8332 (error code 1 - "EOF reached")

Make sure the bitcoind server is running and that you are connecting to the correct RPC port.

-----
bitcoind crashes with "Segmentat
...

📝 stratospher opened a pull request: "test: create deterministic addrman in the functional tests"
(https://github.com/bitcoin/bitcoin/pull/29007)
Testing addrman functionality is limited in the functional tests because `nKey` for the addrman is chosen randomly during every run and there's a small chance for collision when inserting a new address into the addrman. Currently in our tests, we keep only 1 address in the new table, 1 address in the tried table and don't insert new addresses into the addrman because of these possible collisions. See https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1091145647, https://github.com/bitcoin
...

💬 Brock124590 commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#discussion_r1416601435)
Do you have your secret phrase

💬 kevkevinpal commented on pull request "test: fix v2 transport intermittent test failure (#29002)":
(https://github.com/bitcoin/bitcoin/pull/29006#issuecomment-1842092254)
Concept ACK [00e0658](https://github.com/bitcoin/bitcoin/pull/29006/commits/00e0658e77f66103ebdeb29def99dc9f937c049d)

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1416717975)
My concern is less with the specific wording and more that we are logging low-level algorithm internals that at best are not actionable for the node runner and at worst confusing.

For developers, the algorithm and waste metric (along with additional data points) are already captured in `TRACE5`:

https://github.com/bitcoin/bitcoin/blob/efb54a2bd284639722da794650d2afda66fb0770/src/wallet/spend.cpp#L1128

So my questions are:

1. How is the introduction of this logline related to this _sp
...

🤔 stratospher reviewed a pull request: "test: fix v2 transport intermittent test failure (#29002)"
(https://github.com/bitcoin/bitcoin/pull/29006#pullrequestreview-1766740168)
ACK 00e0658.

cross checked node0's debug log to verify that [this getpeerinfo](https://github.com/bitcoin/bitcoin/blob/6d5790956f45e3de5c6c4ee6fda21878b0d1287b/test/functional/p2p_v2_transport.py#L154) gets called before disconnection is complete.


```
2023-12-05T13:26:56.774275Z [net] [net.cpp:1106] [ProcessReceivedKeyBytes] [net] V2 transport error: V1 peer with wrong MessageStart 0a03cf40
2023-12-05T13:26:56.857216Z [net] [net.cpp:554] [CloseSocketDisconnect] [net] disconnectin
...

💬 stratospher commented on pull request "test: fix v2 transport intermittent test failure (#29002)":
(https://github.com/bitcoin/bitcoin/pull/29006#discussion_r1416731520)
00e0658: wondering if we need to wait for previous disconnections here since the unrelated disconnections happen only with peers where we directly open the sockets to the node right? (tests above use TestNode) i like how the approach is consistent with the rest of the test though.

💬 maflcko commented on pull request "test: fix v2 transport intermittent test failure (#29002)":
(https://github.com/bitcoin/bitcoin/pull/29006#discussion_r1416885349)
nit: unrelated: Would be good to wait for CNode desctruction after disconnection by checking the getpeerinfo RPC. This is what I meant in https://github.com/bitcoin/bitcoin/issues/29002#issuecomment-1841702298

💬 maflcko commented on pull request "test: fix v2 transport intermittent test failure (#29002)":
(https://github.com/bitcoin/bitcoin/pull/29006#discussion_r1416891272)
nit: unrelated: Could have a `self.wait_until` to avoid having to pass the timeout factor every time?

💬 S3RK commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1842392872)
ACK ca09415e630f0f7de9160cab234bd5ba6968ff2d

👍 maflcko approved a pull request: "test: fix v2 transport intermittent test failure (#29002)"
(https://github.com/bitcoin/bitcoin/pull/29006#pullrequestreview-1766943717)
lgtm. Can't hurt to have this helper, because it also makes code less verbose

💬 maflcko commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1842408340)
lgtm test-was-added ACK 9075a446461ccbc446d21af778aac50b604f39b3

👍 MarnixCroes approved a pull request: "rpc: encryptwallet help, mention HD seed rotation and backup requirement"
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1766963845)
ack ca09415e630f0f7de9160cab234bd5ba6968ff2d

💬 stratospher commented on pull request "rpc: addpeeraddress tried return error on failure":
(https://github.com/bitcoin/bitcoin/pull/28998#issuecomment-1842434756)
Concept ACK. it's nice to clearly define success/failure paths for `addpeeraddress`. hopefully after #29007, these rare collisions won't happen since a fixed `nKey` can be used.

💬 maflcko commented on pull request "rpc: addpeeraddress tried return error on failure":
(https://github.com/bitcoin/bitcoin/pull/28998#issuecomment-1842440758)
Maybe just do https://github.com/bitcoin/bitcoin/pull/29007 and the close this one?

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1416964693)
Good catch, I think though this file also falls in the category of https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1408086308, and since it does not introduce any further potentially controversial headers, I'd rather drop this change altogether. This can be re-visited once/if we have some accounting of the headers.

💬 willcl-ark commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1842507850)
ack ca09415e630f0f7de9160cab234bd5ba6968ff2d

⚠️ badergithub opened an issue: "Hi"
(https://github.com/bitcoin/bitcoin/issues/29008)