Bitcoin Core Github
43 subscribers
122K links
Download Telegram
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.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1482000338)
In 0dba0b6de03c0c79bb09296b913729ec8eb75aca "wallet: bdb batch 'ErasePrefix', do not create txn internally"

Instead of `assert`ing, I would prefer if this did `if (!Assume(activeTxn)) return false` so we didn't cause nodes to crash if a transaction were missing for whatever reason. `Assume` should still catch this for developers.
💬 hernanmarino commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1482025759)
Will do.
🤔 achow101 reviewed a pull request: "wallet: batch and simplify addressbook migration process"
(https://github.com/bitcoin/bitcoin/pull/26836#pullrequestreview-1868662211)
ACK 1f177ff9a6ab229bd6486941e46daa92ab22b622
💬 achow101 commented on pull request "wallet: batch and simplify addressbook migration process":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1482012171)
In c154e0940b6518487e29de96eb24ecf1839bd3fe "refactor: SetAddrBookWithDB, signal only if write succeeded"

nit: `__func__` is not necessary
💬 achow101 commented on pull request "wallet: batch and simplify addressbook migration process":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1482012269)
In c154e0940b6518487e29de96eb24ecf1839bd3fe "refactor: SetAddrBookWithDB, signal only if write succeeded"

nit: `__func__` is not necessary
⚠️ realsetvin opened an issue: "Bitcoin ubuntu ppa, outdated"
(https://github.com/bitcoin/bitcoin/issues/29406)
### Please describe the feature you'd like to see added.

https://launchpad.net/~bitcoin/+archive/ubuntu/bitcoin

Last update, 2018. Ubuntu 24 is coming soon. Any plans to update this?

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_
📝 theuni opened a pull request: "RFC: build: remove confusing and inconsistent disable-asm option"
(https://github.com/bitcoin/bitcoin/pull/29407)
1. It didn't actually disable asm usage in our code. Regardless of the setting, asm is used in random.cpp and support/cleanse.cpp.
2. The value wasn't forwarded to libsecp as a user might have reasonably expected.
3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm actually did in practice.

If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new configure option that actually does what it says.

Additionally, this is one of the last (THE last?) re
...