📝 theuni opened a pull request: "bitcoin-config.h includes cleanup"
(https://github.com/bitcoin/bitcoin/pull/29404)
As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it.

See also #26972 for discussion.

Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header.

I'm afraid it's a bit tedious
...

💬 theuni commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1932625675)
Ping @maflcko

This (or something like it) is a prerequisite for #29404.

There doesn't seem to be any sane way to make this a scripted diff or a clang-tidy transform, so it would be great if someone could reproduce my results.

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1481918788)
fixed

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1481919762)
good point, done something similar.

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1932645404)
I have addressed feedback and squashed several test fixes into the last commit, so that the "test each commit" CI is hopefully green now. I have an unsquashed version at https://github.com/mzumsande/bitcoin/tree/202401_bip324_alltests_unsquashed, in case that helps with review.

💬 glozow commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1932649179)
> In my understanding, this race condition arises from asynchronous block processing and transaction validation. Specifically, it occurs during the blockchain reorganization process when nodes are reconnected, and their blockchains are synchronized to establish a consensus chain. The asynchronous nature of this process introduces a timing issue where the state of transactions might not be fully updated before the test assertions are executed. This leads to a discrepancy in the expected outcome o
...

👋 mzumsande's pull request is ready for review: "test: use v2 everywhere for P2PConnection if --v2transport is enabled"
(https://github.com/bitcoin/bitcoin/pull/29358)

⚠️ vuccixx21 opened an issue: "Kalm m"
(https://github.com/bitcoin/bitcoin/issues/29405)

✅ fanquake closed an issue: "Kalm m"
(https://github.com/bitcoin/bitcoin/issues/29405)

:lock: fanquake locked an issue: "Kalm m"
(https://github.com/bitcoin/bitcoin/issues/29405)

💬 hernanmarino commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1481952473)
Reusing self.mini_wallet was my first approach but the following error is produced:
```
File "./test/functional/feature_assumeutxo.py", line 143, in test_invalid_mempool_state
tx = self.mini_wallet.send_self_transfer(from_node=node)
File "./test/functional/test_framework/wallet.py", line 251, in send_self_transfer
self.sendrawtransaction(from_node=from_node, tx_hex=tx['hex'])
File "./test/functional/test_framework/wallet.py", line 371, in sendrawtransaction
txid = from_nod
...

💬 araujo88 commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1932689628)
> > In my understanding, this race condition arises from asynchronous block processing and transaction validation. Specifically, it occurs during the blockchain reorganization process when nodes are reconnected, and their blockchains are synchronized to establish a consensus chain. The asynchronous nature of this process introduces a timing issue where the state of transactions might not be fully updated before the test assertions are executed. This leads to a discrepancy in the expected outcome
...

💬 ryanofsky commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1932707335)
re: https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1918947945

> On top of #29354 , the following diff should crash the node:

This is a great test, thank you! I added it to #29370 and it uncovered a new bug that PR introduced to the CheckBlockIndex code (forgetting to reset pindexFirstNeverProcessed after moving upwards from the snapshot node).

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1932712410)
re: https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1931513793

> Could include the test [#29370 (comment)](https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1927200215) , or did you want to leave that for later?

I had just forgotten about this. Added the test now, and fixed another bug uncovered by the test (forgetting to reset pindexFirstNeverProcessed after moving upwards from the snapshot block).

Updated 3f0b8607f549b27e9bb0cc8b189252a5cd954a36 -> 2f4e7e8dc82adb60
...

👍 hebasto approved a pull request: "Enable users to configure their monospace font specifically"
(https://github.com/bitcoin-core/gui/pull/497#pullrequestreview-1868593416)
ACK a17fd33edd1374145fd6986fbe352295355fde4f, I have reviewed the code and tested it on Ubuntu 22.04. This is a UX improvement. https://github.com/bitcoin-core/gui/pull/497#issuecomment-1341222673 might be addressed in a follow-up.

🚀 hebasto merged a pull request: "Enable users to configure their monospace font specifically"
(https://github.com/bitcoin-core/gui/pull/497)

👋 ryanofsky's pull request is ready for review: "assumeutxo: Get rid of faked nTx and nChainTx values"
(https://github.com/bitcoin/bitcoin/pull/29370)

💬 theuni commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1932746572)
Removed the include from `interfaces/wallet.h` which snuck in because of a comment:
```c++
//! function will be undefined in builds where ENABLE_WALLET is false.
```
That was the only example of a false-positive that I managed to find.

🤔 achow101 reviewed a pull request: "wallet: batch erase procedures and improve 'EraseRecords' performance"
(https://github.com/bitcoin/bitcoin/pull/29403#pullrequestreview-1868639990)
ACK 69c5bb5c3ad3da508541a3ef6c29a0bad21f2fc0

💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1481997602)
In 2d770f6ae88d9ee6f1775e011f241274059fe275 "wallet: clean redundancies in DelAddressBook"

I'd prefer to have these occur separately so that a distinct error can be logged for each, that way we can discover which operation failed.