Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 jarolrod approved a pull request: "release: Update translations for v27.0 soft translation string freeze"
(https://github.com/bitcoin/bitcoin/pull/29397#pullrequestreview-1868440624)
ACK 71927b24e5aceecd8a07cdaeb916898d45486bea

Pulled in update with the update-translations.py script and and updated the source files with `make -C src translate`, with a zero-diff
🚀 hebasto merged a pull request: "Change address / amount error background"
(https://github.com/bitcoin-core/gui/pull/553)
hebasto closed a pull request: "Bugfix: When restoring table columns, still set their minimum column width and stretch on last section"
(https://github.com/bitcoin-core/gui/pull/368)
💬 hebasto commented on pull request "Bugfix: When restoring table columns, still set their minimum column width and stretch on last section":
(https://github.com/bitcoin-core/gui/pull/368#issuecomment-1932610148)
Closing due to lack of activity.
💬 araujo88 commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1932611938)
> The issue is a race, so it is not deterministically reproducible. Usually, to deterministically reproduce it on all hardware, a sleep will have to be added in the right place in the source code or test code.
>
> A pull request will either have to explain where to put the sleep, or, if not possible, come up with a plausible explanation how the race could happen. Also, the pull request should explain if the bug is in the wallet code or in the test code, in which case the fix should be applied
...
📝 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.