💬 theStack commented on pull request "wallet: refactor: various master key encryption cleanups":
(https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1999779103)
The memory consumption of an empty vector is negligible (24 bytes on my system [1]) so this is not a big deal, but happy to address this on the next push.

[1] according to
```
std::vector<unsigned char> empty_vec;
printf("%ld\n", sizeof(empty_vec) + memusage::DynamicUsage(empty_vec));
```

💬 theStack commented on pull request "wallet: refactor: various master key encryption cleanups":
(https://github.com/bitcoin/bitcoin/pull/31398#discussion_r1999782628)
Good point, ignoring the return values of database seems to be a really bad idea. Should be addressed in a (non-refactor) followup.

💬 jstefanop commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2731123565)
> If your hardware regularly corrupts the database, the system is just too unreliable, and there is little that can be done long term. Even with a regular snapshot feature, it might even corrupt while taking a snapshot.

All non ECC hardware will corrupt eventually, so just a matter of where the line is drawn. We just see it more in real world because of the large number of nodes deployed on the network, and large timeframes for IBD (~ 1 week) (ie 2-4% of all nodes across thousands have a corrup
...

💬 mabu44 commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2731126667)
In some scenarios where the master branch returns a JSON with "success":false, the proposed code throws an error. I think changing RPC behavior without a specific reason should be avoided.
Example:
(in both cases the same descriptor was imported before using a wider [0, 1000] range)

#### On master
```bash
./bitcoin-cli importdescriptors '[{"desc": "wpkh(tprv8ZgxMBicQKsPebj9z5pERWfTpb4RSrqr9tnvGeNPvTV2iaHNdxz3kTcqMmmHabiXpk9DLKE6v24Q6mJwwaLYFLdjtyWs2wtRQUAo3xTGN7i/84h/1h/0h/0/*)#3v9ceyxw"
...

💬 jimhashhq commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2731137612)
RE multiprocess Bitcoin: Strong concept ACK!

I tried this multiprocess feature out (using cmake/clang/Linux) as follows:

- Full build multiprocess with gui of bitcoin/master (with CMAKE_PREFIX_PATH correctly pointing to a recent external local build of master on libmultiprocess).
- Full build multiprocess with gui of Sjors/02/25/ipc-yea (the multiprocess feature integration branch).
I can confirm that this latter build was indeed simpler with libmultiprocess as a project subtree.

Inte
...

💬 davidgumberg commented on pull request "net: Block v2->v1 transport downgrade if !fNetworkActive":
(https://github.com/bitcoin/bitcoin/pull/32073#discussion_r1999868812)
Not blocking, but this seemed slightly more correct to me before, since now if `fNetworkActive == false` when caching the result, but `true` when disconnecting, we might not attempt v1 reconnect when we should have.

Enumerating the possible states in the old version where `fNetworkActive` is checked "twice", first to decide whether or not we should flag all `m_nodes` for disconnect, and the "second time" where for each node flagged for disconnect, we check whether or not we should retry v1 be
...

🚀 fanquake merged a pull request: "doc: shallow clone `qa-assets`"
(https://github.com/bitcoin/bitcoin/pull/32083)

🚀 fanquake merged a pull request: "[29.x] backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/32062)

💬 fanquake commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2731374665)
`rc2` has been tagged: https://github.com/bitcoin/bitcoin/releases/tag/v29.0rc2.

💬 tnndbtc commented on pull request "test: fix intermittent failure in p2p_orphan_handling.py":
(https://github.com/bitcoin/bitcoin/pull/32063#issuecomment-2731389365)
@mzumsande Regarding to your concerns on `disconnect_p2ps`, I took a look at the implementation of it in `test/functional/test_framework/test_node.py`:

```
def disconnect_p2ps(self):
"""Close all p2p connections to the node.
Use only after each p2p has sent a version message to ensure the wait works."""
for p in self.p2ps:
p.peer_disconnect()
del self.p2ps[:]

self.wait_until(lambda: self.num_test_p2p_connections() == 0)
```


...

💬 davidgumberg commented on pull request "net: Block v2->v1 transport downgrade if !fNetworkActive":
(https://github.com/bitcoin/bitcoin/pull/32073#issuecomment-2731477125)
Tested and Reviewed ACK 6869fb417096b43ba7f7

I could be wrong, but I think https://github.com/bitcoin/bitcoin/commit/0f4e20728d231935f0140845370090a760ae93af was more correct (see https://github.com/bitcoin/bitcoin/pull/32073#discussion_r1999868812), but I don't think it's that important.

Anecdotally, more likely to occur on startup, probably because of how many new connections we're making, so I used this script to reproduce:
```bash
bitcoind -debug=net -daemonwait && sleep 20 \
&&
...

💬 davidgumberg commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#discussion_r2000090721)
Good catch, will address in a refactor follow-up, or if I touch here again.

🚀 fanquake merged a pull request: "ci: [lint] Use Cirrus dockerfile cache"
(https://github.com/bitcoin/bitcoin/pull/31948)

💬 davidgumberg commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2731548516)
> > The only case I'm aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.
>
> in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely.

Okay, I think this branch will break building on NetBSD, and shouldn't be merged as-is, I'll follow up.

💬 fanquake commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2731552311)
> I'll follow up.

Do you want to split out 7d34c19853e7a5528d69c5f30580e7e9712e61f0? That can be merged now.

📝 davidgumberg opened a pull request: "ci: Drop ENABLE_HARDENING=OFF from clang-tidy"
(https://github.com/bitcoin/bitcoin/pull/32087)
Split out from #32071

It's not clear why this was added in the first place, but it is not necessary currently.

https://github.com/bitcoin/bitcoin/pull/32038#issuecomment-2723888193 https://github.com/bitcoin/bitcoin/pull/24753.

💬 davidgumberg commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2731605465)
> Do you want to split out [7d34c19](https://github.com/bitcoin/bitcoin/commit/7d34c19853e7a5528d69c5f30580e7e9712e61f0)? That can be merged now.

Opened https://github.com/bitcoin/bitcoin/pull/32087

👍 fanquake approved a pull request: "ci: Drop ENABLE_HARDENING=OFF from clang-tidy"
(https://github.com/bitcoin/bitcoin/pull/32087#pullrequestreview-2692923853)
ACK 7d34c19853e7a5528d69c5f30580e7e9712e61f0

🚀 fanquake merged a pull request: "ci: Drop ENABLE_HARDENING=OFF from clang-tidy"
(https://github.com/bitcoin/bitcoin/pull/32087)

💬 fanquake commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r2000261713)
s/Bitcoin/Bitcoin Core/

> either on your local machine or through cross-compilation

I think this needs to be rephrased, or just dropped.